xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xen.org>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>,
	Jan Beulich <JBeulich@suse.com>
Subject: [PATCH] x86/boot: Make alternative patching NMI-safe
Date: Mon, 5 Feb 2018 10:24:58 +0000	[thread overview]
Message-ID: <1517826298-5607-1-git-send-email-andrew.cooper3@citrix.com> (raw)

During patching, there is a very slim risk that an NMI or MCE interrupt in the
middle of altering the code in the NMI/MCE paths, in which case bad things
will happen.

The NMI risk can be eliminated by running the patching loop in NMI context, at
which point the CPU will defer further NMIs until patching is complete.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/alternative.c | 61 +++++++++++++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
index ee18e6c..521f896 100644
--- a/xen/arch/x86/alternative.c
+++ b/xen/arch/x86/alternative.c
@@ -16,6 +16,7 @@
  */
 
 #include <xen/types.h>
+#include <asm/apic.h>
 #include <asm/processor.h>
 #include <asm/alternative.h>
 #include <xen/init.h>
@@ -82,11 +83,6 @@ static const unsigned char * const p6_nops[ASM_NOP_MAX+1] init_or_livepatch_cons
 
 static const unsigned char * const *ideal_nops init_or_livepatch_data = p6_nops;
 
-static int __init mask_nmi_callback(const struct cpu_user_regs *regs, int cpu)
-{
-    return 1;
-}
-
 static void __init arch_init_ideal_nops(void)
 {
     switch ( boot_cpu_data.x86_vendor )
@@ -203,24 +199,50 @@ void init_or_livepatch apply_alternatives(const struct alt_instr *start,
 }
 
 /*
+ * At boot time, we patch alternatives in NMI context.  This means that the
+ * active NMI-shadow will defer any further NMIs, removing the slim race
+ * condition where an NMI hits while we are midway though patching some
+ * instructions in the NMI path.
+ */
+static int __init nmi_apply_alternatives(const struct cpu_user_regs *regs,
+                                         int cpu)
+{
+    static bool __initdata once;
+
+    /*
+     * More than one NMI may occur between the two set_nmi_callback() below.
+     * We only need to apply alternatives once.
+     */
+    if ( !once )
+    {
+        unsigned long cr0;
+
+        once = true;
+
+        cr0 = read_cr0();
+
+        /* Disable WP to allow patching read-only pages. */
+        write_cr0(cr0 & ~X86_CR0_WP);
+
+        apply_alternatives(__alt_instructions, __alt_instructions_end);
+
+        write_cr0(cr0);
+    }
+
+    return 1;
+}
+
+/*
  * This routine is called with local interrupt disabled and used during
  * bootup.
  */
 void __init alternative_instructions(void)
 {
     nmi_callback_t *saved_nmi_callback;
-    unsigned long cr0 = read_cr0();
 
     arch_init_ideal_nops();
 
     /*
-     * The patching is not fully atomic, so try to avoid local interruptions
-     * that might execute the to be patched code.
-     * Other CPUs are not running.
-     */
-    saved_nmi_callback = set_nmi_callback(mask_nmi_callback);
-
-    /*
      * Don't stop machine check exceptions while patching.
      * MCEs only happen when something got corrupted and in this
      * case we must do something about the corruption.
@@ -232,13 +254,14 @@ void __init alternative_instructions(void)
      */
     ASSERT(!local_irq_is_enabled());
 
-    /* Disable WP to allow application of alternatives to read-only pages. */
-    write_cr0(cr0 & ~X86_CR0_WP);
-
-    apply_alternatives(__alt_instructions, __alt_instructions_end);
+    /*
+     * As soon as the callback is set up, the next NMI will trigger patching,
+     * even an NMI ahead of our explicit self-NMI.
+     */
+    saved_nmi_callback = set_nmi_callback(nmi_apply_alternatives);
 
-    /* Reinstate WP. */
-    write_cr0(cr0);
+    /* Send ourselves an NMI to trigger the callback. */
+    self_nmi();
 
     set_nmi_callback(saved_nmi_callback);
 }
-- 
2.1.4


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

             reply	other threads:[~2018-02-05 10:24 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-05 10:24 Andrew Cooper [this message]
2018-02-05 14:09 ` [PATCH] x86/boot: Make alternative patching NMI-safe Jan Beulich
2018-02-05 15:16   ` Andrew Cooper
2018-02-05 16:20     ` Jan Beulich
2018-02-05 18:04       ` Andrew Cooper
2018-02-06  9:45         ` Jan Beulich
2018-02-05 17:57 ` Konrad Rzeszutek Wilk

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1517826298-5607-1-git-send-email-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).