Enable CXL PCIe Port Protocol Error handling and logging

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]

Signed-off-by Terry Bowman <[email protected]>

---

Changes in v17 -> v18:
- New patch.

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

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

index 0a8b3b9b63884..44b335d560708 100644
@@ -80,7 +80,8 @@ 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)
+		if ((aer_regs->uncor_status & ~aer_regs->uncor_mask) &
+		    aer_regs->uncor_severity)
 			*severity = AER_FATAL;
 		else
 			*severity = AER_NONFATAL;

Quoted 12 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).

Quoted 3 lines

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

Cc [email protected]

Signed-off-by Terry Bowman <[email protected]>

Reviewed-by Dave Jiang <[email protected]>

This can be picked up ahead looks like?

Quoted 23 lines

---

Changes in v17 -> v18:
- New patch.

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

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

index 0a8b3b9b63884..44b335d560708 100644
@@ -80,7 +80,8 @@ 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)
+		if ((aer_regs->uncor_status & ~aer_regs->uncor_mask) &
+		    aer_regs->uncor_severity)
 			*severity = AER_FATAL;
 		else
 			*severity = AER_NONFATAL;

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]

Signed-off-by Terry Bowman <[email protected]>

---

Changes in v17 -> v18:
- New patch.

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

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

index 0a8b3b9b63884..44b335d560708 100644

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

+		    aer_regs->uncor_severity)
 			*severity = AER_FATAL;
 		else
 			*severity = AER_NONFATAL;

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]

Signed-off-by Terry Bowman <[email protected]>

---

Changes in v17 -> v18:
- New patch.

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

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

index 0a8b3b9b63884..44b335d560708 100644

Signature · 1 line

2.34.1

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

New issues:
- [Low] The patch adds internal cancellation to both unregister functions but misses removing the redundant cancel_work_sync() for the protocol error work in cxl_ras_exit().

Pre-existing issues:
- [High] Fixed-size memcpy in cxl_cper_post_event causes an out-of-bounds read if the provided GHES CPER payload is smaller than sizeof(*rec).
- [High] Missing bounds check on untrusted dvsec_len in cxl_cper_setup_prot_err_work_data leads to an out-of-bounds read. --

--- Patch [2]: [PATCH v18 02/13] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks --- commit 70fa064ce36bd35723148aab9a91c80d0fbef760
Author: Terry Bowman <[email protected]>

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

This commit converts cxl_cper_work_lock and cxl_cper_prot_err_work_lock from spinlock_t to raw_spinlock_t to prevent a deadlock between IRQ and process contexts. It also restructures registration functions to warn on double registration and clear work pointers before cancelling them. Redundant cancel_work_sync() calls are removed since quiescing is handled internally.

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 is a pre-existing issue, but does cxl_cper_setup_prot_err_work_data() lack bounds checking on the untrusted dvsec_len?

If firmware provides a CXL protocol error record with a maliciously large dvsec_len, could this lead to an out-of-bounds read when calculating cap_start?

drivers/acpi/apei/ghes_helpers.c:cxl_cper_setup_prot_err_work_data() {
    ...
    dvsec_start = (u8 *)(prot_err + 1);
    cap_start = dvsec_start + prot_err->dvsec_len;

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

[ ... ]

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 pre-existing issue, but does cxl_cper_post_event() have an out-of-bounds read if the provided GHES CPER payload is smaller than sizeof(*rec)?

If firmware provides a CXL event record where gdata->error_data_length is smaller than sizeof(struct cxl_cper_event_rec), this fixed-size memcpy could exceed the bounds of the error status block:

drivers/acpi/apei/ghes.c:cxl_cper_post_event() {
    ...
    wd.event_type = event_type;
    memcpy(&wd.rec, rec, sizeof(wd.rec));
    ...
}

[ ... ]

drivers/cxl/pci.c static int __init cxl_pci_driver_init(void) · line 1087

Quoted 11 lines

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);
 }

[Severity: Low]
Is there a missed redundant cancel_work_sync() call in cxl_ras_exit() that should also be removed?

Since internal cancellation was added to the unregister functions, cxl_ras_exit() now has redundant synchronization:

drivers/cxl/core/ras.c:cxl_ras_exit() {
    ...
    cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
    cancel_work_sync(&cxl_cper_prot_err_work);
    ...
}

