[PATCH v3 0/3] Fix wrong dpa checking in PPR operation

[PATCH v3 0/3] Fix wrong dpa checking in PPR operation

[Li Ming]

In cxl_do_ppr(), there is a checking to check if a DPA is valid, the
implementation of the checking is check if the DPA is 0, if yes,
consider that DPA is valid. the checking is not right, the correct
implementation is checking if the DPA is in the CXL device DPA range, if
yes, it is valid.

The patchset also includes another part implementing a general helper
function resource_contains_addr() in cxl core so that cxl drivers can
use it for all DPA/HPA/SPA availability checking.


v3:
- Move resource_contains_addr() from include/linux/ioport.h to
cxl/core/hdm.c. (Andy)

v2:
- Implement a general helper resource_contains_addr() for DPA/HPA
resource. (Alison)

base-commit: 0a46f60a9fe16f5596b6b4b3ee1a483ea7854136 cxl/fixes

Li Ming (3):
  cxl/core: Introduce a new helper resource_contains_addr()
  cxl/edac: Fix wrong dpa checking for PPR operation
  cxl/core: Using resource_contains_addr() to check address availability

 drivers/cxl/core/core.h   | 1 +
 drivers/cxl/core/edac.c   | 9 ++++++---
 drivers/cxl/core/hdm.c    | 8 ++++++++
 drivers/cxl/core/memdev.c | 2 +-
 drivers/cxl/core/region.c | 6 +++---
 5 files changed, 19 insertions(+), 7 deletions(-)

-- 
2.34.1

[PATCH v3 1/3] cxl/core: Introduce a new helper resource_contains_addr()

[Li Ming]

In CXL subsystem, many functions need to check an address availability
by checking if the resource range contains the address. Providing a new
helper function resource_contains_addr() to check if the resource range
contains the input address.

Suggested-by: Alison Schofield <alison.schofield@intel.com>
Signed-off-by: Li Ming <ming.li@zohomail.com>
Tested-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/core.h | 1 +
 drivers/cxl/core/hdm.c  | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 29b61828a847..3798e9047175 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -80,6 +80,7 @@ int cxl_dpa_alloc(struct cxl_endpoint_decoder *cxled, u64 size);
 int cxl_dpa_free(struct cxl_endpoint_decoder *cxled);
 resource_size_t cxl_dpa_size(struct cxl_endpoint_decoder *cxled);
 resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled);
+bool resource_contains_addr(const struct resource *res, const resource_size_t addr);
 
 enum cxl_rcrb {
 	CXL_RCRB_DOWNSTREAM,
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index ab1007495f6b..701a6a3baa6a 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -547,6 +547,14 @@ resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled)
 	return base;
 }
 
