[PATCH 0/3] CXL: Add a loadable module for address translation

[PATCH 0/3] CXL: Add a loadable module for address translation

[alison.schofield]

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

This set doesn't have a base commit. It is based on cxl/next plus the
Poison by Region Offset patchset [1], plus the fix to Limit XOR map
application based on host bridge ways [2] which this test module exposed.

[1] https://lore.kernel.org/linux-cxl/cover.1754290144.git.alison.schofield@intel.com/
[2] https://lore.kernel.org/linux-cxl/20250804082357.2590809-1-alison.schofield@intel.com/

This series refactors CXL address translation code to support testing
and adds a dedicated test module for validation of the translation
calculations.

The work is presented in 3 patches:
1. Extracts the core translation logic into standalone, testable functions.
2. Provides access to XOR interleave calculations without exposing internal
   CXIMS structures to test modules.
3. Adds the test module that validates both Host to Device, and Device
   to Host address translations.

Accessing the core functions in the test module was implemented by 
adding the needed core/region functions to cxl_core_exports.c and 
adding a similar 'cxl_acpi_exports.c' to access the cxl/acpi.c funcs.

The companion CXL Unit Test script will be posted separately.


Alison Schofield (3):
  cxl/region: Refactor address translation funcs for testing
  cxl/acpi: Make the XOR calculations available for testing
  cxl/test: Add cxl_translate module for address translation testing

 drivers/cxl/acpi.c                     |  51 ++++-
 drivers/cxl/core/region.c              | 145 +++++++-----
 drivers/cxl/cxl.h                      |   3 +
 include/linux/acpi.h                   |   7 +
 tools/testing/cxl/Kbuild               |   1 +
 tools/testing/cxl/cxl_acpi_exports.c   |  13 ++
 tools/testing/cxl/cxl_core_exports.c   |  11 +
 tools/testing/cxl/test/Kbuild          |   2 +
 tools/testing/cxl/test/cxl_translate.c | 298 +++++++++++++++++++++++++
 9 files changed, 467 insertions(+), 64 deletions(-)
 create mode 100644 tools/testing/cxl/cxl_acpi_exports.c
 create mode 100644 tools/testing/cxl/test/cxl_translate.c

-- 
2.37.3

[PATCH 1/3] cxl/region: Refactor address translation funcs for testing

[alison.schofield]

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

In preparation for adding a test module that exercises the address
translation calculations, extract the core calculations into stand-
alone functions that operate on base parameters without dependencies
on struct cxl_region.

This refactoring enables unit testing of the address translation logic
with controlled inputs, while maintaining identical functionality in
the existing code paths.

The moved code has only one change. In the new cxl_calculate_position()
eiw_to_ways(eiw, &ways) replaces the prior usage of p->interleave_ways,
since the new function cannot depend upon struct cxl_region_params.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/core/region.c | 145 +++++++++++++++++++++++---------------
 drivers/cxl/cxl.h         |   3 +
 2 files changed, 90 insertions(+), 58 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 281a31df686f..ccee5049d700 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2929,13 +2929,91 @@ static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
 	return cxlrd->ops && cxlrd->ops->spa_to_hpa;
 }
 
+u64 cxl_calculate_dpa_offset(u64 hpa_offset, u8 eiw, u16 eig)
+{
+	u64 dpa_offset, bits_lower, bits_upper, temp;
+
+	/*
+	 * DPA offset: CXL Spec 3.2 Section 8.2.4.20.13
+	 * Lower bits [IG+7:0] pass through unchanged
+	 * (eiw < 8)
+	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
+	 *	Clear the position bits to isolate upper section, then
+	 *	reverse the left shift by eiw that occurred during DPA->HPA
+	 * (eiw >= 8)
+	 *	Per spec: DPAOffset[51:IG+8] = HPAOffset[51:IG+IW] / 3
+	 *	Extract upper bits from the correct bit range and divide by 3
+	 *	to recover the original DPA upper bits
+	 */
+	bits_lower = hpa_offset & GENMASK_ULL(eig + 7, 0);
+	if (eiw < 8) {
+		temp = hpa_offset &= ~((u64)GENMASK(eig + eiw + 8 - 1, 0));
+		dpa_offset = temp >> eiw;
+	} else {
+		bits_upper = div64_u64(hpa_offset >> (eig + eiw), 3);
+		dpa_offset = bits_upper << (eig + 8);
+	}
+	dpa_offset |= bits_lower;
+
+	return dpa_offset;
+}
+
+int cxl_calculate_position(u64 hpa_offset, u8 eiw, u16 eig)
+{
+	unsigned int ways = 0;
+	u64 shifted, rem;
+	int pos;
+
+	/*
+	 * Interleave position: CXL Spec 3.2 Section 8.2.4.20.13
+	 * eiw < 8
+	 *	Position is in the IW bits at HPA_OFFSET[IG+8+IW-1:IG+8].
+	 *	Per spec "remove IW bits starting with bit position IG+8"
+	 * eiw >= 8
+	 *	Position is not explicitly stored in HPA_OFFSET bits. It is
+	 *	derived from the modulo operation of the upper bits using
+	 *	the total number of interleave ways.
+	 */
+	if (eiw < 8) {
+		pos = (hpa_offset >> (eig + 8)) & GENMASK(eiw - 1, 0);
+	} else {
+		shifted = hpa_offset >> (eig + 8);
+		eiw_to_ways(eiw, &ways);
+		div64_u64_rem(shifted, ways, &rem);
+		pos = rem;
+	}
+
+	return pos;
+}
+
+u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig)
+{
+	u64 mask_upper, hpa_offset, bits_upper;
+
+	mask_upper = GENMASK_ULL(51, eig + 8);
+
+	if (eiw < 8) {
+		hpa_offset = (dpa_offset & mask_upper) << eiw;
+		hpa_offset |= pos << (eig + 8);
+	} else {
+		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
+		bits_upper = bits_upper * 3;
+		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
+	}
+
+	/* The lower bits remain unchanged */
+	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
+
+	return hpa_offset;
+}
+
 u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 		   u64 dpa)
 {
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
-	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
 	struct cxl_region_params *p = &cxlr->params;
 	struct cxl_endpoint_decoder *cxled = NULL;
+	u64 dpa_offset, hpa_offset, hpa;
 	u16 eig = 0;
 	u8 eiw = 0;
 	int pos;
@@ -2965,19 +3043,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 	/* Remove the dpa base */
 	dpa_offset = dpa - cxl_dpa_resource_start(cxled);
 
-	mask_upper = GENMASK_ULL(51, eig + 8);
-
-	if (eiw < 8) {
-		hpa_offset = (dpa_offset & mask_upper) << eiw;
-		hpa_offset |= pos << (eig + 8);
-	} else {
-		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
-		bits_upper = bits_upper * 3;
-		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
-	}
-
-	/* The lower bits remain unchanged */
-	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
+	/* this chunk was moved, maybe move comment too */
+	hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, eiw, eig);
 
 	/* Apply the hpa_offset to the region base address */
 	hpa = hpa_offset + p->res->start + p->cache_size;
@@ -3011,8 +3078,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
 	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
 	struct cxl_endpoint_decoder *cxled;
 	u64 hpa, hpa_offset, dpa_offset;
-	u64 bits_upper, bits_lower;
-	u64 shifted, rem, temp;
 	u16 eig = 0;
 	u8 eiw = 0;
 	int pos;
@@ -3034,50 +3099,14 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
 	} else {
 		hpa_offset = offset;
 	}
-	/*
-	 * Interleave position: CXL Spec 3.2 Section 8.2.4.20.13
-	 * eiw < 8
-	 *	Position is in the IW bits at HPA_OFFSET[IG+8+IW-1:IG+8].
-	 *	Per spec "remove IW bits starting with bit position IG+8"
-	 * eiw >= 8
-	 *	Position is not explicitly stored in HPA_OFFSET bits. It is
-	 *	derived from the modulo operation of the upper bits using
-	 *	the total number of interleave ways.
-	 */
-	if (eiw < 8) {
-		pos = (hpa_offset >> (eig + 8)) & GENMASK(eiw - 1, 0);
-	} else {
-		shifted = hpa_offset >> (eig + 8);
-		div64_u64_rem(shifted, p->interleave_ways, &rem);
-		pos = rem;
-	}
-	if (pos < 0 || pos >= p->nr_targets) {
-		dev_dbg(&cxlr->dev, "Invalid position %d for %d targets\n",
-			pos, p->nr_targets);
+
+	pos = cxl_calculate_position(hpa_offset, eiw, eig);
+	if (pos < 0 || pos >= p->nr_targets)
 		return -ENXIO;
-	}
 
