[PATCH v2 0/4] cxl/events: Robustify event interrupt handling

[PATCH v2 0/4] cxl/events: Robustify event interrupt handling

[Anisa Su]

Based on the 7.2 tag.

This patchset bundles various fixes along the event interrupt path.
The previous revision was a single patch, but Sashiko reported several
related pre-existing errors so I thought I may as well pick them all up into
one series.

Link to v1:
https://lore.kernel.org/linux-cxl/cover.1787768932.git.anisa.su@samsung.com/T/#m393e57d7f7f3916e8f2826a006ef0bc4dfd3606a

Patch 1:
========
cxl_event_thread() loops until the event status register is clear. The
Event Status register is device owned and read-only (CXL r4.0 8.2.9.3.1 Table 8-203),
so buggy device that never clears the register traps the thread
in an infinite loop. A persistent error would hang the thread too.

The cxl_event_thread() now drops any log whose status bit was set
while it returned 0 event records. Up to CXL_EVENT_DRAIN_ATTEMPTS
number of retries are allowed for transient errors, such as "Retry
Required" rc from the device, or mailbox -ETIMEDOUT/-EBUSY errors.

CXL_EVENT_DRAIN_ATTEMPTS is defined as 3.

Changes from v1:
- added retries for transient errors, pointed out by Richard Cheng

Patch 2:
========
Validates the record count the device reports. It is used
unchecked to index a flexible array in a buffer sized to the mailbox
payload, so an oversized count reads past the buffer, leaks the contents
to tracepoints, and then hands the same bytes back to the device in Clear
Event Records.

Patch 3:
========
Patch 1 addressed the event thread loop, which happens once per IRQ.
The event thread iterates over each of the event logs that have their
status bit set (Informational, Warning, Failure, etc.) Then for each log,
cxl_mem_get_records_log() gets event records until nr_rec reaches 0 for
that log.

A bad device that keeps reporting >0 records would keep the thread stuck
here.

This patch bounds the per-log loop by CXL_EVENT_LOG_MAX_PASSES, currently
set to 128. Probably overkill for a large mailbox, since the command
returns "as many even records... that fit into the mailbox output payload"
(CXL r4.0 Section 8.2.10.2.2 Get Event Records), but a spec-minimum sized
mailbox (256B) only fits 1 record, so I thought probably better to set the
the limit more generously. But it could be lowered.


Patch 4:
========
Returns IRQ_NONE when the handler processed nothing instead of
unconditionally returning IRQ_HANDLED.

Sashiko:
"Returning IRQ_HANDLED when no work was done prevents the kernel's core IRQ
subsystem from detecting and disabling a spurious interrupt storm. If a
failing CXL device floods the CPU with MSI interrupts, the kernel will never
disable the vector, which could completely lock up the processing core"


Testing:
========

NDTL:
ndctl's cxl suite (pmem/ndctl pending, 02754b5) against cxl_test:

15 passed, 2 skipped, 0 failed.
The two skips are cxl-type2.sh and cxl-features.sh, which are
unrelated to this series.

QEMU Tests:
All 4 patches have been tested on a QEMU branch modified to
emulate each of the above scenarios. A test script starts a VM for each
scenario and uses QMP to inject a general media event
(cxl-inject-general-media-event) with DPA = 0x1000 and all other fields set to
0 to trigger the event interrupt.

Each error scenario can be turned on with an environment var in QEMU:

/*   CXL_TEST_STICKY_EVENT_STATUS=1  leave the Event Status bit set once the
 *                                   log has been drained
 *   CXL_TEST_EVENT_RETRY=1          answer Get/Clear Event Records with Retry
 *                                   Required for every log
 *   CXL_TEST_BAD_RECORD_COUNT=1     claim more records than the payload holds
 *   CXL_TEST_EVENT_IRQ_STORM=1      raise the event interrupt without ever
 *                                   putting a record in a log
 *   CXL_TEST_ENDLESS_RECORDS=1      acknowledge Clear Event Records without
 *                                   removing anything, so the log never drains
 */
static bool cxl_test_knob(const char *name)
{
    const char *val = getenv(name);

    return val && val[0] == '1';
}

Then for example in cxl_event_delete_head(), if CXL_TEST_STICKY_EVENT_STATUS=1,
we skip clearing the log status:

-    if (cxl_event_empty(log)) {
+    if (cxl_event_empty(log) && !cxl_test_knob("CXL_TEST_STICKY_EVENT_STATUS")) {
         cxl_event_set_status(cxlds, log_type, false);
     }


For more details on QEMU, see this commit:
https://github.com/anisa-su993/qemu-anisa/commit/61960ab7fd7160226cd131a2cb1091b161c100bd

Test script:
https://github.com/anisa-su993/cxl-tests/blob/main/events/run-all.sh

Patch 1:
--------
  CXL_TEST_STICKY_EVENT_STATUS=1  leave the status bit set after the
                                  event log is cleared

<General Media Event injected w/QMP, triggering event interrupt on log 1>
[    7.421124] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
[    7.424094] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.426894] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.430098] cxl_core:cxl_clear_event_record:1048: cxl_pci 0000:0d:00.0: Event log '0': Clearing 1
[    7.433004] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0101
[    7.435831] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.438775] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.441527] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.444478] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
[    7.447274] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.450102] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.453001] cxl_pci 0000:0d:00.0: Event status 0x1 set with no records to read <---- detected error and stops

  CXL_TEST_EVENT_RETRY=1          answer Get/Clear Event Records with
                                  Retry Required for every log

<General Media Event injected w/QMP, triggering event interrupt on log 1>
[    7.377325] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
[    7.380459] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.383242] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.386105] cxl_pci:__cxl_pci_mbox_send_cmd:346: cxl_pci 0000:0d:00.0: Mailbox operation had an error: temporary error, retry once
[    7.389834] cxl_pci 0000:0d:00.0: Event log '0': Failed to query event records : -11
[    7.393652] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
[    7.396574] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.399323] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.402217] cxl_pci:__cxl_pci_mbox_send_cmd:346: cxl_pci 0000:0d:00.0: Mailbox operation had an error: temporary error, retry once
[    7.405949] cxl_pci 0000:0d:00.0: Event log '0': Failed to query event records : -11
[    7.409794] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
[    7.412716] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.415491] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.418399] cxl_pci:__cxl_pci_mbox_send_cmd:346: cxl_pci 0000:0d:00.0: Mailbox operation had an error: temporary error, retry once
[    7.422016] cxl_pci 0000:0d:00.0: Event log '0': Failed to query event records : -11
[    7.424199] cxl_pci 0000:0d:00.0: Event log drain gave up after 3 attempts: -11 <----- gave up after CXL_EVENT_DRAIN_ATTEMPTS


Patch 2:
--------
  CXL_TEST_BAD_RECORD_COUNT=1     claim 1 more record than the payload holds

[    7.334917] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
[    7.337746] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    7.340545] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    7.343614] cxl_pci 0000:0d:00.0: Event log '0': record count 2 mismatch in 160 byte payload
[    7.346197] cxl_pci 0000:0d:00.0: Event log drain failed: -5 <---- skip reading payload and return -EIO

Patch 3:
--------
  CXL_TEST_ENDLESS_RECORDS=1      acknowledge Clear Event Records without
                                  removing anything, so the log is never emptied
... skipping some logs
[    8.116284] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    8.117190] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    8.118218] cxl_core:cxl_clear_event_record:1048: cxl_pci 0000:0d:00.0: Event log '0': Clearing 1
[    8.119212] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0101
[    8.120130] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    8.121093] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
[    8.121992] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    8.123088] cxl_core:cxl_clear_event_record:1048: cxl_pci 0000:0d:00.0: Event log '0': Clearing 1
[    8.124027] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0101
[    8.124927] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
[    8.125937] cxl_pci 0000:0d:00.0: Event log '0': Still reporting records after 128 passes, giving up <--- hit CXL_EVENT_LOG_MAX_PASSES ceiling
[    8.126878] cxl_pci 0000:0d:00.0: Event log drain failed: -5

Patch 4:
--------
  CXL_TEST_EVENT_IRQ_STORM=1      raise the event interrupt without ever
                                  putting a record in a log

...
[    9.929250] irq 28: nobody cared (try booting with the "irqpoll" option) <---- printed in __report_bad_irq()
[    9.929937] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 7.2.0+ #34 PREEMPT(full)
[    9.929939] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
[    9.929940] Call Trace:
[    9.930417]  <IRQ>
...
[    9.931396]  </TASK>
[    9.931397] handlers:
[    9.945340] [<00000000a6051941>] irq_default_primary_handler threaded [<00000000c56a4fd3>] cxl_event_thread <----- handler is cxl_event_thread
[    9.946222] Disabling IRQ #28

__report_bad_irq() turns off the interrupt "if 99,900 of the previous 100,000 interrupts
have not been handled". With the fix, we see that__report_bad_irq() is reached.
Without the fix (IRQ_HANDLED unconditionally returned), the interrupt is
not disabled and keeps firing.


Anisa Su (4):
  cxl/events: Bound get records loop in cxl_event_thread()
  cxl/events: Validate the record count reported by the device
  cxl/events: Bound the per-log Get Event Records loop
  cxl/events: Return IRQ_NONE when no events were processed

 drivers/cxl/core/mbox.c      | 94 ++++++++++++++++++++++++++++++------
 drivers/cxl/cxlmem.h         |  3 +-
 drivers/cxl/pci.c            | 67 ++++++++++++++++++++++---
 tools/testing/cxl/test/mem.c |  4 +-
 4 files changed, 143 insertions(+), 25 deletions(-)


base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
-- 
2.43.0

[PATCH v2 1/4] cxl/events: Bound get records loop in cxl_event_thread()

[Anisa Su]

cxl_event_thread() drains all logs named in the Event Status register and
repeats until the register reads zero. CXL r4.0 Section 8.2.9.3.1 Table 8-203
marks the register RO and leaves clearing it to the device, so a misbehaving
device that leaves a status bit set spins the thread forever, resending
Get Event Records as fast as the mailbox completes.

Bound the loop on work completed instead. cxl_mem_get_records_log() and
cxl_mem_get_event_records() report failures and which logs returned
records; the thread drops any log whose status bit was set while it
returned nothing, and caps consecutive errors at CXL_EVENT_DRAIN_ATTEMPTS.
Every pass either drains a record, clears a bit from the mask or spends an
attempt, so the loop eventually terminates, even if the device misbehaves.
The first error is returned and an error from one log does not prevent
getting logs from the rest.

Retrying is limited to the failures that can plausibly be cleared:
-EBUSY and -ETIMEDOUT from the mailbox, and Retry Required from the
device. Anything else gives up on the first error, so cycles are not wasted
retrying on a non-retryable error.

Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
Signed-off-by: Anisa Su <anisa.su@samsung.com>

