[PATCH] cxl/hdm: Reset the commit cursor before enumerating decoders

[PATCH] cxl/hdm: Reset the commit cursor before enumerating decoders

[Guixin Liu]

port->commit_end is the cursor that orders decoder commits. It is set to
-1 when the port is allocated and then built up by
devm_cxl_enumerate_decoders(): for each decoder found committed in
hardware, init_hdm_decoder() checks that it follows the last one and
assigns port->commit_end = cxld->id.

Nothing lowers the cursor again when an enumeration attempt fails
midway. The decoders themselves are registered with
cxl_decoder_autoremove() against the port device, so they are released
when their devres scope goes away, and their ids return to
port->decoder_ida from cxl_decoder_release(). The other cursor,
port->hdm_end, is unwound by cxl_dpa_release(). port->commit_end has no
such counterpart - commit_reap() only runs when a decoder is reset - and
the port object outlives the attempt because it belongs to the agent
that called devm_cxl_add_port(), not to the port driver.

For a switch port the retry is automatic and cannot succeed. Decoders
are set up from cxl_port_add_dport() while port->nr_dports is still 0,
inside a devres group that is released when
devm_cxl_switch_port_decoders_setup() fails. That release destroys the
decoders and returns their ids, but leaves the cursor advanced and
leaves nr_dports at 0, so the next memdev that enumerates the same
switch - devm_cxl_enumerate_ports() to find_or_add_dport() to
add_dport() - runs the setup again with a stale committed count.
init_hdm_decoder() now finds decoder0 out of order and returns -ENXIO.
From then on every dport addition fails for a reason unrelated to the
original failure, even once whatever caused that failure is gone, and no
memdev below the port can attach. An endpoint port takes the same damage
across a rebind, where devm_cxl_endpoint_decoders_setup() runs again.

The failure paths that strand the cursor are all driven by device state:
an interleave ways or granularity encoding the driver rejects, a
committed configuration whose size does not divide evenly by the ways,
or a failed DPA reservation on the DVSEC emulation path.

Reset the cursor at the start of devm_cxl_enumerate_decoders() rather
than unwinding it on each error path. Enumeration reconstructs the
commit state from hardware and has no reason to inherit a value from a
previous attempt, so one assignment covers the existing failure paths
and any added later. This matches cxl_switch_port_probe(), which
already resets port->nr_dports so that a second attempt starts from a
known state. The write takes cxl_rwsem.region because that is the lock
cxl_num_decoders_committed() asserts for reading the cursor.

Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
---
Found by the Sashiko review bot while looking at "cxl/hdm: Fix out of
bounds read of the decoder target list" [1]. This is independent of that
patch - the two touch different functions in hdm.c and apply in either
order - so it is sent on its own.

[1] https://lore.kernel.org/linux-cxl/20260812061017.56916-1-kanie@linux.alibaba.com/

 drivers/cxl/core/hdm.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index 0c80b76a5f9b..c69ec539e6d8 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -1163,6 +1163,10 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
 
 	cxl_settle_decoders(cxlhdm);
 
+	/* The commit cursor is rebuilt from hardware below */
+	scoped_guard(rwsem_write, &cxl_rwsem.region)
+		port->commit_end = -1;
+
 	for (i = 0; i < cxlhdm->decoder_count; i++) {
 		int rc, target_count = cxlhdm->target_count;
 		struct cxl_decoder *cxld;

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.7

Re: [PATCH] cxl/hdm: Reset the commit cursor before enumerating decoders

[Li Ming]

在 2026/8/12 16:25, Guixin Liu 写道:
> port->commit_end is the cursor that orders decoder commits. It is set to
> -1 when the port is allocated and then built up by
> devm_cxl_enumerate_decoders(): for each decoder found committed in
> hardware, init_hdm_decoder() checks that it follows the last one and
> assigns port->commit_end = cxld->id.
>
> Nothing lowers the cursor again when an enumeration attempt fails
> midway. The decoders themselves are registered with
> cxl_decoder_autoremove() against the port device, so they are released
> when their devres scope goes away, and their ids return to
> port->decoder_ida from cxl_decoder_release(). The other cursor,
> port->hdm_end, is unwound by cxl_dpa_release(). port->commit_end has no
> such counterpart - commit_reap() only runs when a decoder is reset - and
> the port object outlives the attempt because it belongs to the agent
> that called devm_cxl_add_port(), not to the port driver.
>
> For a switch port the retry is automatic and cannot succeed. Decoders
> are set up from cxl_port_add_dport() while port->nr_dports is still 0,
> inside a devres group that is released when
> devm_cxl_switch_port_decoders_setup() fails. That release destroys the
> decoders and returns their ids, but leaves the cursor advanced and
> leaves nr_dports at 0, so the next memdev that enumerates the same
> switch - devm_cxl_enumerate_ports() to find_or_add_dport() to
> add_dport() - runs the setup again with a stale committed count.
> init_hdm_decoder() now finds decoder0 out of order and returns -ENXIO.
>  From then on every dport addition fails for a reason unrelated to the
> original failure, even once whatever caused that failure is gone, and no
> memdev below the port can attach. An endpoint port takes the same damage
> across a rebind, where devm_cxl_endpoint_decoders_setup() runs again.
>
> The failure paths that strand the cursor are all driven by device state:
> an interleave ways or granularity encoding the driver rejects, a
> committed configuration whose size does not divide evenly by the ways,
> or a failed DPA reservation on the DVSEC emulation path.
>
> Reset the cursor at the start of devm_cxl_enumerate_decoders() rather
> than unwinding it on each error path. Enumeration reconstructs the
> commit state from hardware and has no reason to inherit a value from a
> previous attempt, so one assignment covers the existing failure paths
> and any added later. This matches cxl_switch_port_probe(), which
> already resets port->nr_dports so that a second attempt starts from a
> known state. The write takes cxl_rwsem.region because that is the lock
> cxl_num_decoders_committed() asserts for reading the cursor.
>
> Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
> Signed-off-by: Guixin Liu <kanie@linux.alibaba.com>
> ---
> Found by the Sashiko review bot while looking at "cxl/hdm: Fix out of
> bounds read of the decoder target list" [1]. This is independent of that
> patch - the two touch different functions in hdm.c and apply in either
> order - so it is sent on its own.
>
> [1] https://lore.kernel.org/linux-cxl/20260812061017.56916-1-kanie@linux.alibaba.com/
>
>   drivers/cxl/core/hdm.c | 4 ++++
>   1 file changed, 4 insertions(+)
>
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 0c80b76a5f9b..c69ec539e6d8 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -1163,6 +1163,10 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>   
>   	cxl_settle_decoders(cxlhdm);
>   
> +	/* The commit cursor is rebuilt from hardware below */
> +	scoped_guard(rwsem_write, &cxl_rwsem.region)
> +		port->commit_end = -1;
> +

Already have a same fixup from Alison.

https://lore.kernel.org/linux-cxl/cover.1786143520.git.alison.schofield@intel.com/T/#m7f9c703f0dc021191eed727c1fa285211c3e3743

>   	for (i = 0; i < cxlhdm->decoder_count; i++) {
>   		int rc, target_count = cxlhdm->target_count;
>   		struct cxl_decoder *cxld;
>
> base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07