Re: [PATCH v4 04/14] cxl/region: Add @hpa_range argument to function cxl_calc_interleave_pos()

Dave Jiang
2025-11-03T21:52:03Z

On 11/3/25 11:47 AM, Robert Richter wrote:
> cxl_calc_interleave_pos() uses the endpoint decoder's HPA range to
> determine its interleaving position. This requires the endpoint
> decoders to be an SPA, which is not the case for systems that need
> address translation.
>
> Add a separate @hpa_range argument to function
> cxl_calc_interleave_pos() to specify the address range. Now it is
> possible to pass the SPA translated address range of an endpoint
> decoder to function cxl_calc_interleave_pos().
>
> Refactor only, no functional changes.
>
> Patch is a prerequisite to implement address translation.
>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>> ---
> drivers/cxl/core/region.c | 13 +++++++------
> 1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index bb889c891cf7..d3557d9d5b0f 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1845,11 +1845,11 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
> * Return: position >= 0 on success
> * -ENXIO on failure
> */
> -static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
> +static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled,
> + struct range *hpa_range)
> {
> struct cxl_port *iter, *port = cxled_to_port(cxled);
> struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> - struct range *range = &cxled->cxld.hpa_range;
> int parent_ways = 0, parent_pos = 0, pos = 0;
> int rc;
>
> @@ -1887,7 +1887,8 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
> if (is_cxl_root(iter))
> break;
>
> - rc = find_pos_and_ways(iter, range, &parent_pos, &parent_ways);
> + rc = find_pos_and_ways(iter, hpa_range, &parent_pos,
> + &parent_ways);
> if (rc)
> return rc;
>
> @@ -1897,7 +1898,7 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
> dev_dbg(&cxlmd->dev,
> "decoder:%s parent:%s port:%s range:%#llx-%#llx pos:%d\n",
> dev_name(&cxled->cxld.dev), dev_name(cxlmd->dev.parent),
> - dev_name(&port->dev), range->start, range->end, pos);
> + dev_name(&port->dev), hpa_range->start, hpa_range->end, pos);
>
> return pos;
> }
> @@ -1910,7 +1911,7 @@ static int cxl_region_sort_targets(struct cxl_region *cxlr)
> for (i = 0; i < p->nr_targets; i++) {
> struct cxl_endpoint_decoder *cxled = p->targets[i];
>
> - cxled->pos = cxl_calc_interleave_pos(cxled);
> + cxled->pos = cxl_calc_interleave_pos(cxled, &cxlr->hpa_range);
> /*
> * Record that sorting failed, but still continue to calc
> * cxled->pos so that follow-on code paths can reliably
> @@ -2094,7 +2095,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> struct cxl_endpoint_decoder *cxled = p->targets[i];
> int test_pos;
>
> - test_pos = cxl_calc_interleave_pos(cxled);
> + test_pos = cxl_calc_interleave_pos(cxled, &cxlr->hpa_range);
> dev_dbg(&cxled->cxld.dev,
> "Test cxl_calc_interleave_pos(): %s test_pos:%d cxled->pos:%d\n",
> (test_pos == cxled->pos) ? "success" : "fail",