The LOW issue is valid and the extra cancel_work_sync() needs to be removed. I propose we fix if a series respin is necessary or otherwise fix in future.

The preexisting HIGH issues are GHES area and to be fixed by owners.

Terry

Quoted 108 lines · On 7/17/2026 5:49 PM, [email protected] wrote:

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

New issues:
- [Low] The patch adds internal cancellation to both unregister functions but misses removing the redundant cancel_work_sync() for the protocol error work in cxl_ras_exit().

Pre-existing issues:
- [High] Fixed-size memcpy in cxl_cper_post_event causes an out-of-bounds read if the provided GHES CPER payload is smaller than sizeof(*rec).
- [High] Missing bounds check on untrusted dvsec_len in cxl_cper_setup_prot_err_work_data leads to an out-of-bounds read. --

--- Patch [2]: [PATCH v18 02/13] acpi/apei/ghes: Use raw_spinlock_t for CXL CPER work locks --- commit 70fa064ce36bd35723148aab9a91c80d0fbef760
Author: Terry Bowman <[email protected]>

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

This commit converts cxl_cper_work_lock and cxl_cper_prot_err_work_lock from spinlock_t to raw_spinlock_t to prevent a deadlock between IRQ and process contexts. It also restructures registration functions to warn on double registration and clear work pointers before cancelling them. Redundant cancel_work_sync() calls are removed since quiescing is handled internally.

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 is a pre-existing issue, but does cxl_cper_setup_prot_err_work_data() lack bounds checking on the untrusted dvsec_len?

If firmware provides a CXL protocol error record with a maliciously large dvsec_len, could this lead to an out-of-bounds read when calculating cap_start?

drivers/acpi/apei/ghes_helpers.c:cxl_cper_setup_prot_err_work_data() {
    ...
    dvsec_start = (u8 *)(prot_err + 1);
    cap_start = dvsec_start + prot_err->dvsec_len;

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

[ ... ]

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 pre-existing issue, but does cxl_cper_post_event() have an out-of-bounds read if the provided GHES CPER payload is smaller than sizeof(*rec)?

If firmware provides a CXL event record where gdata->error_data_length is smaller than sizeof(struct cxl_cper_event_rec), this fixed-size memcpy could exceed the bounds of the error status block:

drivers/acpi/apei/ghes.c:cxl_cper_post_event() {
    ...
    wd.event_type = event_type;
    memcpy(&wd.rec, rec, sizeof(wd.rec));
    ...
}

[ ... ]

drivers/cxl/pci.c static int __init cxl_pci_driver_init(void) · line 1087

Quoted 11 lines

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);
 }  

[Severity: Low]
Is there a missed redundant cancel_work_sync() call in cxl_ras_exit() that should also be removed?

Since internal cancellation was added to the unregister functions, cxl_ras_exit() now has redundant synchronization:

drivers/cxl/core/ras.c:cxl_ras_exit() {
    ...
    cxl_cper_unregister_prot_err_work(&cxl_cper_prot_err_work);
    cancel_work_sync(&cxl_cper_prot_err_work);
    ...
}

The CXL CPER work registration and unregistration helpers in drivers/acpi/apei/ghes.c acquire cxl_cper_work_lock and cxl_cper_prot_err_work_lock with guard(spinlock), 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 via guard(spinlock_irqsave).

If a CPU is holding one of these locks via guard(spinlock) 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(raw_spinlock_irqsave) 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 now-redundant cancel_work_sync() call from cxl_pci_driver_exit() - cxl_cper_unregister_work() handles quiescing internally.

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]

---

Changes in v17 -> v18:
- New patch.

---
 drivers/acpi/apei/ghes.c | 50 ++++++++++++++++++++++++++--------------
 drivers/cxl/pci.c        |  1 -
 2 files changed, 33 insertions(+), 18 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/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);
 }
 

Quoted 37 lines · Terry Bowman

The CXL CPER work registration and unregistration helpers in drivers/acpi/apei/ghes.c acquire cxl_cper_work_lock and cxl_cper_prot_err_work_lock with guard(spinlock), 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 via guard(spinlock_irqsave).

