Re: [PATCH 2/6] cxl/region: Derive port granularity from selector bits

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Passthrough decoders (`iw == 1`) in deep topologies fail to configure because calculated `ig` exceeds software limits, breaking valid configurations.
--

commit 0b56c23b98964cba7ba9487773eeb9e989d957d3
Author: Alison Schofield <alison.schofield@intel.com>

cxl/region: Derive port granularity from selector bits

Selector-bit validation uses a single same-granularity formula for
user-created regions. This commit changes the layout rules to derive
the programmed port granularity from selector bits, which lets an
interleaving switch beneath a mixed-granularity root claim inner
selector bits instead of colliding.

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 87fb1a9f2683f..9d310287d48dc 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1548,7 +1548,23 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		goto add_target;
>  	}
>  
> -	ig = p->interleave_granularity * parent_distance;
> +	/*
> +	 * Auto regions keep the firmware value, passthrough decoders consume
> +	 * no selector bits, interleaving decoders claim the lowest available
> +	 * selector bit.
> +	 */
> +	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
> +		ig = cxld->interleave_granularity;
> +	} else if (iw == 1) {
> +		ig = p->interleave_granularity * parent_distance;

[Severity: High]
This is a pre-existing issue, but does this calculation break passthrough
decoders in deep topologies?

If we have a large root interleave width (e.g., an 8-way root),
parent_distance evaluates to 8. For a standard 4K base region granularity,
ig is calculated as 4096 * 8 = 32768.

Later in cxl_port_setup_targets(), we unconditionally call:

	rc = ways_to_eiw(iw, &eiw);
	if (!rc)
		rc = granularity_to_eig(ig, &eig);
	if (rc) {
		...
		return rc;
	}

And granularity_to_eig() explicitly checks for `granularity > SZ_16K`
returning -EINVAL. Since hardware ignores interleave granularity for
passthrough decoders (iw == 1), this seems to artificially reject valid
hardware topologies by failing the configuration.

> +	} else if (selector) {
> +		ig = 1ULL << __ffs64(selector);
> +	} else {
> +		dev_dbg(&cxlr->dev,
> +			"%s:%s: no selector bits available for iw %d\n",
> +			dev_name(port->uport_dev), dev_name(&port->dev), iw);
> +		return -ENXIO;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1780095671.git.alison.schofield@intel.com?part=2