[PATCH v4 00/14] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement

[PATCH v4 00/14] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement

[Robert Richter]

This patch set adds support for address translation using ACPI PRM and
enables this for AMD Zen5 platforms. This is another new appoach in
response to earlier attempts to implement CXL address translation:

 * v1: [1] and the comments on it, esp. Dan's [2],
 * v2: [3] and comments on [4], esp. Dave's [5]
 * v3: [6] and comments on it, esp. Dave's [7]

This version 4 reworks and simplifies code to use an address
translation callback bound to the root port. It moves all address
translation code to a single file, core/atl.c.

Documentation of CXL Address Translation Support will be added to the
Kernel's "Compute Express Link: Linux Conventions". This patch
submission will be the base for a documention patch that describes CXL
Address Translation support accordingly.

The CXL driver currently does not implement address translation which
assumes the host physical addresses (HPA) and system physical
addresses (SPA) are equal.

Systems with different HPA and SPA addresses need address translation.
If this is the case, the hardware addresses esp. used in the HDM
decoder configurations are different to the system's or parent port
address ranges. E.g. AMD Zen5 systems may be configured to use
'Normalized addresses'. Then, CXL endpoints have their own physical
address base which is not the same as the SPA used by the CXL host
bridge. Thus, addresses need to be translated from the endpoint's to
its CXL host bridge's address range.

To enable address translation, the endpoint's HPA range must be
translated to the CXL host bridge's address range. A callback is
introduced to translate a decoder's HPA to the CXL host bridge's
address range. The callback is then used to determine the region
parameters which includes the SPA translated address range of the
endpoint decoder and the interleaving configuration. This is stored in
struct cxl_region which allows an endpoint decoder to determine that
parameters based on its assigned region.

Note that only auto-discovery of decoders is supported. Thus, decoders
are locked and cannot be configured manually.

Finally, Zen5 address translation is enabled using ACPI PRMT.

There are 3 additional cleanup patches at the end of the series. It
might be worth to add them too, but could be dropped if there are
concerns.

This series bases on cxl/next.

V4:
 * rebased onto v6.18-rc2 (cxl/next),
 * updated sob-chain,
 * reworked and simplified code to use an address translation callback
   bound to the root port,
 * moved all address translation code to core/atl.c,
 * cxlr->cxlrd change, updated patch description (Alison),
 * use DEFINE_RANGE() (Jonathan),
 * change name to @hpa_range (Dave, Jonathan),
 * updated patch description if there is a no-op (Gregory),
 * use Designated initializers for struct cxl_region_context (Dave),
 * move callback handler to struct cxl_root_ops (Dave),
 * move hanler inialization to acpi_probe() (Dave),
 * updated comment where Normalized Addressing is checked (Dave),
 * limit PRM enablement only to AMD supported kernel configs (AMD_NB)
   (Jonathan),
 * added 3 related optional cleanup patches at the end of the series,

V3:
 * rebased onto cxl/next,
 * complete rework to reduce number of required changes/patches and to
   remove platform specific code (Dan and Dave),
 * changed implementation allowing to add address translation to the
   CXL specification (documention patch in preparation),
 * simplified and generalized determination of interleaving
   parameters using the address translation callback,
 * depend only on the existence of the ACPI PRM GUID for CXL Address
   Translation enablement, removed platform checks,
 * small changes to region code only which does not require a full
   rework and refactoring of the code, just separating region
   parameter setup and region construction,
 * moved code to new core/atl.c file,
 * fixed subsys_initcall order dependency of EFI runtime services
   (Gregory and Joshua),

V2:
 * rebased onto cxl/next,
 * split of v1 in two parts:
   * removed cleanups and updates from this series to post them as a
     separate series (Dave),
   * this part 2 applies on top of part 1, v3,
 * added tags to SOB chain,
 * reworked architecture, vendor and platform setup (Jonathan):
   * added patch "cxl/x86: Prepare for architectural platform setup",
   * added function arch_cxl_port_platform_setup() plus a __weak
     versions for archs other than x86,
   * moved code to core/x86,
 * added comment to cxl_to_hpa_fn (Ben),
 * updated year in copyright statement (Ben),
 * cxl_port_calc_hpa(): Removed HPA check for zero (Jonathan), return
   1 if modified,
 * cxl_port_calc_pos(): Updated description and wording (Ben),
 * added sereral patches around interleaving and SPA calculation in
   cxl_endpoint_decoder_initialize(),
 * reworked iterator in cxl_endpoint_decoder_initialize() (Gregory),
 * fixed region interleaving parameters() (Alison),
 * fixed check in cxl_region_attach() (Alison),
 * Clarified in coverletter that not all ports in a system must
   implement the to_hpa() callback (Terry).

[1] https://lore.kernel.org/linux-cxl/20240701174754.967954-1-rrichter@amd.com/
[2] https://lore.kernel.org/linux-cxl/669086821f136_5fffa29473@dwillia2-xfh.jf.intel.com.notmuch/
[3] https://patchwork.kernel.org/project/cxl/cover/20250218132356.1809075-1-rrichter@amd.com/
[4] https://patchwork.kernel.org/project/cxl/cover/20250715191143.1023512-1-rrichter@amd.com/
[5] https://lore.kernel.org/all/78284b12-3e0b-4758-af18-397f32136c3f@intel.com/
[6] https://patchwork.kernel.org/project/cxl/cover/20250912144514.526441-1-rrichter@amd.com/
[7] https://lore.kernel.org/all/20250912144514.526441-8-rrichter@amd.com/T/#m23c2adb9d1e20770ccd5d11475288bda382b0af5

Robert Richter (14):
  cxl/region: Store root decoder in struct cxl_region
  cxl/region: Store HPA range in struct cxl_region
  cxl/region: Rename misleading variable name @hpa to @hpa_range
  cxl/region: Add @hpa_range argument to function
    cxl_calc_interleave_pos()
  cxl: Simplify cxl_root_ops allocation and handling
  cxl/region: Separate region parameter setup and region construction
  cxl/region: Use region data to get the root decoder
  cxl: Introduce callback for HPA address ranges translation
  cxl/acpi: Prepare use of EFI runtime services
  cxl: Enable AMD Zen5 address translation using ACPI PRMT
  cxl/atl: Lock decoders that need address translation
  cxl: Simplify cxl_rd_ops allocation and handling
  cxl/acpi: Group xor arithmetric setup code in a single block
  cxl/region: Remove local variable @inc in cxl_port_setup_targets()

 drivers/cxl/Kconfig       |   4 +
 drivers/cxl/acpi.c        |  32 +++---
 drivers/cxl/core/Makefile |   1 +
 drivers/cxl/core/atl.c    | 205 ++++++++++++++++++++++++++++++++++++++
 drivers/cxl/core/cdat.c   |   8 +-
 drivers/cxl/core/core.h   |   9 ++
 drivers/cxl/core/port.c   |   9 +-
 drivers/cxl/core/region.c | 185 +++++++++++++++++++---------------
 drivers/cxl/cxl.h         |  33 ++++--
 9 files changed, 366 insertions(+), 120 deletions(-)
 create mode 100644 drivers/cxl/core/atl.c


base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
-- 
2.47.3

[PATCH v4 01/14] cxl/region: Store root decoder in struct cxl_region

[Robert Richter]

A region is always bound to a root decoder. The region's associated
root decoder is often needed. Add it to struct cxl_region.

This simplifies the code by removing dynamic lookups and the root
decoder argument from the function argument list where possible.

Patch is a prerequisite to implement address translation which uses
struct cxl_region to store all relevant region and interleaving
parameters. It changes the argument list of __construct_region() in
preparation of adding a context argument. Additionally the arg list of
cxl_region_attach_position() is simplified and the use of
to_cxl_root_decoder() removed, which always reconstructs and checks
the pointer. The pointer never changes and is frequently used. Code
becomes more readable as this amphazises the binding between both
objects.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 37 +++++++++++++++++++------------------
 drivers/cxl/cxl.h         |  2 ++
 2 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index b06fee1978ba..45b1386a18d7 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -495,9 +495,9 @@ static ssize_t interleave_ways_store(struct device *dev,
 				     struct device_attribute *attr,
 				     const char *buf, size_t len)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
-	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
 	struct cxl_region *cxlr = to_cxl_region(dev);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
+	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
 	struct cxl_region_params *p = &cxlr->params;
 	unsigned int val, save;
 	int rc;
@@ -558,9 +558,9 @@ static ssize_t interleave_granularity_store(struct device *dev,
 					    struct device_attribute *attr,
 					    const char *buf, size_t len)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
-	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
 	struct cxl_region *cxlr = to_cxl_region(dev);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
+	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
 	struct cxl_region_params *p = &cxlr->params;
 	int rc, val;
 	u16 ig;
@@ -634,7 +634,7 @@ static DEVICE_ATTR_RO(mode);
 
 static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_region_params *p = &cxlr->params;
 	struct resource *res;
 	u64 remainder = 0;
@@ -1327,7 +1327,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 				  struct cxl_region *cxlr,
 				  struct cxl_endpoint_decoder *cxled)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	int parent_iw, parent_ig, ig, iw, rc, inc = 0, pos = cxled->pos;
 	struct cxl_port *parent_port = to_cxl_port(port->dev.parent);
 	struct cxl_region_ref *cxl_rr = cxl_rr_load(port, cxlr);
@@ -1686,10 +1686,10 @@ static int cxl_region_validate_position(struct cxl_region *cxlr,
 }
 
 static int cxl_region_attach_position(struct cxl_region *cxlr,
-				      struct cxl_root_decoder *cxlrd,
 				      struct cxl_endpoint_decoder *cxled,
 				      const struct cxl_dport *dport, int pos)
 {
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
 	struct cxl_decoder *cxld = &cxlsd->cxld;
@@ -1926,7 +1926,7 @@ static int cxl_region_sort_targets(struct cxl_region *cxlr)
 static int cxl_region_attach(struct cxl_region *cxlr,
 			     struct cxl_endpoint_decoder *cxled, int pos)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	struct cxl_region_params *p = &cxlr->params;
@@ -2031,8 +2031,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 			ep_port = cxled_to_port(cxled);
 			dport = cxl_find_dport_by_dev(root_port,
 						      ep_port->host_bridge);
-			rc = cxl_region_attach_position(cxlr, cxlrd, cxled,
-							dport, i);
+			rc = cxl_region_attach_position(cxlr, cxled, dport, i);
 			if (rc)
 				return rc;
 		}
@@ -2055,7 +2054,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
 	if (rc)
 		return rc;
 
-	rc = cxl_region_attach_position(cxlr, cxlrd, cxled, dport, pos);
+	rc = cxl_region_attach_position(cxlr, cxled, dport, pos);
 	if (rc)
 		return rc;
 
@@ -2351,8 +2350,8 @@ static const struct attribute_group *region_groups[] = {
 
 static void cxl_region_release(struct device *dev)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
 	struct cxl_region *cxlr = to_cxl_region(dev);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	int id = atomic_read(&cxlrd->region_id);
 
 	/*
@@ -2435,10 +2434,12 @@ static struct cxl_region *cxl_region_alloc(struct cxl_root_decoder *cxlrd, int i
 	 * region id allocations
 	 */
 	get_device(dev->parent);
+	cxlr->cxlrd = cxlrd;
+	cxlr->id = id;
+
 	device_set_pm_not_required(dev);
 	dev->bus = &cxl_bus_type;
 	dev->type = &cxl_region_type;
-	cxlr->id = id;
 
 	return cxlr;
 }
@@ -2937,7 +2938,7 @@ static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
 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);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
 	struct cxl_region_params *p = &cxlr->params;
 	struct cxl_endpoint_decoder *cxled = NULL;
@@ -3013,7 +3014,7 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
 				       struct dpa_result *result)
 {
 	struct cxl_region_params *p = &cxlr->params;
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_endpoint_decoder *cxled;
 	u64 hpa, hpa_offset, dpa_offset;
 	u64 bits_upper, bits_lower;
@@ -3404,7 +3405,7 @@ static int match_region_by_range(struct device *dev, const void *data)
 static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
 					    struct resource *res)
 {
-	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_region_params *p = &cxlr->params;
 	resource_size_t size = resource_size(res);
 	resource_size_t cache_size, start;
@@ -3440,9 +3441,9 @@ static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
 }
 
 static int __construct_region(struct cxl_region *cxlr,
-			      struct cxl_root_decoder *cxlrd,
 			      struct cxl_endpoint_decoder *cxled)
 {
+	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	struct range *hpa = &cxled->cxld.hpa_range;
 	struct cxl_region_params *p;
@@ -3534,7 +3535,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 		return cxlr;
 	}
 
-	rc = __construct_region(cxlr, cxlrd, cxled);
+	rc = __construct_region(cxlr, cxled);
 	if (rc) {
 		devm_release_action(port->uport_dev, unregister_region, cxlr);
 		return ERR_PTR(rc);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 231ddccf8977..19b8b62a1322 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -521,6 +521,7 @@ enum cxl_partition_mode {
  * struct cxl_region - CXL region
  * @dev: This region's device
  * @id: This region's id. Id is globally unique across all regions
+ * @cxlrd: Region's root decoder
  * @mode: Operational mode of the mapped capacity
  * @type: Endpoint decoder target type
  * @cxl_nvb: nvdimm bridge for coordinating @cxlr_pmem setup / shutdown
@@ -534,6 +535,7 @@ enum cxl_partition_mode {
 struct cxl_region {
 	struct device dev;
 	int id;
+	struct cxl_root_decoder *cxlrd;
 	enum cxl_partition_mode mode;
 	enum cxl_decoder_type type;
 	struct cxl_nvdimm_bridge *cxl_nvb;
-- 
2.47.3

Re: [PATCH v4 01/14] cxl/region: Store root decoder in struct cxl_region

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:42 +0100
Robert Richter <rrichter@amd.com> wrote:

> A region is always bound to a root decoder. The region's associated
> root decoder is often needed. Add it to struct cxl_region.
> 
> This simplifies the code by removing dynamic lookups and the root
> decoder argument from the function argument list where possible.
> 
> Patch is a prerequisite to implement address translation which uses
> struct cxl_region to store all relevant region and interleaving
> parameters. It changes the argument list of __construct_region() in
> preparation of adding a context argument. Additionally the arg list of
> cxl_region_attach_position() is simplified and the use of
> to_cxl_root_decoder() removed, which always reconstructs and checks
> the pointer. The pointer never changes and is frequently used. Code
> becomes more readable as this amphazises the binding between both
> objects.
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>

Just a note to say this will (maybe just context) clash with Alejandro's rework
around construct_region_begin()

https://lore.kernel.org/all/20251110153657.2706192-19-alejandro.lucero-palau@amd.com/

Probably easy to resolve, but worth both of you being aware if you hadn't noticed
already!

Jonathan

> ---
>  drivers/cxl/core/region.c | 37 +++++++++++++++++++------------------
>  drivers/cxl/cxl.h         |  2 ++
>  2 files changed, 21 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index b06fee1978ba..45b1386a18d7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -495,9 +495,9 @@ static ssize_t interleave_ways_store(struct device *dev,
>  				     struct device_attribute *attr,
>  				     const char *buf, size_t len)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
> -	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
>  	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
>  	struct cxl_region_params *p = &cxlr->params;
>  	unsigned int val, save;
>  	int rc;
> @@ -558,9 +558,9 @@ static ssize_t interleave_granularity_store(struct device *dev,
>  					    struct device_attribute *attr,
>  					    const char *buf, size_t len)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
> -	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
>  	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> +	struct cxl_decoder *cxld = &cxlrd->cxlsd.cxld;
>  	struct cxl_region_params *p = &cxlr->params;
>  	int rc, val;
>  	u16 ig;
> @@ -634,7 +634,7 @@ static DEVICE_ATTR_RO(mode);
>  
>  static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_region_params *p = &cxlr->params;
>  	struct resource *res;
>  	u64 remainder = 0;
> @@ -1327,7 +1327,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  				  struct cxl_region *cxlr,
>  				  struct cxl_endpoint_decoder *cxled)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	int parent_iw, parent_ig, ig, iw, rc, inc = 0, pos = cxled->pos;
>  	struct cxl_port *parent_port = to_cxl_port(port->dev.parent);
>  	struct cxl_region_ref *cxl_rr = cxl_rr_load(port, cxlr);
> @@ -1686,10 +1686,10 @@ static int cxl_region_validate_position(struct cxl_region *cxlr,
>  }
>  
>  static int cxl_region_attach_position(struct cxl_region *cxlr,
> -				      struct cxl_root_decoder *cxlrd,
>  				      struct cxl_endpoint_decoder *cxled,
>  				      const struct cxl_dport *dport, int pos)
>  {
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	struct cxl_switch_decoder *cxlsd = &cxlrd->cxlsd;
>  	struct cxl_decoder *cxld = &cxlsd->cxld;
> @@ -1926,7 +1926,7 @@ static int cxl_region_sort_targets(struct cxl_region *cxlr)
>  static int cxl_region_attach(struct cxl_region *cxlr,
>  			     struct cxl_endpoint_decoder *cxled, int pos)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	struct cxl_region_params *p = &cxlr->params;
> @@ -2031,8 +2031,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  			ep_port = cxled_to_port(cxled);
>  			dport = cxl_find_dport_by_dev(root_port,
>  						      ep_port->host_bridge);
> -			rc = cxl_region_attach_position(cxlr, cxlrd, cxled,
> -							dport, i);
> +			rc = cxl_region_attach_position(cxlr, cxled, dport, i);
>  			if (rc)
>  				return rc;
>  		}
> @@ -2055,7 +2054,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  	if (rc)
>  		return rc;
>  
> -	rc = cxl_region_attach_position(cxlr, cxlrd, cxled, dport, pos);
> +	rc = cxl_region_attach_position(cxlr, cxled, dport, pos);
>  	if (rc)
>  		return rc;
>  
> @@ -2351,8 +2350,8 @@ static const struct attribute_group *region_groups[] = {
>  
>  static void cxl_region_release(struct device *dev)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(dev->parent);
>  	struct cxl_region *cxlr = to_cxl_region(dev);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	int id = atomic_read(&cxlrd->region_id);
>  
>  	/*
> @@ -2435,10 +2434,12 @@ static struct cxl_region *cxl_region_alloc(struct cxl_root_decoder *cxlrd, int i
>  	 * region id allocations
>  	 */
>  	get_device(dev->parent);
> +	cxlr->cxlrd = cxlrd;
> +	cxlr->id = id;
> +
>  	device_set_pm_not_required(dev);
>  	dev->bus = &cxl_bus_type;
>  	dev->type = &cxl_region_type;
> -	cxlr->id = id;
>  
>  	return cxlr;
>  }
> @@ -2937,7 +2938,7 @@ static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
>  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);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	u64 dpa_offset, hpa_offset, bits_upper, mask_upper, hpa;
>  	struct cxl_region_params *p = &cxlr->params;
>  	struct cxl_endpoint_decoder *cxled = NULL;
> @@ -3013,7 +3014,7 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  				       struct dpa_result *result)
>  {
>  	struct cxl_region_params *p = &cxlr->params;
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_endpoint_decoder *cxled;
>  	u64 hpa, hpa_offset, dpa_offset;
>  	u64 bits_upper, bits_lower;
> @@ -3404,7 +3405,7 @@ static int match_region_by_range(struct device *dev, const void *data)
>  static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
>  					    struct resource *res)
>  {
> -	struct cxl_root_decoder *cxlrd = to_cxl_root_decoder(cxlr->dev.parent);
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_region_params *p = &cxlr->params;
>  	resource_size_t size = resource_size(res);
>  	resource_size_t cache_size, start;
> @@ -3440,9 +3441,9 @@ static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
>  }
>  
>  static int __construct_region(struct cxl_region *cxlr,
> -			      struct cxl_root_decoder *cxlrd,
>  			      struct cxl_endpoint_decoder *cxled)
>  {
> +	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	struct range *hpa = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
> @@ -3534,7 +3535,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  		return cxlr;
>  	}
>  
> -	rc = __construct_region(cxlr, cxlrd, cxled);
> +	rc = __construct_region(cxlr, cxled);
>  	if (rc) {
>  		devm_release_action(port->uport_dev, unregister_region, cxlr);
>  		return ERR_PTR(rc);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 231ddccf8977..19b8b62a1322 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -521,6 +521,7 @@ enum cxl_partition_mode {
>   * struct cxl_region - CXL region
>   * @dev: This region's device
>   * @id: This region's id. Id is globally unique across all regions
> + * @cxlrd: Region's root decoder
>   * @mode: Operational mode of the mapped capacity
>   * @type: Endpoint decoder target type
>   * @cxl_nvb: nvdimm bridge for coordinating @cxlr_pmem setup / shutdown
> @@ -534,6 +535,7 @@ enum cxl_partition_mode {
>  struct cxl_region {
>  	struct device dev;
>  	int id;
> +	struct cxl_root_decoder *cxlrd;
>  	enum cxl_partition_mode mode;
>  	enum cxl_decoder_type type;
>  	struct cxl_nvdimm_bridge *cxl_nvb;

Re: [PATCH v4 01/14] cxl/region: Store root decoder in struct cxl_region

[Robert Richter]

On 11.11.25 14:45:41, Jonathan Cameron wrote:
> On Mon, 3 Nov 2025 19:47:42 +0100
> Robert Richter <rrichter@amd.com> wrote:
> 
> > A region is always bound to a root decoder. The region's associated
> > root decoder is often needed. Add it to struct cxl_region.
> > 
> > This simplifies the code by removing dynamic lookups and the root
> > decoder argument from the function argument list where possible.
> > 
> > Patch is a prerequisite to implement address translation which uses
> > struct cxl_region to store all relevant region and interleaving
> > parameters. It changes the argument list of __construct_region() in
> > preparation of adding a context argument. Additionally the arg list of
> > cxl_region_attach_position() is simplified and the use of
> > to_cxl_root_decoder() removed, which always reconstructs and checks
> > the pointer. The pointer never changes and is frequently used. Code
> > becomes more readable as this amphazises the binding between both
> > objects.
> > 
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> > Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> > Reviewed-by: Gregory Price <gourry@gourry.net>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> 
> Just a note to say this will (maybe just context) clash with Alejandro's rework
> around construct_region_begin()
> 
> https://lore.kernel.org/all/20251110153657.2706192-19-alejandro.lucero-palau@amd.com/
> 
> Probably easy to resolve, but worth both of you being aware if you hadn't noticed
> already!

I have checked the conflicts and they look trivial. There are no
"functional" conflicts.

Thanks for pointing out.

-Robert

[PATCH v4 02/14] cxl/region: Store HPA range in struct cxl_region

[Robert Richter]

Each region has a known host physical address (HPA) range it is
assigned to. Endpoint decoders assigned to a region share the same HPA
range. The region's address range is the system's physical address
(SPA) range.

Endpoint decoders in systems that need address translation use HPAs
which are not SPAs. To make the SPA range accessible to the endpoint
decoders, store and track the region's SPA range in struct cxl_region.
Introduce the @hpa_range member to the struct. Now, the SPA range of
an endpoint decoder can be determined based on its assigned region.

Patch is a prerequisite to implement address translation which uses
struct cxl_region to store all relevant region and interleaving
parameters.

Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 7 +++++++
 drivers/cxl/cxl.h         | 2 ++
 2 files changed, 9 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 45b1386a18d7..a780e65532a7 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -670,6 +670,8 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
 		return PTR_ERR(res);
 	}
 
+	cxlr->hpa_range = DEFINE_RANGE(0, -1);
+
 	p->res = res;
 	p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
 
@@ -706,6 +708,8 @@ static int free_hpa(struct cxl_region *cxlr)
 	if (p->state >= CXL_CONFIG_ACTIVE)
 		return -EBUSY;
 
+	cxlr->hpa_range = DEFINE_RANGE(0, -1);
+
 	cxl_region_iomem_release(cxlr);
 	p->state = CXL_CONFIG_IDLE;
 	return 0;
@@ -2408,6 +2412,8 @@ static void unregister_region(void *_cxlr)
 	for (i = 0; i < p->interleave_ways; i++)
 		detach_target(cxlr, i);
 
+	cxlr->hpa_range = DEFINE_RANGE(0, -1);
+
 	cxl_region_iomem_release(cxlr);
 	put_device(&cxlr->dev);
 }
@@ -3461,6 +3467,7 @@ static int __construct_region(struct cxl_region *cxlr,
 	}
 
 	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
+	cxlr->hpa_range = *hpa;
 
 	res = kmalloc(sizeof(*res), GFP_KERNEL);
 	if (!res)
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 19b8b62a1322..b57cfa4273b9 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -522,6 +522,7 @@ enum cxl_partition_mode {
  * @dev: This region's device
  * @id: This region's id. Id is globally unique across all regions
  * @cxlrd: Region's root decoder
+ * @hpa_range: Address range occupied by the region
  * @mode: Operational mode of the mapped capacity
  * @type: Endpoint decoder target type
  * @cxl_nvb: nvdimm bridge for coordinating @cxlr_pmem setup / shutdown
@@ -536,6 +537,7 @@ struct cxl_region {
 	struct device dev;
 	int id;
 	struct cxl_root_decoder *cxlrd;
+	struct range hpa_range;
 	enum cxl_partition_mode mode;
 	enum cxl_decoder_type type;
 	struct cxl_nvdimm_bridge *cxl_nvb;
-- 
2.47.3

Re: [PATCH v4 02/14] cxl/region: Store HPA range in struct cxl_region

[Robert Richter]

On 03.11.25 19:47:43, Robert Richter wrote:
> Each region has a known host physical address (HPA) range it is
> assigned to. Endpoint decoders assigned to a region share the same HPA
> range. The region's address range is the system's physical address
> (SPA) range.
> 
> Endpoint decoders in systems that need address translation use HPAs
> which are not SPAs. To make the SPA range accessible to the endpoint
> decoders, store and track the region's SPA range in struct cxl_region.
> Introduce the @hpa_range member to the struct. Now, the SPA range of
> an endpoint decoder can be determined based on its assigned region.
> 
> Patch is a prerequisite to implement address translation which uses
> struct cxl_region to store all relevant region and interleaving
> parameters.
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/region.c | 7 +++++++
>  drivers/cxl/cxl.h         | 2 ++
>  2 files changed, 9 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 45b1386a18d7..a780e65532a7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -670,6 +670,8 @@ static int alloc_hpa(struct cxl_region *cxlr, resource_size_t size)
>  		return PTR_ERR(res);
>  	}
>  
> +	cxlr->hpa_range = DEFINE_RANGE(0, -1);
> +

Alison, thanks for pointing out the wrong initialization for the
non-auto case. I think this must be the following instead:

	cxlr->hpa_range = DEFINE_RANGE(res->start, res->end);

>  	p->res = res;
>  	p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;

Will change that.

-Robert

[PATCH v4 03/14] cxl/region: Rename misleading variable name @hpa to @hpa_range

[Robert Richter]

@hpa is actually a @hpa_range, rename variables accordingly.

Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 30 ++++++++++++++++--------------
 1 file changed, 16 insertions(+), 14 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index a780e65532a7..bb889c891cf7 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3363,9 +3363,9 @@ static int match_decoder_by_range(struct device *dev, const void *data)
 }
 
 static struct cxl_decoder *
-cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
+cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa_range)
 {
-	struct device *cxld_dev = device_find_child(&port->dev, hpa,
+	struct device *cxld_dev = device_find_child(&port->dev, hpa_range,
 						    match_decoder_by_range);
 
 	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
@@ -3378,14 +3378,14 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
 	struct cxl_port *port = cxled_to_port(cxled);
 	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
 	struct cxl_decoder *root, *cxld = &cxled->cxld;
-	struct range *hpa = &cxld->hpa_range;
+	struct range *hpa_range = &cxld->hpa_range;
 
-	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa);
+	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa_range);
 	if (!root) {
 		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);
+			hpa_range->start, hpa_range->end);
 		return NULL;
 	}
 
