Re: [PATCH v2 1/8] x86/fpu: Check for missing AVX and AVX-512 xstate bits

[David Laight <david.laight.linux@gmail.com>]

On Mon, 27 Jul 2026 22:27:27 -0700
Borislav Petkov <bp@alien8.de> wrote:

> 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 <ebiggers@kernel.org>
> > ---
> >  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()

That is too terse.
Since none of the uses are overlong lines it really doesn't matter.

> 
> 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?

More likely userspace running an old kernel on a new cpu.
While unlikely to be a problem with AVX and AVX-512, it did happen when they
were first added and might happen when the next feature is added.

> 
> 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.

I don't want to see the bug reports.
The flames aren't big, what happens is that the registers don't get preserved
across a process switch - so code tends to work a lot of the time.

For userspace the XCR bits are the important ones.

	David

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