| Message ID | 20201026172519.17881-4-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/ucode: Fixes and improvements to ucode revision handling | expand |
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
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 --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 */
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(-)