x86: Cleanup apic/ioapic/x2apic setup code
cover x86: Cleanup apic/ioapic/x2apic setup code
While reviewing Jiangs interrupt remapping patch set, I had several serious WTF moments when trying to understand what that code is doing.
The main issues I've seen are:
- Blindly copy and pasted code
- Random places which initialize bits and pieces
- Code which got mindlessly expanded with duct tape and glue without considering readability and maintainability.
- Missing inline stubs which result in a ifdef nightmare
- Superflous inline stubs to artificially avoid sensible ifdefs
The deeper I looked the more I started to get grumpy about that maze and finally sat down and cleaned it up seriously. One bug I fixed on the way (surprise, surprise) got folded into Jiangs series which is now in tip/x86/apic. The other one is not that crucial and cant be fixed in the previous code without adding more mess.
The following patch series applies on top of tip/x86/apic. It cleans up and consolidates the apic/ioapic/x2apic setup functions.
Please give it a proper review and testing.
Thanks,
tglx
--- arch/x86/include/asm/smpboot_hooks.h | 68 ------ tip/arch/x86/include/asm/apic.h | 57 +---- tip/arch/x86/include/asm/io_apic.h | 5 tip/arch/x86/kernel/apic/apic.c | 395 ++++++++++++++++++----------------- tip/arch/x86/kernel/apic/io_apic.c | 13 - tip/arch/x86/kernel/cpu/common.c | 2 tip/arch/x86/kernel/smpboot.c | 113 +++++----- tip/init/main.c | 14 - 8 files changed, 308 insertions(+), 359 deletions(-)
PATCH 1/23 x86/apic: Avoid open coded x2apic detection
enable_IR_x2apic() grew a open coded x2apic detection. Implement a proper helper function which shares the code with the already existing x2apic_enabled().
--- arch/x86/include/asm/apic.h | 18 +++++++++--------- arch/x86/kernel/apic/apic.c | 5 +---- 2 files changed, 10 insertions(+), 13 deletions(-)
arch/x86/include/asm/apic.h +9 -9
@@ -108,6 +108,14 @@ extern u64 native_apic_icr_read(void);extern int x2apic_mode; +static inline bool apic_is_x2apic_enabled (void ) +{ +u64 msr; + +rdmsrl (MSR_IA32_APICBASE , msr); +return msr &X2APIC_ENABLE ; +} +#ifdef CONFIG_X86_X2APIC /* * Make previous memory operations globally visible before
@@ -175,15 +183,7 @@ extern void check_x2apic(void);extern void enable_x2apic (void );static inline int x2apic_enabled (void ) { -u64 msr; - -if (!cpu_has_x2apic) -return 0 ; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) -return 1 ; -return 0 ; +return cpu_has_x2apic &&apic_is_x2apic_enabled (); }#define x2apic_supported () (cpu_has_x2apic)
arch/x86/kernel/apic/apic.c +1 -4
@@ -1625,10 +1625,7 @@ void __init enable_IR_x2apic(void)int ret, ir_stat;if (!IS_ENABLED (CONFIG_X86_X2APIC )) { -u64 msr; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) +if (apic_is_x2apic_enabled ())panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); }
Borislav Petkov
maintainerRe: [patch 01/23] x86/apic: Avoid open coded x2apic detection
Quoted 15 lines · Thomas Gleixner
enable_IR_x2apic()grew a open coded x2apic detection. Implement a proper helper function which shares the code with the already existingx2apic_enabled().Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/include/asm/apic.h | 18 +++++++++--------- arch/x86/kernel/apic/apic.c | 5 +---- 2 files changed, 10 insertions(+), 13 deletions(-)Index:
tip/arch/x86/include/asm/apic.h
===================================================================
arch/x86/include/asm/apic.h extern u64 native_apic_icr_read(void); · line 115
Quoted 8 lines
arch/x86/include/asm/apic.h +5
-0@@ -108,6 +108,14 @@ extern u64 native_apic_icr_read(void);extern int x2apic_mode; +static inline bool apic_is_x2apic_enabled (void ) +{ +u64 msr; + +rdmsrl (MSR_IA32_APICBASE , msr);
Let's do
struct msr m; if (msr_read(MSR_IA32_APICBASE, &m)) return false; return m.l & X2APIC_ENABLE;
so that we do the safe MSR access too. Who knows where this code gets executed in the future.
Quoted 9 lines
arch/x86/include/asm/apic.h +2
-0+} +#ifdef CONFIG_X86_X2APIC /* * Make previous memory operations globally visible before @@ -175,15 +183,7 @@ extern void check_x2apic(void);extern void enable_x2apic (void );static inline int x2apic_enabled (void ) {
arch/x86/include/asm/apic.h extern void check_x2apic(void); · line 186
Quoted 10 lines
arch/x86/include/asm/apic.h +1
-9-u64 msr; - -if (!cpu_has_x2apic) -return 0 ; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) -return 1 ; -return 0 ; +return cpu_has_x2apic &&apic_is_x2apic_enabled ();
... and then there's
#define x2apic_supported() (cpu_has_x2apic)
which does the cpufeature test.
Can we agree on one interface only and simplify this a bit more?
Quoted 22 lines
arch/x86/include/asm/apic.h +0
-0}#define x2apic_supported () (cpu_has_x2apic)Index:
tip/arch/x86/kernel/apic/apic.c
===================================================================arch/x86/kernel/apic/apic.c +1
-4@@ -1625,10 +1625,7 @@ void __init enable_IR_x2apic(void)int ret, ir_stat;if (!IS_ENABLED (CONFIG_X86_X2APIC )) { -u64 msr; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) +if (apic_is_x2apic_enabled ())panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); }
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Re: [patch 01/23] x86/apic: Avoid open coded x2apic detection
Quoted 34 lines · Borislav Petkov
Quoted 15 lines · Thomas Gleixner
enable_IR_x2apic()grew a open coded x2apic detection. Implement a proper helper function which shares the code with the already existingx2apic_enabled().Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/include/asm/apic.h | 18 +++++++++--------- arch/x86/kernel/apic/apic.c | 5 +---- 2 files changed, 10 insertions(+), 13 deletions(-)Index:
tip/arch/x86/include/asm/apic.h
===================================================================arch/x86/include/asm/apic.h extern u64 native_apic_icr_read(void); · line 115
Quoted 8 lines
arch/x86/include/asm/apic.h +5
-0@@ -108,6 +108,14 @@ extern u64 native_apic_icr_read(void);extern int x2apic_mode; +static inline bool apic_is_x2apic_enabled (void ) +{ +u64 msr; + +rdmsrl (MSR_IA32_APICBASE , msr);Let's do
struct msr m; if (msr_read(MSR_IA32_APICBASE, &m)) return false; return m.l & X2APIC_ENABLE;
Quoted 2 lines
so that we do the safe MSR access too. Who knows where this code gets executed in the future.
Updated.
Quoted 22 lines
Quoted 9 lines
arch/x86/include/asm/apic.h +2
-0+} +#ifdef CONFIG_X86_X2APIC /* * Make previous memory operations globally visible before @@ -175,15 +183,7 @@ extern void check_x2apic(void);extern void enable_x2apic (void );static inline int x2apic_enabled (void ) {arch/x86/include/asm/apic.h extern void check_x2apic(void); · line 186
Quoted 10 lines
arch/x86/include/asm/apic.h +1
-9-u64 msr; - -if (!cpu_has_x2apic) -return 0 ; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) -return 1 ; -return 0 ; +return cpu_has_x2apic &&apic_is_x2apic_enabled ();... and then there's
Quoted 5 lines
#define
x2apic_supported()(cpu_has_x2apic)which does the cpufeature test.
Can we agree on one interface only and simplify this a bit more?
I think I do that later on, but will make sure.
Re: [patch 01/23] x86/apic: Avoid open coded x2apic detection
Quoted 9 lines · Thomas Gleixner
Quoted 6 lines · Borislav Petkov
#define
x2apic_supported()(cpu_has_x2apic)which does the cpufeature test.
Can we agree on one interface only and simplify this a bit more?
I think I do that later on, but will make sure.
We can't do that some places want the cpu_has_x2apic check even when X2APIC=n. x2apic_supported() is compiled to 0 when X2APIC=n.
Thanks,
tglx
tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Avoid open coded x2apic detection
In reply to Thomas Gleixnerx86/apic: Avoid open coded x2apic detection
Commit-ID: 8d80696060eedf49c080c0f2cf39a20ae7e787f9 Gitweb: http://git.kernel.org/tip/8d80696060eedf49c080c0f2cf39a20ae7e787f9 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:09 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/apic: Avoid open coded x2apic detection
enable_IR_x2apic() grew a open coded x2apic detection. Implement a proper helper function which shares the code with the already existing x2apic_enabled().
Made it use rdmsrl_safe as suggested by Boris.
--- arch/x86/include/asm/apic.h | 19 ++++++++++--------- arch/x86/kernel/apic/apic.c | 5 +---- 2 files changed, 11 insertions(+), 13 deletions(-)
arch/x86/include/asm/apic.h +10 -9
@@ -108,6 +108,15 @@ extern u64 native_apic_icr_read(void);extern int x2apic_mode; +static inline bool apic_is_x2apic_enabled (void ) +{ +u64 msr; + +if (rdmsrl_safe (MSR_IA32_APICBASE , &msr)) +return false; +return msr &X2APIC_ENABLE ; +} +#ifdef CONFIG_X86_X2APIC /* * Make previous memory operations globally visible before
@@ -175,15 +184,7 @@ extern void check_x2apic(void);extern void enable_x2apic (void );static inline int x2apic_enabled (void ) { -u64 msr; - -if (!cpu_has_x2apic) -return 0 ; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) -return 1 ; -return 0 ; +return cpu_has_x2apic &&apic_is_x2apic_enabled (); }#define x2apic_supported () (cpu_has_x2apic)
arch/x86/kernel/apic/apic.c +1 -4
@@ -1625,10 +1625,7 @@ void __init enable_IR_x2apic(void)int ret, ir_stat;if (!IS_ENABLED (CONFIG_X86_X2APIC )) { -u64 msr; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) +if (apic_is_x2apic_enabled ())panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); }
PATCH 2/23 x86/apic: Make x2apic_mode depend on CONFIG_X86_X2APIC
In reply to Thomas GleixnerNo point in having a static variable around which is always 0. Let the compiler optimize code out if disabled.
--- arch/x86/include/asm/apic.h | 8 ++++---- arch/x86/kernel/apic/apic.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/include/asm/apic.h +4 -4
@@ -106,8 +106,6 @@ extern u32 native_safe_apic_wait_icr_idlextern void native_apic_icr_write (u32 low,u32 id);extern u64 native_apic_icr_read (void ); -extern int x2apic_mode; -static inline bool apic_is_x2apic_enabled (void ) {u64 msr;
@@ -177,6 +175,7 @@ static inline u64 native_x2apic_icr_readreturn val; } +extern int x2apic_mode;extern int x2apic_phys;extern int x2apic_preenabled;extern void check_x2apic (void );
@@ -209,8 +208,9 @@ static inline void x2apic_force_phys(voi { } -#define x2apic_preenabled 0 -#define x2apic_supported () 0 +#define x2apic_mode (0) +#define x2apic_preenabled (0) +#define x2apic_supported () (0)#endif extern void enable_IR_x2apic (void );
arch/x86/kernel/apic/apic.c +1 -1
@@ -161,8 +161,8 @@ static __init int setup_apicpmtimer(char__setup ("apicpmtimer" , setup_apicpmtimer);#endif -int x2apic_mode;#ifdef CONFIG_X86_X2APIC +int x2apic_mode;/* x2apic enabled before OS handover */ int x2apic_preenabled;static int x2apic_disabled;
Re: [patch 02/23] x86/apic: Make x2apic_mode depend on CONFIG_X86_X2APIC
Quoted 4 lines · Thomas Gleixner
No point in having a static variable around which is always 0. Let the compiler optimize code out if disabled.
Signed-off-by Thomas Gleixner <[email protected]>
Quoted 27 lines
--- arch/x86/include/asm/apic.h | 8 ++++---- arch/x86/kernel/apic/apic.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)Index:
tip/arch/x86/include/asm/apic.h
===================================================================arch/x86/include/asm/apic.h +1
-2@@ -106,8 +106,6 @@ extern u32 native_safe_apic_wait_icr_idlextern void native_apic_icr_write (u32 low,u32 id);extern u64 native_apic_icr_read (void ); -extern int x2apic_mode; -static inline bool apic_is_x2apic_enabled (void ) {u64 msr;@@ -177,6 +175,7 @@ static inline u64 native_x2apic_icr_readreturn val; } +extern int x2apic_mode;extern int x2apic_phys;extern int x2apic_preenabled;extern void check_x2apic (void );
arch/x86/include/asm/apic.h static inline void x2apic_force_phys(voi · line 212
Quoted 7 lines
arch/x86/include/asm/apic.h +2
-2@@ -209,8 +208,9 @@ static inline void x2apic_force_phys(voi { } -#define x2apic_preenabled 0 -#define x2apic_supported () 0 +#define x2apic_mode (0) +#define x2apic_preenabled (0)
Btw, while you're at it, you can kill this yet another knob x2apic_preenabled and use check_x2apic() directly. There are just too many useless knobs in there.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Make x2apic_mode depend on CONFIG_X86_X2APIC
In reply to Thomas Gleixnerx86/apic: Make x2apic_mode depend on CONFIG_X86_X2APIC
Commit-ID: 81a46dd8249d7fa72a8557e58a38aa984e6b5e16 Gitweb: http://git.kernel.org/tip/81a46dd8249d7fa72a8557e58a38aa984e6b5e16 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:11 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/apic: Make x2apic_mode depend on CONFIG_X86_X2APIC
No point in having a static variable around which is always 0. Let the compiler optimize code out if disabled.
--- arch/x86/include/asm/apic.h | 8 ++++---- arch/x86/kernel/apic/apic.c | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-)
arch/x86/include/asm/apic.h +4 -4
@@ -106,8 +106,6 @@ extern u32 native_safe_apic_wait_icr_idle(void);extern void native_apic_icr_write (u32 low,u32 id);extern u64 native_apic_icr_read (void ); -extern int x2apic_mode; -static inline bool apic_is_x2apic_enabled (void ) {u64 msr;
@@ -178,6 +176,7 @@ static inline u64 native_x2apic_icr_read(void)return val; } +extern int x2apic_mode;extern int x2apic_phys;extern int x2apic_preenabled;extern void check_x2apic (void );
@@ -210,8 +209,9 @@ static inline void x2apic_force_phys(void) { } -#define x2apic_preenabled 0 -#define x2apic_supported () 0 +#define x2apic_mode (0) +#define x2apic_preenabled (0) +#define x2apic_supported () (0)#endif extern void enable_IR_x2apic (void );
arch/x86/kernel/apic/apic.c +1 -1
@@ -161,8 +161,8 @@ static __init int setup_apicpmtimer(char *s)__setup ("apicpmtimer" , setup_apicpmtimer);#endif -int x2apic_mode;#ifdef CONFIG_X86_X2APIC +int x2apic_mode;/* x2apic enabled before OS handover */ int x2apic_preenabled;static int x2apic_disabled;
PATCH 3/23 x86/apic: Move x2apic code to one place
In reply to Thomas GleixnerHaving several disjunct pieces of code for x2apic support makes reading the code unnecessarily hard. Move it to one ifdeffed section.
--- arch/x86/kernel/apic/apic.c | 58 +++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 30 deletions(-)
arch/x86/kernel/apic/apic.c +28 -30
@@ -134,9 +134,6 @@ static inline void imcr_apic_to_pic(void*/ static int force_enable_local_apic__initdata ; -/* Control whether x2APIC mode is enabled or not */ -static bool nox2apic__initdata ; -/* * APIC command line parameters */
@@ -161,33 +158,6 @@ static __init int setup_apicpmtimer(char__setup ("apicpmtimer" , setup_apicpmtimer);#endif -#ifdef CONFIG_X86_X2APIC -int x2apic_mode; -/* x2apic enabled before OS handover */ -int x2apic_preenabled; -static int x2apic_disabled; -static int __init setup_nox2apic (char *str) -{ -if (x2apic_enabled ()) { -int apicid =native_apic_msr_read (APIC_ID ); - -if (apicid >=255 ) { -pr_warning ("Apicid: %08x, cannot enforce nox2apic\n" , - apicid); -return 0 ; - } - -pr_warning ("x2apic already enabled. will disable it\n" ); - }else -setup_clear_cpu_cap (X86_FEATURE_X2APIC ); - - nox2apic = true; - -return 0 ; -} -early_param ("nox2apic" , setup_nox2apic); -#endif -unsigned long mp_lapic_addr;int disable_apic;/* Disable local APIC timer from the kernel commandline or via dmi quirk */
@@ -1504,7 +1474,35 @@ void __init bsp_end_local_APIC_setup(voi } +/* Control whether x2APIC mode is enabled or not */ +static bool nox2apic__initdata ; +#ifdef CONFIG_X86_X2APIC +int x2apic_mode; +/* x2apic enabled before OS handover */ +int x2apic_preenabled; +static int x2apic_disabled; +static int __init setup_nox2apic (char *str) +{ +if (x2apic_enabled ()) { +int apicid =native_apic_msr_read (APIC_ID ); + +if (apicid >=255 ) { +pr_warning ("Apicid: %08x, cannot enforce nox2apic\n" , + apicid); +return 0 ; + } + +pr_warning ("x2apic already enabled. will disable it\n" ); + }else +setup_clear_cpu_cap (X86_FEATURE_X2APIC ); + + nox2apic = true; + +return 0 ; +} +early_param ("nox2apic" , setup_nox2apic); +/* * Need to disable xapic and x2apic at the same time and then enable xapic mode */
Re: [patch 03/23] x86/apic: Move x2apic code to one place
Quoted 4 lines · Thomas Gleixner
Having several disjunct pieces of code for x2apic support makes reading the code unnecessarily hard. Move it to one ifdeffed section.
Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Move x2apic code to one place
In reply to Thomas Gleixnerx86/apic: Move x2apic code to one place
Commit-ID: bfb050702990d6a2033d072cb2af583aee5c6fc5 Gitweb: http://git.kernel.org/tip/bfb050702990d6a2033d072cb2af583aee5c6fc5 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:12 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/apic: Move x2apic code to one place
Having several disjunct pieces of code for x2apic support makes reading the code unnecessarily hard. Move it to one ifdeffed section.
--- arch/x86/kernel/apic/apic.c | 58 ++++++++++++++++++++++----------------------- 1 file changed, 28 insertions(+), 30 deletions(-)
arch/x86/kernel/apic/apic.c +28 -30
@@ -134,9 +134,6 @@ static inline void imcr_apic_to_pic(void)*/ static int force_enable_local_apic__initdata ; -/* Control whether x2APIC mode is enabled or not */ -static bool nox2apic__initdata ; -/* * APIC command line parameters */
@@ -161,33 +158,6 @@ static __init int setup_apicpmtimer(char *s)__setup ("apicpmtimer" , setup_apicpmtimer);#endif -#ifdef CONFIG_X86_X2APIC -int x2apic_mode; -/* x2apic enabled before OS handover */ -int x2apic_preenabled; -static int x2apic_disabled; -static int __init setup_nox2apic (char *str) -{ -if (x2apic_enabled ()) { -int apicid =native_apic_msr_read (APIC_ID ); - -if (apicid >=255 ) { -pr_warning ("Apicid: %08x, cannot enforce nox2apic\n" , - apicid); -return 0 ; - } - -pr_warning ("x2apic already enabled. will disable it\n" ); - }else -setup_clear_cpu_cap (X86_FEATURE_X2APIC ); - - nox2apic = true; - -return 0 ; -} -early_param ("nox2apic" , setup_nox2apic); -#endif -unsigned long mp_lapic_addr;int disable_apic;/* Disable local APIC timer from the kernel commandline or via dmi quirk */
@@ -1504,7 +1474,35 @@ void __init bsp_end_local_APIC_setup(void) } +/* Control whether x2APIC mode is enabled or not */ +static bool nox2apic__initdata ; +#ifdef CONFIG_X86_X2APIC +int x2apic_mode; +/* x2apic enabled before OS handover */ +int x2apic_preenabled; +static int x2apic_disabled; +static int __init setup_nox2apic (char *str) +{ +if (x2apic_enabled ()) { +int apicid =native_apic_msr_read (APIC_ID ); + +if (apicid >=255 ) { +pr_warning ("Apicid: %08x, cannot enforce nox2apic\n" , + apicid); +return 0 ; + } + +pr_warning ("x2apic already enabled. will disable it\n" ); + }else +setup_clear_cpu_cap (X86_FEATURE_X2APIC ); + + nox2apic = true; + +return 0 ; +} +early_param ("nox2apic" , setup_nox2apic); +/* * Need to disable xapic and x2apic at the same time and then enable xapic mode */
PATCH 4/23 x86/ioapic: Check x2apic really
In reply to Thomas GleixnerThe x2apic_preenabled flag is just a horrible hack and if X2APIC support is disabled it does not reflect the actual hardware state. Check the hardware instead.
--- arch/x86/include/asm/apic.h | 2 -- arch/x86/kernel/apic/apic.c | 4 +++- arch/x86/kernel/apic/io_apic.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
arch/x86/include/asm/apic.h +0 -2
@@ -177,7 +177,6 @@ static inline u64 native_x2apic_icr_readextern int x2apic_mode;extern int x2apic_phys; -extern int x2apic_preenabled;extern void check_x2apic (void );extern void enable_x2apic (void );static inline int x2apic_enabled (void )
@@ -209,7 +208,6 @@ static inline void x2apic_force_phys(voi }#define x2apic_mode (0) -#define x2apic_preenabled (0)#define x2apic_supported () (0)#endif
arch/x86/kernel/apic/apic.c +3 -1
@@ -1480,7 +1480,7 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode;/* x2apic enabled before OS handover */ -int x2apic_preenabled; +static int x2apic_preenabled;static int x2apic_disabled;static int __init setup_nox2apic (char *str) {
@@ -1569,6 +1569,8 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } +#else +#define x2apic_preenabled (0)#endif /* CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
arch/x86/kernel/apic/io_apic.c +1 -1
@@ -2295,7 +2295,7 @@ static inline void __init check_timer(vo }local_irq_disable ();apic_printk (APIC_QUIET ,KERN_INFO "..... failed :(.\n" ); -if (x2apic_preenabled) +if (apic_is_x2apic_enabled ())apic_printk (APIC_QUIET ,KERN_INFO "Perhaps problem with the pre-enabled x2apic mode\n" "Try booting with x2apic and interrupt-remapping disabled in the bios.\n" );
Borislav Petkov
maintainerRe: [patch 04/23] x86/ioapic: Check x2apic really
Quoted 36 lines · Thomas Gleixner
The
x2apic_preenabledflag is just a horrible hack and if X2APIC support is disabled it does not reflect the actual hardware state. Check the hardware instead.Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/include/asm/apic.h | 2 -- arch/x86/kernel/apic/apic.c | 4 +++- arch/x86/kernel/apic/io_apic.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)Index:
tip/arch/x86/include/asm/apic.h
===================================================================arch/x86/include/asm/apic.h +0
-2@@ -177,7 +177,6 @@ static inline u64 native_x2apic_icr_readextern int x2apic_mode;extern int x2apic_phys; -extern int x2apic_preenabled;extern void check_x2apic (void );extern void enable_x2apic (void );static inline int x2apic_enabled (void )@@ -209,7 +208,6 @@ static inline void x2apic_force_phys(voi }#define x2apic_mode (0) -#define x2apic_preenabled (0)#define x2apic_supported () (0)#endif Index:
tip/arch/x86/kernel/apic/apic.c
===================================================================
arch/x86/kernel/apic/apic.c static bool nox2apic __initdata; · line 1483
Quoted 5 lines
arch/x86/kernel/apic/apic.c +1
-1@@ -1480,7 +1480,7 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode;/* x2apic enabled before OS handover */ -int x2apic_preenabled; +static int x2apic_preenabled;
Bah, let's get rid of this knob completely (ontop of your patch):
---
arch/x86/kernel/apic/apic.c +3 -5
@@ -1480,7 +1480,6 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode;/* x2apic enabled before OS handover */ -static int x2apic_preenabled;static int x2apic_disabled;static int __init setup_nox2apic (char *str) {
@@ -1546,7 +1545,7 @@ void check_x2apic(void) {if (x2apic_enabled ()) {pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); - x2apic_preenabled = x2apic_mode =1 ; + x2apic_mode =1 ; } }
@@ -1570,7 +1569,6 @@ void enable_x2apic(void) } }#else -#define x2apic_preenabled (0)#endif /* CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
@@ -1599,7 +1597,7 @@ static __init void try_to_enable_x2apic( (IS_ENABLED(CONFIG_HYPERVISOR_GUEST) && !hypervisor_x2apic_available())) {pr_info ("IRQ remapping doesn't support X2APIC mode, disable x2apic.\n" ); -if (x2apic_preenabled) +if (x2apic_enabled ())disable_x2apic ();return ; }
@@ -1643,7 +1641,7 @@ void __init enable_IR_x2apic(void) legacy_pic->mask_all ();mask_ioapic_entries (); -if (x2apic_preenabled && nox2apic) +if (x2apic_enabled () && nox2apic)disable_x2apic ();/* If irq_remapping_prepare() succeded, try to enable it */ if (ir_stat >=0 )
--
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--Borislav Petkov
maintainerRe: [patch 04/23] x86/ioapic: Check x2apic really
Quoted 1 line · Borislav Petkov
Bah, let's get rid of this knob completely (ontop of your patch):
... or, maybe I should look at the next patch first :-)
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/ioapic: Check x2apic really
In reply to Thomas Gleixnerx86/ioapic: Check x2apic really
Commit-ID: 2ca5b40479246087695d9e6343075b47ee6887ea Gitweb: http://git.kernel.org/tip/2ca5b40479246087695d9e6343075b47ee6887ea Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:14 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/ioapic: Check x2apic really
The x2apic_preenabled flag is just a horrible hack and if X2APIC support is disabled it does not reflect the actual hardware state. Check the hardware instead.
--- arch/x86/include/asm/apic.h | 2 -- arch/x86/kernel/apic/apic.c | 4 +++- arch/x86/kernel/apic/io_apic.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-)
arch/x86/include/asm/apic.h +0 -2
@@ -178,7 +178,6 @@ static inline u64 native_x2apic_icr_read(void)extern int x2apic_mode;extern int x2apic_phys; -extern int x2apic_preenabled;extern void check_x2apic (void );extern void enable_x2apic (void );static inline int x2apic_enabled (void )
@@ -210,7 +209,6 @@ static inline void x2apic_force_phys(void) }#define x2apic_mode (0) -#define x2apic_preenabled (0)#define x2apic_supported () (0)#endif
arch/x86/kernel/apic/apic.c +3 -1
@@ -1480,7 +1480,7 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode;/* x2apic enabled before OS handover */ -int x2apic_preenabled; +static int x2apic_preenabled;static int x2apic_disabled;static int __init setup_nox2apic (char *str) {
@@ -1569,6 +1569,8 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } +#else +#define x2apic_preenabled (0)#endif /* CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
arch/x86/kernel/apic/io_apic.c +1 -1
@@ -2295,7 +2295,7 @@ static inline void __init check_timer(void) }local_irq_disable ();apic_printk (APIC_QUIET ,KERN_INFO "..... failed :(.\n" ); -if (x2apic_preenabled) +if (apic_is_x2apic_enabled ())apic_printk (APIC_QUIET ,KERN_INFO "Perhaps problem with the pre-enabled x2apic mode\n" "Try booting with x2apic and interrupt-remapping disabled in the bios.\n" );
PATCH 5/23 x86/apic: Make disable x2apic work really
In reply to Thomas GleixnerIf x2apic_preenabled is not enabled, then disable_x2apic() is not called from various places which results in x2apic_disabled not being set. So other code pathes can happily reenable the x2apic.
--- arch/x86/kernel/apic/apic.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
arch/x86/kernel/apic/apic.c +5 -9
@@ -1479,8 +1479,6 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode; -/* x2apic enabled before OS handover */ -static int x2apic_preenabled;static int x2apic_disabled;static int __init setup_nox2apic (char *str) {
@@ -1535,18 +1533,19 @@ static __init void disable_x2apic(void)setup_clear_cpu_cap (X86_FEATURE_X2APIC ); } - x2apic_disabled =1 ; x2apic_mode =0 ;register_lapic_address (mp_lapic_addr); } + + x2apic_disabled =1 ; }void check_x2apic (void ) {if (x2apic_enabled ()) {pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); - x2apic_preenabled = x2apic_mode =1 ; + x2apic_mode =1 ; } }
@@ -1569,8 +1568,6 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } -#else -#define x2apic_preenabled (0)#endif /* CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
@@ -1599,8 +1596,7 @@ static __init void try_to_enable_x2apic( (IS_ENABLED(CONFIG_HYPERVISOR_GUEST) && !hypervisor_x2apic_available())) {pr_info ("IRQ remapping doesn't support X2APIC mode, disable x2apic.\n" ); -if (x2apic_preenabled) -disable_x2apic (); +disable_x2apic ();return ; }
@@ -1643,7 +1639,7 @@ void __init enable_IR_x2apic(void) legacy_pic->mask_all ();mask_ioapic_entries (); -if (x2apic_preenabled && nox2apic) +if (nox2apic)disable_x2apic ();/* If irq_remapping_prepare() succeded, try to enable it */ if (ir_stat >=0 )
Borislav Petkov
maintainerRe: [patch 05/23] x86/apic: Make disable x2apic work really
Quoted 23 lines · Thomas Gleixner
If
x2apic_preenabledis not enabled, thendisable_x2apic()is not called from various places which results inx2apic_disablednot being set. So other code pathes can happily reenable the x2apic.Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/kernel/apic/apic.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)Index:
tip/arch/x86/kernel/apic/apic.c
===================================================================arch/x86/kernel/apic/apic.c +0
-2@@ -1479,8 +1479,6 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode; -/* x2apic enabled before OS handover */ -static int x2apic_preenabled;static int x2apic_disabled;static int __init setup_nox2apic (char *str) {
arch/x86/kernel/apic/apic.c static __init void disable_x2apic(void) · line 1536
Quoted 5 lines
arch/x86/kernel/apic/apic.c +0
-1@@ -1535,18 +1533,19 @@ static __init void disable_x2apic(void)setup_clear_cpu_cap (X86_FEATURE_X2APIC ); } - x2apic_disabled =1 ; x2apic_mode =0 ;
Should we move out that write to x2apic_mode too?
arch/x86/kernel/apic/apic.c static __init void disable_x2apic(void) · line 1541
Quoted 5 lines
arch/x86/kernel/apic/apic.c +2
-0register_lapic_address (mp_lapic_addr); } + + x2apic_disabled =1 ;
i.e., here?
Just in case.
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Make disable x2apic work really
In reply to Thomas Gleixnerx86/apic: Make disable x2apic work really
Commit-ID: 9aa16365275a272283acbda665634ca3dc8b46fe Gitweb: http://git.kernel.org/tip/9aa16365275a272283acbda665634ca3dc8b46fe Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:16 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/apic: Make disable x2apic work really
If x2apic_preenabled is not enabled, then disable_x2apic() is not called from various places which results in x2apic_disabled not being set. So other code pathes can happily reenable the x2apic.
--- arch/x86/kernel/apic/apic.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)
arch/x86/kernel/apic/apic.c +5 -9
@@ -1479,8 +1479,6 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode; -/* x2apic enabled before OS handover */ -static int x2apic_preenabled;static int x2apic_disabled;static int __init setup_nox2apic (char *str) {
@@ -1535,18 +1533,19 @@ static __init void disable_x2apic(void)setup_clear_cpu_cap (X86_FEATURE_X2APIC ); } - x2apic_disabled =1 ; x2apic_mode =0 ;register_lapic_address (mp_lapic_addr); } + + x2apic_disabled =1 ; }void check_x2apic (void ) {if (x2apic_enabled ()) {pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); - x2apic_preenabled = x2apic_mode =1 ; + x2apic_mode =1 ; } }
@@ -1569,8 +1568,6 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } -#else -#define x2apic_preenabled (0)#endif /* CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
@@ -1599,8 +1596,7 @@ static __init void try_to_enable_x2apic(int ir_stat) (IS_ENABLED(CONFIG_HYPERVISOR_GUEST) && !hypervisor_x2apic_available())) {pr_info ("IRQ remapping doesn't support X2APIC mode, disable x2apic.\n" ); -if (x2apic_preenabled) -disable_x2apic (); +disable_x2apic ();return ; }
@@ -1643,7 +1639,7 @@ void __init enable_IR_x2apic(void) legacy_pic->mask_all ();mask_ioapic_entries (); -if (x2apic_preenabled && nox2apic) +if (nox2apic)disable_x2apic ();/* If irq_remapping_prepare() succeded, try to enable it */ if (ir_stat >=0 )
PATCH 6/23 x86/apic: Check x2apic early
In reply to Thomas GleixnerNo point in delaying the x2apic detection for the CONFIG_X86_X2APIC=n case to enable_IR_x2apic(). We rather detect that in the early boot code in check_x2apic().
--- arch/x86/include/asm/apic.h | 6 ++---- arch/x86/kernel/apic/apic.c | 33 +++++++++++++++++++-------------- 2 files changed, 21 insertions(+), 18 deletions(-)
arch/x86/include/asm/apic.h +2 -4
@@ -177,7 +177,6 @@ static inline u64 native_x2apic_icr_readextern int x2apic_mode;extern int x2apic_phys; -extern void check_x2apic (void );extern void enable_x2apic (void );static inline int x2apic_enabled (void ) {
@@ -193,9 +192,6 @@ static inline void x2apic_force_phys(voistatic inline void disable_x2apic (void ) { } -static inline void check_x2apic (void ) -{ -}static inline void enable_x2apic (void ) { }
@@ -429,6 +425,7 @@ static inline u32 safe_apic_wait_icr_idl }extern void __init apic_set_eoi_write (void (*eoi_write)(u32 reg,u32 v)); +extern void __init check_x2apic (void );#else /* CONFIG_X86_LOCAL_APIC */
@@ -440,6 +437,7 @@ static inline void apic_icr_write(u32 lostatic inline void apic_wait_icr_idle (void ) { }static inline u32 safe_apic_wait_icr_idle (void ) {return 0 ; }static inline void apic_set_eoi_write (void (*eoi_write)(u32 reg,u32 v)) {} +static inline void check_x2apic (void ) { }#endif /* CONFIG_X86_LOCAL_APIC */
arch/x86/kernel/apic/apic.c +19 -14
@@ -1541,14 +1541,6 @@ static __init void disable_x2apic(void) x2apic_disabled =1 ; } -void check_x2apic (void ) -{ -if (x2apic_enabled ()) { -pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); - x2apic_mode =1 ; - } -} -void enable_x2apic (void ) {u64 msr;
@@ -1568,7 +1560,25 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } -#endif /* CONFIG_X86_X2APIC */ + +void __init check_x2apic (void ) +{ +if (x2apic_enabled ()) { +pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); + x2apic_mode =1 ; + } +} +#else /* CONFIG_X86_X2APIC */ +void __init check_x2apic (void ) +{ +if (!cpu_has_apic || !apic_is_x2apic_enabled ()) +return ; +/* +* Checkme: Can we simply turn off x2apic here instead of panic? +*/ +panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); +} +#endif /* !CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void ) {
@@ -1620,11 +1630,6 @@ void __init enable_IR_x2apic(void)unsigned long flags;int ret, ir_stat; -if (!IS_ENABLED (CONFIG_X86_X2APIC )) { -if (apic_is_x2apic_enabled ()) -panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); - } - ir_stat =irq_remapping_prepare ();if (ir_stat <0 && !x2apic_supported ())return ;
Re: [patch 06/23] x86/apic: Check x2apic early
Hi Thomas,
check_x2apic() is a little too early. If system panics
in check_x2apic(), it's a blank console if system panics in check_x2apic().
I also tried to disabled x2apic in check_x2apic() if
x2apic is enabled by BIOS but kernel doesn't support x2apic. It may continue for a while, but eventually run into livelock with following log messages:
[ 222.436829] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.436829] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.452737] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.452737] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.468645] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.468645] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.484554] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.484554] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.500463] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.500463] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.516371] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.516371] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.532285] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2 [ 222.532285] INTR-REMAP:[fault reason 36] Detected reserved fields in the IRTE entry [ 222.548200] dmar: INTR-REMAP: Request device [[f0:1f.7] fault index 2
So seems we need to revert to original solution.
Thanks!
Gerry
------------------------------------------------------------------------
On 2015/1/16 5:22, Thomas Gleixner wrote:
No point in delaying the x2apic detection for the CONFIG_X86_X2APIC=n case to enable_IR_x2apic(). We rather detect that in the early boot code in check_x2apic().
--- arch/x86/include/asm/apic.h | 6 ++---- arch/x86/kernel/apic/apic.c | 33 +++++++++++++++++++-------------- 2 files changed, 21 insertions(+), 18 deletions(-)
arch/x86/include/asm/apic.h +2 -4
@@ -177,7 +177,6 @@ static inline u64 native_x2apic_icr_readextern int x2apic_mode;extern int x2apic_phys; -extern void check_x2apic (void );extern void enable_x2apic (void );static inline int x2apic_enabled (void ) {
@@ -193,9 +192,6 @@ static inline void x2apic_force_phys(voistatic inline void disable_x2apic (void ) { } -static inline void check_x2apic (void ) -{ -}static inline void enable_x2apic (void ) { }
@@ -429,6 +425,7 @@ static inline u32 safe_apic_wait_icr_idl }extern void __init apic_set_eoi_write (void (*eoi_write)(u32 reg,u32 v)); +extern void __init check_x2apic (void );#else /* CONFIG_X86_LOCAL_APIC */
@@ -440,6 +437,7 @@ static inline void apic_icr_write(u32 lostatic inline void apic_wait_icr_idle (void ) { }static inline u32 safe_apic_wait_icr_idle (void ) {return 0 ; }static inline void apic_set_eoi_write (void (*eoi_write)(u32 reg,u32 v)) {} +static inline void check_x2apic (void ) { }#endif /* CONFIG_X86_LOCAL_APIC */
arch/x86/kernel/apic/apic.c +17 -9
@@ -1541,14 +1541,6 @@ static __init void disable_x2apic(void) x2apic_disabled =1 ; } -void check_x2apic (void ) -{ -if (x2apic_enabled ()) { -pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); - x2apic_mode =1 ; - } -} -void enable_x2apic (void ) {u64 msr;
@@ -1568,7 +1560,25 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } -#endif /* CONFIG_X86_X2APIC */ + +void __init check_x2apic (void ) +{ +if (x2apic_enabled ()) { +pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); + x2apic_mode =1 ; + } +} +#else /* CONFIG_X86_X2APIC */ +void __init check_x2apic (void ) +{ +if (!cpu_has_apic || !apic_is_x2apic_enabled ()) +return ; +/* +* Checkme: Can we simply turn off x2apic here instead of panic? +*/ + panic("BIOS has enabled x2apic but kernel doesn't support x2apic,
please disable x2apic in BIOS.\n");
+}
+#endif /* !CONFIG_X86_X2APIC */
static int __init try_to_enable_IR(void)
{
@@ -1620,11 +1630,6 @@ void __init enable_IR_x2apic(void)
unsigned long flags;
int ret, ir_stat;
- if (!IS_ENABLED(CONFIG_X86_X2APIC)) {
- if (apic_is_x2apic_enabled())
- panic("BIOS has enabled x2apic but kernel doesn't support x2apic,
please disable x2apic in BIOS.\n");
- }
-
ir_stat = irq_remapping_prepare();
if (ir_stat < 0 && !x2apic_supported())
return;
tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Check x2apic early
In reply to Thomas Gleixnerx86/apic: Check x2apic early
Commit-ID: d524165cb8dbb2ce5916cd7682236b9324ae2644 Gitweb: http://git.kernel.org/tip/d524165cb8dbb2ce5916cd7682236b9324ae2644 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:17 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/apic: Check x2apic early
No point in delaying the x2apic detection for the CONFIG_X86_X2APIC=n case to enable_IR_x2apic(). We rather detect that before we try to setup anything there.
--- arch/x86/include/asm/apic.h | 2 +- arch/x86/kernel/apic/apic.c | 34 ++++++++++++++++++++-------------- 2 files changed, 21 insertions(+), 15 deletions(-)
arch/x86/include/asm/apic.h +1 -1
@@ -178,7 +178,7 @@ static inline u64 native_x2apic_icr_read(void)extern int x2apic_mode;extern int x2apic_phys; -extern void check_x2apic (void ); +extern void __init check_x2apic (void );extern void enable_x2apic (void );static inline int x2apic_enabled (void ) {
arch/x86/kernel/apic/apic.c +20 -14
@@ -1541,14 +1541,6 @@ static __init void disable_x2apic(void) x2apic_disabled =1 ; } -void check_x2apic (void ) -{ -if (x2apic_enabled ()) { -pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); - x2apic_mode =1 ; - } -} -void enable_x2apic (void ) {u64 msr;
@@ -1568,7 +1560,26 @@ void enable_x2apic(void) wrmsrl(MSR_IA32_APICBASE, msr | X2APIC_ENABLE); } } -#endif /* CONFIG_X86_X2APIC */ + +void __init check_x2apic (void ) +{ +if (x2apic_enabled ()) { +pr_info ("x2apic enabled by BIOS, switching to x2apic ops\n" ); + x2apic_mode =1 ; + } +} +#else /* CONFIG_X86_X2APIC */ +static int __init validate_x2apic (void ) +{ +if (!apic_is_x2apic_enabled ()) +return 0 ; +/* +* Checkme: Can we simply turn off x2apic here instead of panic? +*/ +panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); +} +early_initcall (validate_x2apic); +#endif /* !CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void ) {
@@ -1620,11 +1631,6 @@ void __init enable_IR_x2apic(void)unsigned long flags;int ret, ir_stat; -if (!IS_ENABLED (CONFIG_X86_X2APIC )) { -if (apic_is_x2apic_enabled ()) -panic ("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n" ); - } - ir_stat =irq_remapping_prepare ();if (ir_stat <0 && !x2apic_supported ())return ;
PATCH 7/23 x86/x2apic: Move code in conditional region
In reply to Thomas GleixnerNo point in having try_to_enable_x2apic() outside of the CONFIG_X86_X2APIC section and having inline functions and more ifdefs to deal with it. Move the code into the existing ifdef section and remove the inline cruft.
--- arch/x86/include/asm/apic.h | 20 ++----------- arch/x86/kernel/apic/apic.c | 65 +++++++++++++++++++++----------------------- 2 files changed, 35 insertions(+), 50 deletions(-)
arch/x86/include/asm/apic.h +3 -17
@@ -184,24 +184,10 @@ static inline int x2apic_enabled(void) }#define x2apic_supported () (cpu_has_x2apic) -static inline void x2apic_force_phys (void ) -{ - x2apic_phys =1 ; -}#else -static inline void disable_x2apic (void ) -{ -} -static inline void enable_x2apic (void ) -{ -} -static inline int x2apic_enabled (void ) -{ -return 0 ; -} -static inline void x2apic_force_phys (void ) -{ -} +static inline void disable_x2apic (void ) { } +static inline void enable_x2apic (void ) { } +static inline int x2apic_enabled (void ) {return 0 ; }#define x2apic_mode (0)#define x2apic_supported () (0)
arch/x86/kernel/apic/apic.c +32 -33
86 lines not on this page · raw message
Borislav Petkov
maintainerRe: [patch 07/23] x86/x2apic: Move code in conditional region
Quoted 10 lines · Thomas Gleixner
No point in having
try_to_enable_x2apic()outside of theCONFIG_X86_X2APICsection and having inline functions and more ifdefs to deal with it. Move the code into the existing ifdef section and remove the inline cruft.Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/include/asm/apic.h | 20 ++----------- arch/x86/kernel/apic/apic.c | 65 +++++++++++++++++++++----------------------- 2 files changed, 35 insertions(+), 50 deletions(-)
...
arch/x86/kernel/apic/apic.c static __init void try_to_enable_x2apic( · line 1620
Quoted 4 lines
arch/x86/kernel/apic/apic.c +4
-0+#ifdef CONFIG_X86_IO_APIC +if (!x2apic_enabled () && skip_ioapic_setup) { +pr_info ("Skipped enabling intr-remap because of skipping " +"io-apic setup\n" );
Let's correct this message while we're here:
pr_info("Not enabling interrupt remapping due to skipped IO-APIC setup.\n");Quoted 10 lines
arch/x86/kernel/apic/apic.c +3
-0+ return -1; + }#endif +return irq_remapping_enable (); }void __init enable_IR_x2apic (void )
Signature · 5 lines
Regards/Gruss,
Boris.
Sent from a fat crate under my desk. Formatting is fine.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/x2apic: Move code in conditional region
In reply to Thomas Gleixnerx86/x2apic: Move code in conditional region
Commit-ID: 55eae7de727e9ecc814853ec364fbbb352c337df Gitweb: http://git.kernel.org/tip/55eae7de727e9ecc814853ec364fbbb352c337df Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:19 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:54 +0100
x86/x2apic: Move code in conditional region
No point in having try_to_enable_x2apic() outside of the CONFIG_X86_X2APIC section and having inline functions and more ifdefs to deal with it. Move the code into the existing ifdef section and remove the inline cruft.
Fixup the printk about not enabling interrupt remapping as suggested by Boris.
--- arch/x86/include/asm/apic.h | 24 +++------------- arch/x86/kernel/apic/apic.c | 67 ++++++++++++++++++++++----------------------- 2 files changed, 37 insertions(+), 54 deletions(-)
arch/x86/include/asm/apic.h +4 -20
@@ -186,27 +186,11 @@ static inline int x2apic_enabled(void) }#define x2apic_supported () (cpu_has_x2apic) -static inline void x2apic_force_phys (void ) -{ - x2apic_phys =1 ; -}#else -static inline void disable_x2apic (void ) -{ -} -static inline void check_x2apic (void ) -{ -} -static inline void enable_x2apic (void ) -{ -} -static inline int x2apic_enabled (void ) -{ -return 0 ; -} -static inline void x2apic_force_phys (void ) -{ -} +static inline void disable_x2apic (void ) { } +static inline void check_x2apic (void ) { } +static inline void enable_x2apic (void ) { } +static inline int x2apic_enabled (void ) {return 0 ; }#define x2apic_mode (0)#define x2apic_supported () (0)
arch/x86/kernel/apic/apic.c +33 -34
88 lines not on this page · raw message
PATCH 8/23 x86/x2apic: Clarify remapping mode for x2apic enablement
In reply to Thomas GleixnerRename the argument of try_to_enable_x2apic() so the purpose becomes more clear.
Make the pr_warning more consistent and avoid the double print of "disabling".
--- arch/x86/kernel/apic/apic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kernel/apic/apic.c +4 -4
@@ -1561,19 +1561,19 @@ void enable_x2apic(void) } } -static __init void try_to_enable_x2apic (int ir_stat) +static __init void try_to_enable_x2apic (int remap_mode) {if (!x2apic_supported ())return ; -if (ir_stat !=IRQ_REMAP_X2APIC_MODE ) { +if (remap_mode !=IRQ_REMAP_X2APIC_MODE ) {/* IR is required if there is APIC ID > 255 even when running * under KVM */ if (max_physical_apicid >255 || (IS_ENABLED (CONFIG_HYPERVISOR_GUEST ) && !hypervisor_x2apic_available ())) { -pr_info ("IRQ remapping doesn't support X2APIC mode, disable x2apic.\n" ); +pr_info ("x2apic: IRQ remapping doesn't support X2APIC mode\n" );disable_x2apic ();return ; }
@@ -1609,7 +1609,7 @@ void __init check_x2apic(void)*/ panic("BIOS has enabled x2apic but kernel doesn't support x2apic, please disable x2apic in BIOS.\n"); } -static inline void try_to_enable_x2apic (int ir_stat) { } +static inline void try_to_enable_x2apic (int remap_mode) { }#endif /* !CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
Re: [patch 08/23] x86/x2apic: Clarify remapping mode for x2apic enablement
Quoted 7 lines · Thomas Gleixner
Rename the argument of
try_to_enable_x2apic()so the purpose becomes more clear.Make the
pr_warningmore consistent and avoid the double print of "disabling".Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/x2apic: Clarify remapping mode for x2apic enablement
In reply to Thomas Gleixnerx86/x2apic: Clarify remapping mode for x2apic enablement
Commit-ID: 62e61633daeb0b53f0506aa6e170e2e4cc75cd65 Gitweb: http://git.kernel.org/tip/62e61633daeb0b53f0506aa6e170e2e4cc75cd65 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:21 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/x2apic: Clarify remapping mode for x2apic enablement
Rename the argument of try_to_enable_x2apic() so the purpose becomes more clear.
Make the pr_warning more consistent and avoid the double print of "disabling".
--- arch/x86/kernel/apic/apic.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
arch/x86/kernel/apic/apic.c +4 -4
@@ -1561,19 +1561,19 @@ void enable_x2apic(void) } } -static __init void try_to_enable_x2apic (int ir_stat) +static __init void try_to_enable_x2apic (int remap_mode) {if (!x2apic_supported ())return ; -if (ir_stat !=IRQ_REMAP_X2APIC_MODE ) { +if (remap_mode !=IRQ_REMAP_X2APIC_MODE ) {/* IR is required if there is APIC ID > 255 even when running * under KVM */ if (max_physical_apicid >255 || (IS_ENABLED (CONFIG_HYPERVISOR_GUEST ) && !hypervisor_x2apic_available ())) { -pr_info ("IRQ remapping doesn't support X2APIC mode, disable x2apic.\n" ); +pr_info ("x2apic: IRQ remapping doesn't support X2APIC mode\n" );disable_x2apic ();return ; }
@@ -1611,7 +1611,7 @@ static int __init validate_x2apic(void) }early_initcall (validate_x2apic); -static inline void try_to_enable_x2apic (int ir_stat) { } +static inline void try_to_enable_x2apic (int remap_mode) { }#endif /* !CONFIG_X86_X2APIC */ static int __init try_to_enable_IR (void )
PATCH 9/23 x86/x2apic: Add proper state tracking
In reply to Thomas GleixnerHaving 3 different variables to track the state is just silly and error prone. Add a proper state tracking variable which covers the three possible states: ON/OFF/DISABLED.
We cannot use x2apic_mode for this as this would require to change all users of x2apic_mode with explicit comparisons for a state value instead of treating it as boolean.
--- arch/x86/kernel/apic/apic.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
arch/x86/kernel/apic/apic.c +14 -1
@@ -1480,6 +1480,14 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode;static int x2apic_disabled; + +enum { +X2APIC_OFF , +X2APIC_ON , +X2APIC_DISABLED , +}; +static int x2apic_state; +static int __init setup_nox2apic (char *str) {if (x2apic_enabled ()) {
@@ -1496,7 +1504,7 @@ static int __init setup_nox2apic(char *ssetup_clear_cpu_cap (X86_FEATURE_X2APIC ); nox2apic = true; - + x2apic_state =X2APIC_DISABLED ;return 0 ; }early_param ("nox2apic" , setup_nox2apic);
@@ -1539,6 +1547,7 @@ static __init void disable_x2apic(void) } x2apic_disabled =1 ; + x2apic_state =X2APIC_DISABLED ; }void enable_x2apic (void )
@@ -1559,6 +1568,7 @@ void enable_x2apic(void)printk_once (KERN_INFO "Enabling x2apic\n" );wrmsrl (MSR_IA32_APICBASE , msr |X2APIC_ENABLE ); } + x2apic_state =X2APIC_ON ; }static __init void try_to_enable_x2apic (int remap_mode)
@@ -1597,6 +1607,9 @@ void __init check_x2apic(void)if (x2apic_enabled ()) {pr_info ("x2apic: enabled by BIOS, switching to x2apic ops\n" ); x2apic_mode =1 ; + x2apic_state =X2APIC_ON ; + }else if (!cpu_has_x2apic) { + x2apic_state =X2APIC_DISABLED ; } }#else /* CONFIG_X86_X2APIC */
Re: [patch 09/23] x86/x2apic: Add proper state tracking
Quoted 9 lines · Thomas Gleixner
Having 3 different variables to track the state is just silly and error prone. Add a proper state tracking variable which covers the three possible states: ON/OFF/DISABLED.
We cannot use
x2apic_modefor this as this would require to change all users ofx2apic_modewith explicit comparisons for a state value instead of treating it as boolean.Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/x2apic: Add proper state tracking
In reply to Thomas Gleixnerx86/x2apic: Add proper state tracking
Commit-ID: 12e189d3cfa4c64de758bde18626184bf32c65fc Gitweb: http://git.kernel.org/tip/12e189d3cfa4c64de758bde18626184bf32c65fc Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:22 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/x2apic: Add proper state tracking
Having 3 different variables to track the state is just silly and error prone. Add a proper state tracking variable which covers the three possible states: ON/OFF/DISABLED.
We cannot use x2apic_mode for this as this would require to change all users of x2apic_mode with explicit comparisons for a state value instead of treating it as boolean.
--- arch/x86/kernel/apic/apic.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
arch/x86/kernel/apic/apic.c +14 -1
@@ -1480,6 +1480,14 @@ static bool nox2apic __initdata;#ifdef CONFIG_X86_X2APIC int x2apic_mode;static int x2apic_disabled; + +enum { +X2APIC_OFF , +X2APIC_ON , +X2APIC_DISABLED , +}; +static int x2apic_state; +static int __init setup_nox2apic (char *str) {if (x2apic_enabled ()) {
@@ -1496,7 +1504,7 @@ static int __init setup_nox2apic(char *str)setup_clear_cpu_cap (X86_FEATURE_X2APIC ); nox2apic = true; - + x2apic_state =X2APIC_DISABLED ;return 0 ; }early_param ("nox2apic" , setup_nox2apic);
@@ -1539,6 +1547,7 @@ static __init void disable_x2apic(void) } x2apic_disabled =1 ; + x2apic_state =X2APIC_DISABLED ; }void enable_x2apic (void )
@@ -1559,6 +1568,7 @@ void enable_x2apic(void)printk_once (KERN_INFO "Enabling x2apic\n" );wrmsrl (MSR_IA32_APICBASE , msr |X2APIC_ENABLE ); } + x2apic_state =X2APIC_ON ; }static __init void try_to_enable_x2apic (int remap_mode)
@@ -1597,6 +1607,9 @@ void __init check_x2apic(void)if (x2apic_enabled ()) {pr_info ("x2apic: enabled by BIOS, switching to x2apic ops\n" ); x2apic_mode =1 ; + x2apic_state =X2APIC_ON ; + }else if (!cpu_has_x2apic) { + x2apic_state =X2APIC_DISABLED ; } }#else /* CONFIG_X86_X2APIC */
PATCH 10/23 x86/x2apic: Disable x2apic from nox2apic setup
In reply to Thomas GleixnerThere is no point in postponing the hardware disablement of x2apic. It can be disabled right away in the nox2apic setup function.
Disable it right away and set the state to DISABLED . This allows to remove all the nox2apic conditionals all over the place.
--- arch/x86/include/asm/apic.h | 1 arch/x86/kernel/apic/apic.c | 59 +++++++++++++++++--------------------------- 2 files changed, 24 insertions(+), 36 deletions(-)
arch/x86/include/asm/apic.h +0 -1
@@ -185,7 +185,6 @@ static inline int x2apic_enabled(void)#define x2apic_supported () (cpu_has_x2apic)#else -static inline void disable_x2apic (void ) { }static inline void enable_x2apic (void ) { }static inline int x2apic_enabled (void ) {return 0 ; }
arch/x86/kernel/apic/apic.c +24 -35
120 lines not on this page · raw message
Borislav Petkov
maintainerRe: [patch 10/23] x86/x2apic: Disable x2apic from nox2apic setup
Quoted 8 lines · Thomas Gleixner
There is no point in postponing the hardware disablement of x2apic. It can be disabled right away in the nox2apic setup function.
Disable it right away and set the state to DISABLED . This allows to remove all the nox2apic conditionals all over the place.
Signed-off-by Thomas Gleixner <[email protected]>---
...
Quoted 42 lines
arch/x86/kernel/apic/apic.c +18
-8@@ -1488,6 +1484,19 @@ enum { };static int x2apic_state; +static inline void disable_x2apic (void ) +{ +u64 msr; + +rdmsrl (MSR_IA32_APICBASE , msr); +if (!(msr &X2APIC_ENABLE )) +return ; +/* Disable xapic and x2apic first and then reenable xapic mode */ +wrmsrl (MSR_IA32_APICBASE , msr & ~(X2APIC_ENABLE |XAPIC_ENABLE )); +wrmsrl (MSR_IA32_APICBASE , msr & ~X2APIC_ENABLE ); +printk_once (KERN_INFO "x2apic disabled\n" ); +} +static int __init setup_nox2apic (char *str) {if (x2apic_enabled ()) {@@ -1498,28 +1507,17 @@ static int __init setup_nox2apic(char *s apicid);return 0 ; } - -pr_warning ("x2apic already enabled. will disable it\n" ); - }else -setup_clear_cpu_cap (X86_FEATURE_X2APIC ); - - nox2apic = true; +pr_warning ("x2apic already enabled.\n" ); +disable_x2apic (); + } +setup_clear_cpu_cap (X86_FEATURE_X2APIC ); x2apic_state =X2APIC_DISABLED ; + x2apic_mode =0 ;return 0 ; }early_param ("nox2apic" , setup_nox2apic); -/* -* Need to disable xapic and x2apic at the same time and then enable xapic mode
arch/x86/kernel/apic/apic.c static int __init setup_nox2apic(char *s · line 1520
Quoted 10 lines
arch/x86/kernel/apic/apic.c +1
-9-*/ -static inline void __disable_x2apic (u64 msr) -{ -wrmsrl (MSR_IA32_APICBASE , - msr & ~(X2APIC_ENABLE |XAPIC_ENABLE )); -wrmsrl (MSR_IA32_APICBASE , msr & ~X2APIC_ENABLE ); -} - -static __init void disable_x2apic (void ) +static __init void x2apic_disable (void )
This is still misleading: we have x2apic_disable() and disable_x2apic(). What is what? Can we clarify them more and maybe prepend one of them with "__" to show which is the lower level helper...
Or is the logic that all functions beginning with the "x2apic_" prefix are called from outside and the "enable_/disable_x2apic" ones are the internal helpers?
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/x2apic: Disable x2apic from nox2apic setup
In reply to Thomas Gleixnerx86/x2apic: Disable x2apic from nox2apic setup
Commit-ID: 44e25ff9e6912347a1a54c757fc75681d0dc42d0 Gitweb: http://git.kernel.org/tip/44e25ff9e6912347a1a54c757fc75681d0dc42d0 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:24 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/x2apic: Disable x2apic from nox2apic setup
There is no point in postponing the hardware disablement of x2apic. It can be disabled right away in the nox2apic setup function.
Disable it right away and set the state to DISABLED . This allows to remove all the nox2apic conditionals all over the place.
--- arch/x86/include/asm/apic.h | 1 - arch/x86/kernel/apic/apic.c | 59 ++++++++++++++++++--------------------------- 2 files changed, 24 insertions(+), 36 deletions(-)
arch/x86/include/asm/apic.h +0 -1
@@ -187,7 +187,6 @@ static inline int x2apic_enabled(void)#define x2apic_supported () (cpu_has_x2apic)#else -static inline void disable_x2apic (void ) { }static inline void check_x2apic (void ) { }static inline void enable_x2apic (void ) { }static inline int x2apic_enabled (void ) {return 0 ; }
arch/x86/kernel/apic/apic.c +24 -35
120 lines not on this page · raw message
PATCH 11/23 x86/x2apic: Split enable and setup function
In reply to Thomas Gleixnerenable_x2apic() is a convoluted unreadable mess because it is used for both enablement in early boot and for setup in cpu_init().
Split the code into x2apic_enable() for enablement and x2apic_setup() for setup of (secondary cpus). Make use of the new state tracking to simplify the logic.
--- arch/x86/include/asm/apic.h | 4 +-- arch/x86/kernel/apic/apic.c | 55 +++++++++++++++++++++++++------------------ arch/x86/kernel/cpu/common.c | 2 - 3 files changed, 36 insertions(+), 25 deletions(-)
arch/x86/include/asm/apic.h +2 -2
@@ -177,7 +177,7 @@ static inline u64 native_x2apic_icr_readextern int x2apic_mode;extern int x2apic_phys; -extern void enable_x2apic (void ); +extern void x2apic_setup (void );static inline int x2apic_enabled (void ) {return cpu_has_x2apic &&apic_is_x2apic_enabled ();
@@ -185,7 +185,7 @@ static inline int x2apic_enabled(void)#define x2apic_supported () (cpu_has_x2apic)#else -static inline void enable_x2apic (void ) { } +static inline void x2apic_setup (void ) { }static inline int x2apic_enabled (void ) {return 0 ; }#define x2apic_mode (0)
arch/x86/kernel/apic/apic.c +33 -22
93 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +1 -1
@@ -1332,7 +1332,7 @@ void cpu_init(void)barrier ();x86_configure_nx (); -enable_x2apic (); +x2apic_setup ();/* * set up and load the per-CPU TSS
Borislav Petkov
maintainerRe: [patch 11/23] x86/x2apic: Split enable and setup function
Quoted 9 lines · Thomas Gleixner
enable_x2apic()is a convoluted unreadable mess because it is used for both enablement in early boot and for setup incpu_init().Split the code into
x2apic_enable()for enablement andx2apic_setup()for setup of (secondary cpus). Make use of the new state tracking to simplify the logic.Signed-off-by Thomas Gleixner <[email protected]>---
...
arch/x86/kernel/apic/apic.c static int __init setup_nox2apic(char *s · line 1539
Quoted 13 lines
arch/x86/kernel/apic/apic.c +9
-0@@ -1517,6 +1528,21 @@ static int __init setup_nox2apic(char *s }early_param ("nox2apic" , setup_nox2apic); +/* Called from cpu_init() to enable x2apic on (secondary) cpus */ +void x2apic_setup (void ) +{ +/* +* If x2apic is not in ON state, disable it if already enabled +* from BIOS. +*/ +if (x2apic_state !=X2APIC_ON ) { +if (cpu_has_apic)
Let's move this test into disable_x2apic() so that callers don't have to pay attention.
Quoted 9 lines
arch/x86/kernel/apic/apic.c +6
-0+disable_x2apic (); +return ; + } +enable_x2apic (); +} +static __init void x2apic_disable (void ) {u64 msr;
...
arch/x86/kernel/cpu/common.c void cpu_init(void) · line 1335
Quoted 10 lines
Index:
tip/arch/x86/kernel/cpu/common.c
===================================================================arch/x86/kernel/cpu/common.c +1
-1@@ -1332,7 +1332,7 @@ void cpu_init(void)barrier ();x86_configure_nx (); -enable_x2apic (); +x2apic_setup ();
64-bit only?
I'd guess so since the original call enable_x2apic() is 64-bit only too but why?
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [patch 11/23] x86/x2apic: Split enable and setup function
Quoted 13 lines · Borislav Petkov
arch/x86/kernel/apic/apic.c static int __init setup_nox2apic(char *s · line 1539
Quoted 9 lines · Thomas Gleixner
arch/x86/kernel/apic/apic.c +9
-0+/* Called from cpu_init() to enable x2apic on (secondary) cpus */ +void x2apic_setup (void ) +{ +/* +* If x2apic is not in ON state, disable it if already enabled +* from BIOS. +*/ +if (x2apic_state !=X2APIC_ON ) { +if (cpu_has_apic)Let's move this test into
disable_x2apic()so that callers don't have to pay attention.
Si, senor!
Quoted 8 lines
arch/x86/kernel/cpu/common.c void cpu_init(void) · line 1335
Quoted 3 lines
arch/x86/kernel/cpu/common.c +1
-1x86_configure_nx (); -enable_x2apic (); +x2apic_setup ();64-bit only?
I'd guess so since the original call
enable_x2apic()is 64-bit only too but why?
Don't ask me. :)
tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/x2apic: Split enable and setup function
In reply to Thomas Gleixnerx86/x2apic: Split enable and setup function
Commit-ID: 659006bf3ae37a08706907ce1a36ddf57c9131d2 Gitweb: http://git.kernel.org/tip/659006bf3ae37a08706907ce1a36ddf57c9131d2 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:26 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/x2apic: Split enable and setup function
enable_x2apic() is a convoluted unreadable mess because it is used for both enablement in early boot and for setup in cpu_init().
Split the code into x2apic_enable() for enablement and x2apic_setup() for setup of (secondary cpus). Make use of the new state tracking to simplify the logic.
--- arch/x86/include/asm/apic.h | 4 +-- arch/x86/kernel/apic/apic.c | 63 ++++++++++++++++++++++++++------------------ arch/x86/kernel/cpu/common.c | 2 +- 3 files changed, 41 insertions(+), 28 deletions(-)
arch/x86/include/asm/apic.h +2 -2
@@ -179,7 +179,7 @@ static inline u64 native_x2apic_icr_read(void)extern int x2apic_mode;extern int x2apic_phys;extern void __init check_x2apic (void ); -extern void enable_x2apic (void ); +extern void x2apic_setup (void );static inline int x2apic_enabled (void ) {return cpu_has_x2apic &&apic_is_x2apic_enabled ();
@@ -188,7 +188,7 @@ static inline int x2apic_enabled(void)#define x2apic_supported () (cpu_has_x2apic)#else static inline void check_x2apic (void ) { } -static inline void enable_x2apic (void ) { } +static inline void x2apic_setup (void ) { }static inline int x2apic_enabled (void ) {return 0 ; }#define x2apic_mode (0)
arch/x86/kernel/apic/apic.c +38 -25
113 lines not on this page · raw message
arch/x86/kernel/cpu/common.c +1 -1
@@ -1332,7 +1332,7 @@ void cpu_init(void)barrier ();x86_configure_nx (); -enable_x2apic (); +x2apic_setup ();/* * set up and load the per-CPU TSS
PATCH 12/23 x86/x2apic: Use state information for disable
In reply to Thomas GleixnerUse the state information to simplify the disable logic further.
--- arch/x86/kernel/apic/apic.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)
arch/x86/kernel/apic/apic.c +10 -15
@@ -1545,26 +1545,21 @@ void x2apic_setup(void)static __init void x2apic_disable (void ) { -u64 msr; +u32 x2apic_id; -if (!cpu_has_x2apic) -return ; +if (x2apic_state !=X2APIC_ON ) + goto out; -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) { -u32 x2apic_id =read_apic_id (); + x2apic_id =read_apic_id (); +if (x2apic_id >=255 ) +panic ("Cannot disable x2apic, id: %08x\n" , x2apic_id); -if (x2apic_id >=255 ) -panic ("Cannot disable x2apic, id: %08x\n" , x2apic_id); - -disable_x2apic (); - - x2apic_mode =0 ; - -register_lapic_address (mp_lapic_addr); - } +disable_x2apic (); +register_lapic_address (mp_lapic_addr); +out: x2apic_state =X2APIC_DISABLED ; + x2apic_mode =0 ; }static __init void x2apic_enable (void )
Re: [patch 12/23] x86/x2apic: Use state information for disable
Quoted 3 lines · Thomas Gleixner
Use the state information to simplify the disable logic further.
Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/x2apic: Use state information for disable
In reply to Thomas Gleixnerx86/x2apic: Use state information for disable
Commit-ID: 6d2d49d2cd0199ce298d111ee7fd405af3344a70 Gitweb: http://git.kernel.org/tip/6d2d49d2cd0199ce298d111ee7fd405af3344a70 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:27 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/x2apic: Use state information for disable
Use the state information to simplify the disable logic further.
--- arch/x86/kernel/apic/apic.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-)
arch/x86/kernel/apic/apic.c +10 -16
@@ -1547,26 +1547,20 @@ void x2apic_setup(void)static __init void x2apic_disable (void ) { -u64 msr; - -if (!cpu_has_x2apic) -return ; - -rdmsrl (MSR_IA32_APICBASE , msr); -if (msr &X2APIC_ENABLE ) { -u32 x2apic_id =read_apic_id (); +u32 x2apic_id; -if (x2apic_id >=255 ) -panic ("Cannot disable x2apic, id: %08x\n" , x2apic_id); +if (x2apic_state !=X2APIC_ON ) + goto out; -__x2apic_disable (); - - x2apic_mode =0 ; - -register_lapic_address (mp_lapic_addr); - } + x2apic_id =read_apic_id (); +if (x2apic_id >=255 ) +panic ("Cannot disable x2apic, id: %08x\n" , x2apic_id); +__x2apic_disable (); +register_lapic_address (mp_lapic_addr); +out: x2apic_state =X2APIC_DISABLED ; + x2apic_mode =0 ; }static __init void x2apic_enable (void )
PATCH 13/23 x86/smpboot: Move smpboot inlines to code
In reply to Thomas GleixnerNo point for a seperate header file.
--- arch/x86/include/asm/smpboot_hooks.h | 68 ----------------------------------- arch/x86/kernel/smpboot.c | 67 +++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 69 deletions(-)
arch/x86/include/asm/smpboot_hooks.h deleted +0 -68
@@ -1,68 +0,0 @@ -/* two abstractions specific to kernel/smpboot.c, mainly to cater to visws -* which needs to alter them. */ - -static inline void smpboot_clear_io_apic_irqs (void ) -{ -#ifdef CONFIG_X86_IO_APIC - io_apic_irqs =0 ; -#endif -} - -static inline void smpboot_setup_warm_reset_vector (unsigned long start_eip) -{ -unsigned long flags; - -spin_lock_irqsave (&rtc_lock, flags); -CMOS_WRITE (0xa ,0xf ); -spin_unlock_irqrestore (&rtc_lock, flags); -local_flush_tlb (); -pr_debug ("1.\n" ); - *((volatile unsigned short *)phys_to_virt (TRAMPOLINE_PHYS_HIGH )) = - start_eip >>4 ; -pr_debug ("2.\n" ); - *((volatile unsigned short *)phys_to_virt (TRAMPOLINE_PHYS_LOW )) = - start_eip &0xf ; -pr_debug ("3.\n" ); -} - -static inline void smpboot_restore_warm_reset_vector (void ) -{ -unsigned long flags; - -/* -* Install writable page 0 entry to set BIOS data area. -*/ -local_flush_tlb (); - -/* -* Paranoid: Set warm reset code and vector here back -* to default values. -*/ -spin_lock_irqsave (&rtc_lock, flags); -CMOS_WRITE (0 ,0xf ); -spin_unlock_irqrestore (&rtc_lock, flags); - - *((volatile u32 *)phys_to_virt (TRAMPOLINE_PHYS_LOW )) =0 ; -} - -static inline void __init smpboot_setup_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC -/* -* Here we can be sure that there is an IO-APIC in the system. Let's -* go and set it up: -*/ -if (!skip_ioapic_setup && nr_ioapics) -setup_IO_APIC (); -else { - nr_ioapics =0 ; - } -#endif -} - -static inline void smpboot_clear_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC - nr_ioapics =0 ; -#endif -}
arch/x86/kernel/smpboot.c +66 -1
79 lines not on this page · raw message
Borislav Petkov
maintainerRe: [patch 13/23] x86/smpboot: Move smpboot inlines to code
Quoted 1 line · Thomas Gleixner
No point for a seperate header file.
s/seperate/separate/
Quoted 6 lines
Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/include/asm/smpboot_hooks.h | 68 ----------------------------------- arch/x86/kernel/smpboot.c | 67 +++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 69 deletions(-)
...
arch/x86/kernel/smpboot.c EXPORT_PER_CPU_SYMBOL(cpu_info); · line 161
Quoted 12 lines
arch/x86/kernel/smpboot.c +12
-0+static inline void __init smpboot_setup_io_apic (void ) +{ +#ifdef CONFIG_X86_IO_APIC +/* +* Here we can be sure that there is an IO-APIC in the system. Let's +* go and set it up: +*/ +if (!skip_ioapic_setup && nr_ioapics) +setup_IO_APIC (); +else { + nr_ioapics =0 ; + }
No need for those braces.
Quoted 13 lines
arch/x86/kernel/smpboot.c +10
-0+#endif +} + +static inline void smpboot_clear_io_apic (void ) +{ +#ifdef CONFIG_X86_IO_APIC + nr_ioapics =0 ; +#endif +} +/* * Report back to the Boot Processor during boot time or to the caller processor * during CPU online.
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/smpboot: Move smpboot inlines to code
In reply to Thomas Gleixnerx86/smpboot: Move smpboot inlines to code
Commit-ID: f77aa308e5a6144a47311ad6905a1a72bc0014f9 Gitweb: http://git.kernel.org/tip/f77aa308e5a6144a47311ad6905a1a72bc0014f9 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:29 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/smpboot: Move smpboot inlines to code
No point for a separate header file.
--- arch/x86/include/asm/smpboot_hooks.h | 68 ------------------------------------ arch/x86/kernel/smpboot.c | 66 +++++++++++++++++++++++++++++++++- 2 files changed, 65 insertions(+), 69 deletions(-)
arch/x86/include/asm/smpboot_hooks.h deleted +0 -68
@@ -1,68 +0,0 @@ -/* two abstractions specific to kernel/smpboot.c, mainly to cater to visws -* which needs to alter them. */ - -static inline void smpboot_clear_io_apic_irqs (void ) -{ -#ifdef CONFIG_X86_IO_APIC - io_apic_irqs =0 ; -#endif -} - -static inline void smpboot_setup_warm_reset_vector (unsigned long start_eip) -{ -unsigned long flags; - -spin_lock_irqsave (&rtc_lock, flags); -CMOS_WRITE (0xa ,0xf ); -spin_unlock_irqrestore (&rtc_lock, flags); -local_flush_tlb (); -pr_debug ("1.\n" ); - *((volatile unsigned short *)phys_to_virt (TRAMPOLINE_PHYS_HIGH )) = - start_eip >>4 ; -pr_debug ("2.\n" ); - *((volatile unsigned short *)phys_to_virt (TRAMPOLINE_PHYS_LOW )) = - start_eip &0xf ; -pr_debug ("3.\n" ); -} - -static inline void smpboot_restore_warm_reset_vector (void ) -{ -unsigned long flags; - -/* -* Install writable page 0 entry to set BIOS data area. -*/ -local_flush_tlb (); - -/* -* Paranoid: Set warm reset code and vector here back -* to default values. -*/ -spin_lock_irqsave (&rtc_lock, flags); -CMOS_WRITE (0 ,0xf ); -spin_unlock_irqrestore (&rtc_lock, flags); - - *((volatile u32 *)phys_to_virt (TRAMPOLINE_PHYS_LOW )) =0 ; -} - -static inline void __init smpboot_setup_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC -/* -* Here we can be sure that there is an IO-APIC in the system. Let's -* go and set it up: -*/ -if (!skip_ioapic_setup && nr_ioapics) -setup_IO_APIC (); -else { - nr_ioapics =0 ; - } -#endif -} - -static inline void smpboot_clear_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC - nr_ioapics =0 ; -#endif -}
arch/x86/kernel/smpboot.c +65 -1
78 lines not on this page · raw message
PATCH 14/23 x86/ioapic: Provide stub functions for IOAPIC=n
In reply to Thomas GleixnerTo avoid lots of ifdeffery provide proper stubs for setup_IO_APIC(), enable_IO_APIC() and setup_ioapic_dest().
--- arch/x86/include/asm/io_apic.h | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/include/asm/io_apic.h +5 -0
@@ -279,6 +279,11 @@ static inline void disable_ioapic_suppor#define native_ioapic_set_affinity NULL#define native_setup_ioapic_entry NULL#define native_eoi_ioapic_pin NULL + +static inline void setup_IO_APIC (void ) { } +static inline void enable_IO_APIC (void ) { } +static inline void setup_ioapic_dest (void ) { } +#endif #endif /* _ASM_X86_IO_APIC_H */
Re: [patch 14/23] x86/ioapic: Provide stub functions for IOAPIC=n
Quoted 4 lines · Thomas Gleixner
To avoid lots of ifdeffery provide proper stubs for
setup_IO_APIC(),enable_IO_APIC()andsetup_ioapic_dest().Signed-off-by Thomas Gleixner <[email protected]>
Very nice, that ifdeffery was uuugly...
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/ioapic: Provide stub functions for IOAPIC%3Dn
In reply to Thomas Gleixnerx86/ioapic: Provide stub functions for IOAPIC%3Dn
Commit-ID: 8686608336e11276d72d020cb0b67bee70d9a5cd Gitweb: http://git.kernel.org/tip/8686608336e11276d72d020cb0b67bee70d9a5cd Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:30 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/ioapic: Provide stub functions for IOAPIC%3Dn
To avoid lots of ifdeffery provide proper stubs for setup_IO_APIC(), enable_IO_APIC() and setup_ioapic_dest().
--- arch/x86/include/asm/io_apic.h | 5 +++++ 1 file changed, 5 insertions(+)
arch/x86/include/asm/io_apic.h +5 -0
@@ -279,6 +279,11 @@ static inline void disable_ioapic_support(void) { }#define native_ioapic_set_affinity NULL#define native_setup_ioapic_entry NULL#define native_eoi_ioapic_pin NULL + +static inline void setup_IO_APIC (void ) { } +static inline void enable_IO_APIC (void ) { } +static inline void setup_ioapic_dest (void ) { } +#endif #endif /* _ASM_X86_IO_APIC_H */
PATCH 15/23 x86/ioapic: Add proper checks to setp/enable_IO_APIC()
In reply to Thomas GleixnerNo point to have the same checks at every call site. Add them to the functions, so they can be called unconditionally.
--- arch/x86/kernel/apic/io_apic.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
arch/x86/kernel/apic/io_apic.c +7 -4
@@ -1507,7 +1507,10 @@ void __init enable_IO_APIC(void)int i8259_apic, i8259_pin;int apic, pin; -if (!nr_legacy_irqs ()) +if (skip_ioapic_setup) + nr_ioapics =0 ; + +if (!nr_legacy_irqs () || !nr_ioapics)return ;for_each_ioapic_pin (apic, pin) {
@@ -2373,9 +2376,9 @@ void __init setup_IO_APIC(void) {int ioapic; -/* -* calling enable_IO_APIC() is moved to setup_local_APIC for BP -*/ +if (skip_ioapic_setup || !nr_ioapics) +return ; + io_apic_irqs =nr_legacy_irqs () ? ~PIC_IRQS : ~0UL ;apic_printk (APIC_VERBOSE ,"ENABLING IO-APIC IRQs\n" );
Re: [patch 15/23] x86/ioapic: Add proper checks to setp/enable_IO_APIC()
Quoted 4 lines · Thomas Gleixner
No point to have the same checks at every call site. Add them to the functions, so they can be called unconditionally.
Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/ioapic: Add proper checks to setp/ enable_IO_APIC()
In reply to Thomas Gleixnerx86/ioapic: Add proper checks to setp/enable_IO_APIC()
Commit-ID: a46f5c89274245e42834dc976896444efd53ccdc Gitweb: http://git.kernel.org/tip/a46f5c89274245e42834dc976896444efd53ccdc Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:32 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/ioapic: Add proper checks to setp/enable_IO_APIC()
No point to have the same checks at every call site. Add them to the functions, so they can be called unconditionally.
--- arch/x86/kernel/apic/io_apic.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
arch/x86/kernel/apic/io_apic.c +7 -4
@@ -1507,7 +1507,10 @@ void __init enable_IO_APIC(void)int i8259_apic, i8259_pin;int apic, pin; -if (!nr_legacy_irqs ()) +if (skip_ioapic_setup) + nr_ioapics =0 ; + +if (!nr_legacy_irqs () || !nr_ioapics)return ;for_each_ioapic_pin (apic, pin) {
@@ -2373,9 +2376,9 @@ void __init setup_IO_APIC(void) {int ioapic; -/* -* calling enable_IO_APIC() is moved to setup_local_APIC for BP -*/ +if (skip_ioapic_setup || !nr_ioapics) +return ; + io_apic_irqs =nr_legacy_irqs () ? ~PIC_IRQS : ~0UL ;apic_printk (APIC_VERBOSE ,"ENABLING IO-APIC IRQs\n" );
PATCH 16/23 x86/apic: Sanitize ioapic handling
In reply to Thomas GleixnerWe have proper stubs for the IOAPIC=n case and the setup/enable function have the required checks inside now. Remove the ifdeffery and the copy&pasted conditionals.
--- arch/x86/kernel/apic/apic.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-)
arch/x86/kernel/apic/apic.c +3 -14
@@ -1903,24 +1903,13 @@ int __init APIC_init_uniprocessor(void)physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map);setup_local_APIC (); -#ifdef CONFIG_X86_IO_APIC -/* -* Now enable IO-APICs, actually call clear_IO_APIC -* We need clear_IO_APIC before enabling error vector -*/ -if (!skip_ioapic_setup && nr_ioapics) -enable_IO_APIC (); -#endif +/* Enable IO-APICs before enabling error vector */ +enable_IO_APIC ();bsp_end_local_APIC_setup (); -#ifdef CONFIG_X86_IO_APIC -if (smp_found_config && !skip_ioapic_setup && nr_ioapics) +if (smp_found_config)setup_IO_APIC (); -else { - nr_ioapics =0 ; - } -#endif x86_init.timers.setup_percpu_clockev ();return 0 ;
Re: [patch 16/23] x86/apic: Sanitize ioapic handling
Quoted 5 lines · Thomas Gleixner
We have proper stubs for the IOAPIC=n case and the setup/enable function have the required checks inside now. Remove the ifdeffery and the copy&pasted conditionals.
Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Sanitize ioapic handling
In reply to Thomas Gleixnerx86/apic: Sanitize ioapic handling
Commit-ID: 35e4c6d30e6f69745d77afd5f63203ad440bed12 Gitweb: http://git.kernel.org/tip/35e4c6d30e6f69745d77afd5f63203ad440bed12 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:34 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:55 +0100
x86/apic: Sanitize ioapic handling
We have proper stubs for the IOAPIC=n case and the setup/enable function have the required checks inside now. Remove the ifdeffery and the copy&pasted conditionals.
--- arch/x86/kernel/apic/apic.c | 17 +++-------------- 1 file changed, 3 insertions(+), 14 deletions(-)
arch/x86/kernel/apic/apic.c +3 -14
@@ -1905,24 +1905,13 @@ int __init APIC_init_uniprocessor(void)physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map);setup_local_APIC (); -#ifdef CONFIG_X86_IO_APIC -/* -* Now enable IO-APICs, actually call clear_IO_APIC -* We need clear_IO_APIC before enabling error vector -*/ -if (!skip_ioapic_setup && nr_ioapics) -enable_IO_APIC (); -#endif +/* Enable IO-APICs before enabling error vector */ +enable_IO_APIC ();bsp_end_local_APIC_setup (); -#ifdef CONFIG_X86_IO_APIC -if (smp_found_config && !skip_ioapic_setup && nr_ioapics) +if (smp_found_config)setup_IO_APIC (); -else { - nr_ioapics =0 ; - } -#endif x86_init.timers.setup_percpu_clockev ();return 0 ;
PATCH 17/23 x86/smpboot: Cleanup ioapic handling
In reply to Thomas Gleixnersmpboot is very creative with the ways to disable ioapic.
smpboot_clear_io_apic() smpboot_clear_io_apic_irqs() and disable_ioapic_support() serve a similar purpose.
smpboot_clear_io_apic_irqs() is the most useless of all functions as it clears a variable which has not been setup yet.
Aside of that it has the same ifdef mess and conditionals around the ioapic related code, which can now be removed.
--- arch/x86/kernel/smpboot.c | 48 ++++++---------------------------------------- 1 file changed, 7 insertions(+), 41 deletions(-)
arch/x86/kernel/smpboot.c +7 -41
@@ -103,13 +103,6 @@ EXPORT_PER_CPU_SYMBOL(cpu_info);atomic_t init_deasserted; -static inline void smpboot_clear_io_apic_irqs (void ) -{ -#ifdef CONFIG_X86_IO_APIC - io_apic_irqs =0 ; -#endif -} -static inline void smpboot_setup_warm_reset_vector (unsigned long start_eip) {unsigned long flags;
@@ -147,28 +140,6 @@ static inline void smpboot_restore_warm_ *((volatile u32 *)phys_to_virt (TRAMPOLINE_PHYS_LOW )) =0 ; } -static inline void __init smpboot_setup_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC -/* -* Here we can be sure that there is an IO-APIC in the system. Let's -* go and set it up: -*/ -if (!skip_ioapic_setup && nr_ioapics) -setup_IO_APIC (); -else { - nr_ioapics =0 ; - } -#endif -} - -static inline void smpboot_clear_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC - nr_ioapics =0 ; -#endif -} -/* * Report back to the Boot Processor during boot time or to the caller processor * during CPU online.
@@ -1020,9 +991,10 @@ void arch_disable_smp_support(void)*/ static __init void disable_smp (void ) { +disable_ioapic_support (); +init_cpu_present (cpumask_of (0 ));init_cpu_possible (cpumask_of (0 )); -smpboot_clear_io_apic_irqs ();if (smp_found_config)physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map);
@@ -1106,7 +1078,6 @@ static int __init smp_sanity_check(unsig boot_cpu_physical_apicid);pr_err ("... forcing use of dummy APIC emulation (tell your hw vendor)\n" ); } -smpboot_clear_io_apic ();disable_ioapic_support ();return -1 ; }
@@ -1118,7 +1089,7 @@ static int __init smp_sanity_check(unsig*/ if (!max_cpus) {pr_info ("SMP mode deactivated\n" ); -smpboot_clear_io_apic (); +disable_ioapic_support ();connect_bsp_APIC ();setup_local_APIC ();
@@ -1192,18 +1163,15 @@ void __init native_smp_prepare_cpus(unsielse cpu0_logical_apicid =GET_APIC_LOGICAL_ID (apic_read (APIC_LDR )); -/* -* Enable IO APIC before setting up error vector -*/ -if (!skip_ioapic_setup && nr_ioapics) -enable_IO_APIC (); +/* Enable IO APIC before setting up error vector */ +enable_IO_APIC ();bsp_end_local_APIC_setup (); -smpboot_setup_io_apic (); +setup_IO_APIC (); +/* * Set up local APIC timer on boot CPU. */ -pr_info ("CPU%d: " ,0 );print_cpu_info (&cpu_data (0 )); x86_init.timers.setup_percpu_clockev ();
@@ -1242,9 +1210,7 @@ void __init native_smp_cpus_done(unsignenmi_selftest ();impress_friends (); -#ifdef CONFIG_X86_IO_APIC setup_ioapic_dest (); -#endif mtrr_aps_init (); }
Borislav Petkov
maintainerRe: [patch 17/23] x86/smpboot: Cleanup ioapic handling
Quoted 7 lines · Thomas Gleixner
smpboot is very creative with the ways to disable ioapic.
smpboot_clear_io_apic()smpboot_clear_io_apic_irqs()anddisable_ioapic_support()serve a similar purpose.
smpboot_clear_io_apic_irqs()is the most useless of all functions as it clears a variable which has not been setup yet.
Useful stuff that.
Quoted 4 lines
Aside of that it has the same ifdef mess and conditionals around the ioapic related code, which can now be removed.
Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/smpboot: Cleanup ioapic handling
In reply to Thomas Gleixnerx86/smpboot: Cleanup ioapic handling
Commit-ID: ef4c59a4b64c62f977187cae444aee25bebb02fe Gitweb: http://git.kernel.org/tip/ef4c59a4b64c62f977187cae444aee25bebb02fe Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:35 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
x86/smpboot: Cleanup ioapic handling
smpboot is very creative with the ways to disable ioapic.
smpboot_clear_io_apic() smpboot_clear_io_apic_irqs() and disable_ioapic_support() serve a similar purpose.
smpboot_clear_io_apic_irqs() is the most useless of all functions as it clears a variable which has not been setup yet.
Aside of that it has the same ifdef mess and conditionals around the ioapic related code, which can now be removed.
--- arch/x86/kernel/smpboot.c | 47 +++++++---------------------------------------- 1 file changed, 7 insertions(+), 40 deletions(-)
arch/x86/kernel/smpboot.c +7 -40
@@ -103,13 +103,6 @@ EXPORT_PER_CPU_SYMBOL(cpu_info);atomic_t init_deasserted; -static inline void smpboot_clear_io_apic_irqs (void ) -{ -#ifdef CONFIG_X86_IO_APIC - io_apic_irqs =0 ; -#endif -} -static inline void smpboot_setup_warm_reset_vector (unsigned long start_eip) {unsigned long flags;
@@ -147,27 +140,6 @@ static inline void smpboot_restore_warm_reset_vector(void) *((volatile u32 *)phys_to_virt (TRAMPOLINE_PHYS_LOW )) =0 ; } -static inline void __init smpboot_setup_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC -/* -* Here we can be sure that there is an IO-APIC in the system. Let's -* go and set it up: -*/ -if (!skip_ioapic_setup && nr_ioapics) -setup_IO_APIC (); -else - nr_ioapics =0 ; -#endif -} - -static inline void smpboot_clear_io_apic (void ) -{ -#ifdef CONFIG_X86_IO_APIC - nr_ioapics =0 ; -#endif -} -/* * Report back to the Boot Processor during boot time or to the caller processor * during CPU online.
@@ -1019,9 +991,10 @@ void arch_disable_smp_support(void)*/ static __init void disable_smp (void ) { +disable_ioapic_support (); +init_cpu_present (cpumask_of (0 ));init_cpu_possible (cpumask_of (0 )); -smpboot_clear_io_apic_irqs ();if (smp_found_config)physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map);
@@ -1105,7 +1078,6 @@ static int __init smp_sanity_check(unsigned max_cpus) boot_cpu_physical_apicid);pr_err ("... forcing use of dummy APIC emulation (tell your hw vendor)\n" ); } -smpboot_clear_io_apic ();disable_ioapic_support ();return -1 ; }
@@ -1117,7 +1089,7 @@ static int __init smp_sanity_check(unsigned max_cpus)*/ if (!max_cpus) {pr_info ("SMP mode deactivated\n" ); -smpboot_clear_io_apic (); +disable_ioapic_support ();connect_bsp_APIC ();setup_local_APIC ();
@@ -1191,18 +1163,15 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)else cpu0_logical_apicid =GET_APIC_LOGICAL_ID (apic_read (APIC_LDR )); -/* -* Enable IO APIC before setting up error vector -*/ -if (!skip_ioapic_setup && nr_ioapics) -enable_IO_APIC (); +/* Enable IO APIC before setting up error vector */ +enable_IO_APIC ();bsp_end_local_APIC_setup (); -smpboot_setup_io_apic (); +setup_IO_APIC (); +/* * Set up local APIC timer on boot CPU. */ -pr_info ("CPU%d: " ,0 );print_cpu_info (&cpu_data (0 )); x86_init.timers.setup_percpu_clockev ();
@@ -1241,9 +1210,7 @@ void __init native_smp_cpus_done(unsigned int max_cpus)nmi_selftest ();impress_friends (); -#ifdef CONFIG_X86_IO_APIC setup_ioapic_dest (); -#endif mtrr_aps_init (); }
PATCH 18/23 x86/apic: Move apic_init_uniprocessor code
In reply to Thomas GleixnerMove the code to a different place so we can make other functions inline. Preparatory patch for further cleanups. No change.
--- arch/x86/kernel/apic/apic.c | 125 +++++++++++++++++++++----------------------- 1 file changed, 62 insertions(+), 63 deletions(-)
arch/x86/kernel/apic/apic.c +62 -63
@@ -1850,71 +1850,8 @@ void __init register_lapic_address(unsig } } -/* -* This initializes the IO-APIC and APIC hardware if this is -* a UP kernel. -*/ int apic_version[MAX_LOCAL_APIC ]; -int __init APIC_init_uniprocessor (void ) -{ -if (disable_apic) { -pr_info ("Apic disabled\n" ); -return -1 ; - } -#ifdef CONFIG_X86_64 -if (!cpu_has_apic) { - disable_apic =1 ; -pr_info ("Apic disabled by BIOS\n" ); -return -1 ; - } -#else -if (!smp_found_config && !cpu_has_apic) -return -1 ; - -/* -* Complain if the BIOS pretends there is one. -*/ -if (!cpu_has_apic && -APIC_INTEGRATED (apic_version[boot_cpu_physical_apicid])) { -pr_err ("BIOS bug, local APIC 0x%x not detected!...\n" , - boot_cpu_physical_apicid); -return -1 ; - } -#endif - -default_setup_apic_routing (); - -verify_local_APIC (); -connect_bsp_APIC (); - -#ifdef CONFIG_X86_64 -apic_write (APIC_ID ,SET_APIC_ID (boot_cpu_physical_apicid)); -#else -/* -* Hack: In case of kdump, after a crash, kernel might be booting -* on a cpu with non-zero lapic id. But boot_cpu_physical_apicid -* might be zero if read from MP tables. Get it from LAPIC. -*/ -# ifdef CONFIG_CRASH_DUMP - boot_cpu_physical_apicid =read_apic_id (); -# endif -#endif -physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map); -setup_local_APIC (); - -/* Enable IO-APICs before enabling error vector */ -enable_IO_APIC (); - -bsp_end_local_APIC_setup (); - -if (smp_found_config) -setup_IO_APIC (); - - x86_init.timers.setup_percpu_clockev (); -return 0 ; -} -/* * Local APIC interrupts */
@@ -2267,6 +2204,68 @@ void __init apic_set_eoi_write(void (*eo }/* +* This initializes the IO-APIC and APIC hardware if this is +* a UP kernel. +*/ +int __init APIC_init_uniprocessor (void ) +{ +if (disable_apic) { +pr_info ("Apic disabled\n" ); +return -1 ; + } +#ifdef CONFIG_X86_64 +if (!cpu_has_apic) { + disable_apic =1 ; +pr_info ("Apic disabled by BIOS\n" ); +return -1 ; + } +#else +if (!smp_found_config && !cpu_has_apic) +return -1 ; + +/* +* Complain if the BIOS pretends there is one. +*/ +if (!cpu_has_apic && +APIC_INTEGRATED (apic_version[boot_cpu_physical_apicid])) { +pr_err ("BIOS bug, local APIC 0x%x not detected!...\n" , + boot_cpu_physical_apicid); +return -1 ; + } +#endif + +default_setup_apic_routing (); + +verify_local_APIC (); +connect_bsp_APIC (); + +#ifdef CONFIG_X86_64 +apic_write (APIC_ID ,SET_APIC_ID (boot_cpu_physical_apicid)); +#else +/* +* Hack: In case of kdump, after a crash, kernel might be booting +* on a cpu with non-zero lapic id. But boot_cpu_physical_apicid +* might be zero if read from MP tables. Get it from LAPIC. +*/ +# ifdef CONFIG_CRASH_DUMP + boot_cpu_physical_apicid =read_apic_id (); +# endif +#endif +physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map); +setup_local_APIC (); + +if (smp_found_config) +enable_IO_APIC (); + +bsp_end_local_APIC_setup (); + +setup_IO_APIC (); + + x86_init.timers.setup_percpu_clockev (); +return 0 ; +} + +/* * Power management */ #ifdef CONFIG_PM
Re: [patch 18/23] x86/apic: Move apic_init_uniprocessor code
Quoted 4 lines · Thomas Gleixner
Move the code to a different place so we can make other functions inline. Preparatory patch for further cleanups. No change.
Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Move apic_init_uniprocessor code
In reply to Thomas Gleixnerx86/apic: Move apic_init_uniprocessor code
Commit-ID: e714a91f92ca59f7e71e7332b8ec2aa2944f629e Gitweb: http://git.kernel.org/tip/e714a91f92ca59f7e71e7332b8ec2aa2944f629e Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:37 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
x86/apic: Move apic_init_uniprocessor code
Move the code to a different place so we can make other functions inline. Preparatory patch for further cleanups. No change.
--- arch/x86/kernel/apic/apic.c | 125 ++++++++++++++++++++++---------------------- 1 file changed, 62 insertions(+), 63 deletions(-)
arch/x86/kernel/apic/apic.c +62 -63
@@ -1852,71 +1852,8 @@ void __init register_lapic_address(unsigned long address) } } -/* -* This initializes the IO-APIC and APIC hardware if this is -* a UP kernel. -*/ int apic_version[MAX_LOCAL_APIC ]; -int __init APIC_init_uniprocessor (void ) -{ -if (disable_apic) { -pr_info ("Apic disabled\n" ); -return -1 ; - } -#ifdef CONFIG_X86_64 -if (!cpu_has_apic) { - disable_apic =1 ; -pr_info ("Apic disabled by BIOS\n" ); -return -1 ; - } -#else -if (!smp_found_config && !cpu_has_apic) -return -1 ; - -/* -* Complain if the BIOS pretends there is one. -*/ -if (!cpu_has_apic && -APIC_INTEGRATED (apic_version[boot_cpu_physical_apicid])) { -pr_err ("BIOS bug, local APIC 0x%x not detected!...\n" , - boot_cpu_physical_apicid); -return -1 ; - } -#endif - -default_setup_apic_routing (); - -verify_local_APIC (); -connect_bsp_APIC (); - -#ifdef CONFIG_X86_64 -apic_write (APIC_ID ,SET_APIC_ID (boot_cpu_physical_apicid)); -#else -/* -* Hack: In case of kdump, after a crash, kernel might be booting -* on a cpu with non-zero lapic id. But boot_cpu_physical_apicid -* might be zero if read from MP tables. Get it from LAPIC. -*/ -# ifdef CONFIG_CRASH_DUMP - boot_cpu_physical_apicid =read_apic_id (); -# endif -#endif -physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map); -setup_local_APIC (); - -/* Enable IO-APICs before enabling error vector */ -enable_IO_APIC (); - -bsp_end_local_APIC_setup (); - -if (smp_found_config) -setup_IO_APIC (); - - x86_init.timers.setup_percpu_clockev (); -return 0 ; -} -/* * Local APIC interrupts */
@@ -2269,6 +2206,68 @@ void __init apic_set_eoi_write(void (*eoi_write)(u32 reg, u32 v)) }/* +* This initializes the IO-APIC and APIC hardware if this is +* a UP kernel. +*/ +int __init APIC_init_uniprocessor (void ) +{ +if (disable_apic) { +pr_info ("Apic disabled\n" ); +return -1 ; + } +#ifdef CONFIG_X86_64 +if (!cpu_has_apic) { + disable_apic =1 ; +pr_info ("Apic disabled by BIOS\n" ); +return -1 ; + } +#else +if (!smp_found_config && !cpu_has_apic) +return -1 ; + +/* +* Complain if the BIOS pretends there is one. +*/ +if (!cpu_has_apic && +APIC_INTEGRATED (apic_version[boot_cpu_physical_apicid])) { +pr_err ("BIOS bug, local APIC 0x%x not detected!...\n" , + boot_cpu_physical_apicid); +return -1 ; + } +#endif + +default_setup_apic_routing (); + +verify_local_APIC (); +connect_bsp_APIC (); + +#ifdef CONFIG_X86_64 +apic_write (APIC_ID ,SET_APIC_ID (boot_cpu_physical_apicid)); +#else +/* +* Hack: In case of kdump, after a crash, kernel might be booting +* on a cpu with non-zero lapic id. But boot_cpu_physical_apicid +* might be zero if read from MP tables. Get it from LAPIC. +*/ +# ifdef CONFIG_CRASH_DUMP + boot_cpu_physical_apicid =read_apic_id (); +# endif +#endif +physid_set_mask_of_physid (boot_cpu_physical_apicid, &phys_cpu_present_map); +setup_local_APIC (); + +if (smp_found_config) +enable_IO_APIC (); + +bsp_end_local_APIC_setup (); + +setup_IO_APIC (); + + x86_init.timers.setup_percpu_clockev (); +return 0 ; +} + +/* * Power management */ #ifdef CONFIG_PM
PATCH 19/23 init: Get rid of x86isms
In reply to Thomas GleixnerThe UP local API support can be set up from an early initcall. No need for horrible hackery in the init code.
--- arch/x86/kernel/apic/apic.c | 3 +++ init/main.c | 14 +------------- 2 files changed, 4 insertions(+), 13 deletions(-)
arch/x86/kernel/apic/apic.c +3 -0
@@ -2264,6 +2264,9 @@ int __init APIC_init_uniprocessor(void) x86_init.timers.setup_percpu_clockev ();return 0 ; } +#ifndef CONFIG_SMP +early_initcall (APIC_init_uniprocessor); +#endif /* * Power management
init/main.c +1 -13
@@ -87,10 +87,6 @@#include <asm/sections.h> #include <asm/cacheflush.h> -#ifdef CONFIG_X86_LOCAL_APIC -#include <asm/smp.h> -#endif -static int kernel_init (void *);extern void init_IRQ (void );
@@ -351,15 +347,7 @@ __setup("rdinit=", rdinit_setup);#ifndef CONFIG_SMP static const unsigned int setup_max_cpus =NR_CPUS ; -#ifdef CONFIG_X86_LOCAL_APIC -static void __init smp_init (void ) -{ -APIC_init_uniprocessor (); -} -#else -#define smp_init () do { } while (0) -#endif - +static inline void smp_init (void ) { }static inline void setup_nr_cpu_ids (void ) { }static inline void smp_prepare_cpus (unsigned int maxcpus) { }#endif
tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] init: Get rid of x86isms
init: Get rid of x86isms
Commit-ID: 30b8b0066cafef274fc92462578ee346211ce7cb Gitweb: http://git.kernel.org/tip/30b8b0066cafef274fc92462578ee346211ce7cb Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:39 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
init: Get rid of x86isms
The UP local API support can be set up from an early initcall. No need for horrible hackery in the init code.
--- arch/x86/Kconfig | 4 ++++ arch/x86/kernel/apic/apic.c | 7 +++++++ include/linux/smp.h | 7 +++++++ init/main.c | 13 ------------- 4 files changed, 18 insertions(+), 13 deletions(-)
arch/x86/Kconfig +4 -0
@@ -855,6 +855,10 @@ config SCHED_MC source "kernel/Kconfig.preempt" +config UP_LATE_INIT + def_bool y + depends on X86_UP_APIC + config X86_UP_APIC bool "Local APIC support on uniprocessors" depends on X86_32 && !SMP && !X86_32_NON_STANDARD && !PCI_MSI
arch/x86/kernel/apic/apic.c +7 -0
@@ -2267,6 +2267,13 @@ int __init APIC_init_uniprocessor(void)return 0 ; } +#ifdef CONFIG_UP_LATE_INIT +void __init up_late_init (void ) +{ +APIC_init_uniprocessor (); +} +#endif +/* * Power management */
include/linux/smp.h +7 -0
@@ -151,6 +151,13 @@ smp_call_function_any(const struct cpumask *mask, smp_call_func_t func,static inline void kick_all_cpus_sync (void ) { }static inline void wake_up_all_idle_cpus (void ) { } +#ifdef CONFIG_UP_LATE_INIT +extern void __init up_late_init (void ); +static inline void smp_init (void ) {up_late_init (); } +#else +static inline void smp_init (void ) { } +#endif +#endif /* !SMP */ /*
init/main.c +0 -13
@@ -87,10 +87,6 @@#include <asm/sections.h> #include <asm/cacheflush.h> -#ifdef CONFIG_X86_LOCAL_APIC -#include <asm/smp.h> -#endif -static int kernel_init (void *);extern void init_IRQ (void );
@@ -351,15 +347,6 @@ __setup("rdinit=", rdinit_setup);#ifndef CONFIG_SMP static const unsigned int setup_max_cpus =NR_CPUS ; -#ifdef CONFIG_X86_LOCAL_APIC -static void __init smp_init (void ) -{ -APIC_init_uniprocessor (); -} -#else -#define smp_init () do { } while (0) -#endif -static inline void setup_nr_cpu_ids (void ) { }static inline void smp_prepare_cpus (unsigned int maxcpus) { }#endif
PATCH 20/23 x86/smpboot: Move apic init code to apic.c
In reply to Thomas GleixnerWe better provide proper functions which implement the required code flow in the apic code rather than letting the smpboot code open code it. That allows to make more functions static and confines the APIC functionality to apic.c where it belongs.
--- arch/x86/include/asm/apic.h | 6 ++--- arch/x86/kernel/apic/apic.c | 51 +++++++++++++++++++++++++++++++++++--------- arch/x86/kernel/smpboot.c | 27 ++--------------------- 3 files changed, 47 insertions(+), 37 deletions(-)
arch/x86/include/asm/apic.h +3 -3
@@ -198,7 +198,6 @@ extern int get_physical_broadcast(void);extern int lapic_get_maxlvt (void );extern void clear_local_APIC (void ); -extern void connect_bsp_APIC (void );extern void disconnect_bsp_APIC (int virt_wire_setup);extern void disable_local_APIC (void );extern void lapic_shutdown (void );
@@ -206,8 +205,6 @@ extern int verify_local_APIC(void);extern void sync_Arb_IDs (void );extern void init_bsp_APIC (void );extern void setup_local_APIC (void ); -extern void end_local_APIC_setup (void ); -extern void bsp_end_local_APIC_setup (void );extern void init_apic_mappings (void );void register_lapic_address (unsigned long address);extern void setup_boot_APIC_clock (void );
@@ -215,6 +212,9 @@ extern void setup_secondary_APIC_clock(vextern int APIC_init_uniprocessor (void );extern int apic_force_enable (unsigned long addr); +extern int apic_bsp_setup (void ); +extern void apic_ap_setup (void ); +/* * On 32bit this is mach-xxx local */
arch/x86/kernel/apic/apic.c +41 -10
77 lines not on this page · raw message
arch/x86/kernel/smpboot.c +3 -24
@@ -172,8 +172,7 @@ static void smp_callin(void)* CPU, first the APIC. (this is probably redundant on most * boards) */ -setup_local_APIC (); -end_local_APIC_setup (); +apic_ap_setup ();/* * Need to setup vector mappings before we enable interrupts.
@@ -1078,7 +1077,6 @@ static int __init smp_sanity_check(unsig boot_cpu_physical_apicid);pr_err ("... forcing use of dummy APIC emulation (tell your hw vendor)\n" ); } -disable_ioapic_support ();return -1 ; }
@@ -1090,10 +1088,7 @@ static int __init smp_sanity_check(unsigif (!max_cpus) {pr_info ("SMP mode deactivated\n" );disable_ioapic_support (); - -connect_bsp_APIC (); -setup_local_APIC (); -bsp_end_local_APIC_setup (); +apic_bsp_setup ();return -1 ; }
@@ -1151,23 +1146,7 @@ void __init native_smp_prepare_cpus(unsi/* Or can we switch back to PIC here? */ } -connect_bsp_APIC (); - -/* -* Switch from PIC to APIC mode. -*/ -setup_local_APIC (); - -if (x2apic_mode) - cpu0_logical_apicid =apic_read (APIC_LDR ); -else - cpu0_logical_apicid =GET_APIC_LOGICAL_ID (apic_read (APIC_LDR )); - -/* Enable IO APIC before setting up error vector */ -enable_IO_APIC (); - -bsp_end_local_APIC_setup (); -setup_IO_APIC (); + cpu0_logical_apicid =apic_bsp_setup ();/* * Set up local APIC timer on boot CPU.
Re: [patch 20/23] x86/smpboot: Move apic init code to apic.c
Quoted 6 lines · Thomas Gleixner
We better provide proper functions which implement the required code flow in the apic code rather than letting the smpboot code open code it. That allows to make more functions static and confines the APIC functionality to
apic.cwhere it belongs.Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/smpboot: Move apic init code to apic.c
In reply to Thomas Gleixnerx86/smpboot: Move apic init code to apic.c
Commit-ID: 05f7e46d2aac359b6bcfc06b302bdd03ca0bcada Gitweb: http://git.kernel.org/tip/05f7e46d2aac359b6bcfc06b302bdd03ca0bcada Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:40 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
x86/smpboot: Move apic init code to apic.c
We better provide proper functions which implement the required code flow in the apic code rather than letting the smpboot code open code it. That allows to make more functions static and confines the APIC functionality to apic.c where it belongs.
--- arch/x86/include/asm/apic.h | 6 +++--- arch/x86/kernel/apic/apic.c | 51 ++++++++++++++++++++++++++++++++++++--------- arch/x86/kernel/smpboot.c | 27 +++--------------------- 3 files changed, 47 insertions(+), 37 deletions(-)
arch/x86/include/asm/apic.h +3 -3
@@ -201,7 +201,6 @@ extern int get_physical_broadcast(void);extern int lapic_get_maxlvt (void );extern void clear_local_APIC (void ); -extern void connect_bsp_APIC (void );extern void disconnect_bsp_APIC (int virt_wire_setup);extern void disable_local_APIC (void );extern void lapic_shutdown (void );
@@ -209,8 +208,6 @@ extern int verify_local_APIC(void);extern void sync_Arb_IDs (void );extern void init_bsp_APIC (void );extern void setup_local_APIC (void ); -extern void end_local_APIC_setup (void ); -extern void bsp_end_local_APIC_setup (void );extern void init_apic_mappings (void );void register_lapic_address (unsigned long address);extern void setup_boot_APIC_clock (void );
@@ -218,6 +215,9 @@ extern void setup_secondary_APIC_clock(void);extern int APIC_init_uniprocessor (void );extern int apic_force_enable (unsigned long addr); +extern int apic_bsp_setup (void ); +extern void apic_ap_setup (void ); +/* * On 32bit this is mach-xxx local */
arch/x86/kernel/apic/apic.c +41 -10
77 lines not on this page · raw message
arch/x86/kernel/smpboot.c +3 -24
@@ -172,8 +172,7 @@ static void smp_callin(void)* CPU, first the APIC. (this is probably redundant on most * boards) */ -setup_local_APIC (); -end_local_APIC_setup (); +apic_ap_setup ();/* * Need to setup vector mappings before we enable interrupts.
@@ -1078,7 +1077,6 @@ static int __init smp_sanity_check(unsigned max_cpus) boot_cpu_physical_apicid);pr_err ("... forcing use of dummy APIC emulation (tell your hw vendor)\n" ); } -disable_ioapic_support ();return -1 ; }
@@ -1090,10 +1088,7 @@ static int __init smp_sanity_check(unsigned max_cpus)if (!max_cpus) {pr_info ("SMP mode deactivated\n" );disable_ioapic_support (); - -connect_bsp_APIC (); -setup_local_APIC (); -bsp_end_local_APIC_setup (); +apic_bsp_setup ();return -1 ; }
@@ -1151,23 +1146,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)/* Or can we switch back to PIC here? */ } -connect_bsp_APIC (); - -/* -* Switch from PIC to APIC mode. -*/ -setup_local_APIC (); - -if (x2apic_mode) - cpu0_logical_apicid =apic_read (APIC_LDR ); -else - cpu0_logical_apicid =GET_APIC_LOGICAL_ID (apic_read (APIC_LDR )); - -/* Enable IO APIC before setting up error vector */ -enable_IO_APIC (); - -bsp_end_local_APIC_setup (); -setup_IO_APIC (); + cpu0_logical_apicid =apic_bsp_setup ();/* * Set up local APIC timer on boot CPU.
PATCH 21/23 x86/smpboot: Sanitize uniprocessor init
In reply to Thomas GleixnerThe UP related setups for local apic are mangled into smp_sanity_check().
That results in duplicate calls to disable_smp() and makes the code hard to follow. Let smp_sanity_check() return dedicated values for the various exit reasons and handle them at the call site.
--- arch/x86/kernel/smpboot.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)
arch/x86/kernel/smpboot.c +26 -11
@@ -990,6 +990,8 @@ void arch_disable_smp_support(void)*/ static __init void disable_smp (void ) { +pr_info ("SMP disabled\n" ); +disable_ioapic_support ();init_cpu_present (cpumask_of (0 ));
@@ -1003,6 +1005,13 @@ static __init void disable_smp(void)cpumask_set_cpu (0 ,cpu_core_mask (0 )); } +enum { +SMP_OK , +SMP_NO_CONFIG , +SMP_NO_APIC , +SMP_FORCE_UP , +}; +/* * Various sanity checks. */
@@ -1050,10 +1059,7 @@ static int __init smp_sanity_check(unsigif (!smp_found_config && !acpi_lapic) {preempt_enable ();pr_notice ("SMP motherboard not detected\n" ); -disable_smp (); -if (APIC_init_uniprocessor ()) -pr_notice ("Local APIC not detected. Using dummy APIC emulation.\n" ); -return -1 ; +return SMP_NO_CONFIG ; }/*
@@ -1077,7 +1083,7 @@ static int __init smp_sanity_check(unsig boot_cpu_physical_apicid);pr_err ("... forcing use of dummy APIC emulation (tell your hw vendor)\n" ); } -return -1 ; +return SMP_NO_APIC ; }verify_local_APIC ();
@@ -1087,12 +1093,10 @@ static int __init smp_sanity_check(unsig*/ if (!max_cpus) {pr_info ("SMP mode deactivated\n" ); -disable_ioapic_support (); -apic_bsp_setup (); -return -1 ; +return SMP_FORCE_UP ; } -return 0 ; +return SMP_OK ; }static void __init smp_cpu_index_default (void )
@@ -1132,10 +1136,21 @@ void __init native_smp_prepare_cpus(unsi }set_cpu_sibling_map (0 ); -if (smp_sanity_check (max_cpus) <0 ) { -pr_info ("SMP disabled\n" ); +switch (smp_sanity_check (max_cpus)) { +case SMP_NO_CONFIG :disable_smp (); +if (APIC_init_uniprocessor ()) +pr_notice ("Local APIC not detected. Using dummy APIC emulation.\n" ); +return ; +case SMP_NO_APIC : +disable_smp (); +return ; +case SMP_FORCE_UP : +disable_smp (); +apic_bsp_setup ();return ; +case SMP_OK : +break ; }default_setup_apic_routing ();
Re: [patch 21/23] x86/smpboot: Sanitize uniprocessor init
Quoted 7 lines · Thomas Gleixner
The UP related setups for local apic are mangled into
smp_sanity_check().That results in duplicate calls to
disable_smp()and makes the code hard to follow. Letsmp_sanity_check()return dedicated values for the various exit reasons and handle them at the call site.Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/smpboot: Sanitize uniprocessor init
In reply to Thomas Gleixnerx86/smpboot: Sanitize uniprocessor init
Commit-ID: 613c25efbdc763ee8b9d732368106d2456279356 Gitweb: http://git.kernel.org/tip/613c25efbdc763ee8b9d732368106d2456279356 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:42 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
x86/smpboot: Sanitize uniprocessor init
The UP related setups for local apic are mangled into smp_sanity_check().
That results in duplicate calls to disable_smp() and makes the code hard to follow. Let smp_sanity_check() return dedicated values for the various exit reasons and handle them at the call site.
--- arch/x86/kernel/smpboot.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-)
arch/x86/kernel/smpboot.c +26 -11
@@ -990,6 +990,8 @@ void arch_disable_smp_support(void)*/ static __init void disable_smp (void ) { +pr_info ("SMP disabled\n" ); +disable_ioapic_support ();init_cpu_present (cpumask_of (0 ));
@@ -1003,6 +1005,13 @@ static __init void disable_smp(void)cpumask_set_cpu (0 ,cpu_core_mask (0 )); } +enum { +SMP_OK , +SMP_NO_CONFIG , +SMP_NO_APIC , +SMP_FORCE_UP , +}; +/* * Various sanity checks. */
@@ -1050,10 +1059,7 @@ static int __init smp_sanity_check(unsigned max_cpus)if (!smp_found_config && !acpi_lapic) {preempt_enable ();pr_notice ("SMP motherboard not detected\n" ); -disable_smp (); -if (APIC_init_uniprocessor ()) -pr_notice ("Local APIC not detected. Using dummy APIC emulation.\n" ); -return -1 ; +return SMP_NO_CONFIG ; }/*
@@ -1077,7 +1083,7 @@ static int __init smp_sanity_check(unsigned max_cpus) boot_cpu_physical_apicid);pr_err ("... forcing use of dummy APIC emulation (tell your hw vendor)\n" ); } -return -1 ; +return SMP_NO_APIC ; }verify_local_APIC ();
@@ -1087,12 +1093,10 @@ static int __init smp_sanity_check(unsigned max_cpus)*/ if (!max_cpus) {pr_info ("SMP mode deactivated\n" ); -disable_ioapic_support (); -apic_bsp_setup (); -return -1 ; +return SMP_FORCE_UP ; } -return 0 ; +return SMP_OK ; }static void __init smp_cpu_index_default (void )
@@ -1132,10 +1136,21 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus) }set_cpu_sibling_map (0 ); -if (smp_sanity_check (max_cpus) <0 ) { -pr_info ("SMP disabled\n" ); +switch (smp_sanity_check (max_cpus)) { +case SMP_NO_CONFIG :disable_smp (); +if (APIC_init_uniprocessor ()) +pr_notice ("Local APIC not detected. Using dummy APIC emulation.\n" ); +return ; +case SMP_NO_APIC : +disable_smp (); +return ; +case SMP_FORCE_UP : +disable_smp (); +apic_bsp_setup ();return ; +case SMP_OK : +break ; }default_setup_apic_routing ();
PATCH 22/23 x86/apic: Reuse apic_bsp_setup() for UP APIC setup
In reply to Thomas GleixnerExtend apic_bsp_setup() so the same code flow can be used for APIC_init_uniprocessor().
--- arch/x86/include/asm/apic.h | 2 - arch/x86/kernel/apic/apic.c | 54 ++++++++++++++++++-------------------------- arch/x86/kernel/smpboot.c | 4 +-- 3 files changed, 26 insertions(+), 34 deletions(-)
arch/x86/include/asm/apic.h +1 -1
@@ -212,7 +212,7 @@ extern void setup_secondary_APIC_clock(vextern int APIC_init_uniprocessor (void );extern int apic_force_enable (unsigned long addr); -extern int apic_bsp_setup (void ); +extern int apic_bsp_setup (bool upmode);extern void apic_ap_setup (void );/*
arch/x86/kernel/apic/apic.c +23 -31
91 lines not on this page · raw message
arch/x86/kernel/smpboot.c +2 -2
@@ -1147,7 +1147,7 @@ void __init native_smp_prepare_cpus(unsireturn ;case SMP_FORCE_UP :disable_smp (); -apic_bsp_setup (); +apic_bsp_setup (false);return ;case SMP_OK :break ;
@@ -1161,7 +1161,7 @@ void __init native_smp_prepare_cpus(unsi/* Or can we switch back to PIC here? */ } - cpu0_logical_apicid =apic_bsp_setup (); + cpu0_logical_apicid =apic_bsp_setup (false);/* * Set up local APIC timer on boot CPU.
Re: [patch 22/23] x86/apic: Reuse apic_bsp_setup() for UP APIC setup
Quoted 4 lines · Thomas Gleixner
Extend
apic_bsp_setup()so the same code flow can be used forAPIC_init_uniprocessor().Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86/apic: Reuse apic_bsp_setup() for UP APIC setup
In reply to Thomas Gleixnerx86/apic: Reuse apic_bsp_setup() for UP APIC setup
Commit-ID: 374aab339f10f0510cec0e79d752d31d84b08aa2 Gitweb: http://git.kernel.org/tip/374aab339f10f0510cec0e79d752d31d84b08aa2 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:44 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
x86/apic: Reuse apic_bsp_setup() for UP APIC setup
Extend apic_bsp_setup() so the same code flow can be used for APIC_init_uniprocessor().
Folded Jiangs fix to provide proper ordering of the UP setup.
--- arch/x86/include/asm/apic.h | 2 +- arch/x86/kernel/apic/apic.c | 53 +++++++++++++++++++-------------------------- arch/x86/kernel/smpboot.c | 4 ++-- 3 files changed, 25 insertions(+), 34 deletions(-)
arch/x86/include/asm/apic.h +1 -1
@@ -215,7 +215,7 @@ extern void setup_secondary_APIC_clock(void);extern int APIC_init_uniprocessor (void );extern int apic_force_enable (unsigned long addr); -extern int apic_bsp_setup (void ); +extern int apic_bsp_setup (bool upmode);extern void apic_ap_setup (void );/*
arch/x86/kernel/apic/apic.c +22 -31
88 lines not on this page · raw message
arch/x86/kernel/smpboot.c +2 -2
@@ -1147,7 +1147,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)return ;case SMP_FORCE_UP :disable_smp (); -apic_bsp_setup (); +apic_bsp_setup (false);return ;case SMP_OK :break ;
@@ -1161,7 +1161,7 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus)/* Or can we switch back to PIC here? */ } - cpu0_logical_apicid =apic_bsp_setup (); + cpu0_logical_apicid =apic_bsp_setup (false);/* * Set up local APIC timer on boot CPU.
PATCH 23/23 x86: Consolidate boot cpu timer setup
In reply to Thomas GleixnerNow that the APIC bringup is consolidated we can move the setup call for the percpu clock event device to apic_bsp_setup().
--- arch/x86/kernel/apic/apic.c | 4 ++-- arch/x86/kernel/smpboot.c | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-)
arch/x86/kernel/apic/apic.c +2 -2
@@ -2243,6 +2243,8 @@ int __init apic_bsp_setup(bool upmode)end_local_APIC_setup ();irq_remap_enable_fault_handling ();setup_IO_APIC (); +/* Setup local timer */ + x86_init.timers.setup_percpu_clockev ();return id; }
@@ -2283,8 +2285,6 @@ int __init APIC_init_uniprocessor(void)default_setup_apic_routing ();verify_local_APIC ();apic_bsp_setup (true); - - x86_init.timers.setup_percpu_clockev ();return 0 ; }#ifndef CONFIG_SMP
arch/x86/kernel/smpboot.c +0 -4
@@ -1163,12 +1163,8 @@ void __init native_smp_prepare_cpus(unsi cpu0_logical_apicid =apic_bsp_setup (false); -/* -* Set up local APIC timer on boot CPU. -*/ pr_info ("CPU%d: " ,0 );print_cpu_info (&cpu_data (0 )); - x86_init.timers.setup_percpu_clockev ();if (is_uv_system ())uv_system_init ();
Re: [patch 23/23] x86: Consolidate boot cpu timer setup
Quoted 4 lines · Thomas Gleixner
Now that the APIC bringup is consolidated we can move the setup call for the percpu clock event device to
apic_bsp_setup().Signed-off-by Thomas Gleixner <[email protected]>
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--tip-bot for Thomas Gleixner
merge bot[tip:x86/apic] x86: Consolidate boot cpu timer setup
In reply to Thomas Gleixnerx86: Consolidate boot cpu timer setup
Commit-ID: 9c4d9c73dd380ecfe1893600174f96d0eb068997 Gitweb: http://git.kernel.org/tip/9c4d9c73dd380ecfe1893600174f96d0eb068997 Author: Thomas Gleixner <[email protected]> AuthorDate: Thu, 15 Jan 2015 21:22:45 +0000 Committer: Thomas Gleixner <[email protected]> CommitDate: Thu, 22 Jan 2015 15:10:56 +0100
x86: Consolidate boot cpu timer setup
Now that the APIC bringup is consolidated we can move the setup call for the percpu clock event device to apic_bsp_setup().
--- arch/x86/kernel/apic/apic.c | 4 ++-- arch/x86/kernel/smpboot.c | 4 ---- 2 files changed, 2 insertions(+), 6 deletions(-)
arch/x86/kernel/apic/apic.c +2 -2
@@ -2244,6 +2244,8 @@ int __init apic_bsp_setup(bool upmode)end_local_APIC_setup ();irq_remap_enable_fault_handling ();setup_IO_APIC (); +/* Setup local timer */ + x86_init.timers.setup_percpu_clockev ();return id; }
@@ -2284,8 +2286,6 @@ int __init APIC_init_uniprocessor(void)default_setup_apic_routing ();verify_local_APIC ();apic_bsp_setup (true); - - x86_init.timers.setup_percpu_clockev ();return 0 ; }
arch/x86/kernel/smpboot.c +0 -4
@@ -1163,12 +1163,8 @@ void __init native_smp_prepare_cpus(unsigned int max_cpus) cpu0_logical_apicid =apic_bsp_setup (false); -/* -* Set up local APIC timer on boot CPU. -*/ pr_info ("CPU%d: " ,0 );print_cpu_info (&cpu_data (0 )); - x86_init.timers.setup_percpu_clockev ();if (is_uv_system ())uv_system_init ();
Quoted 22 lines · Thomas Gleixner
While reviewing Jiangs interrupt remapping patch set, I had several serious WTF moments when trying to understand what that code is doing.
The main issues I've seen are:
- Blindly copy and pasted code
- Random places which initialize bits and pieces
- Code which got mindlessly expanded with duct tape and glue without considering readability and maintainability.
- Missing inline stubs which result in a ifdef nightmare
- Superflous inline stubs to artificially avoid sensible ifdefs
The deeper I looked the more I started to get grumpy about that maze and finally sat down and cleaned it up seriously. One bug I fixed on the way (surprise, surprise) got folded into Jiangs series which is now in tip/x86/apic. The other one is not that crucial and cant be fixed in the previous code without adding more mess.
Quoted 4 lines
The following patch series applies on top of tip/x86/apic. It cleans up and consolidates the apic/ioapic/x2apic setup functions.
Please give it a proper review and testing.
Hi Thomas,
Have done following test matrix:
1) Hardware X2apic capable: yes, preenabled by BIOS: no,
Kernel: x2apic capable smp
1.1) boot: ok
1.2) boot with "nox2apic": ok
1.3) boot with "nointremap": ok with CPU in physical flat mode
2) Hardware X2apic capable: yes, preenabled by BIOS: no,
Kernel: x2apic incapable smp
2.1) boot: ok
3) Hardware X2apic capable: yes, preenabled by BIOS: yes 3.1) x2apic capable kernel: ok
3.2) x2apic incapable kernel: hang with a blank screen(panic)
Kernel: x2apic capable smp
4) Hardware X2apic capable: no
4.1) boot x2apic capable smp kernel: ok
4.2) boot x2apic incapable up kernel: ok
4.3) boot xen domain0 with x2apic capable smp kernel: ok
5) x86_32, UP, IO_APIC disabled
5.1) boot: panic with following call stack:
do_ono_initcall()->APIC_init_uniprocessor()->setup_local_APIC(). I can't capture the full log message due to lack of serial console. It seems we have trouble with:early_initcall(APIC_init_uniprocessor);
Regards!
Gerry
Quoted 21 lines
Thanks,
tglx
--- arch/x86/include/asm/smpboot_hooks.h | 68 ------ tip/arch/x86/include/asm/apic.h | 57 +---- tip/arch/x86/include/asm/io_apic.h | 5 tip/arch/x86/kernel/apic/apic.c | 395 ++++++++++++++++++----------------- tip/arch/x86/kernel/apic/io_apic.c | 13 - tip/arch/x86/kernel/cpu/common.c | 2 tip/arch/x86/kernel/smpboot.c | 113 +++++----- tip/init/main.c | 14 - 8 files changed, 308 insertions(+), 359 deletions(-)--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 50 lines · Jiang Liu
Quoted 22 lines · Thomas Gleixner
While reviewing Jiangs interrupt remapping patch set, I had several serious WTF moments when trying to understand what that code is doing.
The main issues I've seen are:
- Blindly copy and pasted code
- Random places which initialize bits and pieces
- Code which got mindlessly expanded with duct tape and glue without considering readability and maintainability.
- Missing inline stubs which result in a ifdef nightmare
- Superflous inline stubs to artificially avoid sensible ifdefs
The deeper I looked the more I started to get grumpy about that maze and finally sat down and cleaned it up seriously. One bug I fixed on the way (surprise, surprise) got folded into Jiangs series which is now in tip/x86/apic. The other one is not that crucial and cant be fixed in the previous code without adding more mess.
Quoted 4 lines
The following patch series applies on top of tip/x86/apic. It cleans up and consolidates the apic/ioapic/x2apic setup functions.
Please give it a proper review and testing.
Hi Thomas,
Have done following test matrix:
1) Hardware X2apic capable: yes, preenabled by BIOS: no,
Kernel: x2apic capable smp
1.1) boot: ok
1.2) boot with "nox2apic": ok
1.3) boot with "nointremap": ok with CPU in physical flat mode2) Hardware X2apic capable: yes, preenabled by BIOS: no,
Kernel: x2apic incapable smp
2.1) boot: ok3) Hardware X2apic capable: yes, preenabled by BIOS: yes 3.1) x2apic capable kernel: ok
3.2) x2apic incapable kernel: hang with a blank screen(panic)
Kernel: x2apic capable smp4) Hardware X2apic capable: no
4.1) boot x2apic capable smp kernel: ok
4.2) boot x2apic incapable up kernel: ok
4.3) boot xen domain0 with x2apic capable smp kernel: ok
Quoted 6 lines
5)
x86_32, UP,IO_APICdisabled
5.1) boot: panic with following call stack:
do_ono_initcall()->APIC_init_uniprocessor()->setup_local_APIC(). I can't capture the full log message due to lack of serial console. It seems we have trouble with:early_initcall(APIC_init_uniprocessor);
Hmm, worked here. Lemme find some other machine.
Quoted 51 lines · Thomas Gleixner
Quoted 50 lines · Jiang Liu
Quoted 22 lines · Thomas Gleixner
While reviewing Jiangs interrupt remapping patch set, I had several serious WTF moments when trying to understand what that code is doing.
The main issues I've seen are:
- Blindly copy and pasted code
- Random places which initialize bits and pieces
- Code which got mindlessly expanded with duct tape and glue without considering readability and maintainability.
- Missing inline stubs which result in a ifdef nightmare
- Superflous inline stubs to artificially avoid sensible ifdefs
The deeper I looked the more I started to get grumpy about that maze and finally sat down and cleaned it up seriously. One bug I fixed on the way (surprise, surprise) got folded into Jiangs series which is now in tip/x86/apic. The other one is not that crucial and cant be fixed in the previous code without adding more mess.
Quoted 4 lines
The following patch series applies on top of tip/x86/apic. It cleans up and consolidates the apic/ioapic/x2apic setup functions.
Please give it a proper review and testing.
Hi Thomas,
Have done following test matrix:
1) Hardware X2apic capable: yes, preenabled by BIOS: no,
Kernel: x2apic capable smp
1.1) boot: ok
1.2) boot with "nox2apic": ok
1.3) boot with "nointremap": ok with CPU in physical flat mode2) Hardware X2apic capable: yes, preenabled by BIOS: no,
Kernel: x2apic incapable smp
2.1) boot: ok3) Hardware X2apic capable: yes, preenabled by BIOS: yes 3.1) x2apic capable kernel: ok
3.2) x2apic incapable kernel: hang with a blank screen(panic)
Kernel: x2apic capable smp4) Hardware X2apic capable: no
4.1) boot x2apic capable smp kernel: ok
4.2) boot x2apic incapable up kernel: ok
4.3) boot xen domain0 with x2apic capable smp kernel: ok
Quoted 8 lines
Quoted 6 lines
5)
x86_32, UP,IO_APICdisabled
5.1) boot: panic with following call stack:
do_ono_initcall()->APIC_init_uniprocessor()->setup_local_APIC(). I can't capture the full log message due to lack of serial console. It seems we have trouble with:early_initcall(APIC_init_uniprocessor);Hmm, worked here. Lemme find some other machine.
Hi Thomas,
X86_64 UP with IOAPIC enabled works here too. Not sure the panic
is caused by X86_32 or disabling of IOAPIC. I have no access to the problematic system on weekend, so could only do more tests early next week.
Thanks!
Gerry
Quoted 7 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 9 lines · Thomas Gleixner
Quoted 6 lines · Jiang Liu
5)
x86_32, UP,IO_APICdisabled
5.1) boot: panic with following call stack:
do_ono_initcall()->APIC_init_uniprocessor()->setup_local_APIC(). I can't capture the full log message due to lack of serial console. It seems we have trouble with:early_initcall(APIC_init_uniprocessor);Hmm, worked here. Lemme find some other machine.
Found the issue, won't work. But I still want to remove that x86 nonsense from init. So we need to do it different.
Subject: init: Get rid of x86isms
From: Thomas Gleixner <[email protected]>
Date: Wed, 14 Jan 2015 14:59:48 +0100
Provide a config option and make the UP smp_init() implementation depend on it and confine this to x86.
--- arch/x86/Kconfig | 4 ++++ arch/x86/kernel/apic/apic.c | 7 +++++++ include/linux/smp.h | 6 ++++++ init/main.c | 13 ------------- 4 files changed, 17 insertions(+), 13 deletions(-)
arch/x86/Kconfig +4 -0
@@ -855,6 +855,10 @@ config SCHED_MC source "kernel/Kconfig.preempt" +config UP_SMP_INIT + def_bool y + depends on X86_UP_APIC + config X86_UP_APIC bool "Local APIC support on uniprocessors" depends on X86_32 && !SMP && !X86_32_NON_STANDARD && !PCI_MSI
arch/x86/kernel/apic/apic.c +7 -0
@@ -2265,6 +2265,13 @@ int __init APIC_init_uniprocessor(void)return 0 ; } +#ifdef CONFIG_UP_SMP_INIT +void smp_init (void ) +{ +APIC_init_uniprocessor (); +} +#endif +/* * Power management */
include/linux/smp.h +6 -0
@@ -151,6 +151,12 @@ smp_call_function_any(const struct cpumastatic inline void kick_all_cpus_sync (void ) { }static inline void wake_up_all_idle_cpus (void ) { } +#ifdef CONFIG_UP_SMP_INIT +extern void __init smp_init (void ); +#else +static inline void smp_init (void ) { } +#endif +#endif /* !SMP */ /*
init/main.c +0 -13
@@ -87,10 +87,6 @@#include <asm/sections.h> #include <asm/cacheflush.h> -#ifdef CONFIG_X86_LOCAL_APIC -#include <asm/smp.h> -#endif -static int kernel_init (void *);extern void init_IRQ (void );
@@ -351,15 +347,6 @@ __setup("rdinit=", rdinit_setup);#ifndef CONFIG_SMP static const unsigned int setup_max_cpus =NR_CPUS ; -#ifdef CONFIG_X86_LOCAL_APIC -static void __init smp_init (void ) -{ -APIC_init_uniprocessor (); -} -#else -#define smp_init () do { } while (0) -#endif -static inline void setup_nr_cpu_ids (void ) { }static inline void smp_prepare_cpus (unsigned int maxcpus) { }#endif
Borislav Petkov
maintainerQuoted 34 lines · Thomas Gleixner
Quoted 9 lines · Thomas Gleixner
Quoted 6 lines · Jiang Liu
5)
x86_32, UP,IO_APICdisabled
5.1) boot: panic with following call stack:
do_ono_initcall()->APIC_init_uniprocessor()->setup_local_APIC(). I can't capture the full log message due to lack of serial console. It seems we have trouble with:early_initcall(APIC_init_uniprocessor);Hmm, worked here. Lemme find some other machine.
Found the issue, won't work. But I still want to remove that x86 nonsense from init. So we need to do it different.
Subject: init: Get rid of x86isms
From: Thomas Gleixner <[email protected]>
Date: Wed, 14 Jan 2015 14:59:48 +0100Provide a config option and make the UP
smp_init()implementation depend on it and confine this to x86.Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/Kconfig | 4 ++++ arch/x86/kernel/apic/apic.c | 7 +++++++ include/linux/smp.h | 6 ++++++ init/main.c | 13 ------------- 4 files changed, 17 insertions(+), 13 deletions(-)Index: tip/arch/x86/Kconfig
===================================================================
arch/x86/Kconfig config SCHED_MC · line 858
Quoted 4 lines
arch/x86/Kconfig +1
-0@@ -855,6 +855,10 @@ config SCHED_MC source "kernel/Kconfig.preempt" +config UP_SMP_INIT
UP_SMP might confuse people. UP_INIT looks fine to me too.
And then maybe some more monkey business like this:
---
arch/x86/kernel/apic/apic.c +1 -1
@@ -2266,7 +2266,7 @@ int __init APIC_init_uniprocessor(void) }#ifdef CONFIG_UP_SMP_INIT -void smp_init (void ) +void up_init (void ) {APIC_init_uniprocessor (); }
include/linux/smp.h +3 -2
@@ -152,9 +152,10 @@ static inline void kick_all_cpus_sync(vostatic inline void wake_up_all_idle_cpus (void ) { }#ifdef CONFIG_UP_SMP_INIT -extern void __init smp_init (void ); +#define smp_init up_init +extern void __init up_init (void );#else -static inline void smp_init (void ) { } +static inline void up_init (void ) { }#endif #endif /* !SMP */
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Quoted 100 lines · Thomas Gleixner
Quoted 9 lines · Thomas Gleixner
Quoted 6 lines · Jiang Liu
5)
x86_32, UP,IO_APICdisabled
5.1) boot: panic with following call stack:
do_ono_initcall()->APIC_init_uniprocessor()->setup_local_APIC(). I can't capture the full log message due to lack of serial console. It seems we have trouble with:early_initcall(APIC_init_uniprocessor);Hmm, worked here. Lemme find some other machine.
Found the issue, won't work. But I still want to remove that x86 nonsense from init. So we need to do it different.
Subject: init: Get rid of x86isms
From: Thomas Gleixner <[email protected]>
Date: Wed, 14 Jan 2015 14:59:48 +0100Provide a config option and make the UP
smp_init()implementation depend on it and confine this to x86.Signed-off-by Thomas Gleixner <[email protected]>--- arch/x86/Kconfig | 4 ++++ arch/x86/kernel/apic/apic.c | 7 +++++++ include/linux/smp.h | 6 ++++++ init/main.c | 13 ------------- 4 files changed, 17 insertions(+), 13 deletions(-)Index: tip/arch/x86/Kconfig
===================================================================arch/x86/Kconfig +4
-0@@ -855,6 +855,10 @@ config SCHED_MC source "kernel/Kconfig.preempt" +config UP_SMP_INIT + def_bool y + depends on X86_UP_APIC + config X86_UP_APIC bool "Local APIC support on uniprocessors" depends on X86_32 && !SMP && !X86_32_NON_STANDARD && !PCI_MSIIndex:
tip/arch/x86/kernel/apic/apic.c
===================================================================arch/x86/kernel/apic/apic.c +7
-0@@ -2265,6 +2265,13 @@ int __init APIC_init_uniprocessor(void)return 0 ; } +#ifdef CONFIG_UP_SMP_INIT +void smp_init (void ) +{ +APIC_init_uniprocessor (); +} +#endif +/* * Power management */ Index:
tip/include/linux/smp.h
===================================================================include/linux/smp.h +6
-0@@ -151,6 +151,12 @@ smp_call_function_any(const struct cpumastatic inline void kick_all_cpus_sync (void ) { }static inline void wake_up_all_idle_cpus (void ) { } +#ifdef CONFIG_UP_SMP_INIT +extern void __init smp_init (void ); +#else +static inline void smp_init (void ) { } +#endif +#endif /* !SMP */ /* Index:
tip/init/main.c
===================================================================init/main.c +0
-6@@ -87,10 +87,6 @@#include <asm/sections.h> #include <asm/cacheflush.h> -#ifdef CONFIG_X86_LOCAL_APIC -#include <asm/smp.h> -#endif -static int kernel_init (void *);extern void init_IRQ (void );@@ -351,15 +347,6 @@ __setup("rdinit=", rdinit_setup);#ifndef CONFIG_SMP static const unsigned int setup_max_cpus =NR_CPUS ; -#ifdef CONFIG_X86_LOCAL_APIC -static void __init smp_init (void )
init/main.c __setup("rdinit=", rdinit_setup); · line 352
Quoted 10 lines
init/main.c +0
-7-{ -APIC_init_uniprocessor (); -} -#else -#define smp_init () do { } while (0) -#endif -static inline void setup_nr_cpu_ids (void ) { }static inline void smp_prepare_cpus (unsigned int maxcpus) { }#endif
Hi Thomas,
With above patch applied, it just gives a blank screen
instead of a panic when booting on my problematic laptop. Now I have found the root cause of the failure on my laptop.
On i386, it triggers BUG_ON(!apic->apic_id_registered())
in function setup_local_APIC() because apic_bsp_up_setup() is called too late. So we need a simple patch as below. With following patch applied, it boots successfully with your original patch. Regards,
Gerry
--------------------------------------------------------------------
arch/x86/kernel/apic/apic.c +1 -2
@@ -2245,10 +2245,9 @@ int __init apic_bsp_setup(bool upmode)int id;connect_bsp_APIC (); -setup_local_APIC (); -if (upmode)apic_bsp_up_setup (); +setup_local_APIC ();if (x2apic_mode) id =apic_read (APIC_LDR );
---------------------------------------------------------------------
Quoted 9 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 9 lines · Jiang Liu
On 2015/1/16 19:05, Thomas Gleixner wrote:
Hi Thomas,
With above patch applied, it just gives a blank screen
instead of a panic when booting on my problematic laptop. Now I have found the root cause of the failure on my laptop.On i386, it triggers BUG_ON(!apic->apic_id_registered())in function
setup_local_APIC()becauseapic_bsp_up_setup()is called too late. So we need a simple patch as below. With following patch applied, it boots successfully with your original patch.
Duh. Right you are. I wonder why this boots on any other machine. Seems there is some more magic in this setup maze.
Thanks,
tglx