x86: Move microcode loading earlier

Move microcode loading earlier for the boot CPU and secondary CPUs so
that it takes place before identify_cpu() is called for each CPU.
Without this, the detected features may be wrong if the new microcode
loading adjusts the feature bits. That could mean that some fixes (e.g.
d6e9f8d4f35d ("x86/vmx: fix vmentry failure with TSX bits in LBR"))
don't work as expected.

Previously during boot, the microcode loader was invoked for each
secondary CPU started and then again for each CPU as part of an
initcall. Simplify the code so that it is invoked exactly once for each
CPU during boot.

Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Tested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Release-acked-by: Julien Grall <julien.grall@arm.com>

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index a1494c33c8be307e931c10acb73e52abe907e706..93ead6e5dd715f79e9e0200395cdd1dfee99366e 100644 (file)
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -40,7 +40,6 @@ obj-y += irq.o
 obj-$(CONFIG_KEXEC) += machine_kexec.o
 obj-y += microcode_amd.o
 obj-y += microcode_intel.o
-# This must come after the vendor specific files.
 obj-y += microcode.o
 obj-y += mm.o x86_64/mm.o
 obj-y += monitor.o

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 9c44bbdc28a99688231de50b92e7c4dbdd3b69d4..6c27008d7b7764c9955f6c40973fec542b6568bf 100644 (file)
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -252,6 +252,8 @@ static void __init early_cpu_detect(void)
                if (hap_paddr_bits > PADDR_BITS)
                        hap_paddr_bits = PADDR_BITS;
        }
+
+       initialize_cpu_data(0);
 }
 
 static void generic_identify(struct cpuinfo_x86 *c)

diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 30a08064eea640ce8a1bcdb371508dd6320b61db..4e7dfcd5735723994f4d9124d36c756b21ae0b11 100644 (file)
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -43,7 +43,6 @@ static module_t __initdata ucode_mod;
 static void *(*__initdata ucode_mod_map)(const module_t *);
 static signed int __initdata ucode_mod_idx;
 static bool_t __initdata ucode_mod_forced;
-static cpumask_t __initdata init_mask;
 
 /*
  * If we scan the initramfs.cpio for the early microcode code
@@ -341,50 +340,23 @@ int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void) buf, unsigned long len)
     return continue_hypercall_on_cpu(info->cpu, do_microcode_update, info);
 }
 
-static void __init _do_microcode_update(unsigned long data)
-{
-    void *_data = (void *)data;
-    size_t len = ucode_blob.size ? ucode_blob.size : ucode_mod.mod_end;
-
-    microcode_update_cpu(_data, len);
-    cpumask_set_cpu(smp_processor_id(), &init_mask);
-}
-
 static int __init microcode_init(void)
 {
-    void *data;
-    static struct tasklet __initdata tasklet;
-    unsigned int cpu;
-
-    if ( !microcode_ops )
-        return 0;
-
-    if ( !ucode_mod.mod_end && !ucode_blob.size )
-        return 0;
-
-    data = ucode_blob.size ? ucode_blob.data : ucode_mod_map(&ucode_mod);
-
-    if ( !data )
-        return -ENOMEM;
-
-    if ( microcode_ops->start_update && microcode_ops->start_update() != 0 )
-        goto out;
-
-    softirq_tasklet_init(&tasklet, _do_microcode_update, (unsigned long)data);
-
-    for_each_online_cpu ( cpu )
+    /*
+     * At this point, all CPUs should have updated their microcode
+     * via the early_microcode_* paths so free the microcode blob.
+     */
+    if ( ucode_blob.size )
     {
-        tasklet_schedule_on_cpu(&tasklet, cpu);
-        do {
-            process_pending_softirqs();
-        } while ( !cpumask_test_cpu(cpu, &init_mask) );
+        xfree(ucode_blob.data);
+        ucode_blob.size = 0;
+        ucode_blob.data = NULL;
     }
