[Xen-devel] [PATCH v10 05/16] microcode: remove struct ucode_cpu_info

[Chao Gao <chao.gao@xxxxxxxxx>]

Remove the per-cpu cache field in struct ucode_cpu_info since it has
been replaced by a global cache. It would leads to only one field
remaining in ucode_cpu_info. Then, this struct is removed and the
remaining field (cpu signature) is stored in per-cpu area.

The cpu status notifier is also removed. It was used to free the "mc"
field to avoid memory leak.

Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx>
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
Changes in v9:
 - rebase and fix conflict

Changes in v8:
 - split microcode_resume_cpu() cleanup to a separate patch.

Changes in v6:
 - remove the whole struct ucode_cpu_info instead of the per-cpu cache
  in it.
---
 xen/arch/x86/apic.c             |  2 +-
 xen/arch/x86/microcode.c        | 57 +++++++--------------------------------
 xen/arch/x86/microcode_amd.c    | 59 +++++++++--------------------------------
 xen/arch/x86/microcode_intel.c  | 28 +++++++------------
 xen/arch/x86/spec_ctrl.c        |  2 +-
 xen/include/asm-x86/microcode.h | 12 +--------
 6 files changed, 34 insertions(+), 126 deletions(-)

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index ea0d561..6cdb50c 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -1190,7 +1190,7 @@ static void __init check_deadline_errata(void)
     else
         rev = (unsigned long)m->driver_data;
 
-    if ( this_cpu(ucode_cpu_info).cpu_sig.rev >= rev )
+    if ( this_cpu(cpu_sig).rev >= rev )
         return;
 
     setup_clear_cpu_cap(X86_FEATURE_TSC_DEADLINE);
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 922b94f..d17dbec 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -187,7 +187,7 @@ const struct microcode_ops *microcode_ops;
 
 static DEFINE_SPINLOCK(microcode_mutex);
 
