Re: [Xen-devel] [PATCH v3 for 4.13] x86/microcode: refuse to load the same revision ucode

[Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>]

On 27/11/2019 03:10, Chao Gao wrote:
> On Tue, Nov 26, 2019 at 03:41:53PM +0000, Sergey Dyasli wrote:
>> Currently if a user tries to live-load the same or older ucode revision
>> than CPU already has, he will get a single message in Xen log like:
>>
>>    (XEN) 128 cores are to update their microcode
>>
>> No actual ucode loading will happen and this situation can be quite
>> confusing. Fix this by starting ucode update only when the provided
>> ucode revision is higher than the currently cached one (if any).
>> This is based on the property that if microcode_cache exists, all CPUs
>> in the system should have at least that ucode revision.
>>
>> Additionally, print a user friendly message if no matching or newer
>> ucode can be found in the provided blob. This also requires ignoring
>> -ENODATA in AMD-side code, otherwise the message given to the user is:
>>
>>    (XEN) Parsing microcode blob error -61
>>
>> Which actually means that a ucode blob was parsed fine, but no matching
>> ucode was found.
>>
>> Signed-off-by: Sergey Dyasli <sergey.dyasli@xxxxxxxxxx>
>> ---
>> v2 --> v3:
>> - move ucode comparison to generic code
>> - ignore -ENODATA in a different code section
>>
>> v1 --> v2:
>> - compare provided ucode with the currently cached one
>>
>> CC: Jan Beulich <jbeulich@xxxxxxxx>
>> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>> CC: Chao Gao <chao.gao@xxxxxxxxx>
>> CC: Juergen Gross <jgross@xxxxxxxx>
>> ---
>> xen/arch/x86/microcode.c     | 19 +++++++++++++++++++
>> xen/arch/x86/microcode_amd.c |  7 +++++++
>> 2 files changed, 26 insertions(+)
>>
>> diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
>> index 65d1f41e7c..44efc2d9b3 100644
>> --- a/xen/arch/x86/microcode.c
>> +++ b/xen/arch/x86/microcode.c
>> @@ -640,10 +640,29 @@ int 
>> microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
>>
>>     if ( !patch )
>>     {
>> +        printk(XENLOG_WARNING "microcode: couldn't find any matching ucode 
>> in "
>> +                              "the provided blob!\n");
>>         ret = -ENOENT;
>>         goto put;
>>     }
>>
>> +    /*
>> +     * If microcode_cache exists, all CPUs in the system should have at 
>> least
>> +     * that ucode revision.
>> +     */
>> +    spin_lock(&microcode_mutex);
>> +    if ( microcode_cache &&
>> +         microcode_ops->compare_patch(patch, microcode_cache) != NEW_UCODE )
>> +    {
>> +        spin_unlock(&microcode_mutex);
>> +        printk(XENLOG_WARNING "microcode: couldn't find any newer revision "
>> +                              "in the provided blob!\n");
> 
> The patch needs to be freed.

Thanks for noticing this!

--
Sergey

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel