[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