linux-kernel - Re: [PATCH 2/2] x86/fpu: Support disabling AVX and AVX512

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.DEB.2.20.1703111122400.3542@nanos>
Date:   Sat, 11 Mar 2017 11:46:37 +0100 (CET)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Andi Kleen <andi@...stfloor.org>
cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Andi Kleen <ak@...ux.intel.com>
Subject: Re: [PATCH 2/2] x86/fpu: Support disabling AVX and AVX512

On Fri, 10 Mar 2017, Andi Kleen wrote:
> From: Andi Kleen <ak@...ux.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