Re: [PATCH v4 3/6] cxl/region: Support mixed-granularity auto regions

On Thu, Aug 20, 2026 at 11:43:19PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Incorrect distance calculation for non-mixed-granularity regions with non-power-of-2 root interleaves causes a NULL pointer dereference.
> --

The arithmetic observation is correct. For a modulo-3 root, the formula
does not account for the factor of three in the positional spacing.

However, that case cannot reach check_last_peer(). A same-granularity
region below a modulo-3 root must match the root interleave width, which
places one region position per host bridge. No dport is shared, so
check_last_peer() is not called. The mixed-granularity cases that can
share a dport take the distance = 1 path instead.

Rather than add arithmetic for an unreachable case, I will add a comment
to v5 documenting that invariant.

Separately, the report exposed that a missing peer mapping is
dereferenced without a NULL check. Posting a standalone patch to
guard against that so a future violation fails region creation rather
than causing an oops.

-- Alison



> 
> --- Patch [3]: [PATCH v4 3/6] cxl/region: Support mixed-granularity auto regions ---
> commit 13bc48fe2efef1744b5bbf29c756bd07f400ebe3
> Author: Alison Schofield <[email protected]>
> 
> cxl/region: Support mixed-granularity auto regions
> 
> The CXL driver currently assumes the interleaving root has the same
> granularity as the region, with each successive interleaving level
> becoming coarser toward the endpoints. This patch allows the CXL driver
> to assemble an auto region where the root granularity is coarser than
> the region granularity.
> 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 4f367feaf6c84..637d1b60a570b 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> [ ... ]
> > @@ -1463,26 +1472,21 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> >  	}
> >  
> >  	cxlsd = to_cxl_switch_decoder(&cxld->dev);
> > +	iw = cxl_rr->nr_targets;
> > +
> >  	if (cxl_rr->nr_targets_set) {
> > -		int i, distance = 1;
> > -		struct cxl_region_ref *cxl_rr_iter;
> > +		int i, distance;
> >  
> >  		/*
> > -		 * The "distance" between peer downstream ports represents which
> > -		 * endpoint positions in the region interleave a given port can
> > -		 * host.
> > -		 *
> > -		 * For example, at the root of a hierarchy the distance is
> > -		 * always 1 as every index targets a different host-bridge. At
> > -		 * each subsequent switch level those ports map every Nth region
> > -		 * position where N is the width of the switch == distance.
> > +		 * @distance is the spacing between region positions sharing
> > +		 * this dport. Mixed-granularity regions place those positions
> > +		 * contiguously.
> >  		 */
> > -		do {
> > -			cxl_rr_iter = cxl_rr_load(iter, cxlr);
> > -			distance *= cxl_rr_iter->nr_targets;
> > -			iter = to_cxl_port(iter->dev.parent);
> > -		} while (!is_cxl_root(iter));
> > -		distance *= cxlrd->cxlsd.cxld.interleave_ways;
> > +		if (cxl_region_is_mixed_gran(cxlr))
> > +			distance = 1;
> > +		else
> > +			distance = cxld->interleave_granularity * iw /
> > +				   p->interleave_granularity;
> 
> [Severity: High]
> Does this mathematical distance calculation correctly handle non-power-of-2
> root interleaves?
> 
> When configuring a CXL region with a non-power-of-2 root interleave (like
> 3-way or 6-way) and multiple endpoints sharing a downstream port, this
> formula seems to calculate a distance 3 times smaller than the true
> topological stride. This appears to happen because parent_iw is divided
> by 3 for address routing, which scales down cxld->interleave_granularity.
> 
> If the distance is undersized, check_last_peer() will check an endpoint on
> a completely different host bridge:
> 
> drivers/cxl/core/region.c:check_last_peer() {
>         ...
>         cxled_peer = p->targets[pos - distance];
>         cxlmd_peer = cxled_to_memdev(cxled_peer);
>         ep_peer = cxl_ep_load(port, cxlmd_peer);
>         if (ep->dport != ep_peer->dport) {
>         ...
> }
> 
> Since cxl_ep_load() returns NULL when selecting a peer on a different host
> bridge, will ep_peer->dport trigger a NULL pointer dereference here?
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3