[Xen-devel] [PATCH 7/7] x86/ucode/intel: Fold structures together

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [Xen-devel] [PATCH 7/7] x86/ucode/intel: Fold structures together
Date: Mon, 23 Mar 2020 10:17:24 +0000	[thread overview]
Message-ID: <20200323101724.15655-8-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20200323101724.15655-1-andrew.cooper3@citrix.com>

Currently, we allocate an 8 byte struct microcode_patch to point at a
separately allocated struct microcode_intel.  This is wasteful.

Fold struct microcode_header_intel and microcode_intel into microcode_patch to
simplify the code and remove a level of indirection.

The two semantic changes are in free_patch() and cpu_request_microcode() which
deal with the memory allocation aspects.  Everything else is no functional
change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/cpu/microcode/intel.c | 103 +++++++++++++------------------------
 1 file changed, 37 insertions(+), 66 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 2cccf9c26d..8f4ebbd759 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -32,17 +32,12 @@
 
 #define pr_debug(x...) ((void)0)
 
-struct microcode_header_intel {
+struct microcode_patch {
     unsigned int hdrver;
     unsigned int rev;
-    union {
-        struct {
-            uint16_t year;
-            uint8_t day;
-            uint8_t month;
-        };
-        unsigned int date;
-    };
+    uint16_t year;
+    uint8_t  day;
+    uint8_t  month;
     unsigned int sig;
     unsigned int cksum;
     unsigned int ldrver;
@@ -57,10 +52,7 @@ struct microcode_header_intel {
     unsigned int _datasize;
     unsigned int _totalsize;
     unsigned int reserved[3];
-};
 
-struct microcode_intel {
-    struct microcode_header_intel hdr;
     uint8_t data[];
 };
 
@@ -76,21 +68,17 @@ struct extended_sigtable {
     } sigs[];
 };
 
-struct microcode_patch {
-    struct microcode_intel *mc_intel;
-};
-
 #define PPRO_UCODE_DATASIZE     2000
-#define MC_HEADER_SIZE          (sizeof(struct microcode_header_intel))
+#define MC_HEADER_SIZE          offsetof(struct microcode_patch, data)
 
-static uint32_t get_datasize(const struct microcode_header_intel *hdr)
+static uint32_t get_datasize(const struct microcode_patch *mc)
 {
-    return hdr->_datasize ?: PPRO_UCODE_DATASIZE;
+    return mc->_datasize ?: PPRO_UCODE_DATASIZE;
 }
 
-static uint32_t get_totalsize(const struct microcode_header_intel *hdr)
+static uint32_t get_totalsize(const struct microcode_patch *mc)
 {
-    return hdr->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
+    return mc->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
 }
 
 /*
@@ -100,10 +88,10 @@ static uint32_t get_totalsize(const struct microcode_header_intel *hdr)
  * fields, and no extended signature table.)
  */
 static const struct extended_sigtable *get_ext_sigtable(
-    const struct microcode_intel *mc)
+    const struct microcode_patch *mc)
 {
-    if ( mc->hdr._totalsize > (MC_HEADER_SIZE + mc->hdr._datasize) )
-        return (void *)&mc->data[mc->hdr._datasize];
+    if ( mc->_totalsize > (MC_HEADER_SIZE + mc->_datasize) )
+        return (void *)&mc->data[mc->_datasize];
 
     return NULL;
 }
@@ -158,11 +146,11 @@ static int collect_cpu_info(struct cpu_signature *csig)
  * header is of a known format, and together with totalsize are within the
  * bounds of the container.  Everything else is unchecked.
  */
-static int microcode_sanity_check(const struct microcode_intel *mc)
+static int microcode_sanity_check(const struct microcode_patch *mc)
 {
     const struct extended_sigtable *ext;
-    unsigned int total_size = get_totalsize(&mc->hdr);
-    unsigned int data_size = get_datasize(&mc->hdr);
+    unsigned int total_size = get_totalsize(mc);
+    unsigned int data_size = get_datasize(mc);
     unsigned int i, ext_size;
     uint32_t sum, *ptr;
 
@@ -211,7 +199,7 @@ static int microcode_sanity_check(const struct microcode_intel *mc)
      * Checksum each indiviudal extended signature as if it had been in the
      * main header.
      */
-    sum = mc->hdr.sig + mc->hdr.pf + mc->hdr.cksum;
+    sum = mc->sig + mc->pf + mc->cksum;
     for ( i = 0; i < ext->count; ++i )
         if ( sum != (ext->sigs[i].sig + ext->sigs[i].pf + ext->sigs[i].cksum) )
             return -EINVAL;
@@ -221,7 +209,7 @@ static int microcode_sanity_check(const struct microcode_intel *mc)
 
 /* Check an update against the CPU signature and current update revision */
 static enum microcode_match_result microcode_update_match(
-    const struct microcode_intel *mc)
+    const struct microcode_patch *mc)
 {
     const struct extended_sigtable *ext;
     unsigned int i;
@@ -230,7 +218,7 @@ static enum microcode_match_result microcode_update_match(
     ASSERT(!microcode_sanity_check(mc));
 
     /* Check the main microcode signature. */
-    if ( signature_maches(cpu_sig, mc->hdr.sig, mc->hdr.pf) )
+    if ( signature_maches(cpu_sig, mc->sig, mc->pf) )
         goto found;
 
     /* If there is an extended signature table, check each of them. */
@@ -242,7 +230,7 @@ static enum microcode_match_result microcode_update_match(
     return MIS_UCODE;
 
  found:
-    return mc->hdr.rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
+    return mc->rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
 }
 
 static bool match_cpu(const struct microcode_patch *patch)
@@ -250,16 +238,12 @@ static bool match_cpu(const struct microcode_patch *patch)
     if ( !patch )
         return false;
 
-    return microcode_update_match(patch->mc_intel) == NEW_UCODE;
+    return microcode_update_match(patch) == NEW_UCODE;
 }
 
 static void free_patch(struct microcode_patch *patch)
 {
-    if ( patch )
-    {
-        xfree(patch->mc_intel);
-        xfree(patch);
-    }
+    xfree(patch);
 }
 
 static enum microcode_match_result compare_patch(
@@ -269,11 +253,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) != MIS_UCODE);
-    ASSERT(microcode_update_match(new->mc_intel) != MIS_UCODE);
+    ASSERT(microcode_update_match(old) != MIS_UCODE);
+    ASSERT(microcode_update_match(new) != MIS_UCODE);
 
