Enable CXL PCIe Port Protocol Error handling and logging

---

Changes in v18 -> v19:
- New patch, split from "cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register". Addresses out-of-bounds RCRB MMIO read that Sashiko flagged against the combined v18 patch.

---
 drivers/cxl/core/ras_rch.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

drivers/cxl/core/ras_rch.c +16 -1

index 0a8b3b9b63884..8a3965edaa12c 100644
@@ -58,13 +58,28 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport)
 static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 				 struct aer_capability_regs *aer_regs)
 {
-	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
+	/*
+	 * Bound the copy to the physically-defined AER registers (header
+	 * through the 16-byte Header Log). struct aer_capability_regs is a
+	 * software layout whose embedded struct pcie_tlp_log is larger than
+	 * the on-wire AER capability; copying sizeof(*aer_regs) would
+	 * over-read the RCRB-mapped MMIO block.
+	 */
+	int read_cnt = (PCI_ERR_HEADER_LOG + 16) / sizeof(u32);
 	u32 *aer_regs_buf = (u32 *)aer_regs;
 	int n;
 
 	if (!aer_base)
 		return false;
 
+	/*
+	 * Zero the destination so the software-only tail fields
+	 * (e.g. header_log.header_len) are deterministic rather than
+	 * left as uninitialized stack, which could drive a bogus loop
+	 * length in pcie_print_tlp_log().
+	 */
+	memset(aer_regs, 0, sizeof(*aer_regs));
+
 	/* Use readl() to guarantee 32-bit accesses */
 	for (n = 0; n < read_cnt; n++)
 		aer_regs_buf[n] = readl(aer_base + n * sizeof(u32));

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:42 UTC #2 lore raw

Re: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] cxl_dport_map_rch_aer() requests an MMIO region using the software-only size of struct aer_capability_regs, over-reserving memory beyond the physical capability. --

--- Patch [1]: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read --- commit b208d5e7d4a18c48dd06f33bc3c5667e9a2c13dd
Author: Terry Bowman <[email protected]>

cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read

Quoted 7 lines

drivers/cxl/core/ras_rch.c +0 -0

index 0a8b3b9b63884..8a3965edaa12c 100644
@@ -58,13 +58,28 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport)
 static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 				 struct aer_capability_regs *aer_regs)

drivers/cxl/core/ras_rch.c void cxl_disable_rch_root_ints(struct cxl_dport *dport) · line 68

Quoted 10 lines

drivers/cxl/core/ras_rch.c +8 -1

 {
-	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
+	/*
+	 * Bound the copy to the physically-defined AER registers (header
+	 * through the 16-byte Header Log). struct aer_capability_regs is a
+	 * software layout whose embedded struct pcie_tlp_log is larger than
+	 * the on-wire AER capability; copying sizeof(*aer_regs) would
+	 * over-read the RCRB-mapped MMIO block.
+	 */
+	int read_cnt = (PCI_ERR_HEADER_LOG + 16) / sizeof(u32);

[Severity: High]
This is a pre-existing issue, but since you are fixing the oversized read here, should we also fix the initial mapping size in cxl_dport_map_rch_aer()?

In drivers/cxl/core/ras_rch.c:cxl_dport_map_rch_aer():

	dport->regs.dport_aer =
		devm_cxl_iomap_block(host, aer_phys,
				     sizeof(struct aer_capability_regs));

This uses the same oversized software struct size to request the MMIO region.

If another CXL Extended Capability is densely packed immediately following the AER block, won't any subsequent driver attempting to map it (e.g., cxl_dport_map_rcd_linkcap()) fail with a -EBUSY conflict and break device initialization?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1

Bowman, Terry

author

2026-08-04 16:20 UTC #3 lore raw

Re: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read

Quoted 46 lines · On 8/3/2026 5:42 PM, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] cxl_dport_map_rch_aer() requests an MMIO region using the software-only size of struct aer_capability_regs, over-reserving memory beyond the physical capability. --
--- Patch [1]: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read --- commit b208d5e7d4a18c48dd06f33bc3c5667e9a2c13dd
Author: Terry Bowman <[email protected]>
cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read
cxl_rch_get_aer_info() copies the RCH Downstream Port AER capability from the RCRB MMIO block using a readl() loop bounded by sizeof(struct aer_capability_regs). This struct is a software layout and its embedded struct pcie_tlp_log is larger than the on-wire AER capability. As a result the loop reads past the mapped AER register block.
Quoted 7 lines
drivers/cxl/core/ras_rch.c +0 -0
index 0a8b3b9b63884..8a3965edaa12c 100644
@@ -58,13 +58,28 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport)
 static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 				 struct aer_capability_regs *aer_regs)
drivers/cxl/core/ras_rch.c void cxl_disable_rch_root_ints(struct cxl_dport *dport) · line 68
Quoted 10 lines
drivers/cxl/core/ras_rch.c +8 -1
 {
-	int read_cnt = sizeof(struct aer_capability_regs) / sizeof(u32);
+	/*
+	 * Bound the copy to the physically-defined AER registers (header
+	 * through the 16-byte Header Log). struct aer_capability_regs is a
+	 * software layout whose embedded struct pcie_tlp_log is larger than
+	 * the on-wire AER capability; copying sizeof(*aer_regs) would
+	 * over-read the RCRB-mapped MMIO block.
+	 */
+	int read_cnt = (PCI_ERR_HEADER_LOG + 16) / sizeof(u32);
[Severity: High]
This is a pre-existing issue, but since you are fixing the oversized read here, should we also fix the initial mapping size in cxl_dport_map_rch_aer()?
In drivers/cxl/core/ras_rch.c:cxl_dport_map_rch_aer():
	dport->regs.dport_aer =
		devm_cxl_iomap_block(host, aer_phys,
				     sizeof(struct aer_capability_regs));

Quoted 7 lines

This uses the same oversized software struct size to request the MMIO region.
If another CXL Extended Capability is densely packed immediately following the AER block, won't any subsequent driver attempting to map it (e.g., cxl_dport_map_rcd_linkcap()) fail with a -EBUSY conflict and break device initialization?

I will be adding a patch fix to v20 for the existing cxl_dport_map_rch_aer() issue reported by sashiko.

- Terry

Alison Schofield

maintainer Reviewed-by on v19 1/14

2026-08-04 02:11 UTC #4 lore raw

Re: [PATCH v19 01/14] cxl/ras: Fix cxl_rch_get_aer_info() out-of-bounds AER register read

Quoted 11 lines · Terry Bowman

cxl_rch_get_aer_info() copies the RCH Downstream Port AER capability from the RCRB MMIO block using a readl() loop bounded by sizeof(struct aer_capability_regs). This struct is a software layout and its embedded struct pcie_tlp_log is larger than the on-wire AER capability. As a result the loop reads past the mapped AER register block.
The over-read also populates the software-only tail fields including header_log.header_len. An out-of-range header_len passed to pcie_print_tlp_log() can then loop past the header log buffer and cause a second out-of-bounds read.

Quoted 8 lines

The read was correct when introduced, but struct pcie_tlp_log has since grown (Header Log and TLP Prefix Log sizes, header_len and flit fields), so sizeof(struct aer_capability_regs) no longer matches the physical AER capability.
Bound the read to the physical AER registers, header through the 16 byte Header Log. Zero the destination first so the software-only fields are deterministic.

Terry Bowman

author

2026-08-03 22:18 UTC #5 lore raw

PATCH v19 2/14 cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

cxl_rch_get_aer_severity() classifies RCH Downstream Port uncorrectable errors as fatal or non-fatal by ANDing uncorrectable status with PCI_ERR_ROOT_FATAL_RCV. This is wrong because PCI_ERR_ROOT_FATAL_RCV is a Root Error Status register bit (bit 6), not a severity bit. ANDing it against uncorrectable status tests a reserved bit and produces incorrect severity classification.

Fix by ANDing the unmasked uncor_status against uncor_severity. Per PCIe Base Spec r6.0 Section 7.8.4.4, each bit in the Uncorrectable Error Severity register indicates whether the corresponding error is fatal (1) or non-fatal (0).

Fixes 6ac07883dbb5 ("cxl/pci: Add RCH downstream port error logging")

Cc [email protected]

Reviewed-by Richard Cheng <[email protected]>

---

Changes in v18 -> v19:
- Use local variable for expression in cxl_rch_get_aer_severity()
- Add review-by for DaveJ and Richard Cheng

Changes in v17 -> v18:
- New patch.

---
 drivers/cxl/core/ras_rch.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

drivers/cxl/core/ras_rch.c +5 -5

index 8a3965edaa12c..e0e01aa5eba6c 100644
@@ -94,11 +94,11 @@ static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
 				     int *severity)
 {
-	if (aer_regs->uncor_status & ~aer_regs->uncor_mask) {
-		if (aer_regs->uncor_status & PCI_ERR_ROOT_FATAL_RCV)
-			*severity = AER_FATAL;
-		else
-			*severity = AER_NONFATAL;
+	u32 uncor_status = aer_regs->uncor_status & ~aer_regs->uncor_mask;
+
+	if (uncor_status) {
+		*severity = (uncor_status & aer_regs->uncor_severity) ?
+			     AER_FATAL : AER_NONFATAL;
 		return true;
 	}

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:36 UTC #6 lore raw

Re: [PATCH v19 02/14] cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 7 lines

cxl_rch_get_aer_severity() classifies RCH Downstream Port uncorrectable errors as fatal or non-fatal by ANDing uncorrectable status with PCI_ERR_ROOT_FATAL_RCV. This is wrong because PCI_ERR_ROOT_FATAL_RCV is a Root Error Status register bit (bit 6), not a severity bit. ANDing it against uncorrectable status tests a reserved bit and produces incorrect severity classification.

Quoted 8 lines

Fix by ANDing the unmasked uncor_status against uncor_severity. Per PCIe Base Spec r6.0 Section 7.8.4.4, each bit in the Uncorrectable Error Severity register indicates whether the corresponding error is fatal (1) or non-fatal (0).
Fixes 6ac07883dbb5 ("cxl/pci: Add RCH downstream port error logging")
Cc [email protected]
Signed-off-by Terry Bowman <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Alison Schofield

maintainer Reviewed-by on v19 2/14

2026-08-04 02:12 UTC #7 lore raw

Re: [PATCH v19 02/14] cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

Quoted 11 lines · Terry Bowman

cxl_rch_get_aer_severity() classifies RCH Downstream Port uncorrectable errors as fatal or non-fatal by ANDing uncorrectable status with PCI_ERR_ROOT_FATAL_RCV. This is wrong because PCI_ERR_ROOT_FATAL_RCV is a Root Error Status register bit (bit 6), not a severity bit. ANDing it against uncorrectable status tests a reserved bit and produces incorrect severity classification.
Fix by ANDing the unmasked uncor_status against uncor_severity. Per PCIe Base Spec r6.0 Section 7.8.4.4, each bit in the Uncorrectable Error Severity register indicates whether the corresponding error is fatal (1) or non-fatal (0).

Lukas Wunner

2026-08-09 15:57 UTC #8 lore raw

Re: [PATCH v19 02/14] cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

Quoted 8 lines · Terry Bowman

+++ b/drivers/cxl/core/ras_rch.c

drivers/cxl/core/ras_rch.c +0 -3

@@ -94,11 +94,11 @@ static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
 				     int *severity)
 {
-	if (aer_regs->uncor_status & ~aer_regs->uncor_mask) {
-		if (aer_regs->uncor_status & PCI_ERR_ROOT_FATAL_RCV)
-			*severity = AER_FATAL;

drivers/cxl/core/ras_rch.c static bool cxl_rch_get_aer_info(void __iomem *aer_base, · line 104

Quoted 10 lines

drivers/cxl/core/ras_rch.c +5 -2

-		else
-			*severity = AER_NONFATAL;
+	u32 uncor_status = aer_regs->uncor_status & ~aer_regs->uncor_mask;
+
+	if (uncor_status) {
+		*severity = (uncor_status & aer_regs->uncor_severity) ?
+			     AER_FATAL : AER_NONFATAL;
 		return true;
 	}

Independently of this patch, I'm wondering why the severity is inferred from the AER registers. I would assume that the severity always equals the message received by the RCEC (ERR_COR, ERR_NONFATAL or ERR_FATAL).

So the severity could be passed to cxl_handle_rdport_errors() from its
callers: cxl_cor_err_detected() would pass AER_CORRECTABLE and cxl_error_detected() would pass ERR_NONFATAL or ERR_FATAL (depending on the "state" variable).

cxl_handle_rdport_errors() would no longer need to call cxl_rch_get_aer_severity(), so the latter could be removed.

Am I missing something? Is a scenario ever conceivable where the RCEC receives a message with different severity than what is inferred from the registers by cxl_rch_get_aer_severity()?

And a related question: linux-next commit 21963e6e4e04 ("PCI/AER:
Support Advisory Non-Fatal Errors") enables support for Non-Fatal Errors which are signaled with an ERR_COR message.

These so-called Advisory Non-Fatal Errors set one bit in the Correctable Error Status Register (Advisory Non-Fatal Error Status, bit 13) and additionally one or more bits in the Uncorrectable Error Status Register (see the commit for details).

cxl_rch_get_aer_severity() is not able to cope with such errors and will incorrectly infer that the severity is AER_NONFATAL.

Now I *think* this is not a problem because Advisory Non-Fatal Errors are masked by default and the commit only unmasks them on regular PCI devices, not in the RCRB of a CXL device. Only once bit 13 in the Correctable Error Mask Register is cleared in the RCRB will cxl_rch_get_aer_severity() fail. Right?

Thanks,

Lukas

Lukas Wunner

2026-09-03 08:43 UTC #9 lore raw

Re: [PATCH v19 02/14] cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

Hi Terry,

You've submitted v20 of this series, but the following questions haven't been addressed yet AFAICS. I'd be grateful if you could take a look:

Quoted 3 lines · Lukas Wunner

Am I missing something? Is a scenario ever conceivable where the RCEC receives a message with different severity than what is inferred from the registers by cxl_rch_get_aer_severity()?

[...]

Quoted 5 lines

These so-called Advisory Non-Fatal Errors set one bit in the Correctable Error Status Register (Advisory Non-Fatal Error Status, bit 13) and additionally one or more bits in the Uncorrectable Error Status Register (see the commit for details).

Quoted 8 lines

cxl_rch_get_aer_severity() is not able to cope with such errors and will incorrectly infer that the severity is AER_NONFATAL.
Now I *think* this is not a problem because Advisory Non-Fatal Errors are masked by default and the commit only unmasks them on regular PCI devices, not in the RCRB of a CXL device. Only once bit 13 in the Correctable Error Mask Register is cleared in the RCRB will cxl_rch_get_aer_severity() fail. Right?

Thanks,

Lukas

Terry Bowman

2026-09-07 14:52 UTC #10 lore raw

Re: [PATCH v19 02/14] cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

Quoted 25 lines · Lukas Wunner

Hi Terry,
You've submitted v20 of this series, but the following questions haven't been addressed yet AFAICS. I'd be grateful if you could take a look:
Quoted 3 lines · Lukas Wunner
Am I missing something? Is a scenario ever conceivable where the RCEC receives a message with different severity than what is inferred from the registers by cxl_rch_get_aer_severity()?
[...]
Quoted 5 lines
These so-called Advisory Non-Fatal Errors set one bit in the Correctable Error Status Register (Advisory Non-Fatal Error Status, bit 13) and additionally one or more bits in the Uncorrectable Error Status Register (see the commit for details).
Quoted 8 lines
cxl_rch_get_aer_severity() is not able to cope with such errors and will incorrectly infer that the severity is AER_NONFATAL.
Now I *think* this is not a problem because Advisory Non-Fatal Errors are masked by default and the commit only unmasks them on regular PCI devices, not in the RCRB of a CXL device. Only once bit 13 in the Correctable Error Mask Register is cleared in the RCRB will cxl_rch_get_aer_severity() fail. Right?

Quoted 3 lines

Thanks,
Lukas

Hi Lukas,

I’m sorry for missing your August 9 reply. Also, I am currently on PTO this week and don’t have my work laptop, so I am responding from my personal email account. I’ll be back in the office Wednesday.

Thanks for pointing out the issue with CE and UCE errors reported simultaneously. I agree this is a problem and have addressed it in v20.

In v20 I remove the unused cxl_rch_get_aer_severity() helper, take a snapshot of the AER registers before calling pci_print_aer(), and now handle the CE and UCE parts in separate blocks. The UCE status is logged unconditionally, which prevents a fatal record from being lost under the advisory‑non‑fatal path. These changes directly address the scenario you raised for v19 and also cover any other situation where a CE and a UCE are co‑logged. The patch ishere: <https://lkml.org/lkml/2026/9/2/1629>
(v20 3/9).

Please let me know if you see any remaining concerns or corner cases that are not yet covered. Thanks again for your review and for pointing out the issue.

- Terry

Bowman, Terry

author

2026-09-09 16:13 UTC #11 lore raw

Re: [PATCH v19 02/14] cxl/ras: Fix cxl_rch_get_aer_severity() wrong severity register

In reply to Lukas Wunner

Quoted 25 lines · Lukas Wunner

Hi Terry,
You've submitted v20 of this series, but the following questions haven't been addressed yet AFAICS. I'd be grateful if you could take a look:
Quoted 3 lines · Lukas Wunner
Am I missing something? Is a scenario ever conceivable where the RCEC receives a message with different severity than what is inferred from the registers by cxl_rch_get_aer_severity()?
[...]
Quoted 5 lines
These so-called Advisory Non-Fatal Errors set one bit in the Correctable Error Status Register (Advisory Non-Fatal Error Status, bit 13) and additionally one or more bits in the Uncorrectable Error Status Register (see the commit for details).
Quoted 8 lines
cxl_rch_get_aer_severity() is not able to cope with such errors and will incorrectly infer that the severity is AER_NONFATAL.
Now I *think* this is not a problem because Advisory Non-Fatal Errors are masked by default and the commit only unmasks them on regular PCI devices, not in the RCRB of a CXL device. Only once bit 13 in the Correctable Error Mask Register is cleared in the RCRB will cxl_rch_get_aer_severity() fail. Right?

Quoted 3 lines

Thanks,
Lukas

Hi Lukas,

I’m sorry for missing your August 9 reply.

Thanks for pointing out the issue with CE and UCE errors reported simultaneously. I agree this is a problem and have addressed it in v20.

In v20 I remove the unused cxl_rch_get_aer_severity() helper, take a snapshot of the AER registers before calling pci_print_aer(), and now handle the CE and UCE parts in separate blocks. The UCE status is logged unconditionally, which prevents a fatal record from being lost under the advisory‑non‑fatal path. These changes directly address the scenario you raised for v19 and also cover any other situation where a CE and a UCE are co‑logged. The patch is here: <https://lkml.org/lkml/2026/9/2/1629>
(v20 3/9).

Jonathan added a comment looking for your feedback on if the issue should be solved by changing pci_print_aer() to be non-destructive instead. It is here:
https://lore.kernel.org/linux-cxl/20260908020608.56e8c443@jic23-huawei/

- Terry

Terry Bowman

author

2026-08-03 22:19 UTC #12 lore raw

PATCH v19 3/14 acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

The CXL CPER work registration and unregistration helpers acquire cxl_cper_work_lock and cxl_cper_prot_err_work_lock with a spinlock guard(), which leaves local interrupts enabled. The corresponding post paths (cxl_cper_post_event(), cxl_cper_post_prot_err()) execute in hard IRQ context (they are called from the GHES error notification path) and acquire the same locks with an irqsave guard().

If a CPU is holding one of these locks via a spinlock guard() when a GHES interrupt arrives on the same CPU, the IRQ handler spins on the held lock waiting for it to release, while the lock holder is preempted by the IRQ. The result is a deadlock.

Convert both locks from spinlock_t to raw_spinlock_t and use guard() at all call sites. On PREEMPT_RT kernels spinlock_t is backed by rt_mutex and sleeping from hard IRQ context is not permitted; raw_spinlock_t is safe in both contexts.

Add WARN_ONCE to both register functions to surface double-registration bugs at runtime.

Restructure both unregister functions to clear the global work pointer under the lock before calling cancel_work_sync(), closing the window where a CPER interrupt could schedule work on a pointer about to be freed. Add kfifo_reset() after cancel_work_sync() so stale entries are not replayed on next module load.

Both kfifos are single-consumer: only one work_struct is registered at a time, enforced by the WARN_ONCE guard in the register functions. kfifo_reset() is safe outside the lock because cancel_work_sync() has already quiesced the consumer, and no new consumer can register until the current module exit completes and a fresh module init runs.

Remove the redundant cancel_work_sync() call from cxl_ras_exit() and cxl_pci_driver_exit(). The CPER unregister functions now quiesce the work internally.

Reported-by Sashiko <[email protected]>

Fixes 5e4a264bf8b5 ("acpi/ghes: Process CXL Component Events")

Fixes 36f257e3b0ba ("acpi/ghes, cxl/pci: Process CXL CPER Protocol Errors")

Cc [email protected]

---

Changes in v18 -> v19:
- Remove the redundant cancel_work_sync() from cxl_ras_exit() and cxl_pci_driver_exit().
- Add review-by for DaveJ and Jonathan

Changes in v17 -> v18:
- New patch.

---
 drivers/acpi/apei/ghes.c | 50 ++++++++++++++++++++++++++--------------
 drivers/cxl/core/ras.c   |  1 -
 drivers/cxl/pci.c        |  1 -
 3 files changed, 33 insertions(+), 19 deletions(-)

drivers/acpi/apei/ghes.c +33 ~~-17~~

index 3236a3ce79d6b..ca7a138c1ff2e 100644
@@ -749,7 +749,7 @@ static DEFINE_KFIFO(cxl_cper_prot_err_fifo, struct cxl_cper_prot_err_work_data,
 		    CXL_CPER_PROT_ERR_FIFO_DEPTH);
 
 /* Synchronize schedule_work() with cxl_cper_prot_err_work changes */
-static DEFINE_SPINLOCK(cxl_cper_prot_err_work_lock);
+static DEFINE_RAW_SPINLOCK(cxl_cper_prot_err_work_lock);
 struct work_struct *cxl_cper_prot_err_work;
 
 static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,

@@ -761,7 +761,7 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
 	if (cxl_cper_sec_prot_err_valid(prot_err))
 		return;
 
-	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
+	guard(raw_spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
 
 	if (!cxl_cper_prot_err_work)
 		return;

@@ -780,10 +780,11 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
 
 int cxl_cper_register_prot_err_work(struct work_struct *work)
 {
-	if (cxl_cper_prot_err_work)
-		return -EINVAL;
+	guard(raw_spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
 
-	guard(spinlock)(&cxl_cper_prot_err_work_lock);
+	if (WARN_ONCE(cxl_cper_prot_err_work,
+		      "CPER-CXL kfifo consumer already registered\n"))
+		return -EINVAL;
 	cxl_cper_prot_err_work = work;
 	return 0;
 }

@@ -791,11 +792,18 @@ EXPORT_SYMBOL_NS_GPL(cxl_cper_register_prot_err_work, "CXL");
 
 int cxl_cper_unregister_prot_err_work(struct work_struct *work)
 {
-	if (cxl_cper_prot_err_work != work)
-		return -EINVAL;
+	scoped_guard(raw_spinlock_irqsave, &cxl_cper_prot_err_work_lock) {
+		if (WARN_ONCE(cxl_cper_prot_err_work != work,
+			      "CPER-CXL kfifo consumer mismatch on unregister\n"))
+			return -EINVAL;
+		cxl_cper_prot_err_work = NULL;
+	}
+
+	cancel_work_sync(work);
+
+	/* Discard stale entries so they are not replayed on next module load */
+	kfifo_reset(&cxl_cper_prot_err_fifo);
 
-	guard(spinlock)(&cxl_cper_prot_err_work_lock);
-	cxl_cper_prot_err_work = NULL;
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_prot_err_work, "CXL");

@@ -811,7 +819,7 @@ EXPORT_SYMBOL_NS_GPL(cxl_cper_prot_err_kfifo_get, "CXL");
 DEFINE_KFIFO(cxl_cper_fifo, struct cxl_cper_work_data, CXL_CPER_FIFO_DEPTH);
 
 /* Synchronize schedule_work() with cxl_cper_work changes */
-static DEFINE_SPINLOCK(cxl_cper_work_lock);
+static DEFINE_RAW_SPINLOCK(cxl_cper_work_lock);
 struct work_struct *cxl_cper_work;
 
 static void cxl_cper_post_event(enum cxl_event_type event_type,

@@ -831,7 +839,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
 		return;
 	}
 
-	guard(spinlock_irqsave)(&cxl_cper_work_lock);
+	guard(raw_spinlock_irqsave)(&cxl_cper_work_lock);
 
 	if (!cxl_cper_work)
 		return;

@@ -849,10 +857,11 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
 
 int cxl_cper_register_work(struct work_struct *work)
 {
-	if (cxl_cper_work)
+	guard(raw_spinlock_irqsave)(&cxl_cper_work_lock);
+	if (WARN_ONCE(cxl_cper_work,
+		      "CXL CPER kfifo consumer already registered\n"))
 		return -EINVAL;
 
-	guard(spinlock)(&cxl_cper_work_lock);
 	cxl_cper_work = work;
 	return 0;
 }

@@ -860,11 +869,18 @@ EXPORT_SYMBOL_NS_GPL(cxl_cper_register_work, "CXL");
 
 int cxl_cper_unregister_work(struct work_struct *work)
 {
-	if (cxl_cper_work != work)
-		return -EINVAL;
+	scoped_guard(raw_spinlock_irqsave, &cxl_cper_work_lock) {
+		if (WARN_ONCE(cxl_cper_work != work,
+			      "CXL CPER kfifo consumer mismatch on unregister\n"))
+			return -EINVAL;
+		cxl_cper_work = NULL;
+	}
+
+	cancel_work_sync(work);
+
+	/* Discard stale entries so they are not replayed on next module load */
+	kfifo_reset(&cxl_cper_fifo);
 
-	guard(spinlock)(&cxl_cper_work_lock);
-	cxl_cper_work = NULL;
 	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_work, "CXL");

drivers/cxl/core/ras.c +0 -1

index 99fb00949c2fa..bc74d4848132e 100644
@@ -137,7 +137,6 @@ int cxl_ras_init(void)
 void cxl_ras_exit(void)
 {
 	cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
-	cancel_work_sync(&cxl_cper_prot_err_work);
 }
 
 static void cxl_dport_map_ras(struct cxl_dport *dport)

drivers/cxl/pci.c +0 -1

index 267c679b0b3c2..7c6faee7f85ed 100644
@@ -1083,7 +1083,6 @@ static int __init cxl_pci_driver_init(void)
 static void __exit cxl_pci_driver_exit(void)
 {
 	cxl_cper_unregister_work(&cxl_cper_work);
-	cancel_work_sync(&cxl_cper_work);
 	pci_unregister_driver(&cxl_pci_driver);
 }

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:39 UTC #13 lore raw

Re: [PATCH v19 03/14] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing bounds checks on CPER CXL event and protocol error payloads can cause out-of-bounds reads, potentially leaking kernel memory via trace events or crashing the kernel. --

--- Patch [3]: [PATCH v19 03/14] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks --- commit 9ad943c529c38ff6736192a1064f09367d33c360
Author: Terry Bowman <[email protected]>

acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

The CXL CPER work registration and unregistration helpers acquire cxl_cper_work_lock and cxl_cper_prot_err_work_lock with a spinlock guard(), which leaves local interrupts enabled. This commit converts both locks from spinlock_t to raw_spinlock_t to prevent deadlocks with the GHES error notification path executing in hard IRQ context.

Quoted 4 lines

drivers/acpi/apei/ghes.c +0 -0
index 3236a3ce79d6b..ca7a138c1ff2e 100644

[ ... ]

drivers/acpi/apei/ghes.c static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err, · line 767

Quoted 9 lines

drivers/acpi/apei/ghes.c +1 -1

@@ -761,7 +761,7 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
 	if (cxl_cper_sec_prot_err_valid(prot_err))
 		return;
 
-	guard(spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
+	guard(raw_spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
 
 	if (!cxl_cper_prot_err_work)
 		return;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for this code to read out of bounds?

In cxl_cper_post_prot_err(), it subsequently calls cxl_cper_setup_prot_err_work_data() (in ghes_helpers.c) which does the following:

	memcpy(&wd->prot_err, prot_err, sizeof(wd->prot_err));

	dvsec_start = (u8 *)(prot_err + 1);
	cap_start = dvsec_start + prot_err->dvsec_len;

	memcpy(&wd->ras_cap, cap_start, sizeof(wd->ras_cap));

Could an untrusted or overly large dvsec_len cause cap_start to point to unmapped memory, resulting in a page fault during the memcpy?

If it reads adjacent mapped memory, could that out-of-bounds data be leaked to userspace via trace events when the workqueue processes the struct?

[ ... ]

drivers/acpi/apei/ghes.c static void cxl_cper_post_event(enum cxl_event_type event_type, · line 845

Quoted 9 lines

drivers/acpi/apei/ghes.c +1 -1

@@ -831,7 +839,7 @@ static void cxl_cper_post_event(enum cxl_event_type event_type,
 		return;
 	}
 
-	guard(spinlock_irqsave)(&cxl_cper_work_lock);
+	guard(raw_spinlock_irqsave)(&cxl_cper_work_lock);
 
 	if (!cxl_cper_work)
 		return;

[Severity: High]
This is a similar pre-existing issue. In cxl_cper_post_event() (just below this quoted hunk), the code copies the payload using a fixed size:

	wd.event_type = event_type;
	memcpy(&wd.rec, rec, sizeof(wd.rec));

Could this copy out-of-bounds memory if the firmware provided a payload where the outer envelope (gdata->error_data_length) is smaller than sizeof(wd.rec)?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

Luck, Tony

reviewer Reviewed-by on v19 3/14

2026-08-05 18:41 UTC #14 lore raw

Re: [PATCH v19 03/14] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks

Quoted 36 lines · Terry Bowman

The CXL CPER work registration and unregistration helpers acquire cxl_cper_work_lock and cxl_cper_prot_err_work_lock with a spinlock guard(), which leaves local interrupts enabled. The corresponding post paths (cxl_cper_post_event(), cxl_cper_post_prot_err()) execute in hard IRQ context (they are called from the GHES error notification path) and acquire the same locks with an irqsave guard().
If a CPU is holding one of these locks via a spinlock guard() when a GHES interrupt arrives on the same CPU, the IRQ handler spins on the held lock waiting for it to release, while the lock holder is preempted by the IRQ. The result is a deadlock.
Convert both locks from spinlock_t to raw_spinlock_t and use guard() at all call sites. On PREEMPT_RT kernels spinlock_t is backed by rt_mutex and sleeping from hard IRQ context is not permitted; raw_spinlock_t is safe in both contexts.
Add WARN_ONCE to both register functions to surface double-registration bugs at runtime.
Restructure both unregister functions to clear the global work pointer under the lock before calling cancel_work_sync(), closing the window where a CPER interrupt could schedule work on a pointer about to be freed. Add kfifo_reset() after cancel_work_sync() so stale entries are not replayed on next module load.
Both kfifos are single-consumer: only one work_struct is registered at a time, enforced by the WARN_ONCE guard in the register functions. kfifo_reset() is safe outside the lock because cancel_work_sync() has already quiesced the consumer, and no new consumer can register until the current module exit completes and a fresh module init runs.
Remove the redundant cancel_work_sync() call from cxl_ras_exit() and cxl_pci_driver_exit(). The CPER unregister functions now quiesce the work internally.

Quoted 7 lines

Reported-by Sashiko <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Fixes 5e4a264bf8b5 ("acpi/ghes: Process CXL Component Events")
Fixes 36f257e3b0ba ("acpi/ghes, cxl/pci: Process CXL CPER Protocol Errors")
Cc [email protected]
Reviewed-by Dave Jiang <[email protected]>
Reviewed-by Jonathan Cameron <[email protected]>

Reviewed-by Tony Luck <[email protected]>

[But could this be split ... the commit message feels like a list of three changes. No strong feelings about this]

-Tony

Terry Bowman

author

2026-08-03 22:19 UTC #15 lore raw

PATCH v19 4/14 cxl: Tighten CPER kfifo registration API and symbol visibility

From: Dan Williams <[email protected]>

Tighten the CPER protocol error kfifo registration API and symbol visibility.

Use EXPORT_SYMBOL_FOR_MODULES() instead of EXPORT_SYMBOL_NS_GPL() for the CPER kfifo registration symbols. This names the consuming module explicitly and gives compile-time enforcement.

Drop the work_struct argument from the unregister path. Change the WARN_ONCE condition to a NULL check since there is no caller pointer to compare against anymore.

Return void from the registration helpers whose result callers ignore. cxl_cper_register_work() keeps its int return as the only one consumed by a caller. Flag double registration with WARN_ONCE() inside the lock instead of returning an error.

Change cxl_ras_init() to void because there is one consumer and one producer so the error return was unnecessary. Remove the now-dead error check in cxl_core_init().

Co-developed-by Terry Bowman <[email protected]>

---

Changes in v18 -> v19:
- Tidied up definition line-wrapping in event.h
- Refined commit message reason for converting reg/dreg function return values to NULL.
- Add review-by for DaveJ

Changes in v17 -> v18:
- New patch.

---
 drivers/acpi/apei/ghes.c | 32 +++++++++++++++-----------------
 drivers/cxl/core/core.h  |  7 ++-----
 drivers/cxl/core/port.c  |  6 +-----
 drivers/cxl/core/ras.c   |  6 +++---
 include/cxl/event.h      | 21 ++++++---------------
 5 files changed, 27 insertions(+), 45 deletions(-)

drivers/acpi/apei/ghes.c +15 ~~-17~~

index ca7a138c1ff2e..187f54e31c33e 100644
@@ -778,41 +778,41 @@ static void cxl_cper_post_prot_err(struct cxl_cper_sec_prot_err *prot_err,
 #endif
 }
 
-int cxl_cper_register_prot_err_work(struct work_struct *work)
+void cxl_cper_register_prot_err_work(struct work_struct *work)
 {
 	guard(raw_spinlock_irqsave)(&cxl_cper_prot_err_work_lock);
 
 	if (WARN_ONCE(cxl_cper_prot_err_work,
 		      "CPER-CXL kfifo consumer already registered\n"))
-		return -EINVAL;
+		return;
 	cxl_cper_prot_err_work = work;
-	return 0;
 }
-EXPORT_SYMBOL_NS_GPL(cxl_cper_register_prot_err_work, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_register_prot_err_work, "cxl_core");
 
-int cxl_cper_unregister_prot_err_work(struct work_struct *work)
+void cxl_cper_unregister_prot_err_work(void)
 {
+	struct work_struct *old;
+
 	scoped_guard(raw_spinlock_irqsave, &cxl_cper_prot_err_work_lock) {
-		if (WARN_ONCE(cxl_cper_prot_err_work != work,
-			      "CPER-CXL kfifo consumer mismatch on unregister\n"))
-			return -EINVAL;
+		WARN_ONCE(!cxl_cper_prot_err_work,
+			  "CPER-CXL kfifo consumer not registered on unregister\n");
+		old = cxl_cper_prot_err_work;
 		cxl_cper_prot_err_work = NULL;
 	}
 
-	cancel_work_sync(work);
+	if (old)
+		cancel_work_sync(old);
 
 	/* Discard stale entries so they are not replayed on next module load */
 	kfifo_reset(&cxl_cper_prot_err_fifo);
-
-	return 0;
 }
-EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_prot_err_work, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_unregister_prot_err_work, "cxl_core");
 
 int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd)
 {
 	return kfifo_get(&cxl_cper_prot_err_fifo, wd);
 }
-EXPORT_SYMBOL_NS_GPL(cxl_cper_prot_err_kfifo_get, "CXL");
+EXPORT_SYMBOL_FOR_MODULES(cxl_cper_prot_err_kfifo_get, "cxl_core");
 
 /* Room for 8 entries for each of the 4 event log queues */
 #define CXL_CPER_FIFO_DEPTH 32

@@ -867,12 +867,12 @@ int cxl_cper_register_work(struct work_struct *work)
 }
 EXPORT_SYMBOL_NS_GPL(cxl_cper_register_work, "CXL");
 
-int cxl_cper_unregister_work(struct work_struct *work)
+void cxl_cper_unregister_work(struct work_struct *work)
 {
 	scoped_guard(raw_spinlock_irqsave, &cxl_cper_work_lock) {
 		if (WARN_ONCE(cxl_cper_work != work,
 			      "CXL CPER kfifo consumer mismatch on unregister\n"))
-			return -EINVAL;
+			return;
 		cxl_cper_work = NULL;
 	}

@@ -880,8 +880,6 @@ int cxl_cper_unregister_work(struct work_struct *work)
 
 	/* Discard stale entries so they are not replayed on next module load */
 	kfifo_reset(&cxl_cper_fifo);
-
-	return 0;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_cper_unregister_work, "CXL");

drivers/cxl/core/core.h +2 -5

index 07555ae638594..23fe40ddf4c6b 100644
@@ -183,7 +183,7 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
 	return &port->dev;
 }
 #ifdef CONFIG_CXL_RAS
-int cxl_ras_init(void);
+void cxl_ras_init(void);
 void cxl_ras_exit(void);
 bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
 void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);

@@ -192,10 +192,7 @@ void cxl_disable_rch_root_ints(struct cxl_dport *dport);
 void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds);
 void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
 #else
-static inline int cxl_ras_init(void)
-{
-	return 0;
-}
+static inline void cxl_ras_init(void) { }
 static inline void cxl_ras_exit(void) { }
 static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
 {

drivers/cxl/core/port.c +1 -5

index 1215ee4f40351..f90f899c31d07 100644
@@ -2531,14 +2531,10 @@ static __init int cxl_core_init(void)
 	if (rc)
 		goto err_region;
 
-	rc = cxl_ras_init();
-	if (rc)
-		goto err_ras;
+	cxl_ras_init();
 
 	return 0;
 
-err_ras:
-	cxl_region_exit();
 err_region:
 	bus_unregister(&cxl_bus_type);
 err_bus:

drivers/cxl/core/ras.c +3 -3

index bc74d4848132e..e307361bb39e4 100644
@@ -129,14 +129,14 @@ static void cxl_cper_prot_err_work_fn(struct work_struct *work)
 }
 static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn);
 
-int cxl_ras_init(void)
+void cxl_ras_init(void)
 {
-	return cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
+	cxl_cper_register_prot_err_work(&cxl_cper_prot_err_work);
 }
 
 void cxl_ras_exit(void)
 {
-	cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
+	cxl_cper_unregister_prot_err_work();
 }
 
 static void cxl_dport_map_ras(struct cxl_dport *dport)

include/cxl/event.h +6 ~~-15~~

index ff97fea718d2c..b5673384d9306 100644
@@ -287,10 +287,10 @@ struct cxl_cper_prot_err_work_data {
 
 #ifdef CONFIG_ACPI_APEI_GHES
 int cxl_cper_register_work(struct work_struct *work);
-int cxl_cper_unregister_work(struct work_struct *work);
+void cxl_cper_unregister_work(struct work_struct *work);
 int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd);
-int cxl_cper_register_prot_err_work(struct work_struct *work);
-int cxl_cper_unregister_prot_err_work(struct work_struct *work);
+void cxl_cper_register_prot_err_work(struct work_struct *work);
+void cxl_cper_unregister_prot_err_work(void);
 int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd);
 #else
 static inline int cxl_cper_register_work(struct work_struct *work)

@@ -298,22 +298,13 @@ static inline int cxl_cper_register_work(struct work_struct *work)
 	return 0;
 }
 
-static inline int cxl_cper_unregister_work(struct work_struct *work)
-{
-	return 0;
-}
+static inline void cxl_cper_unregister_work(struct work_struct *work) { }
 static inline int cxl_cper_kfifo_get(struct cxl_cper_work_data *wd)
 {
 	return 0;
 }
-static inline int cxl_cper_register_prot_err_work(struct work_struct *work)
-{
-	return 0;
-}
-static inline int cxl_cper_unregister_prot_err_work(struct work_struct *work)
-{
-	return 0;
-}
+static inline void cxl_cper_register_prot_err_work(struct work_struct *work) { }
+static inline void cxl_cper_unregister_prot_err_work(void) { }
 static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd)
 {
 	return 0;

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:30 UTC #16 lore raw

Re: [PATCH v19 04/14] cxl: Tighten CPER kfifo registration API and symbol visibility

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 18 lines

From: Dan Williams <[email protected]>
Tighten the CPER protocol error kfifo registration API and symbol visibility.
Use EXPORT_SYMBOL_FOR_MODULES() instead of EXPORT_SYMBOL_NS_GPL() for the CPER kfifo registration symbols. This names the consuming module explicitly and gives compile-time enforcement.
Drop the work_struct argument from the unregister path. Change the WARN_ONCE condition to a NULL check since there is no caller pointer to compare against anymore.
Return void from the registration helpers whose result callers ignore. cxl_cper_register_work() keeps its int return as the only one consumed by a caller. Flag double registration with WARN_ONCE() inside the lock instead of returning an error.

Quoted 7 lines

Change cxl_ras_init() to void because there is one consumer and one producer so the error return was unnecessary. Remove the now-dead error check in cxl_core_init().
Co-developed-by Terry Bowman <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Signed-off-by Dan Williams <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4

Alison Schofield

maintainer Reviewed-by on v19 4/14

2026-08-04 02:13 UTC #17 lore raw

Re: [PATCH v19 04/14] cxl: Tighten CPER kfifo registration API and symbol visibility

Quoted 13 lines · Terry Bowman

From: Dan Williams <[email protected]>
Tighten the CPER protocol error kfifo registration API and symbol visibility.
Use EXPORT_SYMBOL_FOR_MODULES() instead of EXPORT_SYMBOL_NS_GPL() for the CPER kfifo registration symbols. This names the consuming module explicitly and gives compile-time enforcement.
Drop the work_struct argument from the unregister path. Change the WARN_ONCE condition to a NULL check since there is no caller pointer to compare against anymore.

Quoted 8 lines

Return void from the registration helpers whose result callers ignore. cxl_cper_register_work() keeps its int return as the only one consumed by a caller. Flag double registration with WARN_ONCE() inside the lock instead of returning an error.
Change cxl_ras_init() to void because there is one consumer and one producer so the error return was unnecessary. Remove the now-dead error check in cxl_core_init().

Terry Bowman

author

2026-08-03 22:19 UTC #18 lore raw

PATCH v19 5/14 cxl: Rename find_cxl_port() to find_cxl_port_by_dport()

From: Dan Williams <[email protected]>

find_cxl_port() and find_cxl_port_by_uport() are internal port lookup functions that search the CXL bus by dport and uport respectively, but their names do not make the lookup method clear.

Rename find_cxl_port() to find_cxl_port_by_dport() to make the lookup method explicit and consistent with find_cxl_port_by_uport(). Both functions remain static to port.c; the upcoming patch that adds the first cross-file caller will widen their scope.

Make the same naming change to static helper __find_cxl_port_by_dport().

Co-developed-by Terry Bowman <[email protected]>

---

Changes in v18 -> v19:
- Update commit message imperative to include __find_cxl_port_by_dport() rename.

Changes in v17 -> v18:
- New commit

---
 drivers/cxl/core/port.c | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

drivers/cxl/core/port.c +14 -6

index f90f899c31d07..cadb51f70f854 100644
@@ -1379,7 +1379,7 @@ static int match_port_by_dport(struct device *dev, const void *data)
 	return dport != NULL;
 }
 
