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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] A malformed CPUID reporting AVX512F=1 but AVX=0 will cause a #GP exception during xsetbv.
--

--- Patch [1]: [PATCH v4 1/8] x86/fpu: Check for missing AVX and AVX-512 xstate bits ---
commit 7ad6682a0d9e6a99cd138d1dc9dfbd430c1da651
Author: Eric Biggers <[email protected]>

x86/fpu: Check for missing AVX and AVX-512 xstate bits

This patch checks if the CPU declares AVX or AVX-512 support, and verifies
that the corresponding xstate bits are also set. If not, it warns and clears
the CPU capabilities.

> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index a7b6524a9dea2..97cfd4fb6cc00 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
[ ... ]
> @@ -844,6 +847,24 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
>  		goto out_disable;
>  	}
>  
> +	mask |= XFEATURE_MASK_YMM;
> +	if (boot_cpu_has(X86_FEATURE_AVX)) {
> +		if ((fpu_kernel_cfg.max_features & mask) != mask) {
> +			pr_err(FW_BUG
> +			       "x86/fpu: Disabling AVX support due to missing xstate features\n");
> +			setup_clear_cpu_cap(X86_FEATURE_AVX);
> +		}
> +	}
> +
> +	mask |= XFEATURE_MASK_AVX512;
> +	if (boot_cpu_has(X86_FEATURE_AVX512F)) {
> +		if ((fpu_kernel_cfg.max_features & mask) != mask) {

[Severity: High]
This is a pre-existing issue, but does this logic handle a malformed CPUID
reporting AVX512F=1 but AVX=0 correctly?

If the xstate leaf reports both YMM and AVX512 support, the AVX validation
block above is skipped because AVX=0. Since mask accumulates both YMM and
AVX512, the check here passes, so X86_FEATURE_AVX512F is not cleared.

Later in fpu__init_system_xstate(), the xsave_cpuid_features loop:

	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);
	}

will clear the YMM bit from max_features because AVX=0, but will retain the
AVX512 bits because AVX512F=1.

Could this result in passing an invalid xstate combination (AVX512 without
YMM) to xsetbv, triggering a #GP exception during boot?

> +			pr_err(FW_BUG
> +			       "x86/fpu: Disabling AVX-512 support due to missing xstate features\n");
> +			setup_clear_cpu_cap(X86_FEATURE_AVX512F);
> +		}
> +	}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1