Re: [PATCH 10/14] [VOYAGER] x86: make disabling the apicsfunctional instead of a flag

From: James Bottomley
Date: Wed Apr 15 2009 - 08:51:21 EST


On Tue, 2009-04-14 at 21:44 +0400, Cyrill Gorcunov wrote:
> [Cyrill Gorcunov - Tue, Apr 14, 2009 at 09:09:42PM +0400]
> ...
> | | 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;
> ...
>
> Ah, I found that link -- http://lkml.org/lkml/2009/4/12/139
> I've promised to take a look, unfortunately out of time now.
> Maybe a bit later.

Hmm, well, looking at this, I think there are two points

The first is that disable_apic should really mean that: It is a
dangerous option; there are modern CPUs that can't deliver interrupts
without apics and if you have one and disable it, there's no way of
preventing the hang that follows (because you can't even get timer
interrupts).

Secondly, I can fix your original emailed problem by having a check for
disable_apic in the shutdown path on disable_local_apic() ... I think
that fixes everything?

James


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/