Re: [PATCH v3 1/2] cxl/region: Allow passthrough decoders with >16K granularity

On Fri, Jul 24, 2026 at 06:08:54PM +0800, Alison Schofield wrote:
> Region configuration rejects valid topologies that contain a
> passthrough decoder beneath a wide parent interleave.
> 
> For example, a passthrough switch below an 8-way root decoder with
> 4K granularity computes a 32K granularity. That exceeds the maximum
> encodable value of the HDM Decoder Control IG field, causing region
> setup to fail even though a non-interleaving decoder does not consume
> the IG field.
> 
> Only require the granularity to be encodable for interleaving decoders,
> both where it is inherited from the parent and where it is computed for
> the current decoder. Keep the computed value for passthrough decoders so
> it can seed descendant decoder setup.
> 
> When committing a non-interleaving decoder, still program the granularity
> if it is encodable; use a don't-care IG encoding only when it is not. This
> preserves an encodable passthrough granularity across re-enumeration, so
> descendants do not inherit a stale value.
> 
> As a consequence, the interleave_granularity attribute of a decoder
> whose interleave_ways is 1 may report a value above 16K. Document that
> the reported granularity for non-interleaving decoders is a don't-care
> value that may exceed the maximum encodable in hardware.
> 
> Fixes: 18f35dc9314d ("cxl/region: Refactor granularity select in cxl_port_setup_targets()")
> Suggested-by: Sashiko AI Review <[email protected]>
> Assisted-by: Claude:Opus-4-8
> Signed-off-by: Alison Schofield <[email protected]>
> ---
>  Documentation/ABI/testing/sysfs-bus-cxl |  5 ++-
>  drivers/cxl/core/hdm.c                  |  8 ++++-
>  drivers/cxl/core/region.c               | 47 +++++++++++++++----------
>  3 files changed, 39 insertions(+), 21 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-bus-cxl b/Documentation/ABI/testing/sysfs-bus-cxl
> index 16a9b3d2e2c0..3f96e5bf1fd3 100644
> --- a/Documentation/ABI/testing/sysfs-bus-cxl
> +++ b/Documentation/ABI/testing/sysfs-bus-cxl
> @@ -407,7 +407,10 @@ Description:
>  		space this decoder claims at address N before the decode rotates
>  		to the next target in the interleave at address N +
>  		interleave_granularity (assuming N is aligned to
> -		interleave_granularity).
> +		interleave_granularity). When 'interleave_ways' is 1, the
> +		decoder does not interleave and the reported granularity is
> +		a don't-care value that may exceed the maximum encodable in
> +		hardware.
>  
>  
>  What:		/sys/bus/cxl/devices/decoderX.Y/create_{pmem,ram}_region
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b..d2ada82fe3c7 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -691,7 +691,13 @@ static void cxld_set_interleave(struct cxl_decoder *cxld, u32 *ctrl)
>  	if (WARN_ONCE(ways_to_eiw(cxld->interleave_ways, &eiw),
>  		      "invalid interleave_ways: %d\n", cxld->interleave_ways))
>  		return;
> -	if (WARN_ONCE(granularity_to_eig(cxld->interleave_granularity, &eig),
> +
> +	/*
> +	 * A non-interleaving decoder ignores the IG field, so an
> +	 * unencodable granularity is a don't-care rather than a failure.
> +	 */
> +	if (granularity_to_eig(cxld->interleave_granularity, &eig) &&
> +	    WARN_ONCE(cxld->interleave_ways > 1,
>  		      "invalid interleave_granularity: %d\n",
>  		      cxld->interleave_granularity))
>  		return;
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 1e211542b6b6..5a443551288c 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1521,12 +1521,20 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		parent_iw = parent_cxld->interleave_ways;
>  	}
>  
> -	rc = granularity_to_eig(parent_ig, &peig);
> -	if (rc) {
> -		dev_dbg(&cxlr->dev, "%s:%s: invalid parent granularity: %d\n",
> -			dev_name(parent_port->uport_dev),
> -			dev_name(&parent_port->dev), parent_ig);
> -		return rc;
> +	/*
> +	 * A non-interleaving parent does not encode its granularity, so its
> +	 * stored value may exceed the maximum encodable and need not be
> +	 * validated here.
> +	 */
> +	if (parent_iw > 1) {
> +		rc = granularity_to_eig(parent_ig, &peig);
> +		if (rc) {
> +			dev_dbg(&cxlr->dev,
> +				"%s:%s: invalid parent granularity: %d\n",
> +				dev_name(parent_port->uport_dev),
> +				dev_name(&parent_port->dev), parent_ig);
> +			return rc;
> +		}
>  	}
>  
>  	rc = ways_to_eiw(parent_iw, &peiw);
> @@ -1549,20 +1557,21 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  	 * Interleave granularity is a multiple of @parent_port granularity.
>  	 * Multiplier is the parent port interleave ways.
>  	 */
> -	rc = granularity_to_eig(parent_ig * parent_iw, &eig);
> -	if (rc) {
> -		dev_dbg(&cxlr->dev,
> -			"%s: invalid granularity calculation (%d * %d)\n",
> -			dev_name(&parent_port->dev), parent_ig, parent_iw);
> -		return rc;
> -	}
> +	ig = parent_ig * parent_iw;
>  
> -	rc = eig_to_granularity(eig, &ig);
> -	if (rc) {
> -		dev_dbg(&cxlr->dev, "%s:%s: invalid interleave: %d\n",
> -			dev_name(port->uport_dev), dev_name(&port->dev),
> -			256 << eig);
> -		return rc;
> +	/*
> +	 * Keep the computed granularity for descendant setup. Only
> +	 * interleaving decoders require an encodable granularity.
> +	 */
> +	if (iw > 1) {
> +		rc = granularity_to_eig(ig, &eig);
> +		if (rc) {
> +			dev_dbg(&cxlr->dev,
> +				"%s: invalid granularity calculation (%d * %d)\n",
> +				dev_name(&parent_port->dev), parent_ig,
> +				parent_iw);
> +			return rc;
> +		}
>  	}
>

Hi Alison,

Looks sane to me, just that this check pattern appears twice and peig
and eig are both write-only afterwards.
Could they be formed into a helper or macro ?

Something like the following might do ?
"""
static bool granularity_is_encodable(int ways, int granularity)
{
    u16 eig;
    if (ways <= 1)
        return true;

    return granularity_to_eig(granularity, &eig) == 0;
}

if (!granularity_is_encodable(parent_iw, parent_ig) {
    dev_dbg();
    return -EINVAL;
}
"""

Reviewed-by: Richard Cheng <[email protected]>

Best regards,
Richard Cheng.

>  	if (iw > 8 || iw > cxlsd->nr_targets) {
> -- 
> 2.37.3
> 
>