When one core is loading ucode, handling NMI on sibling threads or
on other cores in the system might be problematic. By rendezvousing
all CPUs in NMI handler, it prevents NMI acceptance during ucode
loading.
Basically, some work previously done in stop_machine context is
moved to NMI handler. Primary threads call in and load ucode in
NMI handler. Secondary threads wait for the completion of ucode
loading on all CPU cores. An option is introduced to disable this
behavior.
Control thread doesn't rendezvous in NMI handler by calling self_nmi()
(in case of unknown_nmi_error() being triggered). The side effect is
control thread might be handling an NMI while other threads are loading
ucode. If an ucode is to update something shared by a whole socket,
control thread may be accessing things that are being updating by the
ucode loading on other cores. It is not safe. Update ucode on the
control thread first to mitigate this issue.
Signed-off-by: Sergey Dyasli
Signed-off-by: Chao Gao
---
Note:
I plan to finish remaining patches (like handling parked CPU,
BDF90 and WBINVD, IMO, not important as this one) in RCs.
So this v12 only has one patch.
Changes in v12:
- take care that self NMI may not arrive synchronously.
- explain why control thread loads ucode first in patch description.
- use parse_boolean to parse "scan" field in "ucode" option. The change
is compatible with the old style.
- staticify loading_err
- drop primary_nmi_work()
Changes in v11:
- Extend existing 'nmi' option rather than use a new one.
- use per-cpu variable to store error code of xxx_nmi_work()
- rename secondary_thread_work to secondary_nmi_work.
- intialize nmi_patch to ZERO_BLOCK_PTR and make it static.
- constify nmi_cpu
- explain why control thread loads ucode first in patch description
Changes in v10:
- rewrite based on Sergey's idea and patch
- add Sergey's SOB.
- add an option to disable ucode loading in NMI handler
- don't send IPI NMI to the control thread to avoid unknown_nmi_error()
in do_nmi().
- add an assertion to make sure the cpu chosen to handle platform NMI
won't send self NMI. Otherwise, there is a risk that we encounter
unknown_nmi_error() and system crashes.
Changes in v9:
- control threads send NMI to all other threads. Slave threads will
stay in the NMI handling to prevent NMI acceptance during ucode
loading. Note that self-nmi is invalid according to SDM.
- s/rep_nop/cpu_relax
- remove debug message in microcode_nmi_callback(). Printing debug
message would take long times and control thread may timeout.
- rebase and fix conflicts
Changes in v8:
- new
---
docs/misc/xen-command-line.pandoc | 6 +-
xen/arch/x86/microcode.c | 174 +++---
xen/arch/x86/traps.c | 6 +-
xen/include/asm-x86/nmi.h | 3 +
4 files changed, 156 insertions(+), 33 deletions(-)
diff --git a/docs/misc/xen-command-line.pandoc
b/docs/misc/xen-command-line.pandoc
index fc64429..f5410b3 100644
--- a/docs/misc/xen-command-line.pandoc
+++ b/docs/misc/xen-command-line.pandoc
@@ -2053,7 +2053,7 @@ pages) must also be specified via the tbuf_size parameter.
> `= unstable | skewed | stable:socket`
### ucode (x86)
-> `= [ | scan]`
+> `= List of [ | scan=, nmi= ]`
Specify how and where to find CPU microcode update blob.
@@ -2074,6 +2074,10 @@ microcode in the cpio name space must be:
- on Intel: kernel/x86/microcode/GenuineIntel.bin
- on AMD : kernel/x86/microcode/AuthenticAMD.bin
+'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)
> `= `
diff --git a/xen/arch/x86/microcode.c b/xen/arch/x86/microcode.c
index b882ac8..3c0f72e 100644
--- a/xen/arch/x86/microcode.c
+++ b/xen/arch/x86/microcode.c
@@ -36,8 +36,10 @@
#include
#include
+#include
#include
#include
+#include
#include
#include
#include
@@ -95,6 +97,9 @@ static struct ucode_mod_blob __initdata ucode_blob;
*/
static bool_t __initdata ucode_scan;
+/* 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;
@@ -105,23 +110,40 @@ void __init microcode_set_module(unsigned int idx)
}
/*
- * The format is '[|scan]'. Both options are optional.
- * If the EFI has forced which of the multiboot payloads is to be used,
- * no parsing will be attempted.
+ * The format is '[|scan=, nmi=]'. Both options are
+ * optional. If the EFI has forced which of the multiboot payloads is to be
+ * used, only nmi= is parsed.
*/
static int __init parse_ucode(const char *s)
{
-const char *q = NULL;
+const char *ss;
+int val, rc = 0;
-if ( ucode_mod_forced ) /* Forced by EFI */
- return 0;
+do {
+ss = strchr(s, ',');
+if ( !ss )
+ss =
