Re: [PATCH v2] arch/x86: Fix boot_cpu_data.microcode version output

[Prarit Bhargava]

On 08/01/2018 02:38 AM, Oleksandr Natalenko wrote:
> Hi.
> 
>> 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@xxxxxxxxxx>
>> Signed-off-by: Prarit Bhargava <prarit@xxxxxxxxxx>
>> Cc: stable@xxxxxxxxxxxxxxx
>> Cc: sironi@xxxxxxxxx
>> Cc: tony.luck@xxxxxxxxx
>> ---
>> 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;
>> Â}
>>
>> -- 
>> 2.17.0
> 
> After this patch, do we preserve an original microcode version somewhere? If no,
> why? Sometimes it is useful while debugging another crash because of faulty
> microcode.
> 

Interesting, and thanks for bringing this up.  Oleksandr, under what
circumstances would you want to know what the old version was.  AFAICS it is no
longer running and should not have an impact on the system?

P.

> Thanks.
>