-    return (new->mc_intel->hdr.rev > old->mc_intel->hdr.rev) ? NEW_UCODE
-                                                             : OLD_UCODE;
+    return new->rev > old->rev ? NEW_UCODE : OLD_UCODE;
 }
 
 static int apply_microcode(const struct microcode_patch *patch)
@@ -281,7 +264,6 @@ static int apply_microcode(const struct microcode_patch *patch)
     uint64_t msr_content;
     unsigned int cpu = smp_processor_id();
     struct cpu_signature *sig = &this_cpu(cpu_sig);
-    const struct microcode_intel *mc_intel;
     uint32_t rev, old_rev = sig->rev;
 
     if ( !patch )
@@ -290,12 +272,10 @@ static int apply_microcode(const struct microcode_patch *patch)
     if ( !match_cpu(patch) )
         return -EINVAL;
 
-    mc_intel = patch->mc_intel;
-
     BUG_ON(local_irq_is_enabled());
 
     /* write microcode via MSR 0x79 */
-    wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->data);
+    wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)patch->data);
     wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);
 
     /* As documented in the SDM: Do a CPUID 1 here */
@@ -305,18 +285,17 @@ static int apply_microcode(const struct microcode_patch *patch)
     rdmsrl(MSR_IA32_UCODE_REV, msr_content);
     sig->rev = rev = msr_content >> 32;
 
-    if ( rev != mc_intel->hdr.rev )
+    if ( rev != patch->rev )
     {
         printk(XENLOG_ERR
                "microcode: CPU%u update rev %#x to %#x failed, result %#x\n",
-               cpu, old_rev, mc_intel->hdr.rev, rev);
+               cpu, old_rev, patch->rev, rev);
         return -EIO;
     }
 
     printk(XENLOG_WARNING
            "microcode: CPU%u updated from revision %#x to %#x, date = %04x-%02x-%02x\n",
-           cpu, old_rev, rev, mc_intel->hdr.year,
-           mc_intel->hdr.month, mc_intel->hdr.day);
+           cpu, old_rev, rev, patch->year, patch->month, patch->day);
 
     return 0;
 }
@@ -325,19 +304,19 @@ static struct microcode_patch *cpu_request_microcode(const void *buf,
                                                      size_t size)
 {
     int error = 0;
-    const struct microcode_intel *saved = NULL;
+    const struct microcode_patch *saved = NULL;
     struct microcode_patch *patch = NULL;
 
     while ( size )
     {
-        const struct microcode_intel *mc;
+        const struct microcode_patch *mc;
         unsigned int blob_size;
 
         if ( size < MC_HEADER_SIZE ||       /* Insufficient space for header? */
-             (mc = buf)->hdr.hdrver != 1 || /* Unrecognised header version?   */
-             mc->hdr.ldrver != 1 ||         /* Unrecognised loader version?   */
+             (mc = buf)->hdrver != 1 ||     /* Unrecognised header version?   */
+             mc->ldrver != 1 ||             /* Unrecognised loader version?   */
              size < (blob_size =            /* Insufficient space for patch?  */
-                     get_totalsize(&mc->hdr)) )
+                     get_totalsize(mc)) )
         {
             error = -EINVAL;
             break;
@@ -352,7 +331,7 @@ static struct microcode_patch *cpu_request_microcode(const void *buf,
          * one with higher revision.
          */
         if ( (microcode_update_match(mc) != MIS_UCODE) &&
-             (!saved || (mc->hdr.rev > saved->hdr.rev)) )
+             (!saved || (mc->rev > saved->rev)) )
             saved = mc;
 
         buf  += blob_size;
@@ -361,17 +340,9 @@ static struct microcode_patch *cpu_request_microcode(const void *buf,
 
     if ( saved )
     {
-        patch = xmalloc(struct microcode_patch);
-        if ( patch )
-        {
-            patch->mc_intel = xmemdup_bytes(saved, get_totalsize(&saved->hdr));
-            if ( !patch->mc_intel )
-            {
-                XFREE(patch);
-                error = -ENOMEM;
-            }
-        }
-        else
+        patch = xmemdup_bytes(saved, get_totalsize(saved));
+
+        if ( !patch )
             error = -ENOMEM;
     }
 
-- 
2.11.0


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