@@ -3451,7 +3451,7 @@ static int __construct_region(struct cxl_region *cxlr,
 {
 	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
-	struct range *hpa = &cxled->cxld.hpa_range;
+	struct range *hpa_range = &cxled->cxld.hpa_range;
 	struct cxl_region_params *p;
 	struct resource *res;
 	int rc;
@@ -3467,13 +3467,13 @@ static int __construct_region(struct cxl_region *cxlr,
 	}
 
 	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
-	cxlr->hpa_range = *hpa;
+	cxlr->hpa_range = *hpa_range;
 
 	res = kmalloc(sizeof(*res), GFP_KERNEL);
 	if (!res)
 		return -ENOMEM;
 
-	*res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
+	*res = DEFINE_RES_MEM_NAMED(hpa_range->start, range_len(hpa_range),
 				    dev_name(&cxlr->dev));
 
 	rc = cxl_extended_linear_cache_resize(cxlr, res);
@@ -3552,11 +3552,12 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 }
 
 static struct cxl_region *
-cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
+cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
+			 struct range *hpa_range)
 {
 	struct device *region_dev;
 
-	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
+	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa_range,
 				       match_region_by_range);
 	if (!region_dev)
 		return NULL;
@@ -3566,7 +3567,7 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
 
 int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 {
-	struct range *hpa = &cxled->cxld.hpa_range;
+	struct range *hpa_range = &cxled->cxld.hpa_range;
 	struct cxl_region_params *p;
 	bool attach = false;
 	int rc;
@@ -3577,12 +3578,13 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 		return -ENXIO;
 
 	/*
-	 * Ensure that if multiple threads race to construct_region() for @hpa
-	 * one does the construction and the others add to that.
+	 * Ensure that if multiple threads race to construct_region()
+	 * for the HPA range one does the construction and the others
+	 * add to that.
 	 */
 	mutex_lock(&cxlrd->range_lock);
 	struct cxl_region *cxlr __free(put_cxl_region) =
-		cxl_find_region_by_range(cxlrd, hpa);
+		cxl_find_region_by_range(cxlrd, hpa_range);
 	if (!cxlr)
 		cxlr = construct_region(cxlrd, cxled);
 	mutex_unlock(&cxlrd->range_lock);
-- 
2.47.3

Re: [PATCH v4 03/14] cxl/region: Rename misleading variable name @hpa to @hpa_range

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> @hpa is actually a @hpa_range, rename variables accordingly.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/region.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index a780e65532a7..bb889c891cf7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3363,9 +3363,9 @@ static int match_decoder_by_range(struct device *dev, const void *data)
>  }
>  
>  static struct cxl_decoder *
> -cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa)
> +cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa_range)
>  {
> -	struct device *cxld_dev = device_find_child(&port->dev, hpa,
> +	struct device *cxld_dev = device_find_child(&port->dev, hpa_range,
>  						    match_decoder_by_range);
>  
>  	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
> @@ -3378,14 +3378,14 @@ cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
>  	struct cxl_port *port = cxled_to_port(cxled);
>  	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
>  	struct cxl_decoder *root, *cxld = &cxled->cxld;
> -	struct range *hpa = &cxld->hpa_range;
> +	struct range *hpa_range = &cxld->hpa_range;
>  
> -	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa);
> +	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa_range);
>  	if (!root) {
>  		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);
> +			hpa_range->start, hpa_range->end);
>  		return NULL;
>  	}
>  
> @@ -3451,7 +3451,7 @@ static int __construct_region(struct cxl_region *cxlr,
>  {
>  	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -	struct range *hpa = &cxled->cxld.hpa_range;
> +	struct range *hpa_range = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
>  	struct resource *res;
>  	int rc;
> @@ -3467,13 +3467,13 @@ static int __construct_region(struct cxl_region *cxlr,
>  	}
>  
>  	set_bit(CXL_REGION_F_AUTO, &cxlr->flags);
> -	cxlr->hpa_range = *hpa;
> +	cxlr->hpa_range = *hpa_range;
>  
>  	res = kmalloc(sizeof(*res), GFP_KERNEL);
>  	if (!res)
>  		return -ENOMEM;
>  
> -	*res = DEFINE_RES_MEM_NAMED(hpa->start, range_len(hpa),
> +	*res = DEFINE_RES_MEM_NAMED(hpa_range->start, range_len(hpa_range),
>  				    dev_name(&cxlr->dev));
>  
>  	rc = cxl_extended_linear_cache_resize(cxlr, res);
> @@ -3552,11 +3552,12 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  }
>  
>  static struct cxl_region *
> -cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
> +cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
> +			 struct range *hpa_range)
>  {
>  	struct device *region_dev;
>  
> -	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa,
> +	region_dev = device_find_child(&cxlrd->cxlsd.cxld.dev, hpa_range,
>  				       match_region_by_range);
>  	if (!region_dev)
>  		return NULL;
> @@ -3566,7 +3567,7 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd, struct range *hpa)
>  
>  int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  {
> -	struct range *hpa = &cxled->cxld.hpa_range;
> +	struct range *hpa_range = &cxled->cxld.hpa_range;
>  	struct cxl_region_params *p;
>  	bool attach = false;
>  	int rc;
> @@ -3577,12 +3578,13 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  		return -ENXIO;
>  
>  	/*
> -	 * Ensure that if multiple threads race to construct_region() for @hpa
> -	 * one does the construction and the others add to that.
> +	 * Ensure that if multiple threads race to construct_region()
> +	 * for the HPA range one does the construction and the others
> +	 * add to that.
>  	 */
>  	mutex_lock(&cxlrd->range_lock);
>  	struct cxl_region *cxlr __free(put_cxl_region) =
> -		cxl_find_region_by_range(cxlrd, hpa);
> +		cxl_find_region_by_range(cxlrd, hpa_range);
>  	if (!cxlr)
>  		cxlr = construct_region(cxlrd, cxled);
>  	mutex_unlock(&cxlrd->range_lock);

Re: [PATCH v4 03/14] cxl/region: Rename misleading variable name @hpa to @hpa_range

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:44 +0100
Robert Richter <rrichter@amd.com> wrote:

> @hpa is actually a @hpa_range, rename variables accordingly.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
Passing comment below on readability (unrelated to the change you made)

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

Dave, I wonder if it is reasonable to queue at least some of Robert's series
like this one so that people can start basing on top of those and the
merge conflicts will be less painful in the long run.

> ---
>  drivers/cxl/core/region.c | 30 ++++++++++++++++--------------
>  1 file changed, 16 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index a780e65532a7..bb889c891cf7 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c

> @@ -3577,12 +3578,13 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  		return -ENXIO;
>  
>  	/*
> -	 * Ensure that if multiple threads race to construct_region() for @hpa
> -	 * one does the construction and the others add to that.
> +	 * Ensure that if multiple threads race to construct_region()
> +	 * for the HPA range one does the construction and the others
> +	 * add to that.

Unrelated but a few commas would make this easier to read. Something like:

	 * Ensure that, if multiple threads race to construct_region()
	 * for the HPA range, one does the construction and the others
	 * add to that.

>  	 */
>  	mutex_lock(&cxlrd->range_lock);
>  	struct cxl_region *cxlr __free(put_cxl_region) =
> -		cxl_find_region_by_range(cxlrd, hpa);
> +		cxl_find_region_by_range(cxlrd, hpa_range);
>  	if (!cxlr)
>  		cxlr = construct_region(cxlrd, cxled);
>  	mutex_unlock(&cxlrd->range_lock);

Re: [PATCH v4 03/14] cxl/region: Rename misleading variable name @hpa to @hpa_range

[Dave Jiang]

On 11/11/25 7:41 AM, Jonathan Cameron wrote:
> On Mon, 3 Nov 2025 19:47:44 +0100
> Robert Richter <rrichter@amd.com> wrote:
> 
>> @hpa is actually a @hpa_range, rename variables accordingly.
>>
>> Reviewed-by: Gregory Price <gourry@gourry.net>
>> Signed-off-by: Robert Richter <rrichter@amd.com>
> Passing comment below on readability (unrelated to the change you made)
> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> 
> Dave, I wonder if it is reasonable to queue at least some of Robert's series
> like this one so that people can start basing on top of those and the
> merge conflicts will be less painful in the long run.

We can certainly do that, if the patches can be split out to a different series. Next week is the last week I'll take patches for this cycle.

DJ

> 
>> ---
>>  drivers/cxl/core/region.c | 30 ++++++++++++++++--------------
>>  1 file changed, 16 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
>> index a780e65532a7..bb889c891cf7 100644
>> --- a/drivers/cxl/core/region.c
>> +++ b/drivers/cxl/core/region.c
> 
>> @@ -3577,12 +3578,13 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>>  		return -ENXIO;
>>  
>>  	/*
>> -	 * Ensure that if multiple threads race to construct_region() for @hpa
>> -	 * one does the construction and the others add to that.
>> +	 * Ensure that if multiple threads race to construct_region()
>> +	 * for the HPA range one does the construction and the others
>> +	 * add to that.
> 
> Unrelated but a few commas would make this easier to read. Something like:
> 
> 	 * Ensure that, if multiple threads race to construct_region()
> 	 * for the HPA range, one does the construction and the others
> 	 * add to that.
> 
>>  	 */
>>  	mutex_lock(&cxlrd->range_lock);
>>  	struct cxl_region *cxlr __free(put_cxl_region) =
>> -		cxl_find_region_by_range(cxlrd, hpa);
>> +		cxl_find_region_by_range(cxlrd, hpa_range);
>>  	if (!cxlr)
>>  		cxlr = construct_region(cxlrd, cxled);
>>  	mutex_unlock(&cxlrd->range_lock);
> 

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

[Robert Richter]

cxl_calc_interleave_pos() uses the endpoint decoder's HPA range to
determine its interleaving position. This requires the endpoint
decoders to be an SPA, which is not the case for systems that need
address translation.

Add a separate @hpa_range argument to function
cxl_calc_interleave_pos() to specify the address range. Now it is
possible to pass the SPA translated address range of an endpoint
decoder to function cxl_calc_interleave_pos().

Refactor only, no functional changes.

Patch is a prerequisite to implement address translation.

Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

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

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

[Dave Jiang]

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

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

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

[Alison Schofield]

On Mon, Nov 03, 2025 at 07:47:45PM +0100, Robert Richter wrote:
> cxl_calc_interleave_pos() uses the endpoint decoder's HPA range to
> determine its interleaving position. This requires the endpoint
> decoders to be an SPA, which is not the case for systems that need
> address translation.
> 
> Add a separate @hpa_range argument to function
> cxl_calc_interleave_pos() to specify the address range. Now it is
> possible to pass the SPA translated address range of an endpoint
> decoder to function cxl_calc_interleave_pos().
> 
> Refactor only, no functional changes.
> 
> Patch is a prerequisite to implement address translation.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/region.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)

snip

> @@ -2094,7 +2095,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
>  		struct cxl_endpoint_decoder *cxled = p->targets[i];
>  		int test_pos;
>  
> -		test_pos = cxl_calc_interleave_pos(cxled);
> +		test_pos = cxl_calc_interleave_pos(cxled, &cxlr->hpa_range);

This is accessing the new hpa_range that Patch 2/14 started storing in
struct cxl_region. It stores it only for auto-regions and this is
a user created region path.

So the fixup is actually in Patch 2 - but this is the why. It shows
up as a bunch of dev_errs() from find_pos_and_ways() running unit
tests that create regions.


>  		dev_dbg(&cxled->cxld.dev,
>  			"Test cxl_calc_interleave_pos(): %s test_pos:%d cxled->pos:%d\n",
>  			(test_pos == cxled->pos) ? "success" : "fail",
> -- 
> 2.47.3
> 

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

[Robert Richter]

On 03.11.25 19:04:34, Alison Schofield wrote:
> On Mon, Nov 03, 2025 at 07:47:45PM +0100, Robert Richter wrote:
> > cxl_calc_interleave_pos() uses the endpoint decoder's HPA range to
> > determine its interleaving position. This requires the endpoint
> > decoders to be an SPA, which is not the case for systems that need
> > address translation.
> > 
> > Add a separate @hpa_range argument to function
> > cxl_calc_interleave_pos() to specify the address range. Now it is
> > possible to pass the SPA translated address range of an endpoint
> > decoder to function cxl_calc_interleave_pos().
> > 
> > Refactor only, no functional changes.
> > 
> > Patch is a prerequisite to implement address translation.
> > 
> > Reviewed-by: Gregory Price <gourry@gourry.net>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> > ---
> >  drivers/cxl/core/region.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> snip
> 
> > @@ -2094,7 +2095,7 @@ static int cxl_region_attach(struct cxl_region *cxlr,
> >  		struct cxl_endpoint_decoder *cxled = p->targets[i];
> >  		int test_pos;
> >  
> > -		test_pos = cxl_calc_interleave_pos(cxled);
> > +		test_pos = cxl_calc_interleave_pos(cxled, &cxlr->hpa_range);
> 
> This is accessing the new hpa_range that Patch 2/14 started storing in
> struct cxl_region. It stores it only for auto-regions and this is
> a user created region path.
> 
> So the fixup is actually in Patch 2 - but this is the why. It shows
> up as a bunch of dev_errs() from find_pos_and_ways() running unit
> tests that create regions.

See my response to patch 2. I have a fix that properly initializes the
hpa_range now.

Thanks,

-Robert

> 
> 
> >  		dev_dbg(&cxled->cxld.dev,
> >  			"Test cxl_calc_interleave_pos(): %s test_pos:%d cxled->pos:%d\n",
> >  			(test_pos == cxled->pos) ? "success" : "fail",
> > -- 
> > 2.47.3
> > 

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

[kernel test robot]

Hi Robert,

kernel test robot noticed the following build warnings:

[auto build test WARNING on 211ddde0823f1442e4ad052a2f30f050145ccada]

url:    https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-region-Store-root-decoder-in-struct-cxl_region/20251104-025351
base:   211ddde0823f1442e4ad052a2f30f050145ccada
patch link:    https://lore.kernel.org/r/20251103184804.509762-5-rrichter%40amd.com
patch subject: [PATCH v4 04/14] cxl/region: Add @hpa_range argument to function cxl_calc_interleave_pos()
config: x86_64-rhel-9.4-bpf (https://download.01.org/0day-ci/archive/20251105/202511050027.jKrGgaFu-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251105/202511050027.jKrGgaFu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511050027.jKrGgaFu-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> Warning: drivers/cxl/core/region.c:1849 function parameter 'hpa_range' not described in 'cxl_calc_interleave_pos'

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

[Robert Richter]

A root port's callback handlers are collected in struct cxl_root_ops.
The structure is dynamically allocated, though it contains only a
single pointer in it. This also requires to check two pointers to
check for the existance of a callback.

Simplify the allocation, release and handler check by embedding the
ops statical in struct cxl_root.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/acpi.c      |  7 ++-----
 drivers/cxl/core/cdat.c |  8 ++++----
 drivers/cxl/core/port.c |  9 ++-------
 drivers/cxl/cxl.h       | 19 ++++++++++---------
 4 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index bd2e282ca93a..1ab780edf141 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -299,10 +299,6 @@ static int cxl_acpi_qos_class(struct cxl_root *cxl_root,
 	return cxl_acpi_evaluate_qtg_dsm(handle, coord, entries, qos_class);
 }
 
-static const struct cxl_root_ops acpi_root_ops = {
-	.qos_class = cxl_acpi_qos_class,
-};
-
 static void del_cxl_resource(struct resource *res)
 {
 	if (!res)
@@ -914,9 +910,10 @@ static int cxl_acpi_probe(struct platform_device *pdev)
 	cxl_res->end = -1;
 	cxl_res->flags = IORESOURCE_MEM;
 
-	cxl_root = devm_cxl_add_root(host, &acpi_root_ops);
+	cxl_root = devm_cxl_add_root(host);
 	if (IS_ERR(cxl_root))
 		return PTR_ERR(cxl_root);
+	cxl_root->ops.qos_class = cxl_acpi_qos_class;
 	root_port = &cxl_root->port;
 
 	rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
index c4bd6e8a0cf0..b84a9b52942c 100644
--- a/drivers/cxl/core/cdat.c
+++ b/drivers/cxl/core/cdat.c
@@ -213,7 +213,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
 	if (!cxl_root)
 		return -ENODEV;
 
-	if (!cxl_root->ops || !cxl_root->ops->qos_class)
+	if (!cxl_root->ops.qos_class)
 		return -EOPNOTSUPP;
 
 	xa_for_each(dsmas_xa, index, dent) {
@@ -221,9 +221,9 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
 
 		cxl_coordinates_combine(dent->coord, dent->cdat_coord, ep_c);
 		dent->entries = 1;
-		rc = cxl_root->ops->qos_class(cxl_root,
-					      &dent->coord[ACCESS_COORDINATE_CPU],
-					      1, &qos_class);
+		rc = cxl_root->ops.qos_class(cxl_root,
+					     &dent->coord[ACCESS_COORDINATE_CPU],
+					     1, &qos_class);
 		if (rc != 1)
 			continue;
 
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 8128fd2b5b31..2338d146577c 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -459,7 +459,6 @@ static void cxl_root_decoder_release(struct device *dev)
 	if (atomic_read(&cxlrd->region_id) >= 0)
 		memregion_free(atomic_read(&cxlrd->region_id));
 	__cxl_decoder_release(&cxlrd->cxlsd.cxld);
-	kfree(cxlrd->ops);
 	kfree(cxlrd);
 }
 
@@ -955,19 +954,15 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_add_port, "CXL");
 
-struct cxl_root *devm_cxl_add_root(struct device *host,
-				   const struct cxl_root_ops *ops)
+struct cxl_root *devm_cxl_add_root(struct device *host)
 {
-	struct cxl_root *cxl_root;
 	struct cxl_port *port;
 
 	port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL);
 	if (IS_ERR(port))
 		return ERR_CAST(port);
 
-	cxl_root = to_cxl_root(port);
-	cxl_root->ops = ops;
-	return cxl_root;
+	return to_cxl_root(port);
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_add_root, "CXL");
 
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index b57cfa4273b9..9a381c827416 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -638,6 +638,14 @@ struct cxl_port {
 	resource_size_t component_reg_phys;
 };
 
+struct cxl_root;
+
+struct cxl_root_ops {
+	int (*qos_class)(struct cxl_root *cxl_root,
+			 struct access_coordinate *coord, int entries,
+			 int *qos_class);
+};
+
 /**
  * struct cxl_root - logical collection of root cxl_port items
  *
@@ -646,7 +654,7 @@ struct cxl_port {
  */
 struct cxl_root {
 	struct cxl_port port;
-	const struct cxl_root_ops *ops;
+	struct cxl_root_ops ops;
 };
 
 static inline struct cxl_root *
@@ -655,12 +663,6 @@ to_cxl_root(const struct cxl_port *port)
 	return container_of(port, struct cxl_root, port);
 }
 
-struct cxl_root_ops {
-	int (*qos_class)(struct cxl_root *cxl_root,
-			 struct access_coordinate *coord, int entries,
-			 int *qos_class);
-};
-
 static inline struct cxl_dport *
 cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
 {
@@ -755,8 +757,7 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
 				   struct device *uport_dev,
 				   resource_size_t component_reg_phys,
 				   struct cxl_dport *parent_dport);
-struct cxl_root *devm_cxl_add_root(struct device *host,
-				   const struct cxl_root_ops *ops);
+struct cxl_root *devm_cxl_add_root(struct device *host);
 struct cxl_root *find_cxl_root(struct cxl_port *port);
 
 DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
-- 
2.47.3

Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> A root port's callback handlers are collected in struct cxl_root_ops.
> The structure is dynamically allocated, though it contains only a
> single pointer in it. This also requires to check two pointers to
> check for the existance of a callback.
> 
> Simplify the allocation, release and handler check by embedding the
> ops statical in struct cxl_root.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>> ---
>  drivers/cxl/acpi.c      |  7 ++-----
>  drivers/cxl/core/cdat.c |  8 ++++----
>  drivers/cxl/core/port.c |  9 ++-------
>  drivers/cxl/cxl.h       | 19 ++++++++++---------
>  4 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index bd2e282ca93a..1ab780edf141 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -299,10 +299,6 @@ static int cxl_acpi_qos_class(struct cxl_root *cxl_root,
>  	return cxl_acpi_evaluate_qtg_dsm(handle, coord, entries, qos_class);
>  }
>  
> -static const struct cxl_root_ops acpi_root_ops = {
> -	.qos_class = cxl_acpi_qos_class,
> -};
> -
>  static void del_cxl_resource(struct resource *res)
>  {
>  	if (!res)
> @@ -914,9 +910,10 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>  	cxl_res->end = -1;
>  	cxl_res->flags = IORESOURCE_MEM;
>  
> -	cxl_root = devm_cxl_add_root(host, &acpi_root_ops);
> +	cxl_root = devm_cxl_add_root(host);
>  	if (IS_ERR(cxl_root))
>  		return PTR_ERR(cxl_root);
> +	cxl_root->ops.qos_class = cxl_acpi_qos_class;
>  	root_port = &cxl_root->port;
>  
>  	rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
> index c4bd6e8a0cf0..b84a9b52942c 100644
> --- a/drivers/cxl/core/cdat.c
> +++ b/drivers/cxl/core/cdat.c
> @@ -213,7 +213,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>  	if (!cxl_root)
>  		return -ENODEV;
>  
> -	if (!cxl_root->ops || !cxl_root->ops->qos_class)
> +	if (!cxl_root->ops.qos_class)
>  		return -EOPNOTSUPP;
>  
>  	xa_for_each(dsmas_xa, index, dent) {
> @@ -221,9 +221,9 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>  
>  		cxl_coordinates_combine(dent->coord, dent->cdat_coord, ep_c);
>  		dent->entries = 1;
> -		rc = cxl_root->ops->qos_class(cxl_root,
> -					      &dent->coord[ACCESS_COORDINATE_CPU],
> -					      1, &qos_class);
> +		rc = cxl_root->ops.qos_class(cxl_root,
> +					     &dent->coord[ACCESS_COORDINATE_CPU],
> +					     1, &qos_class);
>  		if (rc != 1)
>  			continue;
>  
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index 8128fd2b5b31..2338d146577c 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -459,7 +459,6 @@ static void cxl_root_decoder_release(struct device *dev)
>  	if (atomic_read(&cxlrd->region_id) >= 0)
>  		memregion_free(atomic_read(&cxlrd->region_id));
>  	__cxl_decoder_release(&cxlrd->cxlsd.cxld);
> -	kfree(cxlrd->ops);
>  	kfree(cxlrd);
>  }
>  
> @@ -955,19 +954,15 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_port, "CXL");
>  
> -struct cxl_root *devm_cxl_add_root(struct device *host,
> -				   const struct cxl_root_ops *ops)
> +struct cxl_root *devm_cxl_add_root(struct device *host)
>  {
> -	struct cxl_root *cxl_root;
>  	struct cxl_port *port;
>  
>  	port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL);
>  	if (IS_ERR(port))
>  		return ERR_CAST(port);
>  
> -	cxl_root = to_cxl_root(port);
> -	cxl_root->ops = ops;
> -	return cxl_root;
> +	return to_cxl_root(port);
>  }
>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_root, "CXL");
>  
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index b57cfa4273b9..9a381c827416 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -638,6 +638,14 @@ struct cxl_port {
>  	resource_size_t component_reg_phys;
>  };
>  
> +struct cxl_root;
> +
> +struct cxl_root_ops {
> +	int (*qos_class)(struct cxl_root *cxl_root,
> +			 struct access_coordinate *coord, int entries,
> +			 int *qos_class);
> +};
> +
>  /**
>   * struct cxl_root - logical collection of root cxl_port items
>   *
> @@ -646,7 +654,7 @@ struct cxl_port {
>   */
>  struct cxl_root {
>  	struct cxl_port port;
> -	const struct cxl_root_ops *ops;
> +	struct cxl_root_ops ops;
>  };
>  
>  static inline struct cxl_root *
> @@ -655,12 +663,6 @@ to_cxl_root(const struct cxl_port *port)
>  	return container_of(port, struct cxl_root, port);
>  }
>  
> -struct cxl_root_ops {
> -	int (*qos_class)(struct cxl_root *cxl_root,
> -			 struct access_coordinate *coord, int entries,
> -			 int *qos_class);
> -};
> -
>  static inline struct cxl_dport *
>  cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
>  {
> @@ -755,8 +757,7 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>  				   struct device *uport_dev,
>  				   resource_size_t component_reg_phys,
>  				   struct cxl_dport *parent_dport);
> -struct cxl_root *devm_cxl_add_root(struct device *host,
> -				   const struct cxl_root_ops *ops);
> +struct cxl_root *devm_cxl_add_root(struct device *host);
>  struct cxl_root *find_cxl_root(struct cxl_port *port);
>  
>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))

Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

