Disable APIC on reboot.

Forward port of a patch from Felix [surname mangled by MTA] <[email protected]>

Original mail:-

when I reboot my laptop the BIOS complains about a keyboard controller failure and timer interrupts not working. On the BIOS password screen the keyboard doesn't work. If I disable the BIOS password the system hangs a bit later on the POST screen. I tried all reboot methods via the kernel command line without success. I saw this problem with Debian Woody's precompiled 2.4.18 kernel, self compiled Debian 2.4.20 sources and Linux 2.4.21-rc1+ACPI patch. This problem only occurs if CONFIG_X86_LOCAL_APIC is enabled. With CONFIG_X86_LOCAL_APIC disabled it reboots just fine.

My guess is that it's a BIOS bug as I've never had this problem on other machines before. I found a workaround: disable the local APIC before rebooting. I don't really know how it works. Just calling disable_local_APIC wasn't enough, so I copied a bit more code from apic_pm_suspend (arch/i386/kernel/apic.c:465) to machine_restart (arch/i386/kernel/process.c:369). I'm pretty sure that this is not the proper way to do it but it works here. A patch against 2.4.21-rc1 can be found at the end of this email.

diff -urpN --exclude-from=/home/davej/.exclude bk-linus/arch/i386/kernel/reboot.c linux-2.5/arch/i386/kernel/reboot.c
@@ -8,6 +8,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <asm/uaccess.h>
+#include <asm/apic.h>
 #include "mach_reboot.h"
 
 /*

@@ -250,6 +251,19 @@ void machine_restart(char * __unused)
 	 */
 	smp_send_stop();
 	disable_IO_APIC();
+#else
+#ifdef CONFIG_X86_LOCAL_APIC
+	{
+	unsigned int l, h;
+
+	local_irq_disable();
+	disable_local_APIC();
+	rdmsr(MSR_IA32_APICBASE, l, h);
+	l &= ~MSR_IA32_APICBASE_ENABLE;
+	wrmsr(MSR_IA32_APICBASE, l, h);
+	local_irq_enable();
+}
+#endif
 #endif
 
 	if(!reboot_thru_bios) {

Marcelo,

This patch allows my VAIO to reboot with LOCAL_APIC enabled. I too have a broken BIOS which cannot put hardware into a sane state at boot, and it hangs on IDE detection if the kernel left LOCAL_APIC enabled. Now with this patch upon 2.4.22-rc2, it's OK, so I can enable LOCAL_APIC again. Do you find it too late for 2.4.22 ? Depending on how we see it, it may be considered as a workaround for broken BIOSes, or as a bugfix for a kernel which didn't restore hardware into its original state. Or at least, please accept it for 2.4.23-pre1.

I rediffed it against 2.4.22-rc2, and attached the patch at the end of this mail.

Cheers,
Willy

@@ -42,6 +42,7 @@
 #include <asm/processor.h>
 #include <asm/i387.h>
 #include <asm/irq.h>
+#include <asm/apic.h>
 #include <asm/desc.h>
 #include <asm/mmu_context.h>
 #ifdef CONFIG_MATH_EMULATION

@@ -400,6 +401,17 @@
 	 */
 	smp_send_stop();
 	disable_IO_APIC();
+#elif CONFIG_X86_LOCAL_APIC
+	{
+		unsigned int l, h;
+
+		local_irq_disable();
+		disable_local_APIC();
+		rdmsr(MSR_IA32_APICBASE, l, h);
+		l &= ~MSR_IA32_APICBASE_ENABLE;
+		wrmsr(MSR_IA32_APICBASE, l, h);
+		local_irq_enable();
+	}
 #endif
 
 	if(!reboot_thru_bios) {

Ok. The original poster mentioned that disable_local_apic() didn't do the right thing there, hence the duplication, so making that DTRT may make sense ?

Yes. I'll bounce it to you.

Dave

 Dave Jones     http://www.codemonkey.org.uk

I'd have to check what callers expect from disable_local_APIC(), but having it or a new function do a complete disable seems reasonable.

detect_init_APIC() needs to record whether it did a hard enable via APIC_BASE or not, and the complete-disable code needs to check this before accessing APIC_BASE. That + a cpu_has_apic check should do it.

I'll do a patch this evening if no-one else beats me to it.

/Mikael

Here is a patch for 2.6.0-test3 to disable the local APIC before reboot. This fixes BIOS reboot problems reported by a few people.

disable_local_APIC() now checks if detect_init_APIC() enabled the local APIC via the APIC_BASE MSR, and if so it now disables APIC_BASE. Previously we would leave APIC_BASE enabled, and that made some BIOSen unhappy.

The SMP reboot code calls disable_local_APIC(). On SMP HW there is no change since detect_init_APIC() isn't called and APIC_BASE isn't enabled by us. An SMP kernel on UP HW behaves just like an UP_APIC kernel, so it disables APIC_BASE if we enabled it at boot.

The UP_APIC disable-before-suspend code is simplified since the existing code to disable APIC_BASE is moved into disable_local_APIC().

(Felix Kühling originally reported the BIOS reboot problem. This is a fixed-up version of his preliminary patch.)

/Mikael

@@ -61,6 +61,8 @@
 static DEFINE_PER_CPU(int, prof_old_multiplier) = 1;
 static DEFINE_PER_CPU(int, prof_counter) = 1;
 
+static int enabled_via_apicbase;
+
 void enable_NMI_through_LVT0 (void * dummy)
 {
 	unsigned int v, ver;

@@ -190,6 +192,13 @@
 	value = apic_read(APIC_SPIV);
 	value &= ~APIC_SPIV_APIC_ENABLED;
 	apic_write_around(APIC_SPIV, value);
+
+	if (enabled_via_apicbase) {
+		unsigned int l, h;
+		rdmsr(MSR_IA32_APICBASE, l, h);
+		l &= ~MSR_IA32_APICBASE_ENABLE;
+		wrmsr(MSR_IA32_APICBASE, l, h);
+	}
 }
 
 /*

@@ -485,7 +494,6 @@
 
 static int lapic_suspend(struct sys_device *dev, u32 state)
 {
-	unsigned int l, h;
 	unsigned long flags;
 
 	if (!apic_pm_state.active)

@@ -507,9 +515,6 @@
 	
 	local_irq_save(flags);
 	disable_local_APIC();
-	rdmsr(MSR_IA32_APICBASE, l, h);
-	l &= ~MSR_IA32_APICBASE_ENABLE;
-	wrmsr(MSR_IA32_APICBASE, l, h);
 	local_irq_restore(flags);
 	return 0;
 }

@@ -636,6 +641,7 @@
 			l &= ~MSR_IA32_APICBASE_BASE;
 			l |= MSR_IA32_APICBASE_ENABLE | APIC_DEFAULT_PHYS_BASE;
 			wrmsr(MSR_IA32_APICBASE, l, h);
+			enabled_via_apicbase = 1;
 		}
 	}
 	/*

@@ -8,6 +8,7 @@
 #include <linux/interrupt.h>
 #include <linux/mc146818rtc.h>
 #include <asm/uaccess.h>
+#include <asm/apic.h>
 #include "mach_reboot.h"
 
 /*

@@ -249,6 +250,14 @@
 	 * other OSs see a clean IRQ state.
 	 */
 	smp_send_stop();
+#elif CONFIG_X86_LOCAL_APIC
+	if (cpu_has_apic) {
+		local_irq_disable();
+		disable_local_APIC();
+		local_irq_enable();
+	}
+#endif
+#ifdef CONFIG_X86_IO_APIC
 	disable_IO_APIC();
 #endif
 

This follows the same order that the SMP reboot code has been using for ages. I did check disable_IO_APIC(), and it does not depend on the BP's local APIC being enabled.

I don't think there is a bug, but if there is, it's in the original code too (simply trace what an SMP kernel on UP_IOAPIC HW would do).

/Mikael

Well, I meant the hardware POV.

We might not care that much that hardware goes wild just before it's
disabled, but it doesn't mean it's OK. The bug is indeed in the original code. I think disable_IO_APIC() should precede smp_send_stop() and disconnect_bsp_APIC() should be called from smp_send_stop(). More or less like this (before your fix):

patch-mips-2.6.0-test2-20030804-apic-reboot-0

diff -up --recursive --new-file linux-mips-2.6.0-test2-20030804.macro/arch/i386/kernel/io_apic.c linux-mips-2.6.0-test2-20030804/arch/i386/kernel/io_apic.c
@@ -1598,8 +1598,6 @@ void disable_IO_APIC(void)
 	 * Clear the IO-APIC before rebooting:
 	 */
 	clear_IO_APIC();
-
-	disconnect_bsp_APIC();
 }
 
 /*

diff -up --recursive --new-file linux-mips-2.6.0-test2-20030804.macro/arch/i386/kernel/reboot.c linux-mips-2.6.0-test2-20030804/arch/i386/kernel/reboot.c
@@ -248,8 +248,8 @@ void machine_restart(char * __unused)
 	 * Stop all CPUs and turn off local APICs and the IO-APIC, so
 	 * other OSs see a clean IRQ state.
 	 */
-	smp_send_stop();
 	disable_IO_APIC();
+	smp_send_stop();
 #endif
 
 	if(!reboot_thru_bios) {

diff -up --recursive --new-file linux-mips-2.6.0-test2-20030804.macro/arch/i386/kernel/smp.c linux-mips-2.6.0-test2-20030804/arch/i386/kernel/smp.c
@@ -551,6 +551,7 @@ void smp_send_stop(void)
 
 	local_irq_disable();
 	disable_local_APIC();
+	disconnect_bsp_APIC();
 	local_irq_enable();
 }
 

You should call disconnect_bsp_APIC() just after disable_local_APIC() for the UP case from your new code as well.

Maciej

+  Maciej W. Rozycki, Technical University of Gdansk, Poland   +
+--------------------------------------------------------------+
+        e-mail: [email protected], PGP key available        +

Here is a patch for 2.4.22-rc2 to disable the local APIC before reboot. This fixes BIOS reboot problems reported by a few people.

disable_local_APIC() now checks if detect_init_APIC() enabled the local APIC via the APIC_BASE MSR, and if so it now disables APIC_BASE. Previously we would leave APIC_BASE enabled, and that made some BIOSen unhappy.

The SMP reboot code calls disable_local_APIC(). On SMP HW there is no change since detect_init_APIC() isn't called and APIC_BASE isn't enabled by us. An SMP kernel on UP HW behaves just like an UP_APIC kernel, so it disables APIC_BASE if we enabled it at boot.

The UP_APIC disable-before-suspend code is simplified since the existing code to disable APIC_BASE is moved into disable_local_APIC().

(Felix Kühling originally reported the BIOS reboot problem. This is a fixed-up version of his preliminary patch.)

/Mikael

@@ -38,6 +38,8 @@
 int prof_old_multiplier[NR_CPUS] = { 1, };
 int prof_counter[NR_CPUS] = { 1, };
 
+static int enabled_via_apicbase;
+
 int get_maxlvt(void)
 {
 	unsigned int v, ver, maxlvt;

@@ -142,6 +144,13 @@
 	value = apic_read(APIC_SPIV);
 	value &= ~APIC_SPIV_APIC_ENABLED;
 	apic_write_around(APIC_SPIV, value);
+
+	if (enabled_via_apicbase) {
+		unsigned int l, h;
+		rdmsr(MSR_IA32_APICBASE, l, h);
+		l &= ~MSR_IA32_APICBASE_ENABLE;
+		wrmsr(MSR_IA32_APICBASE, l, h);
+	}
 }
 
 /*

@@ -464,7 +473,6 @@
 
 static void apic_pm_suspend(void *data)
 {
-	unsigned int l, h;
 	unsigned long flags;
 
 	if (apic_pm_state.perfctr_pmdev)

@@ -484,9 +492,6 @@
 	__save_flags(flags);
 	__cli();
 	disable_local_APIC();
-	rdmsr(MSR_IA32_APICBASE, l, h);
-	l &= ~MSR_IA32_APICBASE_ENABLE;
-	wrmsr(MSR_IA32_APICBASE, l, h);
 	__restore_flags(flags);
 }
 

@@ -636,6 +641,7 @@
 			l &= ~MSR_IA32_APICBASE_BASE;
 			l |= MSR_IA32_APICBASE_ENABLE | APIC_DEFAULT_PHYS_BASE;
 			wrmsr(MSR_IA32_APICBASE, l, h);
+			enabled_via_apicbase = 1;
 		}
 	}
 	/*

@@ -47,6 +47,7 @@
 #ifdef CONFIG_MATH_EMULATION
 #include <asm/math_emu.h>
 #endif
+#include <asm/apic.h>
 
 #include <linux/irq.h>
 

@@ -399,6 +400,14 @@
 	 * other OSs see a clean IRQ state.
 	 */
 	smp_send_stop();
+#elif CONFIG_X86_LOCAL_APIC
+	if (cpu_has_apic) {
+		__cli();
+		disable_local_APIC();
+		__sti();
+	}
+#endif
+#ifdef CONFIG_X86_IO_APIC
 	disable_IO_APIC();
 #endif
 

Hi Mikael,

I can happily confirm that this one makes my VAIO reboot correctly. Thanks !

Willy

On Tuesday 12 August 2003 01:33, Mikael Pettersson wrote:

Hi Mikael,

please correct me if I say something really stupid now but shouldn't the APIC be disabled only during reboot time and enabled again at a new boot?

my experience with this patch is, after a reboot he APIC isn't enabled again until I power off my machine.

ciao, Marc

Mikael Pettersson wrote:
:

Wow this one makes my PackardBell Easynote E3245 reboot correctly.

Thanks,
luca