Messages in this thread Patch in this message | | | From | "Sironi, Filippo" <> | Subject | Re: [PATCH v2] arch/x86: Fix boot_cpu_data.microcode version output | Date | Tue, 31 Jul 2018 11:46:09 +0000 |
| |
> On 31. Jul 2018, at 13:27, Prarit Bhargava <prarit@redhat.com> wrote: > > I tested this on AMD Ryzen & Intel Broadwell system and dumped the > boot_cpu_data before and after a microcode update. On the Intel > system I also did a fatal MCE using mce-inject to confirm the output > from the mce handling code. > > P. > > ---8<--- > > On systems where a runtime microcode update has occurred the microcode > version output in a MCE log record is wrong because > boot_cpu_data.microcode is not updated during runtime. > > Update boot_cpu_data.microcode when the BSP's microcode is updated. > > Fixes: fa94d0c6e0f3 ("x86/MCE: Save microcode revision in machine check records") > Suggested-by: Borislav Petkov <bp@alien8.com> > Signed-off-by: Prarit Bhargava <prarit@redhat.com> > Cc: stable@vger.kernel.org > Cc: sironi@amazon.de > Cc: tony.luck@intel.com > --- > Changes in v2: Use mc_amd->hdr.patch_id on AMD > > arch/x86/kernel/cpu/microcode/amd.c | 4 ++++ > arch/x86/kernel/cpu/microcode/intel.c | 4 ++++ > 2 files changed, 8 insertions(+) > > diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c > index 0624957aa068..63b072377ba4 100644 > --- a/arch/x86/kernel/cpu/microcode/amd.c > +++ b/arch/x86/kernel/cpu/microcode/amd.c > @@ -537,6 +537,10 @@ static enum ucode_state apply_microcode_amd(int cpu) > uci->cpu_sig.rev = mc_amd->hdr.patch_id; > c->microcode = mc_amd->hdr.patch_id; > > + /* Update boot_cpu_data's revision too, if we're on the BSP: */ > + if (c->cpu_index == boot_cpu_data.cpu_index) > + boot_cpu_data.microcode = mc_amd->hdr.patch_id; > + > return UCODE_UPDATED; > } > > diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c > index 97ccf4c3b45b..256d336cbc04 100644 > --- a/arch/x86/kernel/cpu/microcode/intel.c > +++ b/arch/x86/kernel/cpu/microcode/intel.c > @@ -851,6 +851,10 @@ static enum ucode_state apply_microcode_intel(int cpu) > uci->cpu_sig.rev = rev; > c->microcode = rev; > > + /* Update boot_cpu_data's revision too, if we're on the BSP: */ > + if (c->cpu_index == boot_cpu_data.cpu_index) > + boot_cpu_data.microcode = rev; > + > return UCODE_UPDATED; > }
There may be a chance of skipping this code, I think.
If the microcode is loaded on the hyperthread sibling of the boot cpu before being loaded on the boot cpu, the boot cpu will exit earlier from apply_microcode_intel() - in if (rev >= mc->hdr.rev) { ... }.
(This seems to be possible in apply_microcode_amd() as well.)
In my tree with the aforementioned change - Intel only - I also had the following patch:
diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 97ccf4c3b45b..4bc869e829eb 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -797,6 +797,7 @@ static enum ucode_state apply_microcode_intel(int cpu) struct microcode_intel *mc; static int prev_rev; u32 rev; + enum ucode_state ret; /* We should bind the task to the CPU */ if (WARN_ON(raw_smp_processor_id() != cpu)) @@ -817,9 +818,8 @@ static enum ucode_state apply_microcode_intel(int cpu) */ rev = intel_get_microcode_revision(); if (rev >= mc->hdr.rev) { - uci->cpu_sig.rev = rev; - c->microcode = rev; - return UCODE_OK; + ret = UCODE_OK; + goto out; } /* @@ -848,10 +848,12 @@ static enum ucode_state apply_microcode_intel(int cpu) prev_rev = rev; } + ret = UCODE_UPDATED; +out: uci->cpu_sig.rev = rev; c->microcode = rev; - return UCODE_UPDATED; + return ret; } static enum ucode_state generic_load_microcode(int cpu, void *data, size_t size, which prevents the issue.
> -- > 2.17.0 > >
Amazon Development Center Germany GmbH Berlin - Dresden - Aachen main office: Krausenstr. 38, 10117 Berlin Geschaeftsfuehrer: Dr. Ralf Herbrich, Christian Schlaeger Ust-ID: DE289237879 Eingetragen am Amtsgericht Charlottenburg HRB 149173 B
| |