---
Changes:
[Richard Cheng]: retry on transient failures
---
 drivers/cxl/core/mbox.c      | 74 +++++++++++++++++++++++++++++-------
 drivers/cxl/cxlmem.h         |  3 +-
 drivers/cxl/pci.c            | 63 ++++++++++++++++++++++++++----
 tools/testing/cxl/test/mem.c |  4 +-
 4 files changed, 120 insertions(+), 24 deletions(-)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 7c6c5b7450a5..2b71a8e1f35f 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -988,6 +988,18 @@ static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
 	cxl_event_trace_record(cxlmd, type, ev_type, uuid, &record->event);
 }
 
+/*
+ * cxl_mbox_cmd_rc2errno() collapses every device return code onto -ENXIO;
+ * pick out the one the spec asks the caller to retry.
+ */
+static int cxl_event_retry_rc(struct cxl_mbox_cmd *mbox_cmd, int rc)
+{
+	if (rc == -ENXIO && mbox_cmd->return_code == CXL_MBOX_CMD_RC_RETRY)
+		return -EAGAIN;
+
+	return rc;
+}
+
 static int cxl_clear_event_record(struct cxl_memdev_state *mds,
 				  enum cxl_event_log_type log,
 				  struct cxl_get_event_payload *get_pl)
@@ -1056,11 +1068,11 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds,
 
 free_pl:
 	kvfree(payload);
-	return rc;
+	return cxl_event_retry_rc(&mbox_cmd, rc);
 }
 
-static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
-				    enum cxl_event_log_type type)
+static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
+				   enum cxl_event_log_type type, bool *got_records)
 {
 	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
 	struct cxl_memdev *cxlmd = mds->cxlds.cxlmd;
@@ -1068,12 +1080,15 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 	struct cxl_get_event_payload *payload;
 	u8 log_type = type;
 	u16 nr_rec;
+	int rc = 0;
+
+	*got_records = false;
 
 	mutex_lock(&mds->event.log_lock);
 	payload = mds->event.buf;
 
 	do {
-		int rc, i;
+		int i;
 		struct cxl_mbox_cmd mbox_cmd = (struct cxl_mbox_cmd) {
 			.opcode = CXL_MBOX_OP_GET_EVENT_RECORD,
 			.payload_in = &log_type,
@@ -1085,6 +1100,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 
 		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
 		if (rc) {
+			rc = cxl_event_retry_rc(&mbox_cmd, rc);
 			dev_err_ratelimited(dev,
 				"Event log '%d': Failed to query event records : %d",
 				type, rc);
@@ -1094,6 +1110,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 		nr_rec = le16_to_cpu(payload->record_count);
 		if (!nr_rec)
 			break;
+		*got_records = true;
 
 		for (i = 0; i < nr_rec; i++)
 			__cxl_event_trace_record(cxlmd, type,
@@ -1112,31 +1129,60 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 	} while (nr_rec);
 
 	mutex_unlock(&mds->event.log_lock);
+
+	return rc;
 }
 
 /**
  * cxl_mem_get_event_records - Get Event Records from the device
  * @mds: The driver data for the operation
  * @status: Event Status register value identifying which events are available.
+ * @drained: Optional mask of the logs in @status that returned records.
  *
  * Retrieve all event records available on the device, report them as trace
- * events, and clear them.
+ * events, and clear them.  Every log named in @status is drained even if
+ * another one fails, so that a broken log does not suppress reporting from
+ * the rest.
+ *
+ * Return: 0, or the first error encountered.  -EAGAIN if the device asked for
+ * a command to be retried.
  *
  * See CXL rev 3.0 @8.2.9.2.2 Get Event Records
  * See CXL rev 3.0 @8.2.9.2.3 Clear Event Records
  */
-void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status)
+int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
+			      u32 *drained)
 {
+	static const struct {
+		u32 status;
+		enum cxl_event_log_type type;
+	} logs[] = {
+		{ CXLDEV_EVENT_STATUS_FATAL, CXL_EVENT_TYPE_FATAL },
+		{ CXLDEV_EVENT_STATUS_FAIL, CXL_EVENT_TYPE_FAIL },
+		{ CXLDEV_EVENT_STATUS_WARN, CXL_EVENT_TYPE_WARN },
+		{ CXLDEV_EVENT_STATUS_INFO, CXL_EVENT_TYPE_INFO },
+	};
+	int ret = 0;
+
 	dev_dbg(mds->cxlds.dev, "Reading event logs: %x\n", status);
 
-	if (status & CXLDEV_EVENT_STATUS_FATAL)
-		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FATAL);
-	if (status & CXLDEV_EVENT_STATUS_FAIL)
-		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FAIL);
-	if (status & CXLDEV_EVENT_STATUS_WARN)
-		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_WARN);
-	if (status & CXLDEV_EVENT_STATUS_INFO)
-		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_INFO);
+	if (drained)
+		*drained = 0;
+
+	for (int i = 0; i < ARRAY_SIZE(logs); i++) {
+		bool got_records;
+		int rc;
+
+		if (!(status & logs[i].status))
+			continue;
+
+		rc = cxl_mem_get_records_log(mds, logs[i].type, &got_records);
+		if (got_records && drained)
+			*drained |= logs[i].status;
+		ret = ret ?: rc;
+	}
+
+	return ret;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, "CXL");
 
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index ed419d0c59f2..cb5372eb2d85 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -803,7 +803,8 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
 				unsigned long *cmds);
 void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
 				  unsigned long *cmds);
-void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
+int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
+			      u32 *drained);
 void cxl_event_trace_record(struct cxl_memdev *cxlmd,
 			    enum cxl_event_log_type type,
 			    enum cxl_event_type event_type,
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 267c679b0b3c..239df18c9c87 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -509,26 +509,75 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
 	return true;
 }
 
+#define CXL_EVENT_DRAIN_ATTEMPTS 3
+
+/*
+ * -EAGAIN is the device asking for a retry, the rest are the mailbox being
+ * momentarily unavailable.  Everything else is permanent as far as this can
+ * tell.
+ */
+static bool cxl_event_drain_retryable(int rc)
+{
+	return rc == -EAGAIN || rc == -EBUSY || rc == -ETIMEDOUT;
+}
+
 static irqreturn_t cxl_event_thread(int irq, void *id)
 {
 	struct cxl_dev_id *dev_id = id;
 	struct cxl_dev_state *cxlds = dev_id->cxlds;
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
-	u32 status;
+	u32 mask = CXLDEV_EVENT_STATUS_ALL;
+	int attempts = CXL_EVENT_DRAIN_ATTEMPTS;
+
+	while (mask) {
+		u32 status, drained, stuck;
+		int rc;
 
-	do {
 		/*
 		 * CXL 3.0 8.2.8.3.1: The lower 32 bits are the status;
 		 * ignore the reserved upper 32 bits
 		 */
 		status = readl(cxlds->regs.status + CXLDEV_DEV_EVENT_STATUS_OFFSET);
-		/* Ignore logs unknown to the driver */
-		status &= CXLDEV_EVENT_STATUS_ALL;
+		/* Ignore logs unknown to the driver, and logs given up on */
+		status &= mask;
 		if (!status)
 			break;
-		cxl_mem_get_event_records(mds, status);
+
+		rc = cxl_mem_get_event_records(mds, status, &drained);
+		if (rc) {
+			if (!cxl_event_drain_retryable(rc)) {
+				dev_warn(cxlds->dev,
+					 "Event log drain failed: %d\n", rc);
+				break;
+			}
+			if (--attempts) {
+				fsleep(1000);
+				continue;
+			}
+			dev_warn(cxlds->dev,
+				 "Event log drain gave up after %d attempts: %d\n",
+				 CXL_EVENT_DRAIN_ATTEMPTS, rc);
+			break;
+		}
+		/* Progress was made, so reset number of attempts */
+		attempts = CXL_EVENT_DRAIN_ATTEMPTS;
+
+		/*
+		 * The Event Status register is device owned and read only, so
+		 * it cannot bound this loop; records read can.  A device that
+		 * leaves a bit set with an empty log makes no progress, and
+		 * only the device can clear that state, so stop polling that
+		 * log rather than spin on it.
+		 */
+		stuck = status & ~drained;
+		if (stuck) {
+			dev_warn_once(cxlds->dev,
+				      "Event status %#x set with no records to read\n",
+				      stuck);
+			mask &= ~stuck;
+		}
 		cond_resched();
-	} while (status);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -682,7 +731,7 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
 	if (rc)
 		return rc;
 
-	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
+	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
 
 	return 0;
 }
diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
index a7da279aa3ef..87e01612cea9 100644
--- a/tools/testing/cxl/test/mem.c
+++ b/tools/testing/cxl/test/mem.c
@@ -372,7 +372,7 @@ static void cxl_mock_event_trigger(struct device *dev)
 			event_reset_log(log);
 	}
 
