On Mon, Jul 27, 2026 at 07:15:56PM -0700, Eric Biggers wrote:
> If the CPU declares AVX or AVX-512 support, verify that the
> corresponding xstate bits are also set.  If not, warn and clear them.
> 
> This eliminates the perceived need for AVX and AVX-512 optimized code in

s/This eliminates/Eliminate/

> the kernel to call cpu_has_xfeatures().  That has never been universally
> done, which strongly suggests that it has never really been needed in
> practice, but this should remove any remaining doubt.
> 
> Signed-off-by: Eric Biggers <[email protected]>
> ---
>  arch/x86/kernel/fpu/xstate.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index a7b6524a9dea2..904ff933c0d88 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -799,6 +799,23 @@ static u64 __init guest_default_mask(void)
>       return ~(u64)XFEATURE_MASK_USER_DYNAMIC;
>  }
>  
> +/* Clear any X86_FEATURE_* used by the kernel whose xfeatures are missing. */
> +static void __init clear_cpu_caps_with_missing_xfeatures(u64 xfeatures)

That function name is a bit too long. How about:

clear_cpu_caps_xft()

or so.

> +{
> +     u64 mask;
> +
> +     mask = XFEATURE_MASK_FPSSE | XFEATURE_MASK_YMM;
> +     if (boot_cpu_has(X86_FEATURE_AVX) && (xfeatures & mask) != mask) {
> +             pr_err("x86/fpu: Disabling AVX support due to missing xstate 
> features\n");
> +             setup_clear_cpu_cap(X86_FEATURE_AVX);
> +     }
> +     mask = XFEATURE_MASK_FPSSE | XFEATURE_MASK_YMM | XFEATURE_MASK_AVX512;
> +     if (boot_cpu_has(X86_FEATURE_AVX512F) && (xfeatures & mask) != mask) {
> +             pr_err("x86/fpu: Disabling AVX-512 support due to missing 
> xstate features\n");
> +             setup_clear_cpu_cap(X86_FEATURE_AVX512F);
> +     }
> +}
> +
>  /*
>   * Enable and initialize the xsave feature.
>   * Called once per system bootup.
> @@ -812,12 +829,14 @@ void __init fpu__init_system_xstate(unsigned int 
> legacy_size)
>  
>       if (!boot_cpu_has(X86_FEATURE_FPU)) {
>               pr_info("x86/fpu: No FPU detected\n");
> +             clear_cpu_caps_with_missing_xfeatures(0);
>               return;
>       }
>  
>       if (!boot_cpu_has(X86_FEATURE_XSAVE)) {
>               pr_info("x86/fpu: x87 FPU will use %s\n",
>                       boot_cpu_has(X86_FEATURE_FXSR) ? "FXSAVE" : "FSAVE");
> +             clear_cpu_caps_with_missing_xfeatures(0);

Also, I'm not really clear on the usage here: if the CPU doesn't have FPU or
XSAVE, we pass in xfeature 0 which is XFEATURE_FP in both cases. And then we
clear AVX and AVX-512.

The 0 is basically forcing the checks in the function to match, i.e., it looks
to me like we're defining a new interface but then we're misusing it so that
those basic CPU flags are cleared.

What are we even protecting against here?

AVX and AVX-512 code needs to check whether it has FPU and XSAVE support?

I.e., we're protecting against some weird guests?

I wanna say, we should not protect but let them crash'n'burn in big big flames
which can be seen from a mile away.

Or do you have a sensible use case in mind which we really wanna protect
against and this all actually makes sense?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Reply via email to