-	/*
-	 * DPA offset: CXL Spec 3.2 Section 8.2.4.20.13
-	 * Lower bits [IG+7:0] pass through unchanged
-	 * (eiw < 8)
-	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
-	 *	Clear the position bits to isolate upper section, then
-	 *	reverse the left shift by eiw that occurred during DPA->HPA
-	 * (eiw >= 8)
-	 *	Per spec: DPAOffset[51:IG+8] = HPAOffset[51:IG+IW] / 3
-	 *	Extract upper bits from the correct bit range and divide by 3
-	 *	to recover the original DPA upper bits
-	 */
-	bits_lower = hpa_offset & GENMASK_ULL(eig + 7, 0);
-	if (eiw < 8) {
-		temp = hpa_offset &= ~((u64)GENMASK(eig + eiw + 8 - 1, 0));
-		dpa_offset = temp >> eiw;
-	} else {
-		bits_upper = div64_u64(hpa_offset >> (eig + eiw), 3);
-		dpa_offset = bits_upper << (eig + 8);
-	}
-	dpa_offset |= bits_lower;
+	dpa_offset = cxl_calculate_dpa_offset(hpa_offset, eiw, eig);
+	if (dpa_offset == ULLONG_MAX)
+		return -ENXIO;
 
 	/* Look-up and return the result: a memdev and a DPA */
 	for (int i = 0; i < p->nr_targets; i++) {
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index f87628a22852..151a8ce698c5 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -731,6 +731,9 @@ static inline bool is_cxl_root(struct cxl_port *port)
 	return port->uport_dev == port->dev.parent;
 }
 
+u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig);
+u64 cxl_calculate_dpa_offset(u64 hpa_offset, u8 eiw, u16 eig);
+int cxl_calculate_position(u64 hpa_offset, u8 eiw, u16 eig);
 int cxl_num_decoders_committed(struct cxl_port *port);
 bool is_cxl_port(const struct device *dev);
 struct cxl_port *to_cxl_port(const struct device *dev);
-- 
2.37.3

Re: [PATCH 1/3] cxl/region: Refactor address translation funcs for testing

[Jonathan Cameron]

On Mon,  4 Aug 2025 01:52:39 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> In preparation for adding a test module that exercises the address
> translation calculations, extract the core calculations into stand-
> alone functions that operate on base parameters without dependencies
> on struct cxl_region.
> 
> This refactoring enables unit testing of the address translation logic
> with controlled inputs, while maintaining identical functionality in
> the existing code paths.
> 
> The moved code has only one change. In the new cxl_calculate_position()
> eiw_to_ways(eiw, &ways) replaces the prior usage of p->interleave_ways,
> since the new function cannot depend upon struct cxl_region_params.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>

A few comments.  In general looks fine to me.

>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa)
>  {
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> -	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
>  	struct cxl_region_params *p = &cxlr->params;
>  	struct cxl_endpoint_decoder *cxled = NULL;
> +	u64 dpa_offset, hpa_offset, hpa;
>  	u16 eig = 0;
>  	u8 eiw = 0;
>  	int pos;
> @@ -2965,19 +3043,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	/* Remove the dpa base */
>  	dpa_offset = dpa - cxl_dpa_resource_start(cxled);
>  
> -	mask_upper = GENMASK_ULL(51, eig + 8);
> -
> -	if (eiw < 8) {
> -		hpa_offset = (dpa_offset & mask_upper) << eiw;
> -		hpa_offset |= pos << (eig + 8);
> -	} else {
> -		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
> -		bits_upper = bits_upper * 3;
> -		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
> -	}
> -
> -	/* The lower bits remain unchanged */
> -	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
> +	/* this chunk was moved, maybe move comment too */

No idea what that refers to!

> +	hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, eiw, eig);
>  
>  	/* Apply the hpa_offset to the region base address */
>  	hpa = hpa_offset + p->res->start + p->cache_size;
> @@ -3011,8 +3078,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
>  	struct cxl_endpoint_decoder *cxled;
>  	u64 hpa, hpa_offset, dpa_offset;
> -	u64 bits_upper, bits_lower;
> -	u64 shifted, rem, temp;
>  	u16 eig = 0;
>  	u8 eiw = 0;
>  	int pos;
> @@ -3034,50 +3099,14 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	} else {
>  		hpa_offset = offset;
>  	}
> -	/*
> -	 * Interleave position: CXL Spec 3.2 Section 8.2.4.20.13
> -	 * eiw < 8
> -	 *	Position is in the IW bits at HPA_OFFSET[IG+8+IW-1:IG+8].
> -	 *	Per spec "remove IW bits starting with bit position IG+8"
> -	 * eiw >= 8
> -	 *	Position is not explicitly stored in HPA_OFFSET bits. It is
> -	 *	derived from the modulo operation of the upper bits using
> -	 *	the total number of interleave ways.
> -	 */
> -	if (eiw < 8) {
> -		pos = (hpa_offset >> (eig + 8)) & GENMASK(eiw - 1, 0);
> -	} else {
> -		shifted = hpa_offset >> (eig + 8);
> -		div64_u64_rem(shifted, p->interleave_ways, &rem);
> -		pos = rem;
> -	}
> -	if (pos < 0 || pos >= p->nr_targets) {
> -		dev_dbg(&cxlr->dev, "Invalid position %d for %d targets\n",
> -			pos, p->nr_targets);

Bit of a shame to loose this debug print if we hit this condition.

> +
> +	pos = cxl_calculate_position(hpa_offset, eiw, eig);
> +	if (pos < 0 || pos >= p->nr_targets)
>  		return -ENXIO;
> -	}
>  
> -	/*
> -	 * DPA offset: CXL Spec 3.2 Section 8.2.4.20.13
> -	 * Lower bits [IG+7:0] pass through unchanged
> -	 * (eiw < 8)
> -	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
> -	 *	Clear the position bits to isolate upper section, then
> -	 *	reverse the left shift by eiw that occurred during DPA->HPA
> -	 * (eiw >= 8)
> -	 *	Per spec: DPAOffset[51:IG+8] = HPAOffset[51:IG+IW] / 3
> -	 *	Extract upper bits from the correct bit range and divide by 3
> -	 *	to recover the original DPA upper bits
> -	 */
> -	bits_lower = hpa_offset & GENMASK_ULL(eig + 7, 0);
> -	if (eiw < 8) {
> -		temp = hpa_offset &= ~((u64)GENMASK(eig + eiw + 8 - 1, 0));
> -		dpa_offset = temp >> eiw;
> -	} else {
> -		bits_upper = div64_u64(hpa_offset >> (eig + eiw), 3);
> -		dpa_offset = bits_upper << (eig + 8);
> -	}
> -	dpa_offset |= bits_lower;
> +	dpa_offset = cxl_calculate_dpa_offset(hpa_offset, eiw, eig);
> +	if (dpa_offset == ULLONG_MAX)

How do we get ULLONG_MAX?  Not obvious that cxl_calculate_dpa_offset()
can generate that.

> +		return -ENXIO;

Re: [PATCH 1/3] cxl/region: Refactor address translation funcs for testing

[Alison Schofield]

On Fri, Aug 08, 2025 at 05:12:18PM +0100, Jonathan Cameron wrote:
> On Mon,  4 Aug 2025 01:52:39 -0700
> alison.schofield@intel.com wrote:
> 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > In preparation for adding a test module that exercises the address
> > translation calculations, extract the core calculations into stand-
> > alone functions that operate on base parameters without dependencies
> > on struct cxl_region.
> > 
> > This refactoring enables unit testing of the address translation logic
> > with controlled inputs, while maintaining identical functionality in
> > the existing code paths.
> > 
> > The moved code has only one change. In the new cxl_calculate_position()
> > eiw_to_ways(eiw, &ways) replaces the prior usage of p->interleave_ways,
> > since the new function cannot depend upon struct cxl_region_params.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> 
> A few comments.  In general looks fine to me.
Thanks for reviewing -

snip

