[PATCH] x86: Fix Intel microcode revision detection

From: Junichi Nomura
Date: Tue Dec 27 2016 - 23:43:13 EST


early_init_intel() calls sync_core() before rdmsr(MSR_IA32_UCODE_REV),
assuming sync_core() is effectively CPUID(eax=1). However the assumption
no longer holds since commit c198b121b1a1 ("x86/asm: Rewrite sync_core()
to use IRET-to-self").

As a result, kernel fails to detect the revision of microcode, such as:
microcode: sig=0x206a7, pf=0x2, revision=0x0

Conversion from sync_core() to native_cpuid() has already been done in
Intel microcode driver by commit 484d0e5c7943 ("x86/microcode/intel:
Replace sync_core() with native_cpuid()"). This patch just extends the
conversion for early_init_intel().

Fixes: c198b121b1a1 ("x86/asm: Rewrite sync_core() to use IRET-to-self")
Signed-off-by: Jun'ichi Nomura <j-nomura@xxxxxxxxxxxxx>
Cc: Andy Lutomirski <luto@xxxxxxxxxxxxxx>

diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h
index 195becc..19cb4c4 100644
--- a/arch/x86/include/asm/microcode_intel.h
+++ b/arch/x86/include/asm/microcode_intel.h
@@ -66,4 +66,24 @@ static inline void show_ucode_info_early(void) {}
static inline void reload_ucode_intel(void) {}
#endif

+static inline void intel_microcode_cpuid_1(void)
+{
+ /*
+ * According to the Intel SDM, Volume 3, 9.11.7:
+ *
+ * CPUID returns a value in a model specific register in
+ * addition to its usual register return values. The
+ * semantics of CPUID cause it to deposit an update ID value
+ * in the 64-bit model-specific register at address 08BH
+ * (IA32_BIOS_SIGN_ID). If no update is present in the
+ * processor, the value in the MSR remains unmodified.
+ *
+ * Use native_cpuid -- this code runs very early and we don't
+ * want to mess with paravirt.
+ */
+ unsigned int eax = 1, ebx, ecx = 0, edx;
+
+ native_cpuid(&eax, &ebx, &ecx, &edx);
+}
+
#endif /* _ASM_X86_MICROCODE_INTEL_H */
diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
index fcd484d..edc38a9 100644
--- a/arch/x86/kernel/cpu/intel.c
+++ b/arch/x86/kernel/cpu/intel.c
@@ -14,6 +14,7 @@
#include <asm/bugs.h>
#include <asm/cpu.h>
#include <asm/intel-family.h>
+#include <asm/microcode_intel.h>

#ifdef CONFIG_X86_64
#include <linux/topology.h>
@@ -83,7 +84,7 @@ static void early_init_intel(struct cpuinfo_x86 *c)

wrmsr(MSR_IA32_UCODE_REV, 0, 0);
/* Required by the SDM */
- sync_core();
+ intel_microcode_cpuid_1();
rdmsr(MSR_IA32_UCODE_REV, lower_word, c->microcode);
}

diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c
index b624b54..546c6cf 100644
--- a/arch/x86/kernel/cpu/microcode/intel.c
+++ b/arch/x86/kernel/cpu/microcode/intel.c
@@ -368,26 +368,6 @@ static int microcode_sanity_check(void *mc, int print_err)
return patch;
}

-static void cpuid_1(void)
-{
- /*
- * According to the Intel SDM, Volume 3, 9.11.7:
- *
- * CPUID returns a value in a model specific register in
- * addition to its usual register return values. The
- * semantics of CPUID cause it to deposit an update ID value
- * in the 64-bit model-specific register at address 08BH
- * (IA32_BIOS_SIGN_ID). If no update is present in the
- * processor, the value in the MSR remains unmodified.
- *
- * Use native_cpuid -- this code runs very early and we don't
- * want to mess with paravirt.
- */
- unsigned int eax = 1, ebx, ecx = 0, edx;
-
- native_cpuid(&eax, &ebx, &ecx, &edx);
-}
-
static int collect_cpu_info_early(struct ucode_cpu_info *uci)
{
unsigned int val[2];
@@ -413,7 +393,7 @@ static int collect_cpu_info_early(struct ucode_cpu_info *uci)
native_wrmsrl(MSR_IA32_UCODE_REV, 0);

/* As documented in the SDM: Do a CPUID 1 here */
- cpuid_1();
+ intel_microcode_cpuid_1();

/* get the current revision from MSR 0x8B */
native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]);
@@ -613,7 +593,7 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early)
native_wrmsrl(MSR_IA32_UCODE_REV, 0);

/* As documented in the SDM: Do a CPUID 1 here */
- cpuid_1();
+ intel_microcode_cpuid_1();

/* get the current revision from MSR 0x8B */
native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]);
@@ -825,7 +805,7 @@ static int apply_microcode_intel(int cpu)
wrmsrl(MSR_IA32_UCODE_REV, 0);

/* As documented in the SDM: Do a CPUID 1 here */
- cpuid_1();
+ intel_microcode_cpuid_1();

/* get the current revision from MSR 0x8B */
rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]);