Clean up x86 CPU feature setup
cover Clean up x86 CPU feature setup
x86 CPU feature flag setup has become impossible to debug. Every user just does set_bit()/clear_bit() or writes the entire set to change the flags, so there's no way to trace how they're being set.
This patchset creates an API and debug messages for tracking how the flags get set. It's not nearly done, but I want to know whether or not to continue.
RFC PATCH 1/3 x86: use defined names for all CPU feature flags
x86: use defined names for all CPU feature flags
Don't use hard coded values for CPU flags.
--- arch/i386/kernel/cpu/amd.c | 2 +- arch/i386/kernel/cpu/centaur.c | 2 +- arch/i386/kernel/cpu/cyrix.c | 6 +++--- arch/x86_64/kernel/setup.c | 2 +- include/asm-i386/cpufeature.h | 4 +++- include/asm-x86_64/cpufeature.h | 1 + 6 files changed, 10 insertions(+), 7 deletions(-)
arch/x86_64/kernel/setup.c +1 -1
@@ -576,7 +576,7 @@ static void __cpuinit init_amd(struct cp/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ -clear_bit (0 *32 +31 , &c->x86_capability); +clear_bit (X86_FEATURE_PBE , &c->x86_capability);/* On C+ stepping K8 rep microcode works well for copy/memset */ level =cpuid_eax (1 );
include/asm-i386/cpufeature.h +3 -1
@@ -42,6 +42,7 @@#define X86_FEATURE_HT (0*32+28)/* Hyper-Threading */ #define X86_FEATURE_ACC (0*32+29)/* Automatic clock control */ #define X86_FEATURE_IA64 (0*32+30)/* IA-64 processor */ +#define X86_FEATURE_PBE (0*32+31)/* PBE */ /* AMD-defined CPU features, CPUID level 0x80000001, word 1 */ /* Don't duplicate feature flags which are redundant with Intel! */
@@ -49,6 +50,7 @@#define X86_FEATURE_MP (1*32+19)/* MP Capable. */ #define X86_FEATURE_NX (1*32+20)/* Execute Disable */ #define X86_FEATURE_MMXEXT (1*32+22)/* AMD MMX extensions */ +#define X86_FEATURE_CXMMXORIG (1*32+24)/* Cyrix MMX, initial location */ #define X86_FEATURE_LM (1*32+29)/* Long Mode (x86-64) */ #define X86_FEATURE_3DNOWEXT (1*32+30)/* AMD 3DNow! extensions */ #define X86_FEATURE_3DNOW (1*32+31)/* 3DNow! */
@@ -60,7 +62,7 @@/* Other features, Linux-defined mapping, word 3 */ /* This range is used for feature bits which conflict or are synthesized */ -#define X86_FEATURE_CXMMX (3*32+ 0)/* Cyrix MMX extensions */ +#define X86_FEATURE_CXMMX (3*32+ 0)/* Cyrix MMX extensions, final location */ #define X86_FEATURE_K6_MTRR (3*32+ 1)/* AMD K6 nonstandard MTRRs */ #define X86_FEATURE_CYRIX_ARR (3*32+ 2)/* Cyrix ARRs (= MTRRs) */ #define X86_FEATURE_CENTAUR_MCR (3*32+ 3)/* Centaur MCRs (= MTRRs) */
arch/i386/kernel/cpu/cyrix.c +3 -3
@@ -192,11 +192,11 @@ static void __cpuinit init_cyrix(struct/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ -clear_bit (0 *32 +31 , c->x86_capability); +clear_bit (X86_FEATURE_PBE , c->x86_capability);/* Cyrix used bit 24 in extended (AMD) CPUID for Cyrix MMX extensions */ -if (test_bit (1 *32 +24 , c->x86_capability) ) { -clear_bit (1 *32 +24 , c->x86_capability); +if (test_bit (X86_FEATURE_CXMMXORIG , c->x86_capability) ) { +clear_bit (X86_FEATURE_CXMMXORIG , c->x86_capability);set_bit (X86_FEATURE_CXMMX , c->x86_capability); }
include/asm-x86_64/cpufeature.h +1 -0
@@ -40,6 +40,7 @@#define X86_FEATURE_HT (0*32+28)/* Hyper-Threading */ #define X86_FEATURE_ACC (0*32+29)/* Automatic clock control */ #define X86_FEATURE_IA64 (0*32+30)/* IA-64 processor */ +#define X86_FEATURE_PBE (0*32+31)/* PBE */ /* AMD-defined CPU features, CPUID level 0x80000001, word 1 */ /* Don't duplicate feature flags which are redundant with Intel! */
arch/i386/kernel/cpu/amd.c +1 -1
@@ -83,7 +83,7 @@ static void __cpuinit init_amd(struct cp/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ -clear_bit (0 *32 +31 , c->x86_capability); +clear_bit (X86_FEATURE_PBE , c->x86_capability); r =get_model_name (c);
arch/i386/kernel/cpu/centaur.c +1 -1
@@ -334,7 +334,7 @@ static void __cpuinit init_centaur(struc/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ -clear_bit (0 *32 +31 , c->x86_capability); +clear_bit (X86_FEATURE_PBE , c->x86_capability);switch (c->x86) {
Re: [RFC PATCH 1/3] x86: use defined names for all CPU feature flags
arch/x86_64/kernel/setup.c static void __cpuinit init_amd(struct cp · line 579
Quoted 8 lines · Chuck Ebbert
arch/x86_64/kernel/setup.c +1
-1@@ -576,7 +576,7 @@ static void __cpuinit init_amd(struct cp/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ -clear_bit (0 *32 +31 , &c->x86_capability); +clear_bit (X86_FEATURE_PBE , &c->x86_capability);
The comment suggests this should be X86_FEATURE_3DNOW Same for the other instances.
Dave
Signature · 1 line
http://www.codemonkey.org.uk
Re: [RFC PATCH 1/3] x86: use defined names for all CPU feature flags
In reply to Chuck EbbertQuoted 5 lines · On Saturday 21 April 2007 00:52:52 Chuck Ebbert wrote:
/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ - clear_bit(0*32+31,
&c->x86_capability);
+ clear_bit(X86_FEATURE_PBE,&c->x86_capability);
That doesn't match the comment.
-Andi
Re: [RFC PATCH 1/3] x86: use defined names for all CPU feature flags
In reply to Chuck Ebbertarch/x86_64/kernel/setup.c static void __cpuinit init_amd(struct cp · line 579
Quoted 8 lines
arch/x86_64/kernel/setup.c +1
-1@@ -576,7 +576,7 @@ static void __cpuinit init_amd(struct cp/* Bit 31 in normal CPUID used for nonstandard 3DNow ID; 3DNow is IDd by bit 31 in extended CPUID (1*32+31) anyway */ -clear_bit (0 *32 +31 , &c->x86_capability); +clear_bit (X86_FEATURE_PBE , &c->x86_capability);
And this is more clear why ?
arch/x86_64/kernel/setup.c static void __cpuinit init_amd(struct cp · line 580
Quoted 1 line
arch/x86_64/kernel/setup.c +1
-0+#define X86_FEATURE_PBE (0*32+31)/* PBE */
For these platforms it isn't "PBE" its 3DNow
Alan
RFC PATCH 2/3 x86: new API for modifying CPU feature flags
In reply to Chuck Ebbertx86: new API for modifying CPU feature flags
Use an API for setting/clearing CPU features, so the process can be debugged.
Adds:set_cpu_feature()clear_cpu_feature()clear_all_cpu_features()
Todo:mask_boot_cpu_features()set_cpu_feature_word()
more?
(Hardcoded printk for now, should be dprintk.)
--- include/asm-i386/cpufeature.h | 17 +++++++++++++++++ 1 file changed, 17 insertions(+)
include/asm-i386/cpufeature.h +17 -0
@@ -108,6 +108,23 @@#define cpu_has (c, bit) test_bit(bit, (c)->x86_capability)#define boot_cpu_has (bit) test_bit(bit, boot_cpu_data.x86_capability) +#define alter_cpu_feature (fn, feat, c) \ + do { typeof(c) __c = (c); \ + printk("CPU: %s: %s feature %s for CPU %p", \ + __func__, #fn, #feat, __c); \ + fn ## _bit(X86_FEATURE_ ## feat, __c->x86_capability); \ + } while (0) + +#define set_cpu_feature (f, c) alter_cpu_feature(set, f, c) +#define clear_cpu_feature (f, c) alter_cpu_feature(clear, f, c) + +#define clear_all_cpu_features (c) \ + do { typeof(c) __c = (c); \ + printk("CPU: %s: clearing all capabilities for CPU %p", \ + __func__, __c); \ + memset(&__c->x86_capability, 0, sizeof __c->x86_capability); \ + } while (0) +#define cpu_has_fpu boot_cpu_has(X86_FEATURE_FPU)#define cpu_has_vme boot_cpu_has(X86_FEATURE_VME)#define cpu_has_de boot_cpu_has(X86_FEATURE_DE)
RFC PATCH 3/3 x86: use the x86 CPU feature API
In reply to Chuck Ebbertx86: use the x86 CPU feature API
Just a small demo for now.
--- arch/i386/kernel/cpu/amd.c | 4 ++-- arch/i386/kernel/cpu/common.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-)
arch/i386/kernel/cpu/amd.c +2 -2
@@ -109,8 +109,8 @@ static void __cpuinit init_amd(struct cp {/* Based on AMD doc 20734R - June 2000 */ if ( c->x86_model ==0 ) { -clear_bit (X86_FEATURE_APIC , c->x86_capability); -set_bit (X86_FEATURE_PGE , c->x86_capability); +clear_cpu_feature (APIC , c); +set_cpu_feature (PGE , c); }break ; }
arch/i386/kernel/cpu/common.c +1 -1
@@ -381,7 +381,7 @@ void __cpuinit identify_cpu(struct cpuin c->x86_model_id[0 ] ='\0' ;/* Unset */ c->x86_max_cores =1 ; c->x86_clflush_size =32 ; -memset (&c->x86_capability,0 ,sizeof c->x86_capability); +clear_all_cpu_features (c);if (!have_cpuid_p ()) {/* First of all, decide if this is a 486 or higher */
Quoted 4 lines · On Saturday 21 April 2007 00:49:48 Chuck Ebbert wrote:
x86 CPU feature flag setup has become impossible to debug. Every user just does set_bit()/clear_bit() or writes the entire set to change the flags, so there's no way to trace how they're being set.
Just use grep or printk? It is not *that* complicated.
Quoted 3 lines
This patchset creates an API and debug messages for tracking how the flags get set. It's not nearly done, but I want to know whether or not to continue.
I don't see any particular value. In theory we could add a "debug API" for nearly everything, but in practice it is usually not needed. Like here.
-Andi