[Xen-devel] [PATCH 5/7] x86/ucode/intel: Clean up microcode_update_match()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

Implement a new get_ext_sigtable() helper to abstract the logic for
identifying whether an extended signature table exists.  As part of this,
rename microcode_intel.bits to data and change its type so it can be usefully
used in combination with the datasize header field.

Also, replace the sigmatch() macro with a static inline with a more useful
API, and an explanation of why it is safe to drop one of the previous
conditionals.

No practical change in behaviour.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Wei Liu <wl@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 xen/arch/x86/cpu/microcode/intel.c | 75 +++++++++++++++++++++++++-------------
 1 file changed, 49 insertions(+), 26 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index dfe44679be..bc3bbf139e 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -61,7 +61,7 @@ struct microcode_header_intel {
 
 struct microcode_intel {
     struct microcode_header_intel hdr;
-    unsigned int bits[0];
+    uint8_t data[];
 };
 
 /* microcode format is extended from prescott processors */
@@ -98,8 +98,41 @@ static uint32_t get_totalsize(const struct 
microcode_header_intel *hdr)
     return hdr->_totalsize ?: PPRO_UCODE_DATASIZE + MC_HEADER_SIZE;
 }
 
-#define sigmatch(s1, s2, p1, p2) \
-        (((s1) == (s2)) && (((p1) & (p2)) || (((p1) == 0) && ((p2) == 0))))
+/*
+ * A piece of microcode has an extended signature table if there is space
+ * between the end of data[] and the total size.  (This logic also works
+ * appropriately for Pentium Pro/II microcode, which has 0 for both size
+ * fields, and no extended signature table.)
+ */
+static const struct extended_sigtable *get_ext_sigtable(
+    const struct microcode_intel *mc)
+{
+    if ( mc->hdr._totalsize > (MC_HEADER_SIZE + mc->hdr._datasize) )
+        return (void *)&mc->data[mc->hdr._datasize];
+
+    return NULL;
+}
+
+/*
+ * A piece of microcode is applicable for a CPU if:
+ *  1) the signatures (CPUID.1.EAX - Family/Model/Stepping) match, and
+ *  2) The Platform Flags bitmap intersect.
+ *
+ * A CPU will have a single Platform Flag bit, while the microcode may be
+ * common to multiple platforms and have multiple bits set.
+ *
+ * Note: The Pentium Pro/II microcode didn't use platform flags, and should
+ * treat 0 as a match.  However, Xen being 64bit means that the cpu signature
+ * won't match, allowing us to simplify the logic.
+ */
+static bool signature_maches(const struct cpu_signature *cpu_sig,
+                             unsigned int ucode_sig, unsigned int ucode_pf)
+{
+    if ( cpu_sig->sig != ucode_sig )
+        return false;
+
+    return cpu_sig->pf & ucode_pf;
+}
 
 #define exttable_size(et) ((et)->count * EXT_SIGNATURE_SIZE + EXT_HEADER_SIZE)
 
@@ -221,36 +254,26 @@ static int microcode_sanity_check(const struct 
microcode_intel *mc)
 static enum microcode_match_result microcode_update_match(
     const struct microcode_intel *mc)
 {
-    const struct microcode_header_intel *mc_header = &mc->hdr;
-    const struct extended_sigtable *ext_header;
-    const struct extended_signature *ext_sig;
+    const struct extended_sigtable *ext;
     unsigned int i;
     struct cpu_signature *cpu_sig = &this_cpu(cpu_sig);
-    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);
 
     ASSERT(!microcode_sanity_check(mc));
-    if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
-        return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
 
-    ext_header = (const void *)(mc_header + 1) + data_size;
-    ext_sig = (const void *)(ext_header + 1);
+    /* Check the main microcode signature. */
+    if ( signature_maches(cpu_sig, mc->hdr.sig, mc->hdr.pf) )
+        goto found;
 
-    /*
-     * Make sure there is enough space to hold an extended header and enough
-     * array elements.
-     */
-    if ( end <= (const void *)ext_sig )
-        return MIS_UCODE;
-
-    for ( i = 0; i < ext_header->count; i++ )
-        if ( sigmatch(sig, ext_sig[i].sig, pf, ext_sig[i].pf) )
-            return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
+    /* If there is an extended signature table, check each of them. */
+    if ( (ext = get_ext_sigtable(mc)) != NULL )
+        for ( i = 0; i < ext->count; ++i )
+            if ( signature_maches(cpu_sig, ext->sigs[i].sig, ext->sigs[i].pf) )
+                goto found;
 
     return MIS_UCODE;
+
+ found:
+    return mc->hdr.rev > cpu_sig->rev ? NEW_UCODE : OLD_UCODE;
 }
 
 static bool match_cpu(const struct microcode_patch *patch)
@@ -303,7 +326,7 @@ static int apply_microcode(const struct microcode_patch 
*patch)
     BUG_ON(local_irq_is_enabled());
 
     /* write microcode via MSR 0x79 */
-    wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->bits);
+    wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc_intel->data);
     wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);
 
     /* As documented in the SDM: Do a CPUID 1 here */
-- 
2.11.0


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