[Dave Jiang]

On 11/3/25 2:53 PM, Dave Jiang wrote:
> 
> 
> On 11/3/25 11:47 AM, Robert Richter wrote:
>> A root port's callback handlers are collected in struct cxl_root_ops.
>> The structure is dynamically allocated, though it contains only a
>> single pointer in it. This also requires to check two pointers to
>> check for the existance of a callback.
>>
>> Simplify the allocation, release and handler check by embedding the
>> ops statical in struct cxl_root.
>>
>> Signed-off-by: Robert Richter <rrichter@amd.com>
> 
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>


Thought about it for a bit, should the callback be with 'cxl_rd_ops' and the rest of the translation functions under the root decoder rather than cxl_root_ops with the cxl root? That seems to be the better fit.

DJ
 >>  drivers/cxl/acpi.c      |  7 ++-----
>>  drivers/cxl/core/cdat.c |  8 ++++----
>>  drivers/cxl/core/port.c |  9 ++-------
>>  drivers/cxl/cxl.h       | 19 ++++++++++---------
>>  4 files changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
>> index bd2e282ca93a..1ab780edf141 100644
>> --- a/drivers/cxl/acpi.c
>> +++ b/drivers/cxl/acpi.c
>> @@ -299,10 +299,6 @@ static int cxl_acpi_qos_class(struct cxl_root *cxl_root,
>>  	return cxl_acpi_evaluate_qtg_dsm(handle, coord, entries, qos_class);
>>  }
>>  
>> -static const struct cxl_root_ops acpi_root_ops = {
>> -	.qos_class = cxl_acpi_qos_class,
>> -};
>> -
>>  static void del_cxl_resource(struct resource *res)
>>  {
>>  	if (!res)
>> @@ -914,9 +910,10 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>>  	cxl_res->end = -1;
>>  	cxl_res->flags = IORESOURCE_MEM;
>>  
>> -	cxl_root = devm_cxl_add_root(host, &acpi_root_ops);
>> +	cxl_root = devm_cxl_add_root(host);
>>  	if (IS_ERR(cxl_root))
>>  		return PTR_ERR(cxl_root);
>> +	cxl_root->ops.qos_class = cxl_acpi_qos_class;
>>  	root_port = &cxl_root->port;
>>  
>>  	rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
>> diff --git a/drivers/cxl/core/cdat.c b/drivers/cxl/core/cdat.c
>> index c4bd6e8a0cf0..b84a9b52942c 100644
>> --- a/drivers/cxl/core/cdat.c
>> +++ b/drivers/cxl/core/cdat.c
>> @@ -213,7 +213,7 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>>  	if (!cxl_root)
>>  		return -ENODEV;
>>  
>> -	if (!cxl_root->ops || !cxl_root->ops->qos_class)
>> +	if (!cxl_root->ops.qos_class)
>>  		return -EOPNOTSUPP;
>>  
>>  	xa_for_each(dsmas_xa, index, dent) {
>> @@ -221,9 +221,9 @@ static int cxl_port_perf_data_calculate(struct cxl_port *port,
>>  
>>  		cxl_coordinates_combine(dent->coord, dent->cdat_coord, ep_c);
>>  		dent->entries = 1;
>> -		rc = cxl_root->ops->qos_class(cxl_root,
>> -					      &dent->coord[ACCESS_COORDINATE_CPU],
>> -					      1, &qos_class);
>> +		rc = cxl_root->ops.qos_class(cxl_root,
>> +					     &dent->coord[ACCESS_COORDINATE_CPU],
>> +					     1, &qos_class);
>>  		if (rc != 1)
>>  			continue;
>>  
>> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
>> index 8128fd2b5b31..2338d146577c 100644
>> --- a/drivers/cxl/core/port.c
>> +++ b/drivers/cxl/core/port.c
>> @@ -459,7 +459,6 @@ static void cxl_root_decoder_release(struct device *dev)
>>  	if (atomic_read(&cxlrd->region_id) >= 0)
>>  		memregion_free(atomic_read(&cxlrd->region_id));
>>  	__cxl_decoder_release(&cxlrd->cxlsd.cxld);
>> -	kfree(cxlrd->ops);
>>  	kfree(cxlrd);
>>  }
>>  
>> @@ -955,19 +954,15 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>>  }
>>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_port, "CXL");
>>  
>> -struct cxl_root *devm_cxl_add_root(struct device *host,
>> -				   const struct cxl_root_ops *ops)
>> +struct cxl_root *devm_cxl_add_root(struct device *host)
>>  {
>> -	struct cxl_root *cxl_root;
>>  	struct cxl_port *port;
>>  
>>  	port = devm_cxl_add_port(host, host, CXL_RESOURCE_NONE, NULL);
>>  	if (IS_ERR(port))
>>  		return ERR_CAST(port);
>>  
>> -	cxl_root = to_cxl_root(port);
>> -	cxl_root->ops = ops;
>> -	return cxl_root;
>> +	return to_cxl_root(port);
>>  }
>>  EXPORT_SYMBOL_NS_GPL(devm_cxl_add_root, "CXL");
>>  
>> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
>> index b57cfa4273b9..9a381c827416 100644
>> --- a/drivers/cxl/cxl.h
>> +++ b/drivers/cxl/cxl.h
>> @@ -638,6 +638,14 @@ struct cxl_port {
>>  	resource_size_t component_reg_phys;
>>  };
>>  
>> +struct cxl_root;
>> +
>> +struct cxl_root_ops {
>> +	int (*qos_class)(struct cxl_root *cxl_root,
>> +			 struct access_coordinate *coord, int entries,
>> +			 int *qos_class);
>> +};
>> +
>>  /**
>>   * struct cxl_root - logical collection of root cxl_port items
>>   *
>> @@ -646,7 +654,7 @@ struct cxl_port {
>>   */
>>  struct cxl_root {
>>  	struct cxl_port port;
>> -	const struct cxl_root_ops *ops;
>> +	struct cxl_root_ops ops;
>>  };
>>  
>>  static inline struct cxl_root *
>> @@ -655,12 +663,6 @@ to_cxl_root(const struct cxl_port *port)
>>  	return container_of(port, struct cxl_root, port);
>>  }
>>  
>> -struct cxl_root_ops {
>> -	int (*qos_class)(struct cxl_root *cxl_root,
>> -			 struct access_coordinate *coord, int entries,
>> -			 int *qos_class);
>> -};
>> -
>>  static inline struct cxl_dport *
>>  cxl_find_dport_by_dev(struct cxl_port *port, const struct device *dport_dev)
>>  {
>> @@ -755,8 +757,7 @@ struct cxl_port *devm_cxl_add_port(struct device *host,
>>  				   struct device *uport_dev,
>>  				   resource_size_t component_reg_phys,
>>  				   struct cxl_dport *parent_dport);
>> -struct cxl_root *devm_cxl_add_root(struct device *host,
>> -				   const struct cxl_root_ops *ops);
>> +struct cxl_root *devm_cxl_add_root(struct device *host);
>>  struct cxl_root *find_cxl_root(struct cxl_port *port);
>>  
>>  DEFINE_FREE(put_cxl_root, struct cxl_root *, if (_T) put_device(&_T->port.dev))
> 
> 

Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

[Robert Richter]

On 04.11.25 16:02:04, Dave Jiang wrote:
> 
> 
> On 11/3/25 2:53 PM, Dave Jiang wrote:
> > 
> > 
> > On 11/3/25 11:47 AM, Robert Richter wrote:
> >> A root port's callback handlers are collected in struct cxl_root_ops.
> >> The structure is dynamically allocated, though it contains only a
> >> single pointer in it. This also requires to check two pointers to
> >> check for the existance of a callback.
> >>
> >> Simplify the allocation, release and handler check by embedding the
> >> ops statical in struct cxl_root.
> >>
> >> Signed-off-by: Robert Richter <rrichter@amd.com>
> > 
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
> 

> Thought about it for a bit, should the callback be with 'cxl_rd_ops'
> and the rest of the translation functions under the root decoder
> rather than cxl_root_ops with the cxl root? That seems to be the
> better fit.

The handler for address translation is needed to determine the root
decoders. Because of that the cxl_rd_ops cannot be used to hold the
callback reference. The use of cxl_root_ops works since the pci tree
can be walked to get the cxl root without any cxl specific knowledge.
A description of that is already in the patch that adds the callback.

I haven't evaluated moving qos to cxl_rd_ops as this is not the scope
of the series.

-Robert

Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

[Dave Jiang]

On 11/7/25 8:45 AM, Robert Richter wrote:
> On 04.11.25 16:02:04, Dave Jiang wrote:
>>
>>
>> On 11/3/25 2:53 PM, Dave Jiang wrote:
>>>
>>>
>>> On 11/3/25 11:47 AM, Robert Richter wrote:
>>>> A root port's callback handlers are collected in struct cxl_root_ops.
>>>> The structure is dynamically allocated, though it contains only a
>>>> single pointer in it. This also requires to check two pointers to
>>>> check for the existance of a callback.
>>>>
>>>> Simplify the allocation, release and handler check by embedding the
>>>> ops statical in struct cxl_root.
>>>>
>>>> Signed-off-by: Robert Richter <rrichter@amd.com>
>>>
>>> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
>>
>>
> 
>> Thought about it for a bit, should the callback be with 'cxl_rd_ops'
>> and the rest of the translation functions under the root decoder
>> rather than cxl_root_ops with the cxl root? That seems to be the
>> better fit.
> 
> The handler for address translation is needed to determine the root
> decoders. Because of that the cxl_rd_ops cannot be used to hold the
> callback reference. The use of cxl_root_ops works since the pci tree
> can be walked to get the cxl root without any cxl specific knowledge.
> A description of that is already in the patch that adds the callback.

Ok that is fair.

> 
> I haven't evaluated moving qos to cxl_rd_ops as this is not the scope
> of the series.

The qos callback belongs with the cxl_root since the ACPI query is against the host bridge.> 
> -Robert

Re: [PATCH v4 05/14] cxl: Simplify cxl_root_ops allocation and handling

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:46 +0100
Robert Richter <rrichter@amd.com> wrote:

> A root port's callback handlers are collected in struct cxl_root_ops.
> The structure is dynamically allocated, though it contains only a
> single pointer in it. This also requires to check two pointers to
> check for the existance of a callback.
> 
> Simplify the allocation, release and handler check by embedding the
> ops statical in struct cxl_root.

statically

> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
Make sense on it's own.  Maybe one to pick up early if there are blockers
on later patches.

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

[PATCH v4 06/14] cxl/region: Separate region parameter setup and region construction

[Robert Richter]

To construct a region, the region parameters such as address range and
interleaving config need to be determined. This is done while
constructing the region by inspecting the endpoint decoder
configuration. The endpoint decoder is passed as a function argument.

With address translation the endpoint decoder data is no longer
sufficient to extract the region parameters as some of the information
is obtained using other methods such as using firmware calls.

In a first step, separate code to determine the region parameters from
the region construction. Temporarily store all the data to create the
region in the new struct cxl_region_context. Once the region data is
determined and struct cxl_region_context is filled, construct the
region.

Patch is a prerequisite to implement address translation. The code
separation helps to later extend it to determine region parameters
using other methods as needed, esp. to support address translation.

Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/core.h   |  9 +++++++++
 drivers/cxl/core/region.c | 32 +++++++++++++++++++++-----------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 1fb66132b777..2bc37f3aee21 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -19,6 +19,15 @@ enum cxl_detach_mode {
 };
 
 #ifdef CONFIG_CXL_REGION
+
+struct cxl_region_context {
+	struct cxl_endpoint_decoder *cxled;
+	struct cxl_memdev *cxlmd;
+	struct range hpa_range;
+	int interleave_ways;
+	int interleave_granularity;
+};
+
 extern struct device_attribute dev_attr_create_pmem_region;
 extern struct device_attribute dev_attr_create_ram_region;
 extern struct device_attribute dev_attr_delete_region;
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index d3557d9d5b0f..2cfc29a2b588 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3448,11 +3448,12 @@ static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
 }
 
 static int __construct_region(struct cxl_region *cxlr,
-			      struct cxl_endpoint_decoder *cxled)
+			      struct cxl_region_context *ctx)
 {
+	struct cxl_endpoint_decoder *cxled = ctx->cxled;
 	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
-	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
-	struct range *hpa_range = &cxled->cxld.hpa_range;
+	struct cxl_memdev *cxlmd = ctx->cxlmd;
+	struct range *hpa_range = &ctx->hpa_range;
 	struct cxl_region_params *p;
 	struct resource *res;
 	int rc;
@@ -3501,8 +3502,8 @@ static int __construct_region(struct cxl_region *cxlr,
 	}
 
 	p->res = res;
-	p->interleave_ways = cxled->cxld.interleave_ways;
-	p->interleave_granularity = cxled->cxld.interleave_granularity;
+	p->interleave_ways = ctx->interleave_ways;
+	p->interleave_granularity = ctx->interleave_granularity;
 	p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
 
 	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
@@ -3522,9 +3523,10 @@ static int __construct_region(struct cxl_region *cxlr,
 
 /* Establish an empty region covering the given HPA range */
 static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
-					   struct cxl_endpoint_decoder *cxled)
+					   struct cxl_region_context *ctx)
 {
-	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
+	struct cxl_endpoint_decoder *cxled = ctx->cxled;
+	struct cxl_memdev *cxlmd = ctx->cxlmd;
 	struct cxl_port *port = cxlrd_to_port(cxlrd);
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	int rc, part = READ_ONCE(cxled->part);
@@ -3543,7 +3545,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
 		return cxlr;
 	}
 
