[PATCH] efi: Skip FPU save / restore if using idle vCPU

Ross Lagerwall posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20260616130051.3359801-1-ross.lagerwall@citrix.com
xen/common/efi/runtime.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
[PATCH] efi: Skip FPU save / restore if using idle vCPU
Posted by Ross Lagerwall 1 month, 1 week ago
The recent changes to remove lazy FPU support mean that
vcpu_{save,restore}_fpu perform the action unconditionally. This trips
the assert when calling these functions from an idle vCPU as might
happen when calling an EFI runtime function. There is no need to
save/restore the FPU in that case so skip the calls if using the idle
vCPU.

Fixes: dba44e051209 ("x86: Remove fully_eager_fpu")
Fixes: 4b9851c64522 ("x86: Remove fpu_initialised/fpu_dirty")
Reported-by: Anthony PERARD <anthony.perard@vates.tech>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
---

Anythony, can you check if this fixes boot on your machine?

 xen/common/efi/runtime.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
index a23fa75e3740..7713676ce288 100644
--- a/xen/common/efi/runtime.c
+++ b/xen/common/efi/runtime.c
@@ -88,6 +88,7 @@ struct efi_rs_state efi_rs_enter(void)
     static const u16 fcw = FCW_DEFAULT;
     static const u32 mxcsr = MXCSR_DEFAULT;
     struct efi_rs_state state = { .cr3 = 0 };
+    struct vcpu *curr = current;
 
     if ( mfn_eq(efi_l4_mfn, INVALID_MFN) )
         return state;
@@ -98,7 +99,8 @@ struct efi_rs_state efi_rs_enter(void)
      */
     sync_local_execstate();
     state.cr3 = read_cr3();
-    vcpu_save_fpu(current);
+    if ( !is_idle_vcpu(curr) )
+        vcpu_save_fpu(curr);
     asm volatile ( "fnclex; fldcw %0" :: "m" (fcw) );
     asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
 
@@ -107,7 +109,7 @@ struct efi_rs_state efi_rs_enter(void)
     /* prevent fixup_page_fault() from doing anything */
     irq_enter();
 
