Linux CXL
 help / color / mirror / Atom feed
* [PATCH v3 0/2] Support zero-sized HDM decoders
@ 2026-06-07  5:38 Richard Cheng
  2026-06-07  5:38 ` [PATCH v3 1/2] cxl/hdm: Allow zero sized " Richard Cheng
  2026-06-07  5:38 ` [PATCH v3 2/2] tools/testing/cxl: Enable zero sized decoder under hb0 Richard Cheng
  0 siblings, 2 replies; 5+ messages in thread
From: Richard Cheng @ 2026-06-07  5:38 UTC (permalink / raw)
  To: dave, jonathan.cameron, dave.jiang, alison.schofield,
	vishal.l.verma, ira.weiny, dan.j.williams
  Cc: linux-cxl, linux-kernel, newtonl, kristinc, kaihengf, kobak,
	vaslot, smadhavan, Richard Cheng

Hello,

This v3 picks up Vishal Aslot's "Support zero-sized decoders" series [1]
and addresses the v2 review feedback [2].

CXL r3.2 §8.2.4.20.12 and §14.13.10 permit committing an HDM decoder with
size 0. BIOS commits and LOCKs such decoders to burn slots so the OS
cannot program new regions through them, e.g. for a Type 3 device in a
Trusted Computing Base (TCB) established via the Trusted Security Protocol
enumeration and aborts the whole port, so affected systems show nothing
under 'cxl list'.

Patch 1 enumerates the committed zero-size decoder into topology with its
HW-reported LOCK state, and resolves the two v2 review issues:

  - hdm_end (sashiko-bot, Alison): the zero-size decoder skips
    devm_cxl_dpa_reserve(), which left port->hdm_end un-incremented, so
    the next committed decoder failed the in-order check in
    __cxl_dpa_reserve() with -EBUSY. hdm_end is now advanced.

  - poison (Alison): commit_end can now reference a decoder with no DPA
    resource. poison_by_decoder() returned before the commit_end check,
    so cxl_get_poison_unmapped() never ran and unmapped-tail poison was
    lost (cxl-poison: 4 expected, 2 found). The fix is folded into
injection behind a new mock_zero_size_decoders module parameter
(default off): the shared topology is unchanged, and enabling it
reproduces the BIOS layout (auto-region at decoder[0], zero-size+locked
above at commit_end).

Patch 2 adds cxl_test coverage. v2 committed the mock zero-size slots
unconditionally on the host-bridge0 auto-region endpoints, which the
region tests reuse, regressing 7 of 17 cxl unit tests. v3 gates the
injection behind a new mock_zero_size_decoders module param (default off).
The shared topology is unchanged, and enabling it reproduces the BIOS
layout.

Tested with the ndctl cxl unit suite: param off, no regressions; param
on, cxl-poison's by-memdev-by-dpa case returns all 4 records (2 without
patch 1's poison fix).

The result is in the following:
"""
$ sudo env "PATH=$PATH" meson test -C build --suite cxl \
> --num-processes 1 -t 6 \
> --print-errorlogs
ninja: Entering directory `/home/nvidia/ndctl/build'
[1/50] Generating version.h with a custom command
 1/14 ndctl:cxl / cxl-topology.sh               OK                3.38s
 2/14 ndctl:cxl / cxl-region-sysfs.sh           OK                2.60s
 3/14 ndctl:cxl / cxl-labels.sh                 OK                2.53s
 4/14 ndctl:cxl / cxl-create-region.sh          OK                3.25s
 5/14 ndctl:cxl / cxl-xor-region.sh             OK                2.61s
 6/14 ndctl:cxl / cxl-events.sh                 OK                2.46s
 7/14 ndctl:cxl / cxl-sanitize.sh               OK                5.42s
 8/14 ndctl:cxl / cxl-destroy-region.sh         OK                2.47s
 9/14 ndctl:cxl / cxl-qos-class.sh              OK                2.54s
10/14 ndctl:cxl / cxl-translate.sh              OK                0.78s
11/14 ndctl:cxl / cxl-elc.sh                    OK                2.45s
12/14 ndctl:cxl / cxl-security.sh               SKIP              0.02s   exit status 77
13/14 ndctl:cxl / cxl-features.sh               OK                1.27s
14/14 ndctl:cxl / cxl-poison.sh                 OK                7.91s

Ok:                 13  
Expected Fail:      0   
Fail:               0   
Unexpected Pass:    0   
Skipped:            1   
Timeout:            0   

Full log written to /home/nvidia/ndctl/build/meson-logs/testlog.txt
"""