If a CPU is holding one of these locks via guard(spinlock) 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(raw_spinlock_irqsave) 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 now-redundant cancel_work_sync() call from cxl_pci_driver_exit() - cxl_cper_unregister_work() handles quiescing internally.

Quoted 5 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]

With the minor sashiko issue addressed,

Reviewed-by Dave Jiang <[email protected]>

This can probably be picked up ahead of the series.

Quoted 137 lines

---

Changes in v17 -> v18:
- New patch.

---
 drivers/acpi/apei/ghes.c | 50 ++++++++++++++++++++++++++--------------
 drivers/cxl/pci.c        |  1 -
 2 files changed, 33 insertions(+), 18 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/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);
 }
 

The CXL CPER work registration and unregistration helpers in drivers/acpi/apei/ghes.c acquire cxl_cper_work_lock and cxl_cper_prot_err_work_lock with guard(spinlock), 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 via guard(spinlock_irqsave).

If a CPU is holding one of these locks via guard(spinlock) 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(raw_spinlock_irqsave) 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 now-redundant cancel_work_sync() call from cxl_pci_driver_exit() - cxl_cper_unregister_work() handles quiescing internally.

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]

---

Changes in v17 -> v18:
- New patch.

---
 drivers/acpi/apei/ghes.c | 50 ++++++++++++++++++++++++++--------------
 drivers/cxl/pci.c        |  1 -
 2 files changed, 33 insertions(+), 18 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/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);
 }
 

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.

Change register/unregister return types to void. 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().

Add a diagnostic log when the driver is not bound in cxl_cper_handle_prot_err().

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

Signed-off-by Terry Bowman <[email protected]>

Signed-off-by Dan Williams <[email protected]>

---

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   | 12 +++++++-----
 include/cxl/event.h      | 17 ++++++-----------
 5 files changed, 31 insertions(+), 43 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 +7 -5

index 99fb00949c2fa..135f1997e6f4f 100644
@@ -104,8 +104,11 @@ void cxl_cper_handle_prot_err(struct cxl_cper_prot_err_work_data *data)
 	}
 
 	guard(device)(&pdev->dev);
-	if (!pdev->dev.driver)
+	if (!pdev->dev.driver) {
+		dev_warn_ratelimited(&pdev->dev,
+				     "Device is unbound, abort CPER error handling\n");
 		return;
+	}
 
 	struct device *mem_dev __free(put_device) = bus_find_device(
 		&cxl_bus_type, NULL, pdev, match_memdev_by_parent);