-DEFINE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
+DEFINE_PER_CPU(struct cpu_signature, cpu_sig);
 
 struct microcode_info {
     unsigned int cpu;
@@ -196,32 +196,17 @@ struct microcode_info {
     char buffer[1];
 };
 
-static void __microcode_fini_cpu(unsigned int cpu)
-{
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
-
-    xfree(uci->mc.mc_valid);
-    memset(uci, 0, sizeof(*uci));
-}
-
-static void microcode_fini_cpu(unsigned int cpu)
-{
-    spin_lock(&microcode_mutex);
-    __microcode_fini_cpu(cpu);
-    spin_unlock(&microcode_mutex);
-}
-
 int microcode_resume_cpu(unsigned int cpu)
 {
     int err;
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
 
     if ( !microcode_ops )
         return 0;
 
     spin_lock(&microcode_mutex);
 
-    err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
+    err = microcode_ops->collect_cpu_info(cpu, sig);
     if ( likely(!err) )
         err = microcode_ops->apply_microcode(cpu);
     spin_unlock(&microcode_mutex);
@@ -268,16 +253,13 @@ static int microcode_update_cpu(const void *buf, size_t 
size)
 {
     int err;
     unsigned int cpu = smp_processor_id();
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
 
     spin_lock(&microcode_mutex);
 
-    err = microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
+    err = microcode_ops->collect_cpu_info(cpu, sig);
     if ( likely(!err) )
         err = microcode_ops->cpu_request_microcode(cpu, buf, size);
-    else
-        __microcode_fini_cpu(cpu);
-
     spin_unlock(&microcode_mutex);
 
     return err;
@@ -364,29 +346,10 @@ static int __init microcode_init(void)
 }
 __initcall(microcode_init);
 
-static int microcode_percpu_callback(
-    struct notifier_block *nfb, unsigned long action, void *hcpu)
-{
-    unsigned int cpu = (unsigned long)hcpu;
-
-    switch ( action )
-    {
-    case CPU_DEAD:
-        microcode_fini_cpu(cpu);
-        break;
-    }
-
-    return NOTIFY_DONE;
-}
-
-static struct notifier_block microcode_percpu_nfb = {
-    .notifier_call = microcode_percpu_callback,
-};
-
 int __init early_microcode_update_cpu(bool start_update)
 {
     unsigned int cpu = smp_processor_id();
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     int rc = 0;
     void *data = NULL;
     size_t len;
@@ -405,7 +368,7 @@ int __init early_microcode_update_cpu(bool start_update)
         data = bootstrap_map(&ucode_mod);
     }
 
-    microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
+    microcode_ops->collect_cpu_info(cpu, sig);
 
     if ( data )
     {
@@ -424,7 +387,7 @@ int __init early_microcode_update_cpu(bool start_update)
 int __init early_microcode_init(void)
 {
     unsigned int cpu = smp_processor_id();
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     int rc;
 
     rc = microcode_init_intel();
@@ -437,12 +400,10 @@ int __init early_microcode_init(void)
 
     if ( microcode_ops )
     {
-        microcode_ops->collect_cpu_info(cpu, &uci->cpu_sig);
+        microcode_ops->collect_cpu_info(cpu, sig);
 
         if ( ucode_mod.mod_end || ucode_blob.size )
             rc = early_microcode_update_cpu(true);
-
-        register_cpu_notifier(&microcode_percpu_nfb);
     }
 
     return rc;
diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 04b00aa..69c9cfe 100644
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -155,7 +155,7 @@ static bool_t find_equiv_cpu_id(const struct 
equiv_cpu_entry *equiv_cpu_table,
 static enum microcode_match_result microcode_fits(
     const struct microcode_amd *mc_amd, unsigned int cpu)
 {
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     const struct microcode_header_amd *mc_header = mc_amd->mpb;
     const struct equiv_cpu_entry *equiv_cpu_table = mc_amd->equiv_cpu_table;
     unsigned int current_cpu_id;
@@ -178,14 +178,14 @@ static enum microcode_match_result microcode_fits(
         return MIS_UCODE;
     }
 
-    if ( mc_header->patch_id <= uci->cpu_sig.rev )
+    if ( mc_header->patch_id <= sig->rev )
     {
         pr_debug("microcode: patch is already at required level or 
greater.\n");
         return OLD_UCODE;
     }
 
     pr_debug("microcode: CPU%d found a matching microcode update with version 
%#x (current=%#x)\n",
-             cpu, mc_header->patch_id, uci->cpu_sig.rev);
+             cpu, mc_header->patch_id, sig->rev);
 
     return NEW_UCODE;
 }
@@ -259,9 +259,9 @@ static enum microcode_match_result compare_patch(
 static int apply_microcode(unsigned int cpu)
 {
     unsigned long flags;
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
     uint32_t rev;
     int hw_err;
+    struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     const struct microcode_header_amd *hdr;
     const struct microcode_patch *patch = microcode_get_cache();
 
@@ -300,9 +300,9 @@ static int apply_microcode(unsigned int cpu)
     }
 
     printk(KERN_WARNING "microcode: CPU%d updated from revision %#x to %#x\n",
-           cpu, uci->cpu_sig.rev, hdr->patch_id);
+           cpu, sig->rev, hdr->patch_id);
 
-    uci->cpu_sig.rev = rev;
+    sig->rev = rev;
 
     return 0;
 }
@@ -448,14 +448,14 @@ static bool_t check_final_patch_levels(unsigned int cpu)
      * any of the 'final_levels', then we should not update the microcode
      * patch on the cpu as system will hang otherwise.
      */
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     unsigned int i;
 
     if ( boot_cpu_data.x86 != 0x10 )
         return 0;
 
     for ( i = 0; i < ARRAY_SIZE(final_levels); i++ )
-        if ( uci->cpu_sig.rev == final_levels[i] )
+        if ( sig->rev == final_levels[i] )
             return 1;
 
     return 0;
@@ -464,13 +464,12 @@ static bool_t check_final_patch_levels(unsigned int cpu)
 static int cpu_request_microcode(unsigned int cpu, const void *buf,
                                  size_t bufsize)
 {
-    struct microcode_amd *mc_amd, *mc_old;
+    struct microcode_amd *mc_amd;
     size_t offset = 0;
-    size_t last_offset, applied_offset = 0;
-    int error = 0, save_error = 1;
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
+    int error = 0;
     unsigned int current_cpu_id;
     unsigned int equiv_cpu_id;
+    const struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
 
     /* We should bind the task to the CPU */
     BUG_ON(cpu != raw_smp_processor_id());
@@ -539,7 +538,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
         {
             printk(KERN_ERR "microcode: CPU%d incorrect or corrupt container 
file\n"
                    "microcode: Failed to update patch level. "
-                   "Current lvl:%#x\n", cpu, uci->cpu_sig.rev);
+                   "Current lvl:%#x\n", cpu, sig->rev);
             break;
         }
     }
@@ -551,15 +550,10 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
         goto out;
     }
 
-    mc_old = uci->mc.mc_amd;
-    /* implicitely validates uci->mc.mc_valid */
-    uci->mc.mc_amd = mc_amd;
-
     /*
      * It's possible the data file has multiple matching ucode,
      * lets keep searching till the latest version
      */
-    last_offset = offset;
     while ( (error = get_ucode_from_buffer_amd(mc_amd, buf, bufsize,
                                                &offset)) == 0 )
     {
@@ -582,11 +576,8 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
             error = apply_microcode(cpu);
             if ( error )
                 break;
-            applied_offset = last_offset;
         }
 
-        last_offset = offset;
-
         if ( offset >= bufsize )
             break;
 
@@ -614,31 +605,7 @@ static int cpu_request_microcode(unsigned int cpu, const 
void *buf,
              *(const uint32_t *)(buf + offset) == UCODE_MAGIC )
             break;
     }
-
-    /* On success keep the microcode patch for
-     * re-apply on resume.
-     */
-    if ( applied_offset )
-    {
-        save_error = get_ucode_from_buffer_amd(
-            mc_amd, buf, bufsize, &applied_offset);
-
-        if ( save_error )
-            error = save_error;
-    }
-
-    if ( save_error )
-    {
-        uci->mc.mc_amd = mc_old;
-        mc_old = mc_amd;
-    }
-
-    if ( mc_old )
-    {
-        xfree(mc_old->mpb);
-        xfree(mc_old->equiv_cpu_table);
-        xfree(mc_old);
-    }
+    free_patch(mc_amd);
 
   out:
 #if CONFIG_HVM
diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index 97f759e..f63e4bd 100644
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -231,10 +231,10 @@ static enum microcode_match_result microcode_update_match(
     const struct extended_sigtable *ext_header;
     const struct extended_signature *ext_sig;
     unsigned int i;
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
-    unsigned int sig = uci->cpu_sig.sig;
-    unsigned int pf = uci->cpu_sig.pf;
-    unsigned int rev = uci->cpu_sig.rev;
+    struct cpu_signature *cpu_sig = &per_cpu(cpu_sig, cpu);
+    unsigned int sig = cpu_sig->sig;
+    unsigned int pf = cpu_sig->pf;
+    unsigned int rev = cpu_sig->rev;
     unsigned long data_size = get_datasize(mc_header);
     const void *end = (const void *)mc_header + get_totalsize(mc_header);
 
@@ -297,7 +297,6 @@ static enum microcode_match_result compare_patch(
  */
 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 = xmalloc_bytes(total_size);
@@ -323,17 +322,8 @@ static int get_matching_microcode(const void *mc, unsigned 
int cpu)
 
     pr_debug("microcode: CPU%d found a matching microcode update with"
              " version %#x (current=%#x)\n",
-             cpu, mc_header->rev, uci->cpu_sig.rev);
-    new_mc = xmalloc_bytes(total_size);
-    if ( new_mc == NULL )
-    {
-        printk(KERN_ERR "microcode: error! Can not allocate memory\n");
-        return -ENOMEM;
-    }
+             cpu, mc_header->rev, per_cpu(cpu_sig, cpu).rev);
 
-    memcpy(new_mc, mc, total_size);
-    xfree(uci->mc.mc_intel);
-    uci->mc.mc_intel = new_mc;
     return 1;
 }
 
@@ -343,7 +333,7 @@ static int apply_microcode(unsigned int cpu)
     uint64_t msr_content;
     unsigned int val[2];
     unsigned int cpu_num = raw_smp_processor_id();
-    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num);
+    struct cpu_signature *sig = &per_cpu(cpu_sig, cpu);
     const struct microcode_intel *mc_intel;
     const struct microcode_patch *patch = microcode_get_cache();
 
@@ -377,14 +367,14 @@ static int apply_microcode(unsigned int cpu)
     {
         printk(KERN_ERR "microcode: CPU%d update from revision "
                "%#x to %#x failed. Resulting revision is %#x.\n", cpu_num,
-               uci->cpu_sig.rev, mc_intel->hdr.rev, val[1]);
+               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], mc_intel->hdr.year,
+           cpu_num, sig->rev, val[1], mc_intel->hdr.year,
            mc_intel->hdr.month, mc_intel->hdr.day);
-    uci->cpu_sig.rev = val[1];
+    sig->rev = val[1];
 
     return 0;
 }
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 468a847..4761be8 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -438,7 +438,7 @@ static bool __init check_smt_enabled(void)
 /* Calculate whether Retpoline is known-safe on this CPU. */
 static bool __init retpoline_safe(uint64_t caps)
 {
-    unsigned int ucode_rev = this_cpu(ucode_cpu_info).cpu_sig.rev;
+    unsigned int ucode_rev = this_cpu(cpu_sig).rev;
 
     if ( boot_cpu_data.x86_vendor & (X86_VENDOR_AMD | X86_VENDOR_HYGON) )
         return true;
diff --git a/xen/include/asm-x86/microcode.h b/xen/include/asm-x86/microcode.h
index da0b156..3f4c4be 100644
--- a/xen/include/asm-x86/microcode.h
+++ b/xen/include/asm-x86/microcode.h
@@ -10,7 +10,6 @@ enum microcode_match_result {
 };
 
 struct cpu_signature;
-struct ucode_cpu_info;
 
 struct microcode_patch {
     union {
@@ -38,16 +37,7 @@ struct cpu_signature {
     unsigned int rev;
 };
 
-struct ucode_cpu_info {
-    struct cpu_signature cpu_sig;
-    union {
-        struct microcode_intel *mc_intel;
-        struct microcode_amd *mc_amd;
-        void *mc_valid;
-    } mc;
-};
-
-DECLARE_PER_CPU(struct ucode_cpu_info, ucode_cpu_info);
+DECLARE_PER_CPU(struct cpu_signature, cpu_sig);
 extern const struct microcode_ops *microcode_ops;
 
 const struct microcode_patch *microcode_get_cache(void);
-- 
1.8.3.1


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