-static struct cxl_port *__find_cxl_port(struct cxl_find_port_ctx *ctx)
+static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
 {
 	struct device *dev;

@@ -1392,8 +1392,16 @@ static struct cxl_port *__find_cxl_port(struct cxl_find_port_ctx *ctx)
 	return NULL;
 }
 
-static struct cxl_port *find_cxl_port(struct device *dport_dev,
-				      struct cxl_dport **dport)
+/**
+ * find_cxl_port_by_dport - find a cxl_port by one of its targets
+ * @dport_dev: device representing the dport target
+ * @dport: optional output of the 'struct cxl_dport' companion of the @dport_dev
+ *
+ * Return a 'struct cxl_port' with an elevated reference if found. Use
+ * __free(put_cxl_port) to release.
+ */
+static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
+					       struct cxl_dport **dport)
 {
 	struct cxl_find_port_ctx ctx = {
 		.dport_dev = dport_dev,

@@ -1401,7 +1409,7 @@ static struct cxl_port *find_cxl_port(struct device *dport_dev,
 	};
 	struct cxl_port *port;
 
-	port = __find_cxl_port(&ctx);
+	port = __find_cxl_port_by_dport(&ctx);
 	return port;
 }

@@ -1895,14 +1903,14 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_enumerate_ports, "CXL");
 struct cxl_port *cxl_pci_find_port(struct pci_dev *pdev,
 				   struct cxl_dport **dport)
 {
-	return find_cxl_port(pdev->dev.parent, dport);
+	return find_cxl_port_by_dport(pdev->dev.parent, dport);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_pci_find_port, "CXL");
 
 struct cxl_port *cxl_mem_find_port(struct cxl_memdev *cxlmd,
 				   struct cxl_dport **dport)
 {
-	return find_cxl_port(grandparent(&cxlmd->dev), dport);
+	return find_cxl_port_by_dport(grandparent(&cxlmd->dev), dport);
 }
 EXPORT_SYMBOL_NS_GPL(cxl_mem_find_port, "CXL");

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:29 UTC #19 lore raw

Re: [PATCH v19 05/14] cxl: Rename find_cxl_port() to find_cxl_port_by_dport()

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 11 lines

From: Dan Williams <[email protected]>
find_cxl_port() and find_cxl_port_by_uport() are internal port lookup functions that search the CXL bus by dport and uport respectively, but their names do not make the lookup method clear.
Rename find_cxl_port() to find_cxl_port_by_dport() to make the lookup method explicit and consistent with find_cxl_port_by_uport(). Both functions remain static to port.c; the upcoming patch that adds the first cross-file caller will widen their scope.

Quoted 5 lines

Make the same naming change to static helper __find_cxl_port_by_dport().
Co-developed-by Terry Bowman <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Signed-off-by Dan Williams <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5

Alison Schofield

maintainer Reviewed-by on v19 5/14

2026-08-04 02:14 UTC #20 lore raw

Re: [PATCH v19 05/14] cxl: Rename find_cxl_port() to find_cxl_port_by_dport()

Quoted 6 lines · Terry Bowman

From: Dan Williams <[email protected]>
find_cxl_port() and find_cxl_port_by_uport() are internal port lookup functions that search the CXL bus by dport and uport respectively, but their names do not make the lookup method clear.

Quoted 7 lines

Rename find_cxl_port() to find_cxl_port_by_dport() to make the lookup method explicit and consistent with find_cxl_port_by_uport(). Both functions remain static to port.c; the upcoming patch that adds the first cross-file caller will widen their scope.
Make the same naming change to static helper __find_cxl_port_by_dport().

Terry Bowman

author

2026-08-03 22:19 UTC #21 lore raw

PATCH v19 6/14 PCI/AER: Introduce AER-CXL protocol error kfifo

CXL VH RAS handling requires a path for the AER driver to hand off CXL protocol errors to cxl_core for logging and recovery before PCIe AER recovery tears down the device. Introduce drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.

Move is_aer_internal_error() into this new file. It identifies AER internal error status bits across both correctable and uncorrectable severities.

Introduce is_cxl_error() to gate the VH kfifo path.

Introduce struct cxl_proto_err_work_data to carry the error source PCI device and severity through the kfifo.

Introduce cxl_forward_error() to enqueue a CXL protocol error. A reference is taken on the PCI device; the consumer releases it via for_each_cxl_proto_err(). On enqueue failure the reference is released immediately, the error is dropped, and the consumer is scheduled to drain existing entries. A subsequent patch wires cxl_forward_error() into handle_error_source() where correctable and uncorrectable status clearing is left to pci_aer_handle_error().

Introduce cxl_proto_err_wait_for_empty() to synchronously wait for the consumer worker to drain the kfifo.

Introduce cxl_register_proto_err_work() and cxl_unregister_proto_err_work() for cxl_core to register and deregister its work handler. On unregistration, pending kfifo entries are drained and their pdev references released before cancel_work_sync() runs. Export these and for_each_cxl_proto_err() via EXPORT_SYMBOL_FOR_MODULES restricted to cxl_core.

Protect the work pointer with a rwsem to correctly serialize registration, deregistration, enqueue, and dequeue against concurrent AER IRQ threads. Serialize concurrent kfifo writers with a spinlock.

Add MAINTAINERS entries for aer_cxl_vh.c and aer_cxl_rch.c under the CXL entry so CXL maintainers are CC'd on changes to the AER-CXL bridging code.

Co-developed-by Dan Williams <[email protected]>

---

Changes in v18 -> v19:
- Rename cxl_proto_err_flush() to cxl_proto_err_wait_for_empty() to better reflect that it waits for the kfifo to drain (Jonathan).

Changes in v17->v18:
- Remove correctable status clear from cxl_forward_error(); the AER core clears all status bits via pci_aer_handle_error() info->status writeback
- Schedule consumer on kfifo overflow so existing entries can be drained

Changes in v16->v17:
- Reword "kfifo semaphore" to "kfifo spinlock" to match fifo_lock.
- Defer the handle_error_source() is_cxl_error() switch to the patch that registers the kfifo consumer to keep each commit bisect-safe.
- Rename rwsema to rwsem
- Change CPER exports to use EXPORT_SYMBOL_FOR_MODULES.
- Add work cancel function.
- Replace kfifo_put() with kfifo_in_spinlocked() for multiple producers
- Add fifo_lock spinlock for concurrent producer serialisation
- Initialize the embedded kfifo with INIT_KFIFO() in a subsys_initcall so kfifo->mask, ->esize and ->data are set before first use.
- Clear PCI_ERR_COR_STATUS in cxl_forward_error() after enqueue so the device is acked for correctable events even when the consumer drops the event. Uncorrectable status is left for cxl_do_recovery() to clear after recovery completes, mirroring the AER core convention.
- WARN on double-registration in cxl_register_proto_err_work() to make an unintended second consumer visible at runtime.
- Add direct rwsem.h, cleanup.h and workqueue.h includes for symbols used in aer_cxl_vh.c
- Add MAINTAINERS entries for drivers/pci/pcie/aer_cxl_*.c
- Update message

---
 MAINTAINERS                   |   2 +
 drivers/pci/pcie/Makefile     |   1 +
 drivers/pci/pcie/aer.c        |  10 --
 drivers/pci/pcie/aer_cxl_vh.c | 227 ++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h    |   6 +
 include/linux/aer.h           |  24 ++++
 6 files changed, 260 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/pcie/aer_cxl_vh.c

MAINTAINERS +2 -0

index 5114e6db7307d..3a1f1057a21e5 100644
@@ -6527,6 +6527,8 @@ S:	Maintained
 F:	Documentation/driver-api/cxl
 F:	Documentation/userspace-api/fwctl/fwctl-cxl.rst
 F:	drivers/cxl/
+F:	drivers/pci/pcie/aer_cxl_rch.c
+F:	drivers/pci/pcie/aer_cxl_vh.c
 F:	include/cxl/
 F:	include/uapi/linux/cxl_mem.h
 F:	tools/testing/cxl/

drivers/pci/pcie/Makefile +1 -0

index b0b43a18c304b..62d3d3c69a5df 100644
@@ -9,6 +9,7 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o bwctrl.o
 obj-y				+= aspm.o
 obj-$(CONFIG_PCIEAER)		+= aer.o err.o tlp.o
 obj-$(CONFIG_CXL_RAS)		+= aer_cxl_rch.o
+obj-$(CONFIG_CXL_RAS)		+= aer_cxl_vh.o
 obj-$(CONFIG_PCIEAER_INJECT)	+= aer_inject.o
 obj-$(CONFIG_PCIE_PME)		+= pme.o
 obj-$(CONFIG_PCIE_DPC)		+= dpc.o

drivers/pci/pcie/aer.c +0 ~~-10~~

index c4fd9c0b2a548..c5bce25df51cb 100644
@@ -1150,16 +1150,6 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
  */
 EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
 
-#ifdef CONFIG_CXL_RAS
-bool is_aer_internal_error(struct aer_err_info *info)
-{
-	if (info->severity == AER_CORRECTABLE)
-		return info->status & PCI_ERR_COR_INTERNAL;
-
-	return info->status & PCI_ERR_UNC_INTN;
-}
-#endif
-
 /**
  * pci_aer_handle_error - handle logging error into an event log
  * @dev: pointer to pci_dev data structure of error source device

drivers/pci/pcie/aer_cxl_vh.c new file +227 -0

227 lines not on this page · raw message

drivers/pci/pcie/portdrv.h +6 -0

index cc58bf2f2c844..357310916088f 100644
@@ -130,9 +130,15 @@ struct aer_err_info;
 bool is_aer_internal_error(struct aer_err_info *info);
 void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info);
 void cxl_rch_enable_rcec(struct pci_dev *rcec);
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info);
+bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info);
+void cxl_proto_err_wait_for_empty(void);
 #else
 static inline bool is_aer_internal_error(struct aer_err_info *info) { return false; }
 static inline void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info) { }
 static inline void cxl_rch_enable_rcec(struct pci_dev *rcec) { }
+static inline bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline void cxl_proto_err_wait_for_empty(void) { }
 #endif /* CONFIG_CXL_RAS */
 #endif /* _PORTDRV_H_ */

include/linux/aer.h +24 -0

index df0f5c382286f..8eba3192e2d15 100644
@@ -25,6 +25,7 @@
 #define PCIE_STD_MAX_TLP_HEADERLOG	(PCIE_STD_NUM_TLP_HEADERLOG + 10)
 
 struct pci_dev;