@@ -129,15 +132,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);
-	cancel_work_sync(&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 -11

index ff97fea718d2c..3471d4f75c025 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,21 +298,16 @@ 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)
+static inline void 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)
+static inline void cxl_cper_unregister_prot_err_work(void)
 {
-	return 0;
 }
 static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd)
 {

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.

Change register/unregister return types to void. 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().

Add a diagnostic log when the driver is not bound in cxl_cper_handle_prot_err().

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

Signed-off-by Terry Bowman <[email protected]>

Signed-off-by Dan Williams <[email protected]>

---

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   | 12 +++++++-----
 include/cxl/event.h      | 17 ++++++-----------
 5 files changed, 31 insertions(+), 43 deletions(-)

include/cxl/event.h +0 -0

index ff97fea718d2c..3471d4f75c025 100644

include/cxl/event.h +0 -1

 {
-	return 0;
 }
 static inline int cxl_cper_prot_err_kfifo_get(struct cxl_cper_prot_err_work_data *wd)
 {

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.

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

Signed-off-by Terry Bowman <[email protected]>

Signed-off-by Dan Williams <[email protected]>

---

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");
 

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.

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

Signed-off-by Terry Bowman <[email protected]>

Signed-off-by Dan Williams <[email protected]>

---

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");
 

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. Add drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.

Introduce is_aer_internal_error() to identify CXL protocol errors from 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. Encapsulate the kfifo, per-producer spinlock, registration rwsem, and work pointer in struct cxl_proto_err_kfifo. Initialize the embedded kfifo via INIT_KFIFO() from a subsys_initcall so its metadata is populated before any producer or consumer runs.

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_flush() to synchronously wait for the consumer worker to drain the kfifo. A subsequent patch wires this into handle_error_source() for UCE events so the CXL plane completes error handling and panic policy before pci_aer_handle_error() drives PCIe recovery.

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]>

---

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 | 221 ++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h    |   6 +
 include/linux/aer.h           |  24 ++++
 6 files changed, 254 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/pcie/aer_cxl_vh.c

MAINTAINERS +2 -0

index 806bd2d80d153..39007aa90b20f 100644
@@ -6526,6 +6526,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 +221 -0

new file mode 100644
index 0000000000000..93bed07936100
@@ -0,0 +1,221 @@
+// 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/rwsem.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;
+	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;
+}
+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;
+}
+
+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 caller must flush the kfifo before AER recovery,
+ * false if no CXL error handling was initiated due to early return on
+ * error.
+ */
+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 consumer (for_each_cxl_proto_err())
+	 * drops 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)) {
+		/* Dropped; no panic - UCE unconfirmed without RAS read */
+		dev_err_ratelimited(&pdev->dev, "AER-CXL kfifo add failed\n");
+		pci_dev_put(pdev);
+		schedule_work(cxl_proto_err_kfifo.work);
+		return true;
+	}
+
+	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_flush() 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);
+		pci_dev_put(wd->pdev);
+	}
+}
+EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
+
+/**
+ * cxl_proto_err_flush - drain pending AER-CXL kfifo work synchronously
+ *
+ * Wait for the consumer worker to finish processing all entries
+ * currently in the kfifo. Used by handle_error_source() for UCE so
+ * the CXL plane can read CXL RAS, apply panic policy, and clear CXL
+ * state before pci_aer_handle_error() drives PCIe recovery.
+ *
+ * Snapshots the flush callback under rwsem_read and releases the rwsem
+ * before calling it.  This avoids holding rwsem_read across flush_work(),
+ * which would deadlock via the rwsem HANDOFF mechanism when a concurrent
+ * rwsem_write waiter (cxl_unregister_proto_err_work) blocks new readers
+ * including the worker's for_each_cxl_proto_err() rwsem_read acquisition.
+ *
+ * The flush_inflight counter prevents cxl_core module unload while a
+ * flush is in progress outside the rwsem. The counter is incremented
+ * under rwsem_read (mutually exclusive with the rwsem_write in
+ * cancel_cxl_proto_err() that NULLs the flush pointer) and decremented
+ * after the flush completes. cxl_unregister_proto_err_work() waits for
+ * the counter to reach zero before proceeding with cancel_work_sync().
+ *
+ * For correctable events the consumer can run asynchronously; AER
+ * does not need to call this helper for AER_CORRECTABLE.
+ */
+void cxl_proto_err_flush(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..fd203010877bf 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_flush(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_flush(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);

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. Add drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.

Introduce is_aer_internal_error() to identify CXL protocol errors from 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. Encapsulate the kfifo, per-producer spinlock, registration rwsem, and work pointer in struct cxl_proto_err_kfifo. Initialize the embedded kfifo via INIT_KFIFO()

from a subsys_initcall so its metadata is populated before any producer or consumer runs.

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_flush() to synchronously wait for the consumer worker to drain the kfifo. A subsequent patch wires this into handle_error_source() for UCE events so the CXL plane completes error handling and panic policy before pci_aer_handle_error() drives PCIe recovery.

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]>

---

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 | 221 ++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h    |   6 +
 include/linux/aer.h           |  24 ++++
 6 files changed, 254 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/pcie/aer_cxl_vh.c

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

new file mode 100644
index 0000000000000..93bed07936100

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

+	atomic_t flush_inflight;
+	DECLARE_KFIFO(fifo, struct cxl_proto_err_work_data,
+		      CXL_ERROR_SOURCES_MAX);
+};
 
+/**
+ * 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 caller must flush the kfifo before AER recovery,
+ * false if no CXL error handling was initiated due to early return on
+ * error.
+ */
+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");

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

