[PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[Dave Jiang]

Remove cxl_hpa_to_spa_fn callback from cxl root decoder and add the
callback function to cxl_decoder_ops. Refactor to group all the root
decoder callbacks together.

Signed-off-by: Dave Jiang <dave.jiang@intel.com>
---
 drivers/cxl/acpi.c        |  9 +++++++--
 drivers/cxl/core/port.c   |  2 +-
 drivers/cxl/core/region.c | 11 ++++++++---
 drivers/cxl/cxl.h         | 13 ++++++++-----
 4 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
index 48478bc406ee..9f7e2a49dc5a 100644
--- a/drivers/cxl/acpi.c
+++ b/drivers/cxl/acpi.c
@@ -20,6 +20,10 @@ static const guid_t acpi_cxl_qtg_id_guid =
 	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
 		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
 
+static u64 cxl_default_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
+{
+	return hpa;
+}
 
 static u64 cxl_xor_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
 {
@@ -342,8 +346,9 @@ static int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
 	return hmat_get_extended_linear_cache_size(backing_res, nid, size);
 }
 
-static const struct cxl_rd_ops acpi_rd_ops = {
+static struct cxl_rd_ops acpi_rd_ops = {
 	.get_extended_linear_cache_size = cxl_acpi_get_extended_linear_cache_size,
+	.hpa_to_spa = cxl_default_hpa_to_spa,
 };
 
 DEFINE_FREE(put_cxlrd, struct cxl_root_decoder *,
@@ -425,7 +430,7 @@ 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->hpa_to_spa = cxl_xor_hpa_to_spa;
+		cxlrd->ops->hpa_to_spa = cxl_xor_hpa_to_spa;
 
 	rc = cxl_decoder_add(cxld, target_map);
 	if (rc)
diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
index 6f4cd50ddf25..081dd59b422e 100644
--- a/drivers/cxl/core/port.c
+++ b/drivers/cxl/core/port.c
@@ -1802,7 +1802,7 @@ static int cxl_switch_decoder_init(struct cxl_port *port,
  */
 struct cxl_root_decoder *cxl_root_decoder_alloc(struct cxl_port *port,
 						unsigned int nr_targets,
-						const struct cxl_rd_ops *ops)
+						struct cxl_rd_ops *ops)
 {
 	struct cxl_root_decoder *cxlrd;
 	struct cxl_switch_decoder *cxlsd;
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 0a5effbc0529..f148c398b7bb 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2902,6 +2902,11 @@ 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;
+}
+
 u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 		   u64 dpa)
 {
@@ -2956,8 +2961,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 (cxlrd->hpa_to_spa)
-		hpa = cxlrd->hpa_to_spa(cxlrd, hpa);
+	if (has_hpa_to_spa(cxlrd))
+		hpa = cxlrd->ops->hpa_to_spa(cxlrd, hpa);
 
 	if (hpa < p->res->start || hpa > p->res->end) {
 		dev_dbg(&cxlr->dev,
@@ -2966,7 +2971,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 (!cxlrd->hpa_to_spa && (!cxl_is_hpa_in_chunk(hpa, cxlr, pos)))
+	if (!has_hpa_to_spa(cxlrd) && (!cxl_is_hpa_in_chunk(hpa, cxlr, pos)))
 		return ULLONG_MAX;
 
 	return hpa;
diff --git a/drivers/cxl/cxl.h b/drivers/cxl/cxl.h
index 91cb1e570907..1e7396d2ca6c 100644
--- a/drivers/cxl/cxl.h
+++ b/drivers/cxl/cxl.h
@@ -418,18 +418,22 @@ struct cxl_switch_decoder {
 };
 
 struct cxl_root_decoder;
-typedef u64 (*cxl_hpa_to_spa_fn)(struct cxl_root_decoder *cxlrd, u64 hpa);
 
+/**
+ * struct cxl_rd_ops - CXL root decoder callback operations
+ * @get_extended_linear_cache_size: Get the extended linear cache size
+ * @hpa_to_spa: Convert host physical address to system physical address
+ */
 struct cxl_rd_ops {
 	int (*get_extended_linear_cache_size)(struct resource *backing_res,
 					      int nid, resource_size_t *size);
+	u64 (*hpa_to_spa)(struct cxl_root_decoder *cxlrd, u64 hpa);
 };
 
 /**
  * struct cxl_root_decoder - Static platform CXL address decoder
  * @res: host / parent resource for region allocations
  * @region_id: region id for next region provisioning event
- * @hpa_to_spa: translate CXL host-physical-address to Platform system-physical-address
  * @platform_data: platform specific configuration data
  * @range_lock: sync region autodiscovery by address range
  * @qos_class: QoS performance class cookie
@@ -439,11 +443,10 @@ struct cxl_rd_ops {
 struct cxl_root_decoder {
 	struct resource *res;
 	atomic_t region_id;
-	cxl_hpa_to_spa_fn hpa_to_spa;
 	void *platform_data;
 	struct mutex range_lock;
 	int qos_class;
-	const struct cxl_rd_ops *ops;
+	struct cxl_rd_ops *ops;
 	struct cxl_switch_decoder cxlsd;
 };
 
@@ -783,7 +786,7 @@ bool is_switch_decoder(struct device *dev);
 bool is_endpoint_decoder(struct device *dev);
 struct cxl_root_decoder *cxl_root_decoder_alloc(struct cxl_port *port,
 						unsigned int nr_targets,
-						const struct cxl_rd_ops *ops);
+						struct cxl_rd_ops *ops);
 struct cxl_switch_decoder *cxl_switch_decoder_alloc(struct cxl_port *port,
 						    unsigned int nr_targets);
 int cxl_decoder_add(struct cxl_decoder *cxld, int *target_map);

base-commit: 1a45e1cd694fdf2d2a22e3c70a6917ff39fe77ab
-- 
2.50.0

Re: [PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[Alison Schofield]

On Wed, Jul 09, 2025 at 01:07:46PM -0700, Dave Jiang wrote:
> Remove cxl_hpa_to_spa_fn callback from cxl root decoder and add the
> callback function to cxl_decoder_ops. Refactor to group all the root
> decoder callbacks together.

This one has cxl unit test coverage for the setup of the callback, but
no test case for the actual usage of the callback. I tested it manually
and I'll take the todo task of adding a test case that covers all the
plumbing.

Suggested-by: Alison Schofield <alison.schofield@intel.com>
Tested-by: Alison Schofield <alison.schofield@intel.com>

Re: [PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[Jonathan Cameron]

On Wed, 9 Jul 2025 13:07:46 -0700
Dave Jiang <dave.jiang@intel.com> wrote:

> Remove cxl_hpa_to_spa_fn callback from cxl root decoder and add the
> callback function to cxl_decoder_ops. Refactor to group all the root
> decoder callbacks together.
> 
> Signed-off-by: Dave Jiang <dave.jiang@intel.com>

Question inline about mixture of types of CFMWS entry.

> ---
>  drivers/cxl/acpi.c        |  9 +++++++--
>  drivers/cxl/core/port.c   |  2 +-
>  drivers/cxl/core/region.c | 11 ++++++++---
>  drivers/cxl/cxl.h         | 13 ++++++++-----
>  4 files changed, 24 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> index 48478bc406ee..9f7e2a49dc5a 100644
> --- a/drivers/cxl/acpi.c
> +++ b/drivers/cxl/acpi.c
> @@ -20,6 +20,10 @@ static const guid_t acpi_cxl_qtg_id_guid =
>  	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
>  		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
>  
> +static u64 cxl_default_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> +{
> +	return hpa;
> +}
>  
>  static u64 cxl_xor_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
>  {
> @@ -342,8 +346,9 @@ static int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
>  	return hmat_get_extended_linear_cache_size(backing_res, nid, size);
>  }
>  
> -static const struct cxl_rd_ops acpi_rd_ops = {
> +static struct cxl_rd_ops acpi_rd_ops = {
>  	.get_extended_linear_cache_size = cxl_acpi_get_extended_linear_cache_size,
> +	.hpa_to_spa = cxl_default_hpa_to_spa,
>  };
>  
>  DEFINE_FREE(put_cxlrd, struct cxl_root_decoder *,
> @@ -425,7 +430,7 @@ 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->hpa_to_spa = cxl_xor_hpa_to_spa;
> +		cxlrd->ops->hpa_to_spa = cxl_xor_hpa_to_spa;

What happens if we have a mixture for different CMWS entries?
Some xor, some not.

>  
>  	rc = cxl_decoder_add(cxld, target_map);
>  	if (rc)

Re: [PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[Alison Schofield]

On Fri, Jul 11, 2025 at 05:45:30PM +0100, Jonathan Cameron wrote:
> On Wed, 9 Jul 2025 13:07:46 -0700
> Dave Jiang <dave.jiang@intel.com> wrote:
> 
> > Remove cxl_hpa_to_spa_fn callback from cxl root decoder and add the
> > callback function to cxl_decoder_ops. Refactor to group all the root
> > decoder callbacks together.
> > 
> > Signed-off-by: Dave Jiang <dave.jiang@intel.com>
> 
> Question inline about mixture of types of CFMWS entry.
> 
> > ---
> >  drivers/cxl/acpi.c        |  9 +++++++--
> >  drivers/cxl/core/port.c   |  2 +-
> >  drivers/cxl/core/region.c | 11 ++++++++---
> >  drivers/cxl/cxl.h         | 13 ++++++++-----
> >  4 files changed, 24 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > index 48478bc406ee..9f7e2a49dc5a 100644
> > --- a/drivers/cxl/acpi.c
> > +++ b/drivers/cxl/acpi.c
> > @@ -20,6 +20,10 @@ static const guid_t acpi_cxl_qtg_id_guid =
> >  	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
> >  		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
> >  
> > +static u64 cxl_default_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> > +{
> > +	return hpa;
> > +}
> >  
> >  static u64 cxl_xor_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> >  {
> > @@ -342,8 +346,9 @@ static int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
> >  	return hmat_get_extended_linear_cache_size(backing_res, nid, size);
> >  }
> >  
> > -static const struct cxl_rd_ops acpi_rd_ops = {
> > +static struct cxl_rd_ops acpi_rd_ops = {
> >  	.get_extended_linear_cache_size = cxl_acpi_get_extended_linear_cache_size,
> > +	.hpa_to_spa = cxl_default_hpa_to_spa,
> >  };
> >  
> >  DEFINE_FREE(put_cxlrd, struct cxl_root_decoder *,
> > @@ -425,7 +430,7 @@ 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->hpa_to_spa = cxl_xor_hpa_to_spa;
> > +		cxlrd->ops->hpa_to_spa = cxl_xor_hpa_to_spa;
> 
> What happens if we have a mixture for different CMWS entries?
> Some xor, some not.

A separate root decoder is created for each CFMWS entry, so no
conflict. Is that what you're wondering?


> 
> >  
> >  	rc = cxl_decoder_add(cxld, target_map);
> >  	if (rc)
> 

Re: [PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[Jonathan Cameron]

On Fri, 11 Jul 2025 11:41:19 -0700
Alison Schofield <alison.schofield@intel.com> wrote:

> On Fri, Jul 11, 2025 at 05:45:30PM +0100, Jonathan Cameron wrote:
> > On Wed, 9 Jul 2025 13:07:46 -0700
> > Dave Jiang <dave.jiang@intel.com> wrote:
> >   
> > > Remove cxl_hpa_to_spa_fn callback from cxl root decoder and add the
> > > callback function to cxl_decoder_ops. Refactor to group all the root
> > > decoder callbacks together.
> > > 
> > > Signed-off-by: Dave Jiang <dave.jiang@intel.com>  
> > 
> > Question inline about mixture of types of CFMWS entry.
> >   
> > > ---
> > >  drivers/cxl/acpi.c        |  9 +++++++--
> > >  drivers/cxl/core/port.c   |  2 +-
> > >  drivers/cxl/core/region.c | 11 ++++++++---
> > >  drivers/cxl/cxl.h         | 13 ++++++++-----
> > >  4 files changed, 24 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > > index 48478bc406ee..9f7e2a49dc5a 100644
> > > --- a/drivers/cxl/acpi.c
> > > +++ b/drivers/cxl/acpi.c
> > > @@ -20,6 +20,10 @@ static const guid_t acpi_cxl_qtg_id_guid =
> > >  	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
> > >  		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
> > >  
> > > +static u64 cxl_default_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> > > +{
> > > +	return hpa;
> > > +}
> > >  
> > >  static u64 cxl_xor_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> > >  {
> > > @@ -342,8 +346,9 @@ static int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
> > >  	return hmat_get_extended_linear_cache_size(backing_res, nid, size);
> > >  }
> > >  
> > > -static const struct cxl_rd_ops acpi_rd_ops = {
> > > +static struct cxl_rd_ops acpi_rd_ops = {
> > >  	.get_extended_linear_cache_size = cxl_acpi_get_extended_linear_cache_size,
> > > +	.hpa_to_spa = cxl_default_hpa_to_spa,
> > >  };
> > >  
> > >  DEFINE_FREE(put_cxlrd, struct cxl_root_decoder *,
> > > @@ -425,7 +430,7 @@ 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->hpa_to_spa = cxl_xor_hpa_to_spa;
> > > +		cxlrd->ops->hpa_to_spa = cxl_xor_hpa_to_spa;  
> > 
> > What happens if we have a mixture for different CMWS entries?
> > Some xor, some not.  
> 
> A separate root decoder is created for each CFMWS entry, so no
> conflict. Is that what you're wondering?

This isn't modifying a copy. It's modifying the global structure
that you make non const above (or I'm lacking caffeine.)


> 
> 
> >   
> > >  
> > >  	rc = cxl_decoder_add(cxld, target_map);
> > >  	if (rc)  
> >   
> 

Re: [PATCH] cxl: Add hpa_to_spa to the root decoder callback operations

[Alison Schofield]

On Tue, Jul 15, 2025 at 04:23:42PM +0100, Jonathan Cameron wrote:
> On Fri, 11 Jul 2025 11:41:19 -0700
> Alison Schofield <alison.schofield@intel.com> wrote:
> 
> > On Fri, Jul 11, 2025 at 05:45:30PM +0100, Jonathan Cameron wrote:
> > > On Wed, 9 Jul 2025 13:07:46 -0700
> > > Dave Jiang <dave.jiang@intel.com> wrote:
> > >   
> > > > Remove cxl_hpa_to_spa_fn callback from cxl root decoder and add the
> > > > callback function to cxl_decoder_ops. Refactor to group all the root
> > > > decoder callbacks together.
> > > > 
> > > > Signed-off-by: Dave Jiang <dave.jiang@intel.com>  
> > > 
> > > Question inline about mixture of types of CFMWS entry.
> > >   
> > > > ---
> > > >  drivers/cxl/acpi.c        |  9 +++++++--
> > > >  drivers/cxl/core/port.c   |  2 +-
> > > >  drivers/cxl/core/region.c | 11 ++++++++---
> > > >  drivers/cxl/cxl.h         | 13 ++++++++-----
> > > >  4 files changed, 24 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/cxl/acpi.c b/drivers/cxl/acpi.c
> > > > index 48478bc406ee..9f7e2a49dc5a 100644
> > > > --- a/drivers/cxl/acpi.c
> > > > +++ b/drivers/cxl/acpi.c
> > > > @@ -20,6 +20,10 @@ static const guid_t acpi_cxl_qtg_id_guid =
> > > >  	GUID_INIT(0xF365F9A6, 0xA7DE, 0x4071,
> > > >  		  0xA6, 0x6A, 0xB4, 0x0C, 0x0B, 0x4F, 0x8E, 0x52);
> > > >  
> > > > +static u64 cxl_default_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> > > > +{
> > > > +	return hpa;
> > > > +}
> > > >  
> > > >  static u64 cxl_xor_hpa_to_spa(struct cxl_root_decoder *cxlrd, u64 hpa)
> > > >  {
> > > > @@ -342,8 +346,9 @@ static int cxl_acpi_get_extended_linear_cache_size(struct resource *backing_res,
> > > >  	return hmat_get_extended_linear_cache_size(backing_res, nid, size);
> > > >  }
> > > >  
> > > > -static const struct cxl_rd_ops acpi_rd_ops = {
> > > > +static struct cxl_rd_ops acpi_rd_ops = {
> > > >  	.get_extended_linear_cache_size = cxl_acpi_get_extended_linear_cache_size,
> > > > +	.hpa_to_spa = cxl_default_hpa_to_spa,
> > > >  };
> > > >  
> > > >  DEFINE_FREE(put_cxlrd, struct cxl_root_decoder *,
> > > > @@ -425,7 +430,7 @@ 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->hpa_to_spa = cxl_xor_hpa_to_spa;
> > > > +		cxlrd->ops->hpa_to_spa = cxl_xor_hpa_to_spa;  
> > > 
> > > What happens if we have a mixture for different CMWS entries?
> > > Some xor, some not.  
> > 
> > A separate root decoder is created for each CFMWS entry, so no
> > conflict. Is that what you're wondering?
> 
> This isn't modifying a copy. It's modifying the global structure
> that you make non const above (or I'm lacking caffeine.)

See it. Every root decoder will save the same address of the 
acpi_rd_ops struct, and then some will modify the contents.

The extended linear cache set that started the callbacks went
in another direction and no longer uses a root decoder callback.
I've adopted this patch as a precursor patch in the poison set,
so you'll see a variation on it soon.

> 
> 
> > 
> > 
> > >   
> > > >  
> > > >  	rc = cxl_decoder_add(cxld, target_map);
> > > >  	if (rc)  
> > >   
> > 
>