And the tests Alison mentioned.
"""
$ sudo env "PATH=$PATH" meson test -C build --num-processes 1 -t 6 --print-errorlogs \
> cxl-region-sysfs.sh cxl-create-region.sh cxl-xor-region.sh \
    cxl-destroy-region.sh cxl-qos-class.sh cxl-poison.sh
ninja: Entering directory `/home/nvidia/ndctl/build'
[1/50] Generating version.h with a custom command
1/6 ndctl:cxl / cxl-region-sysfs.sh          OK                2.62s
2/6 ndctl:cxl / cxl-create-region.sh         OK                3.15s
3/6 ndctl:cxl / cxl-xor-region.sh            OK                2.61s
4/6 ndctl:cxl / cxl-destroy-region.sh        OK                2.39s
5/6 ndctl:cxl / cxl-qos-class.sh             OK                2.47s
6/6 ndctl:cxl / cxl-poison.sh                OK                7.80s

Ok:                 6   
Expected Fail:      0   
Fail:               0   
Unexpected Pass:    0   
Skipped:            0   
Timeout:            0   

Full log written to /home/nvidia/ndctl/build/meson-logs/testlog.txt
"""

[1]
https://lore.kernel.org/all/20251015024019.1189713-1-vaslot@nvidia.com/T/#u
[2] https://lore.kernel.org/all/cover.1779957270.git.icheng@nvidia.com/

Richard Cheng (2):
  cxl/hdm: Allow zero sized HDM decoders
  tools/testing/cxl: Enable zero sized decoder under hb0

 drivers/cxl/core/hdm.c       | 25 ++++++++---
 drivers/cxl/core/region.c    | 42 +++++++++---------
 tools/testing/cxl/test/cxl.c | 83 +++++++++++++++++++++++++++++++++---
 3 files changed, 117 insertions(+), 33 deletions(-)


base-commit: ddd664bbff63e09e7a7f9acae9c43605d4cf185f
-- 
2.43.0


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v3 1/2] cxl/hdm: Allow zero sized HDM decoders
  2026-06-07  5:38 [PATCH v3 0/2] Support zero-sized HDM decoders Richard Cheng
