Re: [PATCH v4 1/2] cxl/region: Allow regions with an unencodable passthrough granularity

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] During region auto-discovery, the driver relaxes the interleave granularity validation for non-interleaving passthrough decoders, but fails to update the decoder's software struct (`cxld->interleave_granularity`) with the computed logical value. As a result, descendant decoders inherit a stale hardware don't-care value.
--

commit 581b6f11f4646ac877c9eaf9fb81a155a40889d5
Author: Alison Schofield <alison.schofield@intel.com>

cxl/region: Allow regions with an unencodable passthrough granularity

Region creation fails when the interleave granularity accumulated below the
root decoder exceeds 16K, the maximum encodable in the HDM Decoder Control
IG field, even when the decoder that computes it does not interleave.

This patch keeps the computed value for passthrough decoders so it can
seed descendant decoder setup.

Closes: https://lore.kernel.org/all/20260530002613.682E71F00893@smtp.kernel.org/

> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 27e63e6dab7c8..89693f0181ddb 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

[ ... ]

> @@ -1550,20 +1562,12 @@ 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) {
> +	ig = parent_ig * parent_iw;
> +	if (!granularity_is_valid(iw, ig)) {
>  		dev_dbg(&cxlr->dev,
>  			"%s: invalid granularity calculation (%d * %d)\n",
>  			dev_name(&parent_port->dev), parent_ig, parent_iw);
> -		return rc;
> -	}
> -
> -	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;
> +		return -EINVAL;
>  	}
>  
>  	if (iw > 8 || iw > cxlsd->nr_targets) {

[Severity: High]
During region auto-discovery, the driver relaxes the interleave granularity
validation for non-interleaving passthrough decoders. However, looking at the
CXL_REGION_F_AUTO block immediately following this diff in
drivers/cxl/core/region.c:cxl_port_setup_targets():

	if (test_bit(CXL_REGION_F_AUTO, &cxlr->flags)) {
		if (cxld->interleave_ways != iw ||
		    (iw > 1 && cxld->interleave_granularity != ig) ||
		    !spa_maps_hpa(p, &cxld->hpa_range) ||
		    ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)) {
			dev_err(&cxlr->dev,

When iw == 1, does this code fail to update cxld->interleave_granularity
with the computed logical ig?

If cxld->interleave_granularity is not updated, it seems descendant decoders
will inherit a stale hardware don't-care value instead of the true accumulated
granularity. Could this cause the kernel to accept physically broken topologies
during auto-discovery, potentially leading to silent memory corruption or
Machine Check Exceptions (MCEs) when the region is accessed?

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