> > @@ -2965,19 +3043,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
> >  	/* Remove the dpa base */
> >  	dpa_offset = dpa - cxl_dpa_resource_start(cxled);
> >  
> > -	mask_upper = GENMASK_ULL(51, eig + 8);
> > -
> > -	if (eiw < 8) {
> > -		hpa_offset = (dpa_offset & mask_upper) << eiw;
> > -		hpa_offset |= pos << (eig + 8);
> > -	} else {
> > -		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
> > -		bits_upper = bits_upper * 3;
> > -		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
> > -	}
> > -
> > -	/* The lower bits remain unchanged */
> > -	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
> > +	/* this chunk was moved, maybe move comment too */
> 
> No idea what that refers to!

You caught me talking to myself and you caught me not listening to myself
too! That comment refers to a block comment that is cut off (from the top)
of this diff. I intended to move it into the new helper as it describes the
algorithm of dpa->hpa work. I've moved it in v2.


> 
snip
> > -	}
> > -	if (pos < 0 || pos >= p->nr_targets) {
> > -		dev_dbg(&cxlr->dev, "Invalid position %d for %d targets\n",
> > -			pos, p->nr_targets);
> 
> Bit of a shame to loose this debug print if we hit this condition.

Yes, that was not good. I've restored it below in v2.

> 
> > +
> > +	pos = cxl_calculate_position(hpa_offset, eiw, eig);
> > +	if (pos < 0 || pos >= p->nr_targets)
> >  		return -ENXIO;
> > -	}

snip


> > -		dpa_offset = bits_upper << (eig + 8);
> > -	}
> > -	dpa_offset |= bits_lower;
> > +	dpa_offset = cxl_calculate_dpa_offset(hpa_offset, eiw, eig);
> > +	if (dpa_offset == ULLONG_MAX)
> 
> How do we get ULLONG_MAX?  Not obvious that cxl_calculate_dpa_offset()
> can generate that.

No, that shouldn't be a check here. Thanks for the catch.

> 
> > +		return -ENXIO;
> 
> 
> 

Re: [PATCH 1/3] cxl/region: Refactor address translation funcs for testing

[Dave Jiang]

On 8/4/25 1:52 AM, alison.schofield@intel.com wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> In preparation for adding a test module that exercises the address
> translation calculations, extract the core calculations into stand-
> alone functions that operate on base parameters without dependencies
> on struct cxl_region.
> 
> This refactoring enables unit testing of the address translation logic
> with controlled inputs, while maintaining identical functionality in
> the existing code paths.
> 
> The moved code has only one change. In the new cxl_calculate_position()
> eiw_to_ways(eiw, &ways) replaces the prior usage of p->interleave_ways,
> since the new function cannot depend upon struct cxl_region_params.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> ---
>  drivers/cxl/core/region.c | 145 +++++++++++++++++++++++---------------
>  drivers/cxl/cxl.h         |   3 +
>  2 files changed, 90 insertions(+), 58 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 281a31df686f..ccee5049d700 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2929,13 +2929,91 @@ static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
>  	return cxlrd->ops && cxlrd->ops->spa_to_hpa;
>  }
>  
> +u64 cxl_calculate_dpa_offset(u64 hpa_offset, u8 eiw, u16 eig)
> +{
> +	u64 dpa_offset, bits_lower, bits_upper, temp;
> +
> +	/*
> +	 * DPA offset: CXL Spec 3.2 Section 8.2.4.20.13
> +	 * Lower bits [IG+7:0] pass through unchanged
> +	 * (eiw < 8)
> +	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
> +	 *	Clear the position bits to isolate upper section, then
> +	 *	reverse the left shift by eiw that occurred during DPA->HPA

May be easier to read if listing the steps instead of continued sentence:
	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
	 *	1. Clear the position bits to isolate upper section.
	 *	2. Reverse the left shift by eiw that occurred during DPA->HPA


> +	 * (eiw >= 8)
> +	 *	Per spec: DPAOffset[51:IG+8] = HPAOffset[51:IG+IW] / 3
> +	 *	Extract upper bits from the correct bit range and divide by 3
> +	 *	to recover the original DPA upper bits

same here as above

DJ

> +	 */
> +	bits_lower = hpa_offset & GENMASK_ULL(eig + 7, 0);
> +	if (eiw < 8) {
> +		temp = hpa_offset &= ~((u64)GENMASK(eig + eiw + 8 - 1, 0));
> +		dpa_offset = temp >> eiw;
> +	} else {
> +		bits_upper = div64_u64(hpa_offset >> (eig + eiw), 3);
> +		dpa_offset = bits_upper << (eig + 8);
> +	}
> +	dpa_offset |= bits_lower;
> +
> +	return dpa_offset;
> +}
> +
> +int cxl_calculate_position(u64 hpa_offset, u8 eiw, u16 eig)
> +{
> +	unsigned int ways = 0;
> +	u64 shifted, rem;
> +	int pos;
> +
> +	/*
> +	 * Interleave position: CXL Spec 3.2 Section 8.2.4.20.13
> +	 * eiw < 8
> +	 *	Position is in the IW bits at HPA_OFFSET[IG+8+IW-1:IG+8].
> +	 *	Per spec "remove IW bits starting with bit position IG+8"
> +	 * eiw >= 8
> +	 *	Position is not explicitly stored in HPA_OFFSET bits. It is
> +	 *	derived from the modulo operation of the upper bits using
> +	 *	the total number of interleave ways.
> +	 */
> +	if (eiw < 8) {
> +		pos = (hpa_offset >> (eig + 8)) & GENMASK(eiw - 1, 0);
> +	} else {
> +		shifted = hpa_offset >> (eig + 8);
> +		eiw_to_ways(eiw, &ways);
> +		div64_u64_rem(shifted, ways, &rem);
> +		pos = rem;
> +	}
> +
> +	return pos;
> +}
> +
> +u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig)
> +{
> +	u64 mask_upper, hpa_offset, bits_upper;
> +
> +	mask_upper = GENMASK_ULL(51, eig + 8);
> +
> +	if (eiw < 8) {
> +		hpa_offset = (dpa_offset & mask_upper) << eiw;
> +		hpa_offset |= pos << (eig + 8);
> +	} else {
> +		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
> +		bits_upper = bits_upper * 3;
> +		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
> +	}
> +
> +	/* The lower bits remain unchanged */
> +	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
> +
> +	return hpa_offset;
> +}
> +
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa)
>  {
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> -	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
>  	struct cxl_region_params *p = &cxlr->params;
>  	struct cxl_endpoint_decoder *cxled = NULL;
> +	u64 dpa_offset, hpa_offset, hpa;
>  	u16 eig = 0;
>  	u8 eiw = 0;
>  	int pos;
> @@ -2965,19 +3043,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	/* Remove the dpa base */
>  	dpa_offset = dpa - cxl_dpa_resource_start(cxled);
>  
> -	mask_upper = GENMASK_ULL(51, eig + 8);
> -
> -	if (eiw < 8) {
> -		hpa_offset = (dpa_offset & mask_upper) << eiw;
> -		hpa_offset |= pos << (eig + 8);
> -	} else {
> -		bits_upper = (dpa_offset & mask_upper) >> (eig + 8);
> -		bits_upper = bits_upper * 3;
> -		hpa_offset = ((bits_upper << (eiw - 8)) + pos) << (eig + 8);
> -	}
> -
> -	/* The lower bits remain unchanged */
> -	hpa_offset |= dpa_offset & GENMASK_ULL(eig + 7, 0);
> +	/* this chunk was moved, maybe move comment too */
> +	hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, eiw, eig);
>  
>  	/* Apply the hpa_offset to the region base address */
>  	hpa = hpa_offset + p->res->start + p->cache_size;
> @@ -3011,8 +3078,6 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
>  	struct cxl_endpoint_decoder *cxled;
>  	u64 hpa, hpa_offset, dpa_offset;
> -	u64 bits_upper, bits_lower;
> -	u64 shifted, rem, temp;
>  	u16 eig = 0;
>  	u8 eiw = 0;
>  	int pos;
> @@ -3034,50 +3099,14 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	} else {
>  		hpa_offset = offset;
>  	}
> -	/*
> -	 * Interleave position: CXL Spec 3.2 Section 8.2.4.20.13
> -	 * eiw < 8
> -	 *	Position is in the IW bits at HPA_OFFSET[IG+8+IW-1:IG+8].
> -	 *	Per spec "remove IW bits starting with bit position IG+8"
> -	 * eiw >= 8
> -	 *	Position is not explicitly stored in HPA_OFFSET bits. It is
> -	 *	derived from the modulo operation of the upper bits using
> -	 *	the total number of interleave ways.
> -	 */
> -	if (eiw < 8) {
> -		pos = (hpa_offset >> (eig + 8)) & GENMASK(eiw - 1, 0);
> -	} else {
> -		shifted = hpa_offset >> (eig + 8);
> -		div64_u64_rem(shifted, p->interleave_ways, &rem);
> -		pos = rem;
> -	}
> -	if (pos < 0 || pos >= p->nr_targets) {
> -		dev_dbg(&cxlr->dev, "Invalid position %d for %d targets\n",
> -			pos, p->nr_targets);
> +
> +	pos = cxl_calculate_position(hpa_offset, eiw, eig);
> +	if (pos < 0 || pos >= p->nr_targets)
>  		return -ENXIO;
> -	}
>  
> -	/*
> -	 * DPA offset: CXL Spec 3.2 Section 8.2.4.20.13
> -	 * Lower bits [IG+7:0] pass through unchanged
> -	 * (eiw < 8)
> -	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
> -	 *	Clear the position bits to isolate upper section, then
> -	 *	reverse the left shift by eiw that occurred during DPA->HPA
> -	 * (eiw >= 8)
> -	 *	Per spec: DPAOffset[51:IG+8] = HPAOffset[51:IG+IW] / 3
> -	 *	Extract upper bits from the correct bit range and divide by 3
> -	 *	to recover the original DPA upper bits
> -	 */
> -	bits_lower = hpa_offset & GENMASK_ULL(eig + 7, 0);
> -	if (eiw < 8) {
> -		temp = hpa_offset &= ~((u64)GENMASK(eig + eiw + 8 - 1, 0));
> -		dpa_offset = temp >> eiw;
> -	} else {
> -		bits_upper = div64_u64(hpa_offset >> (eig + eiw), 3);
> -		dpa_offset = bits_upper << (eig + 8);
> -	}
> -	dpa_offset |= bits_lower;
> +	dpa_offset = cxl_calculate_dpa_offset(hpa_offset, eiw, eig);
> +	if (dpa_offset == ULLONG_MAX)
> +		return -ENXIO;
>  
>  	/* Look-up and return the result: a memdev and a DPA */
>  	for (int i = 0; i < p->nr_targets; i++) {
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index f87628a22852..151a8ce698c5 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -731,6 +731,9 @@ static inline bool is_cxl_root(struct cxl_port *port)
>  	return port->uport_dev == port->dev.parent;
>  }
>  
> +u64 cxl_calculate_hpa_offset(u64 dpa_offset, int pos, u8 eiw, u16 eig);
> +u64 cxl_calculate_dpa_offset(u64 hpa_offset, u8 eiw, u16 eig);
> +int cxl_calculate_position(u64 hpa_offset, u8 eiw, u16 eig);
>  int cxl_num_decoders_committed(struct cxl_port *port);
>  bool is_cxl_port(const struct device *dev);
>  struct cxl_port *to_cxl_port(const struct device *dev);

