Re: [PATCH] x86/microcode/intel: replace sync_core() with native_cpuid_reg(eax)
From: Ben Hutchings
Date: Mon Apr 20 2020 - 09:33:39 EST
On Mon, 2020-04-20 at 11:31 +0200, Borislav Petkov wrote:
> 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.
[...]
> 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@xxxxxxx>
> 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@xxxxxxx>
> Link: http://lkml.kernel.org/r/20170109114147.5082-4-bp@xxxxxxxxx
> Signed-off-by: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> [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.
This commit didn't introduce the call to native_cpuid_eax(), but only
moved it. So I didn't think it made sense for the backport to change
from sync_core().
If it's important to use native_cpuid_eax() then these older branches
should presumably get backports of:
484d0e5c7943 x86/microcode/intel: Replace sync_core() with native_cpuid()
f3e2a51f568d x86/microcode: Use native CPUID to tickle out microcode revision
Ben.
> I believe this is the background info Greg needed to figure out *why*
> you're doing this.
>
--
Ben Hutchings
Klipstein's 4th Law of Prototyping and Production:
A fail-safe circuit will destroy others.
Attachment:
signature.asc
Description: This is a digitally signed message part