[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen staging] microcode: introduce a global cache of ucode patch



commit 9da23943ccd557ddb6c34e96e3bc078a51ad8205
Author:     Chao Gao <chao.gao@xxxxxxxxx>
AuthorDate: Fri Sep 13 12:27:42 2019 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Fri Sep 13 12:27:42 2019 +0200

    microcode: introduce a global cache of ucode patch
    
    to replace the current per-cpu cache 'uci->mc'.
    
    With the assumption that all CPUs in the system have the same signature
    (family, model, stepping and 'pf'), one microcode update matches with
    one cpu should match with others. Having differing microcode revisions
    on cpus would cause system unstable and should be avoided. Hence, caching
    one microcode update is good enough for all cases.
    
    Introduce a global variable, microcode_cache, to store the newest
    matching microcode update. Whenever we get a new valid microcode update,
    its revision id is compared against that of the microcode update to
    determine whether the "microcode_cache" needs to be replaced. And
    this global cache is loaded to cpu in apply_microcode().
    
    All operations on the cache is protected by 'microcode_mutex'.
    
    Note that I deliberately avoid touching the old per-cpu cache ('uci->mc')
    as I am going to remove it completely in the following patches. We copy
    everything to create the new cache blob to avoid reusing some buffers
    previously allocated for the old per-cpu cache. It is not so efficient,
    but it is already corrected by a patch later in this series.
    
    Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
    Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/microcode.c        | 38 ++++++++++++++++
 xen/arch/x86/microcode_amd.c    | 98 ++++++++++++++++++++++++++++++++++++++---
 xen/arch/x86/microcode_intel.c  | 81 +++++++++++++++++++++++++++-------
 xen/include/asm-x86/microcode.h | 16 +++++++
 4 files changed, 211 insertions(+), 22 deletions(-)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 421d57e925..e218a9ddac 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -61,6 +61,9 @@ static struct ucode_mod_blob __initdata ucode_blob;
  */
 static bool_t __initdata ucode_scan;
 
+/* Protected by microcode_mutex */
+static struct microcode_patch *microcode_cache;
+
 void __init microcode_set_module(unsigned int idx)
 {
     ucode_mod_idx = idx;
@@ -262,6 +265,41 @@ int microcode_resume_cpu(unsigned int cpu)
     return err;
 }
 
