lkml.org 
[lkml]   [2017]   [Mar]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH 2/2] x86/fpu: Support disabling AVX and AVX512
On Fri, 10 Mar 2017, Andi Kleen wrote:
> From: Andi Kleen <ak@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

\
 
 \ /
  Last update: 2017-03-11 11:47    [W:0.092 / U:0.080 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site