-	cxl_mem_get_event_records(mdata->mds, mes->ev_status);
+	cxl_mem_get_event_records(mdata->mds, mes->ev_status, NULL);
 }
 
 struct cxl_event_record_raw maint_needed = {
@@ -1805,7 +1805,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
 	if (rc)
 		dev_dbg(dev, "No CXL FWCTL setup\n");
 
-	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
+	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
 	cxl_mock_test_feat_init(mdata);
 
 	return 0;
-- 
2.43.0

Re: [PATCH v2 1/4] cxl/events: Bound get records loop in cxl_event_thread()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Error aggregation masks retryable errors, causing transient failures to be permanently abandoned without retry.
- [Medium] False positive hardware warning due to race condition between concurrent IRQ threads.
--

commit 16c815c8618408dc3d242d5fe42bbf41248f7c71
Author: Anisa Su <anisa.su887@gmail.com>

cxl/events: Bound get records loop in cxl_event_thread()

This commit limits the cxl_event_thread loop by checking completed work
rather than only relying on the hardware Event Status register. It bounds
the retry attempts for retryable errors (-EBUSY, -ETIMEDOUT, -EAGAIN) and
gives up on permanent errors.

> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 7c6c5b7450a54..2b71a8e1f35f7 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
[ ... ]
> @@ -1112,31 +1129,60 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
[ ... ]
> +	for (int i = 0; i < ARRAY_SIZE(logs); i++) {
> +		bool got_records;
> +		int rc;
> +
> +		if (!(status & logs[i].status))
> +			continue;
> +
> +		rc = cxl_mem_get_records_log(mds, logs[i].type, &got_records);
> +		if (got_records && drained)
> +			*drained |= logs[i].status;
> +		ret = ret ?: rc;

[Severity: High]
Does this error aggregation mask retryable errors? 

If an earlier log encounters a non-retryable error (like -EIO), it is stored
in ret. If a subsequent log encounters a retryable error (like -EAGAIN), the
earlier error takes precedence due to the ret = ret ?: rc logic.

Back in the caller cxl_event_thread(), the loop checks the return code:

    rc = cxl_mem_get_event_records(mds, status, &drained);
    if (rc) {
        if (!cxl_event_drain_retryable(rc)) {
            dev_warn(cxlds->dev, "Event log drain failed: %d\n", rc);
            break;
        }
        ...
    }

Will this cause the thread to abort the entire batch and abandon the transient
error without retrying, permanently losing those hardware events?

> +	}
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, "CXL");
>  
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 267c679b0b3c2..239df18c9c87b 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
[ ... ]
> @@ -509,26 +509,75 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
[ ... ]
>  static irqreturn_t cxl_event_thread(int irq, void *id)
>  {
[ ... ]
> +	while (mask) {
> +		u32 status, drained, stuck;
> +		int rc;
>  
> -	do {
>  		/*
>  		 * CXL 3.0 8.2.8.3.1: The lower 32 bits are the status;
>  		 * ignore the reserved upper 32 bits
>  		 */
>  		status = readl(cxlds->regs.status + CXLDEV_DEV_EVENT_STATUS_OFFSET);
[ ... ]
> -		cxl_mem_get_event_records(mds, status);
> +		rc = cxl_mem_get_event_records(mds, status, &drained);
[ ... ]
> +		/*
> +		 * The Event Status register is device owned and read only, so
> +		 * it cannot bound this loop; records read can.  A device that
> +		 * leaves a bit set with an empty log makes no progress, and
> +		 * only the device can clear that state, so stop polling that
> +		 * log rather than spin on it.
> +		 */
> +		stuck = status & ~drained;
> +		if (stuck) {
> +			dev_warn_once(cxlds->dev,
> +				      "Event status %#x set with no records to read\n",
> +				      stuck);
> +			mask &= ~stuck;
> +		}

[Severity: Medium]
Is there a risk of a false positive hardware warning here due to a race
condition? 

If multiple event logs share an IRQ vector, two concurrent cxl_event_thread
instances could race. If Thread A drains the events, Thread B would receive no
records, and its call to cxl_mem_get_event_records() would return with
drained == 0.

Thread B would then evaluate stuck = status & ~drained using the stale status
value read at the beginning of the loop. 

Could this lead to Thread B incorrectly concluding the device is stuck and
prematurely masking the log for its iteration?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901002912.958-1-anisa.su@samsung.com?part=1

Re: [PATCH v2 1/4] cxl/events: Bound get records loop in cxl_event_thread()

[Li Ming]

在 2026/9/1 08:26, Anisa Su 写道:
> cxl_event_thread() drains all logs named in the Event Status register and
> repeats until the register reads zero. CXL r4.0 Section 8.2.9.3.1 Table 8-203
> marks the register RO and leaves clearing it to the device, so a misbehaving
> device that leaves a status bit set spins the thread forever, resending
> Get Event Records as fast as the mailbox completes.

Hi Anisa,

Per the commit log and the description in CXL r4.0 section 8.2.9.3.1 
Table 8-203. SPEC mentions "Once the event log has zero event records, 
this bit(event status) is cleared". So it is a spec-violating issue on 
device side.

My understanding is that CXL driver does not need special handling for 
device behaviors that violate the CXL spec. If we are facing this issue, 
the correct way is fixing it on device side.

Let's see if maintainers have comments on that.


Besides, your patch makes me realize that checking the return value of 
cxl_mem_get_event_records() is valuable. GET_EVENT_RECORD and 
CLEAR_EVENT_RECORD operations could fail in cxl_mem_get_records_logs(), 
which cause that device could not have chance to clear event records, 
then device would not clear event status register. In that case, the 
loop in cxl_event_thread() will never break out.


Ming

>
> Bound the loop on work completed instead. cxl_mem_get_records_log() and
> cxl_mem_get_event_records() report failures and which logs returned
> records; the thread drops any log whose status bit was set while it
> returned nothing, and caps consecutive errors at CXL_EVENT_DRAIN_ATTEMPTS.
> Every pass either drains a record, clears a bit from the mask or spends an
> attempt, so the loop eventually terminates, even if the device misbehaves.
> The first error is returned and an error from one log does not prevent
> getting logs from the rest.
>
> Retrying is limited to the failures that can plausibly be cleared:
> -EBUSY and -ETIMEDOUT from the mailbox, and Retry Required from the
> device. Anything else gives up on the first error, so cycles are not wasted
> retrying on a non-retryable error.
>
> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
>
> ---
> Changes:
> [Richard Cheng]: retry on transient failures
> ---
>   drivers/cxl/core/mbox.c      | 74 +++++++++++++++++++++++++++++-------
>   drivers/cxl/cxlmem.h         |  3 +-
>   drivers/cxl/pci.c            | 63 ++++++++++++++++++++++++++----
>   tools/testing/cxl/test/mem.c |  4 +-
>   4 files changed, 120 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 7c6c5b7450a5..2b71a8e1f35f 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -988,6 +988,18 @@ static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
>   	cxl_event_trace_record(cxlmd, type, ev_type, uuid, &record->event);
>   }
>   
> +/*
> + * cxl_mbox_cmd_rc2errno() collapses every device return code onto -ENXIO;
> + * pick out the one the spec asks the caller to retry.
> + */
> +static int cxl_event_retry_rc(struct cxl_mbox_cmd *mbox_cmd, int rc)
> +{
> +	if (rc == -ENXIO && mbox_cmd->return_code == CXL_MBOX_CMD_RC_RETRY)
> +		return -EAGAIN;
> +
> +	return rc;
> +}
> +
>   static int cxl_clear_event_record(struct cxl_memdev_state *mds,
>   				  enum cxl_event_log_type log,
>   				  struct cxl_get_event_payload *get_pl)
> @@ -1056,11 +1068,11 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds,
>   
>   free_pl:
>   	kvfree(payload);
> -	return rc;
> +	return cxl_event_retry_rc(&mbox_cmd, rc);
>   }
>   
> -static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> -				    enum cxl_event_log_type type)
> +static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> +				   enum cxl_event_log_type type, bool *got_records)
>   {
>   	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
>   	struct cxl_memdev *cxlmd = mds->cxlds.cxlmd;
> @@ -1068,12 +1080,15 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>   	struct cxl_get_event_payload *payload;
>   	u8 log_type = type;
>   	u16 nr_rec;
> +	int rc = 0;
> +
> +	*got_records = false;
>   
>   	mutex_lock(&mds->event.log_lock);
>   	payload = mds->event.buf;
>   
>   	do {
> -		int rc, i;
> +		int i;
>   		struct cxl_mbox_cmd mbox_cmd = (struct cxl_mbox_cmd) {
>   			.opcode = CXL_MBOX_OP_GET_EVENT_RECORD,
>   			.payload_in = &log_type,
> @@ -1085,6 +1100,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>   
>   		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>   		if (rc) {
> +			rc = cxl_event_retry_rc(&mbox_cmd, rc);
>   			dev_err_ratelimited(dev,
>   				"Event log '%d': Failed to query event records : %d",
>   				type, rc);
> @@ -1094,6 +1110,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>   		nr_rec = le16_to_cpu(payload->record_count);
>   		if (!nr_rec)
>   			break;
> +		*got_records = true;
>   
>   		for (i = 0; i < nr_rec; i++)
>   			__cxl_event_trace_record(cxlmd, type,
> @@ -1112,31 +1129,60 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>   	} while (nr_rec);
>   
>   	mutex_unlock(&mds->event.log_lock);
> +
> +	return rc;
>   }
>   
>   /**
>    * cxl_mem_get_event_records - Get Event Records from the device
>    * @mds: The driver data for the operation
>    * @status: Event Status register value identifying which events are available.
> + * @drained: Optional mask of the logs in @status that returned records.
>    *
>    * Retrieve all event records available on the device, report them as trace
> - * events, and clear them.
> + * events, and clear them.  Every log named in @status is drained even if
> + * another one fails, so that a broken log does not suppress reporting from
> + * the rest.
> + *
> + * Return: 0, or the first error encountered.  -EAGAIN if the device asked for
> + * a command to be retried.
>    *
>    * See CXL rev 3.0 @8.2.9.2.2 Get Event Records
>    * See CXL rev 3.0 @8.2.9.2.3 Clear Event Records
>    */
> -void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status)
> +int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
> +			      u32 *drained)
>   {
> +	static const struct {
> +		u32 status;
> +		enum cxl_event_log_type type;
> +	} logs[] = {
> +		{ CXLDEV_EVENT_STATUS_FATAL, CXL_EVENT_TYPE_FATAL },
> +		{ CXLDEV_EVENT_STATUS_FAIL, CXL_EVENT_TYPE_FAIL },
> +		{ CXLDEV_EVENT_STATUS_WARN, CXL_EVENT_TYPE_WARN },
> +		{ CXLDEV_EVENT_STATUS_INFO, CXL_EVENT_TYPE_INFO },
> +	};
> +	int ret = 0;
> +
>   	dev_dbg(mds->cxlds.dev, "Reading event logs: %x\n", status);
>   
> -	if (status & CXLDEV_EVENT_STATUS_FATAL)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FATAL);
> -	if (status & CXLDEV_EVENT_STATUS_FAIL)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FAIL);
> -	if (status & CXLDEV_EVENT_STATUS_WARN)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_WARN);
> -	if (status & CXLDEV_EVENT_STATUS_INFO)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_INFO);
> +	if (drained)
> +		*drained = 0;
> +
> +	for (int i = 0; i < ARRAY_SIZE(logs); i++) {
> +		bool got_records;
> +		int rc;
> +
> +		if (!(status & logs[i].status))
> +			continue;
> +
> +		rc = cxl_mem_get_records_log(mds, logs[i].type, &got_records);
> +		if (got_records && drained)
> +			*drained |= logs[i].status;
> +		ret = ret ?: rc;
> +	}
> +
> +	return ret;
>   }
>   EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, "CXL");
>   
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ed419d0c59f2..cb5372eb2d85 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -803,7 +803,8 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>   				unsigned long *cmds);
>   void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>   				  unsigned long *cmds);
> -void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
> +int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
> +			      u32 *drained);
>   void cxl_event_trace_record(struct cxl_memdev *cxlmd,
>   			    enum cxl_event_log_type type,
>   			    enum cxl_event_type event_type,
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 267c679b0b3c..239df18c9c87 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -509,26 +509,75 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
>   	return true;
>   }
>   
> +#define CXL_EVENT_DRAIN_ATTEMPTS 3
> +
> +/*
> + * -EAGAIN is the device asking for a retry, the rest are the mailbox being
> + * momentarily unavailable.  Everything else is permanent as far as this can
> + * tell.
> + */
> +static bool cxl_event_drain_retryable(int rc)
> +{
> +	return rc == -EAGAIN || rc == -EBUSY || rc == -ETIMEDOUT;
> +}
> +
>   static irqreturn_t cxl_event_thread(int irq, void *id)
>   {
>   	struct cxl_dev_id *dev_id = id;
>   	struct cxl_dev_state *cxlds = dev_id->cxlds;
>   	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> -	u32 status;
> +	u32 mask = CXLDEV_EVENT_STATUS_ALL;
> +	int attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> +
> +	while (mask) {
> +		u32 status, drained, stuck;
> +		int rc;
>   
> -	do {
>   		/*
>   		 * CXL 3.0 8.2.8.3.1: The lower 32 bits are the status;
>   		 * ignore the reserved upper 32 bits
>   		 */
>   		status = readl(cxlds->regs.status + CXLDEV_DEV_EVENT_STATUS_OFFSET);
> -		/* Ignore logs unknown to the driver */
> -		status &= CXLDEV_EVENT_STATUS_ALL;
> +		/* Ignore logs unknown to the driver, and logs given up on */
> +		status &= mask;
>   		if (!status)
>   			break;
> -		cxl_mem_get_event_records(mds, status);
> +
> +		rc = cxl_mem_get_event_records(mds, status, &drained);
> +		if (rc) {
> +			if (!cxl_event_drain_retryable(rc)) {
> +				dev_warn(cxlds->dev,
> +					 "Event log drain failed: %d\n", rc);
> +				break;
> +			}
> +			if (--attempts) {
> +				fsleep(1000);
> +				continue;
> +			}
> +			dev_warn(cxlds->dev,
> +				 "Event log drain gave up after %d attempts: %d\n",
> +				 CXL_EVENT_DRAIN_ATTEMPTS, rc);
> +			break;
> +		}
> +		/* Progress was made, so reset number of attempts */
> +		attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> +
> +		/*
> +		 * The Event Status register is device owned and read only, so
> +		 * it cannot bound this loop; records read can.  A device that
> +		 * leaves a bit set with an empty log makes no progress, and
> +		 * only the device can clear that state, so stop polling that
> +		 * log rather than spin on it.
> +		 */
> +		stuck = status & ~drained;
> +		if (stuck) {
> +			dev_warn_once(cxlds->dev,
> +				      "Event status %#x set with no records to read\n",
> +				      stuck);
> +			mask &= ~stuck;
> +		}
>   		cond_resched();
> -	} while (status);
> +	}
>   
>   	return IRQ_HANDLED;
>   }
> @@ -682,7 +731,7 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
>   	if (rc)
>   		return rc;
>   
> -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
>   
>   	return 0;
>   }
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index a7da279aa3ef..87e01612cea9 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -372,7 +372,7 @@ static void cxl_mock_event_trigger(struct device *dev)
>   			event_reset_log(log);
>   	}
>   
> -	cxl_mem_get_event_records(mdata->mds, mes->ev_status);
> +	cxl_mem_get_event_records(mdata->mds, mes->ev_status, NULL);
>   }
>   
>   struct cxl_event_record_raw maint_needed = {
> @@ -1805,7 +1805,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
>   	if (rc)
>   		dev_dbg(dev, "No CXL FWCTL setup\n");
>   
> -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
>   	cxl_mock_test_feat_init(mdata);
>   
>   	return 0;

Re: [PATCH v2 1/4] cxl/events: Bound get records loop in cxl_event_thread()

[Anisa Su]

On Tue, Sep 01, 2026 at 07:48:02PM +0800, Li Ming wrote:
> 
> 在 2026/9/1 08:26, Anisa Su 写道:
> > cxl_event_thread() drains all logs named in the Event Status register and
> > repeats until the register reads zero. CXL r4.0 Section 8.2.9.3.1 Table 8-203
> > marks the register RO and leaves clearing it to the device, so a misbehaving
> > device that leaves a status bit set spins the thread forever, resending
> > Get Event Records as fast as the mailbox completes.
> 
> Hi Anisa,
> 
> Per the commit log and the description in CXL r4.0 section 8.2.9.3.1
> Table 8-203. SPEC mentions "Once the event log has zero event records,
> this bit(event status) is cleared". So it is a spec-violating issue on
> device side.
> 
Hello Ming,

Yes, that's correct. The event status bit must be cleared by the device.
However, while testing a DCD device, I encountered this scenario due to a
firmware bug. I mentioned more details in the cover letter of v1:
https://lore.kernel.org/linux-cxl/cover.1787768932.git.anisa.su@samsung.com/T/#m393e57d7f7f3916e8f2826a006ef0bc4dfd3606a

Sorry for not mentioning it more clearly this time.

> My understanding is that CXL driver does not need special handling for
> device behaviors that violate the CXL spec. If we are facing this
> issue, the correct way is fixing it on device side.
> 
I agree too, but since I encountered this on a real device, I thought it would
be worth patching up.

Also if anyone is wondering: the reason I used QEMU to test the patch this
time is because the firmware on the device I was testing has been updated,
so the error doesn't happen anymore. Instead, I emulated the bug in QEMU.

Patches 2-4 were just pre-existing errors noticed by Sashiko, not
something I encountered on real hardware, so maybe those are not necessary?

> Let's see if maintainers have comments on that.
> 
> 
> Besides, your patch makes me realize that checking the return value of
> cxl_mem_get_event_records() is valuable. GET_EVENT_RECORD and
> CLEAR_EVENT_RECORD operations could fail in
> cxl_mem_get_records_logs(), which cause that device could not have
> chance to clear event records, then device would not clear event
> status register. In that case, the loop in cxl_event_thread() will
> never break out.
> 
Yes, I think even if we decide not to add the special handling for detecting a
non-compliant device that doesn't clear the bit, checking the rc of
cxl_mem_get_event_records() is necessary.
> 
> Ming
> 
Thanks for the review!
Anisa

> > 
> > Bound the loop on work completed instead. cxl_mem_get_records_log() and
> > cxl_mem_get_event_records() report failures and which logs returned
> > records; the thread drops any log whose status bit was set while it
> > returned nothing, and caps consecutive errors at CXL_EVENT_DRAIN_ATTEMPTS.
> > Every pass either drains a record, clears a bit from the mask or spends an
> > attempt, so the loop eventually terminates, even if the device misbehaves.
> > The first error is returned and an error from one log does not prevent
> > getting logs from the rest.
> > 
> > Retrying is limited to the failures that can plausibly be cleared:
> > -EBUSY and -ETIMEDOUT from the mailbox, and Retry Required from the
> > device. Anything else gives up on the first error, so cycles are not wasted
> > retrying on a non-retryable error.
> > 
> > Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> > Signed-off-by: Anisa Su <anisa.su@samsung.com>
> > 
> > ---
> > Changes:
> > [Richard Cheng]: retry on transient failures
> > ---
> >   drivers/cxl/core/mbox.c      | 74 +++++++++++++++++++++++++++++-------
> >   drivers/cxl/cxlmem.h         |  3 +-
> >   drivers/cxl/pci.c            | 63 ++++++++++++++++++++++++++----
> >   tools/testing/cxl/test/mem.c |  4 +-
> >   4 files changed, 120 insertions(+), 24 deletions(-)
> > 
> > diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> > index 7c6c5b7450a5..2b71a8e1f35f 100644
> > --- a/drivers/cxl/core/mbox.c
> > +++ b/drivers/cxl/core/mbox.c
> > @@ -988,6 +988,18 @@ static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
> >   	cxl_event_trace_record(cxlmd, type, ev_type, uuid, &record->event);
> >   }
> > +/*
> > + * cxl_mbox_cmd_rc2errno() collapses every device return code onto -ENXIO;
> > + * pick out the one the spec asks the caller to retry.
> > + */
> > +static int cxl_event_retry_rc(struct cxl_mbox_cmd *mbox_cmd, int rc)
> > +{
> > +	if (rc == -ENXIO && mbox_cmd->return_code == CXL_MBOX_CMD_RC_RETRY)
> > +		return -EAGAIN;
> > +
> > +	return rc;
> > +}
> > +
> >   static int cxl_clear_event_record(struct cxl_memdev_state *mds,
> >   				  enum cxl_event_log_type log,
> >   				  struct cxl_get_event_payload *get_pl)
> > @@ -1056,11 +1068,11 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds,
> >   free_pl:
> >   	kvfree(payload);
> > -	return rc;
> > +	return cxl_event_retry_rc(&mbox_cmd, rc);
> >   }
> > -static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> > -				    enum cxl_event_log_type type)
> > +static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> > +				   enum cxl_event_log_type type, bool *got_records)
> >   {
> >   	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
> >   	struct cxl_memdev *cxlmd = mds->cxlds.cxlmd;
> > @@ -1068,12 +1080,15 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> >   	struct cxl_get_event_payload *payload;
> >   	u8 log_type = type;
> >   	u16 nr_rec;
> > +	int rc = 0;
> > +
> > +	*got_records = false;
> >   	mutex_lock(&mds->event.log_lock);
> >   	payload = mds->event.buf;
> >   	do {
> > -		int rc, i;
> > +		int i;
> >   		struct cxl_mbox_cmd mbox_cmd = (struct cxl_mbox_cmd) {
> >   			.opcode = CXL_MBOX_OP_GET_EVENT_RECORD,
> >   			.payload_in = &log_type,
> > @@ -1085,6 +1100,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> >   		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
> >   		if (rc) {
> > +			rc = cxl_event_retry_rc(&mbox_cmd, rc);
> >   			dev_err_ratelimited(dev,
> >   				"Event log '%d': Failed to query event records : %d",
> >   				type, rc);
> > @@ -1094,6 +1110,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> >   		nr_rec = le16_to_cpu(payload->record_count);
> >   		if (!nr_rec)
> >   			break;
> > +		*got_records = true;
> >   		for (i = 0; i < nr_rec; i++)
> >   			__cxl_event_trace_record(cxlmd, type,
> > @@ -1112,31 +1129,60 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> >   	} while (nr_rec);
> >   	mutex_unlock(&mds->event.log_lock);
> > +
> > +	return rc;
> >   }
> >   /**
> >    * cxl_mem_get_event_records - Get Event Records from the device
> >    * @mds: The driver data for the operation
> >    * @status: Event Status register value identifying which events are available.
> > + * @drained: Optional mask of the logs in @status that returned records.
> >    *
> >    * Retrieve all event records available on the device, report them as trace
> > - * events, and clear them.
> > + * events, and clear them.  Every log named in @status is drained even if
> > + * another one fails, so that a broken log does not suppress reporting from
> > + * the rest.
> > + *
> > + * Return: 0, or the first error encountered.  -EAGAIN if the device asked for
> > + * a command to be retried.
> >    *
> >    * See CXL rev 3.0 @8.2.9.2.2 Get Event Records
> >    * See CXL rev 3.0 @8.2.9.2.3 Clear Event Records
> >    */
> > -void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status)
> > +int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
> > +			      u32 *drained)
> >   {
> > +	static const struct {
> > +		u32 status;
> > +		enum cxl_event_log_type type;
> > +	} logs[] = {
> > +		{ CXLDEV_EVENT_STATUS_FATAL, CXL_EVENT_TYPE_FATAL },
> > +		{ CXLDEV_EVENT_STATUS_FAIL, CXL_EVENT_TYPE_FAIL },
> > +		{ CXLDEV_EVENT_STATUS_WARN, CXL_EVENT_TYPE_WARN },
> > +		{ CXLDEV_EVENT_STATUS_INFO, CXL_EVENT_TYPE_INFO },
> > +	};
> > +	int ret = 0;
> > +
> >   	dev_dbg(mds->cxlds.dev, "Reading event logs: %x\n", status);
> > -	if (status & CXLDEV_EVENT_STATUS_FATAL)
> > -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FATAL);
> > -	if (status & CXLDEV_EVENT_STATUS_FAIL)
> > -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FAIL);
> > -	if (status & CXLDEV_EVENT_STATUS_WARN)
> > -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_WARN);
> > -	if (status & CXLDEV_EVENT_STATUS_INFO)
> > -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_INFO);
> > +	if (drained)
> > +		*drained = 0;
> > +
> > +	for (int i = 0; i < ARRAY_SIZE(logs); i++) {
> > +		bool got_records;
> > +		int rc;
> > +
> > +		if (!(status & logs[i].status))
> > +			continue;
> > +
> > +		rc = cxl_mem_get_records_log(mds, logs[i].type, &got_records);
> > +		if (got_records && drained)
> > +			*drained |= logs[i].status;
> > +		ret = ret ?: rc;
> > +	}
> > +
> > +	return ret;
> >   }
> >   EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, "CXL");
> > diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> > index ed419d0c59f2..cb5372eb2d85 100644
> > --- a/drivers/cxl/cxlmem.h
> > +++ b/drivers/cxl/cxlmem.h
> > @@ -803,7 +803,8 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> >   				unsigned long *cmds);
> >   void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
> >   				  unsigned long *cmds);
> > -void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
> > +int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
> > +			      u32 *drained);
> >   void cxl_event_trace_record(struct cxl_memdev *cxlmd,
> >   			    enum cxl_event_log_type type,
> >   			    enum cxl_event_type event_type,
> > diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> > index 267c679b0b3c..239df18c9c87 100644
> > --- a/drivers/cxl/pci.c
> > +++ b/drivers/cxl/pci.c
> > @@ -509,26 +509,75 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
> >   	return true;
> >   }
> > +#define CXL_EVENT_DRAIN_ATTEMPTS 3
> > +
> > +/*
> > + * -EAGAIN is the device asking for a retry, the rest are the mailbox being
> > + * momentarily unavailable.  Everything else is permanent as far as this can
> > + * tell.
> > + */
> > +static bool cxl_event_drain_retryable(int rc)
> > +{
> > +	return rc == -EAGAIN || rc == -EBUSY || rc == -ETIMEDOUT;
> > +}
> > +
> >   static irqreturn_t cxl_event_thread(int irq, void *id)
> >   {
> >   	struct cxl_dev_id *dev_id = id;
> >   	struct cxl_dev_state *cxlds = dev_id->cxlds;
> >   	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> > -	u32 status;
> > +	u32 mask = CXLDEV_EVENT_STATUS_ALL;
> > +	int attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> > +
> > +	while (mask) {
> > +		u32 status, drained, stuck;
> > +		int rc;
> > -	do {
> >   		/*
> >   		 * CXL 3.0 8.2.8.3.1: The lower 32 bits are the status;
> >   		 * ignore the reserved upper 32 bits
> >   		 */
> >   		status = readl(cxlds->regs.status + CXLDEV_DEV_EVENT_STATUS_OFFSET);
> > -		/* Ignore logs unknown to the driver */
> > -		status &= CXLDEV_EVENT_STATUS_ALL;
> > +		/* Ignore logs unknown to the driver, and logs given up on */
> > +		status &= mask;
> >   		if (!status)
> >   			break;
> > -		cxl_mem_get_event_records(mds, status);
> > +
> > +		rc = cxl_mem_get_event_records(mds, status, &drained);
> > +		if (rc) {
> > +			if (!cxl_event_drain_retryable(rc)) {
> > +				dev_warn(cxlds->dev,
> > +					 "Event log drain failed: %d\n", rc);
> > +				break;
> > +			}
> > +			if (--attempts) {
> > +				fsleep(1000);
> > +				continue;
> > +			}
> > +			dev_warn(cxlds->dev,
> > +				 "Event log drain gave up after %d attempts: %d\n",
> > +				 CXL_EVENT_DRAIN_ATTEMPTS, rc);
> > +			break;
> > +		}
> > +		/* Progress was made, so reset number of attempts */
> > +		attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> > +
> > +		/*
> > +		 * The Event Status register is device owned and read only, so
> > +		 * it cannot bound this loop; records read can.  A device that
> > +		 * leaves a bit set with an empty log makes no progress, and
> > +		 * only the device can clear that state, so stop polling that
> > +		 * log rather than spin on it.
> > +		 */
> > +		stuck = status & ~drained;
> > +		if (stuck) {
> > +			dev_warn_once(cxlds->dev,
> > +				      "Event status %#x set with no records to read\n",
> > +				      stuck);
> > +			mask &= ~stuck;
> > +		}
> >   		cond_resched();
> > -	} while (status);
> > +	}
> >   	return IRQ_HANDLED;
> >   }
> > @@ -682,7 +731,7 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
> >   	if (rc)
> >   		return rc;
> > -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> > +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
> >   	return 0;
> >   }
> > diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> > index a7da279aa3ef..87e01612cea9 100644
> > --- a/tools/testing/cxl/test/mem.c
> > +++ b/tools/testing/cxl/test/mem.c
> > @@ -372,7 +372,7 @@ static void cxl_mock_event_trigger(struct device *dev)
> >   			event_reset_log(log);
> >   	}
> > -	cxl_mem_get_event_records(mdata->mds, mes->ev_status);
> > +	cxl_mem_get_event_records(mdata->mds, mes->ev_status, NULL);
> >   }
> >   struct cxl_event_record_raw maint_needed = {
> > @@ -1805,7 +1805,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
> >   	if (rc)
> >   		dev_dbg(dev, "No CXL FWCTL setup\n");
> > -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> > +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
> >   	cxl_mock_test_feat_init(mdata);
> >   	return 0;

Re: [PATCH v2 1/4] cxl/events: Bound get records loop in cxl_event_thread()

[Cheatham, Benjamin]

On 8/31/2026 7:26 PM, Anisa Su wrote:
> cxl_event_thread() drains all logs named in the Event Status register and
> repeats until the register reads zero. CXL r4.0 Section 8.2.9.3.1 Table 8-203
> marks the register RO and leaves clearing it to the device, so a misbehaving
> device that leaves a status bit set spins the thread forever, resending
> Get Event Records as fast as the mailbox completes.
> 
> Bound the loop on work completed instead. cxl_mem_get_records_log() and
> cxl_mem_get_event_records() report failures and which logs returned
> records; the thread drops any log whose status bit was set while it
> returned nothing, and caps consecutive errors at CXL_EVENT_DRAIN_ATTEMPTS.
> Every pass either drains a record, clears a bit from the mask or spends an
> attempt, so the loop eventually terminates, even if the device misbehaves.
> The first error is returned and an error from one log does not prevent
> getting logs from the rest.
> 
> Retrying is limited to the failures that can plausibly be cleared:
> -EBUSY and -ETIMEDOUT from the mailbox, and Retry Required from the
> device. Anything else gives up on the first error, so cycles are not wasted
> retrying on a non-retryable error.
> 
> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> 
> ---
> Changes:
> [Richard Cheng]: retry on transient failures
> ---
>  drivers/cxl/core/mbox.c      | 74 +++++++++++++++++++++++++++++-------
>  drivers/cxl/cxlmem.h         |  3 +-
>  drivers/cxl/pci.c            | 63 ++++++++++++++++++++++++++----
>  tools/testing/cxl/test/mem.c |  4 +-
>  4 files changed, 120 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 7c6c5b7450a5..2b71a8e1f35f 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -988,6 +988,18 @@ static void __cxl_event_trace_record(struct cxl_memdev *cxlmd,
>  	cxl_event_trace_record(cxlmd, type, ev_type, uuid, &record->event);
>  }
>  
> +/*
> + * cxl_mbox_cmd_rc2errno() collapses every device return code onto -ENXIO;
> + * pick out the one the spec asks the caller to retry.
> + */
> +static int cxl_event_retry_rc(struct cxl_mbox_cmd *mbox_cmd, int rc)
> +{
> +	if (rc == -ENXIO && mbox_cmd->return_code == CXL_MBOX_CMD_RC_RETRY)
> +		return -EAGAIN;
> +
> +	return rc;
> +}

I think it's more valuable to update cxl_mbox_cmd_rc2errno() to return -EAGAIN for that
mailbox code. It's a smaller change and is more reusable.

> +
>  static int cxl_clear_event_record(struct cxl_memdev_state *mds,
>  				  enum cxl_event_log_type log,
>  				  struct cxl_get_event_payload *get_pl)
> @@ -1056,11 +1068,11 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds,
>  
>  free_pl:
>  	kvfree(payload);
> -	return rc;
> +	return cxl_event_retry_rc(&mbox_cmd, rc);
>  }
>  
> -static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> -				    enum cxl_event_log_type type)
> +static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
> +				   enum cxl_event_log_type type, bool *got_records)
>  {
>  	struct cxl_mailbox *cxl_mbox = &mds->cxlds.cxl_mbox;
>  	struct cxl_memdev *cxlmd = mds->cxlds.cxlmd;
> @@ -1068,12 +1080,15 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  	struct cxl_get_event_payload *payload;
>  	u8 log_type = type;
>  	u16 nr_rec;
> +	int rc = 0;
> +
> +	*got_records = false;
>  
>  	mutex_lock(&mds->event.log_lock);

I was looking at patch 3/4 when I realized you can convert this to a guard() and then
return instead of using break statements below. Would be nice as a clean up, but not necessary.

>  	payload = mds->event.buf;
>  
>  	do {
> -		int rc, i;
> +		int i;
>  		struct cxl_mbox_cmd mbox_cmd = (struct cxl_mbox_cmd) {
>  			.opcode = CXL_MBOX_OP_GET_EVENT_RECORD,
>  			.payload_in = &log_type,
> @@ -1085,6 +1100,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  
>  		rc = cxl_internal_send_cmd(cxl_mbox, &mbox_cmd);
>  		if (rc) {
> +			rc = cxl_event_retry_rc(&mbox_cmd, rc);
>  			dev_err_ratelimited(dev,
>  				"Event log '%d': Failed to query event records : %d",
>  				type, rc);
> @@ -1094,6 +1110,7 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  		nr_rec = le16_to_cpu(payload->record_count);
>  		if (!nr_rec)
>  			break;
> +		*got_records = true;
>  
>  		for (i = 0; i < nr_rec; i++)
>  			__cxl_event_trace_record(cxlmd, type,
> @@ -1112,31 +1129,60 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  	} while (nr_rec);
>  
>  	mutex_unlock(&mds->event.log_lock);
> +
> +	return rc;
>  }
>  
>  /**
>   * cxl_mem_get_event_records - Get Event Records from the device
>   * @mds: The driver data for the operation
>   * @status: Event Status register value identifying which events are available.
> + * @drained: Optional mask of the logs in @status that returned records.
>   *
>   * Retrieve all event records available on the device, report them as trace
> - * events, and clear them.
> + * events, and clear them.  Every log named in @status is drained even if
> + * another one fails, so that a broken log does not suppress reporting from
> + * the rest.
> + *
> + * Return: 0, or the first error encountered.  -EAGAIN if the device asked for
> + * a command to be retried.
>   *
>   * See CXL rev 3.0 @8.2.9.2.2 Get Event Records
>   * See CXL rev 3.0 @8.2.9.2.3 Clear Event Records
>   */
> -void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status)
> +int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
> +			      u32 *drained)
>  {
> +	static const struct {
> +		u32 status;
> +		enum cxl_event_log_type type;
> +	} logs[] = {
> +		{ CXLDEV_EVENT_STATUS_FATAL, CXL_EVENT_TYPE_FATAL },
> +		{ CXLDEV_EVENT_STATUS_FAIL, CXL_EVENT_TYPE_FAIL },
> +		{ CXLDEV_EVENT_STATUS_WARN, CXL_EVENT_TYPE_WARN },
> +		{ CXLDEV_EVENT_STATUS_INFO, CXL_EVENT_TYPE_INFO },
> +	};
> +	int ret = 0;
> +
>  	dev_dbg(mds->cxlds.dev, "Reading event logs: %x\n", status);
>  
> -	if (status & CXLDEV_EVENT_STATUS_FATAL)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FATAL);
> -	if (status & CXLDEV_EVENT_STATUS_FAIL)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_FAIL);
> -	if (status & CXLDEV_EVENT_STATUS_WARN)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_WARN);
> -	if (status & CXLDEV_EVENT_STATUS_INFO)
> -		cxl_mem_get_records_log(mds, CXL_EVENT_TYPE_INFO);
> +	if (drained)
> +		*drained = 0;
> +
> +	for (int i = 0; i < ARRAY_SIZE(logs); i++) {
> +		bool got_records;
> +		int rc;
> +
> +		if (!(status & logs[i].status))
> +			continue;
> +
> +		rc = cxl_mem_get_records_log(mds, logs[i].type, &got_records);
> +		if (got_records && drained)
> +			*drained |= logs[i].status;
> +		ret = ret ?: rc;
> +	}
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_mem_get_event_records, "CXL");
>  
> diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
> index ed419d0c59f2..cb5372eb2d85 100644
> --- a/drivers/cxl/cxlmem.h
> +++ b/drivers/cxl/cxlmem.h
> @@ -803,7 +803,8 @@ void set_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>  				unsigned long *cmds);
>  void clear_exclusive_cxl_commands(struct cxl_memdev_state *mds,
>  				  unsigned long *cmds);
> -void cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status);
> +int cxl_mem_get_event_records(struct cxl_memdev_state *mds, u32 status,
> +			      u32 *drained);
>  void cxl_event_trace_record(struct cxl_memdev *cxlmd,
>  			    enum cxl_event_log_type type,
>  			    enum cxl_event_type event_type,
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 267c679b0b3c..239df18c9c87 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -509,26 +509,75 @@ static bool cxl_alloc_irq_vectors(struct pci_dev *pdev)
>  	return true;
>  }
>  
> +#define CXL_EVENT_DRAIN_ATTEMPTS 3
> +
> +/*
> + * -EAGAIN is the device asking for a retry, the rest are the mailbox being
> + * momentarily unavailable.  Everything else is permanent as far as this can
> + * tell.
> + */
> +static bool cxl_event_drain_retryable(int rc)
> +{
> +	return rc == -EAGAIN || rc == -EBUSY || rc == -ETIMEDOUT;
> +}
> +
>  static irqreturn_t cxl_event_thread(int irq, void *id)
>  {
>  	struct cxl_dev_id *dev_id = id;
>  	struct cxl_dev_state *cxlds = dev_id->cxlds;
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
> -	u32 status;
> +	u32 mask = CXLDEV_EVENT_STATUS_ALL;
> +	int attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> +
> +	while (mask) {
> +		u32 status, drained, stuck;
> +		int rc;

I'd put these declarations at the top of the function, there's not a reason to put them
here AFAICT.

>  
> -	do {
>  		/*
>  		 * CXL 3.0 8.2.8.3.1: The lower 32 bits are the status;
>  		 * ignore the reserved upper 32 bits
>  		 */
>  		status = readl(cxlds->regs.status + CXLDEV_DEV_EVENT_STATUS_OFFSET);
> -		/* Ignore logs unknown to the driver */
> -		status &= CXLDEV_EVENT_STATUS_ALL;
> +		/* Ignore logs unknown to the driver, and logs given up on */
> +		status &= mask;
>  		if (!status)
>  			break;
> -		cxl_mem_get_event_records(mds, status);
> +
> +		rc = cxl_mem_get_event_records(mds, status, &drained);
> +		if (rc) {
> +			if (!cxl_event_drain_retryable(rc)) {
> +				dev_warn(cxlds->dev,
> +					 "Event log drain failed: %d\n", rc);
> +				break;
> +			}
> +			if (--attempts) {
> +				fsleep(1000);
> +				continue;
> +			}
> +			dev_warn(cxlds->dev,
> +				 "Event log drain gave up after %d attempts: %d\n",
> +				 CXL_EVENT_DRAIN_ATTEMPTS, rc);
> +			break;
> +		}
> +		/* Progress was made, so reset number of attempts */
> +		attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> +
> +		/*
> +		 * The Event Status register is device owned and read only, so
> +		 * it cannot bound this loop; records read can.  A device that
> +		 * leaves a bit set with an empty log makes no progress, and
> +		 * only the device can clear that state, so stop polling that
> +		 * log rather than spin on it.
> +		 */
> +		stuck = status & ~drained;
> +		if (stuck) {
> +			dev_warn_once(cxlds->dev,
> +				      "Event status %#x set with no records to read\n",
> +				      stuck);

It may be good to make this an error print instead of warn. This only happens if something on the
device is broken and needs to be fixed, so it may be good to make it hard to miss.

After thinking about what Li said in the other thread, would it be appropriate to just error out here
instead of skipping the record type? My thinking here is it would increase the pressure on the device
vendor to fix their device/firmware instead of the kernel allowing a broken device. Of course that
comes with the downside of the remaining records not being read, but they aren't read anyway at the
moment.

> +			mask &= ~stuck;
> +		}
>  		cond_resched();
> -	} while (status);
> +	}
>  
>  	return IRQ_HANDLED;
>  }
> @@ -682,7 +731,7 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge,
>  	if (rc)
>  		return rc;
>  
> -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
>  
>  	return 0;
>  }
> diff --git a/tools/testing/cxl/test/mem.c b/tools/testing/cxl/test/mem.c
> index a7da279aa3ef..87e01612cea9 100644
> --- a/tools/testing/cxl/test/mem.c
> +++ b/tools/testing/cxl/test/mem.c
> @@ -372,7 +372,7 @@ static void cxl_mock_event_trigger(struct device *dev)
>  			event_reset_log(log);
>  	}
>  
> -	cxl_mem_get_event_records(mdata->mds, mes->ev_status);
> +	cxl_mem_get_event_records(mdata->mds, mes->ev_status, NULL);
>  }
>  
>  struct cxl_event_record_raw maint_needed = {
> @@ -1805,7 +1805,7 @@ static int cxl_mock_mem_probe(struct platform_device *pdev)
>  	if (rc)
>  		dev_dbg(dev, "No CXL FWCTL setup\n");
>  
> -	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL);
> +	cxl_mem_get_event_records(mds, CXLDEV_EVENT_STATUS_ALL, NULL);
>  	cxl_mock_test_feat_init(mdata);
>  
>  	return 0;

