[PATCH v9 0/4] cxl: Repair poison enumeration regressions

[PATCH v9 0/4] cxl: Repair poison enumeration regressions

[Richard Cheng]

This series repairs four poison enumeration regressions introduced by
commit be5cbd084027 ("cxl: Kill enum cxl_decoder_mode"). This revision
rebases the four patches from v8 [1] onto Linus's v7.3 tag and
incorporates the review feedback.

Patch 1: cxl_get_poison_unmapped() ends the partition walk when it
encounters a fully mapped partition. Continue to the next partition
instead, so unmapped poison in later partitions is still collected.

Patch 2: the unmapped scan tolerates -EFAULT for RAM partitions but
leaves the error in rc. If no later query overwrites rc, enumeration
incorrectly reports failure. Clear rc before continuing past the
tolerated error.

Patch 3: use the ctx->offset handoff from the committed-decoder walk
as the starting point for the unmapped scan. Starting after the highest
DPA allocation leaves ranges allocated to uncommitted decoders
unqueried. Resume at the committed boundary and scan later partitions
from their beginning. Also reset both the partition and offset when
falling back to scanning all partitions, as suggested by Alison.

Patch 4: cxl_get_poison_by_memdev() can overwrite an earlier partition
query failure with a later success. Stop on the first error that is
not already tolerated as a RAM -EFAULT, preserving the failure.
Introduce a shared poison_efault_forgiven() helper in core.h and use
it in the memdev and region poison paths, as suggested by Davidlohr.


Changes since v8 [1]:
- Rebase onto Linus's v7.3 tag
- Keep patches 1 and 2 functionally unchanged
- Patch 3: Reset ctx.offset and ctx.part, suggested by Alison Schofield
- Patch 4: Introduce poison_efault_forgiven(), suggested by
  Davidlohr Bueso.
  The helper was present on v8's base but absent from v7.3

Best regards,
Richard Cheng.


Richard Cheng (4):
  cxl/region: Scan all partitions for unmapped poison
  cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
  cxl/region: Start unmapped poison scan at the committed decoder
    boundary
  cxl/memdev: Don't overwrite the error from an earlier partition poison
    query

 drivers/cxl/core/core.h   |  9 +++++++++
 drivers/cxl/core/memdev.c |  8 +++-----
 drivers/cxl/core/region.c | 24 ++++++++++++++----------
 3 files changed, 26 insertions(+), 15 deletions(-)


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.43.0

[PATCH v9 1/4] cxl/region: Scan all partitions for unmapped poison

[Richard Cheng]

cxl_get_poison_unmapped() sweeps the unmapped tail of each partition
from ctx->part onward. A fully-mapped partition has no unmapped tail,
it's a normal per-partition state, but the loop treated it with break,
aborting the whole sweep and silently skipping unmapped poison in all
later partition. Use continue so a fully-mapped partition is skipped and
later partitions are still scanned.

Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Tested-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 27e63e6dab7c..b03f30c04d35 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2928,7 +2928,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 			offset = res->start;
 		length = res->end - offset + 1;
 		if (!length)
-			break;
+			continue;
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
 		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
 			continue;
-- 
2.43.0

[PATCH v9 2/4] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan

[Richard Cheng]

CXL 3.0 section 8.2.9.8.4.1 requires Get Poison List to return Invalid
Physical Address for a volatile range when the device does not support
volatile poison lists.

For example, consider a memdev with a single RAM partition and a
committed decoder covering only part of it:

  RAM partition: [ committed decoder ][ unmapped tail ]

On a device without volatile poison-list support, querying this tail
returns Invalid Physical Address, which cxl_internal_send_cmd() maps
to -EFAULT. The driver tolerates this response for RAM because volatile
poison-list support is optional.

When cxl_get_poison_unmapped() was converted to iterate over partitions,
the RAM-specific error handling became a continue without clearing rc.
If no subsequent query overwrites rc, the function returns the tolerated
-EFAULT and incorrectly reports the enumeration as failed.

Clear rc before continuing, restoring the previous handling of this
expected response for RAM partitions.

Fixes: be5cbd0840275 ("cxl: Kill enum cxl_decoder_mode")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
 drivers/cxl/core/region.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index b03f30c04d35..ddf12075b95c 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2930,8 +2930,10 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 		if (!length)
 			continue;
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
-		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
+		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM) {
+			rc = 0;
 			continue;
+		}
 		if (rc)
 			break;
 	}
-- 
2.43.0

[PATCH v9 3/4] cxl/region: Start unmapped poison scan at the committed decoder boundary

[Richard Cheng]

poison_by_decoder() stops at the last committed decoder and records the
handoff in ctx->offset, but cxl_get_poison_unmapped() ignores it and
starts after the highest DPA allocation instead. Allocations exist for
uncommitted decoders too, so their DPA is skipped by both phases and
poison there is never reported. Resume the scan at ctx->offset, and scan
later partitions in full, restoring the pre-rewrite behavior.

Also reset ctx.offset when falling back to partition 0 in
cxl_get_poison_by_endpoint(). Resetting the partition and address
together makes the starting position for the full-partition scan
explicit.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v8 -> v9:
- Rebase onto v7.3 of Linus tag
- Include the partition-0 fallback in cxl_get_poison_by_endpoint().
- Reset ctx.offset alongside ctx.part in that fallback, suggested by
  Alison Schofield
