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

On Sat, Jul 11, 2026 at 08:15:07PM +0000, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] During auto-discovery (CXL_REGION_F_AUTO), passthrough decoders fail to update their `interleave_granularity` software state to the computed value, causing downstream decoders to inherit incorrect granularity and fail validation upon reboot.
> --
> 
> --- Patch [1]: [PATCH v2 1/2] cxl/region: Allow passthrough decoders with >16K granularity ---
> commit 9e2526342188a97e338bfc63d454c253f1e2b3ef
> Author: Alison Schofield <[email protected]>
> 
> cxl/region: Allow passthrough decoders with >16K granularity
> 
> This commit allows passthrough decoders beneath wide parent interleaves
> to have a granularity exceeding 16K by programming a don't-care value
> in hardware for non-interleaving decoders.
> 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > index 0c80b76a5f9b4..d164662668c19 100644
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> > @@ -691,9 +691,16 @@ 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),
> > -		      "invalid interleave_granularity: %d\n",
> > -		      cxld->interleave_granularity))
> > +
> > +	/*
> > +	 * A non-interleaving decoder ignores the IG field. Encode a
> > +	 * don't-care value instead of validating the stored granularity.
> > +	 */
> > +	if (cxld->interleave_ways == 1)
> > +		eig = 0;
> > +	else if (WARN_ONCE(granularity_to_eig(cxld->interleave_granularity, &eig),
> > +			   "invalid interleave_granularity: %d\n",
> > +			   cxld->interleave_granularity))
> >  		return;
> >  
> >  	u32p_replace_bits(ctrl, eig, CXL_HDM_DECODER0_CTRL_IG_MASK);
> 
> [ ... ]
> 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 1e211542b6b64..5a443551288c4 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> 
> [ ... ]
> 
> > @@ -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;
> > +		}
> >  	}
> >  
> >  	if (iw > 8 || iw > cxlsd->nr_targets) {
> 
> [Severity: High]
> Does this code break region auto-discovery on reboot for topologies
> containing a passthrough decoder?
> 
> When cxld_set_interleave() configures a passthrough decoder, it now sets
> eig = 0 (a don't-care value) in the hardware.
> 
> Upon reboot, reading this hardware state will initialize the decoder's
> software interleave_granularity to a stale value.
> 
> During auto-discovery in cxl_port_setup_targets(), the CXL_REGION_F_AUTO
> path skips validating the granularity for passthrough decoders (iw == 1),
> but it appears to be missing an update to correct the software state:
> 
> 	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)) {
> 			...
> 			return -ENXIO;
> 		}
> 		/* missing update: cxld->interleave_granularity = ig; */
> 	}
> 
> If cxld->interleave_granularity is not updated to the computed ig here, won't
> any interleaving descendants read the stale parent_ig, compute a mismatched
> expected granularity, and fail validation?

In preparing v3 I found I didn't reply to this last piece.
It is real for the encodable case. My v2 clobbered it and read back
state. I've fixed it in v3 at the encode side, so F_AUTO has nothing
to restore.

-- Alison



> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1