[PATCH] Fix paranoia about using BIOS quickboot mechanism.

[PATCH] Fix paranoia about using BIOS quickboot mechanism.

[Alok Kataria]

Make sure that we clear the "shutdown status flag" in the CMOS register
after each CPU is brought up.  This fixes a problem where the "shutdown
status flag" may remain set when a CPU is brought up after booting.

Tested on the X86 tree.

Signed-off-by: Alok N Kataria <akataria@vmware.com>
Signed-off-by: Dan Arai <arai@vmware.com>

---
arch/x86/kernel/smpboot.c |   10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5321e35..0b8bba8 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1010,6 +1010,11 @@ do_rest:
/* mark "stuck" area as not stuck */
*((volatile unsigned long *)trampoline_base) = 0;

+ /*
+ * Cleanup possible dangling ends...
+ */
+ smpboot_restore_warm_reset_vector();
+
return boot_error;
}

@@ -1263,11 +1268,6 @@ void __init native_smp_prepare_boot_cpu(void)

void __init native_smp_cpus_done(unsigned int max_cpus)
{
- /*
- * Cleanup possible dangling ends...
- */
- smpboot_restore_warm_reset_vector();
-
Dprintk("Boot done.\n");

impress_friends();
-- 
1.5.4

Re: [PATCH] Fix paranoia about using BIOS quickboot mechanism.

[Ingo Molnar]

* Alok Kataria <akataria@vmware.com> wrote:

> Make sure that we clear the "shutdown status flag" in the CMOS 
> register after each CPU is brought up.  This fixes a problem where the 
> "shutdown status flag" may remain set when a CPU is brought up after 
> booting.

btw., what problem does this result in, exactly?

> Tested on the X86 tree.

> + * Cleanup possible dangling ends...
> + */
> + smpboot_restore_warm_reset_vector();
> +
> return boot_error;
> }

patch got whitespace damaged - have a look at 
Documentation/email-clients.txt for details about how to send patches as 
in-line text. (or send the patch as an attachment - we accept x86 
patches as attachments too)

	Ingo

Re: [PATCH] Fix paranoia about using BIOS quickboot mechanism.

[Alok Kataria]

[-- Attachment #1: Type: text/plain, Size: 1492 bytes --]

On Mon, 2008-04-07 at 10:22 +0200, Ingo Molnar wrote:
> * Alok Kataria <akataria@vmware.com> wrote:
> 
> > Make sure that we clear the "shutdown status flag" in the CMOS 
> > register after each CPU is brought up.  This fixes a problem where the 
> > "shutdown status flag" may remain set when a CPU is brought up after 
> > booting.
> 
> btw., what problem does this result in, exactly?

The shutdown status flag set to "0xA", corresponds to "JMP double word
request without INT init".

This JMP at reboot time is at an unintended location. And results in
Triple faults in our case.
Though this error at reboot can be safely ignored in a VM environment,
am not sure what the effect would be on a physical system. May be it
will result in a triple fault and an eventual hardware reset thus
masking this BUG in the kernel.

This fix just makes sure that we reset that status flag after
initialization is done.

> 
> > Tested on the X86 tree.
> 
> > + * Cleanup possible dangling ends...
> > + */
> > + smpboot_restore_warm_reset_vector();
> > +
> > return boot_error;
> > }
> 
> patch got whitespace damaged - have a look at 
> Documentation/email-clients.txt for details about how to send patches as 
> in-line text. (or send the patch as an attachment - we accept x86 
> patches as attachments too)

Ah...I think i followed the instructions there, will have to look what
went wrong, anyways, will go for the easier solution, here is the patch
attached.  

Thanks & Regards,
Alok

> 
> 	Ingo

[-- Attachment #2: fix-reboot-after-cpu-hotplug.patch --]
[-- Type: text/x-patch, Size: 1190 bytes --]

Fix paranoia about using BIOS quickboot mechanism.

Make sure that we clear the "shutdown status flag" in the CMOS register
after each CPU is brought up.  This fixes a problem where the "shutdown
status flag" may remain set when a CPU is brought up after booting.

Signed-off-by: Alok N Kataria <akataria@vmware.com>
Signed-off-by: Dan Arai <arai@vmware.com>

---
 arch/x86/kernel/smpboot.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/x86/kernel/smpboot.c b/arch/x86/kernel/smpboot.c
index 5321e35..0b8bba8 100644
--- a/arch/x86/kernel/smpboot.c
+++ b/arch/x86/kernel/smpboot.c
@@ -1010,6 +1010,11 @@ do_rest:
 	/* mark "stuck" area as not stuck */
 	*((volatile unsigned long *)trampoline_base) = 0;
 
+	/*
+	 * Cleanup possible dangling ends...
+	 */
+	smpboot_restore_warm_reset_vector();
+
 	return boot_error;
 }
 