+bool resource_contains_addr(const struct resource *res, const resource_size_t addr)
+{
+	if (res->flags & IORESOURCE_MEM)
+		return res->start <= addr && addr <= res->end;
+
+	return false;
+}
+
 int cxl_dpa_free(struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_port *port = cxled_to_port(cxled);
-- 
2.34.1

[PATCH v3 2/3] cxl/edac: Fix wrong dpa checking for PPR operation

[Li Ming]

DPA 0 is considered invalid in cxl_do_ppr(), but per Table 8-143. "Get
Partition Info Output Payload" in CXL r3.2 section 8.2.10.9.2.1 "Get
Partition Info(Opcode 4100h)", it mentions that DPA 0 is a valid address
of a CXL device. So the correct implementation should be checking if the
DPA is in the DPA range of the CXL device rather than checking if the
DPA is equal to 0.

Fixes: be9b359e056a ("cxl/edac: Add CXL memory device soft PPR control feature")
Signed-off-by: Li Ming <ming.li@zohomail.com>
Tested-by: Shiju Jose <shiju.jose@huawei.com>
Reviewed-by: Shiju Jose <shiju.jose@huawei.com>
---
 drivers/cxl/core/edac.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index 623aaa4439c4..1cf65b1538b9 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -1923,8 +1923,11 @@ static int cxl_ppr_set_nibble_mask(struct device *dev, void *drv_data,
 static int cxl_do_ppr(struct device *dev, void *drv_data, u32 val)
 {
 	struct cxl_ppr_context *cxl_ppr_ctx = drv_data;
+	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
+	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 
-	if (!cxl_ppr_ctx->dpa || val != EDAC_DO_MEM_REPAIR)
+	if (!resource_contains_addr(&cxlds->dpa_res, cxl_ppr_ctx->dpa) ||
+	    val != EDAC_DO_MEM_REPAIR)
 		return -EINVAL;
 
 	return cxl_mem_perform_ppr(cxl_ppr_ctx);
-- 
2.34.1

[PATCH v3 3/3] cxl/core: Using resource_contains_addr() to check address availability

[Li Ming]

Helper function resource_contains_addr() can be used to check if a
resource range contains an input address. Use it to replace all
code that checks whether a resource range contains the input
DPA/HPA/SPA.

Signed-off-by: Li Ming <ming.li@zohomail.com>
---
 drivers/cxl/core/edac.c   | 4 ++--
 drivers/cxl/core/memdev.c | 2 +-
 drivers/cxl/core/region.c | 6 +++---
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index 1cf65b1538b9..a3c19ec8033f 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -1523,7 +1523,7 @@ static int cxl_mem_sparing_set_dpa(struct device *dev, void *drv_data, u64 dpa)
 	struct cxl_memdev *cxlmd = ctx->cxlmd;
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 
-	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end)
+	if (!resource_contains_addr(&cxlds->dpa_res, dpa))
 		return -EINVAL;
 
 	ctx->dpa = dpa;
@@ -1892,7 +1892,7 @@ static int cxl_ppr_set_dpa(struct device *dev, void *drv_data, u64 dpa)
 	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 
-	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end)
+	if (!resource_contains_addr(&cxlds->dpa_res, dpa))
 		return -EINVAL;
 
 	cxl_ppr_ctx->dpa = dpa;
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index f88a13adf7fa..4a5dc1d93af1 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -267,7 +267,7 @@ static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
 		dev_dbg(cxlds->dev, "device has no dpa resource\n");
 		return -EINVAL;
 	}
