[Xen-devel] [PATCH v5 5/8] microcode: split out apply_microcode() from cpu_request_microcode()

[Chao Gao <chao.gao@xxxxxxxxx>]

During late microcode update, apply_microcode() is invoked in
cpu_request_microcode(). To make late microcode update more reliable,
we want to put the apply_microcode() into stop_machine context. So
we split out it from cpu_request_microcode(). As a consequence,
apply_microcode() should be invoked explicitly in the common code.

Also with the global ucode cache, microcode parsing only needs
to be done once; cpu_request_microcode() is also moved out of
microcode_update_cpu().

On AMD side, svm_host_osvw_init() is supposed to be called after
microcode update. As apply_micrcode() won't be called by
cpu_request_microcode() now, svm_host_osvw_init() is also moved to the
end of apply_microcode().

Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
---
 xen/arch/x86/microcode.c       | 80 +++++++++++++++++++++++++-----------------
 xen/arch/x86/microcode_amd.c   | 23 +++++++-----
 xen/arch/x86/microcode_intel.c | 20 ++---------
 3 files changed, 64 insertions(+), 59 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 0c77e90..936f0b8 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -254,47 +254,42 @@ struct microcode_patch *find_patch(unsigned int cpu)
     return NULL;
 }
 
-int microcode_resume_cpu(unsigned int cpu)
+/*
+ * Return the number of ucode patch inserted to the global cache.
+ * Return negtive value on error.
+ */
+static int parse_microcode_blob(const void *buffer, size_t len)
 {
-    int err;
+    unsigned int cpu = smp_processor_id();
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
-
-    if ( !microcode_ops )
-        return 0;
+    int ret;
 
     spin_lock(&microcode_mutex);
-
-    err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
-    if ( err )
-    {
+    ret = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
+    if ( likely(!ret) )
+        ret = microcode_ops->cpu_request_microcode(cpu, buffer, len);
+    else
         microcode_fini_cpu(cpu);
-        spin_unlock(&microcode_mutex);
-        return err;
-    }
-
-    err = microcode_ops->apply_microcode(cpu);
     spin_unlock(&microcode_mutex);
 
-    return err;
+    return ret;
 }
 
-static int microcode_update_cpu(const void *buf, size_t size)
+static int microcode_update_cpu(void)
 {
-    int err;
-    unsigned int cpu = smp_processor_id();
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    int ret;
 
     spin_lock(&microcode_mutex);
-
-    err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
-    if ( likely(!err) )
-        err = microcode_ops->cpu_request_microcode(cpu, buf, size);
-    else
-        microcode_fini_cpu(cpu);
-
+    ret = microcode_ops->apply_microcode(smp_processor_id());
     spin_unlock(&microcode_mutex);
 
-    return err;
+    return ret;
+}
+
+int microcode_resume_cpu(unsigned int cpu)
+{
+    BUG_ON(cpu != smp_processor_id());
+    return microcode_ops ? microcode_update_cpu() : 0;
 }
 
 static long do_microcode_update(void *_info)
@@ -304,7 +299,7 @@ static long do_microcode_update(void *_info)
 
     BUG_ON(info->cpu != smp_processor_id());
 
-    error = microcode_update_cpu(info->buffer, info->buffer_size);
+    error = microcode_update_cpu();
     if ( error )
         info->error = error;
 
@@ -339,10 +334,6 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
         return ret;
     }
 
-    info->buffer_size = len;
-    info->error = 0;
-    info->cpu = cpumask_first(&cpu_online_map);
-
     if ( microcode_ops->start_update )
     {
         ret = microcode_ops->start_update();
@@ -353,6 +344,18 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) 
buf, unsigned long len)
         }
     }
 
+    ret = parse_microcode_blob(info->buffer, len);
+    if ( ret <= 0 )
+    {
+        printk(XENLOG_ERR "No valid or newer ucode found. Update abort!\n");
+        xfree(info);
+        return -EINVAL;
+    }
+
+    info->buffer_size = len;
+    info->error = 0;
+    info->cpu = cpumask_first(&cpu_online_map);
+
     return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info);
 }
 