-    if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
+    if ( is_pv_vcpu(curr) && !is_idle_vcpu(curr) )
     {
         struct desc_ptr gdt_desc = {
             .limit = LAST_RESERVED_GDT_BYTE,
@@ -159,7 +161,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.53.0
Re: [PATCH] efi: Skip FPU save / restore if using idle vCPU
Posted by Teddy Astie 1 month, 1 week ago
Le 16/06/2026 à 15:04, Ross Lagerwall a écrit :
> The recent changes to remove lazy FPU support mean that
> vcpu_{save,restore}_fpu perform the action unconditionally. This trips
> the assert when calling these functions from an idle vCPU as might
> happen when calling an EFI runtime function. There is no need to
> save/restore the FPU in that case so skip the calls if using the idle
> vCPU.
> 
> Fixes: dba44e051209 ("x86: Remove fully_eager_fpu")
> Fixes: 4b9851c64522 ("x86: Remove fpu_initialised/fpu_dirty")
> Reported-by: Anthony PERARD <anthony.perard@vates.tech>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> 
> Anythony, can you check if this fixes boot on your machine?
> 
>   xen/common/efi/runtime.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index a23fa75e3740..7713676ce288 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -88,6 +88,7 @@ struct efi_rs_state efi_rs_enter(void)
>       static const u16 fcw = FCW_DEFAULT;
>       static const u32 mxcsr = MXCSR_DEFAULT;
>       struct efi_rs_state state = { .cr3 = 0 };
> +    struct vcpu *curr = current;
>   
>       if ( mfn_eq(efi_l4_mfn, INVALID_MFN) )
>           return state;
> @@ -98,7 +99,8 @@ struct efi_rs_state efi_rs_enter(void)
>        */
>       sync_local_execstate();
>       state.cr3 = read_cr3();
> -    vcpu_save_fpu(current);
> +    if ( !is_idle_vcpu(curr) )
> +        vcpu_save_fpu(curr);
>       asm volatile ( "fnclex; fldcw %0" :: "m" (fcw) );
>       asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
>   
> @@ -107,7 +109,7 @@ struct efi_rs_state efi_rs_enter(void)
>       /* prevent fixup_page_fault() from doing anything */
>       irq_enter();
>   
> -    if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
> +    if ( is_pv_vcpu(curr) && !is_idle_vcpu(curr) )
>       {
>           struct desc_ptr gdt_desc = {
>               .limit = LAST_RESERVED_GDT_BYTE,
> @@ -159,7 +161,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)

No issues with the patch content, though it's the same as [1].

[1] 
https://lore.kernel.org/xen-devel/8de2649558826621d49b404cae7a874f504e6b86.1781282640.git.bernhard.kaindl@citrix.com/
Re: [PATCH] efi: Skip FPU save / restore if using idle vCPU
Posted by Ross Lagerwall 1 month, 1 week ago
On 6/16/26 2:21 PM, Teddy Astie wrote:
> Le 16/06/2026 à 15:04, Ross Lagerwall a écrit :
>> The recent changes to remove lazy FPU support mean that
>> vcpu_{save,restore}_fpu perform the action unconditionally. This trips
>> the assert when calling these functions from an idle vCPU as might
>> happen when calling an EFI runtime function. There is no need to
>> save/restore the FPU in that case so skip the calls if using the idle
>> vCPU.
>>
>> Fixes: dba44e051209 ("x86: Remove fully_eager_fpu")
>> Fixes: 4b9851c64522 ("x86: Remove fpu_initialised/fpu_dirty")
>> Reported-by: Anthony PERARD <anthony.perard@vates.tech>
>> Suggested-by: Jan Beulich <jbeulich@suse.com>
>> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
>> ---
>>
>> Anythony, can you check if this fixes boot on your machine?
>>
>>   xen/common/efi/runtime.c | 9 ++++++---
>>   1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
>> index a23fa75e3740..7713676ce288 100644
>> --- a/xen/common/efi/runtime.c
>> +++ b/xen/common/efi/runtime.c
>> @@ -88,6 +88,7 @@ struct efi_rs_state efi_rs_enter(void)
>>       static const u16 fcw = FCW_DEFAULT;
>>       static const u32 mxcsr = MXCSR_DEFAULT;
>>       struct efi_rs_state state = { .cr3 = 0 };
>> +    struct vcpu *curr = current;
>>       if ( mfn_eq(efi_l4_mfn, INVALID_MFN) )
>>           return state;
>> @@ -98,7 +99,8 @@ struct efi_rs_state efi_rs_enter(void)
>>        */
>>       sync_local_execstate();
>>       state.cr3 = read_cr3();
>> -    vcpu_save_fpu(current);
>> +    if ( !is_idle_vcpu(curr) )
>> +        vcpu_save_fpu(curr);
>>       asm volatile ( "fnclex; fldcw %0" :: "m" (fcw) );
>>       asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
>> @@ -107,7 +109,7 @@ struct efi_rs_state efi_rs_enter(void)
>>       /* prevent fixup_page_fault() from doing anything */
>>       irq_enter();
>> -    if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
>> +    if ( is_pv_vcpu(curr) && !is_idle_vcpu(curr) )
>>       {
>>           struct desc_ptr gdt_desc = {
>>               .limit = LAST_RESERVED_GDT_BYTE,
>> @@ -159,7 +161,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)
> 
> No issues with the patch content, though it's the same as [1].
> 
> [1] https://lore.kernel.org/xen-devel/8de2649558826621d49b404cae7a874f504e6b86.1781282640.git.bernhard.kaindl@citrix.com/

Sorry, I wasn't CC'ed and missed that.

Ross

Re: [PATCH] efi: Skip FPU save / restore if using idle vCPU
Posted by Teddy Astie 1 month, 1 week ago
Le 16/06/2026 à 15:04, Ross Lagerwall a écrit :
> The recent changes to remove lazy FPU support mean that
> vcpu_{save,restore}_fpu perform the action unconditionally. This trips
> the assert when calling these functions from an idle vCPU as might
> happen when calling an EFI runtime function. There is no need to
> save/restore the FPU in that case so skip the calls if using the idle
> vCPU.
> 
> Fixes: dba44e051209 ("x86: Remove fully_eager_fpu")
> Fixes: 4b9851c64522 ("x86: Remove fpu_initialised/fpu_dirty")
> Reported-by: Anthony PERARD <anthony.perard@vates.tech>
> Suggested-by: Jan Beulich <jbeulich@suse.com>
> Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
> 
> Anythony, can you check if this fixes boot on your machine?
> 
>   xen/common/efi/runtime.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/common/efi/runtime.c b/xen/common/efi/runtime.c
> index a23fa75e3740..7713676ce288 100644
> --- a/xen/common/efi/runtime.c
> +++ b/xen/common/efi/runtime.c
> @@ -88,6 +88,7 @@ struct efi_rs_state efi_rs_enter(void)
>       static const u16 fcw = FCW_DEFAULT;
>       static const u32 mxcsr = MXCSR_DEFAULT;
>       struct efi_rs_state state = { .cr3 = 0 };
> +    struct vcpu *curr = current;
>   
>       if ( mfn_eq(efi_l4_mfn, INVALID_MFN) )
>           return state;
> @@ -98,7 +99,8 @@ struct efi_rs_state efi_rs_enter(void)
>        */
>       sync_local_execstate();
>       state.cr3 = read_cr3();
> -    vcpu_save_fpu(current);
> +    if ( !is_idle_vcpu(curr) )
> +        vcpu_save_fpu(curr);
>       asm volatile ( "fnclex; fldcw %0" :: "m" (fcw) );
>       asm volatile ( "ldmxcsr %0" :: "m" (mxcsr) );
>   
> @@ -107,7 +109,7 @@ struct efi_rs_state efi_rs_enter(void)
>       /* prevent fixup_page_fault() from doing anything */
>       irq_enter();
>   
> -    if ( is_pv_vcpu(current) && !is_idle_vcpu(current) )
> +    if ( is_pv_vcpu(curr) && !is_idle_vcpu(curr) )
>       {
>           struct desc_ptr gdt_desc = {
>               .limit = LAST_RESERVED_GDT_BYTE,
> @@ -159,7 +161,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)

Reviewed-by: Teddy Astie <teddy.astie@vates.tech>

Teddy