[PATCH v2 2/4] cxl/events: Validate the record count reported by the device

[Anisa Su]

cxl_mem_get_records_log() takes the record count straight out of the Get
Event Records payload and uses it to read from payload->records[].
An oversized count reads past that buffer. Both
__cxl_event_trace_record() and cxl_clear_event_record() use the record
count to iterate oer payload->records[].

Bound the count by mbox_cmd.size_out which is already sanitized by
 __cxl_pci_mbox_send_cmd(). Report a bad record count as an error rather than a
drained log, so a device repeating the bad count does not keep the event
thread re-reading it.

Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load")
Signed-off-by: Anisa Su <anisa.su@samsung.com>
---
 drivers/cxl/core/mbox.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 2b71a8e1f35f..02b8c33a74b6 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1110,6 +1110,14 @@ static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 		nr_rec = le16_to_cpu(payload->record_count);
 		if (!nr_rec)
 			break;
+
+		if (struct_size(payload, records, nr_rec) > mbox_cmd.size_out) {
+			dev_err_ratelimited(dev,
+				"Event log '%d': record count %u mismatch in %zu byte payload",
+				type, nr_rec, mbox_cmd.size_out);
+			rc = -EIO;
+			break;
+		}
 		*got_records = true;
 
 		for (i = 0; i < nr_rec; i++)
