On Fri, 17 Jul 2026 17:26:55 -0500 Terry Bowman <[email protected]> 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] Reviewed-by: Jonathan Cameron <[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(-) > > 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); > } >
