x86/ucode: Fold microcode_update_cpu() and fix error handling

Fold microcode_update_cpu() into its single remaining caller.  Explain why we
bother grabbing the microcode revision even if we can't load microcode.

This avoids a double collect_cpu_info() call on each AP.

Furthermore, delete the -EIO path.

A hard error updating a single CPU's microcode on AP bringup or S3 resume is
definitely bad (needing special investigation), but freeing the cache is about
the worst possible action we can take in response; it prevents subsequent APs
from taking an update they might have accepted.

While we expect a homogeneous system with respect to microcode applicability,
this doesn't mean that all cores behave identically when given the same blob.
e.g. one failure mode seen in practice is a checksum failure caused by a bad
SRAM cell.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 81a772cb2b7863b264a9b7b3c24d3d380931bb4d..fd4b08b4538878554d13ab933acab105dfc637b5 100644 (file)
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -263,38 +263,6 @@ static bool cf_check wait_cpu_callout(unsigned int nr)
     return atomic_read(&cpu_out) >= nr;
 }
 
-/*
- * Load a microcode update to current CPU.
- *
- * If no patch is provided, the cached patch will be loaded. Microcode update
- * during APs bringup and CPU resuming falls into this case.
- */
-static int microcode_update_cpu(const struct microcode_patch *patch,
-                                unsigned int flags)
-{
-    int err;
-
-    alternative_vcall(ucode_ops.collect_cpu_info);
-
-    spin_lock(&microcode_mutex);
-    if ( microcode_cache )
-    {
-        err = alternative_call(ucode_ops.apply_microcode, microcode_cache,
-                               flags);
-        if ( err == -EIO )
-        {
-            microcode_free_patch(microcode_cache);
-            microcode_cache = NULL;
-        }
-    }
-    else
-        /* No patch to update */
-        err = -ENOENT;
-    spin_unlock(&microcode_mutex);
-
-    return err;
-}
-
 static bool wait_for_state(typeof(loading_state) state)
 {
     typeof(loading_state) cur_state;
@@ -700,13 +668,26 @@ int microcode_update(XEN_GUEST_HANDLE(const_void) buf,
 /* Load a cached update to current cpu */
 int microcode_update_one(void)
 {
+    int rc;
+
+    /*
+     * This path is used for APs and S3 resume.  Read the microcode revision
+     * if possible, even if we can't load microcode.
+     */
     if ( ucode_ops.collect_cpu_info )
         alternative_vcall(ucode_ops.collect_cpu_info);
 
     if ( !ucode_ops.apply_microcode )
         return -EOPNOTSUPP;
 
-    return microcode_update_cpu(NULL, 0);
+    spin_lock(&microcode_mutex);
+    if ( microcode_cache )
+        rc = alternative_call(ucode_ops.apply_microcode, microcode_cache, 0);
+    else
+        rc = -ENOENT;
+    spin_unlock(&microcode_mutex);
+
+    return rc;
 }
 
 /*