* Support generic disabling of all XSAVE features @ 2017-10-13 21:56 Andi Kleen 2017-10-13 21:56 ` [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h Andi Kleen ` (4 more replies) 0 siblings, 5 replies; 25+ messages in thread From: Andi Kleen @ 2017-10-13 21:56 UTC (permalink / raw) To: x86; +Cc: linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. v4: Rebase to latest tree. Repost. v5: Fix dependency algorithm. Clean dups in table. Rebase. v6: Rebase. No changes to code. v7: Rebase. No changes to code. v8: Address all review comments. Add Reviewed-bys. Dependency checker restructured for feature->dependency Add missing dependency for AVX->AVX512F v9: Remove redundant dependency AVX512F->XSAVE Add dependencies for SHA_NI->XMM, XMM->FXSR, FXSR_OPT->FXSR v10: Move clearcpuid= option parsing to early xsave parsing, and don't move the FPU initialization. Add new clear_bit32/set_bit32 Lots of smaller changes based on review feedback. ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h 2017-10-13 21:56 Support generic disabling of all XSAVE features Andi Kleen @ 2017-10-13 21:56 ` Andi Kleen 2017-10-17 15:14 ` Thomas Gleixner 2017-10-17 16:21 ` [tip:x86/fpu] bitops: Add clear/set_bit32() " tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen ` (3 subsequent siblings) 4 siblings, 2 replies; 25+ messages in thread From: Andi Kleen @ 2017-10-13 21:56 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Add two simple wrappers around set_bit/clear_bit that accept the common case of an u32 array. This avoids writing casts in all callers. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- include/linux/bitops.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/include/linux/bitops.h b/include/linux/bitops.h index 8fbe259b197c..36794f058ba6 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -227,6 +227,32 @@ static inline unsigned long __ffs64(u64 word) return __ffs((unsigned long)word); } +/* + * clear_bit32 - Clear a bit in memory for u32 array + * @nr: Bit to clear + * @addr: u32 * address of bitmap + * + * Same as clear_bit, but avoids needing casts for u32 arrays. + */ + +static __always_inline void clear_bit32(long nr, volatile u32 *addr) +{ + clear_bit(nr, (volatile unsigned long *)addr); +} + +/* + * set_bit32 - Set a bit in memory for u32 array + * @nr: Bit to clear + * @addr: u32 * address of bitmap + * + * Same as set_bit, but avoids needing casts for u32 arrays. + */ + +static __always_inline void set_bit32(long nr, volatile u32 *addr) +{ + set_bit(nr, (volatile unsigned long *)addr); +} + #ifdef __KERNEL__ #ifndef set_mask_bits -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h 2017-10-13 21:56 ` [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h Andi Kleen @ 2017-10-17 15:14 ` Thomas Gleixner 2017-10-17 16:21 ` [tip:x86/fpu] bitops: Add clear/set_bit32() " tip-bot for Andi Kleen 1 sibling, 0 replies; 25+ messages in thread From: Thomas Gleixner @ 2017-10-17 15:14 UTC (permalink / raw) To: Andi Kleen; +Cc: x86, linux-kernel, Andi Kleen On Fri, 13 Oct 2017, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > Add two simple wrappers around set_bit/clear_bit that accept > the common case of an u32 array. This avoids writing > casts in all callers. Nice. Reviewed-by: Thomas Gleixner <tglx@linutronix.de> ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tip:x86/fpu] bitops: Add clear/set_bit32() to linux/bitops.h 2017-10-13 21:56 ` [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h Andi Kleen 2017-10-17 15:14 ` Thomas Gleixner @ 2017-10-17 16:21 ` tip-bot for Andi Kleen 2017-11-05 15:53 ` Heiko Carstens 1 sibling, 1 reply; 25+ messages in thread From: tip-bot for Andi Kleen @ 2017-10-17 16:21 UTC (permalink / raw) To: linux-tip-commits; +Cc: peterz, hpa, mingo, linux-kernel, tglx, ak, torvalds Commit-ID: cbe96375025e14fc76f9ed42ee5225120d7210f8 Gitweb: https://git.kernel.org/tip/cbe96375025e14fc76f9ed42ee5225120d7210f8 Author: Andi Kleen <ak@linux.intel.com> AuthorDate: Fri, 13 Oct 2017 14:56:41 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 17 Oct 2017 17:14:56 +0200 bitops: Add clear/set_bit32() to linux/bitops.h Add two simple wrappers around set_bit/clear_bit() that accept the common case of an u32 array. This avoids writing casts in all callers. Signed-off-by: Andi Kleen <ak@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20171013215645.23166-2-andi@firstfloor.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- include/linux/bitops.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/include/linux/bitops.h b/include/linux/bitops.h index 8fbe259..36794f0 100644 --- a/include/linux/bitops.h +++ b/include/linux/bitops.h @@ -227,6 +227,32 @@ static inline unsigned long __ffs64(u64 word) return __ffs((unsigned long)word); } +/* + * clear_bit32 - Clear a bit in memory for u32 array + * @nr: Bit to clear + * @addr: u32 * address of bitmap + * + * Same as clear_bit, but avoids needing casts for u32 arrays. + */ + +static __always_inline void clear_bit32(long nr, volatile u32 *addr) +{ + clear_bit(nr, (volatile unsigned long *)addr); +} + +/* + * set_bit32 - Set a bit in memory for u32 array + * @nr: Bit to clear + * @addr: u32 * address of bitmap + * + * Same as set_bit, but avoids needing casts for u32 arrays. + */ + +static __always_inline void set_bit32(long nr, volatile u32 *addr) +{ + set_bit(nr, (volatile unsigned long *)addr); +} + #ifdef __KERNEL__ #ifndef set_mask_bits ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [tip:x86/fpu] bitops: Add clear/set_bit32() to linux/bitops.h 2017-10-17 16:21 ` [tip:x86/fpu] bitops: Add clear/set_bit32() " tip-bot for Andi Kleen @ 2017-11-05 15:53 ` Heiko Carstens 2017-11-05 17:40 ` Ingo Molnar 2017-11-05 18:01 ` Linus Torvalds 0 siblings, 2 replies; 25+ messages in thread From: Heiko Carstens @ 2017-11-05 15:53 UTC (permalink / raw) To: hpa, peterz, mingo, tglx, linux-kernel, torvalds, ak; +Cc: linux-tip-commits On Tue, Oct 17, 2017 at 09:21:46AM -0700, tip-bot for Andi Kleen wrote: > Commit-ID: cbe96375025e14fc76f9ed42ee5225120d7210f8 > Gitweb: https://git.kernel.org/tip/cbe96375025e14fc76f9ed42ee5225120d7210f8 > Author: Andi Kleen <ak@linux.intel.com> > AuthorDate: Fri, 13 Oct 2017 14:56:41 -0700 > Committer: Ingo Molnar <mingo@kernel.org> > CommitDate: Tue, 17 Oct 2017 17:14:56 +0200 > > bitops: Add clear/set_bit32() to linux/bitops.h > > Add two simple wrappers around set_bit/clear_bit() that accept > the common case of an u32 array. This avoids writing > casts in all callers. > > Signed-off-by: Andi Kleen <ak@linux.intel.com> > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Link: http://lkml.kernel.org/r/20171013215645.23166-2-andi@firstfloor.org > Signed-off-by: Ingo Molnar <mingo@kernel.org> > --- > include/linux/bitops.h | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) ... > + * set_bit32 - Set a bit in memory for u32 array > + * @nr: Bit to clear > + * @addr: u32 * address of bitmap > + * > + * Same as set_bit, but avoids needing casts for u32 arrays. > + */ > + > +static __always_inline void set_bit32(long nr, volatile u32 *addr) > +{ > + set_bit(nr, (volatile unsigned long *)addr); > +} This does not work at all on 64 bit big endian machines. If e.g. the array would contain only one 32 bit member set_bit() would write to whatever is behind the array. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tip:x86/fpu] bitops: Add clear/set_bit32() to linux/bitops.h 2017-11-05 15:53 ` Heiko Carstens @ 2017-11-05 17:40 ` Ingo Molnar 2017-11-05 18:01 ` Linus Torvalds 1 sibling, 0 replies; 25+ messages in thread From: Ingo Molnar @ 2017-11-05 17:40 UTC (permalink / raw) To: Heiko Carstens Cc: hpa, peterz, tglx, linux-kernel, torvalds, ak, linux-tip-commits * Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Tue, Oct 17, 2017 at 09:21:46AM -0700, tip-bot for Andi Kleen wrote: > > Commit-ID: cbe96375025e14fc76f9ed42ee5225120d7210f8 > > Gitweb: https://git.kernel.org/tip/cbe96375025e14fc76f9ed42ee5225120d7210f8 > > Author: Andi Kleen <ak@linux.intel.com> > > AuthorDate: Fri, 13 Oct 2017 14:56:41 -0700 > > Committer: Ingo Molnar <mingo@kernel.org> > > CommitDate: Tue, 17 Oct 2017 17:14:56 +0200 > > > > bitops: Add clear/set_bit32() to linux/bitops.h > > > > Add two simple wrappers around set_bit/clear_bit() that accept > > the common case of an u32 array. This avoids writing > > casts in all callers. > > > > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > > Cc: Peter Zijlstra <peterz@infradead.org> > > Link: http://lkml.kernel.org/r/20171013215645.23166-2-andi@firstfloor.org > > Signed-off-by: Ingo Molnar <mingo@kernel.org> > > --- > > include/linux/bitops.h | 26 ++++++++++++++++++++++++++ > > 1 file changed, 26 insertions(+) > > ... > > > + * set_bit32 - Set a bit in memory for u32 array > > + * @nr: Bit to clear > > + * @addr: u32 * address of bitmap > > + * > > + * Same as set_bit, but avoids needing casts for u32 arrays. > > + */ > > + > > +static __always_inline void set_bit32(long nr, volatile u32 *addr) > > +{ > > + set_bit(nr, (volatile unsigned long *)addr); > > +} > > This does not work at all on 64 bit big endian machines. If e.g. the array > would contain only one 32 bit member set_bit() would write to whatever is > behind the array. Yeah, indeed - this got reverted via: 1943dc07b45e: bitops: Revert cbe96375025e ("bitops: Add clear/set_bit32() to linux/bitops.h") Thanks, Ingo ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [tip:x86/fpu] bitops: Add clear/set_bit32() to linux/bitops.h 2017-11-05 15:53 ` Heiko Carstens 2017-11-05 17:40 ` Ingo Molnar @ 2017-11-05 18:01 ` Linus Torvalds 1 sibling, 0 replies; 25+ messages in thread From: Linus Torvalds @ 2017-11-05 18:01 UTC (permalink / raw) To: Heiko Carstens Cc: Peter Anvin, Peter Zijlstra, Ingo Molnar, Thomas Gleixner, Linux Kernel Mailing List, Andi Kleen, linux-tip-commits On Sun, Nov 5, 2017 at 7:53 AM, Heiko Carstens <heiko.carstens@de.ibm.com> wrote: > On Tue, Oct 17, 2017 at 09:21:46AM -0700, tip-bot for Andi Kleen wrote: >> >> bitops: Add clear/set_bit32() to linux/bitops.h > > This does not work at all on 64 bit big endian machines. If e.g. the array > would contain only one 32 bit member set_bit() would write to whatever is > behind the array. Oh, it's much worse than that. It's not that it would write beyond the array on big-endian, even if it were to be _in_ the array it simply writes the wrong bit entirely. And even on little-endian, it is complete and utter garbage. On little-endian, it writes the right bit - except when it doesn't work at all! The alignment may be wrong, and even on architectures that handle unaligned loads and stores perfectly well, _atomic_ unaligned may not work at all. x86 is actually unique in handling even unaligned atomics, and even on x86 it's generally something you absolutely need to avoid for performance reasons (most of the time it's fine, but atomics that cross cachelines or pages can be very very expensive). So that commit was beyond broken. It's dangerous and pure shit. There's a very good reason our bitops are defined for "unsigned long *", and why they do *not* take a "void *" or anything like that. Linus ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v10 2/5] x86/cpuid: Add generic table for cpuid dependencies 2017-10-13 21:56 Support generic disabling of all XSAVE features Andi Kleen 2017-10-13 21:56 ` [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h Andi Kleen @ 2017-10-13 21:56 ` Andi Kleen 2017-10-17 16:22 ` [tip:x86/fpu] x86/cpuid: Add generic table for CPUID dependencies tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument Andi Kleen ` (2 subsequent siblings) 4 siblings, 1 reply; 25+ messages in thread From: Andi Kleen @ 2017-10-13 21:56 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen, Jonathan McDowell From: Andi Kleen <ak@linux.intel.com> Some CPUID features depend on other features. Currently it's possible to to clear dependent features, but not clear the base features, which can cause various interesting problems. This patch implements a generic table to describe dependencies between CPUID features, to be used by all code that clears CPUID. Some subsystems (like XSAVE) had an own implementation of this, but it's better to do it all in a single place for everyone. Then clear_cpu_cap and setup_clear_cpu_cap always look up this table and clear all dependencies too. This is intended to be a practical table: only for features that make sense to clear. If someone for example clears FPU, or other features that are essentially part of the required base feature set, not much is going to work. Handling that is right now out of scope. We're only handling features which can be usefully cleared. v2: Add EXPORT_SYMBOL for clear_cpu_cap for lguest v3: Fix handling of depending issues Fix dups in the table (Jonathan McDowell) v4: Remove EXPORT_SYMBOL again as lguest is gone Restructure dependencies as feature, dependency (Thomas Gleixner) Move some code from header file to C file and turn macros into inlines (dito) Simplify if conditions (dito) Add missing dependency for AVX512F->AVX v5: Remove redundant dependency for AVX512F->XSAVE (Thomas Gleixner) Add missing dependency for XMM->FXSR (Thomas Gleixner) v6: Add SHA_NI->XMM and FXSR_OPT->FXSAVE dependencies v7: Use 32bit values in tables Add comment on why __init/__initdata is not possible Use unsigned int instead of unsigned. Rename feat to feature Rename cinfo to c Use DECLARE_BITMAP for bitmap and drop casts. Use clear/set_bit32 Add new line. Cc: Jonathan McDowell <noodles@earth.li> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/include/asm/cpufeature.h | 9 ++- arch/x86/include/asm/cpufeatures.h | 5 ++ arch/x86/kernel/cpu/Makefile | 1 + arch/x86/kernel/cpu/cpuid-deps.c | 113 +++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 5 deletions(-) create mode 100644 arch/x86/kernel/cpu/cpuid-deps.c diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h index d59c15c3defd..225fd8374fae 100644 --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -125,11 +125,10 @@ extern const char * const x86_bug_flags[NBUGINTS*32]; #define boot_cpu_has(bit) cpu_has(&boot_cpu_data, bit) #define set_cpu_cap(c, bit) set_bit(bit, (unsigned long *)((c)->x86_capability)) -#define clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability)) -#define setup_clear_cpu_cap(bit) do { \ - clear_cpu_cap(&boot_cpu_data, bit); \ - set_bit(bit, (unsigned long *)cpu_caps_cleared); \ -} while (0) + +extern void setup_clear_cpu_cap(unsigned int bit); +extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit); + #define setup_force_cpu_cap(bit) do { \ set_cpu_cap(&boot_cpu_data, bit); \ set_bit(bit, (unsigned long *)cpu_caps_set); \ diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 2519c6c801c9..401a70992060 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -21,6 +21,11 @@ * this feature bit is not displayed in /proc/cpuinfo at all. */ +/* + * When adding new features here that depend on other features, + * please update the table in kernel/cpu/cpuid-deps.c + */ + /* Intel-defined CPU features, CPUID level 0x00000001 (edx), word 0 */ #define X86_FEATURE_FPU ( 0*32+ 0) /* Onboard FPU */ #define X86_FEATURE_VME ( 0*32+ 1) /* Virtual Mode Extensions */ diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile index e17942c131c8..de260fae1017 100644 --- a/arch/x86/kernel/cpu/Makefile +++ b/arch/x86/kernel/cpu/Makefile @@ -22,6 +22,7 @@ obj-y += rdrand.o obj-y += match.o obj-y += bugs.o obj-$(CONFIG_CPU_FREQ) += aperfmperf.o +obj-y += cpuid-deps.o obj-$(CONFIG_PROC_FS) += proc.o obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c new file mode 100644 index 000000000000..e48eb7313120 --- /dev/null +++ b/arch/x86/kernel/cpu/cpuid-deps.c @@ -0,0 +1,113 @@ +/* Declare dependencies between CPUIDs */ +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/module.h> +#include <asm/cpufeature.h> + +struct cpuid_dep { + unsigned int feature; + unsigned int depends; +}; + +/* + * Table of CPUID features that depend on others. + * + * This only includes dependencies that can be usefully disabled, not + * features part of the base set (like FPU). + * + * Note this all is not __init / __initdata because it can be + * called from cpu hotplug. It shouldn't do anything in this case, + * but it's difficult to tell that to the init reference checker. + */ +const static struct cpuid_dep cpuid_deps[] = { + { X86_FEATURE_XSAVEOPT, X86_FEATURE_XSAVE }, + { X86_FEATURE_XSAVEC, X86_FEATURE_XSAVE }, + { X86_FEATURE_XSAVES, X86_FEATURE_XSAVE }, + { X86_FEATURE_AVX, X86_FEATURE_XSAVE }, + { X86_FEATURE_PKU, X86_FEATURE_XSAVE }, + { X86_FEATURE_MPX, X86_FEATURE_XSAVE }, + { X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE }, + { X86_FEATURE_FXSR_OPT, X86_FEATURE_FXSR }, + { X86_FEATURE_XMM, X86_FEATURE_FXSR }, + { X86_FEATURE_XMM2, X86_FEATURE_XMM }, + { X86_FEATURE_XMM3, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM4_1, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM4_2, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM3, X86_FEATURE_XMM2 }, + { X86_FEATURE_PCLMULQDQ, X86_FEATURE_XMM2 }, + { X86_FEATURE_SSSE3, X86_FEATURE_XMM2, }, + { X86_FEATURE_F16C, X86_FEATURE_XMM2, }, + { X86_FEATURE_AES, X86_FEATURE_XMM2 }, + { X86_FEATURE_SHA_NI, X86_FEATURE_XMM2 }, + { X86_FEATURE_FMA, X86_FEATURE_AVX }, + { X86_FEATURE_AVX2, X86_FEATURE_AVX, }, + { X86_FEATURE_AVX512F, X86_FEATURE_AVX, }, + { X86_FEATURE_AVX512IFMA, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512PF, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512ER, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512CD, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512DQ, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512BW, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512VL, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512VBMI, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512_4VNNIW, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512_4FMAPS, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512_VPOPCNTDQ, X86_FEATURE_AVX512F }, + {} +}; + +static inline void __clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit) +{ + clear_bit32(bit, c->x86_capability); +} + +static inline void __setup_clear_cpu_cap(unsigned int bit) +{ + clear_cpu_cap(&boot_cpu_data, bit); + set_bit32(bit, cpu_caps_cleared); +} + +static inline void clear_feature(struct cpuinfo_x86 *c, unsigned int feature) +{ + if (!c) + __setup_clear_cpu_cap(feature); + else + __clear_cpu_cap(c, feature); +} + +static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature) +{ + bool changed; + DECLARE_BITMAP(disable, NCAPINTS * sizeof(u32) * 8); + const struct cpuid_dep *d; + + clear_feature(c, feature); + + /* Collect all features to disable, handling dependencies */ + memset(disable, 0, sizeof(disable)); + __set_bit(feature, disable); + + /* Loop until we get a stable state. */ + do { + changed = false; + for (d = cpuid_deps; d->feature; d++) { + if (!test_bit(d->depends, disable)) + continue; + if (__test_and_set_bit(d->feature, disable)) + continue; + + changed = true; + clear_feature(c, d->feature); + } + } while (changed); +} + +void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature) +{ + do_clear_cpu_cap(c, feature); +} + +void setup_clear_cpu_cap(unsigned int feature) +{ + do_clear_cpu_cap(NULL, feature); +} -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [tip:x86/fpu] x86/cpuid: Add generic table for CPUID dependencies 2017-10-13 21:56 ` [PATCH v10 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen @ 2017-10-17 16:22 ` tip-bot for Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: tip-bot for Andi Kleen @ 2017-10-17 16:22 UTC (permalink / raw) To: linux-tip-commits Cc: noodles, tglx, linux-kernel, ak, torvalds, hpa, peterz, mingo Commit-ID: 0b00de857a648dafe7020878c7a27cf776f5edf4 Gitweb: https://git.kernel.org/tip/0b00de857a648dafe7020878c7a27cf776f5edf4 Author: Andi Kleen <ak@linux.intel.com> AuthorDate: Fri, 13 Oct 2017 14:56:42 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 17 Oct 2017 17:14:57 +0200 x86/cpuid: Add generic table for CPUID dependencies Some CPUID features depend on other features. Currently it's possible to to clear dependent features, but not clear the base features, which can cause various interesting problems. This patch implements a generic table to describe dependencies between CPUID features, to be used by all code that clears CPUID. Some subsystems (like XSAVE) had an own implementation of this, but it's better to do it all in a single place for everyone. Then clear_cpu_cap and setup_clear_cpu_cap always look up this table and clear all dependencies too. This is intended to be a practical table: only for features that make sense to clear. If someone for example clears FPU, or other features that are essentially part of the required base feature set, not much is going to work. Handling that is right now out of scope. We're only handling features which can be usefully cleared. Signed-off-by: Andi Kleen <ak@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: Jonathan McDowell <noodles@earth.li> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20171013215645.23166-3-andi@firstfloor.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/include/asm/cpufeature.h | 9 ++- arch/x86/include/asm/cpufeatures.h | 5 ++ arch/x86/kernel/cpu/Makefile | 1 + arch/x86/kernel/cpu/cpuid-deps.c | 113 +++++++++++++++++++++++++++++++++++++ 4 files changed, 123 insertions(+), 5 deletions(-) diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h index d59c15c..225fd83 100644 --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -125,11 +125,10 @@ extern const char * const x86_bug_flags[NBUGINTS*32]; #define boot_cpu_has(bit) cpu_has(&boot_cpu_data, bit) #define set_cpu_cap(c, bit) set_bit(bit, (unsigned long *)((c)->x86_capability)) -#define clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability)) -#define setup_clear_cpu_cap(bit) do { \ - clear_cpu_cap(&boot_cpu_data, bit); \ - set_bit(bit, (unsigned long *)cpu_caps_cleared); \ -} while (0) + +extern void setup_clear_cpu_cap(unsigned int bit); +extern void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit); + #define setup_force_cpu_cap(bit) do { \ set_cpu_cap(&boot_cpu_data, bit); \ set_bit(bit, (unsigned long *)cpu_caps_set); \ diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 2519c6c..401a709 100644 --- a/arch/x86/include/asm/cpufeatures.h +++ b/arch/x86/include/asm/cpufeatures.h @@ -21,6 +21,11 @@ * this feature bit is not displayed in /proc/cpuinfo at all. */ +/* + * When adding new features here that depend on other features, + * please update the table in kernel/cpu/cpuid-deps.c + */ + /* Intel-defined CPU features, CPUID level 0x00000001 (edx), word 0 */ #define X86_FEATURE_FPU ( 0*32+ 0) /* Onboard FPU */ #define X86_FEATURE_VME ( 0*32+ 1) /* Virtual Mode Extensions */ diff --git a/arch/x86/kernel/cpu/Makefile b/arch/x86/kernel/cpu/Makefile index e17942c..de260fa 100644 --- a/arch/x86/kernel/cpu/Makefile +++ b/arch/x86/kernel/cpu/Makefile @@ -22,6 +22,7 @@ obj-y += rdrand.o obj-y += match.o obj-y += bugs.o obj-$(CONFIG_CPU_FREQ) += aperfmperf.o +obj-y += cpuid-deps.o obj-$(CONFIG_PROC_FS) += proc.o obj-$(CONFIG_X86_FEATURE_NAMES) += capflags.o powerflags.o diff --git a/arch/x86/kernel/cpu/cpuid-deps.c b/arch/x86/kernel/cpu/cpuid-deps.c new file mode 100644 index 0000000..e48eb73 --- /dev/null +++ b/arch/x86/kernel/cpu/cpuid-deps.c @@ -0,0 +1,113 @@ +/* Declare dependencies between CPUIDs */ +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/module.h> +#include <asm/cpufeature.h> + +struct cpuid_dep { + unsigned int feature; + unsigned int depends; +}; + +/* + * Table of CPUID features that depend on others. + * + * This only includes dependencies that can be usefully disabled, not + * features part of the base set (like FPU). + * + * Note this all is not __init / __initdata because it can be + * called from cpu hotplug. It shouldn't do anything in this case, + * but it's difficult to tell that to the init reference checker. + */ +const static struct cpuid_dep cpuid_deps[] = { + { X86_FEATURE_XSAVEOPT, X86_FEATURE_XSAVE }, + { X86_FEATURE_XSAVEC, X86_FEATURE_XSAVE }, + { X86_FEATURE_XSAVES, X86_FEATURE_XSAVE }, + { X86_FEATURE_AVX, X86_FEATURE_XSAVE }, + { X86_FEATURE_PKU, X86_FEATURE_XSAVE }, + { X86_FEATURE_MPX, X86_FEATURE_XSAVE }, + { X86_FEATURE_XGETBV1, X86_FEATURE_XSAVE }, + { X86_FEATURE_FXSR_OPT, X86_FEATURE_FXSR }, + { X86_FEATURE_XMM, X86_FEATURE_FXSR }, + { X86_FEATURE_XMM2, X86_FEATURE_XMM }, + { X86_FEATURE_XMM3, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM4_1, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM4_2, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM3, X86_FEATURE_XMM2 }, + { X86_FEATURE_PCLMULQDQ, X86_FEATURE_XMM2 }, + { X86_FEATURE_SSSE3, X86_FEATURE_XMM2, }, + { X86_FEATURE_F16C, X86_FEATURE_XMM2, }, + { X86_FEATURE_AES, X86_FEATURE_XMM2 }, + { X86_FEATURE_SHA_NI, X86_FEATURE_XMM2 }, + { X86_FEATURE_FMA, X86_FEATURE_AVX }, + { X86_FEATURE_AVX2, X86_FEATURE_AVX, }, + { X86_FEATURE_AVX512F, X86_FEATURE_AVX, }, + { X86_FEATURE_AVX512IFMA, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512PF, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512ER, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512CD, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512DQ, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512BW, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512VL, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512VBMI, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512_4VNNIW, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512_4FMAPS, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512_VPOPCNTDQ, X86_FEATURE_AVX512F }, + {} +}; + +static inline void __clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int bit) +{ + clear_bit32(bit, c->x86_capability); +} + +static inline void __setup_clear_cpu_cap(unsigned int bit) +{ + clear_cpu_cap(&boot_cpu_data, bit); + set_bit32(bit, cpu_caps_cleared); +} + +static inline void clear_feature(struct cpuinfo_x86 *c, unsigned int feature) +{ + if (!c) + __setup_clear_cpu_cap(feature); + else + __clear_cpu_cap(c, feature); +} + +static void do_clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature) +{ + bool changed; + DECLARE_BITMAP(disable, NCAPINTS * sizeof(u32) * 8); + const struct cpuid_dep *d; + + clear_feature(c, feature); + + /* Collect all features to disable, handling dependencies */ + memset(disable, 0, sizeof(disable)); + __set_bit(feature, disable); + + /* Loop until we get a stable state. */ + do { + changed = false; + for (d = cpuid_deps; d->feature; d++) { + if (!test_bit(d->depends, disable)) + continue; + if (__test_and_set_bit(d->feature, disable)) + continue; + + changed = true; + clear_feature(c, d->feature); + } + } while (changed); +} + +void clear_cpu_cap(struct cpuinfo_x86 *c, unsigned int feature) +{ + do_clear_cpu_cap(c, feature); +} + +void setup_clear_cpu_cap(unsigned int feature) +{ + do_clear_cpu_cap(NULL, feature); +} ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument 2017-10-13 21:56 Support generic disabling of all XSAVE features Andi Kleen 2017-10-13 21:56 ` [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h Andi Kleen 2017-10-13 21:56 ` [PATCH v10 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen @ 2017-10-13 21:56 ` Andi Kleen 2017-10-17 15:15 ` Thomas Gleixner 2017-10-17 16:22 ` [tip:x86/fpu] " tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 4/5] x86/fpu: Make XSAVE check the base CPUID features before enabling Andi Kleen 2017-10-13 21:56 ` [PATCH v10 5/5] x86/fpu: Remove the explicit clearing of XSAVE dependend features Andi Kleen 4 siblings, 2 replies; 25+ messages in thread From: Andi Kleen @ 2017-10-13 21:56 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> With a followon patch we want to make clearcpuid affect the XSAVE configuration. But xsave is currently initialized before arguments are parsed. Move the clearcpuid= parsing into the special early xsave argument parsing code. Since clearcpuid= contains a = we need to keep the old __setup around as a dummy, otherwise it would end up as a environment variable in init's environment. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/common.c | 16 +++++++--------- arch/x86/kernel/fpu/init.c | 11 +++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index c9176bae7fd8..03bb004bb15e 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1301,18 +1301,16 @@ void print_cpu_info(struct cpuinfo_x86 *c) pr_cont(")\n"); } -static __init int setup_disablecpuid(char *arg) +/* + * clearcpuid= was already parsed in fpu__init_parse_early_param. + * But we need to keep a dummy __setup around otherwise it would + * show up as an environment variable for init. + */ +static __init int setup_clearcpuid(char *arg) { - int bit; - - if (get_option(&arg, &bit) && bit >= 0 && bit < NCAPINTS * 32) - setup_clear_cpu_cap(bit); - else - return 0; - return 1; } -__setup("clearcpuid=", setup_disablecpuid); +__setup("clearcpuid=", setup_clearcpuid); #ifdef CONFIG_X86_64 DEFINE_PER_CPU_FIRST(union irq_stack_union, diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c index 7affb7e3d9a5..6abd83572b01 100644 --- a/arch/x86/kernel/fpu/init.c +++ b/arch/x86/kernel/fpu/init.c @@ -249,6 +249,10 @@ static void __init fpu__init_system_ctx_switch(void) */ static void __init fpu__init_parse_early_param(void) { + char arg[32]; + char *argptr = arg; + int bit; + if (cmdline_find_option_bool(boot_command_line, "no387")) setup_clear_cpu_cap(X86_FEATURE_FPU); @@ -266,6 +270,13 @@ static void __init fpu__init_parse_early_param(void) if (cmdline_find_option_bool(boot_command_line, "noxsaves")) setup_clear_cpu_cap(X86_FEATURE_XSAVES); + + if (cmdline_find_option(boot_command_line, "clearcpuid", arg, + sizeof(arg)) && + get_option(&argptr, &bit) && + bit >= 0 && + bit < NCAPINTS * 32) + setup_clear_cpu_cap(bit); } /* -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument 2017-10-13 21:56 ` [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument Andi Kleen @ 2017-10-17 15:15 ` Thomas Gleixner 2017-10-17 15:19 ` Ingo Molnar 2017-10-17 16:22 ` [tip:x86/fpu] " tip-bot for Andi Kleen 1 sibling, 1 reply; 25+ messages in thread From: Thomas Gleixner @ 2017-10-17 15:15 UTC (permalink / raw) To: Andi Kleen; +Cc: x86, linux-kernel, Andi Kleen On Fri, 13 Oct 2017, Andi Kleen wrote: > From: Andi Kleen <ak@linux.intel.com> > > With a followon patch we want to make clearcpuid affect the XSAVE > configuration. But xsave is currently initialized before arguments > are parsed. Move the clearcpuid= parsing into the special > early xsave argument parsing code. > > Since clearcpuid= contains a = we need to keep the old __setup > around as a dummy, otherwise it would end up as a environment > variable in init's environment. > > Signed-off-by: Andi Kleen <ak@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument 2017-10-17 15:15 ` Thomas Gleixner @ 2017-10-17 15:19 ` Ingo Molnar 0 siblings, 0 replies; 25+ messages in thread From: Ingo Molnar @ 2017-10-17 15:19 UTC (permalink / raw) To: Thomas Gleixner Cc: Andi Kleen, x86, linux-kernel, Andi Kleen, H. Peter Anvin, Thomas Gleixner, Peter Zijlstra * Thomas Gleixner <tglx@linutronix.de> wrote: > On Fri, 13 Oct 2017, Andi Kleen wrote: > > > From: Andi Kleen <ak@linux.intel.com> > > > > With a followon patch we want to make clearcpuid affect the XSAVE > > configuration. But xsave is currently initialized before arguments > > are parsed. Move the clearcpuid= parsing into the special > > early xsave argument parsing code. > > > > Since clearcpuid= contains a = we need to keep the old __setup > > around as a dummy, otherwise it would end up as a environment > > variable in init's environment. > > > > Signed-off-by: Andi Kleen <ak@linux.intel.com> > > Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Thanks guys, I have applied the five patches to tip:x86/fpu and will push it all out once it's gone through a bit of testing. Thanks, Ingo ^ permalink raw reply [flat|nested] 25+ messages in thread
* [tip:x86/fpu] x86/fpu: Parse clearcpuid= as early XSAVE argument 2017-10-13 21:56 ` [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument Andi Kleen 2017-10-17 15:15 ` Thomas Gleixner @ 2017-10-17 16:22 ` tip-bot for Andi Kleen 1 sibling, 0 replies; 25+ messages in thread From: tip-bot for Andi Kleen @ 2017-10-17 16:22 UTC (permalink / raw) To: linux-tip-commits; +Cc: mingo, peterz, torvalds, linux-kernel, hpa, ak, tglx Commit-ID: 0c2a3913d6f50503f7c59d83a6219e39508cc898 Gitweb: https://git.kernel.org/tip/0c2a3913d6f50503f7c59d83a6219e39508cc898 Author: Andi Kleen <ak@linux.intel.com> AuthorDate: Fri, 13 Oct 2017 14:56:43 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 17 Oct 2017 17:14:57 +0200 x86/fpu: Parse clearcpuid= as early XSAVE argument With a followon patch we want to make clearcpuid affect the XSAVE configuration. But xsave is currently initialized before arguments are parsed. Move the clearcpuid= parsing into the special early xsave argument parsing code. Since clearcpuid= contains a = we need to keep the old __setup around as a dummy, otherwise it would end up as a environment variable in init's environment. Signed-off-by: Andi Kleen <ak@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20171013215645.23166-4-andi@firstfloor.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/cpu/common.c | 16 +++++++--------- arch/x86/kernel/fpu/init.c | 11 +++++++++++ 2 files changed, 18 insertions(+), 9 deletions(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index c9176ba..03bb004 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1301,18 +1301,16 @@ void print_cpu_info(struct cpuinfo_x86 *c) pr_cont(")\n"); } -static __init int setup_disablecpuid(char *arg) +/* + * clearcpuid= was already parsed in fpu__init_parse_early_param. + * But we need to keep a dummy __setup around otherwise it would + * show up as an environment variable for init. + */ +static __init int setup_clearcpuid(char *arg) { - int bit; - - if (get_option(&arg, &bit) && bit >= 0 && bit < NCAPINTS * 32) - setup_clear_cpu_cap(bit); - else - return 0; - return 1; } -__setup("clearcpuid=", setup_disablecpuid); +__setup("clearcpuid=", setup_clearcpuid); #ifdef CONFIG_X86_64 DEFINE_PER_CPU_FIRST(union irq_stack_union, diff --git a/arch/x86/kernel/fpu/init.c b/arch/x86/kernel/fpu/init.c index 7affb7e..6abd835 100644 --- a/arch/x86/kernel/fpu/init.c +++ b/arch/x86/kernel/fpu/init.c @@ -249,6 +249,10 @@ static void __init fpu__init_system_ctx_switch(void) */ static void __init fpu__init_parse_early_param(void) { + char arg[32]; + char *argptr = arg; + int bit; + if (cmdline_find_option_bool(boot_command_line, "no387")) setup_clear_cpu_cap(X86_FEATURE_FPU); @@ -266,6 +270,13 @@ static void __init fpu__init_parse_early_param(void) if (cmdline_find_option_bool(boot_command_line, "noxsaves")) setup_clear_cpu_cap(X86_FEATURE_XSAVES); + + if (cmdline_find_option(boot_command_line, "clearcpuid", arg, + sizeof(arg)) && + get_option(&argptr, &bit) && + bit >= 0 && + bit < NCAPINTS * 32) + setup_clear_cpu_cap(bit); } /* ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v10 4/5] x86/fpu: Make XSAVE check the base CPUID features before enabling 2017-10-13 21:56 Support generic disabling of all XSAVE features Andi Kleen ` (2 preceding siblings ...) 2017-10-13 21:56 ` [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument Andi Kleen @ 2017-10-13 21:56 ` Andi Kleen 2017-10-17 16:23 ` [tip:x86/fpu] " tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 5/5] x86/fpu: Remove the explicit clearing of XSAVE dependend features Andi Kleen 4 siblings, 1 reply; 25+ messages in thread From: Andi Kleen @ 2017-10-13 21:56 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen 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) v3: Mark xsave_cpuid_features __initdata Reviewed-by: Thomas Gleixner <tglx@linutronix.de> 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..fb581292975b 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[] __initdata = { + 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: */ -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [tip:x86/fpu] x86/fpu: Make XSAVE check the base CPUID features before enabling 2017-10-13 21:56 ` [PATCH v10 4/5] x86/fpu: Make XSAVE check the base CPUID features before enabling Andi Kleen @ 2017-10-17 16:23 ` tip-bot for Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: tip-bot for Andi Kleen @ 2017-10-17 16:23 UTC (permalink / raw) To: linux-tip-commits; +Cc: peterz, linux-kernel, torvalds, hpa, mingo, ak, tglx Commit-ID: ccb18db2ab9d923df07e7495123fe5fb02329713 Gitweb: https://git.kernel.org/tip/ccb18db2ab9d923df07e7495123fe5fb02329713 Author: Andi Kleen <ak@linux.intel.com> AuthorDate: Fri, 13 Oct 2017 14:56:44 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 17 Oct 2017 17:14:57 +0200 x86/fpu: Make XSAVE check the base CPUID features before enabling 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. Signed-off-by: Andi Kleen <ak@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20171013215645.23166-5-andi@firstfloor.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- 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 f1d5476..fb58129 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[] __initdata = { + 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: */ ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v10 5/5] x86/fpu: Remove the explicit clearing of XSAVE dependend features 2017-10-13 21:56 Support generic disabling of all XSAVE features Andi Kleen ` (3 preceding siblings ...) 2017-10-13 21:56 ` [PATCH v10 4/5] x86/fpu: Make XSAVE check the base CPUID features before enabling Andi Kleen @ 2017-10-13 21:56 ` Andi Kleen 2017-10-17 16:23 ` [tip:x86/fpu] x86/fpu: Remove the explicit clearing of XSAVE dependent features tip-bot for Andi Kleen 4 siblings, 1 reply; 25+ messages in thread From: Andi Kleen @ 2017-10-13 21:56 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Clearing a CPU feature with setup_clear_cpu_cap() clears all features which depend on it. Expressing feature dependencies in one place is easier to maintain than keeping functions like fpu__xstate_clear_all_cpu_caps() up to date. The features which depend on XSAVE have their dependency expressed in the dependency table, so its sufficient to clear X86_FEATURE_XSAVE. Remove the explicit clearing of XSAVE dependend features. v2: Use Thomas Gleixner's new description Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/fpu/xstate.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index fb581292975b..87a57b7642d3 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -73,26 +73,6 @@ unsigned int fpu_user_xstate_size; void fpu__xstate_clear_all_cpu_caps(void) { setup_clear_cpu_cap(X86_FEATURE_XSAVE); - setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT); - setup_clear_cpu_cap(X86_FEATURE_XSAVEC); - setup_clear_cpu_cap(X86_FEATURE_XSAVES); - setup_clear_cpu_cap(X86_FEATURE_AVX); - setup_clear_cpu_cap(X86_FEATURE_AVX2); - setup_clear_cpu_cap(X86_FEATURE_AVX512F); - setup_clear_cpu_cap(X86_FEATURE_AVX512IFMA); - setup_clear_cpu_cap(X86_FEATURE_AVX512PF); - setup_clear_cpu_cap(X86_FEATURE_AVX512ER); - setup_clear_cpu_cap(X86_FEATURE_AVX512CD); - setup_clear_cpu_cap(X86_FEATURE_AVX512DQ); - setup_clear_cpu_cap(X86_FEATURE_AVX512BW); - setup_clear_cpu_cap(X86_FEATURE_AVX512VL); - setup_clear_cpu_cap(X86_FEATURE_MPX); - setup_clear_cpu_cap(X86_FEATURE_XGETBV1); - setup_clear_cpu_cap(X86_FEATURE_AVX512VBMI); - setup_clear_cpu_cap(X86_FEATURE_PKU); - setup_clear_cpu_cap(X86_FEATURE_AVX512_4VNNIW); - setup_clear_cpu_cap(X86_FEATURE_AVX512_4FMAPS); - setup_clear_cpu_cap(X86_FEATURE_AVX512_VPOPCNTDQ); } /* -- 2.13.6 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [tip:x86/fpu] x86/fpu: Remove the explicit clearing of XSAVE dependent features 2017-10-13 21:56 ` [PATCH v10 5/5] x86/fpu: Remove the explicit clearing of XSAVE dependend features Andi Kleen @ 2017-10-17 16:23 ` tip-bot for Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: tip-bot for Andi Kleen @ 2017-10-17 16:23 UTC (permalink / raw) To: linux-tip-commits; +Cc: linux-kernel, ak, torvalds, mingo, hpa, tglx, peterz Commit-ID: 73e3a7d2a7c3be29a5a22b85026f6cfa5664267f Gitweb: https://git.kernel.org/tip/73e3a7d2a7c3be29a5a22b85026f6cfa5664267f Author: Andi Kleen <ak@linux.intel.com> AuthorDate: Fri, 13 Oct 2017 14:56:45 -0700 Committer: Ingo Molnar <mingo@kernel.org> CommitDate: Tue, 17 Oct 2017 17:14:57 +0200 x86/fpu: Remove the explicit clearing of XSAVE dependent features Clearing a CPU feature with setup_clear_cpu_cap() clears all features which depend on it. Expressing feature dependencies in one place is easier to maintain than keeping functions like fpu__xstate_clear_all_cpu_caps() up to date. The features which depend on XSAVE have their dependency expressed in the dependency table, so its sufficient to clear X86_FEATURE_XSAVE. Remove the explicit clearing of XSAVE dependent features. Signed-off-by: Andi Kleen <ak@linux.intel.com> Reviewed-by: Thomas Gleixner <tglx@linutronix.de> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Link: http://lkml.kernel.org/r/20171013215645.23166-6-andi@firstfloor.org Signed-off-by: Ingo Molnar <mingo@kernel.org> --- arch/x86/kernel/fpu/xstate.c | 20 -------------------- 1 file changed, 20 deletions(-) diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index fb58129..87a57b7 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -73,26 +73,6 @@ unsigned int fpu_user_xstate_size; void fpu__xstate_clear_all_cpu_caps(void) { setup_clear_cpu_cap(X86_FEATURE_XSAVE); - setup_clear_cpu_cap(X86_FEATURE_XSAVEOPT); - setup_clear_cpu_cap(X86_FEATURE_XSAVEC); - setup_clear_cpu_cap(X86_FEATURE_XSAVES); - setup_clear_cpu_cap(X86_FEATURE_AVX); - setup_clear_cpu_cap(X86_FEATURE_AVX2); - setup_clear_cpu_cap(X86_FEATURE_AVX512F); - setup_clear_cpu_cap(X86_FEATURE_AVX512IFMA); - setup_clear_cpu_cap(X86_FEATURE_AVX512PF); - setup_clear_cpu_cap(X86_FEATURE_AVX512ER); - setup_clear_cpu_cap(X86_FEATURE_AVX512CD); - setup_clear_cpu_cap(X86_FEATURE_AVX512DQ); - setup_clear_cpu_cap(X86_FEATURE_AVX512BW); - setup_clear_cpu_cap(X86_FEATURE_AVX512VL); - setup_clear_cpu_cap(X86_FEATURE_MPX); - setup_clear_cpu_cap(X86_FEATURE_XGETBV1); - setup_clear_cpu_cap(X86_FEATURE_AVX512VBMI); - setup_clear_cpu_cap(X86_FEATURE_PKU); - setup_clear_cpu_cap(X86_FEATURE_AVX512_4VNNIW); - setup_clear_cpu_cap(X86_FEATURE_AVX512_4FMAPS); - setup_clear_cpu_cap(X86_FEATURE_AVX512_VPOPCNTDQ); } /* ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-10-07 0:03 Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-10-07 0:03 UTC (permalink / raw) To: x86; +Cc: linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. v4: Rebase to latest tree. Repost. v5: Fix dependency algorithm. Clean dups in table. Rebase. v6: Rebase. No changes to code. v7: Rebase. No changes to code. v8: Address all review comments. Add Reviewed-bys. Dependency checker restructured for feature->dependency Add missing dependency for AVX->AVX512F v9: Remove redundant dependency AVX512F->XSAVE Add dependencies for SHA_NI->XMM, XMM->FXSR, FXSR_OPT->FXSR ^ permalink raw reply [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-10-05 21:52 Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-10-05 21:52 UTC (permalink / raw) To: x86; +Cc: hpa, linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. v4: Rebase to latest tree. Repost. v5: Fix dependency algorithm. Clean dups in table. Rebase. v6: Rebase. No changes to code. v7: Rebase. No changes to code. v8: Address all review comments. Add Reviewed-bys. Dependency checker restructured for feature->dependency Add missing dependency for AVX->AVX512F ^ permalink raw reply [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-10-04 23:49 Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-10-04 23:49 UTC (permalink / raw) To: x86; +Cc: hpa, linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. v4: Rebase to latest tree. Repost. v5: Fix dependency algorithm. Clean dups in table. Rebase. v6: Rebase. No changes to code. v7: Rebase. No changes to code. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-09-19 22:26 Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-09-19 22:26 UTC (permalink / raw) To: x86; +Cc: hpa, linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. v4: Rebase to latest tree. Repost. v5: Fix dependency algorithm. Clean dups in table. Rebase. v6: Rebase. No changes to code. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-07-25 0:43 Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-07-25 0:43 UTC (permalink / raw) To: x86; +Cc: hpa, linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. v4: Rebase to latest tree. Repost. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-06-21 23:41 Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-06-21 23:41 UTC (permalink / raw) To: x86; +Cc: linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with clearcpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter v3: Repost. No changes to code. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Support generic disabling of all XSAVE features @ 2017-06-07 23:29 Andi Kleen 2017-06-13 0:17 ` Andi Kleen 0 siblings, 1 reply; 25+ messages in thread From: Andi Kleen @ 2017-06-07 23:29 UTC (permalink / raw) To: x86; +Cc: linux-kernel For performance testing and debugging it can be useful to disable XSAVE features individually. This patchkit hooks up XSAVE with the generic clearcpuid=... option, so that disabling a CPUID feature automatically disables the respective XSAVE feature. It also cleans up CPUID dependency management. Currently it's possible to generate configurations with cleacpuid that crash. It replaces an earlier patchkit that did this with special case options. v1: Initial post v2: Work around broken lguest by exporting set_cpu_cap Repost with cover letter ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: Support generic disabling of all XSAVE features 2017-06-07 23:29 Andi Kleen @ 2017-06-13 0:17 ` Andi Kleen 0 siblings, 0 replies; 25+ messages in thread From: Andi Kleen @ 2017-06-13 0:17 UTC (permalink / raw) To: x86; +Cc: linux-kernel Andi Kleen <andi@firstfloor.org> writes: Any comments on this patchkit? If there are no objections please merge. -Andi > For performance testing and debugging it can be useful to disable XSAVE > features individually. This patchkit hooks up XSAVE with the > generic clearcpuid=... option, so that disabling a CPUID feature > automatically disables the respective XSAVE feature. > > It also cleans up CPUID dependency management. Currently it's > possible to generate configurations with cleacpuid that crash. > > It replaces an earlier patchkit that did this with special > case options. > > v1: > Initial post > v2: > Work around broken lguest by exporting set_cpu_cap > Repost with cover letter ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2017-11-05 18:01 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-10-13 21:56 Support generic disabling of all XSAVE features Andi Kleen 2017-10-13 21:56 ` [PATCH v10 1/5] bitops: Add clear/set_bit32 to linux/bitops.h Andi Kleen 2017-10-17 15:14 ` Thomas Gleixner 2017-10-17 16:21 ` [tip:x86/fpu] bitops: Add clear/set_bit32() " tip-bot for Andi Kleen 2017-11-05 15:53 ` Heiko Carstens 2017-11-05 17:40 ` Ingo Molnar 2017-11-05 18:01 ` Linus Torvalds 2017-10-13 21:56 ` [PATCH v10 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen 2017-10-17 16:22 ` [tip:x86/fpu] x86/cpuid: Add generic table for CPUID dependencies tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 3/5] x86/fpu: Parse clearcpuid= as early XSAVE argument Andi Kleen 2017-10-17 15:15 ` Thomas Gleixner 2017-10-17 15:19 ` Ingo Molnar 2017-10-17 16:22 ` [tip:x86/fpu] " tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 4/5] x86/fpu: Make XSAVE check the base CPUID features before enabling Andi Kleen 2017-10-17 16:23 ` [tip:x86/fpu] " tip-bot for Andi Kleen 2017-10-13 21:56 ` [PATCH v10 5/5] x86/fpu: Remove the explicit clearing of XSAVE dependend features Andi Kleen 2017-10-17 16:23 ` [tip:x86/fpu] x86/fpu: Remove the explicit clearing of XSAVE dependent features tip-bot for Andi Kleen -- strict thread matches above, loose matches on Subject: below -- 2017-10-07 0:03 Support generic disabling of all XSAVE features Andi Kleen 2017-10-05 21:52 Andi Kleen 2017-10-04 23:49 Andi Kleen 2017-09-19 22:26 Andi Kleen 2017-07-25 0:43 Andi Kleen 2017-06-21 23:41 Andi Kleen 2017-06-07 23:29 Andi Kleen 2017-06-13 0:17 ` Andi Kleen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).