-	rc = __construct_region(cxlr, cxled);
+	rc = __construct_region(cxlr, ctx);
 	if (rc) {
 		devm_release_action(port->uport_dev, unregister_region, cxlr);
 		return ERR_PTR(rc);
@@ -3568,11 +3570,19 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
 
 int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 {
-	struct range *hpa_range = &cxled->cxld.hpa_range;
+	struct cxl_region_context ctx;
 	struct cxl_region_params *p;
 	bool attach = false;
 	int rc;
 
+	ctx = (struct cxl_region_context) {
+		.cxled = cxled,
+		.cxlmd = cxled_to_memdev(cxled),
+		.hpa_range = cxled->cxld.hpa_range,
+		.interleave_ways = cxled->cxld.interleave_ways,
+		.interleave_granularity = cxled->cxld.interleave_granularity,
+	};
+
 	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
 		cxl_find_root_decoder(cxled);
 	if (!cxlrd)
@@ -3585,9 +3595,9 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 	 */
 	mutex_lock(&cxlrd->range_lock);
 	struct cxl_region *cxlr __free(put_cxl_region) =
-		cxl_find_region_by_range(cxlrd, hpa_range);
+		cxl_find_region_by_range(cxlrd, &ctx.hpa_range);
 	if (!cxlr)
-		cxlr = construct_region(cxlrd, cxled);
+		cxlr = construct_region(cxlrd, &ctx);
 	mutex_unlock(&cxlrd->range_lock);
 
 	rc = PTR_ERR_OR_ZERO(cxlr);
-- 
2.47.3

Re: [PATCH v4 06/14] cxl/region: Separate region parameter setup and region construction

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> To construct a region, the region parameters such as address range and
> interleaving config need to be determined. This is done while
> constructing the region by inspecting the endpoint decoder
> configuration. The endpoint decoder is passed as a function argument.
> 
> With address translation the endpoint decoder data is no longer
> sufficient to extract the region parameters as some of the information
> is obtained using other methods such as using firmware calls.
> 
> In a first step, separate code to determine the region parameters from
> the region construction. Temporarily store all the data to create the
> region in the new struct cxl_region_context. Once the region data is
> determined and struct cxl_region_context is filled, construct the
> region.
> 
> Patch is a prerequisite to implement address translation. The code
> separation helps to later extend it to determine region parameters
> using other methods as needed, esp. to support address translation.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>

Just a small thing below, otherwise
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/core.h   |  9 +++++++++
>  drivers/cxl/core/region.c | 32 +++++++++++++++++++++-----------
>  2 files changed, 30 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index 1fb66132b777..2bc37f3aee21 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -19,6 +19,15 @@ enum cxl_detach_mode {
>  };
>  
>  #ifdef CONFIG_CXL_REGION
> +
> +struct cxl_region_context {
> +	struct cxl_endpoint_decoder *cxled;
> +	struct cxl_memdev *cxlmd;

cxlmd may not be needed.

struct cxl_memdev *cxlmd = cxled_to_memdev(cxlr_ctx->cxled);

which you used later on in this patch to init the cxlmd member :)

DJ

> +	struct range hpa_range;
> +	int interleave_ways;
> +	int interleave_granularity;
> +};
> +
>  extern struct device_attribute dev_attr_create_pmem_region;
>  extern struct device_attribute dev_attr_create_ram_region;
>  extern struct device_attribute dev_attr_delete_region;
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index d3557d9d5b0f..2cfc29a2b588 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3448,11 +3448,12 @@ static int cxl_extended_linear_cache_resize(struct cxl_region *cxlr,
>  }
>  
>  static int __construct_region(struct cxl_region *cxlr,
> -			      struct cxl_endpoint_decoder *cxled)
> +			      struct cxl_region_context *ctx)
>  {
> +	struct cxl_endpoint_decoder *cxled = ctx->cxled;
>  	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> -	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> -	struct range *hpa_range = &cxled->cxld.hpa_range;
> +	struct cxl_memdev *cxlmd = ctx->cxlmd;
> +	struct range *hpa_range = &ctx->hpa_range;
>  	struct cxl_region_params *p;
>  	struct resource *res;
>  	int rc;
> @@ -3501,8 +3502,8 @@ static int __construct_region(struct cxl_region *cxlr,
>  	}
>  
>  	p->res = res;
> -	p->interleave_ways = cxled->cxld.interleave_ways;
> -	p->interleave_granularity = cxled->cxld.interleave_granularity;
> +	p->interleave_ways = ctx->interleave_ways;
> +	p->interleave_granularity = ctx->interleave_granularity;
>  	p->state = CXL_CONFIG_INTERLEAVE_ACTIVE;
>  
>  	rc = sysfs_update_group(&cxlr->dev.kobj, get_cxl_region_target_group());
> @@ -3522,9 +3523,10 @@ static int __construct_region(struct cxl_region *cxlr,
>  
>  /* Establish an empty region covering the given HPA range */
>  static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
> -					   struct cxl_endpoint_decoder *cxled)
> +					   struct cxl_region_context *ctx)
>  {
> -	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> +	struct cxl_endpoint_decoder *cxled = ctx->cxled;
> +	struct cxl_memdev *cxlmd = ctx->cxlmd;
>  	struct cxl_port *port = cxlrd_to_port(cxlrd);
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  	int rc, part = READ_ONCE(cxled->part);
> @@ -3543,7 +3545,7 @@ static struct cxl_region *construct_region(struct cxl_root_decoder *cxlrd,
>  		return cxlr;
>  	}
>  
> -	rc = __construct_region(cxlr, cxled);
> +	rc = __construct_region(cxlr, ctx);
>  	if (rc) {
>  		devm_release_action(port->uport_dev, unregister_region, cxlr);
>  		return ERR_PTR(rc);
> @@ -3568,11 +3570,19 @@ cxl_find_region_by_range(struct cxl_root_decoder *cxlrd,
>  
>  int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  {
> -	struct range *hpa_range = &cxled->cxld.hpa_range;
> +	struct cxl_region_context ctx;
>  	struct cxl_region_params *p;
>  	bool attach = false;
>  	int rc;
>  
> +	ctx = (struct cxl_region_context) {
> +		.cxled = cxled,
> +		.cxlmd = cxled_to_memdev(cxled),
> +		.hpa_range = cxled->cxld.hpa_range,
> +		.interleave_ways = cxled->cxld.interleave_ways,
> +		.interleave_granularity = cxled->cxld.interleave_granularity,
> +	};
> +
>  	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
>  		cxl_find_root_decoder(cxled);
>  	if (!cxlrd)
> @@ -3585,9 +3595,9 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  	 */
>  	mutex_lock(&cxlrd->range_lock);
>  	struct cxl_region *cxlr __free(put_cxl_region) =
> -		cxl_find_region_by_range(cxlrd, hpa_range);
> +		cxl_find_region_by_range(cxlrd, &ctx.hpa_range);
>  	if (!cxlr)
> -		cxlr = construct_region(cxlrd, cxled);
> +		cxlr = construct_region(cxlrd, &ctx);
>  	mutex_unlock(&cxlrd->range_lock);
>  
>  	rc = PTR_ERR_OR_ZERO(cxlr);

Re: [PATCH v4 06/14] cxl/region: Separate region parameter setup and region construction

[Robert Richter]

On 03.11.25 15:05:28, Dave Jiang wrote:
> 
> 
> On 11/3/25 11:47 AM, Robert Richter wrote:
> > To construct a region, the region parameters such as address range and
> > interleaving config need to be determined. This is done while
> > constructing the region by inspecting the endpoint decoder
> > configuration. The endpoint decoder is passed as a function argument.
> > 
> > With address translation the endpoint decoder data is no longer
> > sufficient to extract the region parameters as some of the information
> > is obtained using other methods such as using firmware calls.
> > 
> > In a first step, separate code to determine the region parameters from
> > the region construction. Temporarily store all the data to create the
> > region in the new struct cxl_region_context. Once the region data is
> > determined and struct cxl_region_context is filled, construct the
> > region.
> > 
> > Patch is a prerequisite to implement address translation. The code
> > separation helps to later extend it to determine region parameters
> > using other methods as needed, esp. to support address translation.
> > 
> > Reviewed-by: Gregory Price <gourry@gourry.net>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> 
> Just a small thing below, otherwise
> Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> 
> > ---
> >  drivers/cxl/core/core.h   |  9 +++++++++
> >  drivers/cxl/core/region.c | 32 +++++++++++++++++++++-----------
> >  2 files changed, 30 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> > index 1fb66132b777..2bc37f3aee21 100644
> > --- a/drivers/cxl/core/core.h
> > +++ b/drivers/cxl/core/core.h
> > @@ -19,6 +19,15 @@ enum cxl_detach_mode {
> >  };
> >  
> >  #ifdef CONFIG_CXL_REGION
> > +
> > +struct cxl_region_context {
> > +	struct cxl_endpoint_decoder *cxled;
> > +	struct cxl_memdev *cxlmd;
> 
> cxlmd may not be needed.
> 
> struct cxl_memdev *cxlmd = cxled_to_memdev(cxlr_ctx->cxled);
> 
> which you used later on in this patch to init the cxlmd member :)

This was on purpose to eliminate an unnecessary frequent call of
cxled_to_memdev() while holding the context. There is at least a
3-level pointer chasing to get to cxlmd.

Maybe it's wort to add it to struct cxl_endpoint_decoder.

-Robert

Re: [PATCH v4 06/14] cxl/region: Separate region parameter setup and region construction

[Jonathan Cameron]

On Fri, 7 Nov 2025 16:59:25 +0100
Robert Richter <rrichter@amd.com> wrote:

> On 03.11.25 15:05:28, Dave Jiang wrote:
> > 
> > 
> > On 11/3/25 11:47 AM, Robert Richter wrote:  
> > > To construct a region, the region parameters such as address range and
> > > interleaving config need to be determined. This is done while
> > > constructing the region by inspecting the endpoint decoder
> > > configuration. The endpoint decoder is passed as a function argument.
> > > 
> > > With address translation the endpoint decoder data is no longer
> > > sufficient to extract the region parameters as some of the information
> > > is obtained using other methods such as using firmware calls.
> > > 
> > > In a first step, separate code to determine the region parameters from
> > > the region construction. Temporarily store all the data to create the
> > > region in the new struct cxl_region_context. Once the region data is
> > > determined and struct cxl_region_context is filled, construct the
> > > region.
> > > 
> > > Patch is a prerequisite to implement address translation. The code
> > > separation helps to later extend it to determine region parameters
> > > using other methods as needed, esp. to support address translation.
> > > 
> > > Reviewed-by: Gregory Price <gourry@gourry.net>
> > > Signed-off-by: Robert Richter <rrichter@amd.com>  
> > 
> > Just a small thing below, otherwise
> > Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> >   
> > > ---
> > >  drivers/cxl/core/core.h   |  9 +++++++++
> > >  drivers/cxl/core/region.c | 32 +++++++++++++++++++++-----------
> > >  2 files changed, 30 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> > > index 1fb66132b777..2bc37f3aee21 100644
> > > --- a/drivers/cxl/core/core.h
> > > +++ b/drivers/cxl/core/core.h
> > > @@ -19,6 +19,15 @@ enum cxl_detach_mode {
> > >  };
> > >  
> > >  #ifdef CONFIG_CXL_REGION
> > > +
> > > +struct cxl_region_context {
> > > +	struct cxl_endpoint_decoder *cxled;
> > > +	struct cxl_memdev *cxlmd;  
> > 
> > cxlmd may not be needed.
> > 
> > struct cxl_memdev *cxlmd = cxled_to_memdev(cxlr_ctx->cxled);
> > 
> > which you used later on in this patch to init the cxlmd member :)  
> 
> This was on purpose to eliminate an unnecessary frequent call of
> cxled_to_memdev() while holding the context. There is at least a
> 3-level pointer chasing to get to cxlmd.
> 
> Maybe it's wort to add it to struct cxl_endpoint_decoder.

This isn't high performance code and the helper is there so I'd just
use it as Dave suggested.

Jonathan

> 
> -Robert
> 

Re: [PATCH v4 06/14] cxl/region: Separate region parameter setup and region construction

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:47 +0100
Robert Richter <rrichter@amd.com> wrote:

> To construct a region, the region parameters such as address range and
> interleaving config need to be determined. This is done while
> constructing the region by inspecting the endpoint decoder
> configuration. The endpoint decoder is passed as a function argument.
> 
> With address translation the endpoint decoder data is no longer
> sufficient to extract the region parameters as some of the information
> is obtained using other methods such as using firmware calls.
> 
> In a first step, separate code to determine the region parameters from
> the region construction. Temporarily store all the data to create the
> region in the new struct cxl_region_context. Once the region data is
> determined and struct cxl_region_context is filled, construct the
> region.
> 
> Patch is a prerequisite to implement address translation. The code
> separation helps to later extend it to determine region parameters
> using other methods as needed, esp. to support address translation.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
Other than Dave's comment on not holding the cxlmd, looks good to me.
Another one that will clash with Alejandro's rework for type 2 support
though.

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

[PATCH v4 07/14] cxl/region: Use region data to get the root decoder

[Robert Richter]

To find a region's root decoder, the endpoint's HPA range is used to
search the matching decoder by its range. With address translation the
endpoint decoder's range is in a different address space and thus
cannot be used to determine the root decoder.

The region parameters are encapsulated within struc cxl_region_context
and may include the translated Host Physical Address (HPA) range. Use
this context to identify the root decoder rather than relying on the
endpoint.

Modify cxl_find_root_decoder() and add the region context as
parameter. Rename this function to get_cxl_root_decoder() as a
counterpart to put_cxl_root_decoder(). Simplify the implementation by
removing function cxl_port_find_switch_decode(). The function is
unnecessary because it is not referenced or utilized elsewhere in the
code.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 50 +++++++++++++++++----------------------
 1 file changed, 22 insertions(+), 28 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 2cfc29a2b588..2dd9e9be4889 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3350,47 +3350,39 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
 	return rc;
 }
 
-static int match_decoder_by_range(struct device *dev, const void *data)
+static int match_root_decoder(struct device *dev, const void *data)
 {
 	const struct range *r1, *r2 = data;
-	struct cxl_decoder *cxld;
+	struct cxl_root_decoder *cxlrd;
 
-	if (!is_switch_decoder(dev))
+	if (!is_root_decoder(dev))
 		return 0;
 
-	cxld = to_cxl_decoder(dev);
-	r1 = &cxld->hpa_range;
-	return range_contains(r1, r2);
-}
+	cxlrd = to_cxl_root_decoder(dev);
+	r1 = &cxlrd->cxlsd.cxld.hpa_range;
 
-static struct cxl_decoder *
-cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa_range)
-{
-	struct device *cxld_dev = device_find_child(&port->dev, hpa_range,
-						    match_decoder_by_range);
-
-	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
+	return range_contains(r1, r2);
 }
 
 static struct cxl_root_decoder *
-cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
+get_cxl_root_decoder(struct cxl_endpoint_decoder *cxled,
+		     struct cxl_region_context *ctx)
 {
-	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
 	struct cxl_port *port = cxled_to_port(cxled);
 	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
-	struct cxl_decoder *root, *cxld = &cxled->cxld;
-	struct range *hpa_range = &cxld->hpa_range;
+	struct device *cxlrd_dev;
 
-	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa_range);
-	if (!root) {
-		dev_err(cxlmd->dev.parent,
+	cxlrd_dev = device_find_child(&cxl_root->port.dev, &ctx->hpa_range,
+				      match_root_decoder);
+	if (!cxlrd_dev) {
+		dev_err(port->uport_dev,
 			"%s:%s no CXL window for range %#llx:%#llx\n",
-			dev_name(&cxlmd->dev), dev_name(&cxld->dev),
-			hpa_range->start, hpa_range->end);
-		return NULL;
+			dev_name(&ctx->cxlmd->dev), dev_name(&cxled->cxld.dev),
+			ctx->hpa_range.start, ctx->hpa_range.end);
+		return ERR_PTR(-ENXIO);
 	}
 
-	return to_cxl_root_decoder(&root->dev);
+	return to_cxl_root_decoder(cxlrd_dev);
 }
 
 static int match_region_by_range(struct device *dev, const void *data)
@@ -3584,9 +3576,11 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
 	};
 
 	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
-		cxl_find_root_decoder(cxled);
-	if (!cxlrd)
-		return -ENXIO;
+		get_cxl_root_decoder(cxled, &ctx);
+
+	rc = PTR_ERR_OR_ZERO(cxlrd);
+	if (rc)
+		return rc;
 
 	/*
 	 * Ensure that if multiple threads race to construct_region()
-- 
2.47.3

Re: [PATCH v4 07/14] cxl/region: Use region data to get the root decoder

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> To find a region's root decoder, the endpoint's HPA range is used to
> search the matching decoder by its range. With address translation the
> endpoint decoder's range is in a different address space and thus
> cannot be used to determine the root decoder.
> 
> The region parameters are encapsulated within struc cxl_region_context
> and may include the translated Host Physical Address (HPA) range. Use
> this context to identify the root decoder rather than relying on the
> endpoint.
> 
> Modify cxl_find_root_decoder() and add the region context as
> parameter. Rename this function to get_cxl_root_decoder() as a
> counterpart to put_cxl_root_decoder(). Simplify the implementation by
> removing function cxl_port_find_switch_decode(). The function is
> unnecessary because it is not referenced or utilized elsewhere in the
> code.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>

Just a nit below. Otherwise
Reviewed-by: Dave Jiang <dave.jiang@intel.com>

> ---
>  drivers/cxl/core/region.c | 50 +++++++++++++++++----------------------
>  1 file changed, 22 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 2cfc29a2b588..2dd9e9be4889 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3350,47 +3350,39 @@ static int devm_cxl_add_dax_region(struct cxl_region *cxlr)
>  	return rc;
>  }
>  
> -static int match_decoder_by_range(struct device *dev, const void *data)
> +static int match_root_decoder(struct device *dev, const void *data)
>  {
>  	const struct range *r1, *r2 = data;
> -	struct cxl_decoder *cxld;
> +	struct cxl_root_decoder *cxlrd;
>  
> -	if (!is_switch_decoder(dev))
> +	if (!is_root_decoder(dev))
>  		return 0;
>  
> -	cxld = to_cxl_decoder(dev);
> -	r1 = &cxld->hpa_range;
> -	return range_contains(r1, r2);
> -}
> +	cxlrd = to_cxl_root_decoder(dev);
> +	r1 = &cxlrd->cxlsd.cxld.hpa_range;
>  
> -static struct cxl_decoder *
> -cxl_port_find_switch_decoder(struct cxl_port *port, struct range *hpa_range)
> -{
> -	struct device *cxld_dev = device_find_child(&port->dev, hpa_range,
> -						    match_decoder_by_range);
> -
> -	return cxld_dev ? to_cxl_decoder(cxld_dev) : NULL;
> +	return range_contains(r1, r2);
>  }
>  
>  static struct cxl_root_decoder *
> -cxl_find_root_decoder(struct cxl_endpoint_decoder *cxled)
> +get_cxl_root_decoder(struct cxl_endpoint_decoder *cxled,
> +		     struct cxl_region_context *ctx)

Would like a comment that make note the caller should call put_device() when done with the root decoder.

DJ
 >  {
> -	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
>  	struct cxl_port *port = cxled_to_port(cxled);
>  	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
> -	struct cxl_decoder *root, *cxld = &cxled->cxld;
> -	struct range *hpa_range = &cxld->hpa_range;
> +	struct device *cxlrd_dev;
>  
> -	root = cxl_port_find_switch_decoder(&cxl_root->port, hpa_range);
> -	if (!root) {
> -		dev_err(cxlmd->dev.parent,
> +	cxlrd_dev = device_find_child(&cxl_root->port.dev, &ctx->hpa_range,
> +				      match_root_decoder);
> +	if (!cxlrd_dev) {
> +		dev_err(port->uport_dev,
>  			"%s:%s no CXL window for range %#llx:%#llx\n",
> -			dev_name(&cxlmd->dev), dev_name(&cxld->dev),
> -			hpa_range->start, hpa_range->end);
> -		return NULL;
> +			dev_name(&ctx->cxlmd->dev), dev_name(&cxled->cxld.dev),
> +			ctx->hpa_range.start, ctx->hpa_range.end);
> +		return ERR_PTR(-ENXIO);
>  	}
>  
> -	return to_cxl_root_decoder(&root->dev);
> +	return to_cxl_root_decoder(cxlrd_dev);
>  }
>  
>  static int match_region_by_range(struct device *dev, const void *data)
> @@ -3584,9 +3576,11 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  	};
>  
>  	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
> -		cxl_find_root_decoder(cxled);
> -	if (!cxlrd)
> -		return -ENXIO;
> +		get_cxl_root_decoder(cxled, &ctx);
> +
> +	rc = PTR_ERR_OR_ZERO(cxlrd);
> +	if (rc)
> +		return rc;
>  
>  	/*
>  	 * Ensure that if multiple threads race to construct_region()

Re: [PATCH v4 07/14] cxl/region: Use region data to get the root decoder

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:48 +0100
Robert Richter <rrichter@amd.com> wrote:

> To find a region's root decoder, the endpoint's HPA range is used to
> search the matching decoder by its range. With address translation the
> endpoint decoder's range is in a different address space and thus
> cannot be used to determine the root decoder.
> 
> The region parameters are encapsulated within struc cxl_region_context
> and may include the translated Host Physical Address (HPA) range. Use
> this context to identify the root decoder rather than relying on the
> endpoint.
> 
> Modify cxl_find_root_decoder() and add the region context as
> parameter. Rename this function to get_cxl_root_decoder() as a
> counterpart to put_cxl_root_decoder(). Simplify the implementation by
> removing function cxl_port_find_switch_decode(). The function is
> unnecessary because it is not referenced or utilized elsewhere in the
> code.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>

One trivial thing that tickled my "that looks different" filter.

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

>  static int match_region_by_range(struct device *dev, const void *data)
> @@ -3584,9 +3576,11 @@ int cxl_add_to_region(struct cxl_endpoint_decoder *cxled)
>  	};
>  
>  	struct cxl_root_decoder *cxlrd __free(put_cxl_root_decoder) =
> -		cxl_find_root_decoder(cxled);
> -	if (!cxlrd)
> -		return -ENXIO;
> +		get_cxl_root_decoder(cxled, &ctx);
> +
> +	rc = PTR_ERR_OR_ZERO(cxlrd);
> +	if (rc)
> +		return rc;

I think this is more often seen as:

	if (IS_ERR(cxlrd))
		return PTR_ERR(cxlrd);

>  
>  	/*
>  	 * Ensure that if multiple threads race to construct_region()

[PATCH v4 08/14] cxl: Introduce callback for HPA address ranges translation

[Robert Richter]

Introduce a callback to translate an endpoint's HPA range to the
address range of the root port which is the System Physical Address
(SPA) range used by a region. The callback can be set if a platform
needs to handle address translation.

The callback is attached to the root port. An endpoint's root port can
easily be determined in the PCI hierarchy without any CXL specific
knowledge. This allows the early use of address translation for CXL
enumeration. Address translation is esp. needed for the detection of
the root decoders. Thus, the callback is embedded in struct
cxl_root_ops instead of struct cxl_rd_ops.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 19 +++++++++++++++++++
 drivers/cxl/cxl.h         |  1 +
 2 files changed, 20 insertions(+)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 2dd9e9be4889..379a67cc8e31 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -3364,6 +3364,15 @@ static int match_root_decoder(struct device *dev, const void *data)
 	return range_contains(r1, r2);
 }
 
+static int translate_hpa_range(struct cxl_root *cxl_root,
+			       struct cxl_region_context *ctx)
+{
+	if (!cxl_root->ops.translate_hpa_range)
+		return 0;
+
+	return cxl_root->ops.translate_hpa_range(cxl_root, ctx);
+}
+
 static struct cxl_root_decoder *
 get_cxl_root_decoder(struct cxl_endpoint_decoder *cxled,
 		     struct cxl_region_context *ctx)
@@ -3371,6 +3380,16 @@ get_cxl_root_decoder(struct cxl_endpoint_decoder *cxled,
 	struct cxl_port *port = cxled_to_port(cxled);
 	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
 	struct device *cxlrd_dev;
+	int rc;
+
+	rc = translate_hpa_range(cxl_root, ctx);
+	if (rc) {
+		dev_err(port->uport_dev,
+			"%s:%s Failed to translate address range %#llx:%#llx\n",
+			dev_name(&ctx->cxlmd->dev), dev_name(&cxled->cxld.dev),
+			ctx->hpa_range.start, ctx->hpa_range.end);
+		return ERR_PTR(rc);
+	}
 
 	cxlrd_dev = device_find_child(&cxl_root->port.dev, &ctx->hpa_range,
 				      match_root_decoder);
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 9a381c827416..94b9fcc07469 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -644,6 +644,7 @@ struct cxl_root_ops {
 	int (*qos_class)(struct cxl_root *cxl_root,
 			 struct access_coordinate *coord, int entries,
 			 int *qos_class);
+	int (*translate_hpa_range)(struct cxl_root *cxl_root, void *data);
 };
 
 /**
-- 
2.47.3

Re: [PATCH v4 08/14] cxl: Introduce callback for HPA address ranges translation

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> Introduce a callback to translate an endpoint's HPA range to the
> address range of the root port which is the System Physical Address
> (SPA) range used by a region. The callback can be set if a platform
> needs to handle address translation.
> 
> The callback is attached to the root port. An endpoint's root port can
> easily be determined in the PCI hierarchy without any CXL specific
> knowledge. This allows the early use of address translation for CXL
> enumeration. Address translation is esp. needed for the detection of
> the root decoders. Thus, the callback is embedded in struct
> cxl_root_ops instead of struct cxl_rd_ops.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>> ---
>  drivers/cxl/core/region.c | 19 +++++++++++++++++++
>  drivers/cxl/cxl.h         |  1 +
>  2 files changed, 20 insertions(+)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 2dd9e9be4889..379a67cc8e31 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -3364,6 +3364,15 @@ static int match_root_decoder(struct device *dev, const void *data)
>  	return range_contains(r1, r2);
>  }
>  
> +static int translate_hpa_range(struct cxl_root *cxl_root,
> +			       struct cxl_region_context *ctx)
> +{
> +	if (!cxl_root->ops.translate_hpa_range)
> +		return 0;
> +
> +	return cxl_root->ops.translate_hpa_range(cxl_root, ctx);
> +}
> +
>  static struct cxl_root_decoder *
>  get_cxl_root_decoder(struct cxl_endpoint_decoder *cxled,
>  		     struct cxl_region_context *ctx)
> @@ -3371,6 +3380,16 @@ get_cxl_root_decoder(struct cxl_endpoint_decoder *cxled,
>  	struct cxl_port *port = cxled_to_port(cxled);
>  	struct cxl_root *cxl_root __free(put_cxl_root) = find_cxl_root(port);
>  	struct device *cxlrd_dev;
> +	int rc;
> +
> +	rc = translate_hpa_range(cxl_root, ctx);
> +	if (rc) {
> +		dev_err(port->uport_dev,
> +			"%s:%s Failed to translate address range %#llx:%#llx\n",
> +			dev_name(&ctx->cxlmd->dev), dev_name(&cxled->cxld.dev),
> +			ctx->hpa_range.start, ctx->hpa_range.end);
> +		return ERR_PTR(rc);
> +	}
>  
>  	cxlrd_dev = device_find_child(&cxl_root->port.dev, &ctx->hpa_range,
>  				      match_root_decoder);
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 9a381c827416..94b9fcc07469 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -644,6 +644,7 @@ struct cxl_root_ops {
>  	int (*qos_class)(struct cxl_root *cxl_root,
>  			 struct access_coordinate *coord, int entries,
>  			 int *qos_class);
> +	int (*translate_hpa_range)(struct cxl_root *cxl_root, void *data);
>  };
>  
>  /**

Re: [PATCH v4 08/14] cxl: Introduce callback for HPA address ranges translation

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:49 +0100
Robert Richter <rrichter@amd.com> wrote:

> Introduce a callback to translate an endpoint's HPA range to the
> address range of the root port which is the System Physical Address
> (SPA) range used by a region. The callback can be set if a platform
> needs to handle address translation.
> 
> The callback is attached to the root port. An endpoint's root port can
> easily be determined in the PCI hierarchy without any CXL specific
> knowledge. This allows the early use of address translation for CXL
> enumeration. Address translation is esp. needed for the detection of
> the root decoders. Thus, the callback is embedded in struct
> cxl_root_ops instead of struct cxl_rd_ops.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH v4 09/14] cxl/acpi: Prepare use of EFI runtime services

[Robert Richter]

In order to use EFI runtime services, esp. ACPI PRM which uses the
efi_rts_wq workqueue, initialize EFI before CXL ACPI.

There is a subsys_initcall order dependency if driver is builtin:

 subsys_initcall(cxl_acpi_init);
 subsys_initcall(efisubsys_init);

Prevent the efi_rts_wq workqueue being used by cxl_acpi_init() before
its allocation. Use subsys_initcall_sync(cxl_acpi_init) to always run
efisubsys_init() first.

Reported-by: Gregory Price <gourry@gourry.net>
Tested-by: Joshua Hahn <joshua.hahnjy@gmail.com>
Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>
Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/acpi.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 1ab780edf141..a54d56376787 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -996,8 +996,12 @@ static void __exit cxl_acpi_exit(void)
 	cxl_bus_drain();
 }
 
-/* load before dax_hmem sees 'Soft Reserved' CXL ranges */
-subsys_initcall(cxl_acpi_init);
+/*
+ * Load before dax_hmem sees 'Soft Reserved' CXL ranges. Use
+ * subsys_initcall_sync() since there is an order dependency with
+ * subsys_initcall(efisubsys_init), which must run first.
+ */
+subsys_initcall_sync(cxl_acpi_init);
 
 /*
  * Arrange for host-bridge ports to be active synchronous with
-- 
2.47.3

Re: [PATCH v4 09/14] cxl/acpi: Prepare use of EFI runtime services

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> In order to use EFI runtime services, esp. ACPI PRM which uses the
> efi_rts_wq workqueue, initialize EFI before CXL ACPI.
> 
> There is a subsys_initcall order dependency if driver is builtin:
> 
>  subsys_initcall(cxl_acpi_init);
>  subsys_initcall(efisubsys_init);
> 
> Prevent the efi_rts_wq workqueue being used by cxl_acpi_init() before
> its allocation. Use subsys_initcall_sync(cxl_acpi_init) to always run
> efisubsys_init() first.
> 
> Reported-by: Gregory Price <gourry@gourry.net>
> Tested-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>> ---
>  drivers/cxl/acpi.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 1ab780edf141..a54d56376787 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -996,8 +996,12 @@ static void __exit cxl_acpi_exit(void)
>  	cxl_bus_drain();
>  }
>  
> -/* load before dax_hmem sees 'Soft Reserved' CXL ranges */
> -subsys_initcall(cxl_acpi_init);
> +/*
> + * Load before dax_hmem sees 'Soft Reserved' CXL ranges. Use
> + * subsys_initcall_sync() since there is an order dependency with
> + * subsys_initcall(efisubsys_init), which must run first.
> + */
> +subsys_initcall_sync(cxl_acpi_init);
>  
>  /*
>   * Arrange for host-bridge ports to be active synchronous with

Re: [PATCH v4 09/14] cxl/acpi: Prepare use of EFI runtime services

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:50 +0100
Robert Richter <rrichter@amd.com> wrote:

> In order to use EFI runtime services, esp. ACPI PRM which uses the
> efi_rts_wq workqueue, initialize EFI before CXL ACPI.
> 
> There is a subsys_initcall order dependency if driver is builtin:
> 
>  subsys_initcall(cxl_acpi_init);
>  subsys_initcall(efisubsys_init);
> 
> Prevent the efi_rts_wq workqueue being used by cxl_acpi_init() before
> its allocation. Use subsys_initcall_sync(cxl_acpi_init) to always run
> efisubsys_init() first.
> 
> Reported-by: Gregory Price <gourry@gourry.net>
> Tested-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Reviewed-by: Joshua Hahn <joshua.hahnjy@gmail.com>
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Robert Richter]

Add AMD Zen5 support for address translation.

Zen5 systems may be configured to use 'Normalized addresses'. Then,
host physical addresses (HPA) are different from their system physical
addresses (SPA). The endpoint has its own physical address space and
an incoming HPA is already converted to the device's physical address
(DPA). Thus it has interleaving disabled and CXL endpoints are
programmed passthrough (DPA == HPA).

Host Physical Addresses (HPAs) need to be translated from the endpoint
to its CXL host bridge, esp. to identify the endpoint's root decoder
and region's address range. ACPI Platform Runtime Mechanism (PRM)
provides a handler to translate the DPA to its SPA. This is documented
in:

 AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
 ACPI v6.5 Porting Guide, Publication # 58088
 https://www.amd.com/en/search/documentation/hub.html

With Normalized Addressing this PRM handler must be used to translate
an HPA of an endpoint to its SPA.

Do the following to implement AMD Zen5 address translation:

Introduce a new file core/atl.c to handle ACPI PRM specific address
translation code. Naming is loosely related to the kernel's AMD
Address Translation Library (CONFIG_AMD_ATL) but implementation does
not depend on it, nor it is vendor specific. Use Kbuild and Kconfig
options respectively to enable the code depending on architecture and
platform options.

AMD Zen5 systems support the ACPI PRM CXL Address Translation firmware
call (see ACPI v6.5 Porting Guide, Address Translation - CXL DPA to
System Physical Address). Firmware enables the PRM handler if the
platform has address translation implemented. Check firmware and
kernel support of ACPI PRM using the specific GUID. On success enable
address translation by setting up the earlier introduced root port
callback, see function cxl_prm_translate_hpa_range(). Setup is done in
cxl_setup_prm_address_translation(), it is the only function that
needs to be exported. For low level PRM firmware calls, use the ACPI
framework.

Identify the region's interleaving ways by inspecting the address
ranges. Also determine the interleaving granularity using the address
translation callback. Note that the position of the chunk from one
interleaving block to the next may vary and thus cannot be considered
constant. Address offsets larger than the interleaving block size
cannot be used to calculate the granularity. Thus, probe the
granularity using address translation for various HPAs in the same
interleaving block.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/Kconfig       |   4 +
 drivers/cxl/acpi.c        |   2 +
 drivers/cxl/core/Makefile |   1 +
 drivers/cxl/core/atl.c    | 195 ++++++++++++++++++++++++++++++++++++++
 drivers/cxl/cxl.h         |   7 ++
 5 files changed, 209 insertions(+)
 create mode 100644 drivers/cxl/core/atl.c

diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
index 48b7314afdb8..e599badba69b 100644
--- a/drivers/cxl/Kconfig
+++ b/drivers/cxl/Kconfig
@@ -233,4 +233,8 @@ config CXL_MCE
 	def_bool y
 	depends on X86_MCE && MEMORY_FAILURE
 
+config CXL_ATL
+       def_bool y
+       depends on ACPI_PRMT && AMD_NB
+
 endif
diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index a54d56376787..f9bbc77f3ec2 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -916,6 +916,8 @@ static int cxl_acpi_probe(struct platform_device *pdev)
 	cxl_root->ops.qos_class = cxl_acpi_qos_class;
 	root_port = &cxl_root->port;
 
+	cxl_setup_prm_address_translation(cxl_root);
+
 	rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
 			      add_host_bridge_dport);
 	if (rc < 0)
diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
index 5ad8fef210b5..11fe272a6e29 100644
--- a/drivers/cxl/core/Makefile
+++ b/drivers/cxl/core/Makefile
@@ -20,3 +20,4 @@ cxl_core-$(CONFIG_CXL_REGION) += region.o
 cxl_core-$(CONFIG_CXL_MCE) += mce.o
 cxl_core-$(CONFIG_CXL_FEATURES) += features.o
 cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
+cxl_core-$(CONFIG_CXL_ATL) += atl.o
diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
new file mode 100644
index 000000000000..d6aa7e6d0ac5
--- /dev/null
+++ b/drivers/cxl/core/atl.c
@@ -0,0 +1,195 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2025 Advanced Micro Devices, Inc.
+ */
+
+#include <linux/prmt.h>
+#include <linux/pci.h>
+#include <linux/acpi.h>
+
+#include <cxlmem.h>
+#include "core.h"
+
+/*
+ * PRM Address Translation - CXL DPA to System Physical Address
+ *
+ * Reference:
+ *
+ * AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
+ * ACPI v6.5 Porting Guide, Publication # 58088
+ */
+
+static const guid_t prm_cxl_dpa_spa_guid =
+	GUID_INIT(0xee41b397, 0x25d4, 0x452c, 0xad, 0x54, 0x48, 0xc6, 0xe3,
+		  0x48, 0x0b, 0x94);
+
+struct prm_cxl_dpa_spa_data {
+	u64 dpa;
+	u8 reserved;
+	u8 devfn;
+	u8 bus;
+	u8 segment;
+	u64 *spa;
+} __packed;
+
+static u64 prm_cxl_dpa_spa(struct pci_dev *pci_dev, u64 dpa)
+{
+	struct prm_cxl_dpa_spa_data data;
+	u64 spa;
+	int rc;
+
+	data = (struct prm_cxl_dpa_spa_data) {
+		.dpa     = dpa,
+		.devfn   = pci_dev->devfn,
+		.bus     = pci_dev->bus->number,
+		.segment = pci_domain_nr(pci_dev->bus),
+		.spa     = &spa,
+	};
+
+	rc = acpi_call_prm_handler(prm_cxl_dpa_spa_guid, &data);
+	if (rc) {
+		pci_dbg(pci_dev, "failed to get SPA for %#llx: %d\n", dpa, rc);
+		return ULLONG_MAX;
+	}
+
+	pci_dbg(pci_dev, "PRM address translation: DPA -> SPA: %#llx -> %#llx\n", dpa, spa);
+
+	return spa;
+}
+
+static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
+{
+	struct cxl_region_context *ctx = data;
+	struct cxl_endpoint_decoder *cxled = ctx->cxled;
+	struct cxl_decoder *cxld = &cxled->cxld;
+	struct cxl_memdev *cxlmd = ctx->cxlmd;
+	struct range hpa_range = ctx->hpa_range;
+	struct pci_dev *pci_dev;
+	u64 spa_len, len = range_len(&hpa_range);
+	u64 addr, base_spa, base = hpa_range.start;
+	int ways, gran;
+
+	/*
+	 * When Normalized Addressing is enabled, the endpoint
+	 * maintains a 1:1 mapping between HPA and DPA. If disabled,
+	 * skip address translation and perform only a range check.
+	 */
+	if (hpa_range.start != cxled->dpa_res->start)
+		return 0;
+
+	if (!IS_ALIGNED(hpa_range.start, SZ_256M) ||
+	    !IS_ALIGNED(hpa_range.end + 1, SZ_256M)) {
+		dev_dbg(cxld->dev.parent,
+			"CXL address translation: Unaligned decoder HPA range: %#llx-%#llx(%s)\n",
+			hpa_range.start, hpa_range.end, dev_name(&cxld->dev));
+		return -ENXIO;
+	}
+
+	/*
+	 * Endpoints are programmed passthrough in Normalized
+	 * Addressing mode.
+	 */
+	if (ctx->interleave_ways != 1) {
+		dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
+			ctx->interleave_ways, ctx->interleave_granularity);
+		return -ENXIO;
+	}
+
+	if (!cxlmd || !dev_is_pci(cxlmd->dev.parent)) {
+		dev_dbg(&cxld->dev, "No endpoint found: %s, range %#llx-%#llx\n",
+			dev_name(cxld->dev.parent), hpa_range.start,
+			hpa_range.end);
+		return -ENXIO;
+	}
+
+	pci_dev = to_pci_dev(cxlmd->dev.parent);
+
+	/* Translate HPA range to SPA. */
+	hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
+	hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
+
+	if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
+		dev_dbg(cxld->dev.parent,
+			"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
+			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
+			ctx->hpa_range.end, dev_name(&cxld->dev));
+		return -ENXIO;
+	}
+
+	/*
+	 * Since translated addresses include the interleaving
+	 * offsets, align the range to 256 MB.
+	 */
+	hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
+	hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
+
+	spa_len = range_len(&hpa_range);
+	if (!len || !spa_len || spa_len % len) {
+		dev_dbg(cxld->dev.parent,
+			"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
+			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
+			ctx->hpa_range.end, dev_name(&cxld->dev));
+		return -ENXIO;
+	}
+
+	ways = spa_len / len;
+	gran = SZ_256;
+
+	/*
+	 * Determine interleave granularity
+	 *
+	 * Note: The position of the chunk from one interleaving block
+	 * to the next may vary and thus cannot be considered
+	 * constant. Address offsets larger than the interleaving
+	 * block size cannot be used to calculate the granularity.
+	 */
+	while (ways > 1 && gran <= SZ_16M) {
+		addr = prm_cxl_dpa_spa(pci_dev, base + gran);
+		if (addr != base_spa + gran)
+			break;
+		gran <<= 1;
+	}
+
+	if (gran > SZ_16M) {
+		dev_dbg(cxld->dev.parent,
+			"CXL address translation: Cannot determine granularity: %#llx-%#llx:%#llx-%#llx(%s)\n",
+			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
+			ctx->hpa_range.end, dev_name(&cxld->dev));
+		return -ENXIO;
+	}
+
+	ctx->hpa_range = hpa_range;
+	ctx->interleave_ways = ways;
+	ctx->interleave_granularity = gran;
+
+	dev_dbg(&cxld->dev,
+		"address mapping found for %s (hpa -> spa): %#llx+%#llx -> %#llx+%#llx ways:%d granularity:%d\n",
+		dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
+		spa_len, ways, gran);
+
+	return 0;
+}
+
+void cxl_setup_prm_address_translation(struct cxl_root *cxl_root)
+{
+	struct device *host = cxl_root->port.uport_dev;
+	u64 spa;
+	struct prm_cxl_dpa_spa_data data = { .spa = &spa, };
+	int rc;
+
+	/*
+	 * Applies only to PCIe Host Bridges which are children of the
+	 * CXL Root Device (HID=“ACPI0017”). Check this and drop
+	 * cxl_test instances.
+	 */
+	if (!acpi_match_device(host->driver->acpi_match_table, host))
+		return;
+
+	/* Check kernel (-EOPNOTSUPP) and firmware support (-ENODEV) */
+	rc = acpi_call_prm_handler(prm_cxl_dpa_spa_guid, &data);
+	if (rc == -EOPNOTSUPP || rc == -ENODEV)
+		return;
+
+	cxl_root->ops.translate_hpa_range = cxl_prm_translate_hpa_range;
+}
+EXPORT_SYMBOL_NS_GPL(cxl_setup_prm_address_translation, "CXL");
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 94b9fcc07469..0af46d1b0abc 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -790,6 +790,13 @@ static inline void cxl_dport_init_ras_reporting(struct cxl_dport *dport,
 						struct device *host) { }
 #endif
 