-	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end) {
+	if (!resource_contains_addr(&cxlds->dpa_res, dpa)) {
 		dev_dbg(cxlds->dev, "dpa:0x%llx not in resource:%pR\n",
 			dpa, &cxlds->dpa_res);
 		return -EINVAL;
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 6e5e1460068d..79d8211f8ed0 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2847,7 +2847,7 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
 	if (!cxled || !cxled->dpa_res || !resource_size(cxled->dpa_res))
 		return 0;
 
-	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
+	if (!resource_contains_addr(cxled->dpa_res, dpa))
 		return 0;
 
 	/*
@@ -2959,7 +2959,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
 	if (cxlrd->hpa_to_spa)
 		hpa = cxlrd->hpa_to_spa(cxlrd, hpa);
 
-	if (hpa < p->res->start || hpa > p->res->end) {
+	if (!resource_contains_addr(p->res, hpa)) {
 		dev_dbg(&cxlr->dev,
 			"Addr trans fail: hpa 0x%llx not in region\n", hpa);
 		return ULLONG_MAX;
@@ -3499,7 +3499,7 @@ u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa)
 	xa_for_each(&endpoint->regions, index, iter) {
 		struct cxl_region_params *p = &iter->region->params;
 
-		if (p->res->start <= spa && spa <= p->res->end) {
+		if (resource_contains_addr(p->res, spa)) {
 			if (!p->cache_size)
 				return ~0ULL;
 
-- 
2.34.1

Re: [PATCH v3 1/3] cxl/core: Introduce a new helper resource_contains_addr()

[Andy Shevchenko]

On Tue, Jul 08, 2025 at 01:15:34PM +0800, Li Ming wrote:
> In CXL subsystem, many functions need to check an address availability
> by checking if the resource range contains the address. Providing a new
> helper function resource_contains_addr() to check if the resource range
> contains the input address.

...

> +bool resource_contains_addr(const struct resource *res, const resource_size_t addr);

Right, the problem is that it collides with the resource namespace. Please, add a prefix.

bool cxl_resource_contains_addr(const struct resource *res, const resource_size_t addr);

...

> +bool resource_contains_addr(const struct resource *res, const resource_size_t addr)
> +{
> +	if (res->flags & IORESOURCE_MEM)

resource_type() ?

> +		return res->start <= addr && addr <= res->end;
> +
> +	return false;

I still think using DEFINE_RES_MEM() with resource_contains() is a better
approach.

> +}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] cxl/edac: Fix wrong dpa checking for PPR operation

[Dave Jiang]

On 7/7/25 10:15 PM, Li Ming wrote:
> DPA 0 is considered invalid in cxl_do_ppr(), but per Table 8-143. "Get
> Partition Info Output Payload" in CXL r3.2 section 8.2.10.9.2.1 "Get
> Partition Info(Opcode 4100h)", it mentions that DPA 0 is a valid address
> of a CXL device. So the correct implementation should be checking if the
> DPA is in the DPA range of the CXL device rather than checking if the
> DPA is equal to 0.
> 
> Fixes: be9b359e056a ("cxl/edac: Add CXL memory device soft PPR control feature")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> Tested-by: Shiju Jose <shiju.jose@huawei.com>
> Reviewed-by: Shiju Jose <shiju.jose@huawei.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/edac.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 623aaa4439c4..1cf65b1538b9 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -1923,8 +1923,11 @@ static int cxl_ppr_set_nibble_mask(struct device *dev, void *drv_data,
>  static int cxl_do_ppr(struct device *dev, void *drv_data, u32 val)
>  {
>  	struct cxl_ppr_context *cxl_ppr_ctx = drv_data;
> +	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -	if (!cxl_ppr_ctx->dpa || val != EDAC_DO_MEM_REPAIR)
> +	if (!resource_contains_addr(&cxlds->dpa_res, cxl_ppr_ctx->dpa) ||
> +	    val != EDAC_DO_MEM_REPAIR)
>  		return -EINVAL;
>  
>  	return cxl_mem_perform_ppr(cxl_ppr_ctx);

Re: [PATCH v3 3/3] cxl/core: Using resource_contains_addr() to check address availability

[Dave Jiang]

On 7/7/25 10:15 PM, Li Ming wrote:
> Helper function resource_contains_addr() can be used to check if a
> resource range contains an input address. Use it to replace all
> code that checks whether a resource range contains the input
> DPA/HPA/SPA.
> 
> Signed-off-by: Li Ming <ming.li@zohomail.com>
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
> ---
>  drivers/cxl/core/edac.c   | 4 ++--
>  drivers/cxl/core/memdev.c | 2 +-
>  drivers/cxl/core/region.c | 6 +++---
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 1cf65b1538b9..a3c19ec8033f 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -1523,7 +1523,7 @@ static int cxl_mem_sparing_set_dpa(struct device *dev, void *drv_data, u64 dpa)
>  	struct cxl_memdev *cxlmd = ctx->cxlmd;
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end)
> +	if (!resource_contains_addr(&cxlds->dpa_res, dpa))
>  		return -EINVAL;
>  
>  	ctx->dpa = dpa;
> @@ -1892,7 +1892,7 @@ static int cxl_ppr_set_dpa(struct device *dev, void *drv_data, u64 dpa)
>  	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end)
> +	if (!resource_contains_addr(&cxlds->dpa_res, dpa))
>  		return -EINVAL;
>  
>  	cxl_ppr_ctx->dpa = dpa;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f88a13adf7fa..4a5dc1d93af1 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -267,7 +267,7 @@ static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
>  		dev_dbg(cxlds->dev, "device has no dpa resource\n");
>  		return -EINVAL;
>  	}
> -	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end) {
> +	if (!resource_contains_addr(&cxlds->dpa_res, dpa)) {
>  		dev_dbg(cxlds->dev, "dpa:0x%llx not in resource:%pR\n",
>  			dpa, &cxlds->dpa_res);
>  		return -EINVAL;
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6e5e1460068d..79d8211f8ed0 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2847,7 +2847,7 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
>  	if (!cxled || !cxled->dpa_res || !resource_size(cxled->dpa_res))
>  		return 0;
>  
> -	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
> +	if (!resource_contains_addr(cxled->dpa_res, dpa))
>  		return 0;
>  
>  	/*
> @@ -2959,7 +2959,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	if (cxlrd->hpa_to_spa)
>  		hpa = cxlrd->hpa_to_spa(cxlrd, hpa);
>  
> -	if (hpa < p->res->start || hpa > p->res->end) {
> +	if (!resource_contains_addr(p->res, hpa)) {
>  		dev_dbg(&cxlr->dev,
>  			"Addr trans fail: hpa 0x%llx not in region\n", hpa);
>  		return ULLONG_MAX;
> @@ -3499,7 +3499,7 @@ u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa)
>  	xa_for_each(&endpoint->regions, index, iter) {
>  		struct cxl_region_params *p = &iter->region->params;
>  
> -		if (p->res->start <= spa && spa <= p->res->end) {
> +		if (resource_contains_addr(p->res, spa)) {
>  			if (!p->cache_size)
>  				return ~0ULL;
>  

Re: [PATCH v3 1/3] cxl/core: Introduce a new helper resource_contains_addr()

[Alison Schofield]

On Tue, Jul 08, 2025 at 01:15:34PM +0800, Li Ming wrote:
> In CXL subsystem, many functions need to check an address availability
> by checking if the resource range contains the address. Providing a new
> helper function resource_contains_addr() to check if the resource range
> contains the input address.

with the cxl_ prefix added to the new helper, you can add:

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

--snip

Re: [PATCH v3 3/3] cxl/core: Using resource_contains_addr() to check address availability

[Alison Schofield]

On Tue, Jul 08, 2025 at 01:15:36PM +0800, Li Ming wrote:
> Helper function resource_contains_addr() can be used to check if a
> resource range contains an input address. Use it to replace all
> code that checks whether a resource range contains the input
> DPA/HPA/SPA.
> 

with the cxl_ prefix update, you can add:

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


> Signed-off-by: Li Ming <ming.li@zohomail.com>
> ---
>  drivers/cxl/core/edac.c   | 4 ++--
>  drivers/cxl/core/memdev.c | 2 +-
>  drivers/cxl/core/region.c | 6 +++---
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 1cf65b1538b9..a3c19ec8033f 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -1523,7 +1523,7 @@ static int cxl_mem_sparing_set_dpa(struct device *dev, void *drv_data, u64 dpa)
>  	struct cxl_memdev *cxlmd = ctx->cxlmd;
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end)
> +	if (!resource_contains_addr(&cxlds->dpa_res, dpa))
>  		return -EINVAL;
>  
>  	ctx->dpa = dpa;
> @@ -1892,7 +1892,7 @@ static int cxl_ppr_set_dpa(struct device *dev, void *drv_data, u64 dpa)
>  	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
>  	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end)
> +	if (!resource_contains_addr(&cxlds->dpa_res, dpa))
>  		return -EINVAL;
>  
>  	cxl_ppr_ctx->dpa = dpa;
> diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
> index f88a13adf7fa..4a5dc1d93af1 100644
> --- a/drivers/cxl/core/memdev.c
> +++ b/drivers/cxl/core/memdev.c
> @@ -267,7 +267,7 @@ static int cxl_validate_poison_dpa(struct cxl_memdev *cxlmd, u64 dpa)
>  		dev_dbg(cxlds->dev, "device has no dpa resource\n");
>  		return -EINVAL;
>  	}
> -	if (dpa < cxlds->dpa_res.start || dpa > cxlds->dpa_res.end) {
> +	if (!resource_contains_addr(&cxlds->dpa_res, dpa)) {
>  		dev_dbg(cxlds->dev, "dpa:0x%llx not in resource:%pR\n",
>  			dpa, &cxlds->dpa_res);
>  		return -EINVAL;
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 6e5e1460068d..79d8211f8ed0 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -2847,7 +2847,7 @@ static int __cxl_dpa_to_region(struct device *dev, void *arg)
>  	if (!cxled || !cxled->dpa_res || !resource_size(cxled->dpa_res))
>  		return 0;
>  
> -	if (dpa > cxled->dpa_res->end || dpa < cxled->dpa_res->start)
> +	if (!resource_contains_addr(cxled->dpa_res, dpa))
>  		return 0;
>  
>  	/*
> @@ -2959,7 +2959,7 @@ u64 cxl_dpa_to_hpa(struct cxl_region *cxlr, const struct cxl_memdev *cxlmd,
>  	if (cxlrd->hpa_to_spa)
>  		hpa = cxlrd->hpa_to_spa(cxlrd, hpa);
>  
> -	if (hpa < p->res->start || hpa > p->res->end) {
> +	if (!resource_contains_addr(p->res, hpa)) {
>  		dev_dbg(&cxlr->dev,
>  			"Addr trans fail: hpa 0x%llx not in region\n", hpa);
>  		return ULLONG_MAX;
> @@ -3499,7 +3499,7 @@ u64 cxl_port_get_spa_cache_alias(struct cxl_port *endpoint, u64 spa)
>  	xa_for_each(&endpoint->regions, index, iter) {
>  		struct cxl_region_params *p = &iter->region->params;
>  
> -		if (p->res->start <= spa && spa <= p->res->end) {
> +		if (resource_contains_addr(p->res, spa)) {
>  			if (!p->cache_size)
>  				return ~0ULL;
>  
> -- 
> 2.34.1
> 

Re: [PATCH v3 2/3] cxl/edac: Fix wrong dpa checking for PPR operation

[Alison Schofield]

On Tue, Jul 08, 2025 at 01:15:35PM +0800, Li Ming wrote:
> DPA 0 is considered invalid in cxl_do_ppr(), but per Table 8-143. "Get
> Partition Info Output Payload" in CXL r3.2 section 8.2.10.9.2.1 "Get
> Partition Info(Opcode 4100h)", it mentions that DPA 0 is a valid address
> of a CXL device. So the correct implementation should be checking if the
> DPA is in the DPA range of the CXL device rather than checking if the
> DPA is equal to 0.
> 

If it needs a fixes tag, doesn't it also need a user visible impact
statement? I get that the PPR won't happen. What does that look like
to the user? Is there a user level error message we can add to the
commit log?

With that, you can add:
Reviewed-by: Alison Schofield <alison.schofield@intel.com>



> Fixes: be9b359e056a ("cxl/edac: Add CXL memory device soft PPR control feature")
> Signed-off-by: Li Ming <ming.li@zohomail.com>
> Tested-by: Shiju Jose <shiju.jose@huawei.com>
> Reviewed-by: Shiju Jose <shiju.jose@huawei.com>
> ---
>  drivers/cxl/core/edac.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
> index 623aaa4439c4..1cf65b1538b9 100644
> --- a/drivers/cxl/core/edac.c
> +++ b/drivers/cxl/core/edac.c
> @@ -1923,8 +1923,11 @@ static int cxl_ppr_set_nibble_mask(struct device *dev, void *drv_data,
>  static int cxl_do_ppr(struct device *dev, void *drv_data, u32 val)
>  {
>  	struct cxl_ppr_context *cxl_ppr_ctx = drv_data;
> +	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>  
> -	if (!cxl_ppr_ctx->dpa || val != EDAC_DO_MEM_REPAIR)
> +	if (!resource_contains_addr(&cxlds->dpa_res, cxl_ppr_ctx->dpa) ||
> +	    val != EDAC_DO_MEM_REPAIR)
>  		return -EINVAL;
>  
>  	return cxl_mem_perform_ppr(cxl_ppr_ctx);
> -- 
> 2.34.1
> 

Re: [PATCH v3 1/3] cxl/core: Introduce a new helper resource_contains_addr()

[Li Ming]

On 7/8/2025 9:20 PM, Andy Shevchenko wrote:
> On Tue, Jul 08, 2025 at 01:15:34PM +0800, Li Ming wrote:
>> In CXL subsystem, many functions need to check an address availability
>> by checking if the resource range contains the address. Providing a new
>> helper function resource_contains_addr() to check if the resource range
>> contains the input address.
> ...
>
>> +bool resource_contains_addr(const struct resource *res, const resource_size_t addr);
> Right, the problem is that it collides with the resource namespace. Please, add a prefix.
>
> bool cxl_resource_contains_addr(const struct resource *res, const resource_size_t addr);
>
> ...
>
>> +bool resource_contains_addr(const struct resource *res, const resource_size_t addr)
>> +{
>> +	if (res->flags & IORESOURCE_MEM)
> resource_type() ?
>
>> +		return res->start <= addr && addr <= res->end;
>> +
>> +	return false;
> I still think using DEFINE_RES_MEM() with resource_contains() is a better
> approach.

Will do all you mentioned, thanks


Ming

Re: [PATCH v3 2/3] cxl/edac: Fix wrong dpa checking for PPR operation

[Li Ming]

On 7/9/2025 9:42 AM, Alison Schofield wrote:
> On Tue, Jul 08, 2025 at 01:15:35PM +0800, Li Ming wrote:
>> DPA 0 is considered invalid in cxl_do_ppr(), but per Table 8-143. "Get
>> Partition Info Output Payload" in CXL r3.2 section 8.2.10.9.2.1 "Get
>> Partition Info(Opcode 4100h)", it mentions that DPA 0 is a valid address
>> of a CXL device. So the correct implementation should be checking if the
>> DPA is in the DPA range of the CXL device rather than checking if the
>> DPA is equal to 0.
>>
> If it needs a fixes tag, doesn't it also need a user visible impact
> statement? I get that the PPR won't happen. What does that look like
> to the user? Is there a user level error message we can add to the
> commit log?
>
> With that, you can add:
> Reviewed-by: Alison Schofield <alison.schofield@intel.com>
>
Will do. Thanks.


Ming

>
>> Fixes: be9b359e056a ("cxl/edac: Add CXL memory device soft PPR control feature")
>> Signed-off-by: Li Ming <ming.li@zohomail.com>
>> Tested-by: Shiju Jose <shiju.jose@huawei.com>
>> Reviewed-by: Shiju Jose <shiju.jose@huawei.com>
>> ---
>>  drivers/cxl/core/edac.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
>> index 623aaa4439c4..1cf65b1538b9 100644
>> --- a/drivers/cxl/core/edac.c
>> +++ b/drivers/cxl/core/edac.c
>> @@ -1923,8 +1923,11 @@ static int cxl_ppr_set_nibble_mask(struct device *dev, void *drv_data,
>>  static int cxl_do_ppr(struct device *dev, void *drv_data, u32 val)
>>  {
>>  	struct cxl_ppr_context *cxl_ppr_ctx = drv_data;
>> +	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
>> +	struct cxl_dev_state *cxlds = cxlmd->cxlds;
>>  
>> -	if (!cxl_ppr_ctx->dpa || val != EDAC_DO_MEM_REPAIR)
>> +	if (!resource_contains_addr(&cxlds->dpa_res, cxl_ppr_ctx->dpa) ||
>> +	    val != EDAC_DO_MEM_REPAIR)
>>  		return -EINVAL;
>>  
>>  	return cxl_mem_perform_ppr(cxl_ppr_ctx);
>> -- 
>> 2.34.1
>>