| Message ID | 20230808130331.27995-2-alejandro.vallejo@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Prevent attempting updates known to fail | expand |
On 08.08.2023 15:03, Alejandro Vallejo 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_WARNING "Microcode loading disabled due to: %s", > + "rev = ~0"); Sadly there now looks to be a newline missing. Easy to add while committing, I suppose, and with it added (back) Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan
On 08.08.2023 15:08, Jan Beulich wrote: > On 08.08.2023 15:03, Alejandro Vallejo 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_WARNING "Microcode loading disabled due to: %s", >> + "rev = ~0"); > > Sadly there now looks to be a newline missing. Easy to add while committing, > I suppose, and with it added (back) > Reviewed-by: Jan Beulich <jbeulich@suse.com> Actually, the 2nd string also is indented too deeply; unindenting will in particular help with line length in the 3rd patch (which isn't to say that the line's getting too long there, but it also doesn't need to grow as long as it does). Jan
Hi, On Tue, Aug 08, 2023 at 03:08:04PM +0200, Jan Beulich wrote: > On 08.08.2023 15:03, Alejandro Vallejo 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_WARNING "Microcode loading disabled due to: %s", > > + "rev = ~0"); > > Sadly there now looks to be a newline missing. Easy to add while committing, > I suppose, and with it added (back) > Reviewed-by: Jan Beulich <jbeulich@suse.com> > > Jan You mean the missing newline after "%s"? It fell under the cracks while adding the substring in v6. It should indeed be there, cheers. Thanks, Alejandro
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c index bec8b55db2..205bb52201 100644 --- 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_WARNING "Microcode loading disabled due to: %s", + "rev = ~0"); + ucode_ops.apply_microcode = NULL; + return -ENODEV; + } + + microcode_grab_module(module_map, mbi); + if ( ucode_mod.mod_end || ucode_blob.size ) rc = early_microcode_update_cpu();
Some hypervisors report ~0 as the microcode revision to mean "don't issue microcode updates". Ignore the microcode loading interface in that case. Signed-off-by: Alejandro Vallejo <alejandro.vallejo@cloud.com> --- v7: * Removed R-by tag. It wasn't meant to be there! * Fix wrong substring inserted in printk statement * Returned the printk severity to WARNING I still think both this new print and the previous one in the function should have severity INFO instead, but I'm not bothered either way and they should both have the same severity because they highlight equivalent events. --- xen/arch/x86/cpu/microcode/core.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)