[Xen-changelog] [xen master] x86/ucode: Rationalise startup and family/model checks

[patchbot@xxxxxxx]

commit dd5f07997f2970e024c44cd8845f67dfdc72c17a
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Thu Mar 19 13:54:19 2020 +0000
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Fri Mar 20 18:42:24 2020 +0000

    x86/ucode: Rationalise startup and family/model checks
    
    Drop microcode_init_{intel,amd}(), export {intel,amd}_ucode_ops, and use a
    switch statement in early_microcode_init() rather than probing each vendor 
in
    turn.  This allows the microcode_ops pointer to become local to core.c.
    
    As there are no external users of microcode_ops, there is no need for
    collect_cpu_info() to implement sanity checks.  Move applicable checks to
    early_microcode_init() so they are performed once, rather than repeatedly.
    
    The Intel logic guarding the read of MSR_PLATFORM_ID is contrary to the SDM,
    which states that the MSR has been architectural since the Pentium Pro
    (06-01-xx), and lists no family/model restrictions in the pseudo-code for
    microcode loading.  Either way, Xen's 64bit-only nature already makes this
    check redundant.
    
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/cpu/microcode/amd.c     | 21 ++------------------
 xen/arch/x86/cpu/microcode/core.c    | 37 ++++++++++++++++++++++--------------
 xen/arch/x86/cpu/microcode/intel.c   | 26 +++----------------------
 xen/arch/x86/cpu/microcode/private.h |  5 +----
 4 files changed, 29 insertions(+), 60 deletions(-)

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 9028889813..768fbcf322 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -76,22 +76,12 @@ struct mpbhdr {
 /* See comment in start_update() for cases when this routine fails */
 static int collect_cpu_info(struct cpu_signature *csig)
 {
-    unsigned int cpu = smp_processor_id();
-    struct cpuinfo_x86 *c = &cpu_data[cpu];
-
     memset(csig, 0, sizeof(*csig));
 
-    if ( (c->x86_vendor != X86_VENDOR_AMD) || (c->x86 < 0x10) )
-    {
-        printk(KERN_ERR "microcode: CPU%d not a capable AMD processor\n",
-               cpu);
-        return -EINVAL;
-    }
-
     rdmsrl(MSR_AMD_PATCHLEVEL, csig->rev);
 
     pr_debug("microcode: CPU%d collect_cpu_info: patch_id=%#x\n",
-             cpu, csig->rev);
+             smp_processor_id(), csig->rev);
 
     return 0;
 }
@@ -601,7 +591,7 @@ static int start_update(void)
 }
 #endif
 