@ 2026-06-07  5:38 ` Richard Cheng
  2026-06-07  5:51   ` sashiko-bot
  2026-06-07  5:38 ` [PATCH v3 2/2] tools/testing/cxl: Enable zero sized decoder under hb0 Richard Cheng
  1 sibling, 1 reply; 5+ messages in thread
From: Richard Cheng @ 2026-06-07  5:38 UTC (permalink / raw)
  To: dave, jonathan.cameron, dave.jiang, alison.schofield,
	vishal.l.verma, ira.weiny, dan.j.williams
  Cc: linux-cxl, linux-kernel, newtonl, kristinc, kaihengf, kobak,
	vaslot, smadhavan, Richard Cheng

CXL r3.2 8.2.4.20.12 and 14.13.10 permit committing on HDM decoder with
size 0. BIOS commits and locks such decoders to burn slots for a Type 3
device in a TSP-established TCB. init_hdm_decoder() rejected them with
-ENXIO and aborted port enumeration, so "cxl list" showed nothing.

Enumerate the decoder with its HW LOCK state instead. Skip the DPA
reservation it does not need, but advance port->hdm_end so a following
committed decoder still passes the in-order check in
__cxl_dpa_reserve(). commit_end can now point at a decoder with no DPA
resource, so poison_by_decoder() must still scan the unmapped DPA tail.

Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
Changelog:

v2->v3:
      - Advance port->hdm_end for the committed zero-size decoder so a
        following committed decoder still passes the in-order check in
        __cxl_dpa_reserve() .
      - commit_end may now reference a zero-size decoder with no DPA
        resource, so poison_by_decoder() falls through to run
        cxl_get_poison_unmapped() and scan the unmapped DPA tail.

v1->v2:
      - Add zero-size committed decoders to the topology instead of
        skipping them. Drop v1's -ENOSPC sentinel and the matching
        "continue" in devm_cxl_enumerate_decoders(); fall through so
        add_hdm_decoder() registers the decoder.
      - Set port->commit_end unconditionally for any committed decoder,
        not only non-zero-size ones, so subsequent decoders satisfy the
        out-of-order check.
      - Add an explicit early-return before devm_cxl_dpa_reserve() in the
        endpoint-decoder path. __cxl_dpa_reserve() rejects zero-size
        decoders.
      - Spell out TSP and TCB and cite spec sections in commit message.
      - Reorder series, implementation first.
---
 drivers/cxl/core/hdm.c    | 25 +++++++++++++++++------
 drivers/cxl/core/region.c | 42 +++++++++++++++++++--------------------
 2 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..b5fe2b053bc2 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1031,13 +1031,17 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
 			return -ENXIO;
 		}
 
-		if (size == 0) {
-			dev_warn(&port->dev,
-				 "decoder%d.%d: Committed with zero size\n",
-				 port->id, cxld->id);
-			return -ENXIO;
-		}
 		port->commit_end = cxld->id;
+
+		/*
+		 * CXL r3.2 8.2.4.20.12 permits committing an HDM decoder with
+		 * size 0. Enumerate it into the topology with its HW-reported
+		 * LOCK state instead of aborting the port.
+		 */
+		if (size == 0)
+			dev_dbg(&port->dev,
+				"decoder%d.%d: Committed with zero size\n",
+				port->id, cxld->id);
 	} else {
 		if (cxled) {
 			struct cxl_memdev *cxlmd = cxled_to_memdev(cxled);
@@ -1096,6 +1100,15 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
 	if (!committed)
 		return 0;
 
+	/*
+	 * A committed zero-size decoder reserves no DPA, but still advance
+	 * the port's DPA watermark.
+	 */
+	if (size == 0) {
+		port->hdm_end = cxld->id;
+		return 0;
+	}
+
 	dpa_size = div_u64_rem(size, cxld->interleave_ways, &remainder);
 	if (remainder) {
 		dev_err(&port->dev,
diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
index e50dc716d4e8..a353d8e7489d 100644
--- a/drivers/cxl/core/region.c
+++ b/drivers/cxl/core/region.c
@@ -2907,38 +2907,38 @@ static int poison_by_decoder(struct device *dev, void *arg)
 		return rc;
 
 	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;
+	if (cxled->dpa_res) {
+		cxlmd = cxled_to_memdev(cxled);
+		cxlds = cxlmd->cxlds;
+		mode = cxlds->part[cxled->part].mode;
+
+		if (cxled->skip) {
+			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)
+				rc = 0;
+			if (rc)
+				return rc;
+		}
 
-	if (cxled->skip) {
-		offset = cxled->dpa_res->start - cxled->skip;
-		length = cxled->skip;
-		rc = cxl_mem_get_poison(cxlmd, offset, length, NULL);
+		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)
 			rc = 0;
 		if (rc)
 			return rc;
-	}
 
-	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)
-		rc = 0;
-	if (rc)
-		return rc;
-
-	/* 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;
 	}
 
+	/* Iterate until commit_end is reached */
+	if (cxled->cxld.id == ctx->port->commit_end)
+		return 1;
+
 	return 0;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v3 2/2] tools/testing/cxl: Enable zero sized decoder under hb0
  2026-06-07  5:38 [PATCH v3 0/2] Support zero-sized HDM decoders Richard Cheng
  2026-06-07  5:38 ` [PATCH v3 1/2] cxl/hdm: Allow zero sized " Richard Cheng