+struct work_struct;
 
 struct pcie_tlp_log {
 	union {

@@ -66,6 +67,29 @@ static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
 #endif
 
+#ifdef CONFIG_CXL_RAS
+/**
+ * struct cxl_proto_err_work_data - Error information used in CXL error handling
+ * @pdev: PCI device detecting the error
+ * @severity: AER severity
+ */
+struct cxl_proto_err_work_data {
+	struct pci_dev *pdev;
+	int severity;
+};
+
+/**
+ * Callback for processing a CXL protocol error from the AER-CXL kfifo.
+ */
+typedef void (*cxl_proto_err_fn_t)(struct cxl_proto_err_work_data *wd);
+
+void cxl_register_proto_err_work(struct work_struct *work,
+				void (*flush)(void));
+void for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+			    cxl_proto_err_fn_t fn);
+void cxl_unregister_proto_err_work(void);
+#endif
+
 void pci_print_aer(struct pci_dev *dev, int aer_severity,
 		    struct aer_capability_regs *aer);
 int cper_severity_to_aer(int cper_severity);

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:29 UTC #22 lore raw

Re: [PATCH v19 06/14] PCI/AER: Introduce AER-CXL protocol error kfifo

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 13 lines

CXL VH RAS handling requires a path for the AER driver to hand off CXL protocol errors to cxl_core for logging and recovery before PCIe AER recovery tears down the device. Introduce drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.
Move is_aer_internal_error() into this new file. It identifies AER internal error status bits across both correctable and uncorrectable severities.
Introduce is_cxl_error() to gate the VH kfifo path.
Introduce struct cxl_proto_err_work_data to carry the error source PCI device and severity through the kfifo.

Quoted 8 lines

Introduce cxl_forward_error() to enqueue a CXL protocol error. A reference is taken on the PCI device; the consumer releases it via
[ ... ]
bridging code.
Co-developed-by Dan Williams <[email protected]>
Signed-off-by Dan Williams <[email protected]>
Signed-off-by Terry Bowman <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6

Richard Cheng

2026-08-04 08:15 UTC #23 lore raw

Re: [PATCH v19 06/14] PCI/AER: Introduce AER-CXL protocol error kfifo

Quoted 174 lines · Terry Bowman

CXL VH RAS handling requires a path for the AER driver to hand off CXL protocol errors to cxl_core for logging and recovery before PCIe AER recovery tears down the device. Introduce drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.
Move is_aer_internal_error() into this new file. It identifies AER internal error status bits across both correctable and uncorrectable severities.
Introduce is_cxl_error() to gate the VH kfifo path.
Introduce struct cxl_proto_err_work_data to carry the error source PCI device and severity through the kfifo.
Introduce cxl_forward_error() to enqueue a CXL protocol error. A reference is taken on the PCI device; the consumer releases it via for_each_cxl_proto_err(). On enqueue failure the reference is released immediately, the error is dropped, and the consumer is scheduled to drain existing entries. A subsequent patch wires cxl_forward_error() into handle_error_source() where correctable and uncorrectable status clearing is left to pci_aer_handle_error().
Introduce cxl_proto_err_wait_for_empty() to synchronously wait for the consumer worker to drain the kfifo.
Introduce cxl_register_proto_err_work() and cxl_unregister_proto_err_work() for cxl_core to register and deregister its work handler. On unregistration, pending kfifo entries are drained and their pdev references released before cancel_work_sync() runs. Export these and for_each_cxl_proto_err() via EXPORT_SYMBOL_FOR_MODULES restricted to cxl_core.
Protect the work pointer with a rwsem to correctly serialize registration, deregistration, enqueue, and dequeue against concurrent AER IRQ threads. Serialize concurrent kfifo writers with a spinlock.
Add MAINTAINERS entries for aer_cxl_vh.c and aer_cxl_rch.c under the CXL entry so CXL maintainers are CC'd on changes to the AER-CXL bridging code.
Co-developed-by Dan Williams <[email protected]>
Signed-off-by Dan Williams <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Reviewed-by Dave Jiang <[email protected]>
---
Changes in v18 -> v19:
- Rename cxl_proto_err_flush() to cxl_proto_err_wait_for_empty() to better reflect that it waits for the kfifo to drain (Jonathan).
Changes in v17->v18:
- Remove correctable status clear from cxl_forward_error(); the AER core clears all status bits via pci_aer_handle_error() info->status writeback
- Schedule consumer on kfifo overflow so existing entries can be drained
Changes in v16->v17:
- Reword "kfifo semaphore" to "kfifo spinlock" to match fifo_lock.
- Defer the handle_error_source() is_cxl_error() switch to the patch that registers the kfifo consumer to keep each commit bisect-safe.
- Rename rwsema to rwsem
- Change CPER exports to use EXPORT_SYMBOL_FOR_MODULES.
- Add work cancel function.
- Replace kfifo_put() with kfifo_in_spinlocked() for multiple producers
- Add fifo_lock spinlock for concurrent producer serialisation
- Initialize the embedded kfifo with INIT_KFIFO() in a subsys_initcall so kfifo->mask, ->esize and ->data are set before first use.
- Clear PCI_ERR_COR_STATUS in cxl_forward_error() after enqueue so the device is acked for correctable events even when the consumer drops the event. Uncorrectable status is left for cxl_do_recovery() to clear after recovery completes, mirroring the AER core convention.
- WARN on double-registration in cxl_register_proto_err_work() to make an unintended second consumer visible at runtime.
- Add direct rwsem.h, cleanup.h and workqueue.h includes for symbols used in aer_cxl_vh.c
- Add MAINTAINERS entries for drivers/pci/pcie/aer_cxl_*.c
- Update message
---
 MAINTAINERS                   |   2 +
 drivers/pci/pcie/Makefile     |   1 +
 drivers/pci/pcie/aer.c        |  10 --
 drivers/pci/pcie/aer_cxl_vh.c | 227 ++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h    |   6 +
 include/linux/aer.h           |  24 ++++
 6 files changed, 260 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/pcie/aer_cxl_vh.c
MAINTAINERS +2 -0
index 5114e6db7307d..3a1f1057a21e5 100644
@@ -6527,6 +6527,8 @@ S:	Maintained
 F:	Documentation/driver-api/cxl
 F:	Documentation/userspace-api/fwctl/fwctl-cxl.rst
 F:	drivers/cxl/
+F:	drivers/pci/pcie/aer_cxl_rch.c
+F:	drivers/pci/pcie/aer_cxl_vh.c
 F:	include/cxl/
 F:	include/uapi/linux/cxl_mem.h
 F:	tools/testing/cxl/
drivers/pci/pcie/Makefile +1 -0
index b0b43a18c304b..62d3d3c69a5df 100644
@@ -9,6 +9,7 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o bwctrl.o
 obj-y				+= aspm.o
 obj-$(CONFIG_PCIEAER)		+= aer.o err.o tlp.o
 obj-$(CONFIG_CXL_RAS)		+= aer_cxl_rch.o
+obj-$(CONFIG_CXL_RAS)		+= aer_cxl_vh.o
 obj-$(CONFIG_PCIEAER_INJECT)	+= aer_inject.o
 obj-$(CONFIG_PCIE_PME)		+= pme.o
 obj-$(CONFIG_PCIE_DPC)		+= dpc.o
drivers/pci/pcie/aer.c +0 ~~-10~~
index c4fd9c0b2a548..c5bce25df51cb 100644
@@ -1150,16 +1150,6 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
  */
 EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
 
-#ifdef CONFIG_CXL_RAS
-bool is_aer_internal_error(struct aer_err_info *info)
-{
-	if (info->severity == AER_CORRECTABLE)
-		return info->status & PCI_ERR_COR_INTERNAL;
-
-	return info->status & PCI_ERR_UNC_INTN;
-}
-#endif
-
 /**
  * pci_aer_handle_error - handle logging error into an event log
  * @dev: pointer to pci_dev data structure of error source device
drivers/pci/pcie/aer_cxl_vh.c new file +37 -0
new file mode 100644
index 0000000000000..cd921ade1a38f
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2026 AMD Corporation. All rights reserved. */
+
+#include <linux/aer.h>
+#include <linux/atomic.h>
+#include <linux/cleanup.h>
+#include <linux/init.h>
+#include <linux/kfifo.h>
+#include <linux/lockdep.h>
+#include <linux/rwsem.h>
+#include <linux/spinlock.h>
+#include <linux/wait_bit.h>
+#include <linux/workqueue.h>
+#include "../pci.h"
+#include "portdrv.h"
+
+#define CXL_ERROR_SOURCES_MAX          128
+
+struct cxl_proto_err_kfifo {
+	struct work_struct *work;
+	void (*flush)(void);
+	struct rw_semaphore rwsem;
+	spinlock_t fifo_lock;           /* Serializes kfifo writers */
+	atomic_t flush_inflight;
+	DECLARE_KFIFO(fifo, struct cxl_proto_err_work_data,
+		      CXL_ERROR_SOURCES_MAX);
+};
+
+static struct cxl_proto_err_kfifo cxl_proto_err_kfifo = {
+	.rwsem = __RWSEM_INITIALIZER(cxl_proto_err_kfifo.rwsem),
+	.fifo_lock = __SPIN_LOCK_UNLOCKED(cxl_proto_err_kfifo.fifo_lock),
+};
+
+static int __init cxl_proto_err_kfifo_init(void)
+{
+	INIT_KFIFO(cxl_proto_err_kfifo.fifo);
+	return 0;

drivers/pci/pcie/aer_cxl_vh.c line 47

Quoted 10 lines

drivers/pci/pcie/aer_cxl_vh.c new file +10 -0

+}
+subsys_initcall(cxl_proto_err_kfifo_init);
+
+bool is_aer_internal_error(struct aer_err_info *info)
+{
+	if (info->severity == AER_CORRECTABLE)
+		return info->status & PCI_ERR_COR_INTERNAL;
+
+	return info->status & PCI_ERR_UNC_INTN;
+}

Hi Terry,

Should we use unmasked status here ? just like other consumer does. aer_get_device_error_info() gates on "!(info->status & ~info->mask)" "info->mask" is always populated on the path that reaches here, so it's free.

Does that make sense to you ? or any specific reason we don't need the mask here ?

Best regards,
Richard Cheng

Quoted 245 lines

drivers/pci/pcie/aer_cxl_vh.c new file +180 -0

+
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info)
+{
+	if (!info || !info->is_cxl)
+		return false;
+
+	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
+		return false;
+
+	return is_aer_internal_error(info);
+}
+
+/**
+ * cxl_forward_error - Forward a CXL protocol error to the CXL subsystem via kfifo
+ * @pdev: PCI device that reported the AER error
+ * @info: AER error info containing severity and status
+ *
+ * Producer side of the AER-CXL kfifo. Enqueues a CXL protocol error work
+ * item and schedules the consumer workqueue. Takes a reference on @pdev
+ * that the consumer releases after handling.
+ *
+ * Return: true if the consumer workqueue was scheduled and the caller may
+ * need to drain the kfifo before AER recovery; false if no CXL error
+ * handling was initiated due to an early return on error (e.g. no kfifo
+ * consumer registered).  Note that on a full kfifo a correctable error is
+ * dropped but true is still returned; this is harmless because the caller
+ * only drains the kfifo for non-correctable events.
+ */
+bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info)
+{
+	struct cxl_proto_err_work_data wd = {
+		.severity = info->severity,
+		.pdev = pdev,
+	};
+
+	guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
+
+	if (!cxl_proto_err_kfifo.work) {
+		dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo reader not registered\n");
+		return false;
+	}
+
+	/*
+	 * Reference discipline: the AER caller (handle_error_source()) holds
+	 * a ref on @pdev for the duration of this call and releases it on
+	 * return. Take a fresh ref here so the pdev stays live while queued
+	 * in the kfifo; the corresponding consumer is for_each_cxl_proto_err()
+	 * and will drop that ref after handling. On enqueue failure below,
+	 * drop the ref we just took to avoid a leak.
+	 */
+	pci_dev_get(pdev);
+
+	/* Serialize concurrent kfifo writers: multiple AER threaded IRQs */
+	if (!kfifo_in_spinlocked(&cxl_proto_err_kfifo.fifo, &wd, 1,
+				 &cxl_proto_err_kfifo.fifo_lock)) {
+
+		if (info->severity != AER_CORRECTABLE) {
+			/*
+			 * Unlike PCIe AER, a dropped CXL.mem uncorrectable
+			 * error cannot be treated as device-local: it may
+			 * signal lost cache coherency over HDM memory in
+			 * active use. The error can no longer be confirmed
+			 * via CXL RAS, so collapse the unknown state to the
+			 * same conservative outcome as a confirmed UCE.
+			 */
+			panic("CXL: dropped uncorrectable protocol error\n");
+		}
+
+		dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo add failed\n");
+		pci_dev_put(pdev);
+	}
+
+	schedule_work(cxl_proto_err_kfifo.work);
+	return true;
+}
+
+void cxl_register_proto_err_work(struct work_struct *work,
+				void (*flush)(void))
+{
+	guard(rwsem_write)(&cxl_proto_err_kfifo.rwsem);
+
+	/*
+	 * Warn on double-registration to surface driver bugs (e.g. missing
+	 * cxl_unregister_proto_err_work() on module exit)
+	 */
+	if (WARN(cxl_proto_err_kfifo.work,
+		 "AER-CXL kfifo consumer already registered\n"))
+		return;
+	cxl_proto_err_kfifo.work = work;
+	cxl_proto_err_kfifo.flush = flush;
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_register_proto_err_work, "cxl_core");
+
+static struct work_struct *cancel_cxl_proto_err(void)
+{
+	struct work_struct *work;
+	struct cxl_proto_err_work_data wd;
+
+	guard(rwsem_write)(&cxl_proto_err_kfifo.rwsem);
+	work = cxl_proto_err_kfifo.work;
+	cxl_proto_err_kfifo.work = NULL;
+	cxl_proto_err_kfifo.flush = NULL;
+
+	/* rwsem_write excludes all producers; fifo_lock not needed */
+	while (kfifo_get(&cxl_proto_err_kfifo.fifo, &wd)) {
+		dev_err_ratelimited(&wd.pdev->dev,
+				    "AER-CXL error report canceled\n");
+		pci_dev_put(wd.pdev);
+	}
+	return work;
+}
+
+void cxl_unregister_proto_err_work(void)
+{
+	struct work_struct *work;
+
+	lockdep_assert_not_held(&cxl_proto_err_kfifo.rwsem);
+
+	work = cancel_cxl_proto_err();
+
+	/* Wait for any in-flight cxl_proto_err_wait_for_empty() calls to complete */
+	wait_var_event(&cxl_proto_err_kfifo.flush_inflight,
+		       atomic_read(&cxl_proto_err_kfifo.flush_inflight) == 0);
+
+	if (work)
+		cancel_work_sync(work);
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_unregister_proto_err_work, "cxl_core");
+
+/**
+ * for_each_cxl_proto_err - Call a function for each kfifo work item
+ *
+ * Single-consumer invariant: this function is only called from
+ * cxl_proto_err_work_fn() via a single DECLARE_WORK.
+ *
+ * Holds rwsem_read internally; fn() must not call cxl_register_proto_err_work()
+ * or cxl_unregister_proto_err_work().
+ */
+void for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+			    cxl_proto_err_fn_t fn)
+{
+	guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
+	while (kfifo_get(&cxl_proto_err_kfifo.fifo, wd)) {
+		fn(wd);
+
+		/* Corresponding ref incr taken in cxl_forward_error() */
+		pci_dev_put(wd->pdev);
+	}
+}
+EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
+
+/**
+ * cxl_proto_err_wait_for_empty - drain pending AER-CXL kfifo work synchronously
+ *
+ * Ensures CXL RAS handling and panic policy complete before AER
+ * recovery proceeds. Only needed for UCE; CE runs asynchronously.
+ *
+ * Snapshots the flush callback under rwsem_read, then releases the
+ * rwsem before calling it to avoid deadlock with a concurrent
+ * rwsem_write from cxl_unregister_proto_err_work().
+ *
+ * The flush_inflight counter (typically 0 or 1) prevents module
+ * unload while a flush is in progress outside the rwsem.
+ */
+void cxl_proto_err_wait_for_empty(void)
+{
+	void (*flush)(void);
+
+	scoped_guard(rwsem_read, &cxl_proto_err_kfifo.rwsem) {
+		flush = cxl_proto_err_kfifo.flush;
+		if (flush)
+			atomic_inc(&cxl_proto_err_kfifo.flush_inflight);
+	}
+
+	if (flush) {
+		flush();
+		if (atomic_dec_and_test(&cxl_proto_err_kfifo.flush_inflight))
+			wake_up_var(&cxl_proto_err_kfifo.flush_inflight);
+	}
+}

drivers/pci/pcie/portdrv.h +6 -0

index cc58bf2f2c844..357310916088f 100644
@@ -130,9 +130,15 @@ struct aer_err_info;
 bool is_aer_internal_error(struct aer_err_info *info);
 void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info);
 void cxl_rch_enable_rcec(struct pci_dev *rcec);
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info);
+bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info);
+void cxl_proto_err_wait_for_empty(void);
 #else
 static inline bool is_aer_internal_error(struct aer_err_info *info) { return false; }
 static inline void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info) { }
 static inline void cxl_rch_enable_rcec(struct pci_dev *rcec) { }
+static inline bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline void cxl_proto_err_wait_for_empty(void) { }
 #endif /* CONFIG_CXL_RAS */
 #endif /* _PORTDRV_H_ */

include/linux/aer.h +24 -0

index df0f5c382286f..8eba3192e2d15 100644
@@ -25,6 +25,7 @@
 #define PCIE_STD_MAX_TLP_HEADERLOG	(PCIE_STD_NUM_TLP_HEADERLOG + 10)
 
 struct pci_dev;
+struct work_struct;
 
 struct pcie_tlp_log {
 	union {

@@ -66,6 +67,29 @@ static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
 #endif
 
+#ifdef CONFIG_CXL_RAS
+/**
+ * struct cxl_proto_err_work_data - Error information used in CXL error handling
+ * @pdev: PCI device detecting the error
+ * @severity: AER severity
+ */
+struct cxl_proto_err_work_data {
+	struct pci_dev *pdev;
+	int severity;
+};
+
+/**
+ * Callback for processing a CXL protocol error from the AER-CXL kfifo.
+ */
+typedef void (*cxl_proto_err_fn_t)(struct cxl_proto_err_work_data *wd);
+
+void cxl_register_proto_err_work(struct work_struct *work,
+				void (*flush)(void));
+void for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+			    cxl_proto_err_fn_t fn);
+void cxl_unregister_proto_err_work(void);
+#endif
+
 void pci_print_aer(struct pci_dev *dev, int aer_severity,
 		    struct aer_capability_regs *aer);
 int cper_severity_to_aer(int cper_severity);

Signature · 1 line

2.34.1

Bowman, Terry

author

2026-08-04 14:10 UTC #24 lore raw

Re: [PATCH v19 06/14] PCI/AER: Introduce AER-CXL protocol error kfifo

Quoted 192 lines · Richard Cheng

Quoted 174 lines · Terry Bowman
CXL VH RAS handling requires a path for the AER driver to hand off CXL protocol errors to cxl_core for logging and recovery before PCIe AER recovery tears down the device. Introduce drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.
Move is_aer_internal_error() into this new file. It identifies AER internal error status bits across both correctable and uncorrectable severities.
Introduce is_cxl_error() to gate the VH kfifo path.
Introduce struct cxl_proto_err_work_data to carry the error source PCI device and severity through the kfifo.
Introduce cxl_forward_error() to enqueue a CXL protocol error. A reference is taken on the PCI device; the consumer releases it via for_each_cxl_proto_err(). On enqueue failure the reference is released immediately, the error is dropped, and the consumer is scheduled to drain existing entries. A subsequent patch wires cxl_forward_error() into handle_error_source() where correctable and uncorrectable status clearing is left to pci_aer_handle_error().
Introduce cxl_proto_err_wait_for_empty() to synchronously wait for the consumer worker to drain the kfifo.
Introduce cxl_register_proto_err_work() and cxl_unregister_proto_err_work() for cxl_core to register and deregister its work handler. On unregistration, pending kfifo entries are drained and their pdev references released before cancel_work_sync() runs. Export these and for_each_cxl_proto_err() via EXPORT_SYMBOL_FOR_MODULES restricted to cxl_core.
Protect the work pointer with a rwsem to correctly serialize registration, deregistration, enqueue, and dequeue against concurrent AER IRQ threads. Serialize concurrent kfifo writers with a spinlock.
Add MAINTAINERS entries for aer_cxl_vh.c and aer_cxl_rch.c under the CXL entry so CXL maintainers are CC'd on changes to the AER-CXL bridging code.
Co-developed-by Dan Williams <[email protected]>
Signed-off-by Dan Williams <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Reviewed-by Dave Jiang <[email protected]>
---
Changes in v18 -> v19:
- Rename cxl_proto_err_flush() to cxl_proto_err_wait_for_empty() to better reflect that it waits for the kfifo to drain (Jonathan).
Changes in v17->v18:
- Remove correctable status clear from cxl_forward_error(); the AER core clears all status bits via pci_aer_handle_error() info->status writeback
- Schedule consumer on kfifo overflow so existing entries can be drained
Changes in v16->v17:
- Reword "kfifo semaphore" to "kfifo spinlock" to match fifo_lock.
- Defer the handle_error_source() is_cxl_error() switch to the patch that registers the kfifo consumer to keep each commit bisect-safe.
- Rename rwsema to rwsem
- Change CPER exports to use EXPORT_SYMBOL_FOR_MODULES.
- Add work cancel function.
- Replace kfifo_put() with kfifo_in_spinlocked() for multiple producers
- Add fifo_lock spinlock for concurrent producer serialisation
- Initialize the embedded kfifo with INIT_KFIFO() in a subsys_initcall so kfifo->mask, ->esize and ->data are set before first use.
- Clear PCI_ERR_COR_STATUS in cxl_forward_error() after enqueue so the device is acked for correctable events even when the consumer drops the event. Uncorrectable status is left for cxl_do_recovery() to clear after recovery completes, mirroring the AER core convention.
- WARN on double-registration in cxl_register_proto_err_work() to make an unintended second consumer visible at runtime.
- Add direct rwsem.h, cleanup.h and workqueue.h includes for symbols used in aer_cxl_vh.c
- Add MAINTAINERS entries for drivers/pci/pcie/aer_cxl_*.c
- Update message
---
 MAINTAINERS                   |   2 +
 drivers/pci/pcie/Makefile     |   1 +
 drivers/pci/pcie/aer.c        |  10 --
 drivers/pci/pcie/aer_cxl_vh.c | 227 ++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h    |   6 +
 include/linux/aer.h           |  24 ++++
 6 files changed, 260 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/pcie/aer_cxl_vh.c
MAINTAINERS +2 -0
index 5114e6db7307d..3a1f1057a21e5 100644
@@ -6527,6 +6527,8 @@ S:	Maintained
 F:	Documentation/driver-api/cxl
 F:	Documentation/userspace-api/fwctl/fwctl-cxl.rst
 F:	drivers/cxl/
+F:	drivers/pci/pcie/aer_cxl_rch.c
+F:	drivers/pci/pcie/aer_cxl_vh.c
 F:	include/cxl/
 F:	include/uapi/linux/cxl_mem.h
 F:	tools/testing/cxl/
drivers/pci/pcie/Makefile +1 -0
index b0b43a18c304b..62d3d3c69a5df 100644
@@ -9,6 +9,7 @@ obj-$(CONFIG_PCIEPORTBUS)	+= pcieportdrv.o bwctrl.o
 obj-y				+= aspm.o
 obj-$(CONFIG_PCIEAER)		+= aer.o err.o tlp.o
 obj-$(CONFIG_CXL_RAS)		+= aer_cxl_rch.o
+obj-$(CONFIG_CXL_RAS)		+= aer_cxl_vh.o
 obj-$(CONFIG_PCIEAER_INJECT)	+= aer_inject.o
 obj-$(CONFIG_PCIE_PME)		+= pme.o
 obj-$(CONFIG_PCIE_DPC)		+= dpc.o
drivers/pci/pcie/aer.c +0 ~~-10~~
index c4fd9c0b2a548..c5bce25df51cb 100644
@@ -1150,16 +1150,6 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
  */
 EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
 
-#ifdef CONFIG_CXL_RAS
-bool is_aer_internal_error(struct aer_err_info *info)
-{
-	if (info->severity == AER_CORRECTABLE)
-		return info->status & PCI_ERR_COR_INTERNAL;
-
-	return info->status & PCI_ERR_UNC_INTN;
-}
-#endif
-
 /**
  * pci_aer_handle_error - handle logging error into an event log
  * @dev: pointer to pci_dev data structure of error source device
drivers/pci/pcie/aer_cxl_vh.c new file +37 -0
new file mode 100644
index 0000000000000..cd921ade1a38f
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/* Copyright(c) 2026 AMD Corporation. All rights reserved. */
+
+#include <linux/aer.h>
+#include <linux/atomic.h>
+#include <linux/cleanup.h>
+#include <linux/init.h>
+#include <linux/kfifo.h>
+#include <linux/lockdep.h>
+#include <linux/rwsem.h>
+#include <linux/spinlock.h>
+#include <linux/wait_bit.h>
+#include <linux/workqueue.h>
+#include "../pci.h"
+#include "portdrv.h"
+
+#define CXL_ERROR_SOURCES_MAX          128
+
+struct cxl_proto_err_kfifo {
+	struct work_struct *work;
+	void (*flush)(void);
+	struct rw_semaphore rwsem;
+	spinlock_t fifo_lock;           /* Serializes kfifo writers */
+	atomic_t flush_inflight;
+	DECLARE_KFIFO(fifo, struct cxl_proto_err_work_data,
+		      CXL_ERROR_SOURCES_MAX);
+};
+
+static struct cxl_proto_err_kfifo cxl_proto_err_kfifo = {
+	.rwsem = __RWSEM_INITIALIZER(cxl_proto_err_kfifo.rwsem),
+	.fifo_lock = __SPIN_LOCK_UNLOCKED(cxl_proto_err_kfifo.fifo_lock),
+};
+
+static int __init cxl_proto_err_kfifo_init(void)
+{
+	INIT_KFIFO(cxl_proto_err_kfifo.fifo);
+	return 0;
drivers/pci/pcie/aer_cxl_vh.c line 47
Quoted 10 lines
drivers/pci/pcie/aer_cxl_vh.c new file +10 -0
+}
+subsys_initcall(cxl_proto_err_kfifo_init);
+
+bool is_aer_internal_error(struct aer_err_info *info)
+{
+	if (info->severity == AER_CORRECTABLE)
+		return info->status & PCI_ERR_COR_INTERNAL;
+
+	return info->status & PCI_ERR_UNC_INTN;
+}
Hi Terry,
Should we use unmasked status here ? just like other consumer does. aer_get_device_error_info() gates on "!(info->status & ~info->mask)" "info->mask" is always populated on the path that reaches here, so it's free.

Quoted 6 lines

Does that make sense to you ? or any specific reason we don't need the mask here ?
Best regards,
Richard Cheng

Hi Richard,

Yes, we should and it is available. This will prevent further processing when the event is masked.

- Terry

Quoted 245 lines

drivers/pci/pcie/aer_cxl_vh.c new file +180 -0

+
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info)
+{
+	if (!info || !info->is_cxl)
+		return false;
+
+	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
+		return false;
+
+	return is_aer_internal_error(info);
+}
+
+/**
+ * cxl_forward_error - Forward a CXL protocol error to the CXL subsystem via kfifo
+ * @pdev: PCI device that reported the AER error
+ * @info: AER error info containing severity and status
+ *
+ * Producer side of the AER-CXL kfifo. Enqueues a CXL protocol error work
+ * item and schedules the consumer workqueue. Takes a reference on @pdev
+ * that the consumer releases after handling.
+ *
+ * Return: true if the consumer workqueue was scheduled and the caller may
+ * need to drain the kfifo before AER recovery; false if no CXL error
+ * handling was initiated due to an early return on error (e.g. no kfifo
+ * consumer registered).  Note that on a full kfifo a correctable error is
+ * dropped but true is still returned; this is harmless because the caller
+ * only drains the kfifo for non-correctable events.
+ */
+bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info)
+{
+	struct cxl_proto_err_work_data wd = {
+		.severity = info->severity,
+		.pdev = pdev,
+	};
+
+	guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
+
+	if (!cxl_proto_err_kfifo.work) {
+		dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo reader not registered\n");
+		return false;
+	}
+
+	/*
+	 * Reference discipline: the AER caller (handle_error_source()) holds
+	 * a ref on @pdev for the duration of this call and releases it on
+	 * return. Take a fresh ref here so the pdev stays live while queued
+	 * in the kfifo; the corresponding consumer is for_each_cxl_proto_err()
+	 * and will drop that ref after handling. On enqueue failure below,
+	 * drop the ref we just took to avoid a leak.
+	 */
+	pci_dev_get(pdev);
+
+	/* Serialize concurrent kfifo writers: multiple AER threaded IRQs */
+	if (!kfifo_in_spinlocked(&cxl_proto_err_kfifo.fifo, &wd, 1,
+				 &cxl_proto_err_kfifo.fifo_lock)) {
+
+		if (info->severity != AER_CORRECTABLE) {
+			/*
+			 * Unlike PCIe AER, a dropped CXL.mem uncorrectable
+			 * error cannot be treated as device-local: it may
+			 * signal lost cache coherency over HDM memory in
+			 * active use. The error can no longer be confirmed
+			 * via CXL RAS, so collapse the unknown state to the
+			 * same conservative outcome as a confirmed UCE.
+			 */
+			panic("CXL: dropped uncorrectable protocol error\n");
+		}
+
+		dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo add failed\n");
+		pci_dev_put(pdev);
+	}
+
+	schedule_work(cxl_proto_err_kfifo.work);
+	return true;
+}
+
+void cxl_register_proto_err_work(struct work_struct *work,
+				void (*flush)(void))
+{
+	guard(rwsem_write)(&cxl_proto_err_kfifo.rwsem);
+
+	/*
+	 * Warn on double-registration to surface driver bugs (e.g. missing
+	 * cxl_unregister_proto_err_work() on module exit)
+	 */
+	if (WARN(cxl_proto_err_kfifo.work,
+		 "AER-CXL kfifo consumer already registered\n"))
+		return;
+	cxl_proto_err_kfifo.work = work;
+	cxl_proto_err_kfifo.flush = flush;
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_register_proto_err_work, "cxl_core");
+
+static struct work_struct *cancel_cxl_proto_err(void)
+{
+	struct work_struct *work;
+	struct cxl_proto_err_work_data wd;
+
+	guard(rwsem_write)(&cxl_proto_err_kfifo.rwsem);
+	work = cxl_proto_err_kfifo.work;
+	cxl_proto_err_kfifo.work = NULL;
+	cxl_proto_err_kfifo.flush = NULL;
+
+	/* rwsem_write excludes all producers; fifo_lock not needed */
+	while (kfifo_get(&cxl_proto_err_kfifo.fifo, &wd)) {
+		dev_err_ratelimited(&wd.pdev->dev,
+				    "AER-CXL error report canceled\n");
+		pci_dev_put(wd.pdev);
+	}
+	return work;
+}
+
+void cxl_unregister_proto_err_work(void)
+{
+	struct work_struct *work;
+
+	lockdep_assert_not_held(&cxl_proto_err_kfifo.rwsem);
+
+	work = cancel_cxl_proto_err();
+
+	/* Wait for any in-flight cxl_proto_err_wait_for_empty() calls to complete */
+	wait_var_event(&cxl_proto_err_kfifo.flush_inflight,
+		       atomic_read(&cxl_proto_err_kfifo.flush_inflight) == 0);
+
+	if (work)
+		cancel_work_sync(work);
+}
+EXPORT_SYMBOL_FOR_MODULES(cxl_unregister_proto_err_work, "cxl_core");
+
+/**
+ * for_each_cxl_proto_err - Call a function for each kfifo work item
+ *
+ * Single-consumer invariant: this function is only called from
+ * cxl_proto_err_work_fn() via a single DECLARE_WORK.
+ *
+ * Holds rwsem_read internally; fn() must not call cxl_register_proto_err_work()
+ * or cxl_unregister_proto_err_work().
+ */
+void for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+			    cxl_proto_err_fn_t fn)
+{
+	guard(rwsem_read)(&cxl_proto_err_kfifo.rwsem);
+	while (kfifo_get(&cxl_proto_err_kfifo.fifo, wd)) {
+		fn(wd);
+
+		/* Corresponding ref incr taken in cxl_forward_error() */
+		pci_dev_put(wd->pdev);
+	}
+}
+EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
+
+/**
+ * cxl_proto_err_wait_for_empty - drain pending AER-CXL kfifo work synchronously
+ *
+ * Ensures CXL RAS handling and panic policy complete before AER
+ * recovery proceeds. Only needed for UCE; CE runs asynchronously.
+ *
+ * Snapshots the flush callback under rwsem_read, then releases the
+ * rwsem before calling it to avoid deadlock with a concurrent
+ * rwsem_write from cxl_unregister_proto_err_work().
+ *
+ * The flush_inflight counter (typically 0 or 1) prevents module
+ * unload while a flush is in progress outside the rwsem.
+ */
+void cxl_proto_err_wait_for_empty(void)
+{
+	void (*flush)(void);
+
+	scoped_guard(rwsem_read, &cxl_proto_err_kfifo.rwsem) {
+		flush = cxl_proto_err_kfifo.flush;
+		if (flush)
+			atomic_inc(&cxl_proto_err_kfifo.flush_inflight);
+	}
+
+	if (flush) {
+		flush();
+		if (atomic_dec_and_test(&cxl_proto_err_kfifo.flush_inflight))
+			wake_up_var(&cxl_proto_err_kfifo.flush_inflight);
+	}
+}

