x86/ucode: Remove unnecessary indirection in struct microcode_patch

Currently, each cpu_request_microcode() allocates a struct microcode_patch,
which is a single pointer to a separate allocated structure.  This is
wasteful.

Fixing this is complicated because the common microcode_free_patch() code is
responsible for freeing struct microcode_patch, despite this being asymmetric
with how it is allocated.

Make struct microcode_patch fully opaque to the common logic.  This involves
moving the responsibility for freeing struct microcode_patch fully into the
free_patch() hook.

In each vendor logic, use some temporary ifdef-ary (cleaned up in subsequent
changes) to reduce the churn as much as possible, and forgo allocating the
intermediate pointer in cpu_request_microcode().

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

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 9efc03c810816efaedc7ed9af547024c4450bb6d..6bf3a054d3883d4aa52132004b1999b3f34a6d6a 100644 (file)
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -58,13 +58,16 @@ struct __packed microcode_header_amd {
 #define UCODE_EQUIV_CPU_TABLE_TYPE 0x00000000
 #define UCODE_UCODE_TYPE           0x00000001
 
-struct microcode_amd {
+struct microcode_patch {
     void *mpb;
     size_t mpb_size;
     struct equiv_cpu_entry *equiv_cpu_table;
     size_t equiv_cpu_table_size;
 };
 
+/* Temporary, until the microcode_* structure are disentangled. */
+#define microcode_amd microcode_patch
+
 struct mpbhdr {
     uint32_t type;
     uint32_t len;
@@ -175,13 +178,11 @@ static enum microcode_match_result microcode_fits(
 
 static bool match_cpu(const struct microcode_patch *patch)
 {
-    return patch && (microcode_fits(patch->mc_amd) == NEW_UCODE);
+    return patch && (microcode_fits(patch) == NEW_UCODE);
 }
 
-static void free_patch(void *mc)
+static void free_patch(struct microcode_patch *mc_amd)
 {
-    struct microcode_amd *mc_amd = mc;
-
     if ( mc_amd )
     {
         xfree(mc_amd->equiv_cpu_table);
@@ -204,12 +205,12 @@ static enum microcode_match_result compare_header(
 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;
+    const struct microcode_header_amd *new_header = new->mpb;
+    const struct microcode_header_amd *old_header = old->mpb;
 
     /* Both patches to compare are supposed to be applicable to local CPU. */
-    ASSERT(microcode_fits(new->mc_amd) != MIS_UCODE);
-    ASSERT(microcode_fits(old->mc_amd) != MIS_UCODE);
+    ASSERT(microcode_fits(new) != MIS_UCODE);
+    ASSERT(microcode_fits(old) != MIS_UCODE);
 
     return compare_header(new_header, old_header);
 }
@@ -228,7 +229,7 @@ static int apply_microcode(const struct microcode_patch *patch)
     if ( !match_cpu(patch) )
         return -EINVAL;
 
-    hdr = patch->mc_amd->mpb;
+    hdr = patch->mpb;
 
     hw_err = wrmsr_safe(MSR_AMD_PATCHLOADER, (unsigned long)hdr);
 
@@ -553,14 +554,7 @@ static struct microcode_patch *cpu_request_microcode(const void *buf,
     {
         mc_amd->mpb = saved;
         mc_amd->mpb_size = saved_size;
-        patch = xmalloc(struct microcode_patch);
-        if ( patch )
-            patch->mc_amd = mc_amd;
-        else
-        {
-            free_patch(mc_amd);
-            error = -ENOMEM;
-        }
+        patch = mc_amd;
     }
     else
         free_patch(mc_amd);

diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 61150e04c8216d3c29fd6bff7926c2ae654ab46c..b3e5913d49f3dabee5a0cf9e8bb07f1c52daa7b2 100644 (file)
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -245,8 +245,7 @@ static struct microcode_patch *parse_blob(const char *buf, size_t len)
 
 static void microcode_free_patch(struct microcode_patch *microcode_patch)
 {
-    microcode_ops->free_patch(microcode_patch->mc);
-    xfree(microcode_patch);
+    microcode_ops->free_patch(microcode_patch);
 }
 
 /* Return true if cache gets updated. Otherwise, return false */

diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 49c46cd146d2871f22a3cdfd9e5b4dfaeaa41ec6..9e23f2a3c7f57f1135579f46dcf1c903ce25ed6f 100644 (file)
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -50,11 +50,14 @@ struct microcode_header_intel {
     unsigned int reserved[3];
 };
 
-struct microcode_intel {
+struct microcode_patch {
     struct microcode_header_intel hdr;
     unsigned int bits[0];
 };
 
+/* Temporary, until the microcode_* structure are disentangled. */
+#define microcode_intel microcode_patch
+
 /* microcode format is extended from prescott processors */
 struct extended_signature {
     unsigned int sig;
@@ -243,12 +246,12 @@ static bool match_cpu(const struct microcode_patch *patch)
     if ( !patch )
         return false;
 
-    return microcode_update_match(&patch->mc_intel->hdr) == NEW_UCODE;
+    return microcode_update_match(&patch->hdr) == NEW_UCODE;
 }
 
-static void free_patch(void *mc)
+static void free_patch(struct microcode_patch *patch)
 {
-    xfree(mc);
+    xfree(patch);
 }
 
 static enum microcode_match_result compare_patch(
@@ -258,11 +261,10 @@ static enum microcode_match_result compare_patch(
      * 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) != MIS_UCODE);
-    ASSERT(microcode_update_match(&new->mc_intel->hdr) != MIS_UCODE);
+    ASSERT(microcode_update_match(&old->hdr) != MIS_UCODE);
+    ASSERT(microcode_update_match(&new->hdr) != MIS_UCODE);
 
-    return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ? NEW_UCODE
-                                                             : OLD_UCODE;
+    return (new->hdr.rev > old->hdr.rev) ? NEW_UCODE : OLD_UCODE;
 }
 
 static int apply_microcode(const struct microcode_patch *patch)
@@ -279,7 +281,7 @@ static int apply_microcode(const struct microcode_patch *patch)
     if ( !match_cpu(patch) )
         return -EINVAL;
 
-    mc_intel = patch->mc_intel;
+    mc_intel = patch;
 
     /* write microcode via MSR 0x79 */
     wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
@@ -368,16 +370,7 @@ static struct microcode_patch *cpu_request_microcode(const void *buf,
         error = offset;
 
     if ( saved )
-    {
-        patch = xmalloc(struct microcode_patch);
-        if ( patch )
-            patch->mc_intel = saved;
-        else
-        {
-            xfree(saved);
-            error = -ENOMEM;
-        }
-    }
+        patch = saved;
 
     if ( error && !patch )
         patch = ERR_PTR(error);

diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index 230b935c947f0671120dc4ce19889fefd726b53a..df0d0852cdedb34a323b2e11e8e83f54ed4517ea 100644 (file)
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -11,13 +11,8 @@ enum microcode_match_result {
     MIS_UCODE, /* signature mismatched */
 };
 
-struct microcode_patch {
-    union {
-        struct microcode_intel *mc_intel;
-        struct microcode_amd *mc_amd;
-        void *mc;
-    };
-};
+/* Opaque.  Internals are vendor-specific. */
+struct microcode_patch;
 
 struct microcode_ops {
     /*
@@ -62,7 +57,7 @@ struct microcode_ops {
     void (*end_update_percpu)(void);
 
     /* Free a patch previously allocated by cpu_request_microcode(). */
-    void (*free_patch)(void *mc);
+    void (*free_patch)(struct microcode_patch *patch);
 
     /*
      * Is the microcode patch applicable for the current CPU, and newer than