@ 2026-06-07  5:38 ` Richard Cheng
  1 sibling, 0 replies; 5+ messages in thread
From: Richard Cheng @ 2026-06-07  5:38 UTC (permalink / raw)
  To: dave, jonathan.cameron, dave.jiang, alison.schofield,
	vishal.l.verma, ira.weiny, dan.j.williams
  Cc: linux-cxl, linux-kernel, newtonl, kristinc, kaihengf, kobak,
	vaslot, smadhavan, Richard Cheng

The kernel now allows committed HDM decoders of zero size so BIOS can
burn slots with LOCK, cxl_test needs to exercise the patch.

Add a mock_zero_size_decoders module param, When set, the special
endpoint under host-bridge0 (cxl_mem.0 and cxl_mem.4) commit decoders 1
and 2 as zero-size + locked above the decoder[0] auto-region, mirrored
on the parent switch and host bridge. commit_end then lands on a decoder
with no DPA resource, exercising the new enumeration and
poison-by-endpoint paths.

Signed-off-by: Vishal Aslot <vaslot@nvidia.com>
Signed-off-by: Richard Cheng <icheng@nvidia.com>
---
v2->v3:
      - Gate the zero-size + locked decoder injection behind a new
        mock_zero_size_decoders module parameter (default off). v2 applied
        it unconditionally on the host-bridge0 auto-region endpoints, which
        the region test suite reuses, regressing 7 of 17 cxl unit tests;
        defaulting off leaves the shared topology untouched.

v1->v2:
      - Replace second_decoder(), third_decoder() with a single
        match_decoder_by_index() helper, so all lookups share one matcher.
      - Use DEFINE_RANGE() for the empty range instead of an open-coded
        struct.
      - Set cxled->state = CXL_DECODER_STATE_MANUAL rather than STATE_AUTO.
      - Set CXL_DECODER_F_LOCK on the mock zero-size decoders to model the
        BIOS-burns-slots case.
---
 tools/testing/cxl/test/cxl.c | 83 +++++++++++++++++++++++++++++++++---
 1 file changed, 77 insertions(+), 6 deletions(-)

diff --git a/tools/testing/cxl/test/cxl.c b/tools/testing/cxl/test/cxl.c
index 296516eecfd6..190cb18d6932 100644
--- a/tools/testing/cxl/test/cxl.c
+++ b/tools/testing/cxl/test/cxl.c
@@ -17,6 +17,7 @@
 static int interleave_arithmetic;
 static bool extended_linear_cache;
 static bool fail_autoassemble;
+static bool mock_zero_size_decoders;
 
 #define FAKE_QTG_ID	42
 
@@ -1041,16 +1042,47 @@ static void default_mock_decoder(struct cxl_decoder *cxld)
 	WARN_ON_ONCE(!cxld_registry_new(cxld));
 }
 
-static int first_decoder(struct device *dev, const void *data)
+static int match_decoder_by_index(struct device *dev, const void *data)
 {
+	int target_id = *(const int *)data;
 	struct cxl_decoder *cxld;
 
 	if (!is_switch_decoder(dev))
 		return 0;
 	cxld = to_cxl_decoder(dev);
-	if (cxld->id == 0)
-		return 1;
-	return 0;
+	return cxld->id == target_id;
+}
+
+/*
+ * Mock a BIOS-burnt slot: a committed, locked, zero-size decoder
+ * (CXL r3.2 8.2.4.20.12). Gated by the mock_zero_size_decoders module
+ * param so the default cxl_test topology, shared by the region test
+ * suite, is left undisturbed.
+ */
+static void size_zero_mock_decoder_ep(struct cxl_decoder *cxld, u64 base)
+{
+	struct cxl_endpoint_decoder *cxled = to_cxl_endpoint_decoder(&cxld->dev);
+
+	cxld->hpa_range = DEFINE_RANGE(base, base - 1);
+	cxld->interleave_ways = 2;
+	cxld->interleave_granularity = 4096;
+	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
+	cxld->flags = CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK;
+	cxled->state = CXL_DECODER_STATE_MANUAL;
+	cxld->commit = mock_decoder_commit;
+	cxld->reset = mock_decoder_reset;
+}
+
+static void size_zero_mock_decoder_sw(struct cxl_decoder *cxld, u64 base,
+				      int level)
+{
+	cxld->flags = CXL_DECODER_F_ENABLE | CXL_DECODER_F_LOCK;
+	cxld->target_type = CXL_DECODER_HOSTONLYMEM;
+	cxld->interleave_ways = level == 0 ? 2 : 1;
+	cxld->interleave_granularity = 4096;
+	cxld->hpa_range = DEFINE_RANGE(base, base - 1);
+	cxld->commit = mock_decoder_commit;
+	cxld->reset = mock_decoder_reset;
 }
 
 /*
@@ -1131,7 +1163,7 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	 * See 'cxl list -BMPu -m cxl_mem.0,cxl_mem.4'
 	 */
 	if (!is_endpoint_decoder(&cxld->dev) || !hb0 || pdev->id % 4 ||
-	    pdev->id > 4 || cxld->id > 0) {
+	    pdev->id > 4 || cxld->id > (mock_zero_size_decoders ? 2 : 0)) {
 		default_mock_decoder(cxld);
 		return false;
 	}
@@ -1145,6 +1177,20 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	base = window->base_hpa;
 	if (extended_linear_cache)
 		base += mock_auto_region_size;
+
+	/*
+	 * With mock_zero_size_decoders, decoders 1 and 2 of the special
+	 * endpoints mock BIOS-burnt zero-size + locked slots above the
+	 * decoder[0] auto-region (CXL r3.2 8.2.4.20.12). commit_end then
+	 * points at a decoder with no DPA resource, exercising the
+	 * zero-size enumeration and poison-by-endpoint code paths.
+	 */
+	if (cxld->id == 1 || cxld->id == 2) {
+		size_zero_mock_decoder_ep(cxld, base);
+		port->commit_end = cxld->id;
+		WARN_ON_ONCE(!cxld_registry_new(cxld));
+		return false;
+	}
 	cxld->hpa_range = (struct range) {
 		.start = base,
 		.end = base + mock_auto_region_size - 1,
@@ -1168,9 +1214,11 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 	 */
 	iter = port;
 	for (i = 0; i < 2; i++) {
+		int id = 0;
+
 		dport = iter->parent_dport;
 		iter = dport->port;
-		dev = device_find_child(&iter->dev, NULL, first_decoder);
+		dev = device_find_child(&iter->dev, &id, match_decoder_by_index);
 		/*
 		 * Ancestor ports are guaranteed to be enumerated before
 		 * @port, and all ports have at least one decoder.
@@ -1214,6 +1262,26 @@ static bool mock_init_hdm_decoder(struct cxl_decoder *cxld)
 
 		cxld_registry_update(cxld);
 		put_device(dev);
+
+		if (!mock_zero_size_decoders)
+			continue;
+
+		/*
+		 * Mirror the endpoint: commit the next two switch decoders
+		 * as zero-size + locked so the burnt-slot layout extends
+		 * end-to-end through the switch and host bridge.
+		 */
+		for (id = 1; id <= 2; id++) {
+			dev = device_find_child(&iter->dev, &id,
+						match_decoder_by_index);
+			if (WARN_ON(!dev))
+				continue;
+			cxld = to_cxl_decoder(dev);
+			size_zero_mock_decoder_sw(cxld, base, i);
+			iter->commit_end = id;
+			cxld_registry_update(cxld);
+			put_device(dev);
+		}
 	}
 
 	return false;
@@ -2030,6 +2098,9 @@ module_param(extended_linear_cache, bool, 0444);
 MODULE_PARM_DESC(extended_linear_cache, "Enable extended linear cache support");
 module_param(fail_autoassemble, bool, 0444);
 MODULE_PARM_DESC(fail_autoassemble, "Simulate missing member of an auto-region");
+module_param(mock_zero_size_decoders, bool, 0444);
+MODULE_PARM_DESC(mock_zero_size_decoders,
+		 "Mock BIOS-burnt committed zero-size locked decoders under host-bridge0");
 module_init(cxl_test_init);
 module_exit(cxl_test_exit);
 MODULE_LICENSE("GPL v2");
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] cxl/hdm: Allow zero sized HDM decoders
  2026-06-07  5:38 ` [PATCH v3 1/2] cxl/hdm: Allow zero sized " Richard Cheng