drivers/pci/pcie/portdrv.h +6 -0

index cc58bf2f2c844..357310916088f 100644
@@ -130,9 +130,15 @@ struct aer_err_info;
 bool is_aer_internal_error(struct aer_err_info *info);
 void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info);
 void cxl_rch_enable_rcec(struct pci_dev *rcec);
+bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info);
+bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info);
+void cxl_proto_err_wait_for_empty(void);
 #else
 static inline bool is_aer_internal_error(struct aer_err_info *info) { return false; }
 static inline void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info) { }
 static inline void cxl_rch_enable_rcec(struct pci_dev *rcec) { }
+static inline bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
+static inline void cxl_proto_err_wait_for_empty(void) { }
 #endif /* CONFIG_CXL_RAS */
 #endif /* _PORTDRV_H_ */

include/linux/aer.h +24 -0

index df0f5c382286f..8eba3192e2d15 100644
@@ -25,6 +25,7 @@
 #define PCIE_STD_MAX_TLP_HEADERLOG	(PCIE_STD_NUM_TLP_HEADERLOG + 10)
 
 struct pci_dev;
+struct work_struct;
 
 struct pcie_tlp_log {
 	union {

@@ -66,6 +67,29 @@ static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
 #endif
 
+#ifdef CONFIG_CXL_RAS
+/**
+ * struct cxl_proto_err_work_data - Error information used in CXL error handling
+ * @pdev: PCI device detecting the error
+ * @severity: AER severity
+ */
+struct cxl_proto_err_work_data {
+	struct pci_dev *pdev;
+	int severity;
+};
+
+/**
+ * Callback for processing a CXL protocol error from the AER-CXL kfifo.
+ */
+typedef void (*cxl_proto_err_fn_t)(struct cxl_proto_err_work_data *wd);
+
+void cxl_register_proto_err_work(struct work_struct *work,
+				void (*flush)(void));
+void for_each_cxl_proto_err(struct cxl_proto_err_work_data *wd,
+			    cxl_proto_err_fn_t fn);
+void cxl_unregister_proto_err_work(void);
+#endif
+
 void pci_print_aer(struct pci_dev *dev, int aer_severity,
 		    struct aer_capability_regs *aer);
 int cper_severity_to_aer(int cper_severity);

Signature · 1 line

2.34.1

Terry Bowman

author

2026-08-03 22:19 UTC #25 lore raw

PATCH v19 7/14 PCI: Establish common CXL Port protocol error flow

Establish a single CXL protocol error path shared by both CXL Virtual Hierarchy (VH) and Restricted CXL Host (RCH) topologies. AER dispatch in handle_error_source() uses is_cxl_error() and cxl_forward_error() to route CXL protocol errors through the AER-CXL kfifo to a cxl_core consumer for logging and recovery. The producer and consumer go live together so CXL errors are never silently dropped across a bisect.

Expand is_cxl_error() from Endpoint-only to also cover Root Port, Upstream Port, and Downstream Port. RCDs (PCI_EXP_TYPE_RC_END) are intentionally excluded here: they report on behalf of an upstream RCH Downstream Port and reach the kfifo via cxl_rch_handle_error() instead. cxl_rch_handle_error() forwards RCH protocol errors through the same kfifo and returns bool so handle_error_source() knows whether to wait for the consumer before AER recovery proceeds.

For uncorrectable events, call cxl_proto_err_wait_for_empty() so the CXL RAS registers are read, the panic policy is applied, and CXL state is cleared before pci_aer_handle_error() drives PCIe recovery. Without it, AER recovery can tear down drivers and unmap the CXL RAS iomaps while the consumer is still reading them. Correctable events do not need the wait and run asynchronously.

Add cxl_handle_proto_error() to dispatch CE and UCE events to the CXL RAS helpers, and cxl_do_recovery() for UCE recovery. Panic on a confirmed UCE, or when the RAS registers cannot be mapped (to_ras_base() returns NULL): an unmappable UCE cannot be confirmed and is conservatively treated as fatal because CXL.mem cache coherency may be lost. If the RAS registers are mapped but report no uncorrectable status, the UCE is logged and treated as spurious (no panic).

The kfifo consumer (__cxl_proto_err_work_fn) holds the Port device lock and skips handling when the Port driver is unbound, so RAS register iomaps are not torn down while the handler reads them. For RCDs it first calls cxl_handle_rdport_errors(), which does its own RCH Downstream Port lookup and locking, keeping the Downstream Port lock separate from the Endpoint Port lock.

Add to_ras_base() to centralize RAS base lookup: dport->regs.ras for Root/Downstream Ports, port->regs.ras for Upstream Ports and Endpoints. Routing all RAS access through it also provides an injection point for CXL RAS status simulation during testing.

cxl_handle_rdport_errors() takes a struct pci_dev * and resolves the RCH Downstream Port under the owning port's device lock. The former cxl_cor_error_detected() AER callback and its .cor_error_detected registration are removed; correctable Endpoint errors now route through the kfifo like every other CXL protocol error.

Co-developed-by Dan Williams <[email protected]>

---

Changes in v18->v19:
- Merge "Establish common CXL Port protocol error flow" and "PCI/CXL: Add RCH support to CXL handlers" into a single patch; route RCD correctable errors through the AER-CXL kfifo and remove cxl_cor_error_detected().
- Rename cxl_proto_err_flush() to cxl_proto_err_wait_for_empty()
- Use guard(device) in __cxl_proto_err_work_fn() instead of scoped_guard
- Reword "silent data corruption" comment; drop "interleaved HDM regions"
- Add blank line after the port lookup in __cxl_proto_err_work_fn()
- Change cxl_do_recovery() to panic if to_ras_base() returns NULL
- Clarify Port driver bound gates RAS access
- Add review-by for DaveJ

Changes in v17->v18:
- Fix pre-existing race: hold memdev device lock around cxl_handle_rdport_errors(), release before port lock
- Fix handle_error_source() to call pci_aer_handle_error() unconditionally so AER handling always runs after cxl_forward_error()
- Add cxl_proto_err_flush() call for CXL UCE to drain kfifo before AER recovery tears down the device
- Fix NULL dereference of dport->dport_dev in cxl_handle_cor_ras() and cxl_handle_ras() for UPSTREAM/ENDPOINT port types: use dport->dport_dev when dport is non-NULL, else fall back to port->uport_dev
- Remove duplicate pcie_clear_device_status() call from cxl_handle_proto_error() CE path; pci_aer_handle_error() already clears it
- Clarify panic policy: panic only on confirmed UCE via RAS status read
- Document kfifo consumer serialization against driver unbind via guard(device)(&port->dev) and port->dev.driver check

Changes in v16->v17:
- get_cxl_port() -> find_cxl_port_by_dev()
- Simplified find_cxl_port_by_dev()
- Replace and remove cxl_serial_number() w/ pci_get_dsn()
- cxl_get_ras_base() -> to_ras_base()
- Drop dependency on PCI_ERS_RESULT_PANIC; cxl_do_recovery() panics directly. (PANIC enum patch dropped from series.)
- Clarify panic semantics: panic on any uncorrectable CXL RAS error, not only AER-FATAL severities.
- Add is_cxl_error() switch in handle_error_source() here, paired with the kfifo consumer registration, to keep each commit bisect-safe.
- Drop pcie_aer_is_native() guard in cxl_do_recovery() (always native).
- Swap order with the "Limit" patch for bisectability w/ cxl_ras_exit()
- Reword for "any uncorrectable" CXL RAS error panics.
- Restore log messages for port-not-found and port-unbound cases.
- Whitespace cleanup (Jonathan)
- Update to get_cxl_port() documentation (Terry)
- Fix __cxl_proto_err_work_fn() to return 0 for transient errors.
- Drop !port check in cxl_do_recovery(), caller already validated
- Fix kerneldoc @pdev -> @dev in find_cxl_port_by_dev()
- Fix missing space in pr_err_ratelimited()
- Made pcie_clear_device_status() and pci_aer_clear_fatal_status() EXPORT_SYMBOL_FOR_MODULES("cxl_core") (Dan)
- Move find_cxl_port_by_dport() and find_cxl_port_by_uport() de-staticisation and core.h declarations from the rename patch to here, where the first cross-file callers in find_cxl_port_by_dev() land.

Changes in v15->v16:
- get_ras_base(), initialize dport to NULL (Jonathan)
- Remove guard(device)(&cxlmd->dev) (Jonathan)
- Fix dev_warns() (Jonathan)
- Remove comment in cxl_port_error_detected() (Dan)
- Update switch-case brackets to follow clang-format (Dan)
- Add PCI_EXP_TYPE_RC_END for cxl_get_ras_base() (Terry)
- Add NULL port check in cxl_serial_number() (Terry)

Changes in v14->v15:
- Update commit message and title. Added Bjorn's ack.
- Move CE and UCE handling logic here

Changes in v13->v14:
- Add Dave Jiang's review-by
- Update commit message & headline (Bjorn)
- Refactor cxl_port_error_detected()/cxl_port_cor_error_detected() to one line (Jonathan)
- Remove cxl_walk_port() (Dan)
- Remove cxl_pci_drv_bound(). Check for 'is_cxl' parent port is sufficient (Dan)
- Remove device_lock_if()
- Combined CE and UCE here (Terry)

Changes in v12->v13:
- Move get_pci_cxl_host_dev() and cxl_handle_proto_error() to Dequeue patch (Terry)
- Remove EP case in cxl_get_ras_base(), not used. (Terry)
- Remove check for dport->dport_dev (Dave)
- Remove whitespace (Terry)

Changes in v11->v12:
- Add call to cxl_pci_drv_bound() in cxl_handle_proto_error() and pci_to_cxl_dev()
- Change cxl_error_detected() -> cxl_cor_error_detected()
- Remove NULL variable assignments
- Replace bus_find_device() with find_cxl_port_by_uport() for upstream port searches.

Changes in v10->v11:
- None

---
 drivers/cxl/core/core.h        |  17 ++-
 drivers/cxl/core/port.c        |   6 +-
 drivers/cxl/core/ras.c         | 222 +++++++++++++++++++++++++--------
 drivers/cxl/core/ras_rch.c     |  16 +--
 drivers/cxl/cxlpci.h           |   3 -
 drivers/cxl/pci.c              |   1 -
 drivers/pci/pci.h              |   1 -
 drivers/pci/pcie/aer.c         |  13 +-
 drivers/pci/pcie/aer_cxl_rch.c |  39 +++---
 drivers/pci/pcie/aer_cxl_vh.c  |  16 ++-
 drivers/pci/pcie/portdrv.h     |   4 +-
 11 files changed, 240 insertions(+), 98 deletions(-)

drivers/cxl/core/core.h +15 -2

index 23fe40ddf4c6b..272634ff2615b 100644
@@ -186,10 +186,13 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
 void cxl_ras_init(void);
 void cxl_ras_exit(void);
 bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
+void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
+		     struct cxl_dport *dport);
 void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);
 void cxl_dport_map_rch_aer(struct cxl_dport *dport);
 void cxl_disable_rch_root_ints(struct cxl_dport *dport);
-void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds);
+void cxl_handle_rdport_errors(struct pci_dev *pdev);
+void __iomem *to_ras_base(struct cxl_port *port, struct cxl_dport *dport);
 void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
 #else
 static inline void cxl_ras_init(void) { }

@@ -198,14 +201,24 @@ static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
 {
 	return false;
 }
+static inline void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
+				   struct cxl_dport *dport) { }
 static inline void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base) { }
 static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
 static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
-static inline void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds) { }
+static inline void cxl_handle_rdport_errors(struct pci_dev *pdev) { }
+static inline void __iomem *to_ras_base(struct cxl_port *port,
+					 struct cxl_dport *dport)
+{
+	return NULL;
+}
 static inline void devm_cxl_dport_ras_setup(struct cxl_dport *dport) { }
 #endif /* CONFIG_CXL_RAS */
 
 int cxl_gpf_port_setup(struct cxl_dport *dport);
+struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
+					struct cxl_dport **dport);
+struct cxl_port *find_cxl_port_by_uport(struct device *uport_dev);
 
 struct cxl_hdm;
 int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,

drivers/cxl/core/port.c +3 -3

index cadb51f70f854..a76f3ee05cba8 100644
@@ -1400,8 +1400,8 @@ static struct cxl_port *__find_cxl_port_by_dport(struct cxl_find_port_ctx *ctx)
  * Return a 'struct cxl_port' with an elevated reference if found. Use
  * __free(put_cxl_port) to release.
  */
-static struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
-					       struct cxl_dport **dport)
+struct cxl_port *find_cxl_port_by_dport(struct device *dport_dev,
+					struct cxl_dport **dport)
 {
 	struct cxl_find_port_ctx ctx = {
 		.dport_dev = dport_dev,

@@ -1596,7 +1596,7 @@ static int match_port_by_uport(struct device *dev, const void *data)
  * Function takes a device reference on the port device. Caller should do a
  * put_device() when done.
  */
-static struct cxl_port *find_cxl_port_by_uport(struct device *uport_dev)
+struct cxl_port *find_cxl_port_by_uport(struct device *uport_dev)
 {
 	struct device *dev;

drivers/cxl/core/ras.c +167 ~~-55~~

278 lines not on this page · raw message

drivers/cxl/core/ras_rch.c +9 -7

index e0e01aa5eba6c..41959c0333711 100644
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0-only
 /* Copyright(c) 2025 AMD Corporation. All rights reserved. */
 
-#include <linux/types.h>
 #include <linux/aer.h>
 #include "cxl.h"
 #include "core.h"

@@ -110,18 +109,21 @@ static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
 	return false;
 }
 
-void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
+void cxl_handle_rdport_errors(struct pci_dev *pdev)
 {
-	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
 	struct aer_capability_regs aer_regs;
 	struct cxl_dport *dport;
 	int severity;
 
-	struct cxl_port *port __free(put_cxl_port) =
-		cxl_pci_find_port(pdev, &dport);
+	struct cxl_port *port __free(put_cxl_port) = cxl_pci_find_port(pdev, NULL);
 	if (!port)
 		return;
 
+	guard(device)(&port->dev);
+	dport = cxl_find_dport_by_dev(port, pdev->dev.parent);
+	if (!dport)
+		return;
+
 	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
 		return;

@@ -130,7 +132,7 @@ void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
 
 	pci_print_aer(pdev, severity, &aer_regs);
 	if (severity == AER_CORRECTABLE)
-		cxl_handle_cor_ras(&cxlds->cxlmd->dev, dport->regs.ras);
+		cxl_handle_cor_ras(&pdev->dev, to_ras_base(port, dport));
 	else
-		cxl_handle_ras(&cxlds->cxlmd->dev, dport->regs.ras);
+		cxl_do_recovery(pdev, dport->port, dport);
 }

drivers/cxl/cxlpci.h +0 -3

index b826eb53cf7ba..06c46adcf0f6c 100644
@@ -89,14 +89,11 @@ struct cxl_dev_state;
 void read_cdat_data(struct cxl_port *port);
 
 #ifdef CONFIG_CXL_RAS
-void cxl_cor_error_detected(struct pci_dev *pdev);
 pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 				    pci_channel_state_t state);
 void devm_cxl_dport_rch_ras_setup(struct cxl_dport *dport);
 void devm_cxl_port_ras_setup(struct cxl_port *port);
 #else
-static inline void cxl_cor_error_detected(struct pci_dev *pdev) { }
-
 static inline pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 						  pci_channel_state_t state)
 {

drivers/cxl/pci.c +0 -1

index 7c6faee7f85ed..5c21db36073fe 100644
@@ -1004,7 +1004,6 @@ static const struct pci_error_handlers cxl_error_handlers = {
 	.error_detected	= cxl_error_detected,
 	.slot_reset	= cxl_slot_reset,
 	.resume		= cxl_error_resume,
-	.cor_error_detected	= cxl_cor_error_detected,
 	.reset_done	= cxl_reset_done,
 };

drivers/pci/pci.h +0 -1

index 4469e1a77f3c1..a83e2aef75912 100644
@@ -1296,7 +1296,6 @@ void pci_restore_aer_state(struct pci_dev *dev);
 static inline void pci_no_aer(void) { }
 static inline void pci_aer_init(struct pci_dev *d) { }
 static inline void pci_aer_exit(struct pci_dev *d) { }
-static inline void pci_aer_clear_fatal_status(struct pci_dev *dev) { }
 static inline int pci_aer_clear_status(struct pci_dev *dev) { return -EINVAL; }
 static inline int pci_aer_raw_clear_status(struct pci_dev *dev) { return -EINVAL; }
 static inline void pci_save_aer_state(struct pci_dev *dev) { }

drivers/pci/pcie/aer.c +12 -1

index c5bce25df51cb..0155f3923c1fd 100644
@@ -1185,7 +1185,18 @@ static void pci_aer_handle_error(struct pci_dev *dev, struct aer_err_info *info)
 
 static void handle_error_source(struct pci_dev *dev, struct aer_err_info *info)
 {
-	cxl_rch_handle_error(dev, info);
+	bool cxl_pending = cxl_rch_handle_error(dev, info);
+
+	if (is_cxl_error(dev, info))
+		cxl_pending |= cxl_forward_error(dev, info);
+
+	/*
+	 * Wait for UCE CXL work to complete before AER recovery
+	 * tears down the device. CE can run asynchronously.
+	 */
+	if (cxl_pending && info->severity != AER_CORRECTABLE)
+		cxl_proto_err_wait_for_empty();
+
 	pci_aer_handle_error(dev, info);
 	pci_dev_put(dev);
 }

drivers/pci/pcie/aer_cxl_rch.c +17 ~~-22~~

index e471eefec9c40..ab31c4281b483 100644
@@ -34,42 +34,37 @@ static bool cxl_error_is_native(struct pci_dev *dev)
 	return (pcie_ports_native || host->native_aer);
 }
 
+struct cxl_rch_error_ctx {
+	struct aer_err_info *info;
+	bool enqueued;
+};
+
 static int cxl_rch_handle_error_iter(struct pci_dev *dev, void *data)
 {
-	struct aer_err_info *info = (struct aer_err_info *)data;
-	const struct pci_error_handlers *err_handler;
+	struct cxl_rch_error_ctx *ctx = data;
 
 	if (!is_cxl_mem_dev(dev) || !cxl_error_is_native(dev))
 		return 0;
 
-	guard(device)(&dev->dev);
-
-	err_handler = dev->driver ? dev->driver->err_handler : NULL;
-	if (!err_handler)
-		return 0;
-
-	if (info->severity == AER_CORRECTABLE) {
-		if (err_handler->cor_error_detected)
-			err_handler->cor_error_detected(dev);
-	} else if (err_handler->error_detected) {
-		if (info->severity == AER_NONFATAL)
-			err_handler->error_detected(dev, pci_channel_io_normal);
-		else if (info->severity == AER_FATAL)
-			err_handler->error_detected(dev, pci_channel_io_frozen);
-	}
+	if (cxl_forward_error(dev, ctx->info))
+		ctx->enqueued = true;
 	return 0;
 }
 
-void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
+bool cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info)
 {
+	struct cxl_rch_error_ctx ctx = { .info = info };
+
 	/*
-	 * Internal errors of an RCEC indicate an AER error in an
-	 * RCH's downstream port. Check and handle them in the CXL.mem
-	 * device driver.
+	 * An RCEC AER internal error indicates an error in an
+	 * associated RCH Downstream Port or RCD device or both.
+	 * Forward to the cxl_core module for handling.
 	 */
 	if (pci_pcie_type(dev) == PCI_EXP_TYPE_RC_EC &&
 	    is_aer_internal_error(info))
-		pcie_walk_rcec(dev, cxl_rch_handle_error_iter, info);
+		pcie_walk_rcec(dev, cxl_rch_handle_error_iter, &ctx);
+
+	return ctx.enqueued;
 }
 
 static int handles_cxl_error_iter(struct pci_dev *dev, void *data)

drivers/pci/pcie/aer_cxl_vh.c +15 -1

index cd921ade1a38f..2edfa69d8a5be 100644
@@ -51,8 +51,22 @@ bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info)
 	if (!info || !info->is_cxl)
 		return false;
 
-	if (pci_pcie_type(pdev) != PCI_EXP_TYPE_ENDPOINT)
+	/*
+	 * RCD (PCI_EXP_TYPE_RC_END) is not included here because RCDs
+	 * report errors on behalf of upstream RCH Downstream Port and thus
+	 * require a unique discovery detailed in CXL4.0 spec (12.2.1.1).
+	 * The RCH device error discovery and RCD forwarding flow begins
+	 * in cxl_rch_handle_error().
+	 */
+	switch (pci_pcie_type(pdev)) {
+	case PCI_EXP_TYPE_ENDPOINT:
+	case PCI_EXP_TYPE_ROOT_PORT:
+	case PCI_EXP_TYPE_UPSTREAM:
+	case PCI_EXP_TYPE_DOWNSTREAM:
+		break;
+	default:
 		return false;
+	}
 
 	return is_aer_internal_error(info);
 }

drivers/pci/pcie/portdrv.h +2 -2

index 357310916088f..b13b9ac9571dc 100644
@@ -128,14 +128,14 @@ struct aer_err_info;
 
 #ifdef CONFIG_CXL_RAS
 bool is_aer_internal_error(struct aer_err_info *info);
-void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info);
+bool cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info);
 void cxl_rch_enable_rcec(struct pci_dev *rcec);
 bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info);
 bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info);
 void cxl_proto_err_wait_for_empty(void);
 #else
 static inline bool is_aer_internal_error(struct aer_err_info *info) { return false; }
-static inline void cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info) { }
+static inline bool cxl_rch_handle_error(struct pci_dev *dev, struct aer_err_info *info) { return false; }
 static inline void cxl_rch_enable_rcec(struct pci_dev *rcec) { }
 static inline bool is_cxl_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }
 static inline bool cxl_forward_error(struct pci_dev *pdev, struct aer_err_info *info) { return false; }

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:56 UTC #26 lore raw

Re: [PATCH v19 07/14] PCI: Establish common CXL Port protocol error flow

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Concurrent devm cleanup and port driver unbinds cause a Use-After-Free of cxl_dport and its MMIO registers in the AER handling workqueue. --

--- Patch [7]: [PATCH v19 07/14] PCI: Establish common CXL Port protocol error flow --- commit cee1514715e6511f03b640de984db92a0e13bd58
Author: Terry Bowman <[email protected]>

PCI: Establish common CXL Port protocol error flow

Quoted 4 lines

drivers/cxl/core/ras.c +0 -0
index e307361bb39e4..83df544e5a656 100644

[ ... ]

Quoted 8 lines

drivers/cxl/core/ras.c +4 -0

@@ -335,3 +371,79 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 	return PCI_ERS_RESULT_NEED_RESET;
 }
 EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
+
+static void cxl_handle_proto_error(struct pci_dev *pdev, struct cxl_port *port,
+				   struct cxl_dport *dport, int severity)
+{

[ ... ]

Quoted 20 lines

drivers/cxl/core/ras.c +20 -0

+}
+
+static void __cxl_proto_err_work_fn(struct cxl_proto_err_work_data *wd)
+{
+	struct cxl_dport *dport;
+
+	/*
+	 * For RCD devices, handle RCH Downstream Port errors first.
+	 * cxl_handle_rdport_errors() does its own port lookup and locking,
+	 * keeping the Downstream Port lock separate from the Endpoint Port
+	 * lock taken below.
+	 */
+	if (is_cxl_restricted(wd->pdev))
+		cxl_handle_rdport_errors(wd->pdev);
+
+	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&wd->pdev->dev, NULL);
+	if (!port) {
+		dev_err_ratelimited(&wd->pdev->dev,
+				    "Failed to find parent port device in CXL topology\n");
+		return;

drivers/cxl/core/ras.c pci_ers_result_t cxl_error_detected(struct pci_dev *pdev, · line 413

Quoted 10 lines

drivers/cxl/core/ras.c +10 -0

+	}
+
+	guard(device)(&port->dev);
+	if (!port->dev.driver) {
+		dev_err_ratelimited(&port->dev,
+				    "Port device is unbound, abort error handling\n");
+		return;
+	}
+
+	dport = cxl_find_dport_by_dev(port, &wd->pdev->dev);

[Severity: Critical]
This is a pre-existing issue, but does checking !port->dev.driver safely guard against an ACPI unbind racing with the AER handling workqueue?

The commit message states the kfifo consumer "holds the Port device lock and skips handling when the Port driver is unbound, so RAS register iomaps are not torn down while the handler reads them."

However, for Root Ports, the cxl_dport and its devres are attached to the ACPI device (port->uport_dev), not &port->dev.

If devres_release_all(acpi_dev) runs concurrently with the AER workqueue, it frees dport and unmaps the registers independently of device_lock(&port->dev).

Because the cxl_port driver remains bound until devres_release_all(acpi_dev) finishes and unregisters the port, checking !port->dev.driver wouldn't protect against this. Could this lead to a use-after-free of the cxl_dport structure and reading from unmapped MMIO (RAS/AER registers) in __cxl_proto_err_work_fn()?

Quoted 9 lines

drivers/cxl/core/ras.c +9 -0

+	if (!dport && (pci_pcie_type(wd->pdev) == PCI_EXP_TYPE_ROOT_PORT ||
+		       pci_pcie_type(wd->pdev) == PCI_EXP_TYPE_DOWNSTREAM)) {
+		dev_err_ratelimited(&wd->pdev->dev,
+				    "Failed to find dport device in CXL topology\n");
+		return;
+	}
+
+	cxl_handle_proto_error(wd->pdev, port, dport, wd->severity);
+}

[ ... ]

Quoted 4 lines

drivers/cxl/core/ras_rch.c +0 -0
index e0e01aa5eba6c..41959c0333711 100644

[ ... ]

Quoted 14 lines

drivers/cxl/core/ras_rch.c +1 -4

@@ -110,18 +109,21 @@ static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
 	return false;
 }
 
-void cxl_handle_rdport_errors(struct cxl_dev_state *cxlds)
+void cxl_handle_rdport_errors(struct pci_dev *pdev)
 {
-	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
 	struct aer_capability_regs aer_regs;
 	struct cxl_dport *dport;
 	int severity;
 
-	struct cxl_port *port __free(put_cxl_port) =
-		cxl_pci_find_port(pdev, &dport);

drivers/cxl/core/ras_rch.c static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs, · line 127

Quoted 10 lines

drivers/cxl/core/ras_rch.c +7 -0

+	struct cxl_port *port __free(put_cxl_port) = cxl_pci_find_port(pdev, NULL);
 	if (!port)
 		return;
 
+	guard(device)(&port->dev);
+	dport = cxl_find_dport_by_dev(port, pdev->dev.parent);
+	if (!dport)
+		return;
+
+	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))

[Severity: Critical]
This is a pre-existing issue, but does this still suffer from the same race?

The dport is looked up under the &port->dev lock, but for Root Ports its devm lifecycle is tied to the parent ACPI device. If a concurrent ACPI unbind occurs, couldn't dport be freed and unmapped before being dereferenced here?

Quoted 1 line

drivers/cxl/core/ras_rch.c +0 -0
 		return;

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7

Terry Bowman

author

2026-08-03 22:19 UTC #27 lore raw

PATCH v19 8/14 cxl/ras: Handle RCH correctable and uncorrectable errors in one pass

cxl_rch_get_aer_info() reads and clears both the correctable and uncorrectable AER status registers in a single pass. The previous severity decode returned after the first matching class, so when a correctable and an uncorrectable error were logged simultaneously the correctable event was cleared in hardware but never traced or handled.

Handle both classes independently: dispatch cxl_handle_cor_ras() when correctable status is set and cxl_do_recovery() when uncorrectable status is set. Remove the now-unused cxl_rch_get_aer_severity() helper and decode the uncorrectable severity inline.

Reported-by Sashiko <[email protected]>

---

Changes in v18 -> v19:
- New patch to process correctable and uncorrectable RCH errors in the same call so a co-logged correctable event is not lost.

---
 drivers/cxl/core/ras_rch.c | 42 ++++++++++++++------------------------
 1 file changed, 15 insertions(+), 27 deletions(-)

drivers/cxl/core/ras_rch.c +15 ~~-27~~

index 41959c0333711..6f95542e2e6a7 100644
@@ -89,31 +89,10 @@ static bool cxl_rch_get_aer_info(void __iomem *aer_base,
 	return true;
 }
 
