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

[Eric Biggers <ebiggers@kernel.org>]

On Tue, Jul 28, 2026 at 03:59:36PM -0700, Borislav Petkov wrote:
> On Tue, Jul 28, 2026 at 06:34:47PM +0000, Eric Biggers wrote:
> > The argument is the bitmask of xfeatures that are present.  0 is an
> > empty bitmask, i.e. no xfeatures are present.
> 
> Ok, so what does that mean when you see it in the code:
> 
> 	clear_cpu_caps_with_missing_xfeatures(0)
> 
> clear the CPU caps with missing xfeatures bitmask 0?

It clears the CPU caps whose xfeature dependencies are missing from the
given mask, which denotes the present xfeatures.  An empty mask means
nothing present, i.e. everything is missing.

It could be a separate function that clears unconditionally, but then
the clearing and the log messages would need to be duplicated.

I don't know what your suggestion is.  Are you still asking for the
function to be renamed to clear_cpu_caps_xft()?  Note that the
abbreviation "xft" doesn't appear anywhere else in arch/x86/.  Maybe you
want the argument inverted?  I don't know what you want, sorry.

> You brought up the UML example upthread. I'm trying to figure out why we care
> about the UML case. I'm also assuming that case will be fixed with whatever we
> end up doing here.

The UML update is in this series.  See patch 2.  Maybe you don't care
about UML but many people do.  kunit.py uses it by default, for example.

> > First, the code being added *is* to fpu__init_system_xstate().  Do you
> > think it needs to be moved later in the function, and if so why?
> 
> Lemme try again:
> 
> there where we sanity check the xfeatures mask, at that same place we can
> sanity check the X86_FEATURE flags we have set in our internal representation
> of CPUID flags. At exactly the same place. And we warn and clear the ones are
> wrong.

This is basically what this patch does already:

	clear_cpu_caps_with_missing_xfeatures(fpu_kernel_cfg.max_features);

	fpu_kernel_cfg.independent_features = fpu_kernel_cfg.max_features &
					      XFEATURE_MASK_INDEPENDENT;

	/*
	 * Clear XSAVE features that are disabled in the normal CPUID.
	 */
	for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
		unsigned short cid = xsave_cpuid_features[i];

		/* Careful: X86_FEATURE_FPU is 0! */
		if ((i != XFEATURE_FP && !cid) || !boot_cpu_has(cid))
			fpu_kernel_cfg.max_features &= ~BIT_ULL(i);
	}

Maybe you want it moved down one statement so that it's immediately
adjacent to the loop?

Or maybe you want it to be integrated into the loop itself?  It's
doable, but a bit awkward because the two CPU features we'd like to
clear each depend on multiple xfeatures rather than just one.  And also
there are those two cases earlier in the function (XSAVE or FPU being
disabled) that do an early return and never reach here.  It's nice to
use the same helper function in those cases.

> > Second, clearing xfeatures is the opposite of what's needed.
> 
> I meant clearing X86_FEATURE flags. Basically what you started doing.
> 
> All I'm proposing is having the *whole* X86_FEATURE clearing concentrated in
> a single function so that it is called once per CPU and then we're done.

fpu__init_system_xstate() is already a function called once on the boot
CPU.  Is your suggestion that the feature clearing should be once per
CPU instead of just on the boot CPU?

- Eric