-- 
2.43.0

Re: [PATCH v2 2/4] cxl/events: Validate the record count reported by the device

[Cheatham, Benjamin]

On 8/31/2026 7:26 PM, Anisa Su wrote:
> cxl_mem_get_records_log() takes the record count straight out of the Get
> Event Records payload and uses it to read from payload->records[].
> An oversized count reads past that buffer. Both
> __cxl_event_trace_record() and cxl_clear_event_record() use the record
> count to iterate oer payload->records[].
		   ^ over> 
> Bound the count by mbox_cmd.size_out which is already sanitized by
>  __cxl_pci_mbox_send_cmd(). Report a bad record count as an error rather than a
> drained log, so a device repeating the bad count does not keep the event
> thread re-reading it.
> 
> Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load")
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> ---
>  drivers/cxl/core/mbox.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 2b71a8e1f35f..02b8c33a74b6 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
> @@ -1110,6 +1110,14 @@ static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  		nr_rec = le16_to_cpu(payload->record_count);
>  		if (!nr_rec)
>  			break;
> +
> +		if (struct_size(payload, records, nr_rec) > mbox_cmd.size_out) {
> +			dev_err_ratelimited(dev,
> +				"Event log '%d': record count %u mismatch in %zu byte payload",

The units should probably match here, so either the record count should be the size in bytes or the
size_out should be the number of records expected.

With that:
Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

> +				type, nr_rec, mbox_cmd.size_out);
> +			rc = -EIO;
> +			break;
> +		}
>  		*got_records = true;
>  
>  		for (i = 0; i < nr_rec; i++)