-/* Get AER severity. Return false if there is no error. */
-static bool cxl_rch_get_aer_severity(struct aer_capability_regs *aer_regs,
-				     int *severity)
-{
-	u32 uncor_status = aer_regs->uncor_status & ~aer_regs->uncor_mask;
-
-	if (uncor_status) {
-		*severity = (uncor_status & aer_regs->uncor_severity) ?
-			     AER_FATAL : AER_NONFATAL;
-		return true;
-	}
-
-	if (aer_regs->cor_status & ~aer_regs->cor_mask) {
-		*severity = AER_CORRECTABLE;
-		return true;
-	}
-
-	return false;
-}
-
 void cxl_handle_rdport_errors(struct pci_dev *pdev)
 {
 	struct aer_capability_regs aer_regs;
 	struct cxl_dport *dport;
-	int severity;
 
 	struct cxl_port *port __free(put_cxl_port) = cxl_pci_find_port(pdev, NULL);
 	if (!port)

@@ -127,12 +106,21 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
 	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
 		return;
 
-	if (!cxl_rch_get_aer_severity(&aer_regs, &severity))
-		return;
+	/*
+	 * Handle correctable and uncorrectable errors independently; both
+	 * may be set in the same pass and cxl_rch_get_aer_info() has already
+	 * cleared both status registers.
+	 */
+	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
+		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
+		cxl_handle_cor_ras(dport->dport_dev, to_ras_base(port, dport));
+	}
+
+	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {
+		int severity = (aer_regs.uncor_status & aer_regs.uncor_severity) ?
+			       AER_FATAL : AER_NONFATAL;
 
-	pci_print_aer(pdev, severity, &aer_regs);
-	if (severity == AER_CORRECTABLE)
-		cxl_handle_cor_ras(&pdev->dev, to_ras_base(port, dport));
-	else
+		pci_print_aer(pdev, severity, &aer_regs);
 		cxl_do_recovery(pdev, dport->port, dport);
+	}
 }

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:29 UTC #28 lore raw

Re: [PATCH v19 08/14] cxl/ras: Handle RCH correctable and uncorrectable errors in one pass

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incorrect inline severity calculation for uncorrectable errors fails to apply the uncorrectable…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect inline severity calculation for uncorrectable errors fails to apply the uncorrectable error mask, potentially escalating unmasked non-fatal errors to AER_FATAL. --

--- Patch [8]: [PATCH v19 08/14] cxl/ras: Handle RCH correctable and uncorrectable errors in one pass --- commit 57f34ecdf069076e73ab0e60927403af518bdfc9
Author: Terry Bowman <[email protected]>

cxl/ras: Handle RCH correctable and uncorrectable errors in one pass

This commit modifies the handling of RCH correctable and uncorrectable errors to process both in a single pass so co-logged events are not lost. It removes the cxl_rch_get_aer_severity helper and decodes the uncorrectable severity inline.

Quoted 3 lines

drivers/cxl/core/ras_rch.c +0 -0

[ ... ]

Quoted 9 lines

drivers/cxl/core/ras_rch.c +3 -2

@@ -127,12 +106,21 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
 	if (!cxl_rch_get_aer_info(dport->regs.dport_aer, &aer_regs))
 		return;
 
-	if (!cxl_rch_get_aer_severity(&aer_regs, &severity))
-		return;
+	/*
+	 * Handle correctable and uncorrectable errors independently; both
+	 * may be set in the same pass and cxl_rch_get_aer_info() has already

drivers/cxl/core/ras_rch.c void cxl_handle_rdport_errors(struct pci_dev *pdev) · line 121

Quoted 10 lines

drivers/cxl/core/ras_rch.c +10 -0

+	 * cleared both status registers.
+	 */
+	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
+		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
+		cxl_handle_cor_ras(dport->dport_dev, to_ras_base(port, dport));
+	}
+
+	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {
+		int severity = (aer_regs.uncor_status & aer_regs.uncor_severity) ?
+			       AER_FATAL : AER_NONFATAL;

[Severity: High]
Does this inline calculation miss the uncorrectable error mask?

In the previous implementation, the severity check masked uncor_status:

    u32 uncor_status = aer_regs->uncor_status & ~aer_regs->uncor_mask;
    if (uncor_status) {
        *severity = (uncor_status & aer_regs->uncor_severity) ?
                     AER_FATAL : AER_NONFATAL;

Without applying ~aer_regs.uncor_mask here in the new inline code, if hardware simultaneously logs a masked fatal error and an unmasked non-fatal error, the masked fatal bit will be evaluated. Could this wrongly escalate the severity to AER_FATAL?

Quoted 9 lines

drivers/cxl/core/ras_rch.c +2 -4

-	pci_print_aer(pdev, severity, &aer_regs);
-	if (severity == AER_CORRECTABLE)
-		cxl_handle_cor_ras(&pdev->dev, to_ras_base(port, dport));
-	else
+		pci_print_aer(pdev, severity, &aer_regs);
 		cxl_do_recovery(pdev, dport->port, dport);
+	}
 }

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8

Alison Schofield

maintainer Reviewed-by on v19 8/14

2026-08-04 02:16 UTC #29 lore raw

Re: [PATCH v19 08/14] cxl/ras: Handle RCH correctable and uncorrectable errors in one pass

Quoted 10 lines · Terry Bowman

cxl_rch_get_aer_info() reads and clears both the correctable and uncorrectable AER status registers in a single pass. The previous severity decode returned after the first matching class, so when a correctable and an uncorrectable error were logged simultaneously the correctable event was cleared in hardware but never traced or handled.
Handle both classes independently: dispatch cxl_handle_cor_ras() when correctable status is set and cxl_do_recovery() when uncorrectable status is set. Remove the now-unused cxl_rch_get_aer_severity() helper and decode the uncorrectable severity inline.

Terry Bowman

author

2026-08-03 22:20 UTC #30 lore raw

PATCH v19 9/14 cxl/pci: Thread port and dport through RAS handling helpers

From: Dan Williams <[email protected]>

The callers of cxl_handle_ras() and cxl_handle_cor_ras() already hold a struct cxl_port * and optionally a struct cxl_dport * for the device being handled. Passing a generic struct device * requires is_cxl_memdev() to distinguish Endpoints from Ports at trace emission time. Threading Port and Downstream Port directly enables is_cxl_endpoint() and explicit dport/port branching for cleaner trace dispatch.

Refactor cxl_handle_ras() and cxl_handle_cor_ras() to accept struct cxl_port * and struct cxl_dport * directly. The CXL RAS trace event emission logic is split into three branches: Endpoint events are identified via is_cxl_endpoint() and emit with the memdev, dport events emit with dport->dport_dev, and Upstream Port events fall back to port->uport_dev. This branching is transitional: the follow-on patch ("cxl: Add port and dport identifiers to CXL AER trace events") unifies the trace events on port/dport and removes it.

Update cxl_handle_rdport_errors() and cxl_handle_proto_error() to pass Port and Downstream Port to the refactored functions.

RCH Downstream Port correctable trace events now report the dport device (dport->dport_dev) as a consequence of threading Port and Downstream Port through the RAS helpers. The following trace event rework ("cxl: Add port and dport identifiers to CXL AER trace events") adds explicit memdev, Port, Downstream Port, and host fields that provide full context for all device types.

Co-developed-by Terry Bowman <[email protected]>

---

Changes in v18 -> v19:
- Added review-by for DaveJ
- Update commit message that three-way trace branching is transitional and removed by the following trace-event unification patch

Changes in v17 -> v18:
- New patch.

---
 drivers/cxl/core/core.h    | 12 ++++++++----
 drivers/cxl/core/ras.c     | 29 +++++++++++++++--------------
 drivers/cxl/core/ras_rch.c |  2 +-
 3 files changed, 24 insertions(+), 19 deletions(-)

drivers/cxl/core/core.h +8 -4

index 272634ff2615b..5ca1275fd8f35 100644
@@ -185,10 +185,12 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
 #ifdef CONFIG_CXL_RAS
 void cxl_ras_init(void);
 void cxl_ras_exit(void);
-bool cxl_handle_ras(struct device *dev, void __iomem *ras_base);
+bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
+		    void __iomem *ras_base);
 void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 		     struct cxl_dport *dport);
-void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base);
+void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
+			void __iomem *ras_base);
 void cxl_dport_map_rch_aer(struct cxl_dport *dport);
 void cxl_disable_rch_root_ints(struct cxl_dport *dport);
 void cxl_handle_rdport_errors(struct pci_dev *pdev);

@@ -197,13 +199,15 @@ void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
 #else
 static inline void cxl_ras_init(void) { }
 static inline void cxl_ras_exit(void) { }
-static inline bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
+static inline bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
+				  void __iomem *ras_base)
 {
 	return false;
 }
 static inline void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport) { }
-static inline void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base) { }
+static inline void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
+				      void __iomem *ras_base) { }
 static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
 static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
 static inline void cxl_handle_rdport_errors(struct pci_dev *pdev) { }

drivers/cxl/core/ras.c +15 ~~-14~~

index 83df544e5a656..ff4fb016ed4a1 100644
@@ -227,20 +227,19 @@ void __iomem *to_ras_base(struct cxl_port *port, struct cxl_dport *dport)
 
 void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dport *dport)
 {
-	struct device *dev = dport ? dport->dport_dev : port->uport_dev;
 	void __iomem *ras_base = to_ras_base(port, dport);
 
 	if (!ras_base)
 		panic("CXL: UCE with unmapped RAS registers");
 
-	if (cxl_handle_ras(dev, ras_base))
+	if (cxl_handle_ras(port, dport, ras_base))
 		panic("CXL cachemem error");
 
 	dev_dbg(&pdev->dev,
 		"CXL UCE signaled but no CXL RAS status bits set\n");
 }
 
-void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
+void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
 {
 	void __iomem *addr;
 	u32 status;

@@ -252,10 +251,12 @@ void cxl_handle_cor_ras(struct device *dev, void __iomem *ras_base)
 	status = readl(addr);
 	if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
 		writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
-		if (is_cxl_memdev(dev))
-			trace_cxl_aer_correctable_error(to_cxl_memdev(dev), status);
+		if (is_cxl_endpoint(port))
+			trace_cxl_aer_correctable_error(to_cxl_memdev(port->uport_dev), status);
+		else if (dport)
+			trace_cxl_port_aer_correctable_error(dport->dport_dev, status);
 		else
-			trace_cxl_port_aer_correctable_error(dev, status);
+			trace_cxl_port_aer_correctable_error(port->uport_dev, status);
 	}
 }

@@ -280,7 +281,7 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
  * Log the state of the RAS status registers and prepare them to log the
  * next error status. Return 1 if reset needed.
  */
-bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
+bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
 {
 	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
 	void __iomem *addr;

@@ -307,10 +308,12 @@ bool cxl_handle_ras(struct device *dev, void __iomem *ras_base)
 	}
 
 	header_log_copy(ras_base, hl);
-	if (is_cxl_memdev(dev))
-		trace_cxl_aer_uncorrectable_error(to_cxl_memdev(dev), status, fe, hl);
+	if (is_cxl_endpoint(port))
+		trace_cxl_aer_uncorrectable_error(to_cxl_memdev(port->uport_dev), status, fe, hl);
+	else if (dport)
+		trace_cxl_port_aer_uncorrectable_error(dport->dport_dev, status, fe, hl);
 	else
-		trace_cxl_port_aer_uncorrectable_error(dev, status, fe, hl);
+		trace_cxl_port_aer_uncorrectable_error(port->uport_dev, status, fe, hl);
 
 	writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);

@@ -344,7 +347,7 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 		 * cache coherency is already lost; continuing risks silent
 		 * data corruption.
 		 */
-		ue = cxl_handle_ras(port->uport_dev, to_ras_base(port, NULL));
+		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
 	}
 
 	/*

@@ -375,10 +378,8 @@ EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
 static void cxl_handle_proto_error(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport, int severity)
 {
-	struct device *dev = dport ? dport->dport_dev : port->uport_dev;
-
 	if (severity == AER_CORRECTABLE)
-		cxl_handle_cor_ras(dev, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
 	else
 		cxl_do_recovery(pdev, port, dport);
 }

drivers/cxl/core/ras_rch.c +1 -1

index 6f95542e2e6a7..a5c62c71060d9 100644
@@ -113,7 +113,7 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
 	 */
 	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
 		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
-		cxl_handle_cor_ras(dport->dport_dev, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
 	}
 
 	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:33 UTC #31 lore raw

Re: [PATCH v19 09/14] cxl/pci: Thread port and dport through RAS handling helpers

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 18 lines

From: Dan Williams <[email protected]>
The callers of cxl_handle_ras() and cxl_handle_cor_ras() already hold a struct cxl_port * and optionally a struct cxl_dport * for the device being handled. Passing a generic struct device * requires is_cxl_memdev() to distinguish Endpoints from Ports at trace emission time. Threading Port and Downstream Port directly enables is_cxl_endpoint() and explicit dport/port branching for cleaner trace dispatch.
Refactor cxl_handle_ras() and cxl_handle_cor_ras() to accept struct cxl_port * and struct cxl_dport * directly. The CXL RAS trace event emission logic is split into three branches: Endpoint events are identified via is_cxl_endpoint() and emit with the memdev, dport events emit with dport->dport_dev, and Upstream Port events fall back to port->uport_dev. This branching is transitional: the follow-on patch [ ... ]
device types.

Quoted 3 lines

Co-developed-by Terry Bowman <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Signed-off-by Dan Williams <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9

Alison Schofield

maintainer Reviewed-by on v19 9/14

2026-08-04 02:17 UTC #32 lore raw

Re: [PATCH v19 09/14] cxl/pci: Thread port and dport through RAS handling helpers

Quoted 21 lines · Terry Bowman

From: Dan Williams <[email protected]>
The callers of cxl_handle_ras() and cxl_handle_cor_ras() already hold a struct cxl_port * and optionally a struct cxl_dport * for the device being handled. Passing a generic struct device * requires is_cxl_memdev() to distinguish Endpoints from Ports at trace emission time. Threading Port and Downstream Port directly enables is_cxl_endpoint() and explicit dport/port branching for cleaner trace dispatch.
Refactor cxl_handle_ras() and cxl_handle_cor_ras() to accept struct cxl_port * and struct cxl_dport * directly. The CXL RAS trace event emission logic is split into three branches: Endpoint events are identified via is_cxl_endpoint() and emit with the memdev, dport events emit with dport->dport_dev, and Upstream Port events fall back to port->uport_dev. This branching is transitional: the follow-on patch ("cxl: Add port and dport identifiers to CXL AER trace events") unifies the trace events on port/dport and removes it.
Update cxl_handle_rdport_errors() and cxl_handle_proto_error() to pass Port and Downstream Port to the refactored functions.

Quoted 6 lines

RCH Downstream Port correctable trace events now report the dport device (dport->dport_dev) as a consequence of threading Port and Downstream Port through the RAS helpers. The following trace event rework ("cxl: Add port and dport identifiers to CXL AER trace events") adds explicit memdev, Port, Downstream Port, and host fields that provide full context for all device types.

Terry Bowman

author

2026-08-03 22:20 UTC #33 lore raw

PATCH v19 10/14 cxl: Update CXL Endpoint AER handler

Rename cxl_error_detected() to cxl_pci_error_detected() and rename the struct pci_error_handlers instance to cxl_pci_error_handlers to avoid shadowing the struct type tag.

Document the unconditional CXL RAS read policy: on a dead link, readl() returns 0xFFFFFFFF which is interpreted as UCE bits set and triggers a panic. If RAS registers are not mapped the read is skipped and the frozen/perm_failure switch cases defer to AER recovery for devices without active CXL.mem traffic.

---

Changes in v18->v19:
- Add review-by for DaveJ and Jonathan
- Remove reindent introduced in v18 at cxl_error_handlers definition.

Changes in v17->v18:
- Fix cxl_pci_error_detected() to use find_cxl_port_by_uport() and port->uport_dev
- Read CXL RAS unconditionally; panic on UCE regardless of channel state
- Document unconditional read policy and 0xFFFFFFFF behavior in comment
- Drop guard removal paragraph from commit message (not in this diff)
- Drop Reviewed-by tags pending re-review after message change

Changes in v16->v17:
- Rename pci_error_handlers struct instance to cxl_pci_error_handlers to avoid shadowing the struct type tag.
- Restore scoped_guard(device) and dev->driver check around AER read.
- NULL-check find_cxl_port_by_dev() before deref of port->uport_dev.
- Updated commit message. (Terry)
- Add scope cleanup for port variable in cxl_pci_error_detected() (Terry)
- Drop cxl_uncor_aer_present(), rely on AER state

Changes in v15->v16:
- Update commit message (DaveJ)
- s/cxl_handle_aer()/cxl_uncor_aer_present()/g (Jonathan)
- cxl_uncor_aer_present(): Leave original result calculation based on if a UCE is present and the provided state (Terry)
- Add call to pci_print_aer(). AER fails to log because is upstream link (Terry)

Changes in v14->v15:
- Update commit message and title. Added Bjorn's ack.
- Move CE and UCE handling logic here

Changes in v10->v11:
- None

---
 drivers/cxl/core/ras.c | 21 ++++++++++++++-------
 drivers/cxl/cxlpci.h   |  8 ++++----
 drivers/cxl/pci.c      |  6 +++---
 3 files changed, 21 insertions(+), 14 deletions(-)

drivers/cxl/core/ras.c +14 -7

index ff4fb016ed4a1..5183b3c532952 100644
@@ -61,7 +61,7 @@ cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
 
 	/*
 	 * ras_cap.header_log[] holds CXL_HEADERLOG_SIZE_U32 (16) hardware
-	 * dwords.  Copy them into the front of a zero-filled
+	 * dwords. Copy them into the front of a zero-filled
 	 * CXL_HEADERLOG_TRACE_SIZE_U32 (128) u32 staging buffer so the trace
 	 * event memcpy sees a full 512-byte source and the userspace ABI
 	 * (rasdaemon) is preserved.

@@ -320,8 +320,8 @@ bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem
 	return true;
 }
 
-pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
-				    pci_channel_state_t state)
+pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
+					pci_channel_state_t state)
 {
 	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_uport(&pdev->dev);
 	bool ue = false;

@@ -343,9 +343,16 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 		/*
 		 * The CXL RAS read is unconditional regardless of channel
 		 * state. Any uncorrectable error bit set in the CXL RAS
-		 * status register triggers a panic below because CXL.mem
-		 * cache coherency is already lost; continuing risks silent
-		 * data corruption.
+		 * status register triggers a panic because CXL.mem cache
+		 * coherency is already lost; continuing risks silent data
+		 * corruption.
+		 *
+		 * On a dead link readl() returns 0xFFFFFFFF which sets all
+		 * UCE bits and also triggers the panic - this is intentional.
+		 * If RAS registers are not mapped the read is skipped, the
+		 * panic is not reached, and the frozen/perm_failure switch
+		 * cases below handle AER recovery for devices without active
+		 * CXL.mem traffic.
 		 */
 		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
 	}

@@ -373,7 +380,7 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 	}
 	return PCI_ERS_RESULT_NEED_RESET;
 }
-EXPORT_SYMBOL_NS_GPL(cxl_error_detected, "CXL");
+EXPORT_SYMBOL_NS_GPL(cxl_pci_error_detected, "CXL");
 
 static void cxl_handle_proto_error(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport, int severity)

drivers/cxl/cxlpci.h +4 -4

index 06c46adcf0f6c..8aeb80a4e5732 100644
@@ -89,13 +89,13 @@ struct cxl_dev_state;
 void read_cdat_data(struct cxl_port *port);
 
 #ifdef CONFIG_CXL_RAS
-pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
-				    pci_channel_state_t state);
+pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
+					pci_channel_state_t state);
 void devm_cxl_dport_rch_ras_setup(struct cxl_dport *dport);
 void devm_cxl_port_ras_setup(struct cxl_port *port);
 #else
-static inline pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
-						  pci_channel_state_t state)
+static inline pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
+						      pci_channel_state_t state)
 {
 	return PCI_ERS_RESULT_NONE;
 }

drivers/cxl/pci.c +3 -3

index 5c21db36073fe..46e60c53d3ca4 100644
@@ -1000,8 +1000,8 @@ static void cxl_reset_done(struct pci_dev *pdev)
 	}
 }
 
-static const struct pci_error_handlers cxl_error_handlers = {
-	.error_detected	= cxl_error_detected,
+static const struct pci_error_handlers cxl_pci_error_handlers = {
+	.error_detected	= cxl_pci_error_detected,
 	.slot_reset	= cxl_slot_reset,
 	.resume		= cxl_error_resume,
 	.reset_done	= cxl_reset_done,

@@ -1011,7 +1011,7 @@ static struct pci_driver cxl_pci_driver = {
 	.name			= KBUILD_MODNAME,
 	.id_table		= cxl_mem_pci_tbl,
 	.probe			= cxl_pci_probe,
-	.err_handler		= &cxl_error_handlers,
+	.err_handler		= &cxl_pci_error_handlers,
 	.dev_groups		= cxl_rcd_groups,
 	.driver	= {
 		.probe_type	= PROBE_PREFER_ASYNCHRONOUS,

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:40 UTC #34 lore raw

Re: [PATCH v19 10/14] cxl: Update CXL Endpoint AER handler

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Unconditional system panic on a frozen PCIe link for idle CXL devices due to an incorrect assumption about RAS register mapping.

Pre-existing issues:
- [Medium] Evaluating BIT(63) during CXL RAS handling on a dead link causes undefined behavior on 32-bit platforms and truncation on 64-bit platforms. --

--- Patch [10]: [PATCH v19 10/14] cxl: Update CXL Endpoint AER handler --- commit 4439ebdc3bdbec21997bf9c57f9c8a4e5e9901ea
Author: Terry Bowman <[email protected]>

cxl: Update CXL Endpoint AER handler

This commit updates the CXL Endpoint AER handler by renaming functions and documenting the unconditional RAS read policy. It establishes that reading a dead link returns 0xFFFFFFFF, which is interpreted as UCE bits being set to trigger a panic.

Quoted 3 lines

drivers/cxl/core/ras.c +0 -0

[ ... ]

Quoted 10 lines

drivers/cxl/core/ras.c +3 -3

@@ -343,9 +343,16 @@ pci_ers_result_t cxl_error_detected(struct pci_dev *pdev,
 		/*
 		 * The CXL RAS read is unconditional regardless of channel
 		 * state. Any uncorrectable error bit set in the CXL RAS
-		 * status register triggers a panic below because CXL.mem
-		 * cache coherency is already lost; continuing risks silent
-		 * data corruption.
+		 * status register triggers a panic because CXL.mem cache
+		 * coherency is already lost; continuing risks silent data
+		 * corruption.

drivers/cxl/core/ras.c pci_ers_result_t cxl_error_detected(struct pci_dev *pdev, · line 358

Quoted 10 lines

drivers/cxl/core/ras.c +7 -0

+		 *
+		 * On a dead link readl() returns 0xFFFFFFFF which sets all
+		 * UCE bits and also triggers the panic - this is intentional.
+		 * If RAS registers are not mapped the read is skipped, the
+		 * panic is not reached, and the frozen/perm_failure switch
+		 * cases below handle AER recovery for devices without active
+		 * CXL.mem traffic.
 		 */
 		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
 	}

[Severity: High]
The comment states that if RAS registers are not mapped, the read is skipped and AER recovery handles devices without active CXL.mem traffic. Is this assumption about unmapped RAS registers accurate?

It appears that cxl_endpoint_port_probe() maps RAS registers unconditionally for all capable endpoints, regardless of CXL.mem activity. If a link freeze occurs on an idle endpoint, won't this dead link read 0xFFFFFFFF and trigger an unintended panic, effectively breaking standard PCIe AER recovery for these devices?

[Severity: Medium]
This is a pre-existing issue, but since this patch intentionally relies on dead links returning 0xFFFFFFFF during the RAS read, what happens inside cxl_handle_ras() when we encounter one?

drivers/cxl/core/ras.c:cxl_handle_ras() {
    ...
    if (hweight32(status) > 1) {
        void __iomem *rcc_addr = ras_base + CXL_RAS_CAP_CONTROL_OFFSET;
        fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK, readl(rcc_addr)));
    ...
}

When readl() returns 0xFFFFFFFF on the dead link, FIELD_GET() extracts 63. Will evaluating BIT(63) cause an out-of-bounds shift on 32-bit platforms, and won't the 64-bit result be silently truncated to 0 when assigned to the u32 fe variable?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=10

Alison Schofield

maintainer Reviewed-by on v19 10/14

2026-08-04 02:17 UTC #35 lore raw

Re: [PATCH v19 10/14] cxl: Update CXL Endpoint AER handler

Quoted 9 lines · Terry Bowman

Rename cxl_error_detected() to cxl_pci_error_detected() and rename the struct pci_error_handlers instance to cxl_pci_error_handlers to avoid shadowing the struct type tag.
Document the unconditional CXL RAS read policy: on a dead link, readl() returns 0xFFFFFFFF which is interpreted as UCE bits set and triggers a panic. If RAS registers are not mapped the read is skipped and the frozen/perm_failure switch cases defer to AER recovery for devices without active CXL.mem traffic.

Terry Bowman

author

2026-08-03 22:20 UTC #36 lore raw

PATCH v19 11/14 PCI: Cache PCI DSN into pci_dev->dsn during probe

Subsequent CXL error-reporting code paths need to log the PCI Device Serial Number (DSN) as part of trace events emitted from interrupt or panic context. Computing the DSN there via pci_get_dsn() requires PCI configuration space reads, which are slow, can fail when the link is down or frozen, and may not be safe in some contexts.

Add a u64 dsn field to struct pci_dev and populate it from pci_get_dsn() during pci_init_capabilities() at probe time via pci_dsn_init(). Only write dev->dsn when the read succeeds. The zero initial value from pci_dev allocation already represents 'no DSN available.'

Remove the now-redundant dsn member from the pciehp struct controller along with its kernel-doc. Drop the pci_get_slot()/pci_dev_put() pairs in pciehp_configure_device() and pcie_init() that existed solely to read the DSN into ctrl->dsn. Use pdev->dsn in pciehp_device_replaced() for the device-replacement comparison.

pci_get_dsn() is not modified because it remains a pure config-space read with no side effects on pci_dev. The cache is written exclusively by pci_dsn_init() at probe time.

---

Changes in v18 -> v19:
- Update pciehp hotplug to use cached DSN.
- Swapped order in series with
("cxl: Add port and dport identifiers to CXL AER trace events ")
- Update function documentation for pci_dsn_init()

Changes in v17->v18:
- New commit.

---
 drivers/cxl/pci.c                |  2 +-
 drivers/pci/hotplug/pciehp.h     |  4 ----
 drivers/pci/hotplug/pciehp_hpc.c |  7 +------
 drivers/pci/hotplug/pciehp_pci.c |  4 ----
 drivers/pci/probe.c              | 17 +++++++++++++++++
 include/linux/pci.h              |  1 +
 6 files changed, 20 insertions(+), 15 deletions(-)

drivers/cxl/pci.c +1 -1

index 46e60c53d3ca4..cb256d194de28 100644
@@ -807,7 +807,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	if (!dvsec)
 		pci_warn(pdev, "Device DVSEC not present, skip CXL.mem init\n");
 
-	mds = cxl_memdev_state_create(&pdev->dev, pci_get_dsn(pdev), dvsec);
+	mds = cxl_memdev_state_create(&pdev->dev, pdev->dsn, dvsec);
 	if (IS_ERR(mds))
 		return PTR_ERR(mds);
 	cxlds = &mds->cxlds;

drivers/pci/hotplug/pciehp.h +0 -4

index debc79b0adfb2..30aad3b66bab9 100644
@@ -46,9 +46,6 @@ extern int pciehp_poll_time;
 /**
  * struct controller - PCIe hotplug controller
  * @pcie: pointer to the controller's PCIe port service device
- * @dsn: cached copy of Device Serial Number of Function 0 in the hotplug slot
- *	(PCIe r6.2 sec 7.9.3); used to determine whether a hotplugged device
- *	was replaced with a different one during system sleep
  * @slot_cap: cached copy of the Slot Capabilities register
  * @inband_presence_disabled: In-Band Presence Detect Disable supported by
  *	controller and disabled per spec recommendation (PCIe r5.0, appendix I

@@ -90,7 +87,6 @@ extern int pciehp_poll_time;
  */
 struct controller {
 	struct pcie_device *pcie;
-	u64 dsn;
 
 	u32 slot_cap;				/* capabilities and quirks */
 	unsigned int inband_presence_disabled:1;

drivers/pci/hotplug/pciehp_hpc.c +1 -6

index 4c62140a3cb44..5336e9c9003ae 100644
@@ -587,7 +587,7 @@ bool pciehp_device_replaced(struct controller *ctrl)
 	     reg != (pdev->subsystem_vendor | (pdev->subsystem_device << 16))))
 		return true;
 
-	if (pci_get_dsn(pdev) != ctrl->dsn)
+	if (pci_get_dsn(pdev) != pdev->dsn)
 		return true;
 
 	return false;

@@ -1085,11 +1085,6 @@ struct controller *pcie_init(struct pcie_device *dev)
 		}
 	}
 
-	pdev = pci_get_slot(subordinate, PCI_DEVFN(0, 0));
-	if (pdev)
-		ctrl->dsn = pci_get_dsn(pdev);
-	pci_dev_put(pdev);
-
 	return ctrl;
 }

drivers/pci/hotplug/pciehp_pci.c +0 -4

index 65e50bee1a8c0..ad12515a4a121 100644
@@ -72,10 +72,6 @@ int pciehp_configure_device(struct controller *ctrl)
 	pci_bus_add_devices(parent);
 	down_read_nested(&ctrl->reset_lock, ctrl->depth);
 
-	dev = pci_get_slot(parent, PCI_DEVFN(0, 0));
-	ctrl->dsn = pci_get_dsn(dev);
-	pci_dev_put(dev);
-
  out:
 	pci_unlock_rescan_remove();
 	return ret;

drivers/pci/probe.c +17 -0

