[PATCH v3] cxl/pci: Skip reset detection for DVSEC emulated decoders

After an FLR or an SBR, cxl_reset_done() walks the endpoint's decoders and
asks __cxl_endpoint_decoder_reset_detected() whether any of them lost its
committed state. That helper filters on CXL_DECODER_F_ENABLE and then
samples the Committed bit in the HDM decoder control register at
cxlhdm->regs.hdm_decoder.

The HDM decoder registers are not always where an enabled decoder's state
lives. A memory device may describe its ranges through the CXL DVSEC range
registers instead, and should_emulate_decoders() picks that path in two
situations: when the component registers expose no HDM decoder capability
at all, and when the capability exists but firmware left Mem_Enable set
with the global HDM decoder enable bit clear.
cxl_setup_hdm_decoder_from_dvsec() publishes the emulated decoders with
CXL_DECODER_F_ENABLE and CXL_DECODER_F_LOCK set, so they pass the filter,
and it leaves cxld->commit NULL because there is no register to commit to.

In the first situation regs.hdm_decoder is NULL and the readl() oopses in
the PCI reset completion path. In the second the pointer is valid but the
registers are unused, so the Committed bit reads zero and the helper
reports a reset that did not happen: cxl_reset_done() prints two dev_crit
lines about an SBR wiping active decoders, taints the kernel, and strips
CXL_DECODER_F_ENABLE and CXL_DECODER_F_LOCK from every endpoint decoder.
Losing the lock flag is the more consequential half of that, because it is
what keeps the emulated ranges from being treated as reprogrammable while
the driver still cannot change the range registers at run time.

Skip the check when cxld->commit is NULL. Only
cxl_setup_hdm_decoder_from_dvsec() leaves that callback unset, and
init_hdm_decoder() installs cxl_decoder_commit() on every decoder that
does come from the registers, so one test covers both emulation paths and
no separate test for the NULL register pointer is needed. A range
described by the DVSEC registers has no Committed bit for a reset to
clear, so there is nothing here for the post-reset warning to observe.

Fixes: 934edcd436dc ("cxl: Add post-reset warning if reset results in loss of previously committed HDM decoders")
Signed-off-by: Guixin Liu <[email protected]>
---
This was patch 4/8 of the "cxl: Assorted fixes" series [1]. Per review
feedback that series is not being reworked as a whole; the fixes are resent
individually instead.

v2 tested cxlhdm->regs.hdm_decoder for NULL. That test turns out to be
subsumed by the commit callback test rather than complementary to it: on
the endpoint path info is never NULL, since
devm_cxl_endpoint_decoders_setup() passes the address of an on-stack struct
to both devm_cxl_setup_hdm() and devm_cxl_enumerate_decoders(). A NULL
regs.hdm_decoder can therefore only come from the "no component registers"
early return in devm_cxl_setup_hdm(), which requires info->mem_enabled,
and should_emulate_decoders() then emulates every decoder on the port.
Keeping both tests would leave one that cannot be reached today, so only
the commit test is here - say the word if you would rather have the NULL
test back as a guard on that invariant.

The Sashiko review bot raised two further pre-existing concerns on v2 that
this patch does not address, since both are about the reset handler's
synchronisation rather than about which registers it reads:

- cxl_reset_done() holds only the memdev device lock, so nothing excludes
  an unbind of the cxl_port driver from the endpoint port, which frees the
  devm allocated cxl_hdm while dev_get_drvdata(&port->dev) keeps returning
  it. An unbind that has completed is harmless: devres frees the decoders
  before the cxl_hdm allocation that predates them, so the child walk finds
  nothing to look at. What is left is the interleaving where the walk has
  already taken its reference on a decoder, which keeps that device alive
  past device_del(), and the unbind reaches the cxl_hdm free first. Narrow,
  and the fix is not local: it means deciding how the PCI error handlers
  should exclude the port driver's binding.

- cxld->flags is updated with a plain read-modify-write in
  cxl_endpoint_decoder_clear_reset_flags(), while cxl_decoder_commit() and
  cxl_decoder_reset() update the same word under cxl_rwsem.region, which
  __commit() and the region reset paths hold across those calls. Having the
  reset handler take that rwsem too looks like the natural fix, but it
  already holds the memdev device lock at that point, so the lock ordering
  wants review first.

Both look worth doing on their own; happy to follow up with separate
patches if that is the preference.

v1->v2:
- rebase onto cxl/next
- rewrite the commit message to describe the behaviour rather than narrate
  the code change (Alison Schofield)

v2->v3:
- test cxld->commit instead of cxlhdm->regs.hdm_decoder, so that decoders
  emulated from the DVSEC ranges are also skipped when the HDM decoder
  registers exist but are globally disabled (Richard Cheng)
- update the subject and the commit message for the widened scope

[1] https://lore.kernel.org/linux-cxl/[email protected]/

 drivers/cxl/core/pci.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
index 9d807c1a002c..d8b07f86bab0 100644
--- a/drivers/cxl/core/pci.c
+++ b/drivers/cxl/core/pci.c
@@ -683,6 +683,13 @@ static int __cxl_endpoint_decoder_reset_detected(struct device *dev, void *data)
 	if ((cxld->flags & CXL_DECODER_F_ENABLE) == 0)
 		return 0;
 
+	/*
+	 * Decoders emulated from the DVSEC range registers have no commit
+	 * callback and no HDM decoder registers to consult.
+	 */
+	if (!cxld->commit)
+		return 0;
+
 	cxlhdm = dev_get_drvdata(&port->dev);
 	hdm = cxlhdm->regs.hdm_decoder;
 	ctrl = readl(hdm + CXL_HDM_DECODER0_CTRL_OFFSET(cxld->id));

base-commit: 7098e9cd98a05c0c5de2fae0c2465f9d966fdd07
-- 
2.43.7