Re: [PATCH 1/3] cxl/region: Refactor address translation funcs for testing

[Alison Schofield]

On Mon, Aug 11, 2025 at 09:00:44AM -0700, Dave Jiang wrote:
> 
> 
> On 8/4/25 1:52 AM, alison.schofield@intel.com wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > In preparation for adding a test module that exercises the address
> > translation calculations, extract the core calculations into stand-
> > alone functions that operate on base parameters without dependencies
> > on struct cxl_region.
> > 
> > This refactoring enables unit testing of the address translation logic
> > with controlled inputs, while maintaining identical functionality in
> > the existing code paths.
> > 
> > The moved code has only one change. In the new cxl_calculate_position()
> > eiw_to_ways(eiw, &ways) replaces the prior usage of p->interleave_ways,
> > since the new function cannot depend upon struct cxl_region_params.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> > ---
> >  drivers/cxl/core/region.c | 145 +++++++++++++++++++++++---------------
> >  drivers/cxl/cxl.h         |   3 +
> >  2 files changed, 90 insertions(+), 58 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index 281a31df686f..ccee5049d700 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -2929,13 +2929,91 @@ static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
> >  	return cxlrd->ops && cxlrd->ops->spa_to_hpa;
> >  }
> >  
> > +u64 cxl_calculate_dpa_offset(u64 hpa_offset, u8 eiw, u16 eig)
> > +{
> > +	u64 dpa_offset, bits_lower, bits_upper, temp;
> > +
> > +	/*
> > +	 * DPA offset: CXL Spec 3.2 Section 8.2.4.20.13
> > +	 * Lower bits [IG+7:0] pass through unchanged
> > +	 * (eiw < 8)
> > +	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
> > +	 *	Clear the position bits to isolate upper section, then
> > +	 *	reverse the left shift by eiw that occurred during DPA->HPA
> 
> May be easier to read if listing the steps instead of continued sentence:
> 	 *	Per spec: DPAOffset[51:IG+8] = (HPAOffset[51:IG+IW+8] >> IW)
> 	 *	1. Clear the position bits to isolate upper section.
> 	 *	2. Reverse the left shift by eiw that occurred during DPA->HPA
> 
> 
> > +	 * (eiw >= 8)
> > +	 *	Per spec: DPAOffset[51:IG+8] = HPAOffset[51:IG+IW] / 3
> > +	 *	Extract upper bits from the correct bit range and divide by 3
> > +	 *	to recover the original DPA upper bits
> 
> same here as above

I didn't pick this up in v2 because it is in moved code, not new code,
so I didn't want to pollute the diff.  (Granted this diff is hard to
read already).

I'll keep this in mind if this needs touching in future revisions.

> 
> DJ
> 

[PATCH 2/3] cxl/acpi: Make the XOR calculations available for testing

[alison.schofield]

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

In preparation for adding a test module that can exercise the address
translation functions performed on XOR configured regions, the XOR
function needs to be refactored and the ability to create a CXIMS
provided.

Refactor the XOR function by extracting the core calculation into a
standalone function. Enhance the parameter validation since this new
function will be called from the test module where the parameters
may not be guaranteed valid.

To allow the test module to create a CXIMS without exposing the full
cxl_cxims_data structure, add new functions that can create and free
a CXIMS.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 drivers/cxl/acpi.c   | 51 ++++++++++++++++++++++++++++++++++++++------
 include/linux/acpi.h |  7 ++++++
 2 files changed, 52 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 547ed116a16f..86ca0466a95b 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -22,6 +22,7 @@ struct cxl_cxims_data {
  * translation is based on the Host Bridge interleave ways of the CXL Window.
  * CXL Spec Secion 9.18.1.4 and Table 9-22.
  */
+static const int valid_hbiw[] = { 1, 2, 3, 4, 6, 8, 12, 16 };
 static const int hbiw_to_nr_maps[] = {
 	[1] = 0, [2] = 1, [3] = 0, [4] = 2, [6] = 1, [8] = 3, [12] = 2, [16] = 4
 };
@@ -30,16 +31,47 @@ static const guid_t acpi_cxl_qtg_id_guid =
 	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
 		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
 
-static u64 cxl_apply_xor_maps(struct cxl_root_decoder *cxlrd, u64 addr)
+struct cxl_cxims_data *cxl_create_cxims_data(int nr_maps, u64 *xormaps)
 {
-	int nr_maps_to_apply = hbiw_to_nr_maps[cxlrd->cxlsd.nr_targets];
-	struct cxl_cxims_data *cximsd = cxlrd->platform_data;
+	struct cxl_cxims_data *cximsd;
+
+	if (nr_maps <= 0 || !xormaps)
+		return NULL;
+
+	cximsd = kzalloc(struct_size(cximsd, xormaps, nr_maps), GFP_KERNEL);
+	if (!cximsd)
+		return NULL;
+
+	memcpy(cximsd->xormaps, xormaps, nr_maps * sizeof(*cximsd->xormaps));
+	cximsd->nr_maps = nr_maps;
+
+	return cximsd;
+}
+
+void cxl_free_cxims_data(struct cxl_cxims_data *cximsd)
+{
+	kfree(cximsd);
+}
+
+u64 cxl_do_xormap_calc(struct cxl_cxims_data *cximsd, u64 addr, int hbiw)
+{
+	int nr_maps_to_apply = -1;
 	u64 val;
 	int pos;
 
-	/* No xormaps for host bridge interleave ways of 1 or 3 */
-	if (!nr_maps_to_apply)
-		return addr;
+	/*
+	 * Strictly validate hbiw since this function is used for testing and
+	 * that nullifies any expectation of trusted parameters from the CXL
+	 * Region Driver.
+	 */
+	for (int i = 0; i < ARRAY_SIZE(valid_hbiw); i++) {
+		if (valid_hbiw[i] == hbiw) {
+			nr_maps_to_apply = hbiw_to_nr_maps[hbiw];
+			break;
+		}
+	}
+	if (nr_maps_to_apply == -1 || nr_maps_to_apply > cximsd->nr_maps)
+		return ULLONG_MAX;
 
 	/*
 	 * In regions using XOR interleave arithmetic the CXL HPA may not
@@ -71,6 +103,13 @@ static u64 cxl_apply_xor_maps(struct cxl_root_decoder *cxlrd, u64 addr)
 	return addr;
 }
 
+static u64 cxl_apply_xor_maps(struct cxl_root_decoder *cxlrd, u64 addr)
+{
+	struct cxl_cxims_data *cximsd = cxlrd->platform_data;
+
+	return cxl_do_xormap_calc(cximsd, addr, cxlrd->cxlsd.nr_targets);
+}
+
 struct cxl_cxims_context {
 	struct device *dev;
 	struct cxl_root_decoder *cxlrd;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index f102c0fe3431..854578c6009f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1616,4 +1616,11 @@ static inline bool acpi_node_backed_by_real_pxm(int nid)
 }
 #endif
 
+#if IS_ENABLED(CONFIG_CXL_ACPI)
+struct cxl_cxims_data;
+void cxl_free_cxims_data(struct cxl_cxims_data *cximsd);
+struct cxl_cxims_data *cxl_create_cxims_data(int nr_maps, u64 *xormaps);
+u64 cxl_do_xormap_calc(struct cxl_cxims_data *cximsd, u64 addr, int hbiw);
+#endif
+
 #endif	/*_LINUX_ACPI_H*/
-- 
2.37.3

Re: [PATCH 2/3] cxl/acpi: Make the XOR calculations available for testing

[Jonathan Cameron]

On Mon,  4 Aug 2025 01:52:40 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> In preparation for adding a test module that can exercise the address
> translation functions performed on XOR configured regions, the XOR
> function needs to be refactored and the ability to create a CXIMS
> provided.
> 
> Refactor the XOR function by extracting the core calculation into a
> standalone function. Enhance the parameter validation since this new
> function will be called from the test module where the parameters
> may not be guaranteed valid.
> 
> To allow the test module to create a CXIMS without exposing the full
> cxl_cxims_data structure, add new functions that can create and free
> a CXIMS.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Hi Alison,

I'm not particularly keen on code that is just there to create opaque
stuff for tests that may or may not be built but I guess it's not huge
so fair enough.

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

Re: [PATCH 2/3] cxl/acpi: Make the XOR calculations available for testing

[Alison Schofield]

On Fri, Aug 08, 2025 at 05:19:21PM +0100, Jonathan Cameron wrote:
> On Mon,  4 Aug 2025 01:52:40 -0700
> alison.schofield@intel.com wrote:
> 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > In preparation for adding a test module that can exercise the address
> > translation functions performed on XOR configured regions, the XOR
> > function needs to be refactored and the ability to create a CXIMS
> > provided.
> > 
> > Refactor the XOR function by extracting the core calculation into a
> > standalone function. Enhance the parameter validation since this new
> > function will be called from the test module where the parameters
> > may not be guaranteed valid.
> > 
> > To allow the test module to create a CXIMS without exposing the full
> > cxl_cxims_data structure, add new functions that can create and free
> > a CXIMS.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> Hi Alison,
> 
> I'm not particularly keen on code that is just there to create opaque
> stuff for tests that may or may not be built but I guess it's not huge
> so fair enough.

Patch 3 is the test module that uses this, and the CXL unit test that
uses the test module is on the list for review too.

Thanks for all the reviews!

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

Re: [PATCH 2/3] cxl/acpi: Make the XOR calculations available for testing

[dan.j.williams]

alison.schofield@ wrote:
> From: Alison Schofield <alison.schofield@intel.com>
> 
> In preparation for adding a test module that can exercise the address
> translation functions performed on XOR configured regions, the XOR
> function needs to be refactored and the ability to create a CXIMS
> provided.
> 
> Refactor the XOR function by extracting the core calculation into a
> standalone function. Enhance the parameter validation since this new
> function will be called from the test module where the parameters
> may not be guaranteed valid.
> 
> To allow the test module to create a CXIMS without exposing the full
> cxl_cxims_data structure, add new functions that can create and free
> a CXIMS.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
[..]
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index f102c0fe3431..854578c6009f 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1616,4 +1616,11 @@ static inline bool acpi_node_backed_by_real_pxm(int nid)
>  }
>  #endif
>  
> +#if IS_ENABLED(CONFIG_CXL_ACPI)
> +struct cxl_cxims_data;
> +void cxl_free_cxims_data(struct cxl_cxims_data *cximsd);

This just wraps kfree(), call kfree() directly.

> +struct cxl_cxims_data *cxl_create_cxims_data(int nr_maps, u64 *xormaps);

The only caller of this is cxl_test, it can just be copied there, no
need for the core to carry it.

> +u64 cxl_do_xormap_calc(struct cxl_cxims_data *cximsd, u64 addr, int hbiw);

This function should be static outside of test builds. See __mock as an
example of optionally marking the function as not static. Move the
header declaration to tools/testing/cxl/ so that most readers can just
assume this function is static.

That said, __mock is currently used to optionally mark a function as
__weak, so maybe this wants a __mock_export for that purpose.

Re: [PATCH 2/3] cxl/acpi: Make the XOR calculations available for testing

[Alison Schofield]

On Tue, Aug 12, 2025 at 07:54:18PM -0700, Dan Williams wrote:
> alison.schofield@ wrote:
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > In preparation for adding a test module that can exercise the address
> > translation functions performed on XOR configured regions, the XOR
> > function needs to be refactored and the ability to create a CXIMS
> > provided.
> > 
> > Refactor the XOR function by extracting the core calculation into a
> > standalone function. Enhance the parameter validation since this new
> > function will be called from the test module where the parameters
> > may not be guaranteed valid.
> > 
> > To allow the test module to create a CXIMS without exposing the full
> > cxl_cxims_data structure, add new functions that can create and free
> > a CXIMS.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> [..]
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index f102c0fe3431..854578c6009f 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1616,4 +1616,11 @@ static inline bool acpi_node_backed_by_real_pxm(int nid)
> >  }
> >  #endif
> >  
> > +#if IS_ENABLED(CONFIG_CXL_ACPI)
> > +struct cxl_cxims_data;
> > +void cxl_free_cxims_data(struct cxl_cxims_data *cximsd);
> 
> This just wraps kfree(), call kfree() directly.

