Bug 5518
Summary: | Reboot-through-BIOS does not mask APIC LVT interrupts - BIOS hang | ||
---|---|---|---|
Product: | Platform Specific/Hardware | Reporter: | Boris Weissman (weissman) |
Component: | i386 | Assignee: | Zwane Mwaikambo (zwane) |
Status: | CLOSED CODE_FIX | ||
Severity: | normal | CC: | akpm |
Priority: | P2 | ||
Hardware: | i386 | ||
OS: | Linux | ||
Kernel Version: | 2.6.12 (earlier?) | Subsystem: | |
Regression: | --- | Bisected commit-id: | |
Attachments: |
Clear LVTs on reboot through BIOS
patch-reboot_through_BIOS_does_not_mask_APIC_LVT_interrupts |
Description
Boris Weissman
2005-10-28 11:59:12 UTC
Could you please test 2.6.14? If the APIC was setup by Linux cpu_has_apic should also be set so you should fall through to disable_local_APIC, what am i missing here? In response to Comment 1 by Andrew Morton: I have tried the fix proposed in the original filing on 2.6.14 with Phoenix BIOS 4.0 Release 6.0. The fix worked fine. I used these config options to be similar to the Ubuntu 5.1 kernel where I first observed the hang (the bug affects only APIC-enabled UP systems): CONFIG_X86_REBOOTFIXUPS=y CONFIG_X86_BIOS_REBOOT=y CONFIG_X86_LOCAL_APIC=y CONFIG_X86_IO_APIC=y I also used a boot option "reboot=b" to force reboot through BIOS. Just like Ubuntu 5.1, the unmodified kernel hanged at: md: md0 switched to read-only mode. Restarting system . The patched kernel worked fine. In response to Comment 2 by Zwane Mwaikambo: cpu_has_apic=1 is insufficient to proceed to disable_local_APIC(). The problem is that if BIOS enabled APIC (as it normally would), enabled_via_apicbase would remain 0 since it means "enabled-by-Linux". Since enabled_via_apicbase=0, lapic_shutdown() returns immediately and never executes disable_local_APIC(): /* * If Linux enabled the LAPIC against the BIOS default * disable it down before re-entering the BIOS on shutdown. * Otherwise the BIOS may get confused and not power-off. */ void lapic_shutdown(void) { if (!cpu_has_apic || !enabled_via_apicbase) return; local_irq_disable(); disable_local_APIC(); local_irq_enable(); } Created attachment 6430 [details]
Clear LVTs on reboot through BIOS
Sorry, i forgot the explanation. Essentially change the logic and do the clear_local_APIC in this case we do it twice, but that's ok. But do not remove it from disable_local_APIC as that is called from other places. Zwane, Your patch looks good. Calling disable_local_APIC() twice is alright and better than not calling it at all. If you don't mind, I'd restructure it a little to get rid of goto and avoid local_irq_disable/enable when there is nothing to do: natasha% diff -u -p -B apic.c apic-patched.c --- apic.c 2005-10-27 17:02:08.000000000 -0700 +++ apic-patched.c 2005-10-31 23:06:42.733651641 -0800 @@ -562,12 +562,15 @@ void __devinit setup_local_APIC(void) */ void lapic_shutdown(void) { - if (!cpu_has_apic || !enabled_via_apicbase) + if (!cpu_has_apic) return; - local_irq_disable(); - disable_local_APIC(); - local_irq_enable(); + clear_local_APIC(); + if (enabled_via_apicbase) { + local_irq_disable(); + disable_local_APIC(); + local_irq_enable(); + } } #ifdef CONFIG_PM Feel free to ignore it though. Boris Created attachment 6437 [details]
patch-reboot_through_BIOS_does_not_mask_APIC_LVT_interrupts
I don't feel comfortable calling clear_local_APIC with interrupts enabled, but
i removed the goto at least ;}
Zwane, your last patch looks great. Many thanks! Boris Thank you very much for the well presented bug report + patch, i'll make sure this gets sent upstream. |