+void microcode_free_patch(struct microcode_patch *microcode_patch)
+{
+    microcode_ops->free_patch(microcode_patch->mc);
+    xfree(microcode_patch);
+}
+
+const struct microcode_patch *microcode_get_cache(void)
+{
+    ASSERT(spin_is_locked(&microcode_mutex));
+
+    return microcode_cache;
+}
+
+/* Return true if cache gets updated. Otherwise, return false */
+bool microcode_update_cache(struct microcode_patch *patch)
+{
+    ASSERT(spin_is_locked(&microcode_mutex));
+
+    if ( !microcode_cache )
+        microcode_cache = patch;
+    else if ( microcode_ops->compare_patch(patch,
+                                           microcode_cache) == NEW_UCODE )
+    {
+        microcode_free_patch(microcode_cache);
+        microcode_cache = patch;
+    }
+    else
+    {
+        microcode_free_patch(patch);
+        return false;
+    }
+
+    return true;
+}
+
 static int microcode_update_cpu(const void *buf, size_t size)
 {
     int err;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 7fa700b15c..2dca1df2c2 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -190,25 +190,92 @@ static enum microcode_match_result microcode_fits(
     return NEW_UCODE;
 }
 
+static bool match_cpu(const struct microcode_patch *patch)
+{
+    if ( !patch )
+        return false;
+    return microcode_fits(patch->mc_amd, smp_processor_id()) == NEW_UCODE;
+}
+
+static struct microcode_patch *alloc_microcode_patch(
+    const struct microcode_amd *mc_amd)
+{
+    struct microcode_patch *microcode_patch = xmalloc(struct microcode_patch);
+    struct microcode_amd *cache = xmalloc(struct microcode_amd);
+    void *mpb = xmalloc_bytes(mc_amd->mpb_size);
+    struct equiv_cpu_entry *equiv_cpu_table =
+                                xmalloc_bytes(mc_amd->equiv_cpu_table_size);
+
+    if ( !microcode_patch || !cache || !mpb || !equiv_cpu_table )
+    {
+        xfree(microcode_patch);
+        xfree(cache);
+        xfree(mpb);
+        xfree(equiv_cpu_table);
+        return ERR_PTR(-ENOMEM);
+    }
+
+    memcpy(mpb, mc_amd->mpb, mc_amd->mpb_size);
+    cache->mpb = mpb;
+    cache->mpb_size = mc_amd->mpb_size;
+    memcpy(equiv_cpu_table, mc_amd->equiv_cpu_table,
+           mc_amd->equiv_cpu_table_size);
+    cache->equiv_cpu_table = equiv_cpu_table;
+    cache->equiv_cpu_table_size = mc_amd->equiv_cpu_table_size;
+    microcode_patch->mc_amd = cache;
+
+    return microcode_patch;
+}
+
+static void free_patch(void *mc)
+{
+    struct microcode_amd *mc_amd = mc;
+
+    if ( mc_amd )
+    {
+        xfree(mc_amd->equiv_cpu_table);
+        xfree(mc_amd->mpb);
+        xfree(mc_amd);
+    }
+}
+
+static enum microcode_match_result compare_patch(
+    const struct microcode_patch *new, const struct microcode_patch *old)
+{
+    const struct microcode_header_amd *new_header = new->mc_amd->mpb;
+    const struct microcode_header_amd *old_header = old->mc_amd->mpb;
+
+    /* Both patches to compare are supposed to be applicable to local CPU. */
+    ASSERT(microcode_fits(new->mc_amd, smp_processor_id()) != MIS_UCODE);
+    ASSERT(microcode_fits(new->mc_amd, smp_processor_id()) != MIS_UCODE);
+
+    if ( new_header->processor_rev_id == old_header->processor_rev_id )
+        return (new_header->patch_id > old_header->patch_id) ?
+                NEW_UCODE : OLD_UCODE;
+
+    return MIS_UCODE;
+}
+
 static int apply_microcode(unsigned int cpu)
 {
     unsigned long flags;
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
     uint32_t rev;
-    struct microcode_amd *mc_amd = uci->mc.mc_amd;
-    struct microcode_header_amd *hdr;
     int hw_err;
+    const struct microcode_header_amd *hdr;
+    const struct microcode_patch *patch = microcode_get_cache();
 
     /* We should bind the task to the CPU */
     BUG_ON(raw_smp_processor_id() != cpu);
 
-    if ( mc_amd == NULL )
-        return -EINVAL;
+    if ( !patch )
+        return -ENOENT;
 
-    hdr = mc_amd->mpb;
-    if ( hdr == NULL )
+    if ( !match_cpu(patch) )
         return -EINVAL;
 
+    hdr = patch->mc_amd->mpb;
+
     spin_lock_irqsave(&microcode_update_lock, flags);
 
     hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
@@ -496,7 +563,21 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
     while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize,
                                                &offset)) == 0 )
     {
-        if ( microcode_fits(mc_amd, cpu) == NEW_UCODE )
+        struct microcode_patch *new_patch = alloc_microcode_patch(mc_amd);
+
+        if ( IS_ERR(new_patch) )
+        {
+            error = PTR_ERR(new_patch);
+            break;
+        }
+
+        /* Update cache if this patch covers current CPU */
+        if ( microcode_fits(new_patch->mc_amd, cpu) != MIS_UCODE )
+            microcode_update_cache(new_patch);
+        else
+            microcode_free_patch(new_patch);
+
+        if ( match_cpu(microcode_get_cache()) )
         {
             error = apply_microcode(cpu);
             if ( error )
@@ -643,6 +724,9 @@ static const struct microcode_ops microcode_amd_ops = {
     .collect_cpu_info                 = collect_cpu_info,
     .apply_microcode                  = apply_microcode,
     .start_update                     = start_update,
+    .free_patch                       = free_patch,
+    .compare_patch                    = compare_patch,
+    .match_cpu                        = match_cpu,
 };
 
 int __init microcode_init_amd(void)
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 6547e35d9e..cddcfa902f 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -259,6 +259,36 @@ static enum microcode_match_result microcode_update_match(
     return MIS_UCODE;
 }
 
+static bool match_cpu(const struct microcode_patch *patch)
+{
+    if ( !patch )
+        return false;
+
+    return microcode_update_match(&patch->mc_intel->hdr,
+                                  smp_processor_id()) == NEW_UCODE;
+}
+
+static void free_patch(void *mc)
+{
+    xfree(mc);
+}
+
+static enum microcode_match_result compare_patch(
+    const struct microcode_patch *new, const struct microcode_patch *old)
+{
+    /*
+     * Both patches to compare are supposed to be applicable to local CPU.
+     * Just compare the revision number.
+     */
+    ASSERT(microcode_update_match(&old->mc_intel->hdr, smp_processor_id()) !=
+           MIS_UCODE);
+    ASSERT(microcode_update_match(&new->mc_intel->hdr, smp_processor_id()) !=
+           MIS_UCODE);
+
+    return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ? NEW_UCODE
+                                                             : OLD_UCODE;
+}
+
 /*
  * return 0 - no update found
  * return 1 - found update
@@ -269,10 +299,26 @@ 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);
-    void *new_mc;
+    void *new_mc = xmalloc_bytes(total_size);
+    struct microcode_patch *new_patch = xmalloc(struct microcode_patch);
 
-    if ( microcode_update_match(mc, cpu) != NEW_UCODE )
+    if ( !new_patch || !new_mc )
+    {
+        xfree(new_patch);
+        xfree(new_mc);
+        return -ENOMEM;
+    }
+    memcpy(new_mc, mc, total_size);
+    new_patch->mc_intel = new_mc;
+
+    /* Make sure that this patch covers current CPU */
+    if ( microcode_update_match(mc, cpu) == MIS_UCODE )
+    {
+        microcode_free_patch(new_patch);
         return 0;
+    }
+
+    microcode_update_cache(new_patch);
 
     pr_debug("microcode: CPU%d found a matching microcode update with"
              " version %#x (current=%#x)\n",
@@ -297,18 +343,25 @@ static int apply_microcode(unsigned int cpu)
     unsigned int val[2];
     unsigned int cpu_num = raw_smp_processor_id();
     struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num);
+    const struct microcode_intel *mc_intel;
+    const struct microcode_patch *patch = microcode_get_cache();
 
     /* We should bind the task to the CPU */
     BUG_ON(cpu_num != cpu);
 
-    if ( uci->mc.mc_intel == NULL )
+    if ( !patch )
+        return -ENOENT;
+
+    if ( !match_cpu(patch) )
         return -EINVAL;
 
+    mc_intel = patch->mc_intel;
+
     /* serialize access to the physical write to MSR 0x79 */
     spin_lock_irqsave(&microcode_update_lock, flags);
 
     /* write microcode via MSR 0x79 */
-    wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)uci->mc.mc_intel->bits);
+    wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
     wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);
 
     /* As documented in the SDM: Do a CPUID 1 here */