@ 2026-06-07  5:51   ` sashiko-bot
  2026-06-07  8:16     ` Richard Cheng
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-06-07  5:51 UTC (permalink / raw)
  To: Richard Cheng; +Cc: linux-cxl

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unbalanced lifecycle operation: manually advancing `port->hdm_end` for a zero-size decoder permanently breaks DPA resource freeing for all preceding decoders.
- [High] The patch modifies the port's DPA watermark (`port->hdm_end`) for zero-sized decoders without acquiring the required `cxl_rwsem.dpa` lock.
--

commit 9ef90cd8aa01ad12f98b0af61f61ae8863b14582
Author: Richard Cheng <icheng@nvidia.com>

cxl/hdm: Allow zero sized HDM decoders

This patch permits committing an HDM decoder with a size of 0. It enumerates
the decoder into the topology and manually advances the port's DPA watermark
to pass in-order checks without reserving actual DPA space.

> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
[ ... ]
> @@ -1096,6 +1100,15 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  	if (!committed)
>  		return 0;
>  
> +	/*
> +	 * A committed zero-size decoder reserves no DPA, but still advance
> +	 * the port's DPA watermark.
> +	 */
> +	if (size == 0) {
> +		port->hdm_end = cxld->id;

[Severity: High]
Does modifying port->hdm_end here require the cxl_rwsem.dpa lock?

It appears other reads and writes to port->hdm_end are protected by the DPA
read-write semaphore. Since devm_cxl_enumerate_decoders() iteratively adds
decoders and exposes them to sysfs before the enumeration loop finishes,
could a concurrent sysfs operation on a previously added decoder race with
this assignment?

[Severity: High]
Could advancing port->hdm_end without registering a devres cleanup action
break DPA freeing for earlier decoders?

Normally devm_cxl_dpa_reserve() registers a cleanup that manages teardown.
If we advance port->hdm_end here but cxl_dpa_free() exits early when it sees
no DPA resource:

cxl_dpa_free() {
        if (!cxled->dpa_res)
                return 0;
        ...
        if (cxled->cxld.id != port->hdm_end) {
                return -EBUSY;
        }
        ...
}

Will port->hdm_end remain permanently trapped at this zero-size decoder's
ID, preventing the strict LIFO un-provisioning of any preceding valid
decoders and breaking manual sysfs teardown?

> +		return 0;
> +	}
> +
>  	dpa_size = div_u64_rem(size, cxld->interleave_ways, &remainder);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260607053837.4389-1-icheng@nvidia.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v3 1/2] cxl/hdm: Allow zero sized HDM decoders
  2026-06-07  5:51   ` sashiko-bot
@ 2026-06-07  8:16     ` Richard Cheng
  0 siblings, 0 replies; 5+ messages in thread
