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