Re: [PATCH v4 2/6] cxl/region: Generalize endpoint position mapping

On Thu, Aug 20, 2026 at 04:31:20PM +0800, Alison Schofield wrote:
Hi Alison,

> 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 each parent
> decoder's 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)
>

This looks correct to me.
Please see the below comment, having a question there.
 
> Use the same relationship to select the root target during user region
> creation.
> 
> For currently supported regions, the new weighted calculation reduces
> to the existing position calculation and produces identical endpoint
> positions.
> 
> Signed-off-by: Alison Schofield <[email protected]>
> ---
>  drivers/cxl/core/region.c | 60 ++++++++++++++++++++++-----------------
>  1 file changed, 34 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 3b640c9ba5a0..4f367feaf6c8 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1805,9 +1805,14 @@ static int cxl_region_attach_position(struct cxl_region *cxlr,
>  	struct cxl_decoder *cxld = &cxlsd->cxld;
>  	int iw = cxld->interleave_ways;
>  	struct cxl_port *iter;
> -	int rc;
> +	int root_pos = pos, rc;
>  
> -	if (dport != cxlrd->cxlsd.target[pos % iw]) {
> +	/* Root target selection advances at root-granularity intervals */
> +	if (iw > 1)
> +		root_pos = pos * cxlr->params.interleave_granularity /
> +			   cxld->interleave_granularity;
> +
> +	if (dport != cxlrd->cxlsd.target[root_pos % iw]) {
>  		dev_dbg(&cxlr->dev, "%s:%s invalid target position for %s\n",
>  			dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
>  			dev_name(&cxlrd->cxlsd.cxld.dev));
> @@ -1908,13 +1913,13 @@ static int match_switch_decoder_by_range(struct device *dev,
>  	return (r1->start == r2->start && r1->end == r2->end);
>  }
>  
> -static int find_pos_and_ways(struct cxl_port *port, struct range *range,
> -			     int *pos, int *ways)
> +static int find_pos_and_gran(struct cxl_port *port, struct range *range,
> +			     int *pos, int *gran)
>  {
>  	struct cxl_switch_decoder *cxlsd;
>  	struct cxl_port *parent;
>  	struct device *dev;
> -	int rc = -ENXIO;
> +	int ways, rc = -ENXIO;
>  
>  	parent = parent_port_of(port);
>  	if (!parent)
> @@ -1929,9 +1934,10 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  		return rc;
>  	}
>  	cxlsd = to_cxl_switch_decoder(dev);
> -	*ways = cxlsd->cxld.interleave_ways;
> +	ways = cxlsd->cxld.interleave_ways;
> +	*gran = cxlsd->cxld.interleave_granularity;
>  
> -	for (int i = 0; i < *ways; i++) {
> +	for (int i = 0; i < ways; i++) {
>  		if (cxlsd->target[i] == port->parent_dport) {
>  			*pos = i;
>  			rc = 0;
> @@ -1955,13 +1961,16 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>   * @cxled: endpoint decoder member of given region
>   * @hpa_range: translated HPA range of the endpoint
>   *
> - * The endpoint position is calculated by traversing the topology from
> - * the endpoint to the root decoder and iteratively applying this
> - * calculation:
> + * The endpoint 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_granularity);
>   *
> - * ...where @position is inferred from switch and root decoder target lists.
> + * ...where @parent_pos is inferred from switch and root decoder target
> + * lists, and the multiplier is the number of region positions that the
> + * level's granularity spans. A level that selects a single target
> + * contributes nothing.
>   *
>   * Return: position >= 0 on success
>   *	   -ENXIO on failure
> @@ -1971,7 +1980,8 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled,
>  {
>  	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 gran = cxled->cxld.interleave_granularity;
> +	int parent_gran = 0, parent_pos = 0, pos = 0;
>  	int rc;
>  

The "gran" here is using EP decoder's granularity, I think maybe th region
granularity is correct ?

They're normally equal, but in the scenario of normalized-addressing auto regions
they're not.
In that case the EP decoder stays in passthrough mode, for example 1W1/IG256,
while ccxl_prm_setup_root() discover a translated region at IW2/IG4K

For root target 1, this code calculates
1 * (4096/256) = 16

The correct region position is 1, a 2-way region only has pos 0 and 1.

I saw in v3 the regrion granularity was passed into this function explicitly,
maybe I know the reason to make this change if I am missing anything there.

Best regards,
Richard Cheng.

>  	/*
> @@ -1984,20 +1994,18 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled,
>  	 *        |    |           |    |
>  	 *       mem0 mem1        mem2 mem3
>  	 *
> -	 * In the example the calculator will iterate twice. The first iteration
> -	 * uses the mem position in the host-bridge and the ways of the host-
> -	 * bridge to generate the first, or local, position. The second
> -	 * iteration uses the host-bridge position in the root_port and the ways
> -	 * of the root_port to refine the position.
> +	 * The region and the root decoder interleave at granularity g, so
> +	 * each host-bridge decoder interleaves at 2g and spans two region
> +	 * positions while the root decoder spans one.
>  	 *
>  	 * A trace of the calculation per endpoint looks like this:
> -	 * mem0: pos = 0 * 2 + 0    mem2: pos = 0 * 2 + 0
> -	 *       pos = 0 * 2 + 0          pos = 0 * 2 + 1
> +	 * mem0: pos += 0 * 2       mem2: pos += 0 * 2
> +	 *       pos += 0 * 1             pos += 1 * 1
>  	 *       pos: 0                   pos: 1
>  	 *
> -	 * mem1: pos = 0 * 2 + 1    mem3: pos = 0 * 2 + 1
> -	 *       pos = 1 * 2 + 0          pos = 1 * 2 + 1
> -	 *       pos: 2                   pos = 3
> +	 * mem1: pos += 1 * 2       mem3: pos += 1 * 2
> +	 *       pos += 0 * 1             pos += 1 * 1
> +	 *       pos: 2                   pos: 3
>  	 *
>  	 * Note that while this example is simple, the method applies to more
>  	 * complex topologies, including those with switches.
> @@ -2008,12 +2016,12 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled,
>  		if (is_cxl_root(iter))
>  			break;
>  
> -		rc = find_pos_and_ways(iter, hpa_range, &parent_pos,
> -				       &parent_ways);
> +		rc = find_pos_and_gran(iter, hpa_range, &parent_pos,
> +				       &parent_gran);
>  		if (rc)
>  			return rc;
>  
> -		pos = pos * parent_ways + parent_pos;
> +		pos += parent_pos * (parent_gran / gran);
>  	}
>  
>  	dev_dbg(&cxlmd->dev,
> -- 
> 2.37.3
> 
>