On Fri, 10 Mar 2017, Andi Kleen wrote:
> From: Andi Kleen <a...@linux.intel.com>
>
> For performance testing it is useful to be able to disable AVX
> and AVX512. User programs check in XGETBV if AVX is supported
> by the OS. If we don't initialize the XSAVE state for AVX it will
> appear as if the OS is not supporting AVX. For kernel users we
> can also clear the internal cpu feature bits.
>
> This patch implements disable options for AVX and AVX512 for
> the XSAVE code.
>
> I originally considered a generic argument that would disable
> any XSAVE feature, but it turns out you need special code
> to also disable all the CPUID bits, because otherwise
> kernel code may assume it exists, when it doesn't. MPX
> already has an own disable flag. Not clear it is useful
> for the others. So we only do it for AVX/AVX512 for now.
Please read and follow Documentation/process/submitting-patches.rst.
Especially this paragraph:
Describe your changes in imperative mood, e.g. "make xyzzy do frotz"
instead of "[This patch] makes xyzzy do frotz" or "[I] changed xyzzy
to do frotz", as if you are giving orders to the codebase to change
its behaviour.
> + disable_avx [X86] Disable support for AVX on Intel CPUs.
So that command line option fails on AMD CPUs, right?
> + disable_avx512 [X86] Disable support for AVX512 on Intel CPUs.
Please drop the Intel stuff here as well. It's just a question of time
until AMD gets that as well.
> disable_1tb_segments [PPC]
> Disables the use of 1TB hash page table segments. This
> causes the kernel to fall back to 256MB segments which
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index c24ac1efb12d..cf75638ec657 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -16,6 +16,20 @@
>
> #include <asm/tlbflush.h>
>
> +enum xsave_features {
> + XSAVE_X87,
> + XSAVE_SSE,
> + XSAVE_AVX,
> + XSAVE_MPX_BOUNDS,
> + XSAVE_MPX_CSR,
> + XSAVE_AVX512_OPMASK,
> + XSAVE_AVX512_HI256,
> + XSAVE_AVX512_ZMM_HI256,
> + XSAVE_PT,
> + XSAVE_PKU,
> + XSAVE_UNKNOWN
> +};
What's that enum for? It's unused ....
> /*
> * Although we spell it out in here, the Processor Trace
> * xfeature is completely unused. We use other mechanisms
> @@ -41,6 +55,8 @@ static const char *xfeature_names[] =
> */
> u64 xfeatures_mask __read_mostly;
>
> +u64 xfeatures_disabled __initdata;
Why is this global?
> + xfeatures_mask &= ~xfeatures_disabled;
> xfeatures_mask &= fpu__get_supported_xfeatures_mask();
Thanks,
tglx
