[PATCH 2/2] x86/ucode: Drop the ucode=nmi option

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

This option is active by default, and despite what the documentation suggests
about choosing ucode=no-nmi, it only controls whether the primary threads move
into NMI context.

Sibling threads unconditionally move into NMI context, which is confirmed by
an in-code comment.

Not discussed is the fact that the BSP never enters NMI context, which works
only by luck (AMD CPUs, where we reload on sibling threads too, has working
in-core rendezvous and doesn't require NMI cover on the primary thread for
safety).  This does want addressing, but requires more untangling first.

Overall, `ucode=no-nmi` is a misleading and pretty useless option.  Drop it,
and simplify the two users.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Despite the reasonably large diff in primary_thread_fn(), it's mostly just
unindenting the block, and dropping the final call to primary_thread_work()
which is made dead by this change.
---
 docs/misc/xen-command-line.pandoc |  8 ++-----
 xen/arch/x86/cpu/microcode/core.c | 38 +++++++++++--------------------
 2 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index 47674025249a..9702c36b8c39 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2721,10 +2721,10 @@ performance.
    Alternatively, selecting `tsx=1` will re-enable TSX at the users own risk.
 
 ### ucode
-> `= List of [ <integer>, scan=<bool>, nmi=<bool> ]`
+> `= List of [ <integer>, scan=<bool ]`
 
     Applicability: x86
-    Default: `scan` is selectable via Kconfig, `nmi=true`
+    Default: `scan` is selectable via Kconfig
 
 Controls for CPU microcode loading.
 
@@ -2758,10 +2758,6 @@ When booting xen.efi natively, the concept of multiboot 
modules doesn't exist.
 Instead, in the [EFI configuration file](efi.html), `ucode=<filename>` can be
 used to identify a file as a raw container (option 1 above).
 
-'nmi' determines late loading is performed in NMI handler or just in
-stop_machine context. In NMI handler, even NMIs are blocked, which is
-considered safer. The default value is `true`.
-
 ### unrestricted_guest (Intel)
 > `= <boolean>`
 
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index c8e14ed9b10c..4898920b9c52 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -82,9 +82,6 @@ struct patch_with_flags {
     const struct microcode_patch *patch;
 };
 
-/* By default, ucode loading is done in NMI handler */
-static bool ucode_in_nmi = true;
-
 /* Protected by microcode_mutex */
 static struct microcode_patch *microcode_cache;
 
@@ -123,9 +120,7 @@ static int __init cf_check parse_ucode(const char *s)
         if ( !ss )
             ss = strchr(s, '\0');
 
-        if ( (val = parse_boolean("nmi", s, ss)) >= 0 )
-            ucode_in_nmi = val;
-        else if ( (val = parse_boolean("scan", s, ss)) >= 0 )
+        if ( (val = parse_boolean("scan", s, ss)) >= 0 )
         {
             if ( ucode_mod_forced )
                 printk(XENLOG_WARNING
@@ -297,12 +292,10 @@ static int cf_check microcode_nmi_callback(
         return 0;
 
     /*
-     * Primary threads load ucode in NMI handler on if ucode_in_nmi is true.
-     * Secondary threads are expected to stay in NMI handler regardless of
-     * ucode_in_nmi.
+     * The BSP doesn't enter NMI context for microcode loading, as it's the
+     * entity organising the rendezvous.
      */
-    if ( cpu == cpumask_first(&cpu_online_map) ||
-         (!ucode_in_nmi && primary_cpu) )
+    if ( cpu == cpumask_first(&cpu_online_map) )
         return 0;
 
     if ( primary_cpu )
@@ -343,22 +336,17 @@ static int primary_thread_fn(const struct microcode_patch 
*patch,
     if ( !wait_for_state(LOADING_CALLIN) )
         return -EBUSY;
 
-    if ( ucode_in_nmi )
-    {
-        self_nmi();
-
-        /*
-         * Wait for ucode loading is done in case that the NMI does not arrive
-         * synchronously, which may lead to a not-yet-updated error is returned
-         * below.
-         */
-        if ( unlikely(!wait_for_state(LOADING_EXIT)) )
-            ASSERT_UNREACHABLE();
+    self_nmi();
 
-        return this_cpu(loading_err);
-    }
+    /*
+     * Wait for ucode loading is done in case that the NMI does not arrive
+     * synchronously, which may lead to a not-yet-updated error is returned
+     * below.
+     */
+    if ( unlikely(!wait_for_state(LOADING_EXIT)) )
+        ASSERT_UNREACHABLE();
 
-    return primary_thread_work(patch, flags);
+    return this_cpu(loading_err);
 }
 
 static int control_thread_fn(const struct microcode_patch *patch,
-- 
2.39.5