Re: [PATCH v2 6/6] efi/runtime-wrappers: retire the worker if a wedged call ever returns

[Ard Biesheuvel]

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 <ardb@xxxxxxxxxx>
> Signed-off-by: Breno Leitao <leitao@xxxxxxxxxx>
> ---
>  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