linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Disable APIC on reboot.
@ 2003-08-11 13:40 davej
  2003-08-11 15:33 ` [PATCH-2.4] " Willy Tarreau
  2003-08-11 16:29 ` [PATCH] " Mikael Pettersson
  0 siblings, 2 replies; 14+ messages in thread
From: davej @ 2003-08-11 13:40 UTC (permalink / raw)
  To: torvalds; +Cc: linux-kernel

Forward port of a patch from Felix [surname mangled by MTA] <fxkuehl@gmx.de>

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) {

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH-2.4] Disable APIC on reboot.
  2003-08-11 13:40 [PATCH] Disable APIC on reboot davej
@ 2003-08-11 15:33 ` Willy Tarreau
  2003-08-11 16:29 ` [PATCH] " Mikael Pettersson
  1 sibling, 0 replies; 14+ messages in thread
From: Willy Tarreau @ 2003-08-11 15:33 UTC (permalink / raw)
  To: davej, marcelo; +Cc: linux-kernel

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, davej@redhat.com wrote:
> Forward port of a patch from Felix [surname mangled by MTA] <fxkuehl@gmx.de>
> 
> 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) {


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Disable APIC on reboot.
  2003-08-11 13:40 [PATCH] Disable APIC on reboot davej
  2003-08-11 15:33 ` [PATCH-2.4] " Willy Tarreau
@ 2003-08-11 16:29 ` Mikael Pettersson
  2003-08-11 16:38   ` Dave Jones
  1 sibling, 1 reply; 14+ messages in thread
From: Mikael Pettersson @ 2003-08-11 16:29 UTC (permalink / raw)
  To: davej; +Cc: torvalds, fxkuehl, linux-kernel, willy, marcelo

davej@redhat.com 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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Disable APIC on reboot.
  2003-08-11 16:29 ` [PATCH] " Mikael Pettersson
@ 2003-08-11 16:38   ` Dave Jones
  2003-08-11 16:57     ` Mikael Pettersson
  0 siblings, 1 reply; 14+ messages in thread
From: Dave Jones @ 2003-08-11 16:38 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: torvalds, fxkuehl, linux-kernel, willy, marcelo

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] Disable APIC on reboot.
  2003-08-11 16:38   ` Dave Jones
@ 2003-08-11 16:57     ` Mikael Pettersson
  2003-08-11 23:33       ` [PATCH][2.6.0-test3] " Mikael Pettersson
  2003-08-11 23:33       ` [PATCH][2.4.22-rc2] " Mikael Pettersson
  0 siblings, 2 replies; 14+ messages in thread
From: Mikael Pettersson @ 2003-08-11 16:57 UTC (permalink / raw)
  To: Dave Jones; +Cc: torvalds, fxkuehl, linux-kernel, willy, marcelo

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH][2.6.0-test3] Disable APIC on reboot.
  2003-08-11 16:57     ` Mikael Pettersson
@ 2003-08-11 23:33       ` Mikael Pettersson
  2003-08-12 13:48         ` Maciej W. Rozycki
  2003-08-11 23:33       ` [PATCH][2.4.22-rc2] " Mikael Pettersson
  1 sibling, 1 reply; 14+ messages in thread
From: Mikael Pettersson @ 2003-08-11 23:33 UTC (permalink / raw)
  To: Dave Jones; +Cc: torvalds, fxkuehl, linux-kernel, willy

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
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH][2.4.22-rc2] Disable APIC on reboot.
  2003-08-11 16:57     ` Mikael Pettersson
  2003-08-11 23:33       ` [PATCH][2.6.0-test3] " Mikael Pettersson
@ 2003-08-11 23:33       ` Mikael Pettersson
  2003-08-12  6:09         ` Willy Tarreau
                           ` (2 more replies)
  1 sibling, 3 replies; 14+ messages in thread
From: Mikael Pettersson @ 2003-08-11 23:33 UTC (permalink / raw)
  To: Dave Jones; +Cc: marcelo, fxkuehl, linux-kernel, willy

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
 

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.4.22-rc2] Disable APIC on reboot.
  2003-08-11 23:33       ` [PATCH][2.4.22-rc2] " Mikael Pettersson