index dd0abbc63e18d..450e5090564e2 100644
@@ -2638,6 +2638,22 @@ void pcie_report_downtraining(struct pci_dev *dev)
 	__pcie_print_link_status(dev, false);
 }
 
+/*
+ * Cache the Device Serial Number for use in contexts where config-space reads
+ * are unsafe (interrupt, panic).  Process-context callers that need a fresh
+ * value (e.g. hotplug device replacement) call pci_get_dsn() and compare it
+ * against this cached pdev->dsn to detect a changed device.  Note pdev->dsn
+ * is 0 for devices without the DSN capability, so such a comparison cannot
+ * distinguish a replacement.
+ */
+static void pci_dsn_init(struct pci_dev *dev)
+{
+	u64 dsn = pci_get_dsn(dev);
+
+	if (dsn)
+		dev->dsn = dsn;
+}
+
 static void pci_imm_ready_init(struct pci_dev *dev)
 {
 	u16 status;

@@ -2674,6 +2690,7 @@ static void pci_init_capabilities(struct pci_dev *dev)
 	pci_rebar_init(dev);		/* Resizable BAR */
 	pci_dev3_init(dev);		/* Device 3 capabilities */
 	pci_ide_init(dev);		/* Link Integrity and Data Encryption */
+	pci_dsn_init(dev);		/* Serial number */
 
 	pcie_report_downtraining(dev);
 	pci_init_reset_methods(dev);

include/linux/pci.h +1 -0

index 64b308b6e61c1..48a1622639190 100644
@@ -386,6 +386,7 @@ struct pci_dev {
 	unsigned long	*dma_alias_mask;/* Mask of enabled devfn aliases */
 
 	struct pci_driver *driver;	/* Driver bound to this device */
+	u64		dsn;		/* PCI Device Serial Number */
 	u64		dma_mask;	/* Mask of the bits of bus address this
 					   device implements.  Normally this is
 					   0xffffffff.  You only need to change

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:30 UTC #37 lore raw

Re: [PATCH v19 11/14] PCI: Cache PCI DSN into pci_dev->dsn during probe

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 17 lines

Subsequent CXL error-reporting code paths need to log the PCI Device Serial Number (DSN) as part of trace events emitted from interrupt or panic context. Computing the DSN there via pci_get_dsn() requires PCI configuration space reads, which are slow, can fail when the link is down or frozen, and may not be safe in some contexts.
Add a u64 dsn field to struct pci_dev and populate it from pci_get_dsn() during pci_init_capabilities() at probe time via pci_dsn_init(). Only write dev->dsn when the read succeeds. The zero initial value from pci_dev allocation already represents 'no DSN available.'
Remove the now-redundant dsn member from the pciehp struct controller along with its kernel-doc. Drop the pci_get_slot()/pci_dev_put() pairs in pciehp_configure_device() and pcie_init() that existed solely to read the DSN into ctrl->dsn. Use pdev->dsn in pciehp_device_replaced() for the device-replacement comparison.

Quoted 5 lines

pci_get_dsn() is not modified because it remains a pure config-space read with no side effects on pci_dev. The cache is written exclusively by pci_dsn_init() at probe time.
Signed-off-by Terry Bowman <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=11

Alison Schofield

maintainer Reviewed-by on v19 11/14

2026-08-04 02:26 UTC #38 lore raw

Re: [PATCH v19 11/14] PCI: Cache PCI DSN into pci_dev->dsn during probe

Quoted 17 lines · Terry Bowman

Subsequent CXL error-reporting code paths need to log the PCI Device Serial Number (DSN) as part of trace events emitted from interrupt or panic context. Computing the DSN there via pci_get_dsn() requires PCI configuration space reads, which are slow, can fail when the link is down or frozen, and may not be safe in some contexts.
Add a u64 dsn field to struct pci_dev and populate it from pci_get_dsn() during pci_init_capabilities() at probe time via pci_dsn_init(). Only write dev->dsn when the read succeeds. The zero initial value from pci_dev allocation already represents 'no DSN available.'
Remove the now-redundant dsn member from the pciehp struct controller along with its kernel-doc. Drop the pci_get_slot()/pci_dev_put() pairs in pciehp_configure_device() and pcie_init() that existed solely to read the DSN into ctrl->dsn. Use pdev->dsn in pciehp_device_replaced() for the device-replacement comparison.

Quoted 4 lines

pci_get_dsn() is not modified because it remains a pure config-space read with no side effects on pci_dev. The cache is written exclusively by pci_dsn_init() at probe time.

Terry Bowman

author

2026-08-03 22:20 UTC #39 lore raw

PATCH v19 12/14 cxl: Add port and dport identifiers to CXL AER trace events

From: Dan Williams <[email protected]>

Pass struct cxl_port * and struct cxl_dport * to the cxl_aer_* trace events instead of a plain struct device * derived at the caller. The trace event helpers then derive the right strings for Endpoints, Switch Ports, Root Ports, and RCH Downstream Ports consistently across the CPER and native AER paths.

The unified cxl_aer_* events keep "memdev" as the legacy field (endpoint events populate it with the memdev name; non-endpoint events emit memdev="") and add new "port" and "dport" string fields populated for all CXL device classes. Updated userspace can key off "port" and "dport" without a parallel set of events.

Remove the separate cxl_port_aer_uncorrectable_error and cxl_port_aer_correctable_error trace events. All CXL AER events now use the unified cxl_aer_* events with port and dport fields.

Rework cxl_cper_handle_prot_err() to use find_cxl_port_by_dev() and the unified trace helpers, replacing the per-port-type branching and bus_find_device() memdev lookup.

The TP_printk format string places "port=%s dport=%s" between "memdev=%s" and "host=%s", changing the text-mode field order from the pre-patch output. This does not affect consumers such as rasdaemon that use libtraceevent to parse fields by name rather than by fixed text position.

For non-Endpoint events (Switch Port, Root Port, RCH Dport), "memdev" is empty and "port"/"dport" carry the topology information.

Below are examples of the different CXL devices' error trace logs after this patch:

     ---------------------
     | CXL RP - 0C:00.0  |
     ---------------------
               |
     ---------------------
     | CXL USP - 0D:00.0 |
     ---------------------
               |
     --------------------
     | CXL DSP - 0E:00.0 |
     --------------------
               |
     ---------------------
     | CXL EP - 0F:00.0  |
     ---------------------

Root Port:
cxl_aer_correctable_error: memdev= port=port1 dport=0000:0c:00.0 \
host=pci0000:0c serial=0: status: 'Memory Data ECC Error'

cxl_aer_uncorrectable_error: memdev= port=port1 dport=0000:0c:00.0 \
host=pci0000:0c serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'

Upstream Switch Port:
cxl_aer_correctable_error: memdev= port=port2 dport= host=0000:0d:00.0 \
serial=0: status: 'Memory Data ECC Error'

UCE NA - Upstream Switch Port UCE's are handled in the portdrv driver's PCI AER callbacks that are not CXL aware.

Downstream Switch Port:
cxl_aer_correctable_error: memdev= port=port2 dport=0000:0e:00.0 \
host=0000:0d:00.0 serial=0: status: 'Memory Data ECC Error'

cxl_aer_uncorrectable_error: memdev= port=port2 dport=0000:0e:00.0 \
host=0000:0d:00.0 serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'

Endpoint:
cxl_aer_uncorrectable_error: memdev=mem1 port=endpoint4 dport= \
host=0000:0f:00.0 serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'

cxl_aer_correctable_error: memdev=mem1 port=endpoint4 dport= host=0000:0f:00.0 \
serial=0: status: 'Memory Data ECC Error'

Co-developed-by Terry Bowman <[email protected]>

---

Changes in v18->v19:
- Drop redundant device lock in cxl_cper_handle_prot_err(); the port reference already keeps the object alive and no RAS iomap is accessed.
- Swap order in series with ("PCI: Cache PCI DSN into pci_dev->dsn during probe")
- Add review-by for DaveJ

Changes in v17->v18:
- Consolidate double find_cxl_port_by_dev() in cxl_cper_handle_prot_err()
- Add comment noting dport is NULL for Endpoint and Upstream Port devices
- Add cxl_trace_* helpers
- Add CPER refactor

Changes in v16->v17:
- Replace cxlds->serial with pci_get_dsn()
- Change 'memdev' to 'device' (Dan)
- Updated Commit message

Changes in v15->v16:
- Add Dan's review-by
- Incorporate Dan's comment into commit message:
"Add the serial number at the end to preserve compatibility with libtraceevent parsing of the parameters."

Changes in v14->v15:
- Update commit message.
- Moved cxl_handle_ras/cxl_handle_cor_ras() changes to future patch (terry)

Changes in v13->v14:
- Update commit headline (Bjorn)

Changes in v12->v13:
- Added Dave Jiang's review-by

Changes in v11 -> v12:
- Correct parameters to call trace_cxl_aer_correctable_error()
- Add reviewed-by for Jonathan and Shiju

Changes in v10->v11:
- Updated CE and UCE trace routines to maintain consistent TP_Struct ABI and unchanged TP_printk() logging.

---
 drivers/cxl/core/core.h    |   8 +--
 drivers/cxl/core/ras.c     | 129 +++++++++++--------------------------
 drivers/cxl/core/ras_rch.c |   3 +-
 drivers/cxl/core/trace.c   |  35 ++++++++++
 drivers/cxl/core/trace.h   |  91 ++++++++------------------
 drivers/cxl/cxlmem.h       |   7 ++
 6 files changed, 113 insertions(+), 160 deletions(-)

drivers/cxl/core/core.h +4 -4

index 5ca1275fd8f35..a55a4e409feda 100644
@@ -186,11 +186,11 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
 void cxl_ras_init(void);
 void cxl_ras_exit(void);
 bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
-		    void __iomem *ras_base);
+		    void __iomem *ras_base, u64 serial);
 void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 		     struct cxl_dport *dport);
 void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
-			void __iomem *ras_base);
+			void __iomem *ras_base, u64 serial);
 void cxl_dport_map_rch_aer(struct cxl_dport *dport);
 void cxl_disable_rch_root_ints(struct cxl_dport *dport);
 void cxl_handle_rdport_errors(struct pci_dev *pdev);

@@ -200,14 +200,14 @@ void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
 static inline void cxl_ras_init(void) { }
 static inline void cxl_ras_exit(void) { }
 static inline bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
-				  void __iomem *ras_base)
+				  void __iomem *ras_base, u64 serial)
 {
 	return false;
 }
 static inline void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport) { }
 static inline void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
-				      void __iomem *ras_base) { }
+				      void __iomem *ras_base, u64 serial) { }
 static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
 static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
 static inline void cxl_handle_rdport_errors(struct pci_dev *pdev) { }

drivers/cxl/core/ras.c +39 ~~-90~~

211 lines not on this page · raw message

drivers/cxl/core/ras_rch.c +2 -1

index a5c62c71060d9..a371174536a8b 100644
@@ -113,7 +113,8 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
 	 */
 	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
 		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
-		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport),
+			pdev->dsn);
 	}
 
 	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {

drivers/cxl/core/trace.c +35 -0

index 7f2a9dd0d0e3f..df42d119c53dd 100644
@@ -2,7 +2,42 @@
 /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
 
 #include <cxl.h>
+#include <cxlmem.h>
 #include "core.h"
 
+const char *cxl_trace_memdev_name(struct cxl_port *port)
+{
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+		return dev_name(&cxlmd->dev);
+	}
+
+	return "";
+}
+
+const char *cxl_trace_host_name(struct cxl_port *port)
+{
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+		return dev_name(cxlmd->dev.parent);
+	}
+
+	return dev_name(port->uport_dev);
+}
+
+const char *cxl_trace_port_name(struct cxl_port *port)
+{
+	return dev_name(&port->dev);
+}
+
+const char *cxl_trace_dport_name(struct cxl_dport *dport)
+{
+	if (dport)
+		return dev_name(dport->dport_dev);
+	return "";
+}
+
 #define CREATE_TRACE_POINTS
 #include "trace.h"

drivers/cxl/core/trace.h +26 ~~-65~~

128 lines not on this page · raw message

drivers/cxl/cxlmem.h +7 -0

index ed419d0c59f2f..f1ef8b78db18a 100644
@@ -125,6 +125,13 @@ static inline int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
 #endif
 
 struct cxl_memdev *devm_cxl_add_classdev(struct cxl_dev_state *cxlds);
+
+/* trace-event helpers */
+const char *cxl_trace_memdev_name(struct cxl_port *port);
+const char *cxl_trace_host_name(struct cxl_port *port);
+const char *cxl_trace_port_name(struct cxl_port *port);
+const char *cxl_trace_dport_name(struct cxl_dport *dport);
+
 struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 					 const struct cxl_memdev_attach *attach);
 int devm_cxl_sanitize_setup_notifier(struct device *host,

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:42 UTC #40 lore raw

Re: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Missing device lock in cxl_cper_handle_prot_err() leads to Use-After-Free of devres-managed…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing device lock in cxl_cper_handle_prot_err() leads to Use-After-Free of devres-managed objects like 'dport' during concurrent driver unbind. --

--- Patch [12]: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events --- commit 3fa84eaf0343ed1c7fb39fd22762efda3659de2c
Author: Dan Williams <[email protected]>

cxl: Add port and dport identifiers to CXL AER trace events

This commit modifies CXL AER trace events to use struct cxl_port and struct cxl_dport pointers instead of deriving device strings at the caller. It unifies the correctable and uncorrectable error trace events and updates cxl_cper_handle_prot_err() to use these new helpers while dropping a device lock it considered redundant.

Quoted 3 lines

Changes in v18->v19:
- Drop redundant device lock in cxl_cper_handle_prot_err(); the port reference already keeps the object alive and no RAS iomap is accessed.

[Severity: High]
Does dropping the device lock here leave devres-managed objects unprotected?

The commit message notes that the port reference keeps the object alive, but a device reference only protects the struct device allocation itself, not the driver binding state or devm actions.

[ ... ]

Quoted 4 lines

drivers/cxl/core/ras.c +0 -0
index 5183b3c532952..0254b7ec64c30 100644

[ ... ]

Quoted 4 lines

drivers/cxl/core/ras.c +0 -0

@@ -108,44 +76,32 @@ static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor
 
 void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
 {

[ ... ]

Quoted 16 lines

drivers/cxl/core/ras.c +7 -6

-	guard(device)(&pdev->dev);
-	if (!pdev->dev.driver)
+	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&pdev->dev, NULL);
+	if (!port) {
+		dev_err_ratelimited(&pdev->dev,
+				    "Failed to find parent port device in CXL topology\n");
 		return;
+	}
 
-	struct device *mem_dev __free(put_device) = bus_find_device(
-		&cxl_bus_type, NULL, pdev, match_memdev_by_parent);
-	if (!mem_dev)
-		return;
+	/* dport is NULL for Endpoint and Upstream Port devices */
+	dport = cxl_find_dport_by_dev(port, &pdev->dev);

drivers/cxl/core/ras.c static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor · line 105

Quoted 10 lines

drivers/cxl/core/ras.c +4 -3

-	cxlmd = to_cxl_memdev(mem_dev);
 	if (data->severity == AER_CORRECTABLE)
-		cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
+		cxl_cper_trace_corr_prot_err(port, dport, pdev->dsn,
+					     &data->ras_cap);
 	else
-		cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
+		cxl_cper_trace_uncorr_prot_err(port, dport, pdev->dsn,
+					       &data->ras_cap);
 }

[Severity: High]
Can accessing dport here race with a driver unbind?

If a concurrent CXL port driver unbind occurs (via sysfs or device hot-remove), the devres release actions will execute while cxl_cper_handle_prot_err() is still running.

This could result in the devm-managed dport being freed, leaving the dport pointer referencing freed memory. This pointer is then passed into the trace events where cxl_trace_dport_name() attempts to read dport->dport_dev:

drivers/cxl/core/trace.c:cxl_trace_dport_name() {
    ...
    if (dport)
        return dev_name(dport->dport_dev);
    ...
}

Could this result in a use-after-free?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12

Alison Schofield

maintainer Reviewed-by on v19 12/14

2026-08-04 02:27 UTC #41 lore raw

Re: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events

Quoted 14 lines · Terry Bowman

From: Dan Williams <[email protected]>
Pass struct cxl_port * and struct cxl_dport * to the cxl_aer_* trace events instead of a plain struct device * derived at the caller. The trace event helpers then derive the right strings for Endpoints, Switch Ports, Root Ports, and RCH Downstream Ports consistently across the CPER and native AER paths.
The unified cxl_aer_* events keep "memdev" as the legacy field (endpoint events populate it with the memdev name; non-endpoint events emit memdev="") and add new "port" and "dport" string fields populated for all CXL device classes. Updated userspace can key off "port" and "dport" without a parallel set of events.

Quoted 7 lines

Remove the separate cxl_port_aer_uncorrectable_error and cxl_port_aer_correctable_error trace events. All CXL AER events now use the unified cxl_aer_* events with port and dport fields.
Rework cxl_cper_handle_prot_err() to use find_cxl_port_by_dev() and the unified trace helpers, replacing the per-port-type branching and bus_find_device() memdev lookup.

Richard Cheng

2026-08-04 07:56 UTC #42 lore raw

Re: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events

Quoted 299 lines · Terry Bowman

From: Dan Williams <[email protected]>
Pass struct cxl_port * and struct cxl_dport * to the cxl_aer_* trace events instead of a plain struct device * derived at the caller. The trace event helpers then derive the right strings for Endpoints, Switch Ports, Root Ports, and RCH Downstream Ports consistently across the CPER and native AER paths.
The unified cxl_aer_* events keep "memdev" as the legacy field (endpoint events populate it with the memdev name; non-endpoint events emit memdev="") and add new "port" and "dport" string fields populated for all CXL device classes. Updated userspace can key off "port" and "dport" without a parallel set of events.
Remove the separate cxl_port_aer_uncorrectable_error and cxl_port_aer_correctable_error trace events. All CXL AER events now use the unified cxl_aer_* events with port and dport fields.
Rework cxl_cper_handle_prot_err() to use find_cxl_port_by_dev() and the unified trace helpers, replacing the per-port-type branching and bus_find_device() memdev lookup.
The TP_printk format string places "port=%s dport=%s" between "memdev=%s" and "host=%s", changing the text-mode field order from the pre-patch output. This does not affect consumers such as rasdaemon that use libtraceevent to parse fields by name rather than by fixed text position.
For non-Endpoint events (Switch Port, Root Port, RCH Dport), "memdev" is empty and "port"/"dport" carry the topology information.
Below are examples of the different CXL devices' error trace logs after this patch:
     ---------------------
     | CXL RP - 0C:00.0  |
     ---------------------
               |
     ---------------------
     | CXL USP - 0D:00.0 |
     ---------------------
               |
     --------------------
     | CXL DSP - 0E:00.0 |
     --------------------
               |
     ---------------------
     | CXL EP - 0F:00.0  |
     ---------------------
Root Port:
cxl_aer_correctable_error: memdev= port=port1 dport=0000:0c:00.0 \
host=pci0000:0c serial=0: status: 'Memory Data ECC Error'
cxl_aer_uncorrectable_error: memdev= port=port1 dport=0000:0c:00.0 \
host=pci0000:0c serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'
Upstream Switch Port:
cxl_aer_correctable_error: memdev= port=port2 dport= host=0000:0d:00.0 \
serial=0: status: 'Memory Data ECC Error'
UCE NA - Upstream Switch Port UCE's are handled in the portdrv driver's PCI AER callbacks that are not CXL aware.
Downstream Switch Port:
cxl_aer_correctable_error: memdev= port=port2 dport=0000:0e:00.0 \
host=0000:0d:00.0 serial=0: status: 'Memory Data ECC Error'
cxl_aer_uncorrectable_error: memdev= port=port2 dport=0000:0e:00.0 \
host=0000:0d:00.0 serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'
Endpoint:
cxl_aer_uncorrectable_error: memdev=mem1 port=endpoint4 dport= \
host=0000:0f:00.0 serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'
cxl_aer_correctable_error: memdev=mem1 port=endpoint4 dport= host=0000:0f:00.0 \
serial=0: status: 'Memory Data ECC Error'
Co-developed-by Terry Bowman <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Signed-off-by Dan Williams <[email protected]>
Reviewed-by Dave Jiang <[email protected]>
---
Changes in v18->v19:
- Drop redundant device lock in cxl_cper_handle_prot_err(); the port reference already keeps the object alive and no RAS iomap is accessed.
- Swap order in series with ("PCI: Cache PCI DSN into pci_dev->dsn during probe")
- Add review-by for DaveJ
Changes in v17->v18:
- Consolidate double find_cxl_port_by_dev() in cxl_cper_handle_prot_err()
- Add comment noting dport is NULL for Endpoint and Upstream Port devices
- Add cxl_trace_* helpers
- Add CPER refactor
Changes in v16->v17:
- Replace cxlds->serial with pci_get_dsn()
- Change 'memdev' to 'device' (Dan)
- Updated Commit message
Changes in v15->v16:
- Add Dan's review-by
- Incorporate Dan's comment into commit message:
"Add the serial number at the end to preserve compatibility with libtraceevent parsing of the parameters."
Changes in v14->v15:
- Update commit message.
- Moved cxl_handle_ras/cxl_handle_cor_ras() changes to future patch (terry)
Changes in v13->v14:
- Update commit headline (Bjorn)
Changes in v12->v13:
- Added Dave Jiang's review-by
Changes in v11 -> v12:
- Correct parameters to call trace_cxl_aer_correctable_error()
- Add reviewed-by for Jonathan and Shiju
Changes in v10->v11:
- Updated CE and UCE trace routines to maintain consistent TP_Struct ABI and unchanged TP_printk() logging.
---
 drivers/cxl/core/core.h    |   8 +--
 drivers/cxl/core/ras.c     | 129 +++++++++++--------------------------
 drivers/cxl/core/ras_rch.c |   3 +-
 drivers/cxl/core/trace.c   |  35 ++++++++++
 drivers/cxl/core/trace.h   |  91 ++++++++------------------
 drivers/cxl/cxlmem.h       |   7 ++
 6 files changed, 113 insertions(+), 160 deletions(-)
drivers/cxl/core/core.h +4 -4
index 5ca1275fd8f35..a55a4e409feda 100644
@@ -186,11 +186,11 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
 void cxl_ras_init(void);
 void cxl_ras_exit(void);
 bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
-		    void __iomem *ras_base);
+		    void __iomem *ras_base, u64 serial);
 void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 		     struct cxl_dport *dport);
 void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
-			void __iomem *ras_base);
+			void __iomem *ras_base, u64 serial);
 void cxl_dport_map_rch_aer(struct cxl_dport *dport);
 void cxl_disable_rch_root_ints(struct cxl_dport *dport);
 void cxl_handle_rdport_errors(struct pci_dev *pdev);
@@ -200,14 +200,14 @@ void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
 static inline void cxl_ras_init(void) { }
 static inline void cxl_ras_exit(void) { }
 static inline bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
-				  void __iomem *ras_base)
+				  void __iomem *ras_base, u64 serial)
 {
 	return false;
 }
 static inline void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport) { }
 static inline void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
-				      void __iomem *ras_base) { }
+				      void __iomem *ras_base, u64 serial) { }
 static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
 static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
 static inline void cxl_handle_rdport_errors(struct pci_dev *pdev) { }
drivers/cxl/core/ras.c +22 ~~-66~~
index 5183b3c532952..0254b7ec64c30 100644
@@ -12,69 +12,37 @@
 static_assert(CXL_HEADERLOG_TRACE_SIZE_U32 == 128,
 	      "rasdaemon ABI requires exactly 128 u32s");
 
-static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev,
-					      struct cxl_ras_capability_regs ras_cap)
-{
-	u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
-
-	trace_cxl_port_aer_correctable_error(&pdev->dev, status);
-}
-
-static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
-						struct cxl_ras_capability_regs ras_cap)
+static void cxl_cper_trace_uncorr_prot_err(struct cxl_port *port, struct cxl_dport *dport,
+					   u64 serial, struct cxl_ras_capability_regs *ras_cap)
 {
 	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
-	u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
+	u32 status = ras_cap->uncor_status & ~ras_cap->uncor_mask;
 	u32 fe;
 
 	if (hweight32(status) > 1)
 		fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
-				   ras_cap.cap_control));
-	else
-		fe = status;
-
-	memcpy(hl, ras_cap.header_log, CXL_HEADERLOG_SIZE);
-	trace_cxl_port_aer_uncorrectable_error(&pdev->dev, status, fe, hl);
-}
-
-static void cxl_cper_trace_corr_prot_err(struct cxl_memdev *cxlmd,
-					 struct cxl_ras_capability_regs ras_cap)
-{
-	u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
-
-	trace_cxl_aer_correctable_error(cxlmd, status);
-}
-
-static void
-cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
-			       struct cxl_ras_capability_regs ras_cap)
-{
-	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
-	u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
-	u32 fe;
-
-	if (hweight32(status) > 1)
-		fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
-				   ras_cap.cap_control));
+				   ras_cap->cap_control));
 	else
 		fe = status;
 
 	/*
-	 * ras_cap.header_log[] holds CXL_HEADERLOG_SIZE_U32 (16) hardware
+	 * ras_cap->header_log[] holds CXL_HEADERLOG_SIZE_U32 (16) hardware
 	 * dwords. Copy them into the front of a zero-filled
 	 * CXL_HEADERLOG_TRACE_SIZE_U32 (128) u32 staging buffer so the trace
 	 * event memcpy sees a full 512-byte source and the userspace ABI
 	 * (rasdaemon) is preserved.
 	 */
-	memcpy(hl, ras_cap.header_log, CXL_HEADERLOG_SIZE);
-	trace_cxl_aer_uncorrectable_error(cxlmd, status, fe, hl);
+	memcpy(hl, ras_cap->header_log, CXL_HEADERLOG_SIZE);
+	trace_cxl_aer_uncorrectable_error(port, dport, status, fe,
+					  hl, serial);
 }
 
-static int match_memdev_by_parent(struct device *dev, const void *uport)
+static void cxl_cper_trace_corr_prot_err(struct cxl_port *port, struct cxl_dport *dport,
+					 u64 serial, struct cxl_ras_capability_regs *ras_cap)
 {
-	if (is_cxl_memdev(dev) && dev->parent == uport)
-		return 1;
-	return 0;
+	u32 status = ras_cap->cor_status & ~ras_cap->cor_mask;
+
+	trace_cxl_aer_correctable_error(port, dport, status, serial);
 }
 
 /**
@@ -108,44 +76,32 @@ static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor
 
 void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
 {
+	struct cxl_dport *dport;
 	unsigned int devfn = PCI_DEVFN(data->prot_err.agent_addr.device,
 				       data->prot_err.agent_addr.function);
-	struct pci_dev *pdev __free(pci_dev_put) =
-		pci_get_domain_bus_and_slot(data->prot_err.agent_addr.segment,
-					    data->prot_err.agent_addr.bus,
-					    devfn);
-	struct cxl_memdev *cxlmd;
-	int port_type;
-
-	if (!pdev)
-		return;
-
-	port_type = pci_pcie_type(pdev);
-	if (port_type == PCI_EXP_TYPE_ROOT_PORT ||
-	    port_type == PCI_EXP_TYPE_DOWNSTREAM ||
-	    port_type == PCI_EXP_TYPE_UPSTREAM) {
-		if (data->severity == AER_CORRECTABLE)
-			cxl_cper_trace_corr_port_prot_err(pdev, data->ras_cap);
-		else
-			cxl_cper_trace_uncorr_port_prot_err(pdev, data->ras_cap);
-
+	struct pci_dev *pdev __free(pci_dev_put) = pci_get_domain_bus_and_slot(
+		data->prot_err.agent_addr.segment, data->prot_err.agent_addr.bus, devfn);
+	if (!pdev) {
+		pr_err_ratelimited("Failed to find CPER device in CXL topology\n");
 		return;
 	}
 
-	guard(device)(&pdev->dev);
-	if (!pdev->dev.driver)
+	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&pdev->dev, NULL);
+	if (!port) {
+		dev_err_ratelimited(&pdev->dev,
+				    "Failed to find parent port device in CXL topology\n");

drivers/cxl/core/ras.c static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor · line 98

Quoted 10 lines

drivers/cxl/core/ras.c +3 -4

 		return;
+	}
 
-	struct device *mem_dev __free(put_device) = bus_find_device(
-		&cxl_bus_type, NULL, pdev, match_memdev_by_parent);
-	if (!mem_dev)
-		return;
+	/* dport is NULL for Endpoint and Upstream Port devices */
+	dport = cxl_find_dport_by_dev(port, &pdev->dev);

Hi Terry,

I have a question here.

Do we need the port device lock here ?

cxl_find_dport_by_dev() is xa_load(), and the free side is serialized by that lock. del_dports() has device_lock_assert(&port->dev) and frees the dport which in the end resolves to kfree().

__free(put_cxl_port) doesn't cover it, that's a kobject ref on &port->dev, so it pins the struct cxl_port but not the dports.

I think cxl_cper_handle_prot_err() might race cxl_detach_ep() calling del_dports(), and cxl_trace_dport_name() then does dev_name(dport->dport_dev) on freed memory.

I see __cxl_proto_err_work_fn(), cxl_handle_rdport_errors() and cxl_pci_error_detected() all take the guard first, should we do the same here?