---
 drivers/cxl/core/region.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index ddf12075b95c..18114d25662b 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2907,7 +2907,6 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 {
 	struct cxl_dev_state *cxlds = cxlmd->cxlds;
 	const struct resource *res;
-	struct resource *p, *last;
 	u64 offset, length;
 	int rc = 0;
 
@@ -2920,10 +2919,8 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 	 */
 	for (int i = ctx->part; i < cxlds->nr_partitions; i++) {
 		res = &cxlds->part[i].res;
-		for (p = res->child, last = NULL; p; p = p->sibling)
-			last = p;
-		if (last)
-			offset = last->end + 1;
+		if (i == ctx->part)
+			offset = ctx->offset;
 		else
 			offset = res->start;
 		length = res->end - offset + 1;
@@ -3001,9 +2998,14 @@ int cxl_get_poison_by_endpoint(struct cxl_port *port)
 	};
 
 	rc = device_for_each_child(&port->dev, &ctx, poison_by_decoder);
-	if (rc == 1)
+	if (rc == 1) {
+		if (ctx.part < 0) {
+			ctx.part = 0;
+			ctx.offset = 0;
+		}
 		rc = cxl_get_poison_unmapped(to_cxl_memdev(port->uport_dev),
 					     &ctx);
+	}
 
 	return rc;
 }
-- 
2.43.0

[PATCH v9 4/4] cxl/memdev: Don't overwrite the error from an earlier partition poison query

[Richard Cheng]

cxl_get_poison_by_memdev() queries Get Poison List per partition but
never checks the result inside the loop, so a later partition's success
overwrites an earlier partition's failure and the whole scan reports
success while that partition's poison went unlisted. Before the loop
conversion the PMEM query returned early on error. Stop the loop on any
error not already tolerated as a RAM -EFAULT.

Introduce poison_efault_forgiven() in core.h and use it in the memdev
and region poison paths to keep the RAM -EFAULT exception in one place.

Fixes: be5cbd084027 ("cxl: Kill enum cxl_decoder_mode")
Reviewed-by: Dave Jiang <dave.jiang@intel.com>
Reviewed-by: Alison Schofield <alison.schofield@intel.com>
Reviewed-by: Jonathan Cameron <jonathan.cameron@oss.qualcomm.com>
Reviewed-by: Davidlohr Bueso <dave@stgolabs.net>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v8 -> v9:
- Rebase onto v7.3
- Introduce poison_efault_forgiven() in core.h as suggested by Davidlohr
  Bueso.
---
 drivers/cxl/core/core.h   | 9 +++++++++
 drivers/cxl/core/memdev.c | 8 +++-----
 drivers/cxl/core/region.c | 6 +++---
 3 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
index 35eaf636adc9..0a8180d7ff2a 100644
--- a/drivers/cxl/core/core.h
+++ b/drivers/cxl/core/core.h
@@ -153,6 +153,15 @@ enum poison_cmd_enabled_bits;
 bool cxl_memdev_has_poison_cmd(struct cxl_memdev *cxlmd,
 			       enum poison_cmd_enabled_bits cmd);
 
+/*
+ * Volatile poison-list support is optional. A device may return Invalid
+ * Physical Address for a RAM poison query, which maps to -EFAULT.
+ */
+static inline bool poison_efault_forgiven(int rc, enum cxl_partition_mode mode)
+{
+	return rc == -EFAULT && mode == CXL_PARTMODE_RAM;
+}
+
 long cxl_pci_get_latency(struct pci_dev *pdev);
 int cxl_pci_get_bandwidth(struct pci_dev *pdev, struct access_coordinate *c);
 int cxl_port_get_switch_dport_bandwidth(struct cxl_port *port,
diff --git a/drivers/cxl/core/memdev.c b/drivers/cxl/core/memdev.c
index b3419df586b9..9652e7d023be 100644
--- a/drivers/cxl/core/memdev.c
+++ b/drivers/cxl/core/memdev.c
@@ -225,12 +225,10 @@ static int cxl_get_poison_by_memdev(struct cxl_memdev *cxlmd)
 		offset = res->start;
 		length = resource_size(res);
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
-		/*
-		 * Invalid Physical Address is not an error for
-		 * volatile addresses. Device support is optional.
-		 */
-		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM)
+		if (poison_efault_forgiven(rc, cxlds->part[i].mode))
 			rc = 0;
+		if (rc)
+			break;
 	}
 	return rc;
 }
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index 18114d25662b..ad9e5b5f5636 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2927,7 +2927,7 @@ static int cxl_get_poison_unmapped(struct cxl_memdev *cxlmd,
 		if (!length)
 			continue;
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
-		if (rc == -EFAULT && cxlds->part[i].mode == CXL_PARTMODE_RAM) {
+		if (poison_efault_forgiven(rc, cxlds->part[i].mode)) {
 			rc = 0;
 			continue;
 		}
@@ -2963,7 +2963,7 @@ static int poison_by_decoder(struct device *dev, void *arg)
 		offset = cxled->dpa_res->start - cxled->skip;
 		length = cxled->skip;
 		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
-		if (rc == -EFAULT && mode == CXL_PARTMODE_RAM)
+		if (poison_efault_forgiven(rc, mode))
 			rc = 0;
 		if (rc)
 			return rc;
@@ -2972,7 +2972,7 @@ static int poison_by_decoder(struct device *dev, void *arg)
 	offset = cxled->dpa_res->start;
 	length = cxled->dpa_res->end - offset + 1;
 	rc = cxl_mem_get_poison(cxlmd, offset, length, cxled->cxld.region);
-	if (rc == -EFAULT && mode == CXL_PARTMODE_RAM)
+	if (poison_efault_forgiven(rc, mode))
 		rc = 0;
 	if (rc)
 		return rc;
-- 
2.43.0