Done.

> 
> > +struct cxl_cxims_data *cxl_create_cxims_data(int nr_maps, u64 *xormaps);
> 
> The only caller of this is cxl_test, it can just be copied there, no
> need for the core to carry it.
> 

Done. To make that work I moved "struct cxl_cxims_data" definition from
cxl/acpi.c to include/linux/acpi.h for module to access.


> > +u64 cxl_do_xormap_calc(struct cxl_cxims_data *cximsd, u64 addr, int hbiw);
> 
> This function should be static outside of test builds. See __mock as an
> example of optionally marking the function as not static. Move the
> header declaration to tools/testing/cxl/ so that most readers can just
> assume this function is static.
> 
> That said, __mock is currently used to optionally mark a function as
> __weak, so maybe this wants a __mock_export for that purpose.

Added a __mock_export and used on all 4 funcs, with a new header 
tools/testing/cxl/cxl_test.h.  Please take a look in v2.

[PATCH 3/3] cxl/test: Add cxl_translate module for address translation testing

[alison.schofield]

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

Add a loadable test module that validates CXL address translation
calculations using parameterized test vectors. The module tests both
host to device and device to host address translations for Modulo and
XOR interleave arithmetic.

Test vectors are provided as module parameters in the format:
"dpa pos r_eiw r_eig hb_ways math expected_spa"

