Messages in this thread | | | Date | Mon, 20 Apr 2020 11:31:29 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH] x86/microcode/intel: replace sync_core() with native_cpuid_reg(eax) |
| |
On Mon, Apr 20, 2020 at 03:00:37PM +0300, Evalds Iodzevics wrote: > sync_core() always jums past cpuid instruction on 32 bit machines > because data structure boot_cpu_data are not populated so early in boot.
I'm guessing because boot_cpu_data.cpuid_level is not properly set and very early code in head_32.S sets it to -1 temporarily until the highest CPUID level has been detected (or not).
But the microcode loading happens *before* that.
I'd probably be interested how you trigger that but since the backport got changed (see below) and yours is correcting it to the upstream variant then perhaps maybe I don't care that much. :)
> It depends on commit 5dedade6dfa243c130b85d1e4daba6f027805033 for > native_cpuid_reg(eax) definitions > > This patch is for 4.4 but also should apply to 4.9 > > Signed-off-by: Evalds Iodzevics <evalds.iodzevics@gmail.com> > --- > arch/x86/include/asm/microcode_intel.h | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/x86/include/asm/microcode_intel.h b/arch/x86/include/asm/microcode_intel.h > index 90343ba50485..92ce9c8a508b 100644 > --- a/arch/x86/include/asm/microcode_intel.h > +++ b/arch/x86/include/asm/microcode_intel.h > @@ -60,7 +60,7 @@ static inline u32 intel_get_microcode_revision(void) > native_wrmsrl(MSR_IA32_UCODE_REV, 0); > > /* As documented in the SDM: Do a CPUID 1 here */ > - sync_core(); > + native_cpuid_eax(1); > > /* get the current revision from MSR 0x8B */ > native_rdmsr(MSR_IA32_UCODE_REV, dummy, rev); > --
Hrm, the original patch of mine did use native_cpuid_eax():
4167709bbf82 ("x86/microcode/intel: Add a helper which gives the microcode revision")
but the backport:
commit 98cc1464cfd6edf9dc7fa96aaaf596aae952029b Author: Borislav Petkov <bp@suse.de> Date: Mon Jan 9 12:41:45 2017 +0100
x86/microcode/intel: Add a helper which gives the microcode revision
commit 4167709bbf826512a52ebd6aafda2be104adaec9 upstream.
Since on Intel we're required to do CPUID(1) first, before reading the microcode revision MSR, let's add a special helper which does the required steps so that we don't forget to do them next time, when we want to read the microcode revision.
Signed-off-by: Borislav Petkov <bp@suse.de> Link: http://lkml.kernel.org/r/20170109114147.5082-4-bp@alien8.de Signed-off-by: Thomas Gleixner <tglx@linutronix.de> [bwh: Backported to 4.4: - Don't touch prev_rev variable in apply_microcode() - Keep using sync_core(), which will alway includes the necessary CPUID ^^^^^^^^^^^^^^^^^^^
decided to use sync_core() for whatever reason. Perhaps because the native_cpuid* things weren't there. Adding Ben to Cc.
I believe this is the background info Greg needed to figure out *why* you're doing this.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |