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

Guixin Liu <[email protected]>
Newsgroups org.kernel.vger.linux-cxl
Message-ID <[email protected]>
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 <[email protected]>
---
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/[email protected]/

 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.