diff mbox series

[3/3] x86/ucode: Introduce ucode=allow-same for testing purposes

Message ID 20201026172519.17881-4-andrew.cooper3@citrix.com (mailing list archive)
State New
Headers show
Series x86/ucode: Fixes and improvements to ucode revision handling | expand

Commit Message

Andrew Cooper Oct. 26, 2020, 5:25 p.m. UTC
Many CPUs will actually reload microcode when offered the same version as
currently loaded.  This allows for easy testing of the late microcode loading
path.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
CC: Juergen Gross <jgross@suse.com>
CC: Igor Druzhinin <igor.druzhinin@citrix.com>

I was hoping to make this a runtime parameter, but I honestly can't figure out
the new HYPFS-only infrastructure is supposed to work.
---
 docs/misc/xen-command-line.pandoc    | 7 ++++++-
 xen/arch/x86/cpu/microcode/amd.c     | 3 +++
 xen/arch/x86/cpu/microcode/core.c    | 4 ++++
 xen/arch/x86/cpu/microcode/intel.c   | 3 +++
 xen/arch/x86/cpu/microcode/private.h | 2 ++
 5 files changed, 18 insertions(+), 1 deletion(-)

Comments

Jan Beulich Oct. 28, 2020, 8:35 a.m. UTC | #1
On 26.10.2020 18:25, Andrew Cooper wrote:
> Many CPUs will actually reload microcode when offered the same version as
> currently loaded.  This allows for easy testing of the late microcode loading
> path.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with one nit:

> @@ -2248,6 +2248,11 @@ precedence over `scan`.
>  stop_machine context. In NMI handler, even NMIs are blocked, which is
>  considered safer. The default value is `true`.
>  
> +'allow-same' alters the default acceptance policy for new microcode to permit
> +trying to reload the same version.  Many CPUs will actually reload microcode
> +of the same version, and this allows for easily testing of the late microcode
> +loading path.

Either "easy" or drop "of"?

Jan
Jürgen Groß Oct. 28, 2020, 8:36 a.m. UTC | #2
On 26.10.20 18:25, Andrew Cooper wrote:
> Many CPUs will actually reload microcode when offered the same version as
> currently loaded.  This allows for easy testing of the late microcode loading
> path.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Wei Liu <wl@xen.org>
> CC: Juergen Gross <jgross@suse.com>
> CC: Igor Druzhinin <igor.druzhinin@citrix.com>
> 
> I was hoping to make this a runtime parameter, but I honestly can't figure out
> the new HYPFS-only infrastructure is supposed to work.

For your use case have a look into xen/arch/x86/hvm/vmx/vmcs.c how the
"ept" runtime parameter is handled.

This is a similar case where one sub-option can be modified at runtime.


Juergen
diff mbox series

Patch

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 4ae9391fcd..97b1cc67a4 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2216,7 +2216,7 @@  logic applies:
    active by default.
 
 ### ucode
-> `= List of [ <integer> | scan=<bool>, nmi=<bool> ]`
+> `= List of [ <integer> | scan=<bool>, nmi=<bool>, allow-same=<bool> ]`
 
     Applicability: x86
     Default: `nmi`
@@ -2248,6 +2248,11 @@  precedence over `scan`.
 stop_machine context. In NMI handler, even NMIs are blocked, which is
 considered safer. The default value is `true`.
 
+'allow-same' alters the default acceptance policy for new microcode to permit
+trying to reload the same version.  Many CPUs will actually reload microcode
+of the same version, and this allows for easily testing of the late microcode
+loading path.
+
 ### unrestricted_guest (Intel)
 > `= <boolean>`
 
diff --git a/xen/arch/x86/cpu/microcode/amd.c b/xen/arch/x86/cpu/microcode/amd.c
index 7d2f57c4cb..5255028af7 100644
--- a/xen/arch/x86/cpu/microcode/amd.c
+++ b/xen/arch/x86/cpu/microcode/amd.c
@@ -174,6 +174,9 @@  static enum microcode_match_result compare_revisions(
     if ( new_rev > old_rev )
         return NEW_UCODE;
 
+    if ( opt_ucode_allow_same && new_rev == old_rev )
+        return NEW_UCODE;
+
     return OLD_UCODE;
 }
 
diff --git a/xen/arch/x86/cpu/microcode/core.c b/xen/arch/x86/cpu/microcode/core.c
index 18ebc07b13..ac3ceb567c 100644
--- a/xen/arch/x86/cpu/microcode/core.c
+++ b/xen/arch/x86/cpu/microcode/core.c
@@ -95,6 +95,8 @@  static bool_t __initdata ucode_scan;
 /* By default, ucode loading is done in NMI handler */
 static bool ucode_in_nmi = true;
 
+bool __read_mostly opt_ucode_allow_same;
+
 /* Protected by microcode_mutex */
 static struct microcode_patch *microcode_cache;
 
@@ -121,6 +123,8 @@  static int __init parse_ucode(const char *s)
 
         if ( (val = parse_boolean("nmi", s, ss)) >= 0 )
             ucode_in_nmi = val;
+        else if ( (val = parse_boolean("allow-same", s, ss)) >= 0 )
+            opt_ucode_allow_same = val;
         else if ( !ucode_mod_forced ) /* Not forced by EFI */
         {
             if ( (val = parse_boolean("scan", s, ss)) >= 0 )
diff --git a/xen/arch/x86/cpu/microcode/intel.c b/xen/arch/x86/cpu/microcode/intel.c
index 5fa2821cdb..f6d01490e0 100644
--- a/xen/arch/x86/cpu/microcode/intel.c
+++ b/xen/arch/x86/cpu/microcode/intel.c
@@ -232,6 +232,9 @@  static enum microcode_match_result compare_revisions(
     if ( new_rev > old_rev )
         return NEW_UCODE;
 
+    if ( opt_ucode_allow_same && new_rev == old_rev )
+        return NEW_UCODE;
+
     /*
      * Treat pre-production as always applicable - anyone using pre-production
      * microcode knows what they are doing, and can keep any resulting pieces.
diff --git a/xen/arch/x86/cpu/microcode/private.h b/xen/arch/x86/cpu/microcode/private.h
index 9a15cdc879..c085a10268 100644
--- a/xen/arch/x86/cpu/microcode/private.h
+++ b/xen/arch/x86/cpu/microcode/private.h
@@ -3,6 +3,8 @@ 
 
 #include <asm/microcode.h>
 
+extern bool opt_ucode_allow_same;
+
 enum microcode_match_result {
     OLD_UCODE, /* signature matched, but revision id is older or equal */
     NEW_UCODE, /* signature matched, but revision id is newer */