Re: [PATCH v2 05/15] cxl/region: Calculate and store the SPA range of an endpoint

[Jonathan Cameron <Jonathan.Cameron@huawei.com>]

On Tue, 18 Feb 2025 14:23:46 +0100
Robert Richter <rrichter@amd.com> wrote:

> To find the correct region and root port of an endpoint of a system
> needing address translation, the endpoint's HPA range must be
> translated to each of the parent port address ranges up to the root
> decoder.
> 
> Calculate the SPA range using the newly introduced callback function
> port->to_hpa() that translates the decoder's HPA range to its parent
> port's HPA range of the next outer memory domain. Introduce the helper
> function cxl_port_calc_hpa() for this to calculate address ranges
> using the low-level port->to_hpa() callbacks. Determine the root port
> SPA range by iterating all the ports up to the root. Store the
> endpoint's SPA range for later use.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/region.c | 81 ++++++++++++++++++++++++++++++++-------
>  drivers/cxl/cxl.h         |  1 +
>  2 files changed, 68 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6f106bfa115f..d898c9f51113 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

> +
>  static int match_auto_decoder(struct device *dev, const void *data)
>  {
>  	const struct cxl_region_params *p = data;
> @@ -1882,6 +1920,11 @@ static int cxl_calc_interleave_pos(struct cxl_endpoint_decoder *cxled)
>  		.hpa_range = &cxled->cxld.hpa_range,
>  	};
>  
> +	/*
> +	 * Address translation is only supported for auto-discovery of
> +	 * decoders. There is no need to support address translation
> +	 * here. That is, do not recalculate ctx.hpa_range here.
> +	 */
>  	for (iter = cxled_to_port(cxled); pos >= 0 && iter;
>  	     iter = parent_port_of(iter))
>  		pos = cxl_port_calc_interleave(iter, &ctx);
> @@ -3262,7 +3305,8 @@ static int cxl_endpoint_decoder_initialize(struct cxl_endpoint_decoder *cxled)
>  {
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	struct cxl_port *iter = cxled_to_port(cxled);
> -	struct cxl_decoder *root, *cxld = &cxled->cxld;
> +	struct cxl_port *parent = parent_port_of(iter);
> +	struct cxl_decoder *cxld = &cxled->cxld;
>  	struct range hpa = cxld->hpa_range;
>  	struct cxl_interleave_context ctx;
>  	int rc;
> @@ -3271,25 +3315,33 @@ static int cxl_endpoint_decoder_initialize(struct cxl_endpoint_decoder *cxled)
>  		.hpa_range = &hpa,
>  	};
>  
> -	while (iter && !is_cxl_root(iter)) {
> +	if (!iter || !parent)
> +		return -ENXIO;
> +
> +	while (iter && parent) {
> +		/* Translate HPA to the next upper memory domain. */
> +		rc = cxl_port_calc_hpa(parent, cxld, &hpa);
> +		if (rc < 0)
> +			return rc;
> +
>  		/* Convert interleave settings to next port upstream. */
>  		rc = cxl_port_calc_interleave(iter, &ctx);
>  		if (rc < 0)
>  			return rc;
>  
> -		iter = parent_port_of(iter);
> -	}
> +		iter = parent;
> +		parent = parent_port_of(iter);
>  
> -	if (!iter)
> -		return -ENXIO;
> +		if (!parent || parent->to_hpa)
> +			cxld = cxl_port_find_switch_decoder(iter, &hpa);
>  
> -	root = cxl_port_find_switch_decoder(iter, hpa);
> -	if (!root) {

I'm not immediately spotting why root needs a rename.  Is it used
for other things now that it wasn't before?

> -		dev_err(cxlmd->dev.parent,
> -			"%s:%s no CXL window for range %#llx:%#llx\n",
> -			dev_name(&cxlmd->dev), dev_name(&cxld->dev),
> -			cxld->hpa_range.start, cxld->hpa_range.end);
> -		return -ENXIO;
> +		if (!cxld) {
> +			dev_err(cxlmd->dev.parent,
> +				"%s:%s no CXL window for range %#llx:%#llx\n",
> +				dev_name(&cxlmd->dev), dev_name(&cxled->cxld.dev),
> +				hpa.start, hpa.end);
> +			return -ENXIO;
> +		}
>  	}