On 7/20/2026 3:12 PM, Dave Jiang wrote:
>
>
> On 7/17/26 3:26 PM, Terry Bowman wrote:
>> 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]
>
> With the minor sashiko issue addressed,
> Reviewed-by: Dave Jiang <[email protected]>
>
> This can probably be picked up ahead of the series.
>
>
Yes.
-Terry
>>
>> ---
>>
>> Changes in v17 -> v18:
>> - New patch.
>> ---
>> drivers/acpi/apei/ghes.c | 50 ++++++++++++++++++++++++++--------------
>> drivers/cxl/pci.c | 1 -
>> 2 files changed, 33 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
>> index 3236a3ce79d6b..ca7a138c1ff2e 100644
>> --- a/drivers/acpi/apei/ghes.c
>> +++ b/drivers/acpi/apei/ghes.c
>> @@ -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");
>> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
>> index 267c679b0b3c2..7c6faee7f85ed 100644
>> --- a/drivers/cxl/pci.c
>> +++ b/drivers/cxl/pci.c
>> @@ -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);
>> }
>>
>
