Re: [PATCH v9 4/5] x86/xsave: Make XSAVE check the base CPUID features before enabling

Ingo Molnar
2017-10-12T08:11:31Z

* Andi Kleen <andi@firstfloor.org> wrote:

> From: Andi Kleen <ak@linux.intel.com>
>
> Before enabling XSAVE, not only check the XSAVE specific CPUID bits,
> but also the base CPUID features of the respective XSAVE feature.
> This allows to disable individual XSAVE states using the existing
> clearcpuid= option, which can be useful for performance testing
> and debugging, and also in general avoids inconsistencies.
>
> v2:
> Add curly brackets (Thomas Gleixner)
> Signed-off-by: Andi Kleen <ak@linux.intel.com>
> ---
> arch/x86/kernel/fpu/xstate.c | 23 +++++++++++++++++++++++
> 1 file changed, 23 insertions(+)
>
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index f1d5476c9022..924bd895b5ee 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -15,6 +15,7 @@
> #include <asm/fpu/xstate.h>
>
> #include <asm/tlbflush.h>
> +#include <asm/cpufeature.h>
>
> /*
> * Although we spell it out in here, the Processor Trace
> @@ -36,6 +37,19 @@ static const char *xfeature_names[] =
> "unknown xstate feature" ,
> };
>
> +static short xsave_cpuid_features[] = {
> + X86_FEATURE_FPU,
> + X86_FEATURE_XMM,
> + X86_FEATURE_AVX,
> + X86_FEATURE_MPX,
> + X86_FEATURE_MPX,
> + X86_FEATURE_AVX512F,
> + X86_FEATURE_AVX512F,
> + X86_FEATURE_AVX512F,
> + X86_FEATURE_INTEL_PT,
> + X86_FEATURE_PKU,
> +};
> +
> /*
> * Mask of xstate features supported by the CPU and the kernel:
> */
> @@ -726,6 +740,7 @@ void __init fpu__init_system_xstate(void)
> unsigned int eax, ebx, ecx, edx;
> static int on_boot_cpu __initdata = 1;
> int err;
> + int i;
>
> WARN_ON_FPU(!on_boot_cpu);
> on_boot_cpu = 0;
> @@ -759,6 +774,14 @@ void __init fpu__init_system_xstate(void)
> goto out_disable;
> }
>
> + /*
> + * Clear XSAVE features that are disabled in the normal CPUID.
> + */
> + for (i = 0; i < ARRAY_SIZE(xsave_cpuid_features); i++) {
> + if (!boot_cpu_has(xsave_cpuid_features[i]))
> + xfeatures_mask &= ~BIT(i);
> + }
> +
> xfeatures_mask &= fpu__get_supported_xfeatures_mask();
>
> /* Enable xstate instructions to be able to continue with initialization: */

This patch has similar problems to the ones I reported against 2/5.

Thanks,

Ingo