-
-out:
-    if ( ucode_blob.size )
-        xfree(data);
-    else
+    else if ( ucode_mod.mod_end )
+    {
         ucode_mod_map(NULL);
+        ucode_mod.mod_end = 0;
+    }
 
     return 0;
 }
@@ -409,50 +381,55 @@ static struct notifier_block microcode_percpu_nfb = {
     .notifier_call = microcode_percpu_callback,
 };
 
-static int __init microcode_presmp_init(void)
+int __init early_microcode_update_cpu(bool start_update)
+{
+    int rc = 0;
+    void *data = NULL;
+    size_t len;
+
+    if ( ucode_blob.size )
+    {
+        len = ucode_blob.size;
+        data = ucode_blob.data;
+    }
+    else if ( ucode_mod.mod_end )
+    {
+        len = ucode_mod.mod_end;
+        data = ucode_mod_map(&ucode_mod);
+    }
+    if ( data )
+    {
+        if ( start_update && microcode_ops->start_update )
+            rc = microcode_ops->start_update();
+
+        if ( rc )
+            return rc;
+
+        return microcode_update_cpu(data, len);
+    }
+    else
+        return -ENOMEM;
+}
+
+int __init early_microcode_init(void)
 {
+    int rc;
+
+    rc = microcode_init_intel();
+    if ( rc )
+        return rc;
+
+    rc = microcode_init_amd();
+    if ( rc )
+        return rc;
+
     if ( microcode_ops )
     {
         if ( ucode_mod.mod_end || ucode_blob.size )
-        {
-            void *data;
-            size_t len;
-            int rc = 0;
-
-            if ( ucode_blob.size )
-            {
-                len = ucode_blob.size;
-                data = ucode_blob.data;
-            }
-            else
-            {
-                len = ucode_mod.mod_end;
-                data = ucode_mod_map(&ucode_mod);
-            }
-            if ( data )
-                rc = microcode_update_cpu(data, len);
-            else
-                rc = -ENOMEM;
-
-            if ( !ucode_blob.size )
-                ucode_mod_map(NULL);
-
-            if ( rc )
-            {
-                if ( ucode_blob.size )
-                {
-                    xfree(ucode_blob.data);
-                    ucode_blob.size = 0;
-                    ucode_blob.data = NULL;
-                }
-                else
-                    ucode_mod.mod_end = 0;
-            }
-        }
+            rc = early_microcode_update_cpu(true);
 
         register_cpu_notifier(&microcode_percpu_nfb);
     }
 
     return 0;
 }
-presmp_initcall(microcode_presmp_init);

diff --git a/xen/arch/x86/microcode_amd.c b/xen/arch/x86/microcode_amd.c
index 47599112b1d1f8df4e54676876158651946addce..b54b0b99e4928bcb9178e59d6520e4d91d776589 100644 (file)
--- a/xen/arch/x86/microcode_amd.c
+++ b/xen/arch/x86/microcode_amd.c
@@ -627,10 +627,9 @@ static const struct microcode_ops microcode_amd_ops = {
     .start_update                     = start_update,
 };
 
-static __init int microcode_init_amd(void)
+int __init microcode_init_amd(void)
 {
     if ( boot_cpu_data.x86_vendor == X86_VENDOR_AMD )
         microcode_ops = &microcode_amd_ops;
     return 0;
 }
-presmp_initcall(microcode_init_amd);

diff --git a/xen/arch/x86/microcode_intel.c b/xen/arch/x86/microcode_intel.c
index ba3971a6598497bf6a3bca10cfcd7e13e609075f..c6b67e4fe7cecd80000ab61f1c56822ff1a7071a 100644 (file)
--- a/xen/arch/x86/microcode_intel.c
+++ b/xen/arch/x86/microcode_intel.c
@@ -404,10 +404,9 @@ static const struct microcode_ops microcode_intel_ops = {
     .apply_microcode                  = apply_microcode,
 };
 