[PATCH v2 3/4] cxl/events: Bound the per-log Get Event Records loop

[Anisa Su]

cxl_mem_get_records_log() re-queries a log until the device reports it
empty. A misbehaving device that never stops reporting records keeps the
event thread stuck in this loop, and there is no cond_resched() to let anything
else on the CPU run.

Cap the passes at 128 and reschedule between them. Each pass gets up to
a full mailbox payload of records from the log, so the cap is a generous
upper bound. Report hitting the cap as an error and let the caller give
up on the log.

Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load")
Signed-off-by: Anisa Su <anisa.su@samsung.com>
---
 drivers/cxl/core/mbox.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
index 02b8c33a74b6..5e49589727d5 100644
--- a/drivers/cxl/core/mbox.c
+++ b/drivers/cxl/core/mbox.c
@@ -1071,6 +1071,8 @@ static int cxl_clear_event_record(struct cxl_memdev_state *mds,
 	return cxl_event_retry_rc(&mbox_cmd, rc);
 }
 
+#define CXL_EVENT_LOG_MAX_PASSES 128
+
 static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 				   enum cxl_event_log_type type, bool *got_records)
 {
@@ -1079,6 +1081,7 @@ static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 	struct device *dev = mds->cxlds.dev;
 	struct cxl_get_event_payload *payload;
 	u8 log_type = type;
+	int passes = 0;
 	u16 nr_rec;
 	int rc = 0;
 
@@ -1134,6 +1137,15 @@ static int cxl_mem_get_records_log(struct cxl_memdev_state *mds,
 				type, rc);
 			break;
 		}
+
+		if (++passes == CXL_EVENT_LOG_MAX_PASSES) {
+			dev_warn_ratelimited(dev,
+				"Event log '%d': Still reporting records after %d passes, giving up",
+				type, passes);
+			rc = -EIO;
+			break;
+		}
+		cond_resched();
 	} while (nr_rec);
 
 	mutex_unlock(&mds->event.log_lock);
-- 
2.43.0

Re: [PATCH v2 3/4] cxl/events: Bound the per-log Get Event Records loop

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Returning -EIO after capping the event log passes causes an infinite IRQ storm rather than letting the caller give up on the log.
--

commit f19a605d6e7399b93479014db2d4bf98656f2614
Author: Anisa Su <anisa.su887@gmail.com>

cxl/events: Bound the per-log Get Event Records loop

This commit bounds the cxl_mem_get_records_log() loop to prevent a misbehaving
device from permanently stalling the event thread. It introduces a 128-pass cap,
returning an error if the cap is reached, and yields the CPU via cond_resched()
between passes.

> diff --git a/drivers/cxl/core/mbox.c b/drivers/cxl/core/mbox.c
> index 7c6c5b7450a54..5e49589727d59 100644
> --- a/drivers/cxl/core/mbox.c
> +++ b/drivers/cxl/core/mbox.c
[ ... ]
> @@ -1109,34 +1137,72 @@ static void cxl_mem_get_records_log(struct cxl_memdev_state *mds,
>  				type, rc);
>  			break;
>  		}
> +
> +		if (++passes == CXL_EVENT_LOG_MAX_PASSES) {
> +			dev_warn_ratelimited(dev,
> +				"Event log '%d': Still reporting records after %d passes, giving up",
> +				type, passes);
> +			rc = -EIO;
> +			break;
> +		}
> +		cond_resched();
>  	} while (nr_rec);

[Severity: High]
Does returning -EIO here cause an infinite interrupt storm rather than letting
the caller give up on the log?

When the 128-pass cap is reached, cxl_mem_get_records_log() returns -EIO. This
propagates to cxl_event_thread(), which breaks out of its event reading loop
and returns IRQ_HANDLED.

Because the event log is not empty, the device will immediately fire another
MSI. Since the thread returned IRQ_HANDLED, the kernel's spurious interrupt
detector won't disable the interrupt line. This wakes up cxl_event_thread()
again, which performs another 128 passes and exits, repeating endlessly.

Should the interrupt be persistently disabled or the log masked when we give
up on reading it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901002912.958-1-anisa.su@samsung.com?part=3

Re: [PATCH v2 3/4] cxl/events: Bound the per-log Get Event Records loop

[Cheatham, Benjamin]

On 8/31/2026 7:26 PM, Anisa Su wrote:
> cxl_mem_get_records_log() re-queries a log until the device reports it
> empty. A misbehaving device that never stops reporting records keeps the
> event thread stuck in this loop, and there is no cond_resched() to let anything
> else on the CPU run.
> 
> Cap the passes at 128 and reschedule between them. Each pass gets up to
> a full mailbox payload of records from the log, so the cap is a generous
> upper bound. Report hitting the cap as an error and let the caller give
> up on the log.
> 
> Fixes: 6ebe28f9ec72 ("cxl/mem: Read, trace, and clear events on driver load")
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> ---

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

[PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed

[Anisa Su]

cxl_event_thread() returns IRQ_HANDLED whether or not it found
anything to do. Claiming every interrupt as handled means a device
flooding its MSI vector is never detected.

Return IRQ_NONE unless the Event Status register names a log to drain.

Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
Signed-off-by: Anisa Su <anisa.su@samsung.com>
---
 drivers/cxl/pci.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 239df18c9c87..94ff2b7d1f09 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -528,6 +528,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
 	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
 	u32 mask = CXLDEV_EVENT_STATUS_ALL;
 	int attempts = CXL_EVENT_DRAIN_ATTEMPTS;
+	bool handled = false;
 
 	while (mask) {
 		u32 status, drained, stuck;
@@ -542,6 +543,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
 		status &= mask;
 		if (!status)
 			break;
+		handled = true;
 
 		rc = cxl_mem_get_event_records(mds, status, &drained);
 		if (rc) {
@@ -579,7 +581,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
 		cond_resched();
 	}
 
-	return IRQ_HANDLED;
+	return handled ? IRQ_HANDLED : IRQ_NONE;
 }
 
 static int cxl_event_req_irq(struct cxl_dev_state *cxlds, u8 setting)
-- 
2.43.0

Re: [PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent execution of the shared IRQ thread for different CXL event vectors will incorrectly return IRQ_NONE, causing the kernel to permanently disable the IRQ.
--

commit e4fb9fa2ad67f892cc6a1e121480e607bbf7ed82
Author: Anisa Su <anisa.su887@gmail.com>

cxl/events: Return IRQ_NONE when no events were processed

This commit changes cxl_event_thread() to return IRQ_NONE when no events
are processed, rather than always returning IRQ_HANDLED. The stated intent
is to allow the kernel to detect devices flooding their MSI vectors.

> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 239df18c9c87b..94ff2b7d1f09a 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -528,6 +528,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  	struct cxl_memdev_state *mds = to_cxl_memdev_state(cxlds);
>  	u32 mask = CXLDEV_EVENT_STATUS_ALL;
>  	int attempts = CXL_EVENT_DRAIN_ATTEMPTS;
> +	bool handled = false;
>  
>  	while (mask) {
>  		u32 status, drained, stuck;
[ ... ]
> @@ -542,6 +543,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  		status &= mask;
>  		if (!status)
>  			break;
> +		handled = true;
>  
>  		rc = cxl_mem_get_event_records(mds, status, &drained);
>  		if (rc) {
[ ... ]
> @@ -579,7 +581,7 @@ static irqreturn_t cxl_event_thread(int irq, void *id)
>  		cond_resched();
>  	}
>  
> -	return IRQ_HANDLED;
> +	return handled ? IRQ_HANDLED : IRQ_NONE;
>  }

[Severity: High]
Will this cause concurrent event interrupts on multi-vector devices to be
permanently disabled?

If a CXL device maps different event logs to different MSI vectors (for
example, Info to vector 1 and Warn to vector 2), and both events occur in
quick succession, the first handler thread will drain both logs because
mask is initialized to CXLDEV_EVENT_STATUS_ALL.

When the second handler thread wakes up, it will read a status of 0, break
the loop without setting handled to true, and return IRQ_NONE.

Could this trigger the kernel's spurious interrupt detector during normal
operation under load, causing it to permanently disable the IRQ and break
hardware event reporting?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260901002912.958-1-anisa.su@samsung.com?part=4

Re: [PATCH v2 4/4] cxl/events: Return IRQ_NONE when no events were processed

[Cheatham, Benjamin]

On 8/31/2026 7:26 PM, Anisa Su wrote:
> cxl_event_thread() returns IRQ_HANDLED whether or not it found
> anything to do. Claiming every interrupt as handled means a device
> flooding its MSI vector is never detected.
> 
> Return IRQ_NONE unless the Event Status register names a log to drain.
> 
> Fixes: a49aa8141b65 ("cxl/mem: Wire up event interrupts")
> Signed-off-by: Anisa Su <anisa.su@samsung.com>
> ---

I'd mention that the MSI will eventually be disabled, but otherwise looks good
to me:

Reviewed-by: Ben Cheatham <benjamin.cheatham@amd.com>

Thanks,
Ben

Re: [PATCH v2 0/4] cxl/events: Robustify event interrupt handling

[Anisa Su]

On Mon, Aug 31, 2026 at 05:26:53PM -0700, Anisa Su wrote:
> Based on the 7.2 tag.
> 
> This patchset bundles various fixes along the event interrupt path.
> The previous revision was a single patch, but Sashiko reported several
> related pre-existing errors so I thought I may as well pick them all up into
> one series.
> 
> Link to v1:
> https://lore.kernel.org/linux-cxl/cover.1787768932.git.anisa.su@samsung.com/T/#m393e57d7f7f3916e8f2826a006ef0bc4dfd3606a
> 
Had a chat with Dave and he mentioned we only need to deal with device quirks
when they show up. So I plan to drop patches 2-4, since they are just Sashiko
reports and not observed on hardware.

For patch 1, there are a few considerations, so if any maintainers could have a quick
glance at the thread?

1. I did observe the bug on hardware, but it's a sample device and the
firmware is being patched, so nobody should see it happen IRL? The consequences
of the issue were:
	- 1 CPU taken up
	- driver can't be unbound since free_irq() waits for threads_active = 0

2. Li Ming mentioned checking the return value of cxl_mem_get_event_records()
is still valuable (currently a void function). So we could drop the other
changes from the patch and just keep that part.

Thanks,
Anisa

> Patch 1:
> ========
> cxl_event_thread() loops until the event status register is clear. The
> Event Status register is device owned and read-only (CXL r4.0 8.2.9.3.1 Table 8-203),
> so buggy device that never clears the register traps the thread
> in an infinite loop. A persistent error would hang the thread too.
> 
> The cxl_event_thread() now drops any log whose status bit was set
> while it returned 0 event records. Up to CXL_EVENT_DRAIN_ATTEMPTS
> number of retries are allowed for transient errors, such as "Retry
> Required" rc from the device, or mailbox -ETIMEDOUT/-EBUSY errors.
> 
> CXL_EVENT_DRAIN_ATTEMPTS is defined as 3.
> 
> Changes from v1:
> - added retries for transient errors, pointed out by Richard Cheng
> 
> Patch 2:
> ========
> Validates the record count the device reports. It is used
> unchecked to index a flexible array in a buffer sized to the mailbox
> payload, so an oversized count reads past the buffer, leaks the contents
> to tracepoints, and then hands the same bytes back to the device in Clear
> Event Records.
> 
> Patch 3:
> ========
> Patch 1 addressed the event thread loop, which happens once per IRQ.
> The event thread iterates over each of the event logs that have their
> status bit set (Informational, Warning, Failure, etc.) Then for each log,
> cxl_mem_get_records_log() gets event records until nr_rec reaches 0 for
> that log.
> 
> A bad device that keeps reporting >0 records would keep the thread stuck
> here.
> 
> This patch bounds the per-log loop by CXL_EVENT_LOG_MAX_PASSES, currently
> set to 128. Probably overkill for a large mailbox, since the command
> returns "as many even records... that fit into the mailbox output payload"
> (CXL r4.0 Section 8.2.10.2.2 Get Event Records), but a spec-minimum sized
> mailbox (256B) only fits 1 record, so I thought probably better to set the
> the limit more generously. But it could be lowered.
> 
> 
> Patch 4:
> ========
> Returns IRQ_NONE when the handler processed nothing instead of
> unconditionally returning IRQ_HANDLED.
> 
> Sashiko:
> "Returning IRQ_HANDLED when no work was done prevents the kernel's core IRQ
> subsystem from detecting and disabling a spurious interrupt storm. If a
> failing CXL device floods the CPU with MSI interrupts, the kernel will never
> disable the vector, which could completely lock up the processing core"
> 
> 
> Testing:
> ========
> 
> NDTL:
> ndctl's cxl suite (pmem/ndctl pending, 02754b5) against cxl_test:
> 
> 15 passed, 2 skipped, 0 failed.
> The two skips are cxl-type2.sh and cxl-features.sh, which are
> unrelated to this series.
> 
> QEMU Tests:
> All 4 patches have been tested on a QEMU branch modified to
> emulate each of the above scenarios. A test script starts a VM for each
> scenario and uses QMP to inject a general media event
> (cxl-inject-general-media-event) with DPA = 0x1000 and all other fields set to
> 0 to trigger the event interrupt.
> 
> Each error scenario can be turned on with an environment var in QEMU:
> 
> /*   CXL_TEST_STICKY_EVENT_STATUS=1  leave the Event Status bit set once the
>  *                                   log has been drained
>  *   CXL_TEST_EVENT_RETRY=1          answer Get/Clear Event Records with Retry
>  *                                   Required for every log
>  *   CXL_TEST_BAD_RECORD_COUNT=1     claim more records than the payload holds
>  *   CXL_TEST_EVENT_IRQ_STORM=1      raise the event interrupt without ever
>  *                                   putting a record in a log
>  *   CXL_TEST_ENDLESS_RECORDS=1      acknowledge Clear Event Records without
>  *                                   removing anything, so the log never drains
>  */
> static bool cxl_test_knob(const char *name)
> {
>     const char *val = getenv(name);
> 
>     return val && val[0] == '1';
> }
> 
> Then for example in cxl_event_delete_head(), if CXL_TEST_STICKY_EVENT_STATUS=1,
> we skip clearing the log status:
> 
> -    if (cxl_event_empty(log)) {
> +    if (cxl_event_empty(log) && !cxl_test_knob("CXL_TEST_STICKY_EVENT_STATUS")) {
>          cxl_event_set_status(cxlds, log_type, false);
>      }
> 
> 
> For more details on QEMU, see this commit:
> https://github.com/anisa-su993/qemu-anisa/commit/61960ab7fd7160226cd131a2cb1091b161c100bd
> 
> Test script:
> https://github.com/anisa-su993/cxl-tests/blob/main/events/run-all.sh
> 
> Patch 1:
> --------
>   CXL_TEST_STICKY_EVENT_STATUS=1  leave the status bit set after the
>                                   event log is cleared
> 
> <General Media Event injected w/QMP, triggering event interrupt on log 1>
> [    7.421124] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
> [    7.424094] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.426894] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.430098] cxl_core:cxl_clear_event_record:1048: cxl_pci 0000:0d:00.0: Event log '0': Clearing 1
> [    7.433004] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0101
> [    7.435831] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.438775] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.441527] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.444478] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
> [    7.447274] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.450102] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.453001] cxl_pci 0000:0d:00.0: Event status 0x1 set with no records to read <---- detected error and stops
> 
>   CXL_TEST_EVENT_RETRY=1          answer Get/Clear Event Records with
>                                   Retry Required for every log
> 
> <General Media Event injected w/QMP, triggering event interrupt on log 1>
> [    7.377325] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
> [    7.380459] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.383242] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.386105] cxl_pci:__cxl_pci_mbox_send_cmd:346: cxl_pci 0000:0d:00.0: Mailbox operation had an error: temporary error, retry once
> [    7.389834] cxl_pci 0000:0d:00.0: Event log '0': Failed to query event records : -11
> [    7.393652] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
> [    7.396574] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.399323] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.402217] cxl_pci:__cxl_pci_mbox_send_cmd:346: cxl_pci 0000:0d:00.0: Mailbox operation had an error: temporary error, retry once
> [    7.405949] cxl_pci 0000:0d:00.0: Event log '0': Failed to query event records : -11
> [    7.409794] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
> [    7.412716] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.415491] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.418399] cxl_pci:__cxl_pci_mbox_send_cmd:346: cxl_pci 0000:0d:00.0: Mailbox operation had an error: temporary error, retry once
> [    7.422016] cxl_pci 0000:0d:00.0: Event log '0': Failed to query event records : -11
> [    7.424199] cxl_pci 0000:0d:00.0: Event log drain gave up after 3 attempts: -11 <----- gave up after CXL_EVENT_DRAIN_ATTEMPTS
> 
> 
> Patch 2:
> --------
>   CXL_TEST_BAD_RECORD_COUNT=1     claim 1 more record than the payload holds
> 
> [    7.334917] cxl_core:cxl_mem_get_event_records:1187: cxl_pci 0000:0d:00.0: Reading event logs: 1
> [    7.337746] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    7.340545] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    7.343614] cxl_pci 0000:0d:00.0: Event log '0': record count 2 mismatch in 160 byte payload
> [    7.346197] cxl_pci 0000:0d:00.0: Event log drain failed: -5 <---- skip reading payload and return -EIO
> 
> Patch 3:
> --------
>   CXL_TEST_ENDLESS_RECORDS=1      acknowledge Clear Event Records without
>                                   removing anything, so the log is never emptied
> ... skipping some logs
> [    8.116284] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    8.117190] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    8.118218] cxl_core:cxl_clear_event_record:1048: cxl_pci 0000:0d:00.0: Event log '0': Clearing 1
> [    8.119212] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0101
> [    8.120130] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    8.121093] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0100
> [    8.121992] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    8.123088] cxl_core:cxl_clear_event_record:1048: cxl_pci 0000:0d:00.0: Event log '0': Clearing 1
> [    8.124027] cxl_pci:__cxl_pci_mbox_send_cmd:263: cxl_pci 0000:0d:00.0: Sending command: 0x0101
> [    8.124927] cxl_pci:cxl_pci_mbox_wait_for_doorbell:74: cxl_pci 0000:0d:00.0: Doorbell wait took 0ms
> [    8.125937] cxl_pci 0000:0d:00.0: Event log '0': Still reporting records after 128 passes, giving up <--- hit CXL_EVENT_LOG_MAX_PASSES ceiling
> [    8.126878] cxl_pci 0000:0d:00.0: Event log drain failed: -5
> 
> Patch 4:
> --------
>   CXL_TEST_EVENT_IRQ_STORM=1      raise the event interrupt without ever
>                                   putting a record in a log
> 
> ...
> [    9.929250] irq 28: nobody cared (try booting with the "irqpoll" option) <---- printed in __report_bad_irq()
> [    9.929937] CPU: 1 UID: 0 PID: 0 Comm: swapper/1 Not tainted 7.2.0+ #34 PREEMPT(full)
> [    9.929939] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.17.0-0-gb52ca86e094d-prebuilt.qemu.org 04/01/2014
> [    9.929940] Call Trace:
> [    9.930417]  <IRQ>
> ...
> [    9.931396]  </TASK>
> [    9.931397] handlers:
> [    9.945340] [<00000000a6051941>] irq_default_primary_handler threaded [<00000000c56a4fd3>] cxl_event_thread <----- handler is cxl_event_thread
> [    9.946222] Disabling IRQ #28
> 
> __report_bad_irq() turns off the interrupt "if 99,900 of the previous 100,000 interrupts
> have not been handled". With the fix, we see that__report_bad_irq() is reached.
> Without the fix (IRQ_HANDLED unconditionally returned), the interrupt is
> not disabled and keeps firing.
> 
> 
> Anisa Su (4):
>   cxl/events: Bound get records loop in cxl_event_thread()
>   cxl/events: Validate the record count reported by the device
>   cxl/events: Bound the per-log Get Event Records loop
>   cxl/events: Return IRQ_NONE when no events were processed
> 
>  drivers/cxl/core/mbox.c      | 94 ++++++++++++++++++++++++++++++------
>  drivers/cxl/cxlmem.h         |  3 +-
>  drivers/cxl/pci.c            | 67 ++++++++++++++++++++++---
>  tools/testing/cxl/test/mem.c |  4 +-
>  4 files changed, 143 insertions(+), 25 deletions(-)
> 
> 
> base-commit: 8d3ae59288f1e7d58d76558a6ee96d533bc5019f
> -- 
> 2.43.0
> 

Re: [PATCH v2 0/4] cxl/events: Robustify event interrupt handling

[Davidlohr Bueso]

On Wed, 02 Sep 2026, Anisa Su wrote:

>On Mon, Aug 31, 2026 at 05:26:53PM -0700, Anisa Su wrote:
>> Based on the 7.2 tag.
>>
>> This patchset bundles various fixes along the event interrupt path.
>> The previous revision was a single patch, but Sashiko reported several
>> related pre-existing errors so I thought I may as well pick them all up into
>> one series.
>>
>> Link to v1:
>> https://lore.kernel.org/linux-cxl/cover.1787768932.git.anisa.su@samsung.com/T/#m393e57d7f7f3916e8f2826a006ef0bc4dfd3606a
>>
>Had a chat with Dave and he mentioned we only need to deal with device quirks
>when they show up. So I plan to drop patches 2-4, since they are just Sashiko
>reports and not observed on hardware.
>
>For patch 1, there are a few considerations, so if any maintainers could have a quick
>glance at the thread?
>
>1. I did observe the bug on hardware, but it's a sample device and the
>firmware is being patched, so nobody should see it happen IRL? The consequences
>of the issue were:

I think this says it all, and therefore is not a quirk imo. We should not cater
to flat out spec violations (for non-public hw/fw) - there are plenty of things
that could easily break all over the place if bits are not set/unset by hardware
when it is supposed to.

So I would pretend this never occurred, and if someone ever complains (it was seen
IRL) we have your solution at hand.

>	- 1 CPU taken up
>	- driver can't be unbound since free_irq() waits for threads_active = 0
>
>2. Li Ming mentioned checking the return value of cxl_mem_get_event_records()
>is still valuable (currently a void function). So we could drop the other
>changes from the patch and just keep that part.

Yes, I agree.

Thanks,
Davidlohr