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@citrix.com>
Acked-by: Jan Beulich <jbeulich@suse.com>

diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 9028889813af94716ed9bbbb934b7a68ecfd4dfa..768fbcf3227ca126eca5e5a4727e7349e18c948f 100644 (file)
--- 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 ac5da6b2fefb24249b201e6cfad3f7c65cdb9958..61150e04c8216d3c29fd6bff7926c2ae654ab46c 100644 (file)
--- 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 90fb006c949d501655fcab4039d669db1ad88da3..48544e8d6da6265aacbc4d0bdc61900c7d32a7ff 100644 (file)
--- 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 459b6a4c54bb018a1c94736d42715cdb783286d7..c32ddc8d190eb03370f936d039e8fb0fc06cf769 100644 (file)
--- 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 */