[PATCH 1/2] x86/ucode: Adjust parse_ucode() to match other list handling

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

parse_ucode() is abnormal compared to similar parsing elsewhere in Xen.

Invert the ucode_mod_forced check with respect to the "scan" and integer
handling, so we can warn the user when we've elected to ignore the parameters,
and yield -EINVAL for any unrecognised list element.

Rewrite the ucode= command line docs for clarity.

No practical change.

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <JBeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
---
 docs/misc/xen-command-line.pandoc | 56 ++++++++++++++++++-------------
 xen/arch/x86/cpu/microcode/core.c | 22 ++++++++----
 2 files changed, 47 insertions(+), 31 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index 0c6225391d55..47674025249a 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2721,34 +2721,42 @@ 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>, nmi=<bool> ]`
 
     Applicability: x86
     Default: `scan` is selectable via Kconfig, `nmi=true`
 
-Controls for CPU microcode loading. For early loading, this parameter can
-specify how and where to find the microcode update blob. For late loading,
-this parameter specifies if the update happens within a NMI handler.
-
-'integer' specifies the CPU microcode update blob module index. When positive,
-this specifies the n-th module (in the GrUB entry, zero based) to be used
-for updating CPU micrcode. When negative, counting starts at the end of
-the modules in the GrUB entry (so with the blob commonly being last,
-one could specify `ucode=-1`). Note that the value of zero is not valid
-here (entry zero, i.e. the first module, is always the Dom0 kernel
-image). Note further that use of this option has an unspecified effect
-when used with xen.efi (there the concept of modules doesn't exist, and
-the blob gets specified via the `ucode=<filename>` config file/section
-entry; see [EFI configuration file description](efi.html)).
-
-'scan' instructs the hypervisor to scan the multiboot images for an cpio
-image that contains microcode. Depending on the platform the blob with the
-microcode in the cpio name space must be:
-  - on Intel: kernel/x86/microcode/GenuineIntel.bin
-  - on AMD  : kernel/x86/microcode/AuthenticAMD.bin
-When using xen.efi, the `ucode=<filename>` config file setting takes
-precedence over `scan`. The default value for `scan` is set with
-`CONFIG_UCODE_SCAN_DEFAULT`.
+Controls for CPU microcode loading.
+
+In order to load microcode at boot, Xen needs to find a suitable update
+amongst the modules provided by the bootloader.  Two kinds of microcode update
+are supported:
+
+ 1. Raw microcode containers.  The format of the container is CPU vendor
+    specific.
+
+ 2. CPIO archive.  This is Linux's preferred mechanism, and involves having
+    the raw containers expressed as files
+    (e.g. `kernel/x86/microcode/{GenuineIntel,AuthenticAMD}.bin`) in a CPIO
+    archive, typically prepended to the initrd.
+
+The `<integer>` and `scan=` options are mutually exclusive and select between
+these two options.  They are also invalid in EFI boots (see below).
+
+ *  The `<integer>` option nominates a specific multiboot module as a raw
+    container (option 1 above).  Valid options start from 1 (module 0 is
+    always the dom0 kernel).  A negative number may be used, and will
+    back-reference from the end of the module list.  i.e. `ucode=-1` will
+    nominate the final multiboot module.
+
+ *  The `scan=` option causes Xen to search all modules in order to find the
+    first CPIO archive containing the appropriate file (option 2 above).  The
+    default for this option can be chosen at build time via
+    `CONFIG_UCODE_SCAN_DEFAULT`.
+
+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
diff --git a/xen/arch/x86/cpu/microcode/core.c 
b/xen/arch/x86/cpu/microcode/core.c
index de00c22b4bd6..c8e14ed9b10c 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -113,11 +113,6 @@ void __init microcode_set_module(unsigned int idx)
     ucode_mod_forced = 1;
 }
 
-/*
- * The format is '[<integer>|scan=<bool>, nmi=<bool>]'. Both options are
- * optional. If the EFI has forced which of the multiboot payloads is to be
- * used, only nmi=<bool> is parsed.
- */
 static int __init cf_check parse_ucode(const char *s)
 {
     const char *ss;
@@ -130,13 +125,24 @@ static int __init cf_check parse_ucode(const char *s)
 
         if ( (val = parse_boolean("nmi", s, ss)) >= 0 )
             ucode_in_nmi = val;
-        else if ( !ucode_mod_forced ) /* Not forced by EFI */
+        else if ( (val = parse_boolean("scan", s, ss)) >= 0 )
         {
-            if ( (val = parse_boolean("scan", s, ss)) >= 0 )
+            if ( ucode_mod_forced )
+                printk(XENLOG_WARNING
+                       "Ignoring ucode=%.*s setting; overridden by EFI\n",
+                       (int)(ss - s), s);
+            else
             {
                 opt_scan = val;
                 opt_mod_idx = 0;
             }
+        }
+        else if ( isdigit(s[0]) || s[0] == '-' )
+        {
+            if ( ucode_mod_forced )
+                printk(XENLOG_WARNING
+                       "Ignoring ucode=%.*s setting; overridden by EFI\n",
+                       (int)(ss - s), s);
             else
             {
                 const char *q;
@@ -151,6 +157,8 @@ static int __init cf_check parse_ucode(const char *s)
                     opt_scan = false;
             }
         }
+        else
+            rc = -EINVAL;
 
         s = ss + 1;
     } while ( *ss );
-- 
2.39.5