Re: [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns
"Ard Biesheuvel" <[email protected]> Fri, 12 Jun 2026 13:11:11 +0200
| Newsgroups | org.kernel.vger.linux-efi,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hi, On Fri, 12 Jun 2026, at 13:01, Breno Leitao wrote: > When __efi_queue_work() times out it disables runtime services and > returns, but the kworker is still blocked inside firmware. If the > firmware eventually unblocks, efi_call_rts() would run its tail on an > efi_rts_work that the timed-out caller has long abandoned: signalling a > stale completion and clearing efi_runtime_lock_owner that may by then > belong to another caller. > > If runtime services have been disabled by the time the call returns, > park the worker in a schedule() loop instead, so it never touches > efi_rts_work again or returns to the workqueue. > > x86's efi_crash_gracefully_on_page_fault() already ends in the same loop > for the page-fault case. Factor it into efi_rts_park_worker() and call it > from both paths. > > Suggested-by: Ard Biesheuvel <[email protected]> > Signed-off-by: Breno Leitao <[email protected]> > --- > arch/x86/platform/efi/quirks.c | 9 +-------- > drivers/firmware/efi/runtime-wrappers.c | 21 +++++++++++++++++++++ > include/linux/efi.h | 2 ++ > 3 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/arch/x86/platform/efi/quirks.c > b/arch/x86/platform/efi/quirks.c > index 90a065fcb1fab..02c56a02eb9bc 100644 > --- a/arch/x86/platform/efi/quirks.c > +++ b/arch/x86/platform/efi/quirks.c > @@ -832,12 +832,5 @@ void efi_crash_gracefully_on_page_fault(unsigned > long phys_addr, > clear_bit(EFI_RUNTIME_SERVICES, &efi.flags); > pr_info("Froze efi_rts_wq and disabled EFI Runtime Services\n"); > > - /* > - * Call schedule() in an infinite loop, so that any spurious wake ups > - * will never run efi_rts_wq again. > - */ > - for (;;) { > - set_current_state(TASK_IDLE); > - schedule(); > - } > + efi_rts_park_worker(); > } > diff --git a/drivers/firmware/efi/runtime-wrappers.c > b/drivers/firmware/efi/runtime-wrappers.c > index 842f72d44211f..998e5f8f1c62c 100644 > --- a/drivers/firmware/efi/runtime-wrappers.c > +++ b/drivers/firmware/efi/runtime-wrappers.c > @@ -219,6 +219,19 @@ static struct task_struct *efi_runtime_lock_owner; > extern struct semaphore __efi_uv_runtime_lock > __alias(efi_runtime_lock); > #endif > > +/* > + * Park a worker that must never run efi_rts_wq again: EFI runtime services > + * have been disabled and its efi_rts_work is abandoned. Loop in schedule() > + * so a spurious wakeup cannot resume it. > + */ > +void efi_rts_park_worker(void) > +{ > + for (;;) { > + set_current_state(TASK_IDLE); > + schedule(); > + } > +} > + > /* > * Calls the appropriate efi_runtime_service() with the appropriate > * arguments. > @@ -320,6 +333,14 @@ static void __nocfi efi_call_rts(struct work_struct *work) > efi_call_virt_check_flags(flags, efi_rts_work.caller); > arch_efi_call_virt_teardown(); > > + /* > + * If __efi_queue_work() timed out and disabled runtime services, the > + * caller is gone and efi_rts_work is no longer ours: park the worker > + * so it never signals the stale completion or runs again. > + */ > + if (!efi_enabled(EFI_RUNTIME_SERVICES)) > + efi_rts_park_worker(); > + So one thing to note here is that the arm64 version of the exception recovery (in efi_runtime_fixup_exception()) will also clear EFI_RUNTIME_SERVICES, and that will occur before this check. This means we will park the worker not only on a time out, but also on an sync exception in the firmware. I suspect this is actually what we want, but it deserves to be called out. > efi_rts_work.status = status; > complete(&efi_rts_work.efi_rts_comp); > efi_runtime_lock_owner = NULL; > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 24221a8424121..015505423277e 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -1256,6 +1256,8 @@ extern struct efi_runtime_work efi_rts_work; > /* Workqueue to queue EFI Runtime Services */ > extern struct workqueue_struct *efi_rts_wq; > > +void efi_rts_park_worker(void); > + > struct linux_efi_memreserve { > int size; // allocated size of the array > atomic_t count; // number of entries used > > -- > 2.53.0-Meta