The protocol to safely read MSR 8BH, described in the Intel SDM vol 3A, section 9.11.7.1, explicitly determines that cpuid with EAX=1 must be used between the wrmsr(0x8B, 0); and the rdmsr(0x8B).
The microcode driver was abusing sync_core() to do this, probably because it predates by nearly a decade the current "asm volatile (:::"memory")" implementation of native_cpuid(), which is required for the Intel MSR 8BH access protocol. sync_core() semanthics are that of being a speculative execution barrier, and not "run cpuid with EAX=1". Change the Intel microcode driver to use native_cpuid instead, which is more appropriate. native_cpuid() is already in use by the early microcode driver in one place. Some small code reordering was done to avoid calling cpuid twice in collect_cpu_info_early(). Signed-off-by: Henrique de Moraes Holschuh <h...@hmh.eng.br> --- arch/x86/include/asm/microcode_intel.h | 12 ++++++++++++ arch/x86/kernel/cpu/microcode/intel.c | 6 ++---- arch/x86/kernel/cpu/microcode/intel_early.c | 25 +++++++------------------ 3 files changed, 21 insertions(+), 22 deletions(-) diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h index bbe296e..d40a45e 100644 --- a/arch/x86/include/asm/microcode_intel.h +++ b/arch/x86/include/asm/microcode_intel.h @@ -84,4 +84,16 @@ static inline int save_mc_for_early(u8 *mc) } #endif +static inline unsigned int intel_ucode_cpuid_1(void) +{ + unsigned int eax, ebx, ecx, edx; + + eax = 1; + ecx = 0; + /* extremely important: must be asm volatile(:::"memory") */ + native_cpuid(&eax, &ebx, &ecx, &edx); + + return eax; +} + #endif /* _ASM_X86_MICROCODE_INTEL_H */ diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index e99cdd8..2182cec 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -94,8 +94,6 @@ static void __collect_cpu_info(int cpu_num, struct cpu_signature *csig) memset(csig, 0, sizeof(*csig)); - csig->sig = cpuid_eax(0x00000001); - if ((c->x86_model >= 5) || (c->x86 > 6)) { /* get processor flags from MSR 0x17 */ rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); @@ -104,7 +102,7 @@ static void __collect_cpu_info(int cpu_num, struct cpu_signature *csig) /* get the current microcode revision from MSR 0x8B */ wrmsr(MSR_IA32_UCODE_REV, 0, 0); - sync_core(); + csig->sig = intel_ucode_cpuid_1(); /* a cpuid(1) must happen here */ rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]); csig->rev = val[1]; @@ -174,7 +172,7 @@ static int apply_microcode_intel(int cpu) /* get the current revision from MSR 0x8B */ wrmsr(MSR_IA32_UCODE_REV, 0, 0); - sync_core(); + intel_ucode_cpuid_1(); rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]); if (val[1] != mc_intel->hdr.rev) { diff --git a/arch/x86/kernel/cpu/microcode/intel_early.c b/arch/x86/kernel/cpu/microcode/intel_early.c index 8ad50d6..92629a8 100644 --- a/arch/x86/kernel/cpu/microcode/intel_early.c +++ b/arch/x86/kernel/cpu/microcode/intel_early.c @@ -379,7 +379,6 @@ static int collect_cpu_info_early(struct ucode_cpu_info *uci) unsigned int val[2]; u8 x86, x86_model; struct cpu_signature csig; - unsigned int eax, ebx, ecx, edx; csig.sig = 0; csig.pf = 0; @@ -387,10 +386,11 @@ static int collect_cpu_info_early(struct ucode_cpu_info *uci) memset(uci, 0, sizeof(*uci)); - eax = 0x00000001; - ecx = 0; - native_cpuid(&eax, &ebx, &ecx, &edx); - csig.sig = eax; + /* get the current revision from MSR 0x8B */ + native_wrmsr(MSR_IA32_UCODE_REV, 0, 0); + csig.sig = intel_ucode_cpuid_1(); /* a cpuid(1) must happen here */ + native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]); + csig.rev = val[1]; x86 = get_x86_family(csig.sig); x86_model = get_x86_model(csig.sig); @@ -400,15 +400,6 @@ static int collect_cpu_info_early(struct ucode_cpu_info *uci) native_rdmsr(MSR_IA32_PLATFORM_ID, val[0], val[1]); csig.pf = 1 << ((val[1] >> 18) & 7); } - native_wrmsr(MSR_IA32_UCODE_REV, 0, 0); - - /* As documented in the SDM: Do a CPUID 1 here */ - sync_core(); - - /* get the current revision from MSR 0x8B */ - native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]); - - csig.rev = val[1]; uci->cpu_sig = csig; uci->valid = 1; @@ -679,12 +670,10 @@ static int apply_microcode_early(struct mc_saved_data *mc_saved_data, native_wrmsr(MSR_IA32_UCODE_WRITE, (unsigned long) mc_intel->bits, (unsigned long) mc_intel->bits >> 16 >> 16); - native_wrmsr(MSR_IA32_UCODE_REV, 0, 0); - - /* As documented in the SDM: Do a CPUID 1 here */ - sync_core(); /* get the current revision from MSR 0x8B */ + native_wrmsr(MSR_IA32_UCODE_REV, 0, 0); + intel_ucode_cpuid_1(); native_rdmsr(MSR_IA32_UCODE_REV, val[0], val[1]); if (val[1] != mc_intel->hdr.rev) { print_ucode(INTEL_EARLYMCU_REJECTED, uci, mc_intel->hdr.rev); -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/