linux-kernel - Re: [PATCH v2 10/15] cxl/region: Use root decoders interleaving parameters to create a region

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250314124947.00001d93@huawei.com>
Date: Fri, 14 Mar 2025 12:49:47 +0000
From: Jonathan Cameron <Jonathan.Cameron@...wei.com>
To: Robert Richter <rrichter@....com>
CC: Alison Schofield <alison.schofield@...el.com>, Vishal Verma
	<vishal.l.verma@...el.com>, Ira Weiny <ira.weiny@...el.com>, Dan Williams
	<dan.j.williams@...el.com>, Dave Jiang <dave.jiang@...el.com>, "Davidlohr
 Bueso" <dave@...olabs.net>, <linux-cxl@...r.kernel.org>,
	<linux-kernel@...r.kernel.org>, Gregory Price <gourry@...rry.net>, "Fabio M.
 De Francesco" <fabio.m.de.francesco@...ux.intel.com>, Terry Bowman
	<terry.bowman@....com>
Subject: Re: [PATCH v2 10/15] cxl/region: Use root decoders interleaving
 parameters to create a region

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

> Endpoints requiring address translation might not be aware of the
> system's interleaving configuration. Instead, interleaving can be
> configured on an upper memory domain (from an endpoint view) and thus
> is not visible to the endpoint. For region creation this might cause
> an invalid interleaving config that does not match the CFMWS entries.
> 
> Use the interleaving configuration of the root decoders to create a
> region which bases on CFMWS entries. This always matches the system's
> interleaving configuration and is independent of the underlying memory
> topology.
> 
> Signed-off-by: Robert Richter <rrichter@....com>
> ---
>  drivers/cxl/core/region.c | 39 ++++++++++++++++++++++++++++++++++-----
>  drivers/cxl/cxl.h         |  2 ++
>  2 files changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6e0434eee6df..3afcc9ca06ae 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1749,6 +1749,15 @@ static int cxl_region_validate_position(struct cxl_region *cxlr,
>  		}
>  	}
>  
> +	if (p->interleave_ways != cxled->interleave_ways ||
> +	    p->interleave_granularity != cxled->interleave_granularity ) {
> +		dev_dbg(&cxlr->dev, "interleaving config mismatch with %s: ways: %d:%d granularity: %d:%d\n",
> +			dev_name(&cxled->cxld.dev), p->interleave_ways,
> +			cxled->interleave_ways, p->interleave_granularity,
> +			cxled->interleave_granularity);
> +		return -ENXIO;
> +	}
> +
>  	return 0;
>  }
>  
> @@ -1852,7 +1861,7 @@ static int match_switch_decoder_by_range(struct device *dev,
>  }
>  
>  static int find_pos_and_ways(struct cxl_port *port, struct range *range,
> -			     int *pos, int *ways)
> +			     int *pos, int *ways, int *granularity)
>  {
>  	struct cxl_switch_decoder *cxlsd;
>  	struct cxl_port *parent;
> @@ -1873,6 +1882,7 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  	}
>  	cxlsd = to_cxl_switch_decoder(dev);
>  	*ways = cxlsd->cxld.interleave_ways;
> +	*granularity = cxlsd->cxld.interleave_granularity;
>  
>  	for (int i = 0; i < *ways; i++) {
>  		if (cxlsd->target[i] == port->parent_dport) {
> @@ -1896,6 +1906,8 @@ static int find_pos_and_ways(struct cxl_port *port, struct range *range,
>  struct cxl_interleave_context {
>  	struct range *hpa_range;
>  	int pos;
> +	int interleave_ways;
> +	int interleave_granularity;

Ah. And here is our context expansion

>  };
>  
>  /**
> @@ -1914,13 +1926,17 @@ struct cxl_interleave_context {
>   * the topology from the endpoint to the root decoder and iteratively
>   * applying the function for each port.
>   *
> + * Calculation of interleaving ways:
> + *
> + *    interleave_ways = interleave_ways * parent_ways;
> + *
>   * Return: position >= 0 on success
>   *	   -ENXIO on failure
>   */
>  static int cxl_port_calc_interleave(struct cxl_port *port,
>  				    struct cxl_interleave_context *ctx)
>  {
> -	int parent_ways = 0, parent_pos = 0;
> +	int parent_ways = 0, parent_pos = 0, parent_granularity = 0;
>  	int rc;
>  
>  	/*
> @@ -1955,12 +1971,23 @@ static int cxl_port_calc_interleave(struct cxl_port *port,
>  	if (is_cxl_root(port))
>  		return 0;
>  
> -	rc = find_pos_and_ways(port, ctx->hpa_range, &parent_pos, &parent_ways);
> +	rc = find_pos_and_ways(port, ctx->hpa_range, &parent_pos, &parent_ways,
> +			&parent_granularity);
>  	if (rc)
>  		return rc;
>  
>  	ctx->pos = ctx->pos * parent_ways + parent_pos;
>  
> +	if (ctx->interleave_ways)
> +		ctx->interleave_ways *= parent_ways;
> +	else
> +		ctx->interleave_ways = parent_ways;
> +
> +	if (ctx->interleave_granularity)
> +		ctx->interleave_granularity *= ctx->interleave_ways;
> +	else
> +		ctx->interleave_granularity = parent_granularity;
> +
>  	return ctx->pos;

I think Gregory called this out in earlier patch.  Mixing and matching
between returning pos and use of ctx makes things hard to read.  If
we need to have it in context, then make this return 0 or -ERR instead.

>  }