-static __init int microcode_init_intel(void)
+int __init microcode_init_intel(void)
 {
     if ( boot_cpu_data.x86_vendor == X86_VENDOR_INTEL )
         microcode_ops = &microcode_intel_ops;
     return 0;
 }
-presmp_initcall(microcode_init_intel);

diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index c3d99e9b54bcf42c0290c5bcf3f546670c6894e2..f7b927858c54a39ec89a57cff21181f469f9926d 100644 (file)
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1461,6 +1461,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
 
     timer_init();
 
+    early_microcode_init();
+
     identify_cpu(&boot_cpu_data);
 
     set_in_cr4(X86_CR4_OSFXSR | X86_CR4_OSXMMEXCPT);

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 82559ed018b0533d8260ca928813da0ab8889c40..50b907b07193628e675d92de73de84189c3ccf35 100644 (file)
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -83,22 +83,22 @@ static enum cpu_state {
 
 void *stack_base[NR_CPUS];
 
+void initialize_cpu_data(unsigned int cpu)
+{
+    cpu_data[cpu] = boot_cpu_data;
+}
+
 static void smp_store_cpu_info(int id)
 {
-    struct cpuinfo_x86 *c = cpu_data + id;
     unsigned int socket;
 
-    *c = boot_cpu_data;
-    if ( id != 0 )
-    {
-        identify_cpu(c);
+    identify_cpu(&cpu_data[id]);
 
-        socket = cpu_to_socket(id);
-        if ( !socket_cpumask[socket] )
-        {
-            socket_cpumask[socket] = secondary_socket_cpumask;
-            secondary_socket_cpumask = NULL;
-        }
+    socket = cpu_to_socket(id);
+    if ( !socket_cpumask[socket] )
+    {
+        socket_cpumask[socket] = secondary_socket_cpumask;
+        secondary_socket_cpumask = NULL;
     }
 }
 
@@ -332,6 +332,13 @@ void start_secondary(void *unused)
 
     cpu_init();
 
+    initialize_cpu_data(cpu);
+
+    if ( system_state <= SYS_STATE_smp_boot )
+        early_microcode_update_cpu(false);
+    else
+        microcode_resume_cpu(cpu);
+
     smp_callin();
 
     init_percpu_time();
@@ -364,8 +371,6 @@ void start_secondary(void *unused)
     local_irq_enable();
     mtrr_ap_init();
 
-    microcode_resume_cpu(cpu);
-
     wmb();
     startup_cpu_idle_loop();
 }
@@ -780,7 +785,7 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
     mtrr_aps_sync_begin();
 
     /* Setup boot CPU information */
-    smp_store_cpu_info(0); /* Final full version of the data */
+    initialize_cpu_data(0); /* Final full version of the data */
     print_cpu_info(0);
 
     boot_cpu_physical_apicid = get_apic_id();

diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 75632d9ce3f98712fa1c46e046d4df7c147583d9..73dd3abd03af840a7975a85ea892501d2df39dcd 100644 (file)
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -563,6 +563,10 @@ int wrmsr_hypervisor_regs(uint32_t idx, uint64_t val);
 void microcode_set_module(unsigned int);
 int microcode_update(XEN_GUEST_HANDLE_PARAM(const_void), unsigned long len);
 int microcode_resume_cpu(unsigned int cpu);
+int early_microcode_update_cpu(bool start_update);
+int early_microcode_init(void);
+int microcode_init_intel(void);
+int microcode_init_amd(void);
 
 enum get_cpu_vendor {
     gcv_host,

diff --git a/xen/include/xen/smp.h b/xen/include/xen/smp.h
index 6febb5605c65b84f47bb13126afa3e3f8cb38910..c55f57f09ef8607bf06df120b928d2e22c1f7f75 100644 (file)
--- a/xen/include/xen/smp.h
+++ b/xen/include/xen/smp.h
@@ -71,4 +71,6 @@ int alloc_cpu_id(void);
 
 extern void *stack_base[NR_CPUS];
 
+void initialize_cpu_data(unsigned int cpu);
+
 #endif /* __XEN_SMP_H__ */