@@ -396,13 +399,24 @@ int __init early_microcode_update_cpu(bool start_update)
     }
     if ( data )
     {
+        static bool parsed = false;
+
         if ( start_update && microcode_ops->start_update )
             rc = microcode_ops->start_update();
 
         if ( rc )
             return rc;
 
-        return microcode_update_cpu(data, len);
+        if ( !parsed )
+        {
+            rc = parse_microcode_blob(data, len);
+            parsed = true;
+
+            if ( rc <= 0 )
+                return -EINVAL;
+        }
+
+        return microcode_update_cpu();
     }
     else
         return -ENOMEM;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index d86a596..80e274e 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -297,6 +297,10 @@ static int apply_microcode(unsigned int cpu)
 
     uci->cpu_sig.rev = rev;
 
+#if CONFIG_HVM
+    svm_host_osvw_init();
+#endif
+
     return 0;
 }
 
@@ -463,6 +467,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
     unsigned int current_cpu_id;
     unsigned int equiv_cpu_id;
+    unsigned int matched_cnt = 0;
 
     /* We should bind the task to the CPU */
     BUG_ON(cpu != raw_smp_processor_id());
@@ -564,11 +569,15 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
          * this ucode patch before checking whether it matches with
          * current CPU.
          */
-        if ( save_patch(microcode_patch) && microcode_fits(mc_amd, cpu) )
+        if ( save_patch(microcode_patch) )
         {
-            error = apply_microcode(cpu);
-            if ( error )
-                break;
+            matched_cnt++;
+            if ( microcode_fits(mc_amd, cpu) )
+            {
+                error = apply_microcode(cpu);
+                if ( error )
+                    break;
+            }
         }
 
         if ( offset >= bufsize )
@@ -600,17 +609,13 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
     }
 
   out:
-#if CONFIG_HVM
-    svm_host_osvw_init();
-#endif
-
     /*
      * In some cases we may return an error even if processor's microcode has
      * been updated. For example, the first patch in a container file is loaded
      * successfully but subsequent container file processing encounters a
      * failure.
      */
-    return error;
+    return error ?: matched_cnt;
 }
 
 static int start_update(void)
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index d227f8a..02fc5a0 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -273,7 +273,6 @@ static enum microcode_match_result replace_patch(struct 
microcode_patch *new,
  */
 static int get_matching_microcode(const void *mc, unsigned int cpu)
 {
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
     const struct microcode_header_intel *mc_header = mc;
     unsigned long total_size = get_totalsize(mc_header);
     struct microcode_patch *microcode_patch = xmalloc(struct microcode_patch);
@@ -291,19 +290,9 @@ static int get_matching_microcode(const void *mc, unsigned 
int cpu)
 
     /*
      * In order to support a system with mixed stepping CPUs, save this ucode
-     * patch before checking whether it matches with current CPU.
+     * patch without checking whether it matches with current CPU.
      */
-    if ( !save_patch(microcode_patch) )
-        return 0;
-
-    if ( microcode_update_match(mc, uci->cpu_sig.sig, uci->cpu_sig.pf,
-                                uci->cpu_sig.rev) != NEW_UCODE )
-        return 0;
-
-    pr_debug("microcode: CPU%d found a matching microcode update with"
-             " version %#x (current=%#x)\n",
-             cpu, mc_header->rev, uci->cpu_sig.rev);
-    return 1;
+    return save_patch(microcode_patch);
 }
 
 static int apply_microcode(unsigned int cpu)
@@ -422,10 +411,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
     if ( offset < 0 )
         error = offset;
 
-    if ( !error && matching_count )
-        error = apply_microcode(cpu);
-
-    return error;
+    return error ?: matching_count;
 }
 
 static const struct microcode_ops microcode_intel_ops = {
-- 
1.8.3.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel