* [PATCH v9 1/4] cxl/region: Scan all partitions for unmapped poison
2026-10-20 5:13 [PATCH v9 0/4] cxl: Repair poison enumeration regressions Richard Cheng
@ 2026-10-20 5:14 ` Richard Cheng
2026-10-20 5:14 ` [PATCH v9 2/4] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Richard Cheng @ 2026-10-20 5:14 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, iweiny,
ming.li
Cc: gourry, terry.bowman, alucerop, icheng, rrichter, linux-cxl,
linux-kernel, kobak, kaihengf, newtonl, kristinc,
Jonathan Cameron
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v9 2/4] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan
2026-10-20 5:13 [PATCH v9 0/4] cxl: Repair poison enumeration regressions Richard Cheng
2026-10-20 5:14 ` [PATCH v9 1/4] cxl/region: Scan all partitions for unmapped poison Richard Cheng
@ 2026-10-20 5:14 ` Richard Cheng
2026-10-20 5:14 ` [PATCH v9 3/4] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
2026-10-20 5:14 ` [PATCH v9 4/4] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
3 siblings, 0 replies; 6+ messages in thread
From: Richard Cheng @ 2026-10-20 5:14 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, iweiny,
ming.li
Cc: gourry, terry.bowman, alucerop, icheng, rrichter, linux-cxl,
linux-kernel, kobak, kaihengf, newtonl, kristinc,
Jonathan Cameron
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v9 3/4] cxl/region: Start unmapped poison scan at the committed decoder boundary
2026-10-20 5:13 [PATCH v9 0/4] cxl: Repair poison enumeration regressions Richard Cheng
2026-10-20 5:14 ` [PATCH v9 1/4] cxl/region: Scan all partitions for unmapped poison Richard Cheng
2026-10-20 5:14 ` [PATCH v9 2/4] cxl/region: Don't leak tolerated RAM -EFAULT from unmapped poison scan Richard Cheng
@ 2026-10-20 5:14 ` Richard Cheng
2026-10-20 8:36 ` sashiko-bot
2026-10-20 5:14 ` [PATCH v9 4/4] cxl/memdev: Don't overwrite the error from an earlier partition poison query Richard Cheng
3 siblings, 1 reply; 6+ messages in thread
From: Richard Cheng @ 2026-10-20 5:14 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, iweiny,
ming.li
Cc: gourry, terry.bowman, alucerop, icheng, rrichter, linux-cxl,
linux-kernel, kobak, kaihengf, newtonl, kristinc,
Jonathan Cameron
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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v9 3/4] cxl/region: Start unmapped poison scan at the committed decoder boundary
2026-10-20 5:14 ` [PATCH v9 3/4] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
@ 2026-10-20 8:36 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-20 8:36 UTC (permalink / raw)
To: Richard Cheng; +Cc: linux-cxl
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cxl/region: broken ctx.part < 0 fallback in cxl_get_poison_by_endpoint()
--
commit 93c022ba94d2f493a95ecc17f926c1ee71ac336f
Author: Richard Cheng <icheng@nvidia.com>
cxl/region: Start unmapped poison scan at the committed decoder boundary
This commit updates cxl_get_poison_unmapped() to resume scanning from
ctx->offset on the starting partition and adds a fallback in
cxl_get_poison_by_endpoint() to reset ctx.part and ctx.offset to 0 when
ctx.part is negative.
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index ddf12075b95c0..18114d25662b3 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;
> }
[Severity: Medium]
Can this ctx.part < 0 fallback in cxl_get_poison_by_endpoint() work as
intended when gated inside the rc == 1 branch?
Looking at poison_by_decoder() in drivers/cxl/core/region.c, it only
returns 1 when reaching port->commit_end on a decoder that has
cxled->dpa_res allocated:
drivers/cxl/core/region.c:poison_by_decoder() {
...
cxled = to_cxl_endpoint_decoder(dev);
if (!cxled->dpa_res)
return rc;
cxlmd = cxled_to_memdev(cxled);
cxlds = cxlmd->cxlds;
mode = cxlds->part[cxled->part].mode;
...
offset = cxled->dpa_res->start;
length = cxled->dpa_res->end - offset + 1;
rc = cxl_mem_get_poison(cxlmd, offset, length, cxled->cxld.region);
...
/* Iterate until commit_end is reached */
if (cxled->cxld.id == ctx->port->commit_end) {
ctx->offset = cxled->dpa_res->end + 1;
ctx->part = cxled->part;
return 1;
}
}
If the decoder at commit_end has !cxled->dpa_res, poison_by_decoder()
returns 0 before checking commit_end, so device_for_each_child() returns 0
and the rc == 1 block is never entered.
Conversely, if rc == 1 is reached with ctx.part < 0, then the decoder at
commit_end had a valid cxled->dpa_res with cxled->part < 0. By that point,
poison_by_decoder() has already indexed cxlds->part[cxled->part].mode out
of bounds with a negative index, queried poison for the committed
decoders' DPA ranges via cxl_mem_get_poison(), and recorded the handoff
boundary in ctx->offset = cxled->dpa_res->end + 1.
Resetting ctx.part = 0 and ctx.offset = 0 here discards that handoff
offset and causes cxl_get_poison_unmapped() to re-scan from DPA 0, issuing
duplicate cxl_mem_get_poison() calls over the already-scanned committed
decoder DPA ranges with region set to NULL.
In addition, because cxl_get_poison_by_endpoint() is the only caller of
cxl_get_poison_unmapped(), overwriting ctx.part < 0 before the call makes
the existing check at the start of cxl_get_poison_unmapped() unreachable:
drivers/cxl/core/region.c:cxl_get_poison_unmapped() {
...
if (ctx->part < 0)
return 0;
}
Should poison_by_decoder() guard against cxled->part < 0 before indexing
cxlds->part[], and is the ctx.part < 0 fallback in
cxl_get_poison_by_endpoint() meant to handle a case where
device_for_each_child() returns 0 rather than 1?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261020051403.14186-1-icheng@nvidia.com?part=3
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v9 4/4] cxl/memdev: Don't overwrite the error from an earlier partition poison query
2026-10-20 5:13 [PATCH v9 0/4] cxl: Repair poison enumeration regressions Richard Cheng
` (2 preceding siblings ...)
2026-10-20 5:14 ` [PATCH v9 3/4] cxl/region: Start unmapped poison scan at the committed decoder boundary Richard Cheng
@ 2026-10-20 5:14 ` Richard Cheng
3 siblings, 0 replies; 6+ messages in thread
From: Richard Cheng @ 2026-10-20 5:14 UTC (permalink / raw)
To: dave, jic23, dave.jiang, alison.schofield, vishal.l.verma, iweiny,
ming.li
Cc: gourry, terry.bowman, alucerop, icheng, rrichter, linux-cxl,
linux-kernel, kobak, kaihengf, newtonl, kristinc,
Jonathan Cameron
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
^ permalink raw reply related [flat|nested] 6+ messages in thread