@ 2003-08-12  6:09         ` Willy Tarreau
  2003-08-15 19:22         ` Marc-Christian Petersen
  2003-08-27  7:19         ` Luca Montecchiani
  2 siblings, 0 replies; 14+ messages in thread
From: Willy Tarreau @ 2003-08-12  6:09 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Dave Jones, marcelo, fxkuehl, linux-kernel, willy

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.6.0-test3] Disable APIC on reboot.
  2003-08-11 23:33       ` [PATCH][2.6.0-test3] " Mikael Pettersson
@ 2003-08-12 13:48         ` Maciej W. Rozycki
  2003-08-12 13:57           ` Mikael Pettersson
  0 siblings, 1 reply; 14+ messages in thread
From: Maciej W. Rozycki @ 2003-08-12 13:48 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Dave Jones, torvalds, fxkuehl, linux-kernel, 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: macro@ds2.pg.gda.pl, PGP key available        +


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.6.0-test3] Disable APIC on reboot.
  2003-08-12 13:48         ` Maciej W. Rozycki
@ 2003-08-12 13:57           ` Mikael Pettersson
  2003-08-12 14:31             ` Maciej W. Rozycki
  0 siblings, 1 reply; 14+ messages in thread
From: Mikael Pettersson @ 2003-08-12 13:57 UTC (permalink / raw)
  To: Maciej W. Rozycki; +Cc: Dave Jones, torvalds, fxkuehl, linux-kernel, willy

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.6.0-test3] Disable APIC on reboot.
  2003-08-12 13:57           ` Mikael Pettersson
@ 2003-08-12 14:31             ` Maciej W. Rozycki
  0 siblings, 0 replies; 14+ messages in thread
From: Maciej W. Rozycki @ 2003-08-12 14:31 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Dave Jones, torvalds, fxkuehl, linux-kernel, willy

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: macro@ds2.pg.gda.pl, PGP key available        +


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.4.22-rc2] Disable APIC on reboot.
  2003-08-11 23:33       ` [PATCH][2.4.22-rc2] " Mikael Pettersson
  2003-08-12  6:09         ` Willy Tarreau
@ 2003-08-15 19:22         ` Marc-Christian Petersen
  2003-08-27  7:19         ` Luca Montecchiani
  2 siblings, 0 replies; 14+ messages in thread
From: Marc-Christian Petersen @ 2003-08-15 19:22 UTC (permalink / raw)
  To: Mikael Pettersson, Dave Jones; +Cc: marcelo, fxkuehl, linux-kernel, willy

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


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.4.22-rc2] Disable APIC on reboot.
  2003-08-11 23:33       ` [PATCH][2.4.22-rc2] " Mikael Pettersson
  2003-08-12  6:09         ` Willy Tarreau
  2003-08-15 19:22         ` Marc-Christian Petersen
@ 2003-08-27  7:19         ` Luca Montecchiani
  2 siblings, 0 replies; 14+ messages in thread
From: Luca Montecchiani @ 2003-08-27  7:19 UTC (permalink / raw)
  To: Mikael Pettersson; +Cc: Dave Jones, marcelo, linux-kernel

Mikael Pettersson wrote:
:

Wow this one makes my PackardBell Easynote E3245 reboot correctly.

Thanks,
luca


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH][2.4.22-rc2] Disable APIC on reboot.
@ 2003-08-18 22:34 Mikael Pettersson
  0 siblings, 0 replies; 14+ messages in thread
From: Mikael Pettersson @ 2003-08-18 22:34 UTC (permalink / raw)
  To: m.c.p; +Cc: fxkuehl, linux-kernel, marcelo, willy

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 1382 bytes --]

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2003-08-27  7:25 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-08-11 13:40 [PATCH] Disable APIC on reboot davej
2003-08-11 15:33 ` [PATCH-2.4] " Willy Tarreau
2003-08-11 16:29 ` [PATCH] " Mikael Pettersson
2003-08-11 16:38   ` Dave Jones
2003-08-11 16:57     ` Mikael Pettersson
2003-08-11 23:33       ` [PATCH][2.6.0-test3] " Mikael Pettersson
2003-08-12 13:48         ` Maciej W. Rozycki
2003-08-12 13:57           ` Mikael Pettersson
2003-08-12 14:31             ` Maciej W. Rozycki
2003-08-11 23:33       ` [PATCH][2.4.22-rc2] " Mikael Pettersson
2003-08-12  6:09         ` Willy Tarreau
2003-08-15 19:22         ` Marc-Christian Petersen
2003-08-27  7:19         ` Luca Montecchiani
2003-08-18 22:34 Mikael Pettersson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).