Though I didn't poke this issue out in runtime, I guess it needs FW-first, cxl_aer_* enabled, and a concurrent teardown. Not an expert of FW, let me knowo if something already rules it out.

Best regards,
Richard Cheng.

Quoted 305 lines

drivers/cxl/core/ras.c +14 ~~-20~~

-	cxlmd = to_cxl_memdev(mem_dev);
 	if (data->severity == AER_CORRECTABLE)
-		cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
+		cxl_cper_trace_corr_prot_err(port, dport, pdev->dsn,
+					     &data->ras_cap);
 	else
-		cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
+		cxl_cper_trace_uncorr_prot_err(port, dport, pdev->dsn,
+					       &data->ras_cap);
 }
 EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);

@@ -232,14 +188,15 @@ void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dpo
 	if (!ras_base)
 		panic("CXL: UCE with unmapped RAS registers");
 
-	if (cxl_handle_ras(port, dport, ras_base))
+	if (cxl_handle_ras(port, dport, ras_base, pdev->dsn))
 		panic("CXL cachemem error");
 
 	dev_dbg(&pdev->dev,
 		"CXL UCE signaled but no CXL RAS status bits set\n");
 }
 
-void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
+void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
+			void __iomem *ras_base, u64 serial)
 {
 	void __iomem *addr;
 	u32 status;

@@ -251,12 +208,7 @@ void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __i
 	status = readl(addr);
 	if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
 		writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
-		if (is_cxl_endpoint(port))
-			trace_cxl_aer_correctable_error(to_cxl_memdev(port->uport_dev), status);
-		else if (dport)
-			trace_cxl_port_aer_correctable_error(dport->dport_dev, status);
-		else
-			trace_cxl_port_aer_correctable_error(port->uport_dev, status);
+		trace_cxl_aer_correctable_error(port, dport, status, serial);
 	}
 }

@@ -281,7 +233,8 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
  * Log the state of the RAS status registers and prepare them to log the
  * next error status. Return 1 if reset needed.
  */
-bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
+bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
+		    void __iomem *ras_base, u64 serial)
 {
 	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
 	void __iomem *addr;

@@ -308,12 +261,7 @@ bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem
 	}
 
 	header_log_copy(ras_base, hl);
-	if (is_cxl_endpoint(port))
-		trace_cxl_aer_uncorrectable_error(to_cxl_memdev(port->uport_dev), status, fe, hl);
-	else if (dport)
-		trace_cxl_port_aer_uncorrectable_error(dport->dport_dev, status, fe, hl);
-	else
-		trace_cxl_port_aer_uncorrectable_error(port->uport_dev, status, fe, hl);
+	trace_cxl_aer_uncorrectable_error(port, dport, status, fe, hl, serial);
 
 	writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);

@@ -354,7 +302,8 @@ pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 		 * cases below handle AER recovery for devices without active
 		 * CXL.mem traffic.
 		 */
-		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
+		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL),
+				    pdev->dsn);
 	}
 
 	/*

@@ -386,7 +335,7 @@ static void cxl_handle_proto_error(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport, int severity)
 {
 	if (severity == AER_CORRECTABLE)
-		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport), pdev->dsn);
 	else
 		cxl_do_recovery(pdev, port, dport);
 }

drivers/cxl/core/ras_rch.c +2 -1

index a5c62c71060d9..a371174536a8b 100644
@@ -113,7 +113,8 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
 	 */
 	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
 		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
-		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport),
+			pdev->dsn);
 	}
 
 	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {

drivers/cxl/core/trace.c +35 -0

index 7f2a9dd0d0e3f..df42d119c53dd 100644
@@ -2,7 +2,42 @@
 /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
 
 #include <cxl.h>
+#include <cxlmem.h>
 #include "core.h"
 
+const char *cxl_trace_memdev_name(struct cxl_port *port)
+{
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+		return dev_name(&cxlmd->dev);
+	}
+
+	return "";
+}
+
+const char *cxl_trace_host_name(struct cxl_port *port)
+{
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+		return dev_name(cxlmd->dev.parent);
+	}
+
+	return dev_name(port->uport_dev);
+}
+
+const char *cxl_trace_port_name(struct cxl_port *port)
+{
+	return dev_name(&port->dev);
+}
+
+const char *cxl_trace_dport_name(struct cxl_dport *dport)
+{
+	if (dport)
+		return dev_name(dport->dport_dev);
+	return "";
+}
+
 #define CREATE_TRACE_POINTS
 #include "trace.h"

drivers/cxl/core/trace.h +26 ~~-65~~

index d37876096dd7c..910aceb2ca3ab 100644
@@ -48,44 +48,15 @@
 	{ CXL_RAS_UC_IDE_RX_ERR, "IDE Rx Error" }			  \
 )
 
-TRACE_EVENT(cxl_port_aer_uncorrectable_error,
-	TP_PROTO(struct device *dev, u32 status, u32 fe, u32 *hl),
-	TP_ARGS(dev, status, fe, hl),
-	TP_STRUCT__entry(
-		__string(device, dev_name(dev))
-		__string(host, dev_name(dev->parent))
-		__field(u32, status)
-		__field(u32, first_error)
-		__array(u32, header_log, CXL_HEADERLOG_TRACE_SIZE_U32)
-	),
-	TP_fast_assign(
-		__assign_str(device);
-		__assign_str(host);
-		__entry->status = status;
-		__entry->first_error = fe;
-		/*
-		 * Embed headerlog data for user app retrieval and parsing,
-		 * but no need to print in the trace buffer. Only
-		 * CXL_HEADERLOG_SIZE_U32 (16) dwords are hardware data;
-		 * the remaining entries preserve the 512-byte ABI layout
-		 * rasdaemon depends on and are zero-filled by the caller.
-		 */
-		memcpy(__entry->header_log, hl,
-			CXL_HEADERLOG_TRACE_SIZE_U32 * sizeof(u32));
-	),
-	TP_printk("device=%s host=%s status: '%s' first_error: '%s'",
-		  __get_str(device), __get_str(host),
-		  show_uc_errs(__entry->status),
-		  show_uc_errs(__entry->first_error)
-	)
-);
-
 TRACE_EVENT(cxl_aer_uncorrectable_error,
-	TP_PROTO(const struct cxl_memdev *cxlmd, u32 status, u32 fe, u32 *hl),
-	TP_ARGS(cxlmd, status, fe, hl),
+	TP_PROTO(struct cxl_port *port, struct cxl_dport *dport,
+		 u32 status, u32 fe, u32 *hl, u64 serial),
+	TP_ARGS(port, dport, status, fe, hl, serial),
 	TP_STRUCT__entry(
-		__string(memdev, dev_name(&cxlmd->dev))
-		__string(host, dev_name(cxlmd->dev.parent))
+		__string(memdev, cxl_trace_memdev_name(port))
+		__string(port, cxl_trace_port_name(port))
+		__string(dport, cxl_trace_dport_name(dport))
+		__string(host, cxl_trace_host_name(port))
 		__field(u64, serial)
 		__field(u32, status)
 		__field(u32, first_error)

@@ -93,8 +64,10 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
 	),
 	TP_fast_assign(
 		__assign_str(memdev);
+		__assign_str(port);
+		__assign_str(dport);
 		__assign_str(host);
-		__entry->serial = cxlmd->cxlds->serial;
+		__entry->serial = serial;
 		__entry->status = status;
 		__entry->first_error = fe;
 		/*

@@ -107,8 +80,9 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
 		memcpy(__entry->header_log, hl,
 			CXL_HEADERLOG_TRACE_SIZE_U32 * sizeof(u32));
 	),
-	TP_printk("memdev=%s host=%s serial=%lld: status: '%s' first_error: '%s'",
-		  __get_str(memdev), __get_str(host), __entry->serial,
+	TP_printk("memdev=%s port=%s dport=%s host=%s serial=%lld: status: '%s' first_error: '%s'",
+		  __get_str(memdev), __get_str(port), __get_str(dport),
+		  __get_str(host), __entry->serial,
 		  show_uc_errs(__entry->status),
 		  show_uc_errs(__entry->first_error)
 	)

@@ -132,42 +106,29 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
 	{ CXL_RAS_CE_PHYS_LAYER_ERR, "Received Error From Physical Layer" }	\
 )
 
-TRACE_EVENT(cxl_port_aer_correctable_error,
-	TP_PROTO(struct device *dev, u32 status),
-	TP_ARGS(dev, status),
-	TP_STRUCT__entry(
-		__string(device, dev_name(dev))
-		__string(host, dev_name(dev->parent))
-		__field(u32, status)
-	),
-	TP_fast_assign(
-		__assign_str(device);
-		__assign_str(host);
-		__entry->status = status;
-	),
-	TP_printk("device=%s host=%s status='%s'",
-		  __get_str(device), __get_str(host),
-		  show_ce_errs(__entry->status)
-	)
-);
-
 TRACE_EVENT(cxl_aer_correctable_error,
-	TP_PROTO(const struct cxl_memdev *cxlmd, u32 status),
-	TP_ARGS(cxlmd, status),
+	TP_PROTO(struct cxl_port *port, struct cxl_dport *dport,
+		 u32 status, u64 serial),
+	TP_ARGS(port, dport, status, serial),
 	TP_STRUCT__entry(
-		__string(memdev, dev_name(&cxlmd->dev))
-		__string(host, dev_name(cxlmd->dev.parent))
+		__string(memdev, cxl_trace_memdev_name(port))
+		__string(port, cxl_trace_port_name(port))
+		__string(dport, cxl_trace_dport_name(dport))
+		__string(host, cxl_trace_host_name(port))
 		__field(u64, serial)
 		__field(u32, status)
 	),
 	TP_fast_assign(
 		__assign_str(memdev);
+		__assign_str(port);
+		__assign_str(dport);
 		__assign_str(host);
-		__entry->serial = cxlmd->cxlds->serial;
+		__entry->serial = serial;
 		__entry->status = status;
 	),
-	TP_printk("memdev=%s host=%s serial=%lld: status: '%s'",
-		  __get_str(memdev), __get_str(host), __entry->serial,
+	TP_printk("memdev=%s port=%s dport=%s host=%s serial=%lld: status: '%s'",
+		  __get_str(memdev), __get_str(port), __get_str(dport),
+		  __get_str(host), __entry->serial,
 		  show_ce_errs(__entry->status)
 	)
 );

drivers/cxl/cxlmem.h +7 -0

index ed419d0c59f2f..f1ef8b78db18a 100644
@@ -125,6 +125,13 @@ static inline int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
 #endif
 
 struct cxl_memdev *devm_cxl_add_classdev(struct cxl_dev_state *cxlds);
+
+/* trace-event helpers */
+const char *cxl_trace_memdev_name(struct cxl_port *port);
+const char *cxl_trace_host_name(struct cxl_port *port);
+const char *cxl_trace_port_name(struct cxl_port *port);
+const char *cxl_trace_dport_name(struct cxl_dport *dport);
+
 struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 					 const struct cxl_memdev_attach *attach);
 int devm_cxl_sanitize_setup_notifier(struct device *host,

Signature · 1 line

2.34.1

Bowman, Terry

author

2026-08-04 13:46 UTC #43 lore raw

Re: [PATCH v19 12/14] cxl: Add port and dport identifiers to CXL AER trace events

Quoted 332 lines · Richard Cheng

Quoted 299 lines · Terry Bowman
From: Dan Williams <[email protected]>
Pass struct cxl_port * and struct cxl_dport * to the cxl_aer_* trace events instead of a plain struct device * derived at the caller. The trace event helpers then derive the right strings for Endpoints, Switch Ports, Root Ports, and RCH Downstream Ports consistently across the CPER and native AER paths.
The unified cxl_aer_* events keep "memdev" as the legacy field (endpoint events populate it with the memdev name; non-endpoint events emit memdev="") and add new "port" and "dport" string fields populated for all CXL device classes. Updated userspace can key off "port" and "dport" without a parallel set of events.
Remove the separate cxl_port_aer_uncorrectable_error and cxl_port_aer_correctable_error trace events. All CXL AER events now use the unified cxl_aer_* events with port and dport fields.
Rework cxl_cper_handle_prot_err() to use find_cxl_port_by_dev() and the unified trace helpers, replacing the per-port-type branching and bus_find_device() memdev lookup.
The TP_printk format string places "port=%s dport=%s" between "memdev=%s" and "host=%s", changing the text-mode field order from the pre-patch output. This does not affect consumers such as rasdaemon that use libtraceevent to parse fields by name rather than by fixed text position.
For non-Endpoint events (Switch Port, Root Port, RCH Dport), "memdev" is empty and "port"/"dport" carry the topology information.
Below are examples of the different CXL devices' error trace logs after this patch:
     ---------------------
     | CXL RP - 0C:00.0  |
     ---------------------
               |
     ---------------------
     | CXL USP - 0D:00.0 |
     ---------------------
               |
     --------------------
     | CXL DSP - 0E:00.0 |
     --------------------
               |
     ---------------------
     | CXL EP - 0F:00.0  |
     ---------------------
Root Port:
cxl_aer_correctable_error: memdev= port=port1 dport=0000:0c:00.0 \
host=pci0000:0c serial=0: status: 'Memory Data ECC Error'
cxl_aer_uncorrectable_error: memdev= port=port1 dport=0000:0c:00.0 \
host=pci0000:0c serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'
Upstream Switch Port:
cxl_aer_correctable_error: memdev= port=port2 dport= host=0000:0d:00.0 \
serial=0: status: 'Memory Data ECC Error'
UCE NA - Upstream Switch Port UCE's are handled in the portdrv driver's PCI AER callbacks that are not CXL aware.
Downstream Switch Port:
cxl_aer_correctable_error: memdev= port=port2 dport=0000:0e:00.0 \
host=0000:0d:00.0 serial=0: status: 'Memory Data ECC Error'
cxl_aer_uncorrectable_error: memdev= port=port2 dport=0000:0e:00.0 \
host=0000:0d:00.0 serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'
Endpoint:
cxl_aer_uncorrectable_error: memdev=mem1 port=endpoint4 dport= \
host=0000:0f:00.0 serial=0: status: 'Cache Address Parity Error' \
first_error: 'Cache Address Parity Error'
cxl_aer_correctable_error: memdev=mem1 port=endpoint4 dport= host=0000:0f:00.0 \
serial=0: status: 'Memory Data ECC Error'
Co-developed-by Terry Bowman <[email protected]>
Signed-off-by Terry Bowman <[email protected]>
Signed-off-by Dan Williams <[email protected]>
Reviewed-by Dave Jiang <[email protected]>
---
Changes in v18->v19:
- Drop redundant device lock in cxl_cper_handle_prot_err(); the port reference already keeps the object alive and no RAS iomap is accessed.
- Swap order in series with ("PCI: Cache PCI DSN into pci_dev->dsn during probe")
- Add review-by for DaveJ
Changes in v17->v18:
- Consolidate double find_cxl_port_by_dev() in cxl_cper_handle_prot_err()
- Add comment noting dport is NULL for Endpoint and Upstream Port devices
- Add cxl_trace_* helpers
- Add CPER refactor
Changes in v16->v17:
- Replace cxlds->serial with pci_get_dsn()
- Change 'memdev' to 'device' (Dan)
- Updated Commit message
Changes in v15->v16:
- Add Dan's review-by
- Incorporate Dan's comment into commit message:
"Add the serial number at the end to preserve compatibility with libtraceevent parsing of the parameters."
Changes in v14->v15:
- Update commit message.
- Moved cxl_handle_ras/cxl_handle_cor_ras() changes to future patch (terry)
Changes in v13->v14:
- Update commit headline (Bjorn)
Changes in v12->v13:
- Added Dave Jiang's review-by
Changes in v11 -> v12:
- Correct parameters to call trace_cxl_aer_correctable_error()
- Add reviewed-by for Jonathan and Shiju
Changes in v10->v11:
- Updated CE and UCE trace routines to maintain consistent TP_Struct ABI and unchanged TP_printk() logging.
---
 drivers/cxl/core/core.h    |   8 +--
 drivers/cxl/core/ras.c     | 129 +++++++++++--------------------------
 drivers/cxl/core/ras_rch.c |   3 +-
 drivers/cxl/core/trace.c   |  35 ++++++++++
 drivers/cxl/core/trace.h   |  91 ++++++++------------------
 drivers/cxl/cxlmem.h       |   7 ++
 6 files changed, 113 insertions(+), 160 deletions(-)
drivers/cxl/core/core.h +4 -4
index 5ca1275fd8f35..a55a4e409feda 100644
@@ -186,11 +186,11 @@ static inline struct device *dport_to_host(struct cxl_dport *dport)
 void cxl_ras_init(void);
 void cxl_ras_exit(void);
 bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
-		    void __iomem *ras_base);
+		    void __iomem *ras_base, u64 serial);
 void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 		     struct cxl_dport *dport);
 void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
-			void __iomem *ras_base);
+			void __iomem *ras_base, u64 serial);
 void cxl_dport_map_rch_aer(struct cxl_dport *dport);
 void cxl_disable_rch_root_ints(struct cxl_dport *dport);
 void cxl_handle_rdport_errors(struct pci_dev *pdev);
@@ -200,14 +200,14 @@ void devm_cxl_dport_ras_setup(struct cxl_dport *dport);
 static inline void cxl_ras_init(void) { }
 static inline void cxl_ras_exit(void) { }
 static inline bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
-				  void __iomem *ras_base)
+				  void __iomem *ras_base, u64 serial)
 {
 	return false;
 }
 static inline void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport) { }
 static inline void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
-				      void __iomem *ras_base) { }
+				      void __iomem *ras_base, u64 serial) { }
 static inline void cxl_dport_map_rch_aer(struct cxl_dport *dport) { }
 static inline void cxl_disable_rch_root_ints(struct cxl_dport *dport) { }
 static inline void cxl_handle_rdport_errors(struct pci_dev *pdev) { }
drivers/cxl/core/ras.c +22 ~~-66~~
index 5183b3c532952..0254b7ec64c30 100644
@@ -12,69 +12,37 @@
 static_assert(CXL_HEADERLOG_TRACE_SIZE_U32 == 128,
 	      "rasdaemon ABI requires exactly 128 u32s");
 
-static void cxl_cper_trace_corr_port_prot_err(struct pci_dev *pdev,
-					      struct cxl_ras_capability_regs ras_cap)
-{
-	u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
-
-	trace_cxl_port_aer_correctable_error(&pdev->dev, status);
-}
-
-static void cxl_cper_trace_uncorr_port_prot_err(struct pci_dev *pdev,
-						struct cxl_ras_capability_regs ras_cap)
+static void cxl_cper_trace_uncorr_prot_err(struct cxl_port *port, struct cxl_dport *dport,
+					   u64 serial, struct cxl_ras_capability_regs *ras_cap)
 {
 	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
-	u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
+	u32 status = ras_cap->uncor_status & ~ras_cap->uncor_mask;
 	u32 fe;
 
 	if (hweight32(status) > 1)
 		fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
-				   ras_cap.cap_control));
-	else
-		fe = status;
-
-	memcpy(hl, ras_cap.header_log, CXL_HEADERLOG_SIZE);
-	trace_cxl_port_aer_uncorrectable_error(&pdev->dev, status, fe, hl);
-}
-
-static void cxl_cper_trace_corr_prot_err(struct cxl_memdev *cxlmd,
-					 struct cxl_ras_capability_regs ras_cap)
-{
-	u32 status = ras_cap.cor_status & ~ras_cap.cor_mask;
-
-	trace_cxl_aer_correctable_error(cxlmd, status);
-}
-
-static void
-cxl_cper_trace_uncorr_prot_err(struct cxl_memdev *cxlmd,
-			       struct cxl_ras_capability_regs ras_cap)
-{
-	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
-	u32 status = ras_cap.uncor_status & ~ras_cap.uncor_mask;
-	u32 fe;
-
-	if (hweight32(status) > 1)
-		fe = BIT(FIELD_GET(CXL_RAS_CAP_CONTROL_FE_MASK,
-				   ras_cap.cap_control));
+				   ras_cap->cap_control));
 	else
 		fe = status;
 
 	/*
-	 * ras_cap.header_log[] holds CXL_HEADERLOG_SIZE_U32 (16) hardware
+	 * ras_cap->header_log[] holds CXL_HEADERLOG_SIZE_U32 (16) hardware
 	 * dwords. Copy them into the front of a zero-filled
 	 * CXL_HEADERLOG_TRACE_SIZE_U32 (128) u32 staging buffer so the trace
 	 * event memcpy sees a full 512-byte source and the userspace ABI
 	 * (rasdaemon) is preserved.
 	 */
-	memcpy(hl, ras_cap.header_log, CXL_HEADERLOG_SIZE);
-	trace_cxl_aer_uncorrectable_error(cxlmd, status, fe, hl);
+	memcpy(hl, ras_cap->header_log, CXL_HEADERLOG_SIZE);
+	trace_cxl_aer_uncorrectable_error(port, dport, status, fe,
+					  hl, serial);
 }
 
-static int match_memdev_by_parent(struct device *dev, const void *uport)
+static void cxl_cper_trace_corr_prot_err(struct cxl_port *port, struct cxl_dport *dport,
+					 u64 serial, struct cxl_ras_capability_regs *ras_cap)
 {
-	if (is_cxl_memdev(dev) && dev->parent == uport)
-		return 1;
-	return 0;
+	u32 status = ras_cap->cor_status & ~ras_cap->cor_mask;
+
+	trace_cxl_aer_correctable_error(port, dport, status, serial);
 }
 
 /**
@@ -108,44 +76,32 @@ static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor
 
 void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
 {
+	struct cxl_dport *dport;
 	unsigned int devfn = PCI_DEVFN(data->prot_err.agent_addr.device,
 				       data->prot_err.agent_addr.function);
-	struct pci_dev *pdev __free(pci_dev_put) =
-		pci_get_domain_bus_and_slot(data->prot_err.agent_addr.segment,
-					    data->prot_err.agent_addr.bus,
-					    devfn);
-	struct cxl_memdev *cxlmd;
-	int port_type;
-
-	if (!pdev)
-		return;
-
-	port_type = pci_pcie_type(pdev);
-	if (port_type == PCI_EXP_TYPE_ROOT_PORT ||
-	    port_type == PCI_EXP_TYPE_DOWNSTREAM ||
-	    port_type == PCI_EXP_TYPE_UPSTREAM) {
-		if (data->severity == AER_CORRECTABLE)
-			cxl_cper_trace_corr_port_prot_err(pdev, data->ras_cap);
-		else
-			cxl_cper_trace_uncorr_port_prot_err(pdev, data->ras_cap);
-
+	struct pci_dev *pdev __free(pci_dev_put) = pci_get_domain_bus_and_slot(
+		data->prot_err.agent_addr.segment, data->prot_err.agent_addr.bus, devfn);
+	if (!pdev) {
+		pr_err_ratelimited("Failed to find CPER device in CXL topology\n");
 		return;
 	}
 
-	guard(device)(&pdev->dev);
-	if (!pdev->dev.driver)
+	struct cxl_port *port __free(put_cxl_port) = find_cxl_port_by_dev(&pdev->dev, NULL);
+	if (!port) {
+		dev_err_ratelimited(&pdev->dev,
+				    "Failed to find parent port device in CXL topology\n");
drivers/cxl/core/ras.c static struct cxl_port *find_cxl_port_by_dev(struct device *dev, struct cxl_dpor · line 98
Quoted 10 lines
drivers/cxl/core/ras.c +3 -4
 		return;
+	}
 
-	struct device *mem_dev __free(put_device) = bus_find_device(
-		&cxl_bus_type, NULL, pdev, match_memdev_by_parent);
-	if (!mem_dev)
-		return;
+	/* dport is NULL for Endpoint and Upstream Port devices */
+	dport = cxl_find_dport_by_dev(port, &pdev->dev);
 
Hi Terry,
I have a question here.
Do we need the port device lock here ?
cxl_find_dport_by_dev() is xa_load(), and the free side is serialized by that lock. del_dports() has device_lock_assert(&port->dev) and frees the dport which in the end resolves to kfree().
__free(put_cxl_port) doesn't cover it, that's a kobject ref on &port->dev, so it pins the struct cxl_port but not the dports.
I think cxl_cper_handle_prot_err() might race cxl_detach_ep() calling del_dports(), and cxl_trace_dport_name() then does dev_name(dport->dport_dev) on freed memory.
I see __cxl_proto_err_work_fn(), cxl_handle_rdport_errors() and cxl_pci_error_detected() all take the guard first, should we do the same here?

Quoted 8 lines

Though I didn't poke this issue out in runtime, I guess it needs FW-first, cxl_aer_* enabled, and a concurrent teardown. Not an expert of FW, let me knowo if something already rules it out.
Best regards,
Richard Cheng.

Hi Richard,

Yes, a port lock is needed for preventing dport being freed here. This could be a use after free in the path's trace dev_name(). Thanks for pointing out. I'll fix this in v20 with adding: guard(device)(&port->dev)

-Terry

Quoted 305 lines

drivers/cxl/core/ras.c +14 ~~-20~~

-	cxlmd = to_cxl_memdev(mem_dev);
 	if (data->severity == AER_CORRECTABLE)
-		cxl_cper_trace_corr_prot_err(cxlmd, data->ras_cap);
+		cxl_cper_trace_corr_prot_err(port, dport, pdev->dsn,
+					     &data->ras_cap);
 	else
-		cxl_cper_trace_uncorr_prot_err(cxlmd, data->ras_cap);
+		cxl_cper_trace_uncorr_prot_err(port, dport, pdev->dsn,
+					       &data->ras_cap);
 }
 EXPORT_SYMBOL_GPL(cxl_cper_handle_prot_err);

@@ -232,14 +188,15 @@ void cxl_do_recovery(struct pci_dev *pdev, struct cxl_port *port, struct cxl_dpo
 	if (!ras_base)
 		panic("CXL: UCE with unmapped RAS registers");
 
-	if (cxl_handle_ras(port, dport, ras_base))
+	if (cxl_handle_ras(port, dport, ras_base, pdev->dsn))
 		panic("CXL cachemem error");
 
 	dev_dbg(&pdev->dev,
 		"CXL UCE signaled but no CXL RAS status bits set\n");
 }
 
-void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
+void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport,
+			void __iomem *ras_base, u64 serial)
 {
 	void __iomem *addr;
 	u32 status;

@@ -251,12 +208,7 @@ void cxl_handle_cor_ras(struct cxl_port *port, struct cxl_dport *dport, void __i
 	status = readl(addr);
 	if (status & CXL_RAS_CORRECTABLE_STATUS_MASK) {
 		writel(status & CXL_RAS_CORRECTABLE_STATUS_MASK, addr);
-		if (is_cxl_endpoint(port))
-			trace_cxl_aer_correctable_error(to_cxl_memdev(port->uport_dev), status);
-		else if (dport)
-			trace_cxl_port_aer_correctable_error(dport->dport_dev, status);
-		else
-			trace_cxl_port_aer_correctable_error(port->uport_dev, status);
+		trace_cxl_aer_correctable_error(port, dport, status, serial);
 	}
 }

@@ -281,7 +233,8 @@ static void header_log_copy(void __iomem *ras_base, u32 *log)
  * Log the state of the RAS status registers and prepare them to log the
  * next error status. Return 1 if reset needed.
  */
-bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem *ras_base)
+bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport,
+		    void __iomem *ras_base, u64 serial)
 {
 	u32 hl[CXL_HEADERLOG_TRACE_SIZE_U32] = {};
 	void __iomem *addr;

@@ -308,12 +261,7 @@ bool cxl_handle_ras(struct cxl_port *port, struct cxl_dport *dport, void __iomem
 	}
 
 	header_log_copy(ras_base, hl);
-	if (is_cxl_endpoint(port))
-		trace_cxl_aer_uncorrectable_error(to_cxl_memdev(port->uport_dev), status, fe, hl);
-	else if (dport)
-		trace_cxl_port_aer_uncorrectable_error(dport->dport_dev, status, fe, hl);
-	else
-		trace_cxl_port_aer_uncorrectable_error(port->uport_dev, status, fe, hl);
+	trace_cxl_aer_uncorrectable_error(port, dport, status, fe, hl, serial);
 
 	writel(status & CXL_RAS_UNCORRECTABLE_STATUS_MASK, addr);

@@ -354,7 +302,8 @@ pci_ers_result_t cxl_pci_error_detected(struct pci_dev *pdev,
 		 * cases below handle AER recovery for devices without active
 		 * CXL.mem traffic.
 		 */
-		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL));
+		ue = cxl_handle_ras(port, NULL, to_ras_base(port, NULL),
+				    pdev->dsn);
 	}
 
 	/*

@@ -386,7 +335,7 @@ static void cxl_handle_proto_error(struct pci_dev *pdev, struct cxl_port *port,
 				   struct cxl_dport *dport, int severity)
 {
 	if (severity == AER_CORRECTABLE)
-		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport), pdev->dsn);
 	else
 		cxl_do_recovery(pdev, port, dport);
 }

drivers/cxl/core/ras_rch.c +2 -1

index a5c62c71060d9..a371174536a8b 100644
@@ -113,7 +113,8 @@ void cxl_handle_rdport_errors(struct pci_dev *pdev)
 	 */
 	if (aer_regs.cor_status & ~aer_regs.cor_mask) {
 		pci_print_aer(pdev, AER_CORRECTABLE, &aer_regs);
-		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport));
+		cxl_handle_cor_ras(port, dport, to_ras_base(port, dport),
+			pdev->dsn);
 	}
 
 	if (aer_regs.uncor_status & ~aer_regs.uncor_mask) {

drivers/cxl/core/trace.c +35 -0

index 7f2a9dd0d0e3f..df42d119c53dd 100644
@@ -2,7 +2,42 @@
 /* Copyright(c) 2022 Intel Corporation. All rights reserved. */
 
 #include <cxl.h>
+#include <cxlmem.h>
 #include "core.h"
 
+const char *cxl_trace_memdev_name(struct cxl_port *port)
+{
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+		return dev_name(&cxlmd->dev);
+	}
+
+	return "";
+}
+
+const char *cxl_trace_host_name(struct cxl_port *port)
+{
+	if (is_cxl_endpoint(port)) {
+		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
+
+		return dev_name(cxlmd->dev.parent);
+	}
+
+	return dev_name(port->uport_dev);
+}
+
+const char *cxl_trace_port_name(struct cxl_port *port)
+{
+	return dev_name(&port->dev);
+}
+
+const char *cxl_trace_dport_name(struct cxl_dport *dport)
+{
+	if (dport)
+		return dev_name(dport->dport_dev);
+	return "";
+}
+
 #define CREATE_TRACE_POINTS
 #include "trace.h"

drivers/cxl/core/trace.h +26 ~~-65~~

index d37876096dd7c..910aceb2ca3ab 100644
@@ -48,44 +48,15 @@
 	{ CXL_RAS_UC_IDE_RX_ERR, "IDE Rx Error" }			  \
 )
 
-TRACE_EVENT(cxl_port_aer_uncorrectable_error,
-	TP_PROTO(struct device *dev, u32 status, u32 fe, u32 *hl),
-	TP_ARGS(dev, status, fe, hl),
-	TP_STRUCT__entry(
-		__string(device, dev_name(dev))
-		__string(host, dev_name(dev->parent))
-		__field(u32, status)
-		__field(u32, first_error)
-		__array(u32, header_log, CXL_HEADERLOG_TRACE_SIZE_U32)
-	),
-	TP_fast_assign(
-		__assign_str(device);
-		__assign_str(host);
-		__entry->status = status;
-		__entry->first_error = fe;
-		/*
-		 * Embed headerlog data for user app retrieval and parsing,
-		 * but no need to print in the trace buffer. Only
-		 * CXL_HEADERLOG_SIZE_U32 (16) dwords are hardware data;
-		 * the remaining entries preserve the 512-byte ABI layout
-		 * rasdaemon depends on and are zero-filled by the caller.
-		 */
-		memcpy(__entry->header_log, hl,
-			CXL_HEADERLOG_TRACE_SIZE_U32 * sizeof(u32));
-	),
-	TP_printk("device=%s host=%s status: '%s' first_error: '%s'",
-		  __get_str(device), __get_str(host),
-		  show_uc_errs(__entry->status),
-		  show_uc_errs(__entry->first_error)
-	)
-);
-
 TRACE_EVENT(cxl_aer_uncorrectable_error,
-	TP_PROTO(const struct cxl_memdev *cxlmd, u32 status, u32 fe, u32 *hl),
-	TP_ARGS(cxlmd, status, fe, hl),
+	TP_PROTO(struct cxl_port *port, struct cxl_dport *dport,
+		 u32 status, u32 fe, u32 *hl, u64 serial),
+	TP_ARGS(port, dport, status, fe, hl, serial),
 	TP_STRUCT__entry(
-		__string(memdev, dev_name(&cxlmd->dev))
-		__string(host, dev_name(cxlmd->dev.parent))
+		__string(memdev, cxl_trace_memdev_name(port))
+		__string(port, cxl_trace_port_name(port))
+		__string(dport, cxl_trace_dport_name(dport))
+		__string(host, cxl_trace_host_name(port))
 		__field(u64, serial)
 		__field(u32, status)
 		__field(u32, first_error)

@@ -93,8 +64,10 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
 	),
 	TP_fast_assign(
 		__assign_str(memdev);
+		__assign_str(port);
+		__assign_str(dport);
 		__assign_str(host);
-		__entry->serial = cxlmd->cxlds->serial;
+		__entry->serial = serial;
 		__entry->status = status;
 		__entry->first_error = fe;
 		/*

@@ -107,8 +80,9 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
 		memcpy(__entry->header_log, hl,
 			CXL_HEADERLOG_TRACE_SIZE_U32 * sizeof(u32));
 	),
-	TP_printk("memdev=%s host=%s serial=%lld: status: '%s' first_error: '%s'",
-		  __get_str(memdev), __get_str(host), __entry->serial,
+	TP_printk("memdev=%s port=%s dport=%s host=%s serial=%lld: status: '%s' first_error: '%s'",
+		  __get_str(memdev), __get_str(port), __get_str(dport),
+		  __get_str(host), __entry->serial,
 		  show_uc_errs(__entry->status),
 		  show_uc_errs(__entry->first_error)
 	)

@@ -132,42 +106,29 @@ TRACE_EVENT(cxl_aer_uncorrectable_error,
 	{ CXL_RAS_CE_PHYS_LAYER_ERR, "Received Error From Physical Layer" }	\
 )
 
-TRACE_EVENT(cxl_port_aer_correctable_error,
-	TP_PROTO(struct device *dev, u32 status),
-	TP_ARGS(dev, status),
-	TP_STRUCT__entry(
-		__string(device, dev_name(dev))
-		__string(host, dev_name(dev->parent))
-		__field(u32, status)
-	),
-	TP_fast_assign(
-		__assign_str(device);
-		__assign_str(host);
-		__entry->status = status;
-	),
-	TP_printk("device=%s host=%s status='%s'",
-		  __get_str(device), __get_str(host),
-		  show_ce_errs(__entry->status)
-	)
-);
-
 TRACE_EVENT(cxl_aer_correctable_error,
-	TP_PROTO(const struct cxl_memdev *cxlmd, u32 status),
-	TP_ARGS(cxlmd, status),
+	TP_PROTO(struct cxl_port *port, struct cxl_dport *dport,
+		 u32 status, u64 serial),
+	TP_ARGS(port, dport, status, serial),
 	TP_STRUCT__entry(
-		__string(memdev, dev_name(&cxlmd->dev))
-		__string(host, dev_name(cxlmd->dev.parent))
+		__string(memdev, cxl_trace_memdev_name(port))
+		__string(port, cxl_trace_port_name(port))
+		__string(dport, cxl_trace_dport_name(dport))
+		__string(host, cxl_trace_host_name(port))
 		__field(u64, serial)
 		__field(u32, status)
 	),
 	TP_fast_assign(
 		__assign_str(memdev);
+		__assign_str(port);
+		__assign_str(dport);
 		__assign_str(host);
-		__entry->serial = cxlmd->cxlds->serial;
+		__entry->serial = serial;
 		__entry->status = status;
 	),
-	TP_printk("memdev=%s host=%s serial=%lld: status: '%s'",
-		  __get_str(memdev), __get_str(host), __entry->serial,
+	TP_printk("memdev=%s port=%s dport=%s host=%s serial=%lld: status: '%s'",
+		  __get_str(memdev), __get_str(port), __get_str(dport),
+		  __get_str(host), __entry->serial,
 		  show_ce_errs(__entry->status)
 	)
 );

drivers/cxl/cxlmem.h +7 -0

index ed419d0c59f2f..f1ef8b78db18a 100644
@@ -125,6 +125,13 @@ static inline int cxl_memdev_attach_region(struct cxl_memdev *cxlmd)
 #endif
 
 struct cxl_memdev *devm_cxl_add_classdev(struct cxl_dev_state *cxlds);
+
+/* trace-event helpers */
+const char *cxl_trace_memdev_name(struct cxl_port *port);
+const char *cxl_trace_host_name(struct cxl_port *port);
+const char *cxl_trace_port_name(struct cxl_port *port);
+const char *cxl_trace_dport_name(struct cxl_dport *dport);
+
 struct cxl_memdev *__devm_cxl_add_memdev(struct cxl_dev_state *cxlds,
 					 const struct cxl_memdev_attach *attach);
 int devm_cxl_sanitize_setup_notifier(struct device *host,

Signature · 1 line

2.34.1

Terry Bowman

author

2026-08-03 22:20 UTC #44 lore raw

PATCH v19 13/14 PCI/CXL: Mask/Unmask CXL protocol errors

CXL protocol errors must be unmasked to be reported. Add pci_aer_mask_internal_errors() as the symmetric counterpart to pci_aer_unmask_internal_errors() and export both for cxl_core.

Unmask CXL internal errors in cxl_dport_map_ras() and devm_cxl_port_ras_setup() after the RAS register block is successfully mapped. Register a devm action to restore the mask on teardown. The unmask/mask helpers gate on dev_is_pci() and pcie_aer_is_native() internally so callers need no special-casing.

Remove the dev_is_pci(dport->dport_dev) guard in devm_cxl_dport_rch_ras_setup(). On RCH systems dport->dport_dev is the pci_host_bridge device which is not on pci_bus_type, so this guard blocked real hardware. The caller already gates on dport->rch.

Co-developed-by Dan Williams <[email protected]>

---

Changes in v18 -> v19:
- Reworded commit message to be more concise.
- Use pci_clear_and_set_config_dword() in pci_aer_mask_internal_errors()
- Add review-by for DaveJ and Jonathan

Changes in v17->v18:
- Make cxl_unmask_proto_interrupts() and cxl_mask_proto_interrupts() static
- Remove dev_is_pci() guard from devm_cxl_dport_rch_ras_setup(); the guard blocked real RCH hardware because pci_host_bridge is not on pci_bus_type

Changes in v16->v17:
- Drop redundant cxl_mask_proto_interrupts() calls from unregister_port() and cxl_dport_remove(); the devres action registered alongside the unmask is the sole mask path.
- Update title
- Remove unnecessary check for aer_capabilities
- Gate cxl_unmask_proto_interrupts() on pcie_aer_is_native()
- Add pci_aer_mask_internal_errors() and cxl_mask_proto_interrupts()
- Only unmask on successful cxl_map_component_regs()
- NULL-check @dev in cxl_{un,}mask_proto_interrupts()
- Drop static and declare in core/core.h

Change in v15 -> v16:
- None

Change in v14 -> v15:
- None

Changes in v13->v14:
- Update commit title's prefix (Bjorn)

Changes in v12->v13:
- Add dev and dev_is_pci() NULL checks in cxl_unmask_proto_interrupts() (Terry)
- Add Dave Jiang's and Ben's review-by

Changes in v11->v12:
- None

---
 drivers/cxl/core/ras.c        | 73 +++++++++++++++++++++++++++++++----
 drivers/pci/pcie/aer.c        | 22 +++++++++--
 include/linux/aer.h           |  2 +
 tools/testing/cxl/Kbuild      |  1 +
 tools/testing/cxl/test/mock.c | 12 ++++++
 5 files changed, 99 insertions(+), 11 deletions(-)

drivers/cxl/core/ras.c +66 -7

index 0254b7ec64c30..40a8a03070652 100644
@@ -114,16 +114,64 @@ static void cxl_cper_prot_err_work_fn(struct work_struct *work)
 }
 static DECLARE_WORK(cxl_cper_prot_err_work, cxl_cper_prot_err_work_fn);
 
+static void cxl_unmask_proto_interrupts(struct device *dev)
+{
+	struct pci_dev *pdev;
+
+	if (!dev || !dev_is_pci(dev))
+		return;
+
+	pdev = to_pci_dev(dev);
+	if (!pcie_aer_is_native(pdev))
+		return;
+
+	pci_aer_unmask_internal_errors(pdev);
+}
+
+static void cxl_mask_proto_interrupts(struct device *dev)
+{
+	struct pci_dev *pdev;
+
+	if (!dev || !dev_is_pci(dev))
+		return;
+
+	pdev = to_pci_dev(dev);
+	if (!pcie_aer_is_native(pdev))
+		return;
+
+	pci_aer_mask_internal_errors(pdev);
+}
+
+static void cxl_mask_proto_irqs(void *dev)
+{
+	cxl_mask_proto_interrupts(dev);
+}
+
 static void cxl_dport_map_ras(struct cxl_dport *dport)
 {
 	struct cxl_register_map *map = &dport->reg_map;
 	struct device *dev = dport->dport_dev;
 
-	if (!map->component_map.ras.valid)
+	if (!map->component_map.ras.valid) {
 		dev_dbg(dev, "RAS registers not found\n");
-	else if (cxl_map_component_regs(map, &dport->regs.component,
-					BIT(CXL_CM_CAP_CAP_ID_RAS)))
+		return;
+	}
+
+	if (cxl_map_component_regs(map, &dport->regs.component,
+				   BIT(CXL_CM_CAP_CAP_ID_RAS))) {
 		dev_dbg(dev, "Failed to map RAS capability.\n");
+		return;
+	}
+
+	if (!dev_is_pci(dev))
+		return;
+
+	cxl_unmask_proto_interrupts(dev);
+	if (devm_add_action_or_reset(dport_to_host(dport),
+				     cxl_mask_proto_irqs, dev)) {
+		dev_warn(dev, "failed to defer CXL proto-irq mask; CXL protocol error reporting disabled\n");
+		dport->regs.component.ras = NULL;
+	}
 }
 
 /**

@@ -140,9 +188,6 @@ void devm_cxl_dport_rch_ras_setup(struct cxl_dport *dport)
 {
 	struct pci_host_bridge *host_bridge;
 
-	if (!dev_is_pci(dport->dport_dev))
-		return;
-
 	devm_cxl_dport_ras_setup(dport);
 
 	host_bridge = to_pci_host_bridge(dport->dport_dev);

@@ -157,6 +202,7 @@ EXPORT_SYMBOL_NS_GPL(devm_cxl_dport_rch_ras_setup, "CXL");
 void devm_cxl_port_ras_setup(struct cxl_port *port)
 {
 	struct cxl_register_map *map = &port->reg_map;
+	struct device *dev;
 
 	if (!map->component_map.ras.valid) {
 		dev_dbg(&port->dev, "RAS registers not found\n");

@@ -165,8 +211,21 @@ void devm_cxl_port_ras_setup(struct cxl_port *port)
 
 	map->host = &port->dev;
 	if (cxl_map_component_regs(map, &port->regs,
-				   BIT(CXL_CM_CAP_CAP_ID_RAS)))
+				   BIT(CXL_CM_CAP_CAP_ID_RAS))) {
 		dev_dbg(&port->dev, "Failed to map RAS capability\n");
+		return;
+	}
+
+	dev = is_cxl_endpoint(port) ? port->uport_dev->parent : port->uport_dev;
+	if (!dev_is_pci(dev))
+		return;
+
+	cxl_unmask_proto_interrupts(dev);
+	if (devm_add_action_or_reset(&port->dev, cxl_mask_proto_irqs, dev)) {
+		dev_warn(&port->dev,
+			 "failed to defer CXL proto-irq mask; CXL protocol error reporting disabled\n");
+		port->regs.ras = NULL;
+	}
 }
 EXPORT_SYMBOL_NS_GPL(devm_cxl_port_ras_setup, "CXL");

drivers/pci/pcie/aer.c +18 -4

index 0155f3923c1fd..d461f281e5372 100644
@@ -1143,12 +1143,26 @@ void pci_aer_unmask_internal_errors(struct pci_dev *dev)
 	mask &= ~PCI_ERR_COR_INTERNAL;
 	pci_write_config_dword(dev, aer + PCI_ERR_COR_MASK, mask);
 }
+EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
 
-/*
- * Internal errors are too device-specific to enable generally, however for CXL
- * their behavior is standardized for conveying CXL protocol errors.
+/**
+ * pci_aer_mask_internal_errors - mask internal errors
+ * @dev: pointer to the pci_dev data structure
+ *
+ * Mask internal errors in the Uncorrectable and Correctable Error
+ * Mask registers.
+ *
+ * Note: AER must be enabled and supported by the device which must be
+ * checked in advance, e.g. with pcie_aer_is_native().
  */
-EXPORT_SYMBOL_FOR_MODULES(pci_aer_unmask_internal_errors, "cxl_core");
+void pci_aer_mask_internal_errors(struct pci_dev *dev)
+{
+	pci_clear_and_set_config_dword(dev, dev->aer_cap + PCI_ERR_UNCOR_MASK,
+				       0, PCI_ERR_UNC_INTN);
+	pci_clear_and_set_config_dword(dev, dev->aer_cap + PCI_ERR_COR_MASK,
+				       0, PCI_ERR_COR_INTERNAL);
+}
+EXPORT_SYMBOL_FOR_MODULES(pci_aer_mask_internal_errors, "cxl_core");
 
 /**
  * pci_aer_handle_error - handle logging error into an event log

include/linux/aer.h +2 -0

index 8eba3192e2d15..b3657b80564b9 100644
@@ -58,6 +58,7 @@ struct aer_capability_regs {
 int pci_aer_clear_nonfatal_status(struct pci_dev *dev);
 int pcie_aer_is_native(struct pci_dev *dev);
 void pci_aer_unmask_internal_errors(struct pci_dev *dev);
+void pci_aer_mask_internal_errors(struct pci_dev *dev);
 #else
 static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
 {

@@ -65,6 +66,7 @@ static inline int pci_aer_clear_nonfatal_status(struct pci_dev *dev)
 }
 static inline int pcie_aer_is_native(struct pci_dev *dev) { return 0; }
 static inline void pci_aer_unmask_internal_errors(struct pci_dev *dev) { }
+static inline void pci_aer_mask_internal_errors(struct pci_dev *dev) { }
 #endif
 
 #ifdef CONFIG_CXL_RAS

tools/testing/cxl/Kbuild +1 -0

index 2be1df80fcc93..957945201f04d 100644
@@ -6,6 +6,7 @@ ldflags-y += --wrap=acpi_pci_find_root
 ldflags-y += --wrap=nvdimm_bus_register
 ldflags-y += --wrap=cxl_await_media_ready
 ldflags-y += --wrap=devm_cxl_add_rch_dport
+ldflags-y += --wrap=devm_cxl_dport_rch_ras_setup
 ldflags-y += --wrap=cxl_endpoint_parse_cdat
 ldflags-y += --wrap=devm_cxl_endpoint_decoders_setup
 ldflags-y += --wrap=hmat_get_extended_linear_cache_size

tools/testing/cxl/test/mock.c +12 -0

index 6454b868b122c..5ad3243da8d29 100644
@@ -220,6 +220,18 @@ struct cxl_dport *__wrap_devm_cxl_add_rch_dport(struct cxl_port *port,
 }
 EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_add_rch_dport, "CXL");
 
+void __wrap_devm_cxl_dport_rch_ras_setup(struct cxl_dport *dport)
+{
+	int index;
+	struct cxl_mock_ops *ops = get_cxl_mock_ops(&index);
+
+	if (!ops || !ops->is_mock_port(dport->dport_dev))
+		devm_cxl_dport_rch_ras_setup(dport);
+
+	put_cxl_mock_ops(index);
+}
+EXPORT_SYMBOL_NS_GPL(__wrap_devm_cxl_dport_rch_ras_setup, "CXL");
+
 void __wrap_cxl_endpoint_parse_cdat(struct cxl_port *port)
 {
 	int index;

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:55 UTC #45 lore raw

Re: [PATCH v19 13/14] PCI/CXL: Mask/Unmask CXL protocol errors

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Setting `ras = NULL` on devm_add_action_or_reset failure causes a deterministic kernel panic on…

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Setting ras = NULL on devm_add_action_or_reset failure causes a deterministic kernel panic on subsequent uncorrectable errors.
- [High] Root port dport->regs.ras is exposed to concurrent error handlers before it is mapped, leading to a kernel panic. --

--- Patch [13]: [PATCH v19 13/14] PCI/CXL: Mask/Unmask CXL protocol errors --- commit 882744bb847d3cd61f8f100025ea14b41fb68044
Author: Terry Bowman <[email protected]>

PCI/CXL: Mask/Unmask CXL protocol errors

CXL protocol errors must be unmasked to be reported. Add pci_aer_mask_internal_errors() and pci_aer_unmask_internal_errors() helpers to mask and unmask internal errors, and register a devm action to restore the mask on teardown.

Quoted 5 lines

drivers/cxl/core/ras.c +0 -0

index 0254b7ec64c30..40a8a03070652 100644
@@ -114,16 +114,64 @@ static void cxl_cper_prot_err_work_fn(struct work_struct *work)

[ ... ]

Quoted 17 lines

static void cxl_dport_map_ras(struct cxl_dport *dport) {

drivers/cxl/core/ras.c +7 -3

 	struct cxl_register_map *map = &dport->reg_map;
 	struct device *dev = dport->dport_dev;
 
-	if (!map->component_map.ras.valid)
+	if (!map->component_map.ras.valid) {
 		dev_dbg(dev, "RAS registers not found\n");
-	else if (cxl_map_component_regs(map, &dport->regs.component,
-					BIT(CXL_CM_CAP_CAP_ID_RAS)))
+		return;
+	}
+
+	if (cxl_map_component_regs(map, &dport->regs.component,
+				   BIT(CXL_CM_CAP_CAP_ID_RAS))) {
 		dev_dbg(dev, "Failed to map RAS capability.\n");
+		return;

drivers/cxl/core/ras.c static void cxl_cper_prot_err_work_fn(struct work_struct *work) · line 173

Quoted 10 lines

drivers/cxl/core/ras.c +10 -0

+	}
+
+	if (!dev_is_pci(dev))
+		return;
+
+	cxl_unmask_proto_interrupts(dev);
+	if (devm_add_action_or_reset(dport_to_host(dport),
+				     cxl_mask_proto_irqs, dev)) {
+		dev_warn(dev, "failed to defer CXL proto-irq mask; CXL protocol error reporting disabled\n");
+		dport->regs.component.ras = NULL;

[Severity: High]
Does setting ras to NULL here risk a kernel panic?

If a CXL protocol error triggers in the brief window between unmasking and devm_add_action_or_reset() masking it again on failure, the AER IRQ handler will enqueue the error to the kfifo.

When the kfifo worker dequeues the error and calls cxl_do_recovery(), will it encounter a NULL ras_base and unconditionally call panic("CXL: UCE with unmapped RAS registers")?

Quoted 2 lines

drivers/cxl/core/ras.c +1 -0
+	}
 }

[ ... ]

Quoted 9 lines

drivers/cxl/core/ras.c +3 -1

@@ -165,8 +211,21 @@ void devm_cxl_port_ras_setup(struct cxl_port *port)
 
 	map->host = &port->dev;
 	if (cxl_map_component_regs(map, &port->regs,
-				   BIT(CXL_CM_CAP_CAP_ID_RAS)))
+				   BIT(CXL_CM_CAP_CAP_ID_RAS))) {
 		dev_dbg(&port->dev, "Failed to map RAS capability\n");
+		return;
+	}

drivers/cxl/core/ras.c void devm_cxl_port_ras_setup(struct cxl_port *port) · line 227

Quoted 10 lines

drivers/cxl/core/ras.c +10 -0

+
+	dev = is_cxl_endpoint(port) ? port->uport_dev->parent : port->uport_dev;
+	if (!dev_is_pci(dev))
+		return;
+
+	cxl_unmask_proto_interrupts(dev);
+	if (devm_add_action_or_reset(&port->dev, cxl_mask_proto_irqs, dev)) {
+		dev_warn(&port->dev,
+			 "failed to defer CXL proto-irq mask; CXL protocol error reporting disabled\n");
+		port->regs.ras = NULL;

[Severity: High]
Can setting ras to NULL here also cause a panic?

Similar to the dport case, if an error is triggered and logged before the devm action masks the interrupts again, will cxl_do_recovery() panic when it accesses the NULL ras_base?

drivers/cxl/core/ras.c void devm_cxl_port_ras_setup(struct cxl_port *port) · line 229

Quoted 2 lines

drivers/cxl/core/ras.c +1 -0
+	}
 }

[Severity: High]
Can exposing a partially initialized dport cause a panic?

Looking at __devm_cxl_add_dport() in drivers/cxl/core/port.c, the dport is published to the xarray before its RAS registers are mapped:

drivers/cxl/core/port.c:__devm_cxl_add_dport() {
	...
	rc = add_dport(port, dport);
	if (rc)
		goto err;
	...
	devm_cxl_dport_ras_setup(dport);
	...
}

If firmware left CXL protocol errors unmasked, or if an error is already pending in the kfifo, the kfifo consumer can execute concurrently. It successfully looks up the partially initialized dport in the xarray and passes it to cxl_do_recovery().

Because the mapping hasn't completed, to_ras_base() returns NULL, resulting in an immediate panic("CXL: UCE with unmapped RAS registers").

Since cxl_do_recovery() was introduced earlier in this series, does this make the race fatal?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=13

Alison Schofield

maintainer Reviewed-by on v19 13/14

2026-08-04 02:29 UTC #46 lore raw

Re: [PATCH v19 13/14] PCI/CXL: Mask/Unmask CXL protocol errors

Quoted 10 lines · Terry Bowman

CXL protocol errors must be unmasked to be reported. Add pci_aer_mask_internal_errors() as the symmetric counterpart to pci_aer_unmask_internal_errors() and export both for cxl_core.
Unmask CXL internal errors in cxl_dport_map_ras() and devm_cxl_port_ras_setup() after the RAS register block is successfully mapped. Register a devm action to restore the mask on teardown. The unmask/mask helpers gate on dev_is_pci() and pcie_aer_is_native() internally so callers need no special-casing.

Quoted 5 lines

Remove the dev_is_pci(dport->dport_dev) guard in devm_cxl_dport_rch_ras_setup(). On RCH systems dport->dport_dev is the pci_host_bridge device which is not on pci_bus_type, so this guard blocked real hardware. The caller already gates on dport->rch.

Terry Bowman

author

2026-08-03 22:20 UTC #47 lore raw

PATCH v19 14/14 Documentation: cxl: Document CXL protocol error handling

Add Documentation/driver-api/cxl/linux/protocol-error-handling.rst describing the end-to-end CXL protocol error path: AER ingress, the AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special cases, severity policy, trace events, and a source code map.

This documents the architecture introduced by the preceding patches in this series.

Assisted-by Claude:claude-opus-4.9

---

Changes in v18->v19:
- Alignment fixes
- Add RC_END/RCiEP to diagram
- Wrap past 80 columns.
- Update cxl_forward_error() return-value behavior
- Add review-by for Jonathan Cameron
- Document the CPER/firmware-first flow (GHES + ext-log producers, CPER-CXL kfifo, trace-only consumer)
- Document the fatal EP/RCD UCE flow via the .error_detected (cxl_pci_error_detected) RAS path, including channel-state handling
- Add an "AER handlers vs RAS handlers" section clarifying the two handler layers and their relationship

Changes in v17->v18:
- Simplify document for readability (Jonathan)
- Drop historical context that goes stale (Jonathan)
- Shorten ASCII flow diagram (Jonathan)
- Drop manual backtick markup, use automarkup (Jonathan)
- Clarify USP/DSP as single switch component (Dave)
- Fix line wrapping to 80 chars (Jonathan)

---
 Documentation/driver-api/cxl/index.rst        |   1 +
 .../cxl/linux/protocol-error-handling.rst     | 401 ++++++++++++++++++
 2 files changed, 402 insertions(+)
 create mode 100644 Documentation/driver-api/cxl/linux/protocol-error-handling.rst

Documentation/driver-api/cxl/index.rst +1 -0

index 3dfae1d310ca5..6861b2e5726a3 100644
@@ -42,6 +42,7 @@ that have impacts on each other.  The docs here break up configurations steps.
    linux/dax-driver
    linux/memory-hotplug
    linux/access-coordinates
+   linux/protocol-error-handling
 
 .. toctree::
    :maxdepth: 2

Documentation/driver-api/cxl/linux/protocol-error-handling.rst new file +401 -0

401 lines not on this page · raw message

Signature · 1 line

2.34.1

[email protected]

AI review

2026-08-03 22:31 UTC #48 lore raw

Re: [PATCH v19 14/14] Documentation: cxl: Document CXL protocol error handling

Sashiko has reviewed this patch and found no issues. It looks great!

Quoted 10 lines

Add Documentation/driver-api/cxl/linux/protocol-error-handling.rst describing the end-to-end CXL protocol error path: AER ingress, the AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special cases, severity policy, trace events, and a source code map.
This documents the architecture introduced by the preceding patches in this series.
Assisted-by Claude:claude-opus-4.9
Signed-off-by Terry Bowman <[email protected]>

Sashiko has reviewed this patch and found no issues. It looks great!

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=14

Alison Schofield

maintainer Reviewed-by on v19 14/14

2026-08-04 02:30 UTC #49 lore raw

Re: [PATCH v19 14/14] Documentation: cxl: Document CXL protocol error handling

Quoted 8 lines · Terry Bowman

Add Documentation/driver-api/cxl/linux/protocol-error-handling.rst describing the end-to-end CXL protocol error path: AER ingress, the AER-CXL kfifo handoff, the cxl_core consumer worker, RCD/RCH special cases, severity policy, trace events, and a source code map.
This documents the architecture introduced by the preceding patches in this series.

Dave Jiang

maintainer Applied

2026-08-05 21:19 UTC #50 lore raw