From: Richard Cheng @ 2026-06-07  8:16 UTC (permalink / raw)
  To: sashiko-reviews; +Cc: linux-cxl

On Sun, Jun 07, 2026 at 05:51:00AM +0800, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Unbalanced lifecycle operation: manually advancing `port->hdm_end` for a zero-size decoder permanently breaks DPA resource freeing for all preceding decoders.
> - [High] The patch modifies the port's DPA watermark (`port->hdm_end`) for zero-sized decoders without acquiring the required `cxl_rwsem.dpa` lock.

Hello,

I've sent v4 to address these issues.

--Richard

> --
> 
> commit 9ef90cd8aa01ad12f98b0af61f61ae8863b14582
> Author: Richard Cheng <icheng@nvidia.com>
> 
> cxl/hdm: Allow zero sized HDM decoders
> 
> This patch permits committing an HDM decoder with a size of 0. It enumerates
> the decoder into the topology and manually advances the port's DPA watermark
> to pass in-order checks without reserving actual DPA space.
> 
> > diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> > --- a/drivers/cxl/core/hdm.c
> > +++ b/drivers/cxl/core/hdm.c
> [ ... ]
> > @@ -1096,6 +1100,15 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
> >  	if (!committed)
> >  		return 0;
> >  
> > +	/*
> > +	 * A committed zero-size decoder reserves no DPA, but still advance
> > +	 * the port's DPA watermark.
> > +	 */
> > +	if (size == 0) {
> > +		port->hdm_end = cxld->id;
> 
> [Severity: High]
> Does modifying port->hdm_end here require the cxl_rwsem.dpa lock?
> 
> It appears other reads and writes to port->hdm_end are protected by the DPA
> read-write semaphore. Since devm_cxl_enumerate_decoders() iteratively adds
> decoders and exposes them to sysfs before the enumeration loop finishes,
> could a concurrent sysfs operation on a previously added decoder race with
> this assignment?
> 
> [Severity: High]
> Could advancing port->hdm_end without registering a devres cleanup action
> break DPA freeing for earlier decoders?
> 
> Normally devm_cxl_dpa_reserve() registers a cleanup that manages teardown.
> If we advance port->hdm_end here but cxl_dpa_free() exits early when it sees
> no DPA resource:
> 
> cxl_dpa_free() {
>         if (!cxled->dpa_res)
>                 return 0;
>         ...
>         if (cxled->cxld.id != port->hdm_end) {
>                 return -EBUSY;
>         }
>         ...
> }
> 
> Will port->hdm_end remain permanently trapped at this zero-size decoder's
> ID, preventing the strict LIFO un-provisioning of any preceding valid
> decoders and breaking manual sysfs teardown?
> 
> > +		return 0;
> > +	}
> > +
> >  	dpa_size = div_u64_rem(size, cxld->interleave_ways, &remainder);
> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260607053837.4389-1-icheng@nvidia.com?part=1

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-06-07  8:16 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-07  5:38 [PATCH v3 0/2] Support zero-sized HDM decoders Richard Cheng
2026-06-07  5:38 ` [PATCH v3 1/2] cxl/hdm: Allow zero sized " Richard Cheng
2026-06-07  5:51   ` sashiko-bot
2026-06-07  8:16     ` Richard Cheng
2026-06-07  5:38 ` [PATCH v3 2/2] tools/testing/cxl: Enable zero sized decoder under hb0 Richard Cheng

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox