Re: [PATCH v2 1/8] x86/fpu: Check for missing AVX and AVX-512 xstate bits
Borislav Petkov <[email protected]> Mon, 27 Jul 2026 22:27:27 -0700
| Newsgroups | org.infradead.lists.linux-um,org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel,org.kernel.vger.linux-raid |
|---|---|
| Message-ID | <20260728052727.GCamg9v5WxNieTMUwE@fat_crate.local> |
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