[PATCH] cxl/mbox: Store event records for EDAC repair regardless of tracing

[Dave Jiang <dave.jiang@intel.com>]

The driver throws away media error records unless a tracepoint is enabled.

cxl_event_trace_record() stores general media and DRAM records for EDAC
repair, but both calls sit inside a block gated on the cxl_general_media
and cxl_dram tracepoints. That gate only exists to skip the DPA-to-HPA
lookup used to annotate those tracepoints. Storing a record does not need
the lookup.

With both tracepoints off, two things break. The kernel clears the device
event log either way, so the records are destroyed in hardware and kept
nowhere else. And live repair stops working altogether: sPPR and memory
sparing require the target DPA to have reported an error, no DPA ever has
a record, so every request fails with -EINVAL.

Move the two stores ahead of the tracepoint check. Keep them under the
memdev lock, which is what stops err_rec_free() from freeing
cxlmd->err_rec_array while the CPER callback runs. Leave the region and
DPA rwsems with the lookup.

Fixes: 0b5ccb0de1e2 ("cxl/edac: Support for finding memory operation attributes from the current boot")
Signed-off-by: Dave Jiang <dave.jiang@intel.com>
Assisted-by: LLM
---
Found by reviewing the gate, not from a failure report.

The documented flow is unaffected, which is why this has gone unnoticed:
rasdaemon watches the tracepoints, so they are on and the records exist
(Documentation/edac/memory_repair.rst:94-97). Offline repair is unaffected
too, since both checks sit under cxl_is_memdev_memory_online():

  edac.c:1807  sPPR     -EINVAL when neither cxl_find_rec_dram() nor
                        cxl_find_rec_gen_media() finds a record
  edac.c:1357  sparing  -EINVAL when cxl_mem_get_rec_dram() finds none

The unconditional log clear is at mbox.c:1115, after the record loop.

The memdev lock has to cover the store. err_rec_free() is a devm action on
&cxlmd->dev (edac.c:2021) that NULLs err_rec_array and frees it, and the
CPER path (cxl/pci.c:1053) can run while cxl_mem unbinds. Hoisting the
stores out from under guard(device) would trade the gating bug for a
use-after-free.

The casts on cxlmd go away because the parameter has not been const since
dc372e5f429c ("cxl/pci: Hold memdev lock in cxl_event_trace_record()").

Tested in QEMU with cxl_test. kprobe entry counts over the same 220 mock
event records, using cxl_event_trace_record() as the control:

  tracepoints off      control   store_gen_media   store_dram
    before                 220                 0            0
    after                  220                22           33

  tracepoints on       control   store_gen_media   store_dram
    before                 220                22           33
    after                  220                22           33

Records are kept with tracing off, and the tracing-on path is unchanged -
the hoist neither drops nor duplicates a store. The guest log shows
cxl_clear_event_record() draining the device log in both cases, so the
records really are gone from hardware whether or not the kernel kept them.
Builds warning-free with CONFIG_CXL_EDAC_MEM_FEATURES=y and disabled.

Not covered: the -EINVAL itself. cxl_test mocks no sPPR or sparing feature,
so the repair path cannot be driven end to end.

Left alone to keep this minimal: the WARN_ON_ONCE checks on the AP/CME
counter-expire descriptor (mbox.c:952-964) stay inside the gate, and are
arguably mis-gated for the same reason.
---
 drivers/cxl/core/mbox.c | 23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 55828a836c01..89fb2f569e4b 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -913,6 +913,22 @@ void cxl_event_trace_record(struct cxl_memdev *cxlmd,
 		return;
 	}
 
+	/*
+	 * EDAC memory repair looks these records up from the current boot, so
+	 * store them whether or not tracing is enabled. Hold the memdev lock
+	 * across the store to keep cxlmd->err_rec_array alive against a
+	 * concurrent cxl_mem unbind.
+	 */
+	guard(device)(&cxlmd->dev);
+
+	if (event_type == CXL_CPER_EVENT_GEN_MEDIA) {
+		if (cxl_store_rec_gen_media(cxlmd, evt))
+			dev_dbg(&cxlmd->dev, "CXL store rec_gen_media failed\n");
+	} else if (event_type == CXL_CPER_EVENT_DRAM) {
+		if (cxl_store_rec_dram(cxlmd, evt))
+			dev_dbg(&cxlmd->dev, "CXL store rec_dram failed\n");
+	}
+
 	if (trace_cxl_general_media_enabled() || trace_cxl_dram_enabled()) {
 		u64 dpa, hpa = ULLONG_MAX, hpa_alias = ULLONG_MAX;
 		struct cxl_region *cxlr;
@@ -922,7 +938,6 @@ void cxl_event_trace_record(struct cxl_memdev *cxlmd,
 		 * translations. Take topology mutation locks and lookup
 		 * { HPA, REGION } from { DPA, MEMDEV } in the event record.
 		 */
-		guard(device)(&cxlmd->dev);
 		guard(rwsem_read)(&cxl_rwsem.region);
 		guard(rwsem_read)(&cxl_rwsem.dpa);
 
@@ -937,9 +952,6 @@ void cxl_event_trace_record(struct cxl_memdev *cxlmd,
 		}
 
 		if (event_type == CXL_CPER_EVENT_GEN_MEDIA) {
-			if (cxl_store_rec_gen_media((struct cxl_memdev *)cxlmd, evt))
-				dev_dbg(&cxlmd->dev, "CXL store rec_gen_media failed\n");
-
 			if (evt->gen_media.media_hdr.descriptor &
 			    CXL_GMER_EVT_DESC_THRESHOLD_EVENT)
 				WARN_ON_ONCE((evt->gen_media.media_hdr.type &
@@ -952,9 +964,6 @@ void cxl_event_trace_record(struct cxl_memdev *cxlmd,
 			trace_cxl_general_media(cxlmd, type, cxlr, hpa,
 						hpa_alias, &evt->gen_media);
 		} else if (event_type == CXL_CPER_EVENT_DRAM) {
-			if (cxl_store_rec_dram((struct cxl_memdev *)cxlmd, evt))
-				dev_dbg(&cxlmd->dev, "CXL store rec_dram failed\n");
-
 			if (evt->dram.media_hdr.descriptor &
 			    CXL_GMER_EVT_DESC_THRESHOLD_EVENT)
 				WARN_ON_ONCE((evt->dram.media_hdr.type &

base-commit: fd73f4a6659897191fa0d40695fe370925dd3780
-- 
2.54.0