Re: [PATCH v5 3/7] cxl/region: Generalize endpoint position mapping

[Alison Schofield <alison.schofield@intel.com>]

On Tue, Sep 08, 2026 at 12:42:07AM +0100, Jonathan Cameron wrote:
> On Thu,  3 Sep 2026 16:23:45 -0700
> Alison Schofield <alison.schofield@intel.com> wrote:
> 
> > Endpoint position calculation currently relies on the requirement that an
> > interleaving root have the same granularity as the region. Auto region
> > creation builds the position by multiplying by the parent decoders' ways,
> > while user region creation selects the root target with 'pos % ways'. Those
> > calculations are sufficient under the current granularity restriction.
> > 
> > In order to support mixed-granularity regions, that granularity restriction
> > will need to be removed so decoder granularity can change between levels of
> > the interleave hierarchy. The position calculation needs to account for
> > those changes to produce the correct endpoint ordering.
> > 
> > Change the position calculation so each decoder's contribution is weighted
> > by its granularity relative to the region granularity:
> > 
> >     position += target_pos *
> >                 (decoder_granularity / region_granularity)
> 
> In the code target_pos becomes parent_pos. The parent_pos naming
> seems more logical to me but maybe I'm missing something! (more
> that likely given it is interleave maths!)

Agree. I'll change the messaging above to be parent_pos.

-- Alison
> 
> > 
> > Use the same relationship to select the root target during user region
> > creation.
> > 
> > Weight each level by the region granularity, passed in by the caller,
> > rather than by the granularity programmed in the endpoint decoder. The two
> > are equal for most configurations, but not when Normalized Addressing
> > leaves the endpoint decoder programmed passthrough while the region
> > interleaves.
> > 
> > For currently supported regions, the new weighted calculation reduces to
> > the existing position calculation and produces identical endpoint
> > positions. The exception is a region wider than a same-granularity Mod3
> > root, where address bit routing programs the level below the root at the
> > region granularity, so the ratio derives a weight of one rather than three.
> > A later patch in this series rejects that layout.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> I'll assume you'll clarify the naming thing.  Otherwise this LGTM
> Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
> 
> > ---
> >  drivers/cxl/core/region.c | 69 ++++++++++++++++++++++++---------------
> >  1 file changed, 42 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 6a698f36aa6d..506b1cba1a92 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> 
> > @@ -1951,24 +1959,29 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
> >   * cxl_calc_interleave_pos() - calculate an endpoint position in a region
> >   * @cxled: endpoint decoder member of given region
> >   * @hpa_range: translated HPA range of the endpoint
> > + * @region_gran: interleave granularity of the region
> >   *
> > - * The endpoint position is calculated by traversing the topology from
> > - * the endpoint to the root decoder and iteratively applying this
> > - * calculation:
> > + * A region position is the index of a region-granularity chunk within one full
> > + * pass of the region interleave. The position is calculated by traversing the
> > + * topology from the endpoint to the root decoder and accumulating the
> > + * contribution of each decoder level:
> >   *
> > - *    position = position * parent_ways + parent_pos;
> > + *    position += parent_pos * (parent_granularity / region_gran);
> 
> Here is the naming difference from the patch description.
> 
> >   *
> > - * ...where @position is inferred from switch and root decoder target lists.
> > + * ...where @parent_pos is inferred from switch and root decoder target lists.
> > + * The multiplier is the weight of that level: how many region positions pass
> > + * between successive advances of the level's target index. A level that selects
> > + * a single target contributes nothing.
> >   *
> >   * Return: position >= 0 on success
> >   *	   -ENXIO on failure
> >   */
> >  static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled,
> > -				   struct range *hpa_range)
> > +				   struct range *hpa_range, int region_gran)
> >  {
> >  	struct cxl_port *iter, *port = cxled_to_port(cxled);
> >  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> > -	int parent_ways = 0, parent_pos = 0, pos = 0;
> > +	int parent_gran = 0, parent_pos = 0, pos = 0;
> >  	int rc;
> >