Messages in this thread | | | From | Evalds Iodzevics <> | Date | Mon, 20 Apr 2020 17:01:01 +0300 | Subject | Re: [PATCH] x86/microcode/intel: replace sync_core() with native_cpuid_reg(eax) |
| |
On Mon, Apr 20, 2020 at 4:33 PM Ben Hutchings <ben@decadent.org.uk> wrote: > > 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@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. > > 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. > As far as i can tell, these two:
484d0e5c7943 x86/microcode/intel: Replace sync_core() with native_cpuid() f3e2a51f568d x86/microcode: Use native CPUID to tickle out microcode revision
are already made obsolete by:
98cc1464cfd6 x86/microcode/intel: Add a helper which gives the microcode revision
except that it still relied on sync_core() witch did not behave correctly on 32 bit machines
| |