[Xen-devel] [PATCH v1 2/4] x86/microcode: Improve parsing for ucode=

[Eslam Elnikety <elnikety@xxxxxxxxxx>]

Decouple the microcode indexing mechanism when using GRUB to that
when using EFI. This allows us to avoid the "unspecified effect" of
using `<integer>` when booting via EFI. With that, Xen can explicitly
ignore that option when using EFI. This is the only functinal change
this commit introduces. Update the command line documentation for
consistency. As an added benefit, the 'parse_ucode' logic becomes
independent of GRUB vs. EFI.

While at it, drop the leading comment for parse_ucode. No practical
use for it given this commit.

Signed-off-by: Eslam Elnikety <elnikety@xxxxxxxxxx>
---
 docs/misc/xen-command-line.pandoc |  6 ++---
 xen/arch/x86/microcode.c          | 38 +++++++++++++++++--------------
 2 files changed, 24 insertions(+), 20 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc 
b/docs/misc/xen-command-line.pandoc
index ebec6d387e..821b9281a1 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2147,9 +2147,9 @@ 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
+image). This option should be used only with legacy boot, as it is explicitly
+ignored in EFI boot. When booting via EFI, the microcode update blob for
+early loading can be 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
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index 6ced293d88..e1d98fa55e 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -35,6 +35,7 @@
 #include <xen/guest_access.h>
 #include <xen/earlycpio.h>
 #include <xen/watchdog.h>
+#include <xen/efi.h>
 
 #include <asm/apic.h>
 #include <asm/delay.h>
@@ -61,6 +62,7 @@
 static module_t __initdata ucode_mod;
 static signed int __initdata ucode_mod_idx;
 static bool_t __initdata ucode_mod_forced;
+static unsigned int __initdata ucode_mod_efi_idx;
 static unsigned int nr_cores;
 
 /*
@@ -105,15 +107,10 @@ static struct microcode_patch *microcode_cache;
 
 void __init microcode_set_module(unsigned int idx)
 {
-    ucode_mod_idx = idx;
+    ucode_mod_efi_idx = 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 parse_ucode(const char *s)
 {
     const char *ss;
@@ -126,18 +123,15 @@ static int __init 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 )
+            ucode_scan = val;
+        else
         {
-            if ( (val = parse_boolean("scan", s, ss)) >= 0 )
-                ucode_scan = val;
-            else
-            {
-                const char *q;
-
-                ucode_mod_idx = simple_strtol(s, &q, 0);
-                if ( q != ss )
-                    rc = -EINVAL;
-            }
+            const char *q;
+
+            ucode_mod_idx = simple_strtol(s, &q, 0);
+            if ( q != ss )
+                rc = -EINVAL;
         }
 
         s = ss + 1;
@@ -228,6 +222,16 @@ void __init microcode_grab_module(
 {
     module_t *mod = (module_t *)__va(mbi->mods_addr);
 
+    if ( efi_enabled(EFI_BOOT) )
+    {
+        if ( ucode_mod_forced ) /* Microcode specified by EFI */
+        {
+            ucode_mod = mod[ucode_mod_efi_idx];
+            return;
+        }
+        goto scan;
+    }
+
     if ( ucode_mod_idx < 0 )
         ucode_mod_idx += mbi->mods_count;
     if ( ucode_mod_idx <= 0 || ucode_mod_idx >= mbi->mods_count ||
-- 
2.17.1


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel