Re: Fail to early boot with v2.6.27-rc2 to at least v2.6.29-rc2 due to dc1e35c

[Suresh Siddha]

On Tue, Jan 20, 2009 at 09:20:37PM -0800, Andi Kleen wrote:
> "H. Peter Anvin" <hpa@xxxxxxxxx> writes:
> >
> > We should, or if this block is reversible, we should probably just
> > undo it (the reason people put this block in places is because of,
> > ahem, inferior operating systems having bugs.)
> 
> Even if it's undone it would be still a good idea to make the cpuid
> code bulletproof, just in case someone writes a broken emulator or similar.

Ok. Here is the patch for this aswell. Thanks.

---
From: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
Subject: [patch] x86: check for the presence of cpuid xsave leaf

Avuton Olrich reported early boot crashes with v2.6.28 and
bisected it down to dc1e35c6e95e8923cf1d3510438b63c600fee1e2
("x86, xsave: enable xsave/xrstor on cpus with xsave support").

Root cause of this showed that bios has enabled the "Max CPUID Value Limit:"
option which confuses the kernel by showing xsave capability without
the cpuid xsave leaf.

Peter fixed the impact of bios limiting the cpuid limit by checking
for the limit bit set in the MSR_IA32_MISC_ENABLE is set and clearing it.

Andi suggests to make xsave code also bulletproof, just incase if someone
writes a  broken simulator.

Check for the presence of CPUID_XSAVE_LEAF before enabling it.

Reported-and-bisected-by: Avuton Olrich <avuton@xxxxxxxxx>
Tested-by: Avuton Olrich <avuton@xxxxxxxxx>
Signed-off-by: Suresh Siddha <suresh.b.siddha@xxxxxxxxx>
---

diff --git a/arch/x86/include/asm/xsave.h b/arch/x86/include/asm/xsave.h
index 08e9a1a..96bb62f 100644
--- a/arch/x86/include/asm/xsave.h
+++ b/arch/x86/include/asm/xsave.h
@@ -27,7 +27,7 @@ extern unsigned int xstate_size;
 extern u64 pcntxt_mask;
 extern struct xsave_struct *init_xstate_buf;
 
-extern void xsave_cntxt_init(void);
+extern int xsave_cntxt_init(void);
 extern void xsave_init(void);
 extern int init_fpu(struct task_struct *child);
 extern int check_for_xstate(struct i387_fxsave_struct __user *buf,
diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
index b0f61f0..2e2e8b1 100644
--- a/arch/x86/kernel/i387.c
+++ b/arch/x86/kernel/i387.c
@@ -65,10 +65,8 @@ void __cpuinit init_thread_xstate(void)
 		return;
 	}
 
-	if (cpu_has_xsave) {
-		xsave_cntxt_init();
+	if (!xsave_cntxt_init())
 		return;
-	}
 
 	if (cpu_has_fxsr)
 		xstate_size = sizeof(struct i387_fxsave_struct);
diff --git a/arch/x86/kernel/xsave.c b/arch/x86/kernel/xsave.c
index 2b54fe0..5384a3b 100644
--- a/arch/x86/kernel/xsave.c
+++ b/arch/x86/kernel/xsave.c
@@ -307,14 +307,25 @@ static void __init setup_xstate_init(void)
 	init_xstate_buf->i387.mxcsr = MXCSR_DEFAULT;
 }
 
+#define CPUID_XSAVE_LEAF	0xd
+
 /*
  * Enable and initialize the xsave feature.
  */
-void __ref xsave_cntxt_init(void)
+int __ref xsave_cntxt_init(void)
 {
 	unsigned int eax, ebx, ecx, edx;
 
-	cpuid_count(0xd, 0, &eax, &ebx, &ecx, &edx);
+	if (!cpu_has_xsave)
+		return -1;
+
+	if (boot_cpu_data.cpuid_level < CPUID_XSAVE_LEAF) {
+		printk(KERN_ERR "cpuid xsave leaf 0xd not supported\n");
+		setup_clear_cpu_cap(X86_FEATURE_XSAVE);
+		return -1;
+	}
+
+	cpuid_count(CPUID_XSAVE_LEAF, 0, &eax, &ebx, &ecx, &edx);
 	pcntxt_mask = eax + ((u64)edx << 32);
 
 	if ((pcntxt_mask & XSTATE_FPSSE) != XSTATE_FPSSE) {
@@ -342,4 +353,5 @@ void __ref xsave_cntxt_init(void)
 	printk(KERN_INFO "xsave/xrstor: enabled xstate_bv 0x%llx, "
 	       "cntxt size 0x%x\n",
 	       pcntxt_mask, xstate_size);
+	return 0;
 }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/