[PATCH 0/2] CXL XOR Interleave Arithmetic

[PATCH 0/2] CXL XOR Interleave Arithmetic

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

Add support for the new 'XOR' Interleave Arithmetic as defined
in the CXL 3.0 Specification [1].

A linux-ized ACPI patch is included here for reference. The actual
pull request is pending at https://github.com/acpica/acpica/pull/787

[1]: https://www.computeexpresslink.org/download-the-specification

Alison Schofield (2):
  For ACPICA: Add the CXIMS structure definition to the CEDT table
  cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

 include/acpi/actbl1.h | 14 ++++++-
 drivers/cxl/cxl.h     |  2 +
 drivers/cxl/acpi.c    | 96 ++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 106 insertions(+), 6 deletions(-)


base-commit: 1cd8a2537eb07751d405ab7e2223f20338a90506
-- 
2.31.1

[PATCH 1/2] For ACPICA: Add the CXIMS structure definition to the CEDT table

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

A linux-ized ACPI patch is included here for reference. The actual
pull request is pending: https://github.com/acpica/acpica/pull/787

The CXL XOR Interleave Math Structure (CXIMS) is added to the
CXL Early Discovery Table (CEDT). This new structure is defined
in the CXL 3.0 specification Section 9.17.1.4

https://www.computeexpresslink.org/spec-landing

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 include/acpi/actbl1.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/acpi/actbl1.h b/include/acpi/actbl1.h
index 15c78678c5d3..f96f4fe5328d 100644
--- a/include/acpi/actbl1.h
+++ b/include/acpi/actbl1.h
@@ -329,7 +329,8 @@ struct acpi_cedt_header {
 enum acpi_cedt_type {
 	ACPI_CEDT_TYPE_CHBS = 0,
 	ACPI_CEDT_TYPE_CFMWS = 1,
-	ACPI_CEDT_TYPE_RESERVED = 2,
+	ACPI_CEDT_TYPE_CXIMS = 2,
+	ACPI_CEDT_TYPE_RESERVED = 3,
 };
 
 /* Values for version field above */
@@ -380,6 +381,7 @@ struct acpi_cedt_cfmws_target_element {
 /* Values for Interleave Arithmetic field above */
 
 #define ACPI_CEDT_CFMWS_ARITHMETIC_MODULO   (0)
+#define ACPI_CEDT_CFMWS_ARITHMETIC_XOR	    (1)
 
 /* Values for Restrictions field above */
 
@@ -389,6 +391,16 @@ struct acpi_cedt_cfmws_target_element {
 #define ACPI_CEDT_CFMWS_RESTRICT_PMEM       (1<<3)
 #define ACPI_CEDT_CFMWS_RESTRICT_FIXED      (1<<4)
 
+/* 2: CXL XOR Interleave Math Structure */
+
+struct acpi_cedt_cxims {
+	struct acpi_cedt_header header;
+	u16 reserved1;
+	u8 hbig;
+	u8 nr_xormaps;
+	u64 xormap_list[];
+};
+
 /*******************************************************************************
  *
  * CPEP - Corrected Platform Error Polling table (ACPI 4.0)
-- 
2.31.1

[PATCH 2/2] cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

[alison.schofield]

From: Alison Schofield <alison.schofield@intel.com>

When the CFMWS is using XOR math, parse the corresponding
CXIMS structure and store the xormaps in the root decoder.
Use the xormaps in a new lookup, cxl_hb_xor(), to discover
a targets entry in a host bridge interleave target list.

Defined in CXL Spec 3.0 Section: 9.17.1

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/cxl.h  |  2 +
 drivers/cxl/acpi.c | 96 +++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 93 insertions(+), 5 deletions(-)

diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index f680450f0b16..0a17a7007bff 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -330,12 +330,14 @@ struct cxl_switch_decoder {
  * @res: host / parent resource for region allocations
  * @region_id: region id for next region provisioning event
  * @calc_hb: which host bridge covers the n'th position by granularity
+ * @platform_data: platform specific configuration data
  * @cxlsd: base cxl switch decoder
  */
 struct cxl_root_decoder {
 	struct resource *res;
 	atomic_t region_id;
 	struct cxl_dport *(*calc_hb)(struct cxl_root_decoder *cxlrd, int pos);
+	void *platform_data;
 	struct cxl_switch_decoder cxlsd;
 };
 
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index fb649683dd3a..6ac6751c7f4e 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -9,6 +9,79 @@
 #include "cxlpci.h"
 #include "cxl.h"
 
+struct cxims_data {
+	int nr_maps;
+	u64 xormaps[];
+};
+
+static struct cxl_dport *cxl_hb_xor(struct cxl_root_decoder *cxlrd, int pos)
+{
+	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
+	struct cxims_data *cximsd = cxlrd->platform_data;
+	struct cxl_decoder *cxld = &cxlsd->cxld;
+	int ig = cxld->interleave_granularity;
+	int i, n = 0;
+	u64 hpa;
+
+	if (dev_WARN_ONCE(&cxld->dev,
+			  cxld->interleave_ways != cxlsd->nr_targets,
+			  "misconfigured root decoder\n"))
+		return NULL;
+	/*
+	 * Find this targets entry (n) in the host bridge interleave
+	 * list. Defined in CXL Spec 3.0 Section 9.17.1.3 Table 9-22
+	 */
+	hpa = cxlrd->res->start + pos * ig;
+	for (i = 0; i < cximsd->nr_maps; i++)
+		n |= (hweight64(hpa & cximsd->xormaps[i]) & 1) << i;
+
+	return cxlrd->cxlsd.target[n];
+}
+
+struct cxl_cxims_context {
+	struct device *dev;
+	struct cxl_root_decoder *cxlrd;
+};
+
+static int cxl_parse_cxims(union acpi_subtable_headers *header, void *arg,
+			   const unsigned long end)
+{
+	struct acpi_cedt_cxims *cxims = (struct acpi_cedt_cxims *)header;
+	struct cxl_cxims_context *ctx = arg;
+	struct cxl_root_decoder *cxlrd = ctx->cxlrd;
+	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
+	struct device *dev = ctx->dev;
+	struct cxims_data *cximsd;
+	unsigned int hbig;
+	u8 eiw;
+	int rc;
+
+	rc = cxl_to_granularity(cxims->hbig, &hbig);
+	if (rc)
+		return rc;
+
+	rc = ways_to_cxl(cxld->interleave_ways, &eiw);
+	if (rc)
+		return rc;
+
+	if (hbig == cxld->interleave_granularity) {
+		if (cxims->nr_xormaps < eiw) {
+			dev_dbg(dev, "CXIMS nr_xormaps[%d] expected[%d]\n",
+				cxims->nr_xormaps, eiw);
+			return -ENXIO;
+		}
+
+		cximsd = devm_kzalloc(dev, struct_size(cximsd, xormaps, eiw),
+				      GFP_KERNEL);
+		memcpy(cximsd->xormaps, cxims->xormap_list,
+		       eiw * sizeof(*cximsd->xormaps));
+		cximsd->nr_maps = eiw;
+		cxlrd->platform_data = cximsd;
+		cxlrd->calc_hb = cxl_hb_xor;
+	}
+	return 0;
+}
+
 static unsigned long cfmws_to_decoder_flags(int restrictions)
 {
 	unsigned long flags = CXL_DECODER_F_ENABLE;
@@ -33,11 +106,6 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
 	int rc, expected_len;
 	unsigned int ways;
 
-	if (cfmws->interleave_arithmetic != ACPI_CEDT_CFMWS_ARITHMETIC_MODULO) {
-		dev_err(dev, "CFMWS Unsupported Interleave Arithmetic\n");
-		return -EINVAL;
-	}
-
 	if (!IS_ALIGNED(cfmws->base_hpa, SZ_256M)) {
 		dev_err(dev, "CFMWS Base HPA not 256MB aligned\n");
 		return -EINVAL;
@@ -84,6 +152,7 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
 	struct cxl_cfmws_context *ctx = arg;
 	struct cxl_port *root_port = ctx->root_port;
 	struct resource *cxl_res = ctx->cxl_res;
+	struct cxl_cxims_context cxims_ctx;
 	struct cxl_root_decoder *cxlrd;
 	struct device *dev = ctx->dev;
 	struct acpi_cedt_cfmws *cfmws;
@@ -148,7 +217,24 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
 		ig = CXL_DECODER_MIN_GRANULARITY;
 	cxld->interleave_granularity = ig;
 
+	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
+		cxims_ctx = (struct cxl_cxims_context) {
+			.dev = dev,
+			.cxlrd = cxlrd,
+		};
+		rc = acpi_table_parse_cedt(ACPI_CEDT_TYPE_CXIMS,
+					   cxl_parse_cxims, &cxims_ctx);
+		if (rc < 0)
+			goto err_xormap;
+
+		if (cxlrd->calc_hb != cxl_hb_xor) {
+			rc = -ENXIO;
+			goto err_xormap;
+		}
+	}
 	rc = cxl_decoder_add(cxld, target_map);
+
+err_xormap:
 	if (rc)
 		put_device(&cxld->dev);
 	else
-- 
2.31.1

RE: [PATCH 2/2] cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

[Dan Williams]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> When the CFMWS is using XOR math, parse the corresponding
> CXIMS structure and store the xormaps in the root decoder.
> Use the xormaps in a new lookup, cxl_hb_xor(), to discover
> a targets entry in a host bridge interleave target list.
> 
> Defined in CXL Spec 3.0 Section: 9.17.1
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  drivers/cxl/cxl.h  |  2 +
>  drivers/cxl/acpi.c | 96 +++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 93 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f680450f0b16..0a17a7007bff 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -330,12 +330,14 @@ struct cxl_switch_decoder {
>   * @res: host / parent resource for region allocations
>   * @region_id: region id for next region provisioning event
>   * @calc_hb: which host bridge covers the n'th position by granularity
> + * @platform_data: platform specific configuration data
>   * @cxlsd: base cxl switch decoder
>   */
>  struct cxl_root_decoder {
>  	struct resource *res;
>  	atomic_t region_id;
>  	struct cxl_dport *(*calc_hb)(struct cxl_root_decoder *cxlrd, int pos);
> +	void *platform_data;
>  	struct cxl_switch_decoder cxlsd;
>  };
>  
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index fb649683dd3a..6ac6751c7f4e 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -9,6 +9,79 @@
>  #include "cxlpci.h"
>  #include "cxl.h"
>  
> +struct cxims_data {
> +	int nr_maps;
> +	u64 xormaps[];
> +};
> +
> +static struct cxl_dport *cxl_hb_xor(struct cxl_root_decoder *cxlrd, int pos)
> +{
> +	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
> +	struct cxims_data *cximsd = cxlrd->platform_data;
> +	struct cxl_decoder *cxld = &cxlsd->cxld;
> +	int ig = cxld->interleave_granularity;
> +	int i, n = 0;
> +	u64 hpa;
> +
> +	if (dev_WARN_ONCE(&cxld->dev,
> +			  cxld->interleave_ways != cxlsd->nr_targets,
> +			  "misconfigured root decoder\n"))
> +		return NULL;
> +	/*
> +	 * Find this targets entry (n) in the host bridge interleave
> +	 * list. Defined in CXL Spec 3.0 Section 9.17.1.3 Table 9-22
> +	 */
> +	hpa = cxlrd->res->start + pos * ig;
> +	for (i = 0; i < cximsd->nr_maps; i++)
> +		n |= (hweight64(hpa & cximsd->xormaps[i]) & 1) << i;
> +
> +	return cxlrd->cxlsd.target[n];
> +}
> +
> +struct cxl_cxims_context {
> +	struct device *dev;
> +	struct cxl_root_decoder *cxlrd;
> +};
> +
> +static int cxl_parse_cxims(union acpi_subtable_headers *header, void *arg,
> +			   const unsigned long end)
> +{
> +	struct acpi_cedt_cxims *cxims = (struct acpi_cedt_cxims *)header;
> +	struct cxl_cxims_context *ctx = arg;
> +	struct cxl_root_decoder *cxlrd = ctx->cxlrd;
> +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
> +	struct device *dev = ctx->dev;
> +	struct cxims_data *cximsd;
> +	unsigned int hbig;
> +	u8 eiw;

Per your comment on Dave's patch lets keep eiw as a shorthand for
"encoded interleave ways" i.e. the ilog2()'ish value of interleave-ways.
So just: s/eiw/iw/.

Otherwise, looks good to me. Just need the ACPICA side to land.

Re: [PATCH 2/2] cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

[Alison Schofield]

On Tue, Aug 09, 2022 at 04:36:14PM -0700, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > When the CFMWS is using XOR math, parse the corresponding
> > CXIMS structure and store the xormaps in the root decoder.
> > Use the xormaps in a new lookup, cxl_hb_xor(), to discover
> > a targets entry in a host bridge interleave target list.
> > 
> > Defined in CXL Spec 3.0 Section: 9.17.1
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  drivers/cxl/cxl.h  |  2 +
> >  drivers/cxl/acpi.c | 96 +++++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 93 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > index f680450f0b16..0a17a7007bff 100644
> > --- a/drivers/cxl/cxl.h
> > +++ b/drivers/cxl/cxl.h
> > @@ -330,12 +330,14 @@ struct cxl_switch_decoder {
> >   * @res: host / parent resource for region allocations
> >   * @region_id: region id for next region provisioning event
> >   * @calc_hb: which host bridge covers the n'th position by granularity
> > + * @platform_data: platform specific configuration data
> >   * @cxlsd: base cxl switch decoder
> >   */
> >  struct cxl_root_decoder {
> >  	struct resource *res;
> >  	atomic_t region_id;
> >  	struct cxl_dport *(*calc_hb)(struct cxl_root_decoder *cxlrd, int pos);
> > +	void *platform_data;
> >  	struct cxl_switch_decoder cxlsd;
> >  };
> >  
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index fb649683dd3a..6ac6751c7f4e 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -9,6 +9,79 @@
> >  #include "cxlpci.h"
> >  #include "cxl.h"
> >  
> > +struct cxims_data {
> > +	int nr_maps;
> > +	u64 xormaps[];
> > +};
> > +
> > +static struct cxl_dport *cxl_hb_xor(struct cxl_root_decoder *cxlrd, int pos)
> > +{
> > +	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
> > +	struct cxims_data *cximsd = cxlrd->platform_data;
> > +	struct cxl_decoder *cxld = &cxlsd->cxld;
> > +	int ig = cxld->interleave_granularity;
> > +	int i, n = 0;
> > +	u64 hpa;
> > +
> > +	if (dev_WARN_ONCE(&cxld->dev,
> > +			  cxld->interleave_ways != cxlsd->nr_targets,
> > +			  "misconfigured root decoder\n"))
> > +		return NULL;
> > +	/*
> > +	 * Find this targets entry (n) in the host bridge interleave
> > +	 * list. Defined in CXL Spec 3.0 Section 9.17.1.3 Table 9-22
> > +	 */
> > +	hpa = cxlrd->res->start + pos * ig;
> > +	for (i = 0; i < cximsd->nr_maps; i++)
> > +		n |= (hweight64(hpa & cximsd->xormaps[i]) & 1) << i;
> > +
> > +	return cxlrd->cxlsd.target[n];
> > +}
> > +
> > +struct cxl_cxims_context {
> > +	struct device *dev;
> > +	struct cxl_root_decoder *cxlrd;
> > +};
> > +
> > +static int cxl_parse_cxims(union acpi_subtable_headers *header, void *arg,
> > +			   const unsigned long end)
> > +{
> > +	struct acpi_cedt_cxims *cxims = (struct acpi_cedt_cxims *)header;
> > +	struct cxl_cxims_context *ctx = arg;
> > +	struct cxl_root_decoder *cxlrd = ctx->cxlrd;
> > +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
> > +	struct device *dev = ctx->dev;
> > +	struct cxims_data *cximsd;
> > +	unsigned int hbig;
> > +	u8 eiw;
> 
> Per your comment on Dave's patch lets keep eiw as a shorthand for
> "encoded interleave ways" i.e. the ilog2()'ish value of interleave-ways.
> So just: s/eiw/iw/.

Then maybe it's intentionally wrong. The eiw tells the number of
xormaps to store for use in cxl_hb_xor(). The 'iw', as stored in
cxld->interleave_ways would be too many, so I translate back to the
encoded version here.

> 
> Otherwise, looks good to me. Just need the ACPICA side to land.

Re: [PATCH 2/2] cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

[Dan Williams]

Alison Schofield wrote:
> On Tue, Aug 09, 2022 at 04:36:14PM -0700, Dan Williams wrote:
> > alison.schofield@ wrote:
> > > From: Alison Schofield <alison.schofield@intel.com>
> > > 
> > > When the CFMWS is using XOR math, parse the corresponding
> > > CXIMS structure and store the xormaps in the root decoder.
> > > Use the xormaps in a new lookup, cxl_hb_xor(), to discover
> > > a targets entry in a host bridge interleave target list.
> > > 
> > > Defined in CXL Spec 3.0 Section: 9.17.1
> > > 
> > > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > > ---
> > >  drivers/cxl/cxl.h  |  2 +
> > >  drivers/cxl/acpi.c | 96 +++++++++++++++++++++++++++++++++++++++++++---
> > >  2 files changed, 93 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> > > index f680450f0b16..0a17a7007bff 100644
> > > --- a/drivers/cxl/cxl.h
> > > +++ b/drivers/cxl/cxl.h
> > > @@ -330,12 +330,14 @@ struct cxl_switch_decoder {
> > >   * @res: host / parent resource for region allocations
> > >   * @region_id: region id for next region provisioning event
> > >   * @calc_hb: which host bridge covers the n'th position by granularity
> > > + * @platform_data: platform specific configuration data
> > >   * @cxlsd: base cxl switch decoder
> > >   */
> > >  struct cxl_root_decoder {
> > >  	struct resource *res;
> > >  	atomic_t region_id;
> > >  	struct cxl_dport *(*calc_hb)(struct cxl_root_decoder *cxlrd, int pos);
> > > +	void *platform_data;
> > >  	struct cxl_switch_decoder cxlsd;
> > >  };
> > >  
> > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > > index fb649683dd3a..6ac6751c7f4e 100644
> > > --- a/drivers/cxl/acpi.c
> > > +++ b/drivers/cxl/acpi.c
> > > @@ -9,6 +9,79 @@
> > >  #include "cxlpci.h"
> > >  #include "cxl.h"
> > >  
> > > +struct cxims_data {
> > > +	int nr_maps;
> > > +	u64 xormaps[];
> > > +};
> > > +
> > > +static struct cxl_dport *cxl_hb_xor(struct cxl_root_decoder *cxlrd, int pos)
> > > +{
> > > +	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
> > > +	struct cxims_data *cximsd = cxlrd->platform_data;
> > > +	struct cxl_decoder *cxld = &cxlsd->cxld;
> > > +	int ig = cxld->interleave_granularity;
> > > +	int i, n = 0;
> > > +	u64 hpa;
> > > +
> > > +	if (dev_WARN_ONCE(&cxld->dev,
> > > +			  cxld->interleave_ways != cxlsd->nr_targets,
> > > +			  "misconfigured root decoder\n"))
> > > +		return NULL;
> > > +	/*
> > > +	 * Find this targets entry (n) in the host bridge interleave
> > > +	 * list. Defined in CXL Spec 3.0 Section 9.17.1.3 Table 9-22
> > > +	 */
> > > +	hpa = cxlrd->res->start + pos * ig;
> > > +	for (i = 0; i < cximsd->nr_maps; i++)
> > > +		n |= (hweight64(hpa & cximsd->xormaps[i]) & 1) << i;
> > > +
> > > +	return cxlrd->cxlsd.target[n];
> > > +}
> > > +
> > > +struct cxl_cxims_context {
> > > +	struct device *dev;
> > > +	struct cxl_root_decoder *cxlrd;
> > > +};
> > > +
> > > +static int cxl_parse_cxims(union acpi_subtable_headers *header, void *arg,
> > > +			   const unsigned long end)
> > > +{
> > > +	struct acpi_cedt_cxims *cxims = (struct acpi_cedt_cxims *)header;
> > > +	struct cxl_cxims_context *ctx = arg;
> > > +	struct cxl_root_decoder *cxlrd = ctx->cxlrd;
> > > +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
> > > +	struct device *dev = ctx->dev;
> > > +	struct cxims_data *cximsd;
> > > +	unsigned int hbig;
> > > +	u8 eiw;
> > 
> > Per your comment on Dave's patch lets keep eiw as a shorthand for
> > "encoded interleave ways" i.e. the ilog2()'ish value of interleave-ways.
> > So just: s/eiw/iw/.
> 
> Then maybe it's intentionally wrong. The eiw tells the number of
> xormaps to store for use in cxl_hb_xor(). The 'iw', as stored in
> cxld->interleave_ways would be too many, so I translate back to the
> encoded version here.

Oh, what threw me off was the:

+               cximsd->nr_maps = eiw;

...because nr_maps is not @eiw. My suggestion to just change it back to
cxld->interleave_ways though is also wrong, sorry about that. 

The calculation for nr_maps is not ways_to_cxl() it is just plain
"ilog2(cxld->interleave_ways)" because the nr_maps needed for 3, 6, 12,
are 0, 1, 2 respectively, not 8, 9, 10 which is what @eiw would be.

Re: [PATCH 2/2] cxl/acpi: Support CXL XOR Interleave Math (CXIMS)

[Jonathan Cameron]

On Tue,  9 Aug 2022 14:44:10 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> When the CFMWS is using XOR math, parse the corresponding
> CXIMS structure and store the xormaps in the root decoder.
> Use the xormaps in a new lookup, cxl_hb_xor(), to discover
> a targets entry in a host bridge interleave target list.
> 
> Defined in CXL Spec 3.0 Section: 9.17.1

9.17.1.4 ?  Not important though as I'm sure people can scan
down a few pages from the CEDT start.

> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
One question inline - I think I know the answer but could be wrong!

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>

> ---
>  drivers/cxl/cxl.h  |  2 +
>  drivers/cxl/acpi.c | 96 +++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 93 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f680450f0b16..0a17a7007bff 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -330,12 +330,14 @@ struct cxl_switch_decoder {
>   * @res: host / parent resource for region allocations
>   * @region_id: region id for next region provisioning event
>   * @calc_hb: which host bridge covers the n'th position by granularity
> + * @platform_data: platform specific configuration data
>   * @cxlsd: base cxl switch decoder
>   */
>  struct cxl_root_decoder {
>  	struct resource *res;
>  	atomic_t region_id;
>  	struct cxl_dport *(*calc_hb)(struct cxl_root_decoder *cxlrd, int pos);
> +	void *platform_data;

I guess left as void * to allow for mocking easily?

>  	struct cxl_switch_decoder cxlsd;
>  };
>  
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index fb649683dd3a..6ac6751c7f4e 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -9,6 +9,79 @@
>  #include "cxlpci.h"
>  #include "cxl.h"
>  
> +struct cxims_data {
> +	int nr_maps;
> +	u64 xormaps[];
> +};
> +
p> +static struct cxl_dport *cxl_hb_xor(struct cxl_root_decoder *cxlrd, int pos)
> +{
> +	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
> +	struct cxims_data *cximsd = cxlrd->platform_data;
> +	struct cxl_decoder *cxld = &cxlsd->cxld;
> +	int ig = cxld->interleave_granularity;
> +	int i, n = 0;
> +	u64 hpa;
> +
> +	if (dev_WARN_ONCE(&cxld->dev,
> +			  cxld->interleave_ways != cxlsd->nr_targets,
> +			  "misconfigured root decoder\n"))
> +		return NULL;
> +	/*
> +	 * Find this targets entry (n) in the host bridge interleave
> +	 * list. Defined in CXL Spec 3.0 Section 9.17.1.3 Table 9-22
> +	 */
> +	hpa = cxlrd->res->start + pos * ig;
> +	for (i = 0; i < cximsd->nr_maps; i++)
> +		n |= (hweight64(hpa & cximsd->xormaps[i]) & 1) << i;
> +
> +	return cxlrd->cxlsd.target[n];
> +}
> +
> +struct cxl_cxims_context {
> +	struct device *dev;
> +	struct cxl_root_decoder *cxlrd;
> +};
> +
> +static int cxl_parse_cxims(union acpi_subtable_headers *header, void *arg,
> +			   const unsigned long end)
> +{
> +	struct acpi_cedt_cxims *cxims = (struct acpi_cedt_cxims *)header;
> +	struct cxl_cxims_context *ctx = arg;
> +	struct cxl_root_decoder *cxlrd = ctx->cxlrd;
> +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
> +	struct device *dev = ctx->dev;
> +	struct cxims_data *cximsd;
> +	unsigned int hbig;
> +	u8 eiw;
> +	int rc;
> +
> +	rc = cxl_to_granularity(cxims->hbig, &hbig);
> +	if (rc)
> +		return rc;
> +
> +	rc = ways_to_cxl(cxld->interleave_ways, &eiw);
> +	if (rc)
> +		return rc;
> +
> +	if (hbig == cxld->interleave_granularity) {
> +		if (cxims->nr_xormaps < eiw) {
> +			dev_dbg(dev, "CXIMS nr_xormaps[%d] expected[%d]\n",
> +				cxims->nr_xormaps, eiw);
> +			return -ENXIO;
> +		}
> +
> +		cximsd = devm_kzalloc(dev, struct_size(cximsd, xormaps, eiw),
> +				      GFP_KERNEL);
> +		memcpy(cximsd->xormaps, cxims->xormap_list,
> +		       eiw * sizeof(*cximsd->xormaps));
> +		cximsd->nr_maps = eiw;
> +		cxlrd->platform_data = cximsd;
> +		cxlrd->calc_hb = cxl_hb_xor;
> +	}
> +	return 0;
> +}
> +
>  static unsigned long cfmws_to_decoder_flags(int restrictions)
>  {
>  	unsigned long flags = CXL_DECODER_F_ENABLE;
> @@ -33,11 +106,6 @@ static int cxl_acpi_cfmws_verify(struct device *dev,
>  	int rc, expected_len;
>  	unsigned int ways;
>  
> -	if (cfmws->interleave_arithmetic != ACPI_CEDT_CFMWS_ARITHMETIC_MODULO) {
> -		dev_err(dev, "CFMWS Unsupported Interleave Arithmetic\n");
> -		return -EINVAL;
> -	}
> -
>  	if (!IS_ALIGNED(cfmws->base_hpa, SZ_256M)) {
>  		dev_err(dev, "CFMWS Base HPA not 256MB aligned\n");
>  		return -EINVAL;
> @@ -84,6 +152,7 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>  	struct cxl_cfmws_context *ctx = arg;
>  	struct cxl_port *root_port = ctx->root_port;
>  	struct resource *cxl_res = ctx->cxl_res;
> +	struct cxl_cxims_context cxims_ctx;
>  	struct cxl_root_decoder *cxlrd;
>  	struct device *dev = ctx->dev;
>  	struct acpi_cedt_cfmws *cfmws;
> @@ -148,7 +217,24 @@ static int cxl_parse_cfmws(union acpi_subtable_headers *header, void *arg,
>  		ig = CXL_DECODER_MIN_GRANULARITY;
>  	cxld->interleave_granularity = ig;
>  
> +	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
> +		cxims_ctx = (struct cxl_cxims_context) {
> +			.dev = dev,
> +			.cxlrd = cxlrd,
> +		};
> +		rc = acpi_table_parse_cedt(ACPI_CEDT_TYPE_CXIMS,
> +					   cxl_parse_cxims, &cxims_ctx);
> +		if (rc < 0)
> +			goto err_xormap;
> +
> +		if (cxlrd->calc_hb != cxl_hb_xor) {
> +			rc = -ENXIO;
> +			goto err_xormap;
> +		}
> +	}
>  	rc = cxl_decoder_add(cxld, target_map);
> +
> +err_xormap:
>  	if (rc)
>  		put_device(&cxld->dev);
>  	else