@@ -1263,11 +1268,6 @@ void __init native_smp_prepare_boot_cpu(void)
 
 void __init native_smp_cpus_done(unsigned int max_cpus)
 {
-	/*
-	 * Cleanup possible dangling ends...
-	 */
-	smpboot_restore_warm_reset_vector();
-
 	Dprintk("Boot done.\n");
 
 	impress_friends();
-- 
1.5.4

Re: [PATCH] Fix paranoia about using BIOS quickboot mechanism.

[Alok Kataria]

Hi Andrew,


Here is the patch that fits on the mainline tree 2.6.25-rc8. The earlier patch that I sent was on X86-git tree. 

Tested for both 32bit and 64bit.

--
Fix paranoia about using BIOS quickboot mechanism.

Make sure that we clear the "shutdown status flag" in the CMOS register
after each CPU is brought up.  This fixes a problem where the "shutdown
status flag" may remain set when a CPU is brought up after booting.

Patch on top of linux-2.6.25-rc8.

Signed-off-by: Alok N Kataria <akataria@vmware.com>
Signed-off-by: Dan Arai <arai@vmware.com>

Index: linux-2.6.25-rc8/arch/x86/kernel/smpboot_32.c
===================================================================
--- linux-2.6.25-rc8.orig/arch/x86/kernel/smpboot_32.c	2008-04-08 11:36:37.000000000 -0700
+++ linux-2.6.25-rc8/arch/x86/kernel/smpboot_32.c	2008-04-08 12:04:00.000000000 -0700
@@ -869,6 +869,11 @@
 	/* mark "stuck" area as not stuck */
 	*((volatile unsigned long *)trampoline_base) = 0;
 
+	/*
+	 * Cleanup possible dangling ends...
+	 */
+	smpboot_restore_warm_reset_vector();
+
 	return boot_error;
 }
 
@@ -1070,11 +1075,6 @@
 	}
 
 	/*
-	 * Cleanup possible dangling ends...
-	 */
-	smpboot_restore_warm_reset_vector();
-
-	/*
 	 * Allow the user to impress friends.
 	 */
 	Dprintk("Before bogomips.\n");
Index: linux-2.6.25-rc8/arch/x86/kernel/smpboot_64.c
===================================================================
--- linux-2.6.25-rc8.orig/arch/x86/kernel/smpboot_64.c	2008-04-08 11:36:37.000000000 -0700
+++ linux-2.6.25-rc8/arch/x86/kernel/smpboot_64.c	2008-04-08 12:04:01.000000000 -0700
@@ -546,6 +546,23 @@
 }
 
 /*
+ * Cleanup possible dangling ends...
+ */
+static __cpuinit void smp_cleanup_boot(void)
+{
+	/*
+	 * Paranoid:  Set warm reset code and vector here back
+	 * to default values.
+	 */
+	CMOS_WRITE(0, 0xf);
+
+	/*
+	 * Reset trampoline flag
+	 */
+	*((volatile int *) phys_to_virt(0x467)) = 0;
+}
+
+/*
  * Boot one CPU.
  */
 static int __cpuinit do_boot_cpu(int cpu, int apicid)
@@ -699,6 +716,7 @@
 #endif
 		}
 	}
+	smp_cleanup_boot();
 	if (boot_error) {
 		cpu_clear(cpu, cpu_callout_map); /* was set here (do_boot_cpu()) */
 		clear_bit(cpu, (unsigned long *)&cpu_initialized); /* was set by cpu_init() */
@@ -716,23 +734,6 @@
 unsigned long cache_decay_ticks;
 
 /*
- * Cleanup possible dangling ends...
- */
-static __cpuinit void smp_cleanup_boot(void)
-{
-	/*
-	 * Paranoid:  Set warm reset code and vector here back
-	 * to default values.
-	 */
-	CMOS_WRITE(0, 0xf);
-
-	/*
-	 * Reset trampoline flag
-	 */
-	*((volatile int *) phys_to_virt(0x467)) = 0;
-}
-
-/*
  * Fall back to non SMP mode after errors.
  *
  * RED-PEN audit/test this more. I bet there is more state messed up here.
@@ -989,7 +990,6 @@
  */
 void __init smp_cpus_done(unsigned int max_cpus)
 {
-	smp_cleanup_boot();
 	setup_ioapic_dest();
 	check_nmi_watchdog();
 }