The module performs round-trip validation:
1. Translate a DPA and position to a SPA
2. Verify the result matches expected SPA
3. Translate that SPA back to a DPA and position
4. Verify round-trip consistency

The module accesses the refactored translation functions through the
exports made available only to CXL test modules.

Signed-off-by: Alison Schofield <alison.schofield@intel.com>
---
 tools/testing/cxl/Kbuild               |   1 +
 tools/testing/cxl/cxl_acpi_exports.c   |  13 ++
 tools/testing/cxl/cxl_core_exports.c   |  11 +
 tools/testing/cxl/test/Kbuild          |   2 +
 tools/testing/cxl/test/cxl_translate.c | 298 +++++++++++++++++++++++++
 5 files changed, 325 insertions(+)
 create mode 100644 tools/testing/cxl/cxl_acpi_exports.c
 create mode 100644 tools/testing/cxl/test/cxl_translate.c

diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
index d07f14cb7aa4..8b16fbb44858 100644
--- a/tools/testing/cxl/Kbuild
+++ b/tools/testing/cxl/Kbuild
@@ -29,6 +29,7 @@ cxl_acpi-y := $(CXL_SRC)/acpi.o
 cxl_acpi-y += mock_acpi.o
 cxl_acpi-y += config_check.o
 cxl_acpi-y += cxl_acpi_test.o
+cxl_acpi-y += cxl_acpi_exports.o
 
 obj-m += cxl_pmem.o
 
diff --git a/tools/testing/cxl/cxl_acpi_exports.c b/tools/testing/cxl/cxl_acpi_exports.c
new file mode 100644
index 000000000000..ea00772ce74d
--- /dev/null
+++ b/tools/testing/cxl/cxl_acpi_exports.c
@@ -0,0 +1,13 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
+
+#include <linux/acpi.h>
+
+/*
+ * Exporting of cxl_acpi (acpi.o) symbols that are only used by
+ * the test module cxl_translate.
+ */
+
+EXPORT_SYMBOL_NS_GPL(cxl_create_cxims_data, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_free_cxims_data, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_do_xormap_calc, "CXL");
diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
index f088792a8925..30c9284c26a5 100644
--- a/tools/testing/cxl/cxl_core_exports.c
+++ b/tools/testing/cxl/cxl_core_exports.c
@@ -5,3 +5,14 @@
 
 /* Exporting of cxl_core symbols that are only used by cxl_test */
 EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
+
+/*
+ * Exporting of cxl_core symbols used only by the cxl_translate module to test
+ * the CXL Region Drivers's address translation calculations.
+ *
+ * See tools/testing/cxl/cxl_translate.c
+ * See the CXL unit test cxl-translate.sh for usage
+ */
+EXPORT_SYMBOL_NS_GPL(cxl_calculate_hpa_offset, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_calculate_dpa_offset, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_calculate_position, "CXL");
diff --git a/tools/testing/cxl/test/Kbuild b/tools/testing/cxl/test/Kbuild
index 6b1927897856..d55973e61fdd 100644
--- a/tools/testing/cxl/test/Kbuild
+++ b/tools/testing/cxl/test/Kbuild
@@ -5,6 +5,8 @@ obj-m += cxl_test.o
 obj-m += cxl_mock.o
 obj-m += cxl_mock_mem.o
 
+obj-m += cxl_translate.o
+
 cxl_test-y := cxl.o
 cxl_mock-y := mock.o
 cxl_mock_mem-y := mem.o
