diff mbox series

[v2,3/4] x86/microcode: Ignore microcode loading interface for revision = -1

Message ID 20230605170817.9913-4-alejandro.vallejo@cloud.com (mailing list archive)
State Superseded
Headers show
Series Prevent attempting updates known to fail | expand

Commit Message

Alejandro Vallejo June 5, 2023, 5:08 p.m. UTC
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>
---
v2:
  * New addition
---
 xen/arch/x86/cpu/microcode/core.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

Comments

Andrew Cooper June 12, 2023, 6:36 p.m. UTC | #1
On 05/06/2023 6:08 pm, Alejandro Vallejo wrote:
> diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
> index 892bcec901..4f60d96d98 100644
> --- a/xen/arch/x86/cpu/microcode/core.c
> +++ b/xen/arch/x86/cpu/microcode/core.c
> @@ -874,6 +874,21 @@ int __init early_microcode_init(unsigned long *module_map,
>          break;
>      }
>  
> +    if ( ucode_ops.collect_cpu_info )
> +        ucode_ops.collect_cpu_info();
> +
> +    /*
> +     * This is a special case for virtualized Xen.

I'm not sure this first sentence is useful.  I'd just start with "Some
hypervisors ..."

>  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 )
> +    {
> +        this_cpu(cpu_sig) = (struct cpu_signature){ 0 };
> +        ucode_ops = (struct microcode_ops){ 0 };

I think we want to retain XENPF_get_ucode_revision's ability to see this ~0.

As with the following patch, we want to retain the ability to query, so
leave cpu_sig alone and only remove the apply_microcode hook.  In turn,
that probably means this wants to be an else if in the next clause down.

Moving it down also means you can drop the check for collect_cpu_info,
because it's a mandatory hook if ucode_ops was filled in.

~Andrew

> +    }
> +
>      if ( !ucode_ops.apply_microcode )
>      {
>          printk(XENLOG_WARNING "Microcode loading not available\n");
diff mbox series

Patch

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 892bcec901..4f60d96d98 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -874,6 +874,21 @@  int __init early_microcode_init(unsigned long *module_map,
         break;
     }
 
+    if ( ucode_ops.collect_cpu_info )
+        ucode_ops.collect_cpu_info();
+
+    /*
+     * This is a special case for virtualized Xen. 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 )
+    {
+        this_cpu(cpu_sig) = (struct cpu_signature){ 0 };
+        ucode_ops = (struct microcode_ops){ 0 };
+    }
+
     if ( !ucode_ops.apply_microcode )
     {
         printk(XENLOG_WARNING "Microcode loading not available\n");
@@ -882,8 +897,6 @@  int __init early_microcode_init(unsigned long *module_map,
 
     microcode_grab_module(module_map, mbi);
 
-    ucode_ops.collect_cpu_info();
-
     if ( ucode_mod.mod_end || ucode_blob.size )
         rc = early_microcode_update_cpu();