Re: [PATCH] x86/efi: Skip FPU save/restore for idle vCPU in EFI, runtime path

[Jan Beulich <jbeulich@xxxxxxxx>]

On 12.06.2026 17:41, Bernhard Kaindl wrote:
> Hi Anthony, could you test this patch which exactly applies the changes 
> Jan suggested? Summary:

So I'm a little irritated by this: The subject suggests this is a proper
patch submission, yet about everything else here suggests it is not. For
the eventual real patch, may I minimally suggest ...

> Guard both EFI runtime FPU calls with !is_idle_vcpu() to skip save/restore
> for idle vCPUs, which don't have an FPU context to save/restore,
> much like the calls are guarded in __context_switch(),
> where save/restore is done only for non-idle vCPUs.
> As these simple guards should preferably go into Xen 4.22: Please test 
> if there are any further regressions with the 'cmos-rtc-probe' 
> workaround you just added removed to check if guarding the assertions as 
> Jan suggested is enough to fix the issues triggered on your machine. 
> Thanks, Bernhard The patch to test follows: [PATCH] x86/efi: Skip FPU 
> save/restore for idle vCPU in EFI, runtime path
> Anthony reported a boot-time crash in init_xen_time() via efi_get_time()
> on a Broadwell-D system:
>    Assertion '!is_idle_vcpu(v)' failed at arch/x86/i387.c:195

... to resolve this line number to a function name, to provide sufficient
context.

> The failing path is an EFI runtime call reached early during boot,
> where current may still be the idle vCPU.
> This became fragile after the lazy-FPU removal cleanup series.
> In 1792bb9a99d2 ("x86: Cleanup cr0.TS flag handling"),
> efi_rs_enter() was changed from save_fpu_enable() to vcpu_save_fpu(curr),
> which unconditionally asserts !is_idle_vcpu(v)
> so an EFI runtime call in idle context now asserts.
> Likewise, in dba44e051209 ("x86: Remove fully_eager_fpu"),
> efi_rs_leave() was changed to call vcpu_restore_fpu(curr),
> which has the same assertion and can fail for the same reason.
> Guard both EFI runtime FPU calls with !is_idle_vcpu() to skip save/restore
> for idle vCPUs, which don't have an FPU context to save/restore,
> much like the calls are guarded in __context_switch(),
> where save/restore is done only for non-idle vCPUs.

I further would help if it was explicitly stated that no other uses of
the two functions are affected (provided the necessary auditing was done,
but ftoad I did go through that already on Friday and didn't find other
problematic call sites).

Jan

> Fixes: 1792bb9a99d2 ("x86: Cleanup cr0.TS flag handling")
> Fixes: dba44e051209 ("x86: Remove fully_eager_fpu")
> Reported-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> Suggested-by: Jan Beulich <jbeulich@xxxxxxxx>
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@xxxxxxxxxx>
> ---
>   xen/common/efi/runtime.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index a23fa75e37..596f2710fb 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -98,7 +98,8 @@ struct efi_rs_state efi_rs_enter(void)
>        */
>       sync_local_execstate();
>       state.cr3 = read_cr3();
> -    vcpu_save_fpu(current);
> +    if ( !is_idle_vcpu(current) )
> +        vcpu_save_fpu(current);
>       asm volatile ( "fnclex; fldcw %0" :: "m" (fcw) );
>       asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
> @@ -159,7 +160,8 @@ void efi_rs_leave(struct efi_rs_state *state)
>       }
>       irq_exit();
>       spin_unlock(&efi_rs_lock);
> -    vcpu_restore_fpu(curr);
> +    if ( !is_idle_vcpu(curr) )
> +        vcpu_restore_fpu(curr);
>   }
>   unsigned long efi_get_time(void)