+#ifdef CONFIG_CXL_ATL
+void cxl_setup_prm_address_translation(struct cxl_root *cxl_root);
+#else
+static inline
+void cxl_setup_prm_address_translation(struct cxl_root *cxl_root) {}
+#endif
+
 struct cxl_decoder *to_cxl_decoder(struct device *dev);
 struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev);
 struct cxl_switch_decoder *to_cxl_switch_decoder(struct device *dev);
-- 
2.47.3

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> Add AMD Zen5 support for address translation.
> 
> Zen5 systems may be configured to use 'Normalized addresses'. Then,
> host physical addresses (HPA) are different from their system physical
> addresses (SPA). The endpoint has its own physical address space and
> an incoming HPA is already converted to the device's physical address
> (DPA). Thus it has interleaving disabled and CXL endpoints are
> programmed passthrough (DPA == HPA).
> 
> Host Physical Addresses (HPAs) need to be translated from the endpoint
> to its CXL host bridge, esp. to identify the endpoint's root decoder
> and region's address range. ACPI Platform Runtime Mechanism (PRM)
> provides a handler to translate the DPA to its SPA. This is documented
> in:
> 
>  AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
>  ACPI v6.5 Porting Guide, Publication # 58088
>  https://www.amd.com/en/search/documentation/hub.html
> 
> With Normalized Addressing this PRM handler must be used to translate
> an HPA of an endpoint to its SPA.
> 
> Do the following to implement AMD Zen5 address translation:
> 
> Introduce a new file core/atl.c to handle ACPI PRM specific address
> translation code. Naming is loosely related to the kernel's AMD
> Address Translation Library (CONFIG_AMD_ATL) but implementation does
> not depend on it, nor it is vendor specific. Use Kbuild and Kconfig
> options respectively to enable the code depending on architecture and
> platform options.
> 
> AMD Zen5 systems support the ACPI PRM CXL Address Translation firmware
> call (see ACPI v6.5 Porting Guide, Address Translation - CXL DPA to
> System Physical Address). Firmware enables the PRM handler if the
> platform has address translation implemented. Check firmware and
> kernel support of ACPI PRM using the specific GUID. On success enable
> address translation by setting up the earlier introduced root port
> callback, see function cxl_prm_translate_hpa_range(). Setup is done in
> cxl_setup_prm_address_translation(), it is the only function that
> needs to be exported. For low level PRM firmware calls, use the ACPI
> framework.
> 
> Identify the region's interleaving ways by inspecting the address
> ranges. Also determine the interleaving granularity using the address
> translation callback. Note that the position of the chunk from one
> interleaving block to the next may vary and thus cannot be considered
> constant. Address offsets larger than the interleaving block size
> cannot be used to calculate the granularity. Thus, probe the
> granularity using address translation for various HPAs in the same
> interleaving block.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
Just a small thing below, otherwise:
Reviewed-by: Dave Jiang <dave.jiang@intel.com>> ---
>  drivers/cxl/Kconfig       |   4 +
>  drivers/cxl/acpi.c        |   2 +
>  drivers/cxl/core/Makefile |   1 +
>  drivers/cxl/core/atl.c    | 195 ++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/cxl.h         |   7 ++
>  5 files changed, 209 insertions(+)
>  create mode 100644 drivers/cxl/core/atl.c
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 48b7314afdb8..e599badba69b 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -233,4 +233,8 @@ config CXL_MCE
>  	def_bool y
>  	depends on X86_MCE && MEMORY_FAILURE
>  
> +config CXL_ATL
> +       def_bool y
> +       depends on ACPI_PRMT && AMD_NB
> +
>  endif
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index a54d56376787..f9bbc77f3ec2 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -916,6 +916,8 @@ static int cxl_acpi_probe(struct platform_device *pdev)
>  	cxl_root->ops.qos_class = cxl_acpi_qos_class;
>  	root_port = &cxl_root->port;
>  
> +	cxl_setup_prm_address_translation(cxl_root);
> +
>  	rc = bus_for_each_dev(adev->dev.bus, NULL, root_port,
>  			      add_host_bridge_dport);
>  	if (rc < 0)
> diff --git a/drivers/cxl/core/Makefile b/drivers/cxl/core/Makefile
> index 5ad8fef210b5..11fe272a6e29 100644
> --- a/drivers/cxl/core/Makefile
> +++ b/drivers/cxl/core/Makefile
> @@ -20,3 +20,4 @@ cxl_core-$(CONFIG_CXL_REGION) += region.o
>  cxl_core-$(CONFIG_CXL_MCE) += mce.o
>  cxl_core-$(CONFIG_CXL_FEATURES) += features.o
>  cxl_core-$(CONFIG_CXL_EDAC_MEM_FEATURES) += edac.o
> +cxl_core-$(CONFIG_CXL_ATL) += atl.o
> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> new file mode 100644
> index 000000000000..d6aa7e6d0ac5
> --- /dev/null
> +++ b/drivers/cxl/core/atl.c
> @@ -0,0 +1,195 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2025 Advanced Micro Devices, Inc.
> + */
> +
> +#include <linux/prmt.h>
> +#include <linux/pci.h>
> +#include <linux/acpi.h>
> +
> +#include <cxlmem.h>
> +#include "core.h"
> +
> +/*
> + * PRM Address Translation - CXL DPA to System Physical Address
> + *
> + * Reference:
> + *
> + * AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
> + * ACPI v6.5 Porting Guide, Publication # 58088
> + */
> +
> +static const guid_t prm_cxl_dpa_spa_guid =
> +	GUID_INIT(0xee41b397, 0x25d4, 0x452c, 0xad, 0x54, 0x48, 0xc6, 0xe3,
> +		  0x48, 0x0b, 0x94);
> +
> +struct prm_cxl_dpa_spa_data {
> +	u64 dpa;
> +	u8 reserved;
> +	u8 devfn;
> +	u8 bus;
> +	u8 segment;
> +	u64 *spa;
> +} __packed;
> +
> +static u64 prm_cxl_dpa_spa(struct pci_dev *pci_dev, u64 dpa)
> +{
> +	struct prm_cxl_dpa_spa_data data;
> +	u64 spa;
> +	int rc;
> +
> +	data = (struct prm_cxl_dpa_spa_data) {
> +		.dpa     = dpa,
> +		.devfn   = pci_dev->devfn,
> +		.bus     = pci_dev->bus->number,
> +		.segment = pci_domain_nr(pci_dev->bus),
> +		.spa     = &spa,
> +	};
> +
> +	rc = acpi_call_prm_handler(prm_cxl_dpa_spa_guid, &data);
> +	if (rc) {
> +		pci_dbg(pci_dev, "failed to get SPA for %#llx: %d\n", dpa, rc);
> +		return ULLONG_MAX;
> +	}
> +
> +	pci_dbg(pci_dev, "PRM address translation: DPA -> SPA: %#llx -> %#llx\n", dpa, spa);
> +
> +	return spa;
> +}
> +
> +static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> +{
> +	struct cxl_region_context *ctx = data;
> +	struct cxl_endpoint_decoder *cxled = ctx->cxled;
> +	struct cxl_decoder *cxld = &cxled->cxld;
> +	struct cxl_memdev *cxlmd = ctx->cxlmd;
> +	struct range hpa_range = ctx->hpa_range;
> +	struct pci_dev *pci_dev;
> +	u64 spa_len, len = range_len(&hpa_range);
> +	u64 addr, base_spa, base = hpa_range.start;
> +	int ways, gran;
> +
> +	/*
> +	 * When Normalized Addressing is enabled, the endpoint
> +	 * maintains a 1:1 mapping between HPA and DPA. If disabled,
> +	 * skip address translation and perform only a range check.
> +	 */
> +	if (hpa_range.start != cxled->dpa_res->start)
> +		return 0;
> +
> +	if (!IS_ALIGNED(hpa_range.start, SZ_256M) ||
> +	    !IS_ALIGNED(hpa_range.end + 1, SZ_256M)) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Unaligned decoder HPA range: %#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * Endpoints are programmed passthrough in Normalized
> +	 * Addressing mode.
> +	 */
> +	if (ctx->interleave_ways != 1) {
> +		dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
> +			ctx->interleave_ways, ctx->interleave_granularity);
> +		return -ENXIO;
> +	}
> +
> +	if (!cxlmd || !dev_is_pci(cxlmd->dev.parent)) {
> +		dev_dbg(&cxld->dev, "No endpoint found: %s, range %#llx-%#llx\n",
> +			dev_name(cxld->dev.parent), hpa_range.start,
> +			hpa_range.end);
> +		return -ENXIO;
> +	}
> +
> +	pci_dev = to_pci_dev(cxlmd->dev.parent);
> +
> +	/* Translate HPA range to SPA. */
> +	hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
> +	hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
> +
> +	if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * Since translated addresses include the interleaving
> +	 * offsets, align the range to 256 MB.
> +	 */
> +	hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
> +	hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
> +
> +	spa_len = range_len(&hpa_range);
> +	if (!len || !spa_len || spa_len % len) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	ways = spa_len / len;
> +	gran = SZ_256;

maybe init 'base' and 'base_hpa' here. Makes it easier to recall rather than having to go up to recall what it was.> +
> +	/*
> +	 * Determine interleave granularity
> +	 *
> +	 * Note: The position of the chunk from one interleaving block
> +	 * to the next may vary and thus cannot be considered
> +	 * constant. Address offsets larger than the interleaving
> +	 * block size cannot be used to calculate the granularity.
> +	 */
> +	while (ways > 1 && gran <= SZ_16M) {
> +		addr = prm_cxl_dpa_spa(pci_dev, base + gran);
> +		if (addr != base_spa + gran)
> +			break;
> +		gran <<= 1;
> +	}
> +
> +	if (gran > SZ_16M) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Cannot determine granularity: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	ctx->hpa_range = hpa_range;
> +	ctx->interleave_ways = ways;
> +	ctx->interleave_granularity = gran;
> +
> +	dev_dbg(&cxld->dev,
> +		"address mapping found for %s (hpa -> spa): %#llx+%#llx -> %#llx+%#llx ways:%d granularity:%d\n",
> +		dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
> +		spa_len, ways, gran);
> +
> +	return 0;
> +}
> +
> +void cxl_setup_prm_address_translation(struct cxl_root *cxl_root)
> +{
> +	struct device *host = cxl_root->port.uport_dev;
> +	u64 spa;
> +	struct prm_cxl_dpa_spa_data data = { .spa = &spa, };
> +	int rc;
> +
> +	/*
> +	 * Applies only to PCIe Host Bridges which are children of the
> +	 * CXL Root Device (HID=“ACPI0017”). Check this and drop
> +	 * cxl_test instances.
> +	 */
> +	if (!acpi_match_device(host->driver->acpi_match_table, host))
> +		return;
> +
> +	/* Check kernel (-EOPNOTSUPP) and firmware support (-ENODEV) */
> +	rc = acpi_call_prm_handler(prm_cxl_dpa_spa_guid, &data);
> +	if (rc == -EOPNOTSUPP || rc == -ENODEV)
> +		return;
> +
> +	cxl_root->ops.translate_hpa_range = cxl_prm_translate_hpa_range;
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_setup_prm_address_translation, "CXL");
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 94b9fcc07469..0af46d1b0abc 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -790,6 +790,13 @@ static inline void cxl_dport_init_ras_reporting(struct cxl_dport *dport,
>  						struct device *host) { }
>  #endif
>  
> +#ifdef CONFIG_CXL_ATL
> +void cxl_setup_prm_address_translation(struct cxl_root *cxl_root);
> +#else
> +static inline
> +void cxl_setup_prm_address_translation(struct cxl_root *cxl_root) {}
> +#endif
> +
>  struct cxl_decoder *to_cxl_decoder(struct device *dev);
>  struct cxl_root_decoder *to_cxl_root_decoder(struct device *dev);
>  struct cxl_switch_decoder *to_cxl_switch_decoder(struct device *dev);

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Robert Richter]

On 03.11.25 18:00:08, Dave Jiang wrote:
> On 11/3/25 11:47 AM, Robert Richter wrote:

> > +	/* Translate HPA range to SPA. */
> > +	hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
> > +	hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
> > +
> > +	if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
> > +		dev_dbg(cxld->dev.parent,
> > +			"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
> > +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> > +			ctx->hpa_range.end, dev_name(&cxld->dev));
> > +		return -ENXIO;
> > +	}
> > +
> > +	/*
> > +	 * Since translated addresses include the interleaving
> > +	 * offsets, align the range to 256 MB.
> > +	 */
> > +	hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
> > +	hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
> > +
> > +	spa_len = range_len(&hpa_range);
> > +	if (!len || !spa_len || spa_len % len) {
> > +		dev_dbg(cxld->dev.parent,
> > +			"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
> > +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> > +			ctx->hpa_range.end, dev_name(&cxld->dev));
> > +		return -ENXIO;
> > +	}
> > +
> > +	ways = spa_len / len;
> > +	gran = SZ_256;
>
> maybe init 'base' and 'base_hpa' here. Makes it easier to recall
> rather than having to go up to recall what it was.> +

I've ended up to initialize base variable close together for a better
context:

	/* Translate HPA range to SPA. */                           
	base = hpa_range.start;                                     
	hpa_range.start = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
	hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);    
	base_spa = hpa_range.start;                                 

Values change an thus it connot be init later.

-Robert

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[kernel test robot]

Hi Robert,

kernel test robot noticed the following build errors:

[auto build test ERROR on 211ddde0823f1442e4ad052a2f30f050145ccada]

url:    https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-region-Store-root-decoder-in-struct-cxl_region/20251104-025351
base:   211ddde0823f1442e4ad052a2f30f050145ccada
patch link:    https://lore.kernel.org/r/20251103184804.509762-11-rrichter%40amd.com
patch subject: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT
config: x86_64-randconfig-072-20251104 (https://download.01.org/0day-ci/archive/20251104/202511041721.oCz4BaCp-lkp@intel.com/config)
compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511041721.oCz4BaCp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511041721.oCz4BaCp-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/cxl/core/atl.c:63:42: error: incomplete definition of type 'struct cxl_region_context'
      63 |         struct cxl_endpoint_decoder *cxled = ctx->cxled;
         |                                              ~~~^
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:65:32: error: incomplete definition of type 'struct cxl_region_context'
      65 |         struct cxl_memdev *cxlmd = ctx->cxlmd;
         |                                    ~~~^
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:66:30: error: incomplete definition of type 'struct cxl_region_context'
      66 |         struct range hpa_range = ctx->hpa_range;
         |                                  ~~~^
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:92:9: error: incomplete definition of type 'struct cxl_region_context'
      92 |         if (ctx->interleave_ways != 1) {
         |             ~~~^
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:94:7: error: incomplete definition of type 'struct cxl_region_context'
      94 |                         ctx->interleave_ways, ctx->interleave_granularity);
         |                         ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:94:29: error: incomplete definition of type 'struct cxl_region_context'
      94 |                         ctx->interleave_ways, ctx->interleave_granularity);
         |                                               ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:114:39: error: incomplete definition of type 'struct cxl_region_context'
     114 |                         hpa_range.start, hpa_range.end, ctx->hpa_range.start,
         |                                                         ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:115:7: error: incomplete definition of type 'struct cxl_region_context'
     115 |                         ctx->hpa_range.end, dev_name(&cxld->dev));
         |                         ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:130:39: error: incomplete definition of type 'struct cxl_region_context'
     130 |                         hpa_range.start, hpa_range.end, ctx->hpa_range.start,
         |                                                         ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:131:7: error: incomplete definition of type 'struct cxl_region_context'
     131 |                         ctx->hpa_range.end, dev_name(&cxld->dev));
         |                         ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)
         |                                                        ^~~~~~~~~~~
   include/linux/dev_printk.h:139:35: note: expanded from macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:62:9: note: forward declaration of 'struct cxl_region_context'
      62 |         struct cxl_region_context *ctx = data;
         |                ^
   drivers/cxl/core/atl.c:156:39: error: incomplete definition of type 'struct cxl_region_context'
     156 |                         hpa_range.start, hpa_range.end, ctx->hpa_range.start,
         |                                                         ~~~^
   include/linux/dev_printk.h:171:49: note: expanded from macro 'dev_dbg'
     171 |         dev_no_printk(KERN_DEBUG, dev, dev_fmt(fmt), ##__VA_ARGS__)


vim +63 drivers/cxl/core/atl.c

    59	
    60	static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
    61	{
    62		struct cxl_region_context *ctx = data;
  > 63		struct cxl_endpoint_decoder *cxled = ctx->cxled;
    64		struct cxl_decoder *cxld = &cxled->cxld;
    65		struct cxl_memdev *cxlmd = ctx->cxlmd;
    66		struct range hpa_range = ctx->hpa_range;
    67		struct pci_dev *pci_dev;
    68		u64 spa_len, len = range_len(&hpa_range);
    69		u64 addr, base_spa, base = hpa_range.start;
    70		int ways, gran;
    71	
    72		/*
    73		 * When Normalized Addressing is enabled, the endpoint
    74		 * maintains a 1:1 mapping between HPA and DPA. If disabled,
    75		 * skip address translation and perform only a range check.
    76		 */
    77		if (hpa_range.start != cxled->dpa_res->start)
    78			return 0;
    79	
    80		if (!IS_ALIGNED(hpa_range.start, SZ_256M) ||
    81		    !IS_ALIGNED(hpa_range.end + 1, SZ_256M)) {
    82			dev_dbg(cxld->dev.parent,
    83				"CXL address translation: Unaligned decoder HPA range: %#llx-%#llx(%s)\n",
    84				hpa_range.start, hpa_range.end, dev_name(&cxld->dev));
    85			return -ENXIO;
    86		}
    87	
    88		/*
    89		 * Endpoints are programmed passthrough in Normalized
    90		 * Addressing mode.
    91		 */
    92		if (ctx->interleave_ways != 1) {
    93			dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
    94				ctx->interleave_ways, ctx->interleave_granularity);
    95			return -ENXIO;
    96		}
    97	
    98		if (!cxlmd || !dev_is_pci(cxlmd->dev.parent)) {
    99			dev_dbg(&cxld->dev, "No endpoint found: %s, range %#llx-%#llx\n",
   100				dev_name(cxld->dev.parent), hpa_range.start,
   101				hpa_range.end);
   102			return -ENXIO;
   103		}
   104	
   105		pci_dev = to_pci_dev(cxlmd->dev.parent);
   106	
   107		/* Translate HPA range to SPA. */
   108		hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
   109		hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
   110	
   111		if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
   112			dev_dbg(cxld->dev.parent,
   113				"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
   114				hpa_range.start, hpa_range.end, ctx->hpa_range.start,
   115				ctx->hpa_range.end, dev_name(&cxld->dev));
   116			return -ENXIO;
   117		}
   118	
   119		/*
   120		 * Since translated addresses include the interleaving
   121		 * offsets, align the range to 256 MB.
   122		 */
   123		hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
   124		hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
   125	
   126		spa_len = range_len(&hpa_range);
   127		if (!len || !spa_len || spa_len % len) {
   128			dev_dbg(cxld->dev.parent,
   129				"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
   130				hpa_range.start, hpa_range.end, ctx->hpa_range.start,
   131				ctx->hpa_range.end, dev_name(&cxld->dev));
   132			return -ENXIO;
   133		}
   134	
   135		ways = spa_len / len;
   136		gran = SZ_256;
   137	
   138		/*
   139		 * Determine interleave granularity
   140		 *
   141		 * Note: The position of the chunk from one interleaving block
   142		 * to the next may vary and thus cannot be considered
   143		 * constant. Address offsets larger than the interleaving
   144		 * block size cannot be used to calculate the granularity.
   145		 */
   146		while (ways > 1 && gran <= SZ_16M) {
   147			addr = prm_cxl_dpa_spa(pci_dev, base + gran);
   148			if (addr != base_spa + gran)
   149				break;
   150			gran <<= 1;
   151		}
   152	
   153		if (gran > SZ_16M) {
   154			dev_dbg(cxld->dev.parent,
   155				"CXL address translation: Cannot determine granularity: %#llx-%#llx:%#llx-%#llx(%s)\n",
   156				hpa_range.start, hpa_range.end, ctx->hpa_range.start,
   157				ctx->hpa_range.end, dev_name(&cxld->dev));
   158			return -ENXIO;
   159		}
   160	
   161		ctx->hpa_range = hpa_range;
   162		ctx->interleave_ways = ways;
   163		ctx->interleave_granularity = gran;
   164	
   165		dev_dbg(&cxld->dev,
   166			"address mapping found for %s (hpa -> spa): %#llx+%#llx -> %#llx+%#llx ways:%d granularity:%d\n",
   167			dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
   168			spa_len, ways, gran);
   169	
   170		return 0;
   171	}
   172	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Robert Richter]

