Re: [PATCH v3 3/9] cxl/region: Derive port granularity from selector bits

On Tue, Aug 18, 2026 at 12:01:11PM +0200, Robert Richter wrote:
> On 30.07.26 15:20:23, Alison Schofield wrote:
> > A user-created region currently derives each port decoder granularity
> > from the parent granularity and ways. That recurrence assumes the
> > selector bits follow the existing same-granularity ordering and cannot
> > derive the decoder settings for a mixed-granularity region.
> > 
> > Add cxl_region_is_mixed_gran() as the common predicate for regions
> > whose granularity is finer than an interleaving root decoder.
> > 
> > Update derive_port_granularity() to choose each port decoder's
> > granularity from the selector bits not already used by its ancestors.
> > For auto regions, compare the firmware-programmed granularity with the
> > derived value and reject a mismatch.
> 
> The only requirement I see here is that firmware programmed selector
> bits must be within the endpoint's bit mask (which matches the
> region's total ways and gran configuration). The selector and
> granularity of a single decoder within the chain may vary, as long as
> it is within the total selector and does not overlap with other
> decoders. It is not possible to calculate a single correct granularity
> value for auto-mode. And in user-mode there are muliple setups and
> values possible too, unless there are strict assignment rules given
> that only allow a single subset.
> 

Thanks for the review Robert,

I agree with the general point that the CXL Specification permits
multiple selector/granularity arrangements, so there is not one
universally correct decoder granularity across all legal layouts.

v4 makes the support restriction explicit instead of trying to model
all of those layouts. Linux supports only monotonic coarse-to-fine
mixed-granularity configurations. Within that subset, the parent
interleave geometry and the decoder target count determine the child
granularity, so the auto path can validate firmware against that
supported geometry.

The 3-, 6-, and 12-way configurations also need the mod3 contribution
accounted for as part of that geometry, rather than treating the whole
problem as power-of-two selector placement.


> > 
> > Originally-by: Robert Richter <[email protected]>
> > Signed-off-by: Alison Schofield <[email protected]>
> > ---
> >  drivers/cxl/core/region.c | 90 ++++++++++++++++++++++++++++++++++-----
> >  1 file changed, 79 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index b0061f03892a..871dedd37cc8 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1552,32 +1552,94 @@ static bool region_selectors_fit(struct cxl_port *port,
> >  	return true;
> >  }
> >  
> > +/**
> > + * cxl_region_is_mixed_gran() - Test for a mixed-granularity region
> > + * @cxlr: region
> > + *
> > + * A region is mixed-granularity when an interleaving root decoder uses a
> > + * larger granularity than the region.
> > + *
> > + * Return: true for a mixed-granularity region.
> > + */
> > +static inline bool cxl_region_is_mixed_gran(struct cxl_region *cxlr)
> > +{
> > +	struct cxl_decoder *cxld = &cxlr->cxlrd->cxlsd.cxld;
> > +
> > +	return cxld->interleave_ways > 1 &&
> > +	       cxld->interleave_granularity > cxlr->params.interleave_granularity;
> > +}
> > +
> >  /**
> >   * derive_port_granularity() - Calculate the granularity for a port decoder
> > + * @port: port being configured
> >   * @cxlr: region under construction
> > + * @accum: selectors used by the ancestor decoders
> >   * @fanout: product of the ancestor switch ways
> > - * @ig: filled with the port decoder granularity
> > + * @iw: port decoder interleave ways
> > + * @ig: filled with the derived granularity
> >   *
> > - * Preserve the existing parent-granularity times parent-ways recurrence in
> > - * terms of the region granularity and the fan-out above this port.
> > + * Select the decoder granularity from the region selector bits not already
> > + * used by its ancestors. Same-granularity regions allocate the lowest
> > + * available selector bits first. Mixed-granularity regions allocate the
> > + * highest available selector bits first so decoder granularities decrease
> > + * from the root toward the endpoints.
> >   *
> > - * Return: 0 on success.
> > + * A passthrough decoder uses no selector bits and retains the granularity
> > + * implied by the ancestor fan-out.
> > + *
> > + * Return: 0 on success, -ENXIO when no valid selector remains.
> >   */
> > -static int derive_port_granularity(struct cxl_region *cxlr, int fanout,
> > -				   int *ig)
> > +static int derive_port_granularity(struct cxl_port *port,
> > +				   struct cxl_region *cxlr, u64 accum,
> > +				   int fanout, int iw, int *ig)
> 
> This function interface gets really out of control now. You need 5
> args to calc the granularity and return 2 values? Something is wrong
> here.
> 
> I guess we need to change the approach here. Maybe split target setup
> for auto- and user-mode. We must simplify the code, I think that is
> possible.
> 
> Will take a look.


Agreed. This comment, along with the broader complexity concern you
raised in the collab, stopped me from trying to simplify the v3 helper
structure and rework the series from the existing region setup flow.

v4 drops derive_port_granularity(), the selector accumulation, and the
fanout state entirely. The target setup stays shared between auto and
user regions, but the local decoder geometry is now derived directly
from the parent granularity and target count.

The target setup stays shared between auto and user regions

> 
> -Robert
> 
snip