* Support generic disabling of all XSAVE features
@ 2017-06-21 23:41 Andi Kleen
2017-06-21 23:41 ` [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters Andi Kleen
` (4 more replies)
0 siblings, 5 replies; 10+ 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] 10+ messages in thread* [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters 2017-06-21 23:41 Support generic disabling of all XSAVE features Andi Kleen @ 2017-06-21 23:41 ` Andi Kleen 2017-06-21 23:41 ` [PATCH 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen ` (3 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2017-06-21 23:41 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Move the XSAVE initialization code to be after parsing early parameters. I don't see any reason why the FPU code needs to be initialized that early, nothing else in the initialization phase uses XSAVE. This is useful to be able to handle command line parameters in the XSAVE initialization code. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/common.c | 1 - arch/x86/kernel/setup.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index c8b39870f33e..60b7aa6330ed 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -885,7 +885,6 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c) } setup_force_cpu_cap(X86_FEATURE_ALWAYS); - fpu__init_system(c); } void __init early_cpu_init(void) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index f81823695014..77db31315af3 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -91,6 +91,7 @@ #include <asm/processor.h> #include <asm/bugs.h> #include <asm/kasan.h> +#include <asm/fpu/internal.h> #include <asm/vsyscall.h> #include <asm/cpu.h> @@ -982,6 +983,8 @@ void __init setup_arch(char **cmdline_p) parse_early_param(); + fpu__init_system(&boot_cpu_data); + #ifdef CONFIG_MEMORY_HOTPLUG /* * Memory used by the kernel cannot be hot-removed because Linux -- 2.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/5] x86/cpuid: Add generic table for cpuid dependencies 2017-06-21 23:41 Support generic disabling of all XSAVE features Andi Kleen 2017-06-21 23:41 ` [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters Andi Kleen @ 2017-06-21 23:41 ` Andi Kleen 2017-06-22 9:28 ` Jonathan McDowell 2017-06-21 23:41 ` [PATCH 3/5] x86/cpuid: Make clearcpuid an early param Andi Kleen ` (2 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Andi Kleen @ 2017-06-21 23:41 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen 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_id for lguest Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/include/asm/cpufeature.h | 8 +++- arch/x86/include/asm/cpufeatures.h | 5 +++ arch/x86/kernel/cpu/Makefile | 1 + arch/x86/kernel/cpu/cpuid-deps.c | 92 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 104 insertions(+), 2 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..e6145f383ff8 100644 --- a/arch/x86/include/asm/cpufeature.h +++ b/arch/x86/include/asm/cpufeature.h @@ -125,8 +125,12 @@ 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 { \ +#define __clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability)) + +extern void setup_clear_cpu_cap(int bit); +extern void clear_cpu_cap(struct cpuinfo_x86 *cpu, int bit); + +#define __setup_clear_cpu_cap(bit) do { \ clear_cpu_cap(&boot_cpu_data, bit); \ set_bit(bit, (unsigned long *)cpu_caps_cleared); \ } while (0) diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h index 2701e5f8145b..8f371a5966e7 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 52000010c62e..274fc0fee1e1 100644 --- a/arch/x86/kernel/cpu/Makefile +++ b/arch/x86/kernel/cpu/Makefile @@ -21,6 +21,7 @@ obj-y += common.o obj-y += rdrand.o obj-y += match.o obj-y += bugs.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..08aff02cf2ff --- /dev/null +++ b/arch/x86/kernel/cpu/cpuid-deps.c @@ -0,0 +1,92 @@ +/* Declare dependencies between CPUIDs */ +#include <linux/kernel.h> +#include <linux/init.h> +#include <linux/module.h> +#include <asm/cpufeature.h> + +struct cpuid_dep { + int feature; + int dep; +}; + +/* + * 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). + */ +const static struct cpuid_dep cpuid_deps[] = { + { X86_FEATURE_XSAVE, 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_AVX512F }, + { X86_FEATURE_XSAVE, X86_FEATURE_PKU }, + { X86_FEATURE_XSAVE, X86_FEATURE_MPX }, + { X86_FEATURE_XSAVE, X86_FEATURE_XGETBV1 }, + { X86_FEATURE_XMM, X86_FEATURE_XMM2 }, + { X86_FEATURE_XMM2, 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_FMA, X86_FEATURE_AVX }, + { X86_FEATURE_XMM2, X86_FEATURE_F16C }, + { X86_FEATURE_XMM2, X86_FEATURE_AES }, + { X86_FEATURE_XSAVE, X86_FEATURE_AVX }, + { X86_FEATURE_XSAVE, X86_FEATURE_AVX512F }, + { X86_FEATURE_AVX512F, 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_AVX, X86_FEATURE_AVX2 }, + {} +}; + +static void do_clear_cpu_cap(struct cpuinfo_x86 *cpu, int feat) +{ + int i, newfeat; + bool changed; + + if (!cpu) + __setup_clear_cpu_cap(feat); + else + __clear_cpu_cap(cpu, feat); +again: + changed = false; + for (i = 0; cpuid_deps[i].feature; i++) { + if (feat == cpuid_deps[i].feature) { + newfeat = cpuid_deps[i].dep; + if (!cpu) + __setup_clear_cpu_cap(newfeat); + else + __clear_cpu_cap(cpu, newfeat); + changed = true; + } + } + /* Handle multi-level dependencies */ + if (changed) { + feat = newfeat; + goto again; + } +} + +void clear_cpu_cap(struct cpuinfo_x86 *cpu, int feat) +{ + do_clear_cpu_cap(cpu, feat); +} + +EXPORT_SYMBOL_GPL(clear_cpu_cap); + +void setup_clear_cpu_cap(int feat) +{ + do_clear_cpu_cap(NULL, feat); +} -- 2.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/5] x86/cpuid: Add generic table for cpuid dependencies 2017-06-21 23:41 ` [PATCH 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen @ 2017-06-22 9:28 ` Jonathan McDowell 0 siblings, 0 replies; 10+ messages in thread From: Jonathan McDowell @ 2017-06-22 9:28 UTC (permalink / raw) To: andi; +Cc: linux-kernel In article <20170621234106.16548-3-andi@firstfloor.org> you wrote: > 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_id for lguest > Signed-off-by: Andi Kleen <ak@linux.intel.com> > --- > arch/x86/include/asm/cpufeature.h | 8 +++- > arch/x86/include/asm/cpufeatures.h | 5 +++ > arch/x86/kernel/cpu/Makefile | 1 + > arch/x86/kernel/cpu/cpuid-deps.c | 92 ++++++++++++++++++++++++++++++++++++++ > 4 files changed, 104 insertions(+), 2 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..e6145f383ff8 100644 > --- a/arch/x86/include/asm/cpufeature.h > +++ b/arch/x86/include/asm/cpufeature.h > @@ -125,8 +125,12 @@ 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 { \ > +#define __clear_cpu_cap(c, bit) clear_bit(bit, (unsigned long *)((c)->x86_capability)) > + > +extern void setup_clear_cpu_cap(int bit); > +extern void clear_cpu_cap(struct cpuinfo_x86 *cpu, int bit); > + > +#define __setup_clear_cpu_cap(bit) do { \ > clear_cpu_cap(&boot_cpu_data, bit); \ > set_bit(bit, (unsigned long *)cpu_caps_cleared); \ > } while (0) > diff --git a/arch/x86/include/asm/cpufeatures.h b/arch/x86/include/asm/cpufeatures.h > index 2701e5f8145b..8f371a5966e7 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 52000010c62e..274fc0fee1e1 100644 > --- a/arch/x86/kernel/cpu/Makefile > +++ b/arch/x86/kernel/cpu/Makefile > @@ -21,6 +21,7 @@ obj-y += common.o > obj-y += rdrand.o > obj-y += match.o > obj-y += bugs.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..08aff02cf2ff > --- /dev/null > +++ b/arch/x86/kernel/cpu/cpuid-deps.c > @@ -0,0 +1,92 @@ > +/* Declare dependencies between CPUIDs */ > +#include <linux/kernel.h> > +#include <linux/init.h> > +#include <linux/module.h> > +#include <asm/cpufeature.h> > + > +struct cpuid_dep { > + int feature; > + int dep; > +}; This seems a little confusing; I had to read through a couple of times to understand that "dep" represents the feature that needs to be disabled if "feature" is disabled, rather than dep being a dependency of feature. > + > +/* > + * 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). > + */ > +const static struct cpuid_dep cpuid_deps[] = { > + { X86_FEATURE_XSAVE, 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_AVX512F }, > + { X86_FEATURE_XSAVE, X86_FEATURE_PKU }, > + { X86_FEATURE_XSAVE, X86_FEATURE_MPX }, > + { X86_FEATURE_XSAVE, X86_FEATURE_XGETBV1 }, > + { X86_FEATURE_XMM, X86_FEATURE_XMM2 }, > + { X86_FEATURE_XMM2, 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_FMA, X86_FEATURE_AVX }, > + { X86_FEATURE_XMM2, X86_FEATURE_F16C }, > + { X86_FEATURE_XMM2, X86_FEATURE_AES }, > + { X86_FEATURE_XSAVE, X86_FEATURE_AVX }, > + { X86_FEATURE_XSAVE, X86_FEATURE_AVX512F }, Duplicates from above. (Might it be a better idea for the table to be sorted to ease spotting such things and future patch merging?) > + { X86_FEATURE_AVX512F, 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_AVX, X86_FEATURE_AVX2 }, > + {} > +}; > + > +static void do_clear_cpu_cap(struct cpuinfo_x86 *cpu, int feat) > +{ > + int i, newfeat; > + bool changed; > + > + if (!cpu) > + __setup_clear_cpu_cap(feat); > + else > + __clear_cpu_cap(cpu, feat); > +again: > + changed = false; > + for (i = 0; cpuid_deps[i].feature; i++) { > + if (feat == cpuid_deps[i].feature) { > + newfeat = cpuid_deps[i].dep; > + if (!cpu) > + __setup_clear_cpu_cap(newfeat); > + else > + __clear_cpu_cap(cpu, newfeat); > + changed = true; > + } > + } > + /* Handle multi-level dependencies */ > + if (changed) { > + feat = newfeat; > + goto again; > + } I don't think this works? You're only picking up the last newfeat to process again. So if I disable X86_FEATURE_XSAVE the last newfeat will be X86_FEATURE_AVX512F and the code won't correctly disable the features which require X86_FEATURE_AVX? > +} > + > +void clear_cpu_cap(struct cpuinfo_x86 *cpu, int feat) > +{ > + do_clear_cpu_cap(cpu, feat); > +} > + > +EXPORT_SYMBOL_GPL(clear_cpu_cap); > + > +void setup_clear_cpu_cap(int feat) > +{ > + do_clear_cpu_cap(NULL, feat); > +} J. -- Web [ < fivemack> it is bruter-force than a really really stupid ] site: http:// [ elephant [on his Python suduku solver] ] Made by www.earth.li/~noodles/ [ ] HuggieTag 0.0.24 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] x86/cpuid: Make clearcpuid an early param 2017-06-21 23:41 Support generic disabling of all XSAVE features Andi Kleen 2017-06-21 23:41 ` [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters Andi Kleen 2017-06-21 23:41 ` [PATCH 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen @ 2017-06-21 23:41 ` Andi Kleen 2017-06-21 23:41 ` [PATCH 4/5] x86/xsave: Make XSAVE check the base CPUID features before enabling Andi Kleen 2017-06-21 23:41 ` [PATCH 5/5] x86/xsave: Using generic CPUID clearing when disabling XSAVE Andi Kleen 4 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2017-06-21 23:41 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Make clearcpuid= an early param, to make sure it is parsed before the XSAVE initialization. This allows to modify XSAVE state by clearing specific CPUID bits. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index 60b7aa6330ed..600e0cf714b9 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -1285,7 +1285,7 @@ static __init int setup_disablecpuid(char *arg) return 1; } -__setup("clearcpuid=", setup_disablecpuid); +early_param("clearcpuid", setup_disablecpuid); #ifdef CONFIG_X86_64 struct desc_ptr idt_descr __ro_after_init = { -- 2.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 4/5] x86/xsave: Make XSAVE check the base CPUID features before enabling 2017-06-21 23:41 Support generic disabling of all XSAVE features Andi Kleen ` (2 preceding siblings ...) 2017-06-21 23:41 ` [PATCH 3/5] x86/cpuid: Make clearcpuid an early param Andi Kleen @ 2017-06-21 23:41 ` Andi Kleen 2017-06-21 23:41 ` [PATCH 5/5] x86/xsave: Using generic CPUID clearing when disabling XSAVE Andi Kleen 4 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2017-06-21 23:41 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. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/fpu/xstate.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index c24ac1efb12d..8f0b96320cc9 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[] = { + 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: */ @@ -702,6 +716,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; @@ -735,6 +750,13 @@ 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.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 5/5] x86/xsave: Using generic CPUID clearing when disabling XSAVE 2017-06-21 23:41 Support generic disabling of all XSAVE features Andi Kleen ` (3 preceding siblings ...) 2017-06-21 23:41 ` [PATCH 4/5] x86/xsave: Make XSAVE check the base CPUID features before enabling Andi Kleen @ 2017-06-21 23:41 ` Andi Kleen 4 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2017-06-21 23:41 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Now that the generic CPUID clearing understands dependencies, it's enough to clear the XSAVE CPUID bit to clear all depending features when XSAVE gets disabled. So we don't need this hard to maintain explicit list of features depending on XSAVE anymore. Just call the generic clear_cpu_cap() function for XSAVE. 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 8f0b96320cc9..73c4fda76d32 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.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters @ 2017-06-05 22:59 Andi Kleen 2017-06-06 7:18 ` Thomas Gleixner 0 siblings, 1 reply; 10+ messages in thread From: Andi Kleen @ 2017-06-05 22:59 UTC (permalink / raw) To: x86; +Cc: linux-kernel, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Move the XSAVE initialization code to be after parsing early parameters. I don't see any reason why the FPU code needs to be initialized that early, nothing else in the initialization phase uses XSAVE. This is useful to be able to handle command line parameters in the XSAVE initialization code. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/common.c | 1 - arch/x86/kernel/setup.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index c8b39870f33e..60b7aa6330ed 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -885,7 +885,6 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c) } setup_force_cpu_cap(X86_FEATURE_ALWAYS); - fpu__init_system(c); } void __init early_cpu_init(void) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index f81823695014..77db31315af3 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -91,6 +91,7 @@ #include <asm/processor.h> #include <asm/bugs.h> #include <asm/kasan.h> +#include <asm/fpu/internal.h> #include <asm/vsyscall.h> #include <asm/cpu.h> @@ -982,6 +983,8 @@ void __init setup_arch(char **cmdline_p) parse_early_param(); + fpu__init_system(&boot_cpu_data); + #ifdef CONFIG_MEMORY_HOTPLUG /* * Memory used by the kernel cannot be hot-removed because Linux -- 2.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters 2017-06-05 22:59 [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters Andi Kleen @ 2017-06-06 7:18 ` Thomas Gleixner 0 siblings, 0 replies; 10+ messages in thread From: Thomas Gleixner @ 2017-06-06 7:18 UTC (permalink / raw) To: Andi Kleen; +Cc: x86, linux-kernel, Andi Kleen Andi, can you please add a 0/N cover letter to patch series and a Vn after PATCH so series can be distinguished? Thanks, tglx ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters @ 2017-05-31 22:22 Andi Kleen 0 siblings, 0 replies; 10+ messages in thread From: Andi Kleen @ 2017-05-31 22:22 UTC (permalink / raw) To: x86; +Cc: linux-kernel, hpa, Andi Kleen From: Andi Kleen <ak@linux.intel.com> Move the XSAVE initialization code to be after parsing early parameters. I don't see any reason why the FPU code needs to be initialized that early, nothing else in the initialization phase uses XSAVE. This is useful to be able to handle command line parameters in the XSAVE initialization code. Signed-off-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/kernel/cpu/common.c | 1 - arch/x86/kernel/setup.c | 3 +++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c index c8b39870f33e..60b7aa6330ed 100644 --- a/arch/x86/kernel/cpu/common.c +++ b/arch/x86/kernel/cpu/common.c @@ -885,7 +885,6 @@ static void __init early_identify_cpu(struct cpuinfo_x86 *c) } setup_force_cpu_cap(X86_FEATURE_ALWAYS); - fpu__init_system(c); } void __init early_cpu_init(void) diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index f81823695014..77db31315af3 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -91,6 +91,7 @@ #include <asm/processor.h> #include <asm/bugs.h> #include <asm/kasan.h> +#include <asm/fpu/internal.h> #include <asm/vsyscall.h> #include <asm/cpu.h> @@ -982,6 +983,8 @@ void __init setup_arch(char **cmdline_p) parse_early_param(); + fpu__init_system(&boot_cpu_data); + #ifdef CONFIG_MEMORY_HOTPLUG /* * Memory used by the kernel cannot be hot-removed because Linux -- 2.9.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2017-06-22 10:03 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-06-21 23:41 Support generic disabling of all XSAVE features Andi Kleen 2017-06-21 23:41 ` [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters Andi Kleen 2017-06-21 23:41 ` [PATCH 2/5] x86/cpuid: Add generic table for cpuid dependencies Andi Kleen 2017-06-22 9:28 ` Jonathan McDowell 2017-06-21 23:41 ` [PATCH 3/5] x86/cpuid: Make clearcpuid an early param Andi Kleen 2017-06-21 23:41 ` [PATCH 4/5] x86/xsave: Make XSAVE check the base CPUID features before enabling Andi Kleen 2017-06-21 23:41 ` [PATCH 5/5] x86/xsave: Using generic CPUID clearing when disabling XSAVE Andi Kleen -- strict thread matches above, loose matches on Subject: below -- 2017-06-05 22:59 [PATCH 1/5] x86/xsave: Move xsave initialization to after parsing early parameters Andi Kleen 2017-06-06 7:18 ` Thomas Gleixner 2017-05-31 22:22 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).