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

[PATCH 1/2] 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>
---
base-commit: 0a46f60a9fe16f5596b6b4b3ee1a483ea7854136 cxl/fixes
---
 drivers/cxl/core/edac.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index 623aaa4439c4..11eaa2eea642 100644
--- a/drivers/cxl/core/edac.c
+++ b/drivers/cxl/core/edac.c
@@ -1923,8 +1923,12 @@ 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 (cxl_ppr_ctx->dpa < cxlds->dpa_res.start ||
+	    cxl_ppr_ctx->dpa > cxlds->dpa_res.end ||
+	    val != EDAC_DO_MEM_REPAIR)
 		return -EINVAL;
 
 	return cxl_mem_perform_ppr(cxl_ppr_ctx);
-- 
2.34.1

[PATCH 2/2] cxl/core: Add a helper function to check dpa availability

[Li Ming]

In CXL subsystem, many pieces of code that need to check dpa
availability in case using wrong dpa. Add a helper function called
is_valid_dpa() for dpa availability checking.

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

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 29b61828a847..38d619df4772 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -136,4 +136,5 @@ int cxl_set_feature(struct cxl_mailbox *cxl_mbox, const uuid_t *feat_uuid,
 		    u16 *return_code);
 #endif
 
+bool is_valid_dpa(struct resource *res, u64 dpa);
 #endif /* __CXL_CORE_H__ */
diff --git a/drivers/cxl/core/edac.c b/drivers/cxl/core/edac.c
index 11eaa2eea642..469be1ecb703 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 (!is_valid_dpa(&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 (!is_valid_dpa(&cxlds->dpa_res, dpa))
 		return -EINVAL;
 
 	cxl_ppr_ctx->dpa = dpa;
@@ -1926,8 +1926,7 @@ static int cxl_do_ppr(struct device *dev, void *drv_data, u32 val)
 	struct cxl_memdev *cxlmd = cxl_ppr_ctx->cxlmd;
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 
-	if (cxl_ppr_ctx->dpa < cxlds->dpa_res.start ||
-	    cxl_ppr_ctx->dpa > cxlds->dpa_res.end ||
+	if (!is_valid_dpa(&cxlds->dpa_res, cxl_ppr_ctx->dpa) ||
 	    val != EDAC_DO_MEM_REPAIR)
 		return -EINVAL;
 
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index ab1007495f6b..3dc9215ea94d 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -547,6 +547,11 @@ resource_size_t cxl_dpa_resource_start(struct cxl_endpoint_decoder *cxled)
 	return base;
 }
 
+bool is_valid_dpa(struct resource *res, u64 dpa)
+{
+	return (dpa >= res->start && dpa <= res->end);
+}
+
 int cxl_dpa_free(struct cxl_endpoint_decoder *cxled)
 {
 	struct cxl_port *port = cxled_to_port(cxled);
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index f88a13adf7fa..46ea0536221c 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 (!is_valid_dpa(&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..994ca02b2b48 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 (!is_valid_dpa(cxled->dpa_res, dpa))
 		return 0;
 
 	/*
-- 
2.34.1

Re: [PATCH 2/2] cxl/core: Add a helper function to check dpa availability

[Alison Schofield]

On Sun, Jun 29, 2025 at 04:20:42PM +0800, Li Ming wrote:
> In CXL subsystem, many pieces of code that need to check dpa
> availability in case using wrong dpa. Add a helper function called
> is_valid_dpa() for dpa availability checking.

Does this have anything DPA special about it. Could it be done in
in a new helper like resource_contains_addr() for all to employ.
In CXL, we'd use it in at least one more place, checking for HPA
in a region resource. 

-- Alison


> 
snip

> +bool is_valid_dpa(struct resource *res, u64 dpa)
> +{
> +	return (dpa >= res->start && dpa <= res->end);
> +}
> +

snip

> 

Re: [PATCH 2/2] cxl/core: Add a helper function to check dpa availability

[Li Ming]

On 6/30/2025 9:41 AM, Alison Schofield wrote:
> On Sun, Jun 29, 2025 at 04:20:42PM +0800, Li Ming wrote:
>> In CXL subsystem, many pieces of code that need to check dpa
>> availability in case using wrong dpa. Add a helper function called
>> is_valid_dpa() for dpa availability checking.
> Does this have anything DPA special about it. Could it be done in
I think no special DPA here, my understanding is that all DPA should be included in cxlds->dpa_res.
> in a new helper like resource_contains_addr() for all to employ.
> In CXL, we'd use it in at least one more place, checking for HPA
> in a region resource. 
That is a good idea, I will add it into include/linux/ioport.h
>
> -- Alison
>

Ming

[snip]