diff --git a/tools/testing/cxl/test/cxl_translate.c b/tools/testing/cxl/test/cxl_translate.c
new file mode 100644
index 000000000000..b163ebc4a82b
--- /dev/null
+++ b/tools/testing/cxl/test/cxl_translate.c
@@ -0,0 +1,298 @@
+// SPDX-License-Identifier: GPL-2.0-only
+// Copyright(c) 2025 Intel Corporation. All rights reserved.
+
+#include <linux/moduleparam.h>
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/init.h>
+#include <linux/slab.h>
+#include <linux/acpi.h>
+#include <cxlmem.h>
+
+/* Maximum number of test vectors and entry length */
+#define MAX_TABLE_ENTRIES 128
+#define MAX_ENTRY_LEN 128
+
+/* Expected number of parameters in each test vector */
+#define EXPECTED_PARAMS 7
+
+/* Module parameters for test vectors */
+static char *table[MAX_TABLE_ENTRIES];
+static int table_num;
+
+/* Interleave Arithmetic */
+#define MODULO_MATH 0
+#define XOR_MATH 1
+
+/*
+ * XOR mapping configuration
+ * The test data sets all use the same set of xormaps. When additional
+ * data sets arrive for validation, this static setup will need to
+ * be changed to accept xormaps as additional parameters.
+ */
+struct cxl_cxims_data *cximsd;
+static u64 xormap_list[] = {
+	0x2020900,
+	0x4041200,
+	0x1010400,
+	0x800,
+};
+
+static int nr_maps = ARRAY_SIZE(xormap_list);
+
+/*
+ * to_hpa - translate a DPA offset and position to HPA offset
+ *
+ * @dpa_offset: device physical address offset
+ * @pos: devices position in interleave
+ * @r_eiw: region encoded interleave ways
+ * @r_eig: region encoded interleave granularity
+ * @hb_ways: host bridge interleave ways
+ * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
+ *
+ * Returns: host physical address offset
+ */
+static u64 to_hpa(u64 dpa_offset, int pos, u8 r_eiw, u16 r_eig, u8 hb_ways,
+		  u8 math)
+{
+	u64 hpa_offset;
+
+	/* Calculate base HPA offset from DPA and position */
+	hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, r_eiw, r_eig);
+
+	/* Apply XOR mapping if specified */
+	if (math == XOR_MATH)
+		hpa_offset = cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
+
+	return hpa_offset;
+}
+
+/*
+ * to_dpa - Convert HPA offset to DPA offset
+ *
+ * @hpa_offset: host physical address offset
+ * @r_eiw: region encoded interleave ways
+ * @r_eig: region encoded interleave granularity
+ * @hb_ways: host bridge interleave ways
+ * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
+ *
+ * Returns: device physical address offset
+ */
+static u64 to_dpa(u64 hpa_offset, u8 r_eiw, u16 r_eig, u8 hb_ways, u8 math)
+{
+	u64 offset = hpa_offset;
+
+	/* Reverse XOR mapping if specified */
+	if (math == XOR_MATH)
+		offset = cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
+
+	return cxl_calculate_dpa_offset(offset, r_eiw, r_eig);
+}
+
+/**
+ * to_pos - Convert HPA offset to interleave position
+ *
+ * @hpa_offset: host physical address offset
+ * @r_eiw: region encoded interleave ways
+ * @r_eig: region encoded interleave granularity
+ * @hb_ways: host bridge interleave ways
+ * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
+ *
+ * Returns: devices position in region interleave
+ */
+static u64 to_pos(u64 hpa_offset, u8 r_eiw, u16 r_eig, u8 hb_ways, u8 math)
+{
+	u64 offset = hpa_offset;
+
+	/* Reverse XOR mapping if specified */
+	if (math == XOR_MATH)
+		offset = cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
+
+	return cxl_calculate_position(offset, r_eiw, r_eig);
+}
+
+/*
+ * run_translation_test - Tests DPA->SPA and SPA->DPA & POS translations
+ *
+ * @dpa: device physical address
+ * @pos: expected position in region interleave
+ * @r_eiw: region encoded interleave ways
+ * @r_eig: region encoded interleave granularity
+ * @hb_ways: host bridge interleave ways
+ * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
+ * @expect_spa: expected system physical address
+ *
+ * Returns: 0 on success, -1 on failure
+ */
+static int run_translation_test(u64 dpa, int pos, u8 r_eiw, u16 r_eig,
+				u8 hb_ways, int math, u64 expect_hpa)
+{
+	u64 translated_spa, reverse_dpa;
+	int reverse_pos;
+
+	/* Test Device to Host translation: DPA + POS -> SPA */
+	translated_spa = to_hpa(dpa, pos, r_eiw, r_eig, hb_ways, math);
+	if (translated_spa != expect_hpa) {
+		pr_err("Device to host failed: expected HPA %llu, got %llu\n",
+		       expect_hpa, translated_spa);
+		return -1;
+	}
+
+	/* Test Host to Device DPA translation: SPA -> DPA */
+	reverse_dpa = to_dpa(translated_spa, r_eiw, r_eig, hb_ways, math);
+	if (reverse_dpa != dpa) {
+		pr_err("Host to Device DPA failed: expected %llu, got %llu\n",
+		       dpa, reverse_dpa);
+		return -1;
+	}
+
+	/* Test Host to Device Position translation: SPA -> POS */
+	reverse_pos = to_pos(translated_spa, r_eiw, r_eig, hb_ways, math);
+	if (reverse_pos != pos) {
+		pr_err("Position lookup failed: expected %d, got %d\n", pos,
+		       reverse_pos);
+		return -1;
+	}
+
+	return 0;
+}
+
+/*
+ * parse_test_vector - parse a single test vector string
+ *
+ * @entry: test vector string to parse
+ * @dpa: device physical address
+ * @pos: expected position in region interleave
+ * @r_eiw: region encoded interleave ways
+ * @r_eig: region encoded interleave granularity
+ * @hb_ways: host bridge interleave ways
+ * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
+ * @expect_spa: expected system physical address
+ *
+ * Returns: 0 on success, negative error code on failure
+ */
+static int parse_test_vector(const char *entry, u64 *dpa, int *pos, u8 *r_eiw,
+			     u16 *r_eig, u8 *hb_ways, int *math,
+			     u64 *expect_hpa)
+{
+	unsigned int tmp_r_eiw, tmp_r_eig, tmp_hb_ways;
+	int parsed;
+
+	parsed = sscanf(entry, "%llu %d %u %u %u %d %llu", dpa, pos, &tmp_r_eiw,
+			&tmp_r_eig, &tmp_hb_ways, math, expect_hpa);
+
+	if (parsed != EXPECTED_PARAMS) {
+		pr_err("Parse error: expected %d parameters, got %d in '%s'\n",
+		       EXPECTED_PARAMS, parsed, entry);
+		return -EINVAL;
+	}
+	if (tmp_r_eiw > U8_MAX || tmp_r_eig > U16_MAX || tmp_hb_ways > U8_MAX) {
+		pr_err("Parameter overflow in entry: '%s'\n", entry);
+		return -ERANGE;
+	}
+	if (*math != MODULO_MATH && *math != XOR_MATH) {
+		pr_err("Invalid math type %d in entry: '%s'\n", *math, entry);
+		return -EINVAL;
+	}
+	*r_eiw = tmp_r_eiw;
+	*r_eig = tmp_r_eig;
+	*hb_ways = tmp_hb_ways;
+
+	return 0;
+}
+
+/*
+ * setup_xor_mapping - Initialize XOR mapping data structure
+ *
+ * The test data sets all use the same set of xormaps. When additional
+ * data sets arrive for validation, this static setup will need to
+ * be changed to accept xormaps as additional parameters.
+ *
+ * Returns: 0 on success, negative error code on failure
+ */
+static int setup_xor_mapping(void)
+{
+	cximsd = cxl_create_cxims_data(nr_maps, xormap_list);
+	if (!cximsd) {
+		pr_err("Failed to create XOR mapping data\n");
+		return -ENOMEM;
+	}
+	return 0;
+}
+
+/*
+ * cxl_translate_init - parse test vectors and kicks off translation tests
+ *
+ * Returns: 0 on success, negative error code on failure
+ */
+static int __init cxl_translate_init(void)
+{
+	int ret, i;
+
+	/* Validate module parameters */
+	if (table_num == 0) {
+		pr_err("No test vectors provided\n");
+		return -EINVAL;
+	}
+
+	pr_info("CXL translate test module loaded with %d test vectors\n",
+		table_num);
+
+	ret = setup_xor_mapping();
+	if (ret)
+		return ret;
+
+	/* Process each test vector */
+	for (i = 0; i < table_num; i++) {
+		u64 dpa, expect_spa;
+		int pos, math;
+		u8 r_eiw, hb_ways;
+		u16 r_eig;
+
+		pr_debug("Processing test vector %d: '%s'\n", i, table[i]);
+
+		/* Parse the test vector */
+		ret = parse_test_vector(table[i], &dpa, &pos, &r_eiw, &r_eig,
+					&hb_ways, &math, &expect_spa);
+		if (ret) {
+			pr_err("CXL Translate Test %d: FAIL\n"
+			       "    Failed to parse test vector '%s'\n",
+			       i, table[i]);
+			continue;
+		}
+		/* Run the translation test */
+		ret = run_translation_test(dpa, pos, r_eiw, r_eig, hb_ways,
+					   math, expect_spa);
+		if (ret) {
+			pr_err("CXL Translate Test %d: FAIL\n"
+			       "    dpa=%llu pos=%d r_eiw=%u r_eig=%u hb_ways=%u math=%s expect_spa=%llu\n",
+			       i, dpa, pos, r_eiw, r_eig, hb_ways,
+			       (math == XOR_MATH) ? "XOR" : "MODULO",
+			       expect_spa);
+		} else {
+			pr_info("CXL Translate Test %d: PASS\n", i);
+		}
+	}
+
+	pr_info("CXL translate test completed\n");
+	return 0;
+}
+
+static void __exit cxl_translate_exit(void)
+{
+	if (cximsd) {
+		cxl_free_cxims_data(cximsd);
+		cximsd = NULL;
+	}
+	pr_info("CXL translate test module unloaded\n");
+}
+
+module_param_array(table, charp, &table_num, 0444);
+MODULE_PARM_DESC(table, "Test vectors as space-separated decimal strings");
+
+MODULE_LICENSE("GPL");
+MODULE_DESCRIPTION("cxl_test: cxl address translation test module");
+MODULE_IMPORT_NS("CXL");
+
+module_init(cxl_translate_init);
+module_exit(cxl_translate_exit);
-- 
2.37.3

Re: [PATCH 3/3] cxl/test: Add cxl_translate module for address translation testing

[Jonathan Cameron]

On Mon,  4 Aug 2025 01:52:41 -0700
alison.schofield@intel.com wrote:

