From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755570AbdCKKqx (ORCPT ); Sat, 11 Mar 2017 05:46:53 -0500 Received: from Galois.linutronix.de ([146.0.238.70]:60662 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752792AbdCKKqo (ORCPT ); Sat, 11 Mar 2017 05:46:44 -0500 Date: Sat, 11 Mar 2017 11:46:37 +0100 (CET) From: Thomas Gleixner To: Andi Kleen cc: x86@kernel.org, linux-kernel@vger.kernel.org, Andi Kleen Subject: Re: [PATCH 2/2] x86/fpu: Support disabling AVX and AVX512 In-Reply-To: <20170311003241.13127-2-andi@firstfloor.org> Message-ID: References: <20170311003241.13127-1-andi@firstfloor.org> <20170311003241.13127-2-andi@firstfloor.org> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, 10 Mar 2017, Andi Kleen wrote: > From: Andi Kleen > > 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 > > +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