On 04.11.25 17:33:06, kernel test robot wrote:
> Hi Robert,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on 211ddde0823f1442e4ad052a2f30f050145ccada]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-region-Store-root-decoder-in-struct-cxl_region/20251104-025351
> base:   211ddde0823f1442e4ad052a2f30f050145ccada
> patch link:    https://lore.kernel.org/r/20251103184804.509762-11-rrichter%40amd.com
> patch subject: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT
> config: x86_64-randconfig-072-20251104 (https://download.01.org/0day-ci/archive/20251104/202511041721.oCz4BaCp-lkp@intel.com/config)
> compiler: clang version 20.1.8 (https://github.com/llvm/llvm-project 87f0227cb60147a26a1eeb4fb06e3b505e9c7261)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251104/202511041721.oCz4BaCp-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202511041721.oCz4BaCp-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
> >> drivers/cxl/core/atl.c:63:42: error: incomplete definition of type 'struct cxl_region_context'
>       63 |         struct cxl_endpoint_decoder *cxled = ctx->cxled;
>          |                                              ~~~^

I have a fix for that which will be part of v5.

-Robert

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[kernel test robot]

Hi Robert,

kernel test robot noticed the following build errors:

[auto build test ERROR on 211ddde0823f1442e4ad052a2f30f050145ccada]

url:    https://github.com/intel-lab-lkp/linux/commits/Robert-Richter/cxl-region-Store-root-decoder-in-struct-cxl_region/20251104-025351
base:   211ddde0823f1442e4ad052a2f30f050145ccada
patch link:    https://lore.kernel.org/r/20251103184804.509762-11-rrichter%40amd.com
patch subject: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT
config: x86_64-randconfig-071-20251105 (https://download.01.org/0day-ci/archive/20251105/202511050720.Hy1VQf0n-lkp@intel.com/config)
compiler: gcc-14 (Debian 14.2.0-19) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20251105/202511050720.Hy1VQf0n-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202511050720.Hy1VQf0n-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/cxl/core/atl.c: In function 'cxl_prm_translate_hpa_range':
>> drivers/cxl/core/atl.c:63:49: error: invalid use of undefined type 'struct cxl_region_context'
      63 |         struct cxl_endpoint_decoder *cxled = ctx->cxled;
         |                                                 ^~
   drivers/cxl/core/atl.c:65:39: error: invalid use of undefined type 'struct cxl_region_context'
      65 |         struct cxl_memdev *cxlmd = ctx->cxlmd;
         |                                       ^~
   drivers/cxl/core/atl.c:66:37: error: invalid use of undefined type 'struct cxl_region_context'
      66 |         struct range hpa_range = ctx->hpa_range;
         |                                     ^~
   drivers/cxl/core/atl.c:92:16: error: invalid use of undefined type 'struct cxl_region_context'
      92 |         if (ctx->interleave_ways != 1) {
         |                ^~
   In file included from include/linux/device.h:15,
                    from include/linux/pci.h:37,
                    from drivers/cxl/core/atl.c:7:
   drivers/cxl/core/atl.c:94:28: error: invalid use of undefined type 'struct cxl_region_context'
      94 |                         ctx->interleave_ways, ctx->interleave_granularity);
         |                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:93:17: note: in expansion of macro 'dev_dbg'
      93 |                 dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:94:50: error: invalid use of undefined type 'struct cxl_region_context'
      94 |                         ctx->interleave_ways, ctx->interleave_granularity);
         |                                                  ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:93:17: note: in expansion of macro 'dev_dbg'
      93 |                 dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:114:60: error: invalid use of undefined type 'struct cxl_region_context'
     114 |                         hpa_range.start, hpa_range.end, ctx->hpa_range.start,
         |                                                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:112:17: note: in expansion of macro 'dev_dbg'
     112 |                 dev_dbg(cxld->dev.parent,
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:115:28: error: invalid use of undefined type 'struct cxl_region_context'
     115 |                         ctx->hpa_range.end, dev_name(&cxld->dev));
         |                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:112:17: note: in expansion of macro 'dev_dbg'
     112 |                 dev_dbg(cxld->dev.parent,
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:130:60: error: invalid use of undefined type 'struct cxl_region_context'
     130 |                         hpa_range.start, hpa_range.end, ctx->hpa_range.start,
         |                                                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:128:17: note: in expansion of macro 'dev_dbg'
     128 |                 dev_dbg(cxld->dev.parent,
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:131:28: error: invalid use of undefined type 'struct cxl_region_context'
     131 |                         ctx->hpa_range.end, dev_name(&cxld->dev));
         |                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:128:17: note: in expansion of macro 'dev_dbg'
     128 |                 dev_dbg(cxld->dev.parent,
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:156:60: error: invalid use of undefined type 'struct cxl_region_context'
     156 |                         hpa_range.start, hpa_range.end, ctx->hpa_range.start,
         |                                                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:154:17: note: in expansion of macro 'dev_dbg'
     154 |                 dev_dbg(cxld->dev.parent,
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:157:28: error: invalid use of undefined type 'struct cxl_region_context'
     157 |                         ctx->hpa_range.end, dev_name(&cxld->dev));
         |                            ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \
         |                                                        ^~~~~~~~~~~
   drivers/cxl/core/atl.c:154:17: note: in expansion of macro 'dev_dbg'
     154 |                 dev_dbg(cxld->dev.parent,
         |                 ^~~~~~~
   drivers/cxl/core/atl.c:161:12: error: invalid use of undefined type 'struct cxl_region_context'
     161 |         ctx->hpa_range = hpa_range;
         |            ^~
   drivers/cxl/core/atl.c:162:12: error: invalid use of undefined type 'struct cxl_region_context'
     162 |         ctx->interleave_ways = ways;
         |            ^~
   drivers/cxl/core/atl.c:163:12: error: invalid use of undefined type 'struct cxl_region_context'
     163 |         ctx->interleave_granularity = gran;
         |            ^~
   drivers/cxl/core/atl.c:167:29: error: invalid use of undefined type 'struct cxl_region_context'
     167 |                 dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
         |                             ^~
   include/linux/dev_printk.h:139:56: note: in definition of macro 'dev_no_printk'
     139 |                         _dev_printk(level, dev, fmt, ##__VA_ARGS__);    \


vim +63 drivers/cxl/core/atl.c

    59	
    60	static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
    61	{
    62		struct cxl_region_context *ctx = data;
  > 63		struct cxl_endpoint_decoder *cxled = ctx->cxled;
    64		struct cxl_decoder *cxld = &cxled->cxld;
    65		struct cxl_memdev *cxlmd = ctx->cxlmd;
    66		struct range hpa_range = ctx->hpa_range;
    67		struct pci_dev *pci_dev;
    68		u64 spa_len, len = range_len(&hpa_range);
    69		u64 addr, base_spa, base = hpa_range.start;
    70		int ways, gran;
    71	
    72		/*
    73		 * When Normalized Addressing is enabled, the endpoint
    74		 * maintains a 1:1 mapping between HPA and DPA. If disabled,
    75		 * skip address translation and perform only a range check.
    76		 */
    77		if (hpa_range.start != cxled->dpa_res->start)
    78			return 0;
    79	
    80		if (!IS_ALIGNED(hpa_range.start, SZ_256M) ||
    81		    !IS_ALIGNED(hpa_range.end + 1, SZ_256M)) {
    82			dev_dbg(cxld->dev.parent,
    83				"CXL address translation: Unaligned decoder HPA range: %#llx-%#llx(%s)\n",
    84				hpa_range.start, hpa_range.end, dev_name(&cxld->dev));
    85			return -ENXIO;
    86		}
    87	
    88		/*
    89		 * Endpoints are programmed passthrough in Normalized
    90		 * Addressing mode.
    91		 */
    92		if (ctx->interleave_ways != 1) {
    93			dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
    94				ctx->interleave_ways, ctx->interleave_granularity);
    95			return -ENXIO;
    96		}
    97	
    98		if (!cxlmd || !dev_is_pci(cxlmd->dev.parent)) {
    99			dev_dbg(&cxld->dev, "No endpoint found: %s, range %#llx-%#llx\n",
   100				dev_name(cxld->dev.parent), hpa_range.start,
   101				hpa_range.end);
   102			return -ENXIO;
   103		}
   104	
   105		pci_dev = to_pci_dev(cxlmd->dev.parent);
   106	
   107		/* Translate HPA range to SPA. */
   108		hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
   109		hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
   110	
   111		if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
   112			dev_dbg(cxld->dev.parent,
   113				"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
   114				hpa_range.start, hpa_range.end, ctx->hpa_range.start,
   115				ctx->hpa_range.end, dev_name(&cxld->dev));
   116			return -ENXIO;
   117		}
   118	
   119		/*
   120		 * Since translated addresses include the interleaving
   121		 * offsets, align the range to 256 MB.
   122		 */
   123		hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
   124		hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
   125	
   126		spa_len = range_len(&hpa_range);
   127		if (!len || !spa_len || spa_len % len) {
   128			dev_dbg(cxld->dev.parent,
   129				"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
   130				hpa_range.start, hpa_range.end, ctx->hpa_range.start,
   131				ctx->hpa_range.end, dev_name(&cxld->dev));
   132			return -ENXIO;
   133		}
   134	
   135		ways = spa_len / len;
   136		gran = SZ_256;
   137	
   138		/*
   139		 * Determine interleave granularity
   140		 *
   141		 * Note: The position of the chunk from one interleaving block
   142		 * to the next may vary and thus cannot be considered
   143		 * constant. Address offsets larger than the interleaving
   144		 * block size cannot be used to calculate the granularity.
   145		 */
   146		while (ways > 1 && gran <= SZ_16M) {
   147			addr = prm_cxl_dpa_spa(pci_dev, base + gran);
   148			if (addr != base_spa + gran)
   149				break;
   150			gran <<= 1;
   151		}
   152	
   153		if (gran > SZ_16M) {
   154			dev_dbg(cxld->dev.parent,
   155				"CXL address translation: Cannot determine granularity: %#llx-%#llx:%#llx-%#llx(%s)\n",
   156				hpa_range.start, hpa_range.end, ctx->hpa_range.start,
   157				ctx->hpa_range.end, dev_name(&cxld->dev));
   158			return -ENXIO;
   159		}
   160	
   161		ctx->hpa_range = hpa_range;
   162		ctx->interleave_ways = ways;
   163		ctx->interleave_granularity = gran;
   164	
   165		dev_dbg(&cxld->dev,
   166			"address mapping found for %s (hpa -> spa): %#llx+%#llx -> %#llx+%#llx ways:%d granularity:%d\n",
   167			dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
   168			spa_len, ways, gran);
   169	
   170		return 0;
   171	}
   172	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:51 +0100
Robert Richter <rrichter@amd.com> wrote:

> Add AMD Zen5 support for address translation.
> 
> Zen5 systems may be configured to use 'Normalized addresses'. Then,
> host physical addresses (HPA) are different from their system physical
> addresses (SPA). The endpoint has its own physical address space and
> an incoming HPA is already converted to the device's physical address
> (DPA). Thus it has interleaving disabled and CXL endpoints are
> programmed passthrough (DPA == HPA).
> 
> Host Physical Addresses (HPAs) need to be translated from the endpoint
> to its CXL host bridge, esp. to identify the endpoint's root decoder
> and region's address range. ACPI Platform Runtime Mechanism (PRM)
> provides a handler to translate the DPA to its SPA. This is documented
> in:
> 
>  AMD Family 1Ah Models 00h–0Fh and Models 10h–1Fh
>  ACPI v6.5 Porting Guide, Publication # 58088
>  https://www.amd.com/en/search/documentation/hub.html
> 
> With Normalized Addressing this PRM handler must be used to translate
> an HPA of an endpoint to its SPA.
> 
> Do the following to implement AMD Zen5 address translation:
> 
> Introduce a new file core/atl.c to handle ACPI PRM specific address
> translation code. Naming is loosely related to the kernel's AMD
> Address Translation Library (CONFIG_AMD_ATL) but implementation does
> not depend on it, nor it is vendor specific. Use Kbuild and Kconfig
> options respectively to enable the code depending on architecture and
> platform options.
> 
> AMD Zen5 systems support the ACPI PRM CXL Address Translation firmware
> call (see ACPI v6.5 Porting Guide, Address Translation - CXL DPA to
> System Physical Address). Firmware enables the PRM handler if the
> platform has address translation implemented. Check firmware and
> kernel support of ACPI PRM using the specific GUID. On success enable
> address translation by setting up the earlier introduced root port
> callback, see function cxl_prm_translate_hpa_range(). Setup is done in
> cxl_setup_prm_address_translation(), it is the only function that
> needs to be exported. For low level PRM firmware calls, use the ACPI
> framework.
> 
> Identify the region's interleaving ways by inspecting the address
> ranges. Also determine the interleaving granularity using the address
> translation callback. Note that the position of the chunk from one
> interleaving block to the next may vary and thus cannot be considered
> constant. Address offsets larger than the interleaving block size
> cannot be used to calculate the granularity. Thus, probe the
> granularity using address translation for various HPAs in the same
> interleaving block.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
A few things below.  Given they are just trivial formatting things
and I assume you'll fix the build bot issues.

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

> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> new file mode 100644
> index 000000000000..d6aa7e6d0ac5
> --- /dev/null
> +++ b/drivers/cxl/core/atl.c
> @@ -0,0 +1,195 @@

> +static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> +{
> +	struct cxl_region_context *ctx = data;
> +	struct cxl_endpoint_decoder *cxled = ctx->cxled;
> +	struct cxl_decoder *cxld = &cxled->cxld;
> +	struct cxl_memdev *cxlmd = ctx->cxlmd;
> +	struct range hpa_range = ctx->hpa_range;
> +	struct pci_dev *pci_dev;
> +	u64 spa_len, len = range_len(&hpa_range);
> +	u64 addr, base_spa, base = hpa_range.start;

This is getting a bit nasty to read. I'd split the declarations
that initialize from the ones that don't. So use a couple more lines
to help readability a little.

> +	int ways, gran;
> +
> +	/*
> +	 * When Normalized Addressing is enabled, the endpoint
> +	 * maintains a 1:1 mapping between HPA and DPA. If disabled,
> +	 * skip address translation and perform only a range check.

Wrap is a little short. 80 chars for code and comments (unless there
is reason to go longer).

> +	 */
> +	if (hpa_range.start != cxled->dpa_res->start)
> +		return 0;
> +
> +	if (!IS_ALIGNED(hpa_range.start, SZ_256M) ||
> +	    !IS_ALIGNED(hpa_range.end + 1, SZ_256M)) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Unaligned decoder HPA range: %#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * Endpoints are programmed passthrough in Normalized
> +	 * Addressing mode.

Tiny bit early on the wrap.  Aim for 80 chars, which puts addressing on the line above.

> +	 */
> +	if (ctx->interleave_ways != 1) {
> +		dev_dbg(&cxld->dev, "unexpected interleaving config: ways: %d granularity: %d\n",
> +			ctx->interleave_ways, ctx->interleave_granularity);
> +		return -ENXIO;
> +	}
> +
> +	if (!cxlmd || !dev_is_pci(cxlmd->dev.parent)) {
> +		dev_dbg(&cxld->dev, "No endpoint found: %s, range %#llx-%#llx\n",
> +			dev_name(cxld->dev.parent), hpa_range.start,
> +			hpa_range.end);
> +		return -ENXIO;
> +	}
> +
> +	pci_dev = to_pci_dev(cxlmd->dev.parent);
> +
> +	/* Translate HPA range to SPA. */
> +	hpa_range.start = base_spa = prm_cxl_dpa_spa(pci_dev, hpa_range.start);
> +	hpa_range.end = prm_cxl_dpa_spa(pci_dev, hpa_range.end);
> +
> +	if (hpa_range.start == ULLONG_MAX || hpa_range.end == ULLONG_MAX) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Failed to translate HPA range: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	/*
> +	 * Since translated addresses include the interleaving
> +	 * offsets, align the range to 256 MB.
> +	 */
> +	hpa_range.start = ALIGN_DOWN(hpa_range.start, SZ_256M);
> +	hpa_range.end = ALIGN(hpa_range.end, SZ_256M) - 1;
> +
> +	spa_len = range_len(&hpa_range);
> +	if (!len || !spa_len || spa_len % len) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: HPA range not contiguous: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	ways = spa_len / len;
> +	gran = SZ_256;
> +
> +	/*
> +	 * Determine interleave granularity
> +	 *
> +	 * Note: The position of the chunk from one interleaving block
> +	 * to the next may vary and thus cannot be considered
> +	 * constant. Address offsets larger than the interleaving
> +	 * block size cannot be used to calculate the granularity.

Wrap looks short.

> +	 */
> +	while (ways > 1 && gran <= SZ_16M) {

As ways isn't modified in here, I think it would clearer as
	if (ways > 1) {
		while (gran < SZ_16M) {
			addr = prm_cxl_dpa_spa(pci_dev, base + gran);
			if (addr != base_spa + gran)
				break;
			gran <<= 1;
		}
	}
> +		addr = prm_cxl_dpa_spa(pci_dev, base + gran);
> +		if (addr != base_spa + gran)
> +			break;
> +		gran <<= 1;
> +	}
> +
> +	if (gran > SZ_16M) {
> +		dev_dbg(cxld->dev.parent,
> +			"CXL address translation: Cannot determine granularity: %#llx-%#llx:%#llx-%#llx(%s)\n",
> +			hpa_range.start, hpa_range.end, ctx->hpa_range.start,
> +			ctx->hpa_range.end, dev_name(&cxld->dev));
> +		return -ENXIO;
> +	}
> +
> +	ctx->hpa_range = hpa_range;
> +	ctx->interleave_ways = ways;
> +	ctx->interleave_granularity = gran;
> +
> +	dev_dbg(&cxld->dev,
> +		"address mapping found for %s (hpa -> spa): %#llx+%#llx -> %#llx+%#llx ways:%d granularity:%d\n",
> +		dev_name(ctx->cxlmd->dev.parent), base, len, hpa_range.start,
> +		spa_len, ways, gran);
> +
> +	return 0;
> +}
> +
> +void cxl_setup_prm_address_translation(struct cxl_root *cxl_root)
> +{
> +	struct device *host = cxl_root->port.uport_dev;
> +	u64 spa;
> +	struct prm_cxl_dpa_spa_data data = { .spa = &spa, };

Trailing comma is a tiny bit pointless as any change will have to replace
the line anyway.  So I'd drop it. (trivial so ignore if you like ;)

> +	int rc;

Re: [PATCH v4 10/14] cxl: Enable AMD Zen5 address translation using ACPI PRMT

[Robert Richter]

On 11.11.25 15:30:49, Jonathan Cameron wrote:
> On Mon, 3 Nov 2025 19:47:51 +0100
> Robert Richter <rrichter@amd.com> wrote:

> > +void cxl_setup_prm_address_translation(struct cxl_root *cxl_root)
> > +{
> > +	struct device *host = cxl_root->port.uport_dev;
> > +	u64 spa;
> > +	struct prm_cxl_dpa_spa_data data = { .spa = &spa, };
> 
> Trailing comma is a tiny bit pointless as any change will have to replace
> the line anyway.  So I'd drop it. (trivial so ignore if you like ;)

I have chosen the MIT comma here, but dropped it in v5. Are you
Cambridge or Oxford? ;)

 https://xkcd.com/2995/

Thanks for your reviews.

-Robert

[PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Robert Richter]

There is only support to translate addresses from an endpoint to its
CXL host bridge, but not in the opposite direction from the bridge to
the endpoint. Thus, the endpoint address range cannot be determined
and setup manually for a given SPA range of a region. If the endpoint
has address translation enabled, lock it to prevent the kernel from
reconfiguring it.

Reviewed-by: Gregory Price <gourry@gourry.net>
Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/atl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
index d6aa7e6d0ac5..5c15e4d12193 100644
--- a/drivers/cxl/core/atl.c
+++ b/drivers/cxl/core/atl.c
@@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
 		return -ENXIO;
 	}
 
+	/*
+	 * There is only support to translate from the endpoint to its
+	 * parent port, but not in the opposite direction from the
+	 * parent to the endpoint. Thus, the endpoint address range
+	 * cannot be determined and setup manually. If the address range
+	 * was translated and modified, forbid reprogramming of the
+	 * decoders and lock them.
+	 */
+	cxld->flags |= CXL_DECODER_F_LOCK;
+
 	ctx->hpa_range = hpa_range;
 	ctx->interleave_ways = ways;
 	ctx->interleave_granularity = gran;
-- 
2.47.3

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> There is only support to translate addresses from an endpoint to its
> CXL host bridge, but not in the opposite direction from the bridge to
> the endpoint. Thus, the endpoint address range cannot be determined
> and setup manually for a given SPA range of a region. If the endpoint
> has address translation enabled, lock it to prevent the kernel from
> reconfiguring it.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/core/atl.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> index d6aa7e6d0ac5..5c15e4d12193 100644
> --- a/drivers/cxl/core/atl.c
> +++ b/drivers/cxl/core/atl.c
> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
>  		return -ENXIO;
>  	}
>  
> +	/*
> +	 * There is only support to translate from the endpoint to its
> +	 * parent port, but not in the opposite direction from the
> +	 * parent to the endpoint. Thus, the endpoint address range
> +	 * cannot be determined and setup manually. If the address range
> +	 * was translated and modified, forbid reprogramming of the
> +	 * decoders and lock them.
> +	 */
> +	cxld->flags |= CXL_DECODER_F_LOCK;

Feels like this should be something the BIOS should enforce if that is the expectation? And the kernel checks and warns if that is not the case.


> +
>  	ctx->hpa_range = hpa_range;
>  	ctx->interleave_ways = ways;
>  	ctx->interleave_granularity = gran;

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Robert Richter]

On 04.11.25 10:13:34, Dave Jiang wrote:
> 
> 
> On 11/3/25 11:47 AM, Robert Richter wrote:
> > There is only support to translate addresses from an endpoint to its
> > CXL host bridge, but not in the opposite direction from the bridge to
> > the endpoint. Thus, the endpoint address range cannot be determined
> > and setup manually for a given SPA range of a region. If the endpoint
> > has address translation enabled, lock it to prevent the kernel from
> > reconfiguring it.
> > 
> > Reviewed-by: Gregory Price <gourry@gourry.net>
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> > ---
> >  drivers/cxl/core/atl.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> > index d6aa7e6d0ac5..5c15e4d12193 100644
> > --- a/drivers/cxl/core/atl.c
> > +++ b/drivers/cxl/core/atl.c
> > @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> >  		return -ENXIO;
> >  	}
> >  
> > +	/*
> > +	 * There is only support to translate from the endpoint to its
> > +	 * parent port, but not in the opposite direction from the
> > +	 * parent to the endpoint. Thus, the endpoint address range
> > +	 * cannot be determined and setup manually. If the address range
> > +	 * was translated and modified, forbid reprogramming of the
> > +	 * decoders and lock them.
> > +	 */
> > +	cxld->flags |= CXL_DECODER_F_LOCK;
>
 
> Feels like this should be something the BIOS should enforce if that
> is the expectation? And the kernel checks and warns if that is not
> the case.

I think this is more a limitation of the kernel implementation rather
than the BIOS. The BIOS provides enought information by CFMWS, PRM,
HDM and PCI topology. In theory and if there is demand for it, support
could be added for driver region setup.

-Robert

> 
> > +
> >  	ctx->hpa_range = hpa_range;
> >  	ctx->interleave_ways = ways;
> >  	ctx->interleave_granularity = gran;
> 
?

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Dave Jiang]