> From: Alison Schofield <alison.schofield@intel.com>
> 
> Add a loadable test module that validates CXL address translation
> calculations using parameterized test vectors. The module tests both
> host to device and device to host address translations for Modulo and
> XOR interleave arithmetic.
> 
> Test vectors are provided as module parameters in the format:
> "dpa pos r_eiw r_eig hb_ways math expected_spa"
> 
> The module performs round-trip validation:
> 1. Translate a DPA and position to a SPA
> 2. Verify the result matches expected SPA
> 3. Translate that SPA back to a DPA and position
> 4. Verify round-trip consistency
> 
> The module accesses the refactored translation functions through the
> exports made available only to CXL test modules.
> 
> Signed-off-by: Alison Schofield <alison.schofield@intel.com>
Nice little test.  Trivial comments inline.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  tools/testing/cxl/Kbuild               |   1 +
>  tools/testing/cxl/cxl_acpi_exports.c   |  13 ++
>  tools/testing/cxl/cxl_core_exports.c   |  11 +
>  tools/testing/cxl/test/Kbuild          |   2 +
>  tools/testing/cxl/test/cxl_translate.c | 298 +++++++++++++++++++++++++
>  5 files changed, 325 insertions(+)
>  create mode 100644 tools/testing/cxl/cxl_acpi_exports.c
>  create mode 100644 tools/testing/cxl/test/cxl_translate.c
> 
> diff --git a/tools/testing/cxl/Kbuild b/tools/testing/cxl/Kbuild
> index d07f14cb7aa4..8b16fbb44858 100644
> --- a/tools/testing/cxl/Kbuild
> +++ b/tools/testing/cxl/Kbuild
> @@ -29,6 +29,7 @@ cxl_acpi-y := $(CXL_SRC)/acpi.o
>  cxl_acpi-y += mock_acpi.o
>  cxl_acpi-y += config_check.o
>  cxl_acpi-y += cxl_acpi_test.o
> +cxl_acpi-y += cxl_acpi_exports.o
>  
>  obj-m += cxl_pmem.o
>  
> diff --git a/tools/testing/cxl/cxl_acpi_exports.c b/tools/testing/cxl/cxl_acpi_exports.c
> new file mode 100644
> index 000000000000..ea00772ce74d
> --- /dev/null
> +++ b/tools/testing/cxl/cxl_acpi_exports.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright(c) 2022 Intel Corporation. All rights reserved. */
> +
> +#include <linux/acpi.h>
> +
> +/*
> + * Exporting of cxl_acpi (acpi.o) symbols that are only used by
> + * the test module cxl_translate.
> + */
> +
> +EXPORT_SYMBOL_NS_GPL(cxl_create_cxims_data, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_free_cxims_data, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_do_xormap_calc, "CXL");
> diff --git a/tools/testing/cxl/cxl_core_exports.c b/tools/testing/cxl/cxl_core_exports.c
> index f088792a8925..30c9284c26a5 100644
> --- a/tools/testing/cxl/cxl_core_exports.c
> +++ b/tools/testing/cxl/cxl_core_exports.c
> @@ -5,3 +5,14 @@
>  
>  /* Exporting of cxl_core symbols that are only used by cxl_test */
>  EXPORT_SYMBOL_NS_GPL(cxl_num_decoders_committed, "CXL");
> +
> +/*
> + * Exporting of cxl_core symbols used only by the cxl_translate module to test
> + * the CXL Region Drivers's address translation calculations.
> + *
> + * See tools/testing/cxl/cxl_translate.c
> + * See the CXL unit test cxl-translate.sh for usage
> + */
> +EXPORT_SYMBOL_NS_GPL(cxl_calculate_hpa_offset, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_calculate_dpa_offset, "CXL");
> +EXPORT_SYMBOL_NS_GPL(cxl_calculate_position, "CXL");
> diff --git a/tools/testing/cxl/test/Kbuild b/tools/testing/cxl/test/Kbuild
> index 6b1927897856..d55973e61fdd 100644
> --- a/tools/testing/cxl/test/Kbuild
> +++ b/tools/testing/cxl/test/Kbuild
> @@ -5,6 +5,8 @@ obj-m += cxl_test.o
>  obj-m += cxl_mock.o
>  obj-m += cxl_mock_mem.o
>  
> +obj-m += cxl_translate.o
> +
>  cxl_test-y := cxl.o
>  cxl_mock-y := mock.o
>  cxl_mock_mem-y := mem.o
> diff --git a/tools/testing/cxl/test/cxl_translate.c b/tools/testing/cxl/test/cxl_translate.c
> new file mode 100644
> index 000000000000..b163ebc4a82b
> --- /dev/null
> +++ b/tools/testing/cxl/test/cxl_translate.c
> @@ -0,0 +1,298 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +// Copyright(c) 2025 Intel Corporation. All rights reserved.
> +
> +#include <linux/moduleparam.h>
> +#include <linux/module.h>
> +#include <linux/kernel.h>
> +#include <linux/init.h>
> +#include <linux/slab.h>
> +#include <linux/acpi.h>
> +#include <cxlmem.h>
> +
> +/* Maximum number of test vectors and entry length */
> +#define MAX_TABLE_ENTRIES 128
> +#define MAX_ENTRY_LEN 128
> +
> +/* Expected number of parameters in each test vector */
> +#define EXPECTED_PARAMS 7
> +
> +/* Module parameters for test vectors */
> +static char *table[MAX_TABLE_ENTRIES];
> +static int table_num;
> +
> +/* Interleave Arithmetic */
> +#define MODULO_MATH 0
> +#define XOR_MATH 1
> +
> +/*
> + * XOR mapping configuration
> + * The test data sets all use the same set of xormaps. When additional
> + * data sets arrive for validation, this static setup will need to
> + * be changed to accept xormaps as additional parameters.
> + */
> +struct cxl_cxims_data *cximsd;
> +static u64 xormap_list[] = {
> +	0x2020900,
> +	0x4041200,
> +	0x1010400,
> +	0x800,
> +};
> +
> +static int nr_maps = ARRAY_SIZE(xormap_list);
> +
> +/*

Might as well make this kernel-doc even if nothing is going to
build these docs currently.  You can probably still point
the scripts at the file and to check docs are complete etc.

> + * to_hpa - translate a DPA offset and position to HPA offset
> + *
> + * @dpa_offset: device physical address offset
> + * @pos: devices position in interleave
> + * @r_eiw: region encoded interleave ways
> + * @r_eig: region encoded interleave granularity
> + * @hb_ways: host bridge interleave ways
> + * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
> + *
> + * Returns: host physical address offset
> + */
> +static u64 to_hpa(u64 dpa_offset, int pos, u8 r_eiw, u16 r_eig, u8 hb_ways,
> +		  u8 math)
> +{
> +	u64 hpa_offset;
> +
> +	/* Calculate base HPA offset from DPA and position */
> +	hpa_offset = cxl_calculate_hpa_offset(dpa_offset, pos, r_eiw, r_eig);
> +
> +	/* Apply XOR mapping if specified */
> +	if (math == XOR_MATH)
> +		hpa_offset = cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
> +
> +	return hpa_offset;
> +}
> +
> +/*
Likewise on kernel-doc /**

> + * to_dpa - Convert HPA offset to DPA offset
> + *
> + * @hpa_offset: host physical address offset
> + * @r_eiw: region encoded interleave ways
> + * @r_eig: region encoded interleave granularity
> + * @hb_ways: host bridge interleave ways
> + * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
> + *
> + * Returns: device physical address offset
> + */
> +static u64 to_dpa(u64 hpa_offset, u8 r_eiw, u16 r_eig, u8 hb_ways, u8 math)
> +{
> +	u64 offset = hpa_offset;
> +
> +	/* Reverse XOR mapping if specified */
> +	if (math == XOR_MATH)
> +		offset = cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
> +
> +	return cxl_calculate_dpa_offset(offset, r_eiw, r_eig);
> +}
> +
> +/**

Not sure why this one was special.

> + * to_pos - Convert HPA offset to interleave position
> + *
> + * @hpa_offset: host physical address offset
> + * @r_eiw: region encoded interleave ways
> + * @r_eig: region encoded interleave granularity
> + * @hb_ways: host bridge interleave ways
> + * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
> + *
> + * Returns: devices position in region interleave
> + */
> +static u64 to_pos(u64 hpa_offset, u8 r_eiw, u16 r_eig, u8 hb_ways, u8 math)
> +{
> +	u64 offset = hpa_offset;
> +
> +	/* Reverse XOR mapping if specified */
> +	if (math == XOR_MATH)
> +		offset = cxl_do_xormap_calc(cximsd, hpa_offset, hb_ways);
> +
> +	return cxl_calculate_position(offset, r_eiw, r_eig);
> +}
> +

Re: [PATCH 3/3] cxl/test: Add cxl_translate module for address translation testing

[Alison Schofield]

On Fri, Aug 08, 2025 at 05:24:19PM +0100, Jonathan Cameron wrote:
> On Mon,  4 Aug 2025 01:52:41 -0700
> alison.schofield@intel.com wrote:
> 
> > From: Alison Schofield <alison.schofield@intel.com>
> > 
> > Add a loadable test module that validates CXL address translation
> > calculations using parameterized test vectors. The module tests both
> > host to device and device to host address translations for Modulo and
> > XOR interleave arithmetic.
> > 
> > Test vectors are provided as module parameters in the format:
> > "dpa pos r_eiw r_eig hb_ways math expected_spa"
> > 
> > The module performs round-trip validation:
> > 1. Translate a DPA and position to a SPA
> > 2. Verify the result matches expected SPA
> > 3. Translate that SPA back to a DPA and position
> > 4. Verify round-trip consistency
> > 
> > The module accesses the refactored translation functions through the
> > exports made available only to CXL test modules.
> > 
> > Signed-off-by: Alison Schofield <alison.schofield@intel.com>
> Nice little test.  Trivial comments inline.
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> 
:)

snip

> 
> Might as well make this kernel-doc even if nothing is going to
> build these docs currently.  You can probably still point
> the scripts at the file and to check docs are complete etc.

Will do for all of these and tidy'd up the format too.

> 
> > + * to_hpa - translate a DPA offset and position to HPA offset
> > + *
> > + * @dpa_offset: device physical address offset
> > + * @pos: devices position in interleave
> > + * @r_eiw: region encoded interleave ways
> > + * @r_eig: region encoded interleave granularity
> > + * @hb_ways: host bridge interleave ways
> > + * @math: interleave arithmetic (MODULO_MATH or XOR_MATH)
> > + *
> > + * Returns: host physical address offset
> > + */
> > +static u64 to_hpa(u64 dpa_offset, int pos, u8 r_eiw, u16 r_eig, u8 hb_ways,
> > +		  u8 math)

snip