-static const struct microcode_ops microcode_amd_ops = {
+const struct microcode_ops amd_ucode_ops = {
     .cpu_request_microcode            = cpu_request_microcode,
     .collect_cpu_info                 = collect_cpu_info,
     .apply_microcode                  = apply_microcode,
@@ -613,10 +603,3 @@ static const struct microcode_ops microcode_amd_ops = {
     .compare_patch                    = compare_patch,
     .match_cpu                        = match_cpu,
 };
-
-int __init microcode_init_amd(void)
-{
-    if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
-        microcode_ops = &microcode_amd_ops;
-    return 0;
-}
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index ac5da6b2fe..61150e04c8 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -210,7 +210,7 @@ scan:
         microcode_scan_module(module_map, mbi);
 }
 
-const struct microcode_ops *microcode_ops;
+static const struct microcode_ops __read_mostly *microcode_ops;
 
 static DEFINE_SPINLOCK(microcode_mutex);
 
@@ -798,23 +798,32 @@ static int __init early_microcode_update_cpu(void)
 
 int __init early_microcode_init(void)
 {
-    int rc;
-
-    rc = microcode_init_intel();
-    if ( rc )
-        return rc;
-
-    rc = microcode_init_amd();
-    if ( rc )
-        return rc;
+    const struct cpuinfo_x86 *c = &boot_cpu_data;
+    int rc = 0;
 
-    if ( microcode_ops )
+    switch ( c->x86_vendor )
     {
-        microcode_ops->collect_cpu_info(&this_cpu(cpu_sig));
+    case X86_VENDOR_AMD:
+        if ( c->x86 >= 0x10 )
+            microcode_ops = &amd_ucode_ops;
+        break;
+
+    case X86_VENDOR_INTEL:
+        if ( c->x86 >= 6 )
+            microcode_ops = &intel_ucode_ops;
+        break;
+    }
 
-        if ( ucode_mod.mod_end || ucode_blob.size )
-            rc = early_microcode_update_cpu();
+    if ( !microcode_ops )
+    {
+        printk(XENLOG_WARNING "Microcode loading not available\n");
+        return -ENODEV;
     }
 
+    microcode_ops->collect_cpu_info(&this_cpu(cpu_sig));
+
+    if ( ucode_mod.mod_end || ucode_blob.size )
+        rc = early_microcode_update_cpu();
+
     return rc;
 }
diff --git a/xen/arch/x86/cpu/microcode/intel.c 
b/xen/arch/x86/cpu/microcode/intel.c
index 90fb006c94..48544e8d6d 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -93,27 +93,14 @@ struct extended_sigtable {
 
 static int collect_cpu_info(struct cpu_signature *csig)
 {
-    unsigned int cpu_num = smp_processor_id();
-    struct cpuinfo_x86 *c = &cpu_data[cpu_num];
     uint64_t msr_content;
 
     memset(csig, 0, sizeof(*csig));
 
-    if ( (c->x86_vendor != X86_VENDOR_INTEL) || (c->x86 < 6) )
-    {
-        printk(KERN_ERR "microcode: CPU%d not a capable Intel "
-               "processor\n", cpu_num);
-        return -1;
-    }
-
     csig->sig = cpuid_eax(0x00000001);
 
-    if ( (c->x86_model >= 5) || (c->x86 > 6) )
-    {
-        /* get processor flags from MSR 0x17 */
-        rdmsrl(MSR_IA32_PLATFORM_ID, msr_content);
-        csig->pf = 1 << ((msr_content >> 50) & 7);
-    }
+    rdmsrl(MSR_IA32_PLATFORM_ID, msr_content);
+    csig->pf = 1 << ((msr_content >> 50) & 7);
 
     wrmsrl(MSR_IA32_UCODE_REV, 0x0ULL);
     /* As documented in the SDM: Do a CPUID 1 here */
@@ -405,7 +392,7 @@ static struct microcode_patch *cpu_request_microcode(const 
void *buf,
     return patch;
 }
 
-static const struct microcode_ops microcode_intel_ops = {
+const struct microcode_ops intel_ucode_ops = {
     .cpu_request_microcode            = cpu_request_microcode,
     .collect_cpu_info                 = collect_cpu_info,
     .apply_microcode                  = apply_microcode,
@@ -413,10 +400,3 @@ static const struct microcode_ops microcode_intel_ops = {
     .compare_patch                    = compare_patch,
     .match_cpu                        = match_cpu,
 };
-
-int __init microcode_init_intel(void)
-{
-    if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
-        microcode_ops = &microcode_intel_ops;
-    return 0;
-}
diff --git a/xen/arch/x86/cpu/microcode/private.h 
b/xen/arch/x86/cpu/microcode/private.h
index 459b6a4c54..c32ddc8d19 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -32,9 +32,6 @@ struct microcode_ops {
         const struct microcode_patch *new, const struct microcode_patch *old);
 };
 
-extern const struct microcode_ops *microcode_ops;
-
-int microcode_init_intel(void);
-int microcode_init_amd(void);
+extern const struct microcode_ops amd_ucode_ops, intel_ucode_ops;
 
 #endif /* ASM_X86_MICROCODE_PRIVATE_H */
--
generated by git-patchbot for /home/xen/git/xen.git#master

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