On 11/11/25 5:54 AM, Robert Richter wrote:
> On 04.11.25 10:13:34, Dave Jiang wrote:
>>
>>
>> On 11/3/25 11:47 AM, Robert Richter wrote:
>>> There is only support to translate addresses from an endpoint to its
>>> CXL host bridge, but not in the opposite direction from the bridge to
>>> the endpoint. Thus, the endpoint address range cannot be determined
>>> and setup manually for a given SPA range of a region. If the endpoint
>>> has address translation enabled, lock it to prevent the kernel from
>>> reconfiguring it.
>>>
>>> Reviewed-by: Gregory Price <gourry@gourry.net>
>>> Signed-off-by: Robert Richter <rrichter@amd.com>
>>> ---
>>>  drivers/cxl/core/atl.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
>>> index d6aa7e6d0ac5..5c15e4d12193 100644
>>> --- a/drivers/cxl/core/atl.c
>>> +++ b/drivers/cxl/core/atl.c
>>> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
>>>  		return -ENXIO;
>>>  	}
>>>  
>>> +	/*
>>> +	 * There is only support to translate from the endpoint to its
>>> +	 * parent port, but not in the opposite direction from the
>>> +	 * parent to the endpoint. Thus, the endpoint address range
>>> +	 * cannot be determined and setup manually. If the address range
>>> +	 * was translated and modified, forbid reprogramming of the
>>> +	 * decoders and lock them.
>>> +	 */
>>> +	cxld->flags |= CXL_DECODER_F_LOCK;
>>
>  
>> Feels like this should be something the BIOS should enforce if that
>> is the expectation? And the kernel checks and warns if that is not
>> the case.
> 
> I think this is more a limitation of the kernel implementation rather
> than the BIOS. The BIOS provides enought information by CFMWS, PRM,
> HDM and PCI topology. In theory and if there is demand for it, support
> could be added for driver region setup.

But shouldn't the BIOS set the decoder lock rather than the kernel setting a software lock flag based on assumption of the PRM based setup?

DJ

> 
> -Robert
> 
>>
>>> +
>>>  	ctx->hpa_range = hpa_range;
>>>  	ctx->interleave_ways = ways;
>>>  	ctx->interleave_granularity = gran;
>>
> ?

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Robert Richter]

On 12.11.25 09:34:34, Dave Jiang wrote:
> 
> 
> On 11/11/25 5:54 AM, Robert Richter wrote:
> > On 04.11.25 10:13:34, Dave Jiang wrote:
> >>
> >>
> >> On 11/3/25 11:47 AM, Robert Richter wrote:
> >>> There is only support to translate addresses from an endpoint to its
> >>> CXL host bridge, but not in the opposite direction from the bridge to
> >>> the endpoint. Thus, the endpoint address range cannot be determined
> >>> and setup manually for a given SPA range of a region. If the endpoint
> >>> has address translation enabled, lock it to prevent the kernel from
> >>> reconfiguring it.
> >>>
> >>> Reviewed-by: Gregory Price <gourry@gourry.net>
> >>> Signed-off-by: Robert Richter <rrichter@amd.com>
> >>> ---
> >>>  drivers/cxl/core/atl.c | 10 ++++++++++
> >>>  1 file changed, 10 insertions(+)
> >>>
> >>> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> >>> index d6aa7e6d0ac5..5c15e4d12193 100644
> >>> --- a/drivers/cxl/core/atl.c
> >>> +++ b/drivers/cxl/core/atl.c
> >>> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> >>>  		return -ENXIO;
> >>>  	}
> >>>  
> >>> +	/*
> >>> +	 * There is only support to translate from the endpoint to its
> >>> +	 * parent port, but not in the opposite direction from the
> >>> +	 * parent to the endpoint. Thus, the endpoint address range
> >>> +	 * cannot be determined and setup manually. If the address range
> >>> +	 * was translated and modified, forbid reprogramming of the
> >>> +	 * decoders and lock them.
> >>> +	 */
> >>> +	cxld->flags |= CXL_DECODER_F_LOCK;
> >>
> >  
> >> Feels like this should be something the BIOS should enforce if that
> >> is the expectation? And the kernel checks and warns if that is not
> >> the case.
> > 
> > I think this is more a limitation of the kernel implementation rather
> > than the BIOS. The BIOS provides enought information by CFMWS, PRM,
> > HDM and PCI topology. In theory and if there is demand for it, support
> > could be added for driver region setup.
> 

> But shouldn't the BIOS set the decoder lock rather than the kernel
> setting a software lock flag based on assumption of the PRM based
> setup?

If BIOS locks the decoders, it cannot be removed even for the case
there the OS can actually handle it.

-Robert

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Dave Jiang]

On 11/13/25 1:05 PM, Robert Richter wrote:
> On 12.11.25 09:34:34, Dave Jiang wrote:
>>
>>
>> On 11/11/25 5:54 AM, Robert Richter wrote:
>>> On 04.11.25 10:13:34, Dave Jiang wrote:
>>>>
>>>>
>>>> On 11/3/25 11:47 AM, Robert Richter wrote:
>>>>> There is only support to translate addresses from an endpoint to its
>>>>> CXL host bridge, but not in the opposite direction from the bridge to
>>>>> the endpoint. Thus, the endpoint address range cannot be determined
>>>>> and setup manually for a given SPA range of a region. If the endpoint
>>>>> has address translation enabled, lock it to prevent the kernel from
>>>>> reconfiguring it.
>>>>>
>>>>> Reviewed-by: Gregory Price <gourry@gourry.net>
>>>>> Signed-off-by: Robert Richter <rrichter@amd.com>
>>>>> ---
>>>>>  drivers/cxl/core/atl.c | 10 ++++++++++
>>>>>  1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
>>>>> index d6aa7e6d0ac5..5c15e4d12193 100644
>>>>> --- a/drivers/cxl/core/atl.c
>>>>> +++ b/drivers/cxl/core/atl.c
>>>>> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
>>>>>  		return -ENXIO;
>>>>>  	}
>>>>>  
>>>>> +	/*
>>>>> +	 * There is only support to translate from the endpoint to its
>>>>> +	 * parent port, but not in the opposite direction from the
>>>>> +	 * parent to the endpoint. Thus, the endpoint address range
>>>>> +	 * cannot be determined and setup manually. If the address range
>>>>> +	 * was translated and modified, forbid reprogramming of the
>>>>> +	 * decoders and lock them.
>>>>> +	 */
>>>>> +	cxld->flags |= CXL_DECODER_F_LOCK;
>>>>
>>>  
>>>> Feels like this should be something the BIOS should enforce if that
>>>> is the expectation? And the kernel checks and warns if that is not
>>>> the case.
>>>
>>> I think this is more a limitation of the kernel implementation rather
>>> than the BIOS. The BIOS provides enought information by CFMWS, PRM,
>>> HDM and PCI topology. In theory and if there is demand for it, support
>>> could be added for driver region setup.
>>
> 
>> But shouldn't the BIOS set the decoder lock rather than the kernel
>> setting a software lock flag based on assumption of the PRM based
>> setup?
> 
> If BIOS locks the decoders, it cannot be removed even for the case
> there the OS can actually handle it.

Oh so the current implementation is auto region by BIOS but in the future it may not be? But if you add a lock flag, you wouldn't be able to remove it later anyhow since it's presented as locked?


> 
> -Robert

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Robert Richter]

On Thu, Nov 13, 2025 at 01:36:26PM -0700, Dave Jiang wrote:
> 
> 
> On 11/13/25 1:05 PM, Robert Richter wrote:
> > On 12.11.25 09:34:34, Dave Jiang wrote:
> >>
> >>
> >> On 11/11/25 5:54 AM, Robert Richter wrote:
> >>> On 04.11.25 10:13:34, Dave Jiang wrote:
> >>>>
> >>>>
> >>>> On 11/3/25 11:47 AM, Robert Richter wrote:
> >>>>> There is only support to translate addresses from an endpoint to its
> >>>>> CXL host bridge, but not in the opposite direction from the bridge to
> >>>>> the endpoint. Thus, the endpoint address range cannot be determined
> >>>>> and setup manually for a given SPA range of a region. If the endpoint
> >>>>> has address translation enabled, lock it to prevent the kernel from
> >>>>> reconfiguring it.
> >>>>>
> >>>>> Reviewed-by: Gregory Price <gourry@gourry.net>
> >>>>> Signed-off-by: Robert Richter <rrichter@amd.com>
> >>>>> ---
> >>>>>  drivers/cxl/core/atl.c | 10 ++++++++++
> >>>>>  1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
> >>>>> index d6aa7e6d0ac5..5c15e4d12193 100644
> >>>>> --- a/drivers/cxl/core/atl.c
> >>>>> +++ b/drivers/cxl/core/atl.c
> >>>>> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
> >>>>>  		return -ENXIO;
> >>>>>  	}
> >>>>>  
> >>>>> +	/*
> >>>>> +	 * There is only support to translate from the endpoint to its
> >>>>> +	 * parent port, but not in the opposite direction from the
> >>>>> +	 * parent to the endpoint. Thus, the endpoint address range
> >>>>> +	 * cannot be determined and setup manually. If the address range
> >>>>> +	 * was translated and modified, forbid reprogramming of the
> >>>>> +	 * decoders and lock them.
> >>>>> +	 */
> >>>>> +	cxld->flags |= CXL_DECODER_F_LOCK;
> >>>>
> >>>  
> >>>> Feels like this should be something the BIOS should enforce if that
> >>>> is the expectation? And the kernel checks and warns if that is not
> >>>> the case.
> >>>
> >>> I think this is more a limitation of the kernel implementation rather
> >>> than the BIOS. The BIOS provides enought information by CFMWS, PRM,
> >>> HDM and PCI topology. In theory and if there is demand for it, support
> >>> could be added for driver region setup.
> >>
> > 
> >> But shouldn't the BIOS set the decoder lock rather than the kernel
> >> setting a software lock flag based on assumption of the PRM based
> >> setup?
> > 
> > If BIOS locks the decoders, it cannot be removed even for the case
> > there the OS can actually handle it.
> 

> Oh so the current implementation is auto region by BIOS but in the
> future it may not be? But if you add a lock flag, you wouldn't be
> able to remove it later anyhow since it's presented as locked?

The BIOS provides all necessary data for address translation, so that
decoders can be reconfigured (including normalized endpoint
addresses). There is no reason to lock the decoders by the BIOS, as
otherwise, with a capable kernel (or other OS), it would not be
possible to shutdown auto-generated regions.

However, current kernel implementation does not support this and is
unable to create the region. That is why the kernel and not the BIOS
should lock the decoders.

-Robert

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Dave Jiang]

On 11/14/25 12:34 AM, Robert Richter wrote:
> On Thu, Nov 13, 2025 at 01:36:26PM -0700, Dave Jiang wrote:
>>
>>
>> On 11/13/25 1:05 PM, Robert Richter wrote:
>>> On 12.11.25 09:34:34, Dave Jiang wrote:
>>>>
>>>>
>>>> On 11/11/25 5:54 AM, Robert Richter wrote:
>>>>> On 04.11.25 10:13:34, Dave Jiang wrote:
>>>>>>
>>>>>>
>>>>>> On 11/3/25 11:47 AM, Robert Richter wrote:
>>>>>>> There is only support to translate addresses from an endpoint to its
>>>>>>> CXL host bridge, but not in the opposite direction from the bridge to
>>>>>>> the endpoint. Thus, the endpoint address range cannot be determined
>>>>>>> and setup manually for a given SPA range of a region. If the endpoint
>>>>>>> has address translation enabled, lock it to prevent the kernel from
>>>>>>> reconfiguring it.
>>>>>>>
>>>>>>> Reviewed-by: Gregory Price <gourry@gourry.net>
>>>>>>> Signed-off-by: Robert Richter <rrichter@amd.com>
>>>>>>> ---
>>>>>>>  drivers/cxl/core/atl.c | 10 ++++++++++
>>>>>>>  1 file changed, 10 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/cxl/core/atl.c b/drivers/cxl/core/atl.c
>>>>>>> index d6aa7e6d0ac5..5c15e4d12193 100644
>>>>>>> --- a/drivers/cxl/core/atl.c
>>>>>>> +++ b/drivers/cxl/core/atl.c
>>>>>>> @@ -158,6 +158,16 @@ static int cxl_prm_translate_hpa_range(struct cxl_root *cxl_root, void *data)
>>>>>>>  		return -ENXIO;
>>>>>>>  	}
>>>>>>>  
>>>>>>> +	/*
>>>>>>> +	 * There is only support to translate from the endpoint to its
>>>>>>> +	 * parent port, but not in the opposite direction from the
>>>>>>> +	 * parent to the endpoint. Thus, the endpoint address range
>>>>>>> +	 * cannot be determined and setup manually. If the address range
>>>>>>> +	 * was translated and modified, forbid reprogramming of the
>>>>>>> +	 * decoders and lock them.
>>>>>>> +	 */
>>>>>>> +	cxld->flags |= CXL_DECODER_F_LOCK;
>>>>>>
>>>>>  
>>>>>> Feels like this should be something the BIOS should enforce if that
>>>>>> is the expectation? And the kernel checks and warns if that is not
>>>>>> the case.
>>>>>
>>>>> I think this is more a limitation of the kernel implementation rather
>>>>> than the BIOS. The BIOS provides enought information by CFMWS, PRM,
>>>>> HDM and PCI topology. In theory and if there is demand for it, support
>>>>> could be added for driver region setup.
>>>>
>>>
>>>> But shouldn't the BIOS set the decoder lock rather than the kernel
>>>> setting a software lock flag based on assumption of the PRM based
>>>> setup?
>>>
>>> If BIOS locks the decoders, it cannot be removed even for the case
>>> there the OS can actually handle it.
>>
> 
>> Oh so the current implementation is auto region by BIOS but in the
>> future it may not be? But if you add a lock flag, you wouldn't be
>> able to remove it later anyhow since it's presented as locked?
> 
> The BIOS provides all necessary data for address translation, so that
> decoders can be reconfigured (including normalized endpoint
> addresses). There is no reason to lock the decoders by the BIOS, as
> otherwise, with a capable kernel (or other OS), it would not be
> possible to shutdown auto-generated regions.
> 
> However, current kernel implementation does not support this and is
> unable to create the region. That is why the kernel and not the BIOS
> should lock the decoders.

Ok I see what you are saying. Fair enough. As long as we have a comment that makes note of this detail.

> 
> -Robert

Re: [PATCH v4 11/14] cxl/atl: Lock decoders that need address translation

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:52 +0100
Robert Richter <rrichter@amd.com> wrote:

> There is only support to translate addresses from an endpoint to its
> CXL host bridge, but not in the opposite direction from the bridge to
> the endpoint. Thus, the endpoint address range cannot be determined
> and setup manually for a given SPA range of a region. If the endpoint
> has address translation enabled, lock it to prevent the kernel from
> reconfiguring it.
> 
> Reviewed-by: Gregory Price <gourry@gourry.net>
> Signed-off-by: Robert Richter <rrichter@amd.com>
I'm fine with your explanation to Dave so
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH v4 12/14] cxl: Simplify cxl_rd_ops allocation and handling

[Robert Richter]

A root decoder's callback handlers are collected in struct cxl_rd_ops.
The structure is dynamically allocated, though it contains only a few
pointers in it. This also requires to check two pointes to check for
the existance of a callback.

Simplify the allocation, release and handler check by embedding the
ops statical in struct cxl_root_decoder.

Implementation is equivalent to how struct cxl_root_ops handles the
callbacks.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/acpi.c        |  8 ++------
 drivers/cxl/core/region.c | 20 +++++---------------
 drivers/cxl/cxl.h         |  2 +-
 3 files changed, 8 insertions(+), 22 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index f9bbc77f3ec2..778ee29430ea 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -471,12 +471,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
 	cxlrd->qos_class = cfmws->qtg_id;
 
 	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
-		cxlrd->ops = kzalloc(sizeof(*cxlrd->ops), GFP_KERNEL);
-		if (!cxlrd->ops)
-			return -ENOMEM;
-
-		cxlrd->ops->hpa_to_spa = cxl_apply_xor_maps;
-		cxlrd->ops->spa_to_hpa = cxl_apply_xor_maps;
+		cxlrd->ops.hpa_to_spa = cxl_apply_xor_maps;
+		cxlrd->ops.spa_to_hpa = cxl_apply_xor_maps;
 	}
 
 	rc = cxl_decoder_add(cxld);
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 379a67cc8e31..dec003084521 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2932,16 +2932,6 @@ static bool cxl_is_hpa_in_chunk(u64 hpa, struct cxl_region *cxlr, int pos)
 	return false;
 }
 
