Re: [PATCH 3.17 100/141] x86, microcode: Fix accessing dis_ucode_ldr on 32-bit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 11/25/2014 05:18 PM, Borislav Petkov wrote:
Adding x86 people.

On Tue, Nov 25, 2014 at 04:59:34PM -0500, Boris Ostrovsky wrote:
This should be cpuid(0x1, &eax, &ebx, &ecx, &edx). Otherwise we are not
getting bits that the hypervisor wants the guest to see (on Xen cpuid()
turns into hypercall, on baremetal it's native).

With that change it works and

Tested-by: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>

Sigh... I take this back. It breaks 32-bit baremetal. I haven't looked any further but it seems to be dying very early. I suspect cpuid pv_op is not set up yet. If that's true, perhaps you could check whether it is valid in x86_guest()?

I won't be able to do anything tomorrow morning, the best I can hope for is evening.


-boris



Thanks for testing.

(May be worth adding a comment as to what is_guest() is checking for since
31 is a magic number).
See below.

BTW, the crash had nothing to do with accessing dis_ucode_ldr, we are
crashing much later, in load_ucode_intel_ap(), trying to access
*initrd_start_p. And the reason we didn't crash before was because compiler
optimized out whole load_ucode_ap() since check_loader_disabled_ap() was
always true.
Right, and my fix actually uncovered the original issue :-\

Ok, here's a v2 which adds the check to the late loader too, for
completeness. I'll write a proper commit message tomorrow.

---
diff --git a/arch/x86/include/asm/microcode.h b/arch/x86/include/asm/microcode.h
index 64dc362506b7..654907db5f09 100644
--- a/arch/x86/include/asm/microcode.h
+++ b/arch/x86/include/asm/microcode.h
@@ -87,4 +87,9 @@ static inline int __init save_microcode_in_initrd(void)
  }
  #endif
+/* Check whether we're running as a guest on a hypervisor. */
+static inline bool x86_guest(void)
+{
+	return !!(cpuid_ecx(1) & BIT(31));
+}
  #endif /* _ASM_X86_MICROCODE_H */
diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index 2ce9051174e6..0b6db2a97f61 100644
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -557,6 +557,9 @@ static int __init microcode_init(void)
  	struct cpuinfo_x86 *c = &cpu_data(0);
  	int error;
+ if (x86_guest())
+		return 0;
+
  	if (dis_ucode_ldr)
  		return 0;
diff --git a/arch/x86/kernel/cpu/microcode/core_early.c b/arch/x86/kernel/cpu/microcode/core_early.c
index 2c017f242a78..dfa93e74c370 100644
--- a/arch/x86/kernel/cpu/microcode/core_early.c
+++ b/arch/x86/kernel/cpu/microcode/core_early.c
@@ -98,6 +98,9 @@ void __init load_ucode_bsp(void)
  {
  	int vendor, x86;
+ if (x86_guest())
+		return;
+
  	if (check_loader_disabled_bsp())
  		return;
@@ -134,6 +137,9 @@ void load_ucode_ap(void)
  {
  	int vendor, x86;
+ if (x86_guest())
+		return;
+
  	if (check_loader_disabled_ap())
  		return;

--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]