Re: [PATCH v5 2/4] x86/microcode: Ignore microcode loading interface for revision = -1

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.07.2023 16:28, Andrew Cooper wrote:
> On 05/07/2023 3:24 pm, Jan Beulich wrote:
>> On 05.07.2023 16:13, Andrew Cooper wrote:
>>> On 29/06/2023 4:26 pm, Alejandro Vallejo wrote:
>>>> diff --git a/xen/arch/x86/cpu/microcode/core.c 
>>>> b/xen/arch/x86/cpu/microcode/core.c
>>>> index bec8b55db2..b620e3bfa6 100644
>>>> --- a/xen/arch/x86/cpu/microcode/core.c
>>>> +++ b/xen/arch/x86/cpu/microcode/core.c
>>>> @@ -867,10 +867,22 @@ int __init early_microcode_init(unsigned long 
>>>> *module_map,
>>>>          return -ENODEV;
>>>>      }
>>>>  
>>>> -    microcode_grab_module(module_map, mbi);
>>>> -
>>>>      ucode_ops.collect_cpu_info();
>>>>  
>>>> +    /*
>>>> +     * Some hypervisors deliberately report a microcode revision of -1 to
>>>> +     * mean that they will not accept microcode updates. We take the hint
>>>> +     * and ignore the microcode interface in that case.
>>>> +     */
>>>> +    if ( this_cpu(cpu_sig).rev == ~0 )
>>>> +    {
>>>> +        printk(XENLOG_WARNING "Microcode loading disabled\n");
>>> XENLOG_INFO "Found microcode revision ~0;  Disabling loading because of
>>> virt\n"
>>>
>>> It's normal (and not a warning) when running under other hypervisors,
>> Except that INFO won't be visible by default in release configurations.
> 
> Well that's not a bug with microcode then, is it...
> 
> I can't believe I'm having to say no to emitting messages at the wrong
> log level to work around a bug with selecting the default log level in
> the first place.

Hmm, I think not emitting info level messages is quite right. If I'm
not mistaken we emit various others at warning level to "account" for
this (you might instead call it "to work around this").

Furthermore, as to your suggestions here: What would you expect patch
4 to do? Emit yet another message, instead of having both conditions
fold to just one of them?

Jan