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

["Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>]

[-- Attachment #1: Type: text/plain, Size: 3478 bytes --]

On Fri, Jun 12, 2026 at 05:54:36PM +0100, Bernhard Kaindl wrote:
> Anthony reported a boot-time assertion in init_xen_time() via efi_get_time()
> -> efi_rs_enter() in vcpu_save_fpu() on a Broadwell-D system:
> 
>   Assertion '!is_idle_vcpu(v)' failed at arch/x86/i387.c:195
> 
> 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.
> 
> Fixes: 1792bb9a99d2 ("x86: Cleanup cr0.TS flag handling")
> Fixes: dba44e051209 ("x86: Remove fully_eager_fpu")
> Reported-by: Anthony PERARD <anthony.perard@vates.tech>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Bernhard Kaindl <bernhard.kaindl@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> ---
>  xen/common/efi/runtime.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> Jan Beulich's suggestion to guard the calls to vcpu_save_fpu() and
> vcpu_restore_fpu() in the EFI runtime path with is_idle_vcpu() checks
> seems to be the right approach to fix the assertion failure for idle vCPUs:
> 
> > The thinko looks to be in 4b9851c64522 ("x86: Remove fpu_initialised/fpu_dirty"):
> > While vcpu_restore_fpu() indeed unconditionally set the two boolean fields to
> > true at that point, idle vCPU-s may never make it through that function, and
> > hence ->fpu_dirtied would have remained false, triggering the (original) early
> > exit from _vcpu_save_fpu(). Perhaps all we can do now is guard the call to
> > vcpu_save_fpu() (and also the one to vcpu_restore_fpu() out of efi_rs_leave())
> > by explicit is_idle_vcpu() checks. Much like the calls are guarded in
> > __context_switch().
> 
> Anthony, could you test this 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?
> 
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index a23fa75e3740..596f2710fb21 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)
> -- 
> 2.39.5
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]