@@ -319,19 +372,17 @@ static int apply_microcode(unsigned int cpu)
     val[1] = (uint32_t)(msr_content >> 32);
 
     spin_unlock_irqrestore(&microcode_update_lock, flags);
-    if ( val[1] != uci->mc.mc_intel->hdr.rev )
+    if ( val[1] != mc_intel->hdr.rev )
     {
         printk(KERN_ERR "microcode: CPU%d update from revision "
                "%#x to %#x failed. Resulting revision is %#x.\n", cpu_num,
-               uci->cpu_sig.rev, uci->mc.mc_intel->hdr.rev, val[1]);
+               uci->cpu_sig.rev, mc_intel->hdr.rev, val[1]);
         return -EIO;
     }
     printk(KERN_INFO "microcode: CPU%d updated from revision "
            "%#x to %#x, date = %04x-%02x-%02x \n",
-           cpu_num, uci->cpu_sig.rev, val[1],
-           uci->mc.mc_intel->hdr.year,
-           uci->mc.mc_intel->hdr.month,
-           uci->mc.mc_intel->hdr.day);
+           cpu_num, uci->cpu_sig.rev, val[1], mc_intel->hdr.year,
+           mc_intel->hdr.month, mc_intel->hdr.day);
     uci->cpu_sig.rev = val[1];
 
     return 0;
@@ -371,7 +422,6 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
     long offset = 0;
     int error = 0;
     void *mc;
-    unsigned int matching_count = 0;
 
     /* We should bind the task to the CPU */
     BUG_ON(cpu != raw_smp_processor_id());
@@ -389,10 +439,8 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
          * lets keep searching till the latest version
          */
         if ( error == 1 )
-        {
-            matching_count++;
             error = 0;
-        }
+
         xfree(mc);
     }
     if ( offset > 0 )
@@ -400,7 +448,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
     if ( offset < 0 )
         error = offset;
 
-    if ( !error && matching_count )
+    if ( !error && match_cpu(microcode_get_cache()) )
         error = apply_microcode(cpu);
 
     return error;
@@ -416,6 +464,9 @@ static const struct microcode_ops microcode_intel_ops = {
     .cpu_request_microcode            = cpu_request_microcode,
     .collect_cpu_info                 = collect_cpu_info,
     .apply_microcode                  = apply_microcode,
+    .free_patch                       = free_patch,
+    .compare_patch                    = compare_patch,
+    .match_cpu                        = match_cpu,
 };
 
 int __init microcode_init_intel(void)
diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h
index 882f560694..4d45401448 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/include/asm-x86/microcode.h
@@ -12,6 +12,14 @@ enum microcode_match_result {
 struct cpu_signature;
 struct ucode_cpu_info;
 
+struct microcode_patch {
+    union {
+        struct microcode_intel *mc_intel;
+        struct microcode_amd *mc_amd;
+        void *mc;
+    };
+};
+
 struct microcode_ops {
     int (*microcode_resume_match)(unsigned int cpu, const void *mc);
     int (*cpu_request_microcode)(unsigned int cpu, const void *buf,
@@ -19,6 +27,10 @@ struct microcode_ops {
     int (*collect_cpu_info)(unsigned int cpu, struct cpu_signature *csig);
     int (*apply_microcode)(unsigned int cpu);
     int (*start_update)(void);
+    void (*free_patch)(void *mc);
+    bool (*match_cpu)(const struct microcode_patch *patch);
+    enum microcode_match_result (*compare_patch)(
+        const struct microcode_patch *new, const struct microcode_patch *old);
 };
 
 struct cpu_signature {
@@ -39,4 +51,8 @@ struct ucode_cpu_info {
 DECLARE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
 extern const struct microcode_ops *microcode_ops;
 
+const struct microcode_patch *microcode_get_cache(void);
+bool microcode_update_cache(struct microcode_patch *patch);
+void microcode_free_patch(struct microcode_patch *patch);
+
 #endif /* ASM_X86__MICROCODE_H */
--
generated by git-patchbot for /home/xen/git/xen.git#staging

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.