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
--- bk-linus/arch/i386/kernel/reboot.c 2003-05-13 11:51:12.000000000 +0100
+++ linux-2.5/arch/i386/kernel/reboot.c 2003-07-16 02:54:29.000000000 +0100
@@ -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
On Mon, Aug 11, 2003 at 02:40:24PM +0100, [email protected] wrote:
> 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.
>
--- linux-2.4.22-rc2/arch/i386/kernel/process.c Wed Jul 30 09:18:21 2003
+++ linux-2.4.22-rc2-lapic/arch/i386/kernel/process.c Mon Aug 11 17:18:57 2003
@@ -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) {
[email protected] writes:
> diff -urpN --exclude-from=/home/davej/.exclude bk-linus/arch/i386/kernel/reboot.c linux-2.5/arch/i386/kernel/reboot.c
> --- bk-linus/arch/i386/kernel/reboot.c 2003-05-13 11:51:12.000000000 +0100
> +++ linux-2.5/arch/i386/kernel/reboot.c 2003-07-16 02:54:29.000000000 +0100
> @@ -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
I agree we should probably disable the local APIC at reboot if we
enabled it previously, but this patch is broken. CONFIG_X86_LOCAL_APIC
doesn't imply that the CPU actually has one, and even if it does, the
access method may be different (e.g. P5 vs P6/K7/P4, and who knows how
the future C3 with local APIC will do it).
Only apic.c knows how the local APIC was initialised (if at all), so the
"disable it dammit" procedure needs to be in apic.c too.
Was the original bug report posted to LKML? I don't remember seeing it.
/Mikael
On Mon, Aug 11, 2003 at 06:29:21PM +0200, Mikael Pettersson wrote:
> I agree we should probably disable the local APIC at reboot if we
> enabled it previously, but this patch is broken. CONFIG_X86_LOCAL_APIC
> doesn't imply that the CPU actually has one, and even if it does, the
> access method may be different (e.g. P5 vs P6/K7/P4, and who knows how
> the future C3 with local APIC will do it).
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 ?
> Was the original bug report posted to LKML? I don't remember seeing it.
Yes. I'll bounce it to you.
Dave
--
Dave Jones http://www.codemonkey.org.uk
Dave Jones writes:
> On Mon, Aug 11, 2003 at 06:29:21PM +0200, Mikael Pettersson wrote:
> > I agree we should probably disable the local APIC at reboot if we
> > enabled it previously, but this patch is broken. CONFIG_X86_LOCAL_APIC
> > doesn't imply that the CPU actually has one, and even if it does, the
> > access method may be different (e.g. P5 vs P6/K7/P4, and who knows how
> > the future C3 with local APIC will do it).
>
> 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 ?
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.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
--- linux-2.4.22-rc2/arch/i386/kernel/apic.c.~1~ 2003-06-14 13:30:19.000000000 +0200
+++ linux-2.4.22-rc2/arch/i386/kernel/apic.c 2003-08-11 23:42:39.000000000 +0200
@@ -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;
}
}
/*
--- linux-2.4.22-rc2/arch/i386/kernel/process.c.~1~ 2003-08-11 21:27:40.000000000 +0200
+++ linux-2.4.22-rc2/arch/i386/kernel/process.c 2003-08-11 23:11:44.000000000 +0200
@@ -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
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
--- linux-2.6.0-test3/arch/i386/kernel/apic.c.~1~ 2003-07-03 12:32:41.000000000 +0200
+++ linux-2.6.0-test3/arch/i386/kernel/apic.c 2003-08-11 23:49:06.000000000 +0200
@@ -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;
}
}
/*
--- linux-2.6.0-test3/arch/i386/kernel/reboot.c.~1~ 2003-05-28 22:15:58.000000000 +0200
+++ linux-2.6.0-test3/arch/i386/kernel/reboot.c 2003-08-11 23:55:58.000000000 +0200
@@ -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
Hi Mikael,
On Tue, Aug 12, 2003 at 01:33:17AM +0200, Mikael Pettersson wrote:
> 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.
I can happily confirm that this one makes my VAIO reboot correctly.
Thanks !
Willy
On Tue, 12 Aug 2003, Mikael Pettersson wrote:
> @@ -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
You obviously want to disable I/O APICs first.
--
+ Maciej W. Rozycki, Technical University of Gdansk, Poland +
+--------------------------------------------------------------+
+ e-mail: [email protected], PGP key available +
Maciej W. Rozycki writes:
> On Tue, 12 Aug 2003, Mikael Pettersson wrote:
>
> > @@ -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
>
> You obviously want to disable I/O APICs first.
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
On Tue, 12 Aug 2003, Mikael Pettersson wrote:
> > You obviously want to disable I/O APICs first.
>
> 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.
Well, I meant the hardware POV.
> 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).
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
--- linux-mips-2.6.0-test2-20030804.macro/arch/i386/kernel/io_apic.c 2003-07-30 03:56:43.000000000 +0000
+++ linux-mips-2.6.0-test2-20030804/arch/i386/kernel/io_apic.c 2003-08-12 14:21:24.000000000 +0000
@@ -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
--- linux-mips-2.6.0-test2-20030804.macro/arch/i386/kernel/reboot.c 2003-06-06 03:57:19.000000000 +0000
+++ linux-mips-2.6.0-test2-20030804/arch/i386/kernel/reboot.c 2003-08-12 14:25:00.000000000 +0000
@@ -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
--- linux-mips-2.6.0-test2-20030804.macro/arch/i386/kernel/smp.c 2003-06-23 03:57:08.000000000 +0000
+++ linux-mips-2.6.0-test2-20030804/arch/i386/kernel/smp.c 2003-08-12 14:22:49.000000000 +0000
@@ -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 +
On Tuesday 12 August 2003 01:33, Mikael Pettersson wrote:
Hi Mikael,
> 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.)
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
On Fri, 15 Aug 2003 21:22:10 +0200, Marc-Christian Petersen wrote:
>> 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.)
>
>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.
Seems impossible to me. We _know_ how to enable APIC_BASE,
otherwise this code wouldn't trigger at all.
Please give evidence as to what goes wrong, i.e., dmesg logs
from first (cold) and subsequent (warm) boots.
(Preferably from a standard 2.4.22-rc2 so I can verify the code.)
/Mikael
Mikael Pettersson wrote:
:
Wow this one makes my PackardBell Easynote E3245 reboot correctly.
Thanks,
luca