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

[Jonathan Cameron <jic23@kernel.org>]

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!)

> 
> 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;
>