Re: [PATCH v6 1/3] x86/microcode: Ignore microcode loading interface for revision = -1

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On Tue, Jul 25, 2023 at 01:50:52PM +0100, Alejandro Vallejo wrote:
> On Tue, Jul 25, 2023 at 08:40:31AM +0200, Jan Beulich wrote:
> > > --- a/xen/arch/x86/cpu/microcode/core.c
> > > +++ b/xen/arch/x86/cpu/microcode/core.c
> > > @@ -867,10 +867,23 @@ 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_INFO "Microcode loading disabled due to: %s",
> > 
> [snip]
>
> > As it stands this message
> > will be invisible by default.
> Arguably, that's not necessarily a bad thing. The fact that microcode
> cannot be updated is expected behaviour and it makes little sense to warn
> about it. If only because they should already be aware of it through their
> agreement with their provider.
> 
> The case I can think of where a warning would be sensible is where the
> system has a microcode blob more recent than the currently installed
> revision. This is something the admin may want to be aware of in order to
> pester their provider for updates. In the common case the machine won't
> even need such an update, so sending unconditional warnings per boot seems
> unwarranted.

Actually, the previous message probably ought to be an INFO too. It's an
unconditional warning on old AMD and anything non AMD/Intel for no good
reason.

Alejandro