Messages in this thread | | | Date | Tue, 14 Apr 2009 21:09:42 +0400 | From | Cyrill Gorcunov <> | Subject | Re: [PATCH 10/14] [VOYAGER] x86: make disabling the apics functional instead of a flag |
| |
[James Bottomley - Tue, Apr 14, 2009 at 10:51:36AM -0500] | This allows the elimination of some ifdef guards in setup.c. | Additionally probe_32.c doesn't need to run through the apics in | generic_apic_probe() if they're disabled. | | Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com> | --- | arch/x86/include/asm/apic.h | 6 ++++++ | arch/x86/kernel/apic/apic.c | 3 +++ | arch/x86/kernel/apic/probe_32.c | 3 +++ | arch/x86/kernel/setup.c | 4 +--- | 4 files changed, 13 insertions(+), 3 deletions(-) | | diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h | index 42f2f83..391c464 100644 | --- a/arch/x86/include/asm/apic.h | +++ b/arch/x86/include/asm/apic.h | @@ -246,12 +246,18 @@ static inline int apic_is_clustered_box(void) | extern u8 setup_APIC_eilvt_mce(u8 vector, u8 msg_type, u8 mask); | extern u8 setup_APIC_eilvt_ibs(u8 vector, u8 msg_type, u8 mask); | | +static inline void disable_APIC(void) | +{ | + disable_local_APIC(); | + disable_apic = 1; | +} | | #else /* !CONFIG_X86_LOCAL_APIC */ | static inline void lapic_shutdown(void) { } | #define local_apic_timer_c2_ok 1 | static inline void init_apic_mappings(void) { } | static inline void disable_local_APIC(void) { } | +static inline void disable_APIC(void) { } | | #endif /* !CONFIG_X86_LOCAL_APIC */ | | diff --git a/arch/x86/kernel/apic/apic.c b/arch/x86/kernel/apic/apic.c | index f9e830e..aa96dbe 100644 | --- a/arch/x86/kernel/apic/apic.c | +++ b/arch/x86/kernel/apic/apic.c | @@ -1539,6 +1539,9 @@ void __init early_init_lapic_mapping(void) | */ | void __init init_apic_mappings(void) | { | + if (disable_apic) | + return; | +
No, we shouldn't do that without additional code review (otherwise we will loose mapping for fake apic page). And I suspect we get NULL deref on reboot procedure if kernel was compiled _with_ SMP support but APIC disabled by kernel option. hmm, can't find this reference in LKML. I was telling Ingo about my suspicious on smp operations when APIC is disabled by option and we're safe _only_ 'cause we have fake mapping page reserved. For example call of smp_send_stop() in kernel/panic.c if kernel was compiled _with_ SMP support and then APIC disabled by kernel option.
| if (x2apic) { | boot_cpu_physical_apicid = read_apic_id(); | return; | diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c | index 01eda2a..049d7a0 100644 | --- a/arch/x86/kernel/apic/probe_32.c | +++ b/arch/x86/kernel/apic/probe_32.c | @@ -226,6 +226,9 @@ void __init generic_bigsmp_probe(void) | | void __init generic_apic_probe(void) | { | + if (disable_apic) | + return; | +
Look suspicious too, sorry. If kernel compiled with local apic set we rely on apic read ops regardless if apic facility has been disabled by option or not.
I could be missing something /which has quite a high probability magnitude/. Yinghai help needed :) CC'ed
| if (!cmdline_apic) { | int i; | for (i = 0; apic_probe[i]; i++) { | diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c | index bee0914..13779e2 100644 | --- a/arch/x86/kernel/setup.c | +++ b/arch/x86/kernel/setup.c | @@ -770,9 +770,7 @@ void __init setup_arch(char **cmdline_p) | reserve_early_setup_data(); | | if (acpi_mps_check()) { | -#ifdef CONFIG_X86_LOCAL_APIC | - disable_apic = 1; | -#endif | + disable_APIC(); | setup_clear_cpu_cap(X86_FEATURE_APIC); | } | | -- | 1.6.2.1 |
Cyrill
| |