-static bool has_hpa_to_spa(struct cxl_root_decoder *cxlrd)
-{
-	return cxlrd->ops && cxlrd->ops->hpa_to_spa;
-}
-
-static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
-{
-	return cxlrd->ops && cxlrd->ops->spa_to_hpa;
-}
-
 u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 		   u64 dpa)
 {
@@ -2996,8 +2986,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 	hpa = hpa_offset + p->res->start + p->cache_size;
 
 	/* Root decoder translation overrides typical modulo decode */
-	if (has_hpa_to_spa(cxlrd))
-		hpa = cxlrd->ops->hpa_to_spa(cxlrd, hpa);
+	if (cxlrd->ops.hpa_to_spa)
+		hpa = cxlrd->ops.hpa_to_spa(cxlrd, hpa);
 
 	if (!cxl_resource_contains_addr(p->res, hpa)) {
 		dev_dbg(&cxlr->dev,
@@ -3006,7 +2996,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 	}
 
 	/* Simple chunk check, by pos & gran, only applies to modulo decodes */
-	if (!has_hpa_to_spa(cxlrd) && (!cxl_is_hpa_in_chunk(hpa, cxlr, pos)))
+	if (!cxlrd->ops.hpa_to_spa && !cxl_is_hpa_in_chunk(hpa, cxlr, pos))
 		return ULLONG_MAX;
 
 	return hpa;
@@ -3041,8 +3031,8 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
 	 * If the root decoder has SPA to CXL HPA callback, use it. Otherwise
 	 * CXL HPA is assumed to equal SPA.
 	 */
-	if (has_spa_to_hpa(cxlrd)) {
-		hpa = cxlrd->ops->spa_to_hpa(cxlrd, p->res->start + offset);
+	if (cxlrd->ops.spa_to_hpa) {
+		hpa = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
 		hpa_offset = hpa - p->res->start;
 	} else {
 		hpa_offset = offset;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 0af46d1b0abc..75fd45ddca38 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -451,7 +451,7 @@ struct cxl_root_decoder {
 	void *platform_data;
 	struct mutex range_lock;
 	int qos_class;
-	struct cxl_rd_ops *ops;
+	struct cxl_rd_ops ops;
 	struct cxl_switch_decoder cxlsd;
 };
 
-- 
2.47.3

Re: [PATCH v4 12/14] cxl: Simplify cxl_rd_ops allocation and handling

[Dave Jiang]

On 11/3/25 11:47 AM, Robert Richter wrote:
> A root decoder's callback handlers are collected in struct cxl_rd_ops.
> The structure is dynamically allocated, though it contains only a few
> pointers in it. This also requires to check two pointes to check for
> the existance of a callback.
> 
> Simplify the allocation, release and handler check by embedding the
> ops statical in struct cxl_root_decoder.
> 
> Implementation is equivalent to how struct cxl_root_ops handles the
> callbacks.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>

Reviewed-by: Dave Jiang <dave.jiang@intel.com>

I think this can be split out send ahead. It's not tied to this series right?

> ---
>  drivers/cxl/acpi.c        |  8 ++------
>  drivers/cxl/core/region.c | 20 +++++---------------
>  drivers/cxl/cxl.h         |  2 +-
>  3 files changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index f9bbc77f3ec2..778ee29430ea 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -471,12 +471,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>  	cxlrd->qos_class = cfmws->qtg_id;
>  
>  	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
> -		cxlrd->ops = kzalloc(sizeof(*cxlrd->ops), GFP_KERNEL);
> -		if (!cxlrd->ops)
> -			return -ENOMEM;
> -
> -		cxlrd->ops->hpa_to_spa = cxl_apply_xor_maps;
> -		cxlrd->ops->spa_to_hpa = cxl_apply_xor_maps;
> +		cxlrd->ops.hpa_to_spa = cxl_apply_xor_maps;
> +		cxlrd->ops.spa_to_hpa = cxl_apply_xor_maps;
>  	}
>  
>  	rc = cxl_decoder_add(cxld);
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 379a67cc8e31..dec003084521 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2932,16 +2932,6 @@ static bool cxl_is_hpa_in_chunk(u64 hpa, struct cxl_region *cxlr, int pos)
>  	return false;
>  }
>  
> -static bool has_hpa_to_spa(struct cxl_root_decoder *cxlrd)
> -{
> -	return cxlrd->ops && cxlrd->ops->hpa_to_spa;
> -}
> -
> -static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
> -{
> -	return cxlrd->ops && cxlrd->ops->spa_to_hpa;
> -}
> -
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa)
>  {
> @@ -2996,8 +2986,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	hpa = hpa_offset + p->res->start + p->cache_size;
>  
>  	/* Root decoder translation overrides typical modulo decode */
> -	if (has_hpa_to_spa(cxlrd))
> -		hpa = cxlrd->ops->hpa_to_spa(cxlrd, hpa);
> +	if (cxlrd->ops.hpa_to_spa)
> +		hpa = cxlrd->ops.hpa_to_spa(cxlrd, hpa);
>  
>  	if (!cxl_resource_contains_addr(p->res, hpa)) {
>  		dev_dbg(&cxlr->dev,
> @@ -3006,7 +2996,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	}
>  
>  	/* Simple chunk check, by pos & gran, only applies to modulo decodes */
> -	if (!has_hpa_to_spa(cxlrd) && (!cxl_is_hpa_in_chunk(hpa, cxlr, pos)))
> +	if (!cxlrd->ops.hpa_to_spa && !cxl_is_hpa_in_chunk(hpa, cxlr, pos))
>  		return ULLONG_MAX;
>  
>  	return hpa;
> @@ -3041,8 +3031,8 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	 * If the root decoder has SPA to CXL HPA callback, use it. Otherwise
>  	 * CXL HPA is assumed to equal SPA.
>  	 */
> -	if (has_spa_to_hpa(cxlrd)) {
> -		hpa = cxlrd->ops->spa_to_hpa(cxlrd, p->res->start + offset);
> +	if (cxlrd->ops.spa_to_hpa) {
> +		hpa = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
>  		hpa_offset = hpa - p->res->start;
>  	} else {
>  		hpa_offset = offset;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 0af46d1b0abc..75fd45ddca38 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -451,7 +451,7 @@ struct cxl_root_decoder {
>  	void *platform_data;
>  	struct mutex range_lock;
>  	int qos_class;
> -	struct cxl_rd_ops *ops;
> +	struct cxl_rd_ops ops;
>  	struct cxl_switch_decoder cxlsd;
>  };
>  

Re: [PATCH v4 12/14] cxl: Simplify cxl_rd_ops allocation and handling

[Alison Schofield]

On Mon, Nov 03, 2025 at 07:47:53PM +0100, Robert Richter wrote:
> A root decoder's callback handlers are collected in struct cxl_rd_ops.
> The structure is dynamically allocated, though it contains only a few
> pointers in it. This also requires to check two pointes to check for
> the existance of a callback.
> 
> Simplify the allocation, release and handler check by embedding the
> ops statical in struct cxl_root_decoder.
> 
> Implementation is equivalent to how struct cxl_root_ops handles the
> callbacks.

The allocation was intentionally dynamic because the root decoder ops
only existed for CFMWS's defined w XOR Arithmetic.

From the commit msg:
>> To avoid maintaining a static ops instance populated with mostly NULL
>> pointers, allocate the ops structure dynamically only when a platform
>> requires overrides (e.g. XOR interleave decoding).
>> The setup can be extended as additional callbacks are added.
See: 524b2b76f365 ("cxl: Move hpa_to_spa callback to a new root decoder ops structure")

Has the usage changed?

-- Alison


> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
> ---
>  drivers/cxl/acpi.c        |  8 ++------
>  drivers/cxl/core/region.c | 20 +++++---------------
>  drivers/cxl/cxl.h         |  2 +-
>  3 files changed, 8 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index f9bbc77f3ec2..778ee29430ea 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -471,12 +471,8 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>  	cxlrd->qos_class = cfmws->qtg_id;
>  
>  	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
> -		cxlrd->ops = kzalloc(sizeof(*cxlrd->ops), GFP_KERNEL);
> -		if (!cxlrd->ops)
> -			return -ENOMEM;
> -
> -		cxlrd->ops->hpa_to_spa = cxl_apply_xor_maps;
> -		cxlrd->ops->spa_to_hpa = cxl_apply_xor_maps;
> +		cxlrd->ops.hpa_to_spa = cxl_apply_xor_maps;
> +		cxlrd->ops.spa_to_hpa = cxl_apply_xor_maps;
>  	}
>  
>  	rc = cxl_decoder_add(cxld);
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 379a67cc8e31..dec003084521 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2932,16 +2932,6 @@ static bool cxl_is_hpa_in_chunk(u64 hpa, struct cxl_region *cxlr, int pos)
>  	return false;
>  }
>  
> -static bool has_hpa_to_spa(struct cxl_root_decoder *cxlrd)
> -{
> -	return cxlrd->ops && cxlrd->ops->hpa_to_spa;
> -}
> -
> -static bool has_spa_to_hpa(struct cxl_root_decoder *cxlrd)
> -{
> -	return cxlrd->ops && cxlrd->ops->spa_to_hpa;
> -}
> -
>  u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  		   u64 dpa)
>  {
> @@ -2996,8 +2986,8 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	hpa = hpa_offset + p->res->start + p->cache_size;
>  
>  	/* Root decoder translation overrides typical modulo decode */
> -	if (has_hpa_to_spa(cxlrd))
> -		hpa = cxlrd->ops->hpa_to_spa(cxlrd, hpa);
> +	if (cxlrd->ops.hpa_to_spa)
> +		hpa = cxlrd->ops.hpa_to_spa(cxlrd, hpa);
>  
>  	if (!cxl_resource_contains_addr(p->res, hpa)) {
>  		dev_dbg(&cxlr->dev,
> @@ -3006,7 +2996,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	}
>  
>  	/* Simple chunk check, by pos & gran, only applies to modulo decodes */
> -	if (!has_hpa_to_spa(cxlrd) && (!cxl_is_hpa_in_chunk(hpa, cxlr, pos)))
> +	if (!cxlrd->ops.hpa_to_spa && !cxl_is_hpa_in_chunk(hpa, cxlr, pos))
>  		return ULLONG_MAX;
>  
>  	return hpa;
> @@ -3041,8 +3031,8 @@ static int region_offset_to_dpa_result(struct cxl_region *cxlr, u64 offset,
>  	 * If the root decoder has SPA to CXL HPA callback, use it. Otherwise
>  	 * CXL HPA is assumed to equal SPA.
>  	 */
> -	if (has_spa_to_hpa(cxlrd)) {
> -		hpa = cxlrd->ops->spa_to_hpa(cxlrd, p->res->start + offset);
> +	if (cxlrd->ops.spa_to_hpa) {
> +		hpa = cxlrd->ops.spa_to_hpa(cxlrd, p->res->start + offset);
>  		hpa_offset = hpa - p->res->start;
>  	} else {
>  		hpa_offset = offset;
> diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
> index 0af46d1b0abc..75fd45ddca38 100644
> --- a/drivers/cxl/cxl.h
> +++ b/drivers/cxl/cxl.h
> @@ -451,7 +451,7 @@ struct cxl_root_decoder {
>  	void *platform_data;
>  	struct mutex range_lock;
>  	int qos_class;
> -	struct cxl_rd_ops *ops;
> +	struct cxl_rd_ops ops;
>  	struct cxl_switch_decoder cxlsd;
>  };
>  
> -- 
> 2.47.3
> 

Re: [PATCH v4 12/14] cxl: Simplify cxl_rd_ops allocation and handling

[Robert Richter]

Alison,

On 04.11.25 15:02:19, Alison Schofield wrote:
> On Mon, Nov 03, 2025 at 07:47:53PM +0100, Robert Richter wrote:
> > A root decoder's callback handlers are collected in struct cxl_rd_ops.
> > The structure is dynamically allocated, though it contains only a few
> > pointers in it. This also requires to check two pointes to check for
> > the existance of a callback.
> > 
> > Simplify the allocation, release and handler check by embedding the
> > ops statical in struct cxl_root_decoder.
> > 
> > Implementation is equivalent to how struct cxl_root_ops handles the
> > callbacks.
> 
> The allocation was intentionally dynamic because the root decoder ops
> only existed for CFMWS's defined w XOR Arithmetic.
> 
> From the commit msg:
> >> To avoid maintaining a static ops instance populated with mostly NULL
> >> pointers, allocate the ops structure dynamically only when a platform
> >> requires overrides (e.g. XOR interleave decoding).
> >> The setup can be extended as additional callbacks are added.
> See: 524b2b76f365 ("cxl: Move hpa_to_spa callback to a new root decoder ops structure")
> 
> Has the usage changed?

No, code did not change. In another patch I had to change struct
cxl_root and the cxl_root_ops allocation there. The pattern here is
the same and the code would benefit from the same change to simplify
it.

Dynamic allocation does not save much here (only a single pointer
entry) but requires resource allocation and a 2-level pointer check
which must be run even if the pointers are NULL. If there is at least
one pointer set (which becomes more likely with an increased number of
ops) you need to allocate the whole struct anyway and dynamic alloc
does net help then. The diffstat also shows the simplification:

 3 files changed, 8 insertions(+), 22 deletions(-)

-Robert

Re: [PATCH v4 12/14] cxl: Simplify cxl_rd_ops allocation and handling

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:53 +0100
Robert Richter <rrichter@amd.com> wrote:

> A root decoder's callback handlers are collected in struct cxl_rd_ops.
> The structure is dynamically allocated, though it contains only a few
> pointers in it. This also requires to check two pointes to check for
> the existance of a callback.
> 
> Simplify the allocation, release and handler check by embedding the
> ops statical in struct cxl_root_decoder.
> 
> Implementation is equivalent to how struct cxl_root_ops handles the
> callbacks.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
Given we aren't picking between big sets of static const ops but
instead just 2 (in which case the indirection would be a good idea),
I'm fine with this as a simplication over dynamic allocation.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

[PATCH v4 13/14] cxl/acpi: Group xor arithmetric setup code in a single block

[Robert Richter]

Simplify the xor arithmetric setup code by grouping it in a single
block. No need to split the block for QoS setup.

It is save to reorder the call of cxl_setup_extended_linear_cache()
because there are no dependencies.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/acpi.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 778ee29430ea..40894e2156ce 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -449,8 +449,6 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
 		ig = CXL_DECODER_MIN_GRANULARITY;
 	cxld->interleave_granularity = ig;
 
-	cxl_setup_extended_linear_cache(cxlrd);
-
 	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
 		if (ways != 1 && ways != 3) {
 			cxims_ctx = (struct cxl_cxims_context) {
@@ -466,15 +464,14 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
 				return -EINVAL;
 			}
 		}
-	}
-
-	cxlrd->qos_class = cfmws->qtg_id;
-
-	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
 		cxlrd->ops.hpa_to_spa = cxl_apply_xor_maps;
 		cxlrd->ops.spa_to_hpa = cxl_apply_xor_maps;
 	}
 
+	cxl_setup_extended_linear_cache(cxlrd);
+
+	cxlrd->qos_class = cfmws->qtg_id;
+
 	rc = cxl_decoder_add(cxld);
 	if (rc)
 		return rc;
-- 
2.47.3

Re: [PATCH v4 13/14] cxl/acpi: Group xor arithmetric setup code in a single block

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:54 +0100
Robert Richter <rrichter@amd.com> wrote:

> Simplify the xor arithmetric setup code by grouping it in a single
> block. No need to split the block for QoS setup.
> 
> It is save to reorder the call of cxl_setup_extended_linear_cache()

safe rather than save

> because there are no dependencies.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
Seems fine to me.
Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>

> ---
>  drivers/cxl/acpi.c | 11 ++++-------
>  1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 778ee29430ea..40894e2156ce 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -449,8 +449,6 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>  		ig = CXL_DECODER_MIN_GRANULARITY;
>  	cxld->interleave_granularity = ig;
>  
> -	cxl_setup_extended_linear_cache(cxlrd);
> -
>  	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
>  		if (ways != 1 && ways != 3) {
>  			cxims_ctx = (struct cxl_cxims_context) {
> @@ -466,15 +464,14 @@ static int __cxl_parse_cfmws(struct acpi_cedt_cfmws *cfmws,
>  				return -EINVAL;
>  			}
>  		}
> -	}
> -
> -	cxlrd->qos_class = cfmws->qtg_id;
> -
> -	if (cfmws->interleave_arithmetic == ACPI_CEDT_CFMWS_ARITHMETIC_XOR) {
>  		cxlrd->ops.hpa_to_spa = cxl_apply_xor_maps;
>  		cxlrd->ops.spa_to_hpa = cxl_apply_xor_maps;
>  	}
>  
> +	cxl_setup_extended_linear_cache(cxlrd);
> +
> +	cxlrd->qos_class = cfmws->qtg_id;
> +
>  	rc = cxl_decoder_add(cxld);
>  	if (rc)
>  		return rc;

[PATCH v4 14/14] cxl/region: Remove local variable @inc in cxl_port_setup_targets()

[Robert Richter]

Simplify the code by removing local variable @inc. The variable is not
used elsewhere, remove it and directly increment the target number.

Signed-off-by: Robert Richter <rrichter@amd.com>
---
 drivers/cxl/core/region.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index dec003084521..a81fbe9cedae 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -1332,7 +1332,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 				  struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
-	int parent_iw, parent_ig, ig, iw, rc, inc = 0, pos = cxled->pos;
+	int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos;
 	struct cxl_port *parent_port = to_cxl_port(port->dev.parent);
 	struct cxl_region_ref *cxl_rr = cxl_rr_load(port, cxlr);
 	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
@@ -1524,9 +1524,8 @@ static int cxl_port_setup_targets(struct cxl_port *port,
 		cxlsd->target[cxl_rr->nr_targets_set] = ep->dport;
 		cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id;
 	}
-	inc = 1;
+	cxl_rr->nr_targets_set++;
 out_target_set:
-	cxl_rr->nr_targets_set += inc;
 	dev_dbg(&cxlr->dev, "%s:%s target[%d] = %s for %s:%s @ %d\n",
 		dev_name(port->uport_dev), dev_name(&port->dev),
 		cxl_rr->nr_targets_set - 1, dev_name(ep->dport->dport_dev),
-- 
2.47.3

Re: [PATCH v4 14/14] cxl/region: Remove local variable @inc in cxl_port_setup_targets()

[Jonathan Cameron]

On Mon, 3 Nov 2025 19:47:55 +0100
Robert Richter <rrichter@amd.com> wrote:

> Simplify the code by removing local variable @inc. The variable is not
> used elsewhere, remove it and directly increment the target number.
> 
> Signed-off-by: Robert Richter <rrichter@amd.com>
Hmm. I wonder what the thought process behind this once was? Ah well
clearly not needed now and I can't be bothered to do the archeology /
have too much other stuff to review!

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

> ---
>  drivers/cxl/core/region.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index dec003084521..a81fbe9cedae 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -1332,7 +1332,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  				  struct cxl_endpoint_decoder *cxled)
>  {
>  	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> -	int parent_iw, parent_ig, ig, iw, rc, inc = 0, pos = cxled->pos;
> +	int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos;
>  	struct cxl_port *parent_port = to_cxl_port(port->dev.parent);
>  	struct cxl_region_ref *cxl_rr = cxl_rr_load(port, cxlr);
>  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> @@ -1524,9 +1524,8 @@ static int cxl_port_setup_targets(struct cxl_port *port,
>  		cxlsd->target[cxl_rr->nr_targets_set] = ep->dport;
>  		cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id;
>  	}
> -	inc = 1;
> +	cxl_rr->nr_targets_set++;
>  out_target_set:
> -	cxl_rr->nr_targets_set += inc;
>  	dev_dbg(&cxlr->dev, "%s:%s target[%d] = %s for %s:%s @ %d\n",
>  		dev_name(port->uport_dev), dev_name(&port->dev),
>  		cxl_rr->nr_targets_set - 1, dev_name(ep->dport->dport_dev),

Re: [PATCH v4 14/14] cxl/region: Remove local variable @inc in cxl_port_setup_targets()

[Robert Richter]

On 11.11.25 15:36:44, Jonathan Cameron wrote:
> On Mon, 3 Nov 2025 19:47:55 +0100
> Robert Richter <rrichter@amd.com> wrote:
> 
> > Simplify the code by removing local variable @inc. The variable is not
> > used elsewhere, remove it and directly increment the target number.
> > 
> > Signed-off-by: Robert Richter <rrichter@amd.com>
> Hmm. I wonder what the thought process behind this once was? Ah well
> clearly not needed now and I can't be bothered to do the archeology /
> have too much other stuff to review!

The inc was there from the beginning (27b3f8d13830c). Maybe code
changed in the area during review process and left this as a
remainder from something else.

Thanks,

-Robert

> 
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
> 
> > ---
> >  drivers/cxl/core/region.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> > index dec003084521..a81fbe9cedae 100644
> > --- a/drivers/cxl/core/region.c
> > +++ b/drivers/cxl/core/region.c
> > @@ -1332,7 +1332,7 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> >  				  struct cxl_endpoint_decoder *cxled)
> >  {
> >  	struct cxl_root_decoder *cxlrd = cxlr->cxlrd;
> > -	int parent_iw, parent_ig, ig, iw, rc, inc = 0, pos = cxled->pos;
> > +	int parent_iw, parent_ig, ig, iw, rc, pos = cxled->pos;
> >  	struct cxl_port *parent_port = to_cxl_port(port->dev.parent);
> >  	struct cxl_region_ref *cxl_rr = cxl_rr_load(port, cxlr);
> >  	struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
> > @@ -1524,9 +1524,8 @@ static int cxl_port_setup_targets(struct cxl_port *port,
> >  		cxlsd->target[cxl_rr->nr_targets_set] = ep->dport;
> >  		cxlsd->cxld.target_map[cxl_rr->nr_targets_set] = ep->dport->port_id;
> >  	}
> > -	inc = 1;
> > +	cxl_rr->nr_targets_set++;
> >  out_target_set:
> > -	cxl_rr->nr_targets_set += inc;
> >  	dev_dbg(&cxlr->dev, "%s:%s target[%d] = %s for %s:%s @ %d\n",
> >  		dev_name(port->uport_dev), dev_name(&port->dev),
> >  		cxl_rr->nr_targets_set - 1, dev_name(ep->dport->dport_dev),
> 

Re: [PATCH v4 00/14] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement

[Alison Schofield]

On Mon, Nov 03, 2025 at 07:47:41PM +0100, Robert Richter wrote:
> This patch set adds support for address translation using ACPI PRM and
> enables this for AMD Zen5 platforms. This is another new appoach in
> response to earlier attempts to implement CXL address translation:
> 
>  * v1: [1] and the comments on it, esp. Dan's [2],
>  * v2: [3] and comments on [4], esp. Dave's [5]
>  * v3: [6] and comments on it, esp. Dave's [7]
> 
> This version 4 reworks and simplifies code to use an address
> translation callback bound to the root port. It moves all address
> translation code to a single file, core/atl.c.
> 
> Documentation of CXL Address Translation Support will be added to the
> Kernel's "Compute Express Link: Linux Conventions". This patch
> submission will be the base for a documention patch that describes CXL
> Address Translation support accordingly.

Hi Robert,

I see above that the documentation is expected to follow this patchset.
I'd been holding off expecting to have that in hand to do a decent
review here. Is that doc not necessary for review? Do commit messages,
code comments, and spec references give reviewers all they need?

> 
> The CXL driver currently does not implement address translation which
> assumes the host physical addresses (HPA) and system physical
> addresses (SPA) are equal.
> 
> Systems with different HPA and SPA addresses need address translation.
> If this is the case, the hardware addresses esp. used in the HDM
> decoder configurations are different to the system's or parent port
> address ranges. E.g. AMD Zen5 systems may be configured to use
> 'Normalized addresses'. Then, CXL endpoints have their own physical
> address base which is not the same as the SPA used by the CXL host
> bridge. Thus, addresses need to be translated from the endpoint's to
> its CXL host bridge's address range.
> 
> To enable address translation, the endpoint's HPA range must be
> translated to the CXL host bridge's address range. A callback is
> introduced to translate a decoder's HPA to the CXL host bridge's
> address range. The callback is then used to determine the region
> parameters which includes the SPA translated address range of the
> endpoint decoder and the interleaving configuration. This is stored in
> struct cxl_region which allows an endpoint decoder to determine that
> parameters based on its assigned region.
> 
> Note that only auto-discovery of decoders is supported. Thus, decoders
> are locked and cannot be configured manually.
> 
> Finally, Zen5 address translation is enabled using ACPI PRMT.
> 
> There are 3 additional cleanup patches at the end of the series. It
> might be worth to add them too, but could be dropped if there are
> concerns.

I peeked at the 3 trailing patches and they don't seem to depend upon
the series. I'd suggest submitting those individually for review.
That'll pull in more reviewers to those, ie folks who might not jump
into the entire patchset.

Thanks!
Alison

> 
> This series bases on cxl/next.
> 
> V4:
>  * rebased onto v6.18-rc2 (cxl/next),
>  * updated sob-chain,
>  * reworked and simplified code to use an address translation callback
>    bound to the root port,
>  * moved all address translation code to core/atl.c,
>  * cxlr->cxlrd change, updated patch description (Alison),
>  * use DEFINE_RANGE() (Jonathan),
>  * change name to @hpa_range (Dave, Jonathan),
>  * updated patch description if there is a no-op (Gregory),
>  * use Designated initializers for struct cxl_region_context (Dave),
>  * move callback handler to struct cxl_root_ops (Dave),
>  * move hanler inialization to acpi_probe() (Dave),
>  * updated comment where Normalized Addressing is checked (Dave),
>  * limit PRM enablement only to AMD supported kernel configs (AMD_NB)
>    (Jonathan),
>  * added 3 related optional cleanup patches at the end of the series,
> 
> V3:
>  * rebased onto cxl/next,
>  * complete rework to reduce number of required changes/patches and to
>    remove platform specific code (Dan and Dave),
>  * changed implementation allowing to add address translation to the
>    CXL specification (documention patch in preparation),
>  * simplified and generalized determination of interleaving
>    parameters using the address translation callback,
>  * depend only on the existence of the ACPI PRM GUID for CXL Address
>    Translation enablement, removed platform checks,
>  * small changes to region code only which does not require a full
>    rework and refactoring of the code, just separating region
>    parameter setup and region construction,
>  * moved code to new core/atl.c file,
>  * fixed subsys_initcall order dependency of EFI runtime services
>    (Gregory and Joshua),
> 
> V2:
>  * rebased onto cxl/next,
>  * split of v1 in two parts:
>    * removed cleanups and updates from this series to post them as a
>      separate series (Dave),
>    * this part 2 applies on top of part 1, v3,
>  * added tags to SOB chain,
>  * reworked architecture, vendor and platform setup (Jonathan):
>    * added patch "cxl/x86: Prepare for architectural platform setup",
>    * added function arch_cxl_port_platform_setup() plus a __weak
>      versions for archs other than x86,
>    * moved code to core/x86,
>  * added comment to cxl_to_hpa_fn (Ben),
>  * updated year in copyright statement (Ben),
>  * cxl_port_calc_hpa(): Removed HPA check for zero (Jonathan), return
>    1 if modified,
>  * cxl_port_calc_pos(): Updated description and wording (Ben),
>  * added sereral patches around interleaving and SPA calculation in
>    cxl_endpoint_decoder_initialize(),
>  * reworked iterator in cxl_endpoint_decoder_initialize() (Gregory),
>  * fixed region interleaving parameters() (Alison),
>  * fixed check in cxl_region_attach() (Alison),
>  * Clarified in coverletter that not all ports in a system must
>    implement the to_hpa() callback (Terry).
> 
> [1] https://lore.kernel.org/linux-cxl/20240701174754.967954-1-rrichter@amd.com/
> [2] https://lore.kernel.org/linux-cxl/669086821f136_5fffa29473@dwillia2-xfh.jf.intel.com.notmuch/
> [3] https://patchwork.kernel.org/project/cxl/cover/20250218132356.1809075-1-rrichter@amd.com/
> [4] https://patchwork.kernel.org/project/cxl/cover/20250715191143.1023512-1-rrichter@amd.com/
> [5] https://lore.kernel.org/all/78284b12-3e0b-4758-af18-397f32136c3f@intel.com/
> [6] https://patchwork.kernel.org/project/cxl/cover/20250912144514.526441-1-rrichter@amd.com/
> [7] https://lore.kernel.org/all/20250912144514.526441-8-rrichter@amd.com/T/#m23c2adb9d1e20770ccd5d11475288bda382b0af5
> 
> Robert Richter (14):
>   cxl/region: Store root decoder in struct cxl_region
>   cxl/region: Store HPA range in struct cxl_region
>   cxl/region: Rename misleading variable name @hpa to @hpa_range
>   cxl/region: Add @hpa_range argument to function
>     cxl_calc_interleave_pos()
>   cxl: Simplify cxl_root_ops allocation and handling
>   cxl/region: Separate region parameter setup and region construction
>   cxl/region: Use region data to get the root decoder
>   cxl: Introduce callback for HPA address ranges translation
>   cxl/acpi: Prepare use of EFI runtime services
>   cxl: Enable AMD Zen5 address translation using ACPI PRMT
>   cxl/atl: Lock decoders that need address translation
>   cxl: Simplify cxl_rd_ops allocation and handling
>   cxl/acpi: Group xor arithmetric setup code in a single block
>   cxl/region: Remove local variable @inc in cxl_port_setup_targets()
> 
>  drivers/cxl/Kconfig       |   4 +
>  drivers/cxl/acpi.c        |  32 +++---
>  drivers/cxl/core/Makefile |   1 +
>  drivers/cxl/core/atl.c    | 205 ++++++++++++++++++++++++++++++++++++++
>  drivers/cxl/core/cdat.c   |   8 +-
>  drivers/cxl/core/core.h   |   9 ++
>  drivers/cxl/core/port.c   |   9 +-
>  drivers/cxl/core/region.c | 185 +++++++++++++++++++---------------
>  drivers/cxl/cxl.h         |  33 ++++--
>  9 files changed, 366 insertions(+), 120 deletions(-)
>  create mode 100644 drivers/cxl/core/atl.c
> 
> 
> base-commit: 211ddde0823f1442e4ad052a2f30f050145ccada
> -- 
> 2.47.3
> 
> 

Re: [PATCH v4 00/14] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement

[Robert Richter]

On Tue, Nov 04, 2025 at 08:17:13AM -0800, Alison Schofield wrote:

> > Documentation of CXL Address Translation Support will be added to the
> > Kernel's "Compute Express Link: Linux Conventions". This patch
> > submission will be the base for a documention patch that describes CXL
> > Address Translation support accordingly.

I am aware of the lack of documentation here and am preparing a patch
for this. Though, I hope the patch descriptions are clear and
descriptive on their own.

-Robert

Re: [PATCH v4 00/14] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement

[Gregory Price]

On Mon, Nov 17, 2025 at 04:34:02PM +0100, Robert Richter wrote:
> On Tue, Nov 04, 2025 at 08:17:13AM -0800, Alison Schofield wrote:
> 
> > > Documentation of CXL Address Translation Support will be added to the
> > > Kernel's "Compute Express Link: Linux Conventions". This patch
> > > submission will be the base for a documention patch that describes CXL
> > > Address Translation support accordingly.
> 
> I am aware of the lack of documentation here and am preparing a patch
> for this. Though, I hope the patch descriptions are clear and
> descriptive on their own.
>

If it would help, I can take a quick run at this doc this week, unless
you already have a draft.  Let me know if I can help.

~Gregory

Re: [PATCH v4 00/14] cxl: ACPI PRM Address Translation Support and AMD Zen5 enablement

[Gregory Price]

On Mon, Nov 03, 2025 at 07:47:41PM +0100, Robert Richter wrote:
> This patch set adds support for address translation using ACPI PRM and
> enables this for AMD Zen5 platforms. This is another new appoach in
> response to earlier attempts to implement CXL address translation:
> 
>  * v1: [1] and the comments on it, esp. Dan's [2],
>  * v2: [3] and comments on [4], esp. Dave's [5]
>  * v3: [6] and comments on it, esp. Dave's [7]
> 

Have been testing this on my Zen5 systems backported to v6.16 with
Terry's updated PCIe set as well and everything looks stable.

Tested-by: Gregory Price <gourry@gourry.net>