x86/microcode: Handle "nosmt" correctly

On CPUs where microcode loading is not NMI-safe the SMT siblings which
are parked in one of the play_dead() variants still react to NMIs.

So if an NMI hits while the primary thread updates the microcode the
resulting behaviour is undefined. The default play_dead() implementation on
modern CPUs is using MWAIT which is not guaranteed to be safe against
a microcode update which affects MWAIT.

Take the cpus_booted_once_mask into account to detect this case and
refuse to load late if the vendor specific driver does not advertise
that late loading is NMI safe.

AMD stated that this is safe, so mark the AMD driver accordingly.

This requirement will be partially lifted in later changes.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Borislav Petkov (AMD) <bp@alien8.de>
Link: https://lore.kernel.org/r/20231002115903.087472735@linutronix.de

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 011cb7f200d98631573c60d3efe53441462fb1cc..db7512008ff778c55adc7914ad92e0eed662aafe 100644 (file)
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -1320,7 +1320,7 @@ config MICROCODE_INITRD32
 config MICROCODE_LATE_LOADING
        bool "Late microcode loading (DANGEROUS)"
        default n
-       depends on MICROCODE
+       depends on MICROCODE && SMP
        help
          Loading microcode late, when the system is up and executing instructions
          is a tricky business and should be avoided if possible. Just the sequence

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index 99aa5a88d668d0c6f754fba58f135cf978d4bb45..102aebcdb7665b86e36195663e86cb264bcc9d3e 100644 (file)
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -917,10 +917,11 @@ static void microcode_fini_cpu_amd(int cpu)
 }
 
 static struct microcode_ops microcode_amd_ops = {
-       .request_microcode_fw             = request_microcode_amd,
-       .collect_cpu_info                 = collect_cpu_info_amd,
-       .apply_microcode                  = apply_microcode_amd,
-       .microcode_fini_cpu               = microcode_fini_cpu_amd,
+       .request_microcode_fw   = request_microcode_amd,
+       .collect_cpu_info       = collect_cpu_info_amd,
+       .apply_microcode        = apply_microcode_amd,
+       .microcode_fini_cpu     = microcode_fini_cpu_amd,
+       .nmi_safe               = true,
 };
 
 struct microcode_ops * __init init_amd_microcode(void)

diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c
index e306feeb374f894f27c6a56e019200f1a24f3dfe..7af1b60527ec40f2c03994baa7f75857fa970bc5 100644 (file)
--- a/arch/x86/kernel/cpu/microcode/core.c
+++ b/arch/x86/kernel/cpu/microcode/core.c
@@ -254,23 +254,6 @@ static struct platform_device      *microcode_pdev;
  */
 #define SPINUNIT 100 /* 100 nsec */
 
-static int check_online_cpus(void)
-{
-       unsigned int cpu;
-
-       /*
-        * Make sure all CPUs are online.  It's fine for SMT to be disabled if
-        * all the primary threads are still online.
-        */
-       for_each_present_cpu(cpu) {
-               if (topology_is_primary_thread(cpu) && !cpu_online(cpu)) {
-                       pr_err("Not all CPUs online, aborting microcode update.\n");
-                       return -EINVAL;
-               }
-       }
-
-       return 0;
-}
 
 static atomic_t late_cpus_in;
 static atomic_t late_cpus_out;
@@ -387,6 +370,35 @@ static int microcode_reload_late(void)
        return ret;
 }
 
+/*
+ *  Ensure that all required CPUs which are present and have been booted
+ *  once are online.
+ *
+ *    To pass this check, all primary threads must be online.
+ *
+ *    If the microcode load is not safe against NMI then all SMT threads
+ *    must be online as well because they still react to NMIs when they are
+ *    soft-offlined and parked in one of the play_dead() variants. So if a
+ *    NMI hits while the primary thread updates the microcode the resulting
+ *    behaviour is undefined. The default play_dead() implementation on
+ *    modern CPUs uses MWAIT, which is also not guaranteed to be safe
+ *    against a microcode update which affects MWAIT.
+ */
+static bool ensure_cpus_are_online(void)
+{
+       unsigned int cpu;
+
+       for_each_cpu_and(cpu, cpu_present_mask, &cpus_booted_once_mask) {
+               if (!cpu_online(cpu)) {
+                       if (topology_is_primary_thread(cpu) || !microcode_ops->nmi_safe) {
+                               pr_err("CPU %u not online\n", cpu);
+                               return false;
+                       }
+               }
+       }
+       return true;
+}
+
 static ssize_t reload_store(struct device *dev,
                            struct device_attribute *attr,
                            const char *buf, size_t size)
@@ -402,9 +414,10 @@ static ssize_t reload_store(struct device *dev,
 
        cpus_read_lock();
 
-       ret = check_online_cpus();
-       if (ret)
+       if (!ensure_cpus_are_online()) {
+               ret = -EBUSY;
                goto put;
+       }
 
        tmp_ret = microcode_ops->request_microcode_fw(bsp, &microcode_pdev->dev);
        if (tmp_ret != UCODE_NEW)

diff --git a/arch/x86/kernel/cpu/microcode/internal.h b/arch/x86/kernel/cpu/microcode/internal.h
index 4bef216f8d6c739ff44d03344e1f6be20256020c..07aa5f8be69bdf0597eeb442e4763e1db909f003 100644 (file)
--- a/arch/x86/kernel/cpu/microcode/internal.h
+++ b/arch/x86/kernel/cpu/microcode/internal.h
@@ -20,18 +20,17 @@ enum ucode_state {
 
 struct microcode_ops {
        enum ucode_state (*request_microcode_fw)(int cpu, struct device *dev);
-
        void (*microcode_fini_cpu)(int cpu);
 
        /*
-        * The generic 'microcode_core' part guarantees that
-        * the callbacks below run on a target cpu when they
-        * are being called.
+        * The generic 'microcode_core' part guarantees that the callbacks
+        * below run on a target CPU when they are being called.
         * See also the "Synchronization" section in microcode_core.c.
         */
-       enum ucode_state (*apply_microcode)(int cpu);
-       int (*collect_cpu_info)(int cpu, struct cpu_signature *csig);
-       void (*finalize_late_load)(int result);
+       enum ucode_state        (*apply_microcode)(int cpu);
+       int                     (*collect_cpu_info)(int cpu, struct cpu_signature *csig);
+       void                    (*finalize_late_load)(int result);
+       unsigned int            nmi_safe        : 1;
 };
 
 extern struct ucode_cpu_info ucode_cpu_info[];