+		return true;
+	}
+
+	schedule_work(cxl_proto_err_kfifo.work);
+	return true;
+}
 
 
+/**
+ * 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.

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

+		pci_dev_put(wd->pdev);
+	}
+}
+EXPORT_SYMBOL_FOR_MODULES(for_each_cxl_proto_err, "cxl_core");
+
+/**
+ * cxl_proto_err_flush - drain pending AER-CXL kfifo work synchronously
+ *
+ * Wait for the consumer worker to finish processing all entries
+ * currently in the kfifo. Used by handle_error_source() for UCE so
+ * the CXL plane can read CXL RAS, apply panic policy, and clear CXL
+ * state before pci_aer_handle_error() drives PCIe recovery.
+ *
+ * Snapshots the flush callback under rwsem_read and releases the rwsem
+ * before calling it.  This avoids holding rwsem_read across flush_work(),

+ *
+ * For correctable events the consumer can run asynchronously; AER
+ * does not need to call this helper for AER_CORRECTABLE.

+ */

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

+void cxl_proto_err_flush(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);
+	}
+}

On Fri, 17 Jul 2026 17:26:58 -0500
Terry Bowman <[email protected]> wrote:

Hi Terry.

This is a rather length patch description. Might be worth a parse to see if key details can be covered in something that folk are more likely to read!

Quoted 9 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. Add drivers/pci/pcie/aer_cxl_vh.c to implement this handoff via a kfifo-backed work item.

Introduce is_aer_internal_error() to identify CXL protocol errors from AER internal error status bits across both correctable and uncorrectable severities.

Already existed. Just in a different location.

Quoted 2 lines

Introduce is_cxl_error() to gate the VH kfifo path.

Feels like to much detail to me given not really anything to say about it other than the obvious.

Quoted 5 lines

Introduce struct cxl_proto_err_work_data to carry the error source PCI device and severity through the kfifo. Encapsulate the kfifo, per-producer spinlock, registration rwsem, and work pointer in struct cxl_proto_err_kfifo. Initialize the embedded kfifo via INIT_KFIFO()

Can we cut this down a little where we don't use useful info. e.g. "Initialize the kfifo from a subsys_initcall() to it is ready before any producer or consumer runs."

Quoted 9 lines

from a subsys_initcall so its metadata is populated before any producer or consumer runs.

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.

We don't need to cover error paths in the patch description unless they are really complex and need explanation. Even then probably belongs more in comments.

Anyhow you get the idea..

Quoted 17 lines

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_flush() to synchronously wait for the consumer worker to drain the kfifo. A subsequent patch wires this into handle_error_source() for UCE events so the CXL plane completes error handling and panic policy before pci_aer_handle_error() drives PCIe recovery.

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.

Quoted 7 lines

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.

Various things inline. The one potential thing I'd like to highlight is the loss of tracking assume panic isn't appropriate. My paranoid hat says always go the other way. If we don't know we didn't lose an uncorrectable error panic. Maybe it's worth us considering when this might happen in a real system?

Quoted 51 lines

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

Signed-off-by Dan Williams <[email protected]>

Signed-off-by Terry Bowman <[email protected]>

---

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 | 221 ++++++++++++++++++++++++++++++++++
 drivers/pci/pcie/portdrv.h    |   6 +
 include/linux/aer.h           |  24 ++++
 6 files changed, 254 insertions(+), 10 deletions(-)
 create mode 100644 drivers/pci/pcie/aer_cxl_vh.c

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

new file mode 100644
index 0000000000000..93bed07936100

drivers/pci/pcie/aer_cxl_vh.c line 9

Quoted 9 lines

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

@@ -0,0 +1,221 @@
+// 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/rwsem.h>

Check these. I'd expect at least spinlock.h to meet the rough include what you use aim.

drivers/pci/pcie/aer_cxl_vh.c line 21

Quoted 12 lines

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

+#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;

Ideally add a quick comment to every lock to say what data it covers. Kind of obvious for this one but in general it is good practice and reduces chance of later scope confusion.

Quoted 29 lines

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

+	atomic_t flush_inflight;
+	DECLARE_KFIFO(fifo, struct cxl_proto_err_work_data,
+		      CXL_ERROR_SOURCES_MAX);
+};
 
+/**
+ * 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 caller must flush the kfifo before AER recovery,
+ * false if no CXL error handling was initiated due to early return on
+ * error.
+ */
+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");

drivers/pci/pcie/aer_cxl_vh.c line 91

Quoted 10 lines

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

+		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 consumer (for_each_cxl_proto_err())
+	 * drops that ref after handling. On enqueue failure below, drop
+	 * the ref we just took to avoid a leak.

Most of this is useful. The what we do in error handling (given so local) probably not.