From: Andy Lutomirski <luto@kernel.org>
To: X86 ML <x86@kernel.org>, linux-kernel@vger.kernel.org
Cc: Brian Gerst <brgerst@gmail.com>,
Steven Rostedt <rostedt@goodmis.org>, Willy Tarreau <w@1wt.eu>,
Borislav Petkov <bp@alien8.de>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <peterz@infradead.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Andy Lutomirski <luto@kernel.org>
Subject: [PATCH v2 1/3] x86/entry/64: Refactor IRQ stacks and make them NMI-safe
Date: Fri, 24 Jul 2015 22:57:04 -0700 [thread overview]
Message-ID: <13a3ceb8d36e5fa77b3b9e7a2006245a72b25f5d.1437803711.git.luto@kernel.org> (raw)
In-Reply-To: <cover.1437803711.git.luto@kernel.org>
In-Reply-To: <cover.1437803711.git.luto@kernel.org>
This will allow IRQ stacks to nest inside NMIs or similar entries
that can happen during IRQ stack setup or teardown.
The Xen code here has a confusing comment.
The new macros won't work correctly if they're invoked with IRQs on.
Add a check under CONFIG_DEBUG_ENTRY to detect that.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
arch/x86/Kconfig.debug | 2 -
arch/x86/entry/entry_64.S | 89 +++++++++++++++++++++++++++++---------------
arch/x86/kernel/process_64.c | 4 ++
3 files changed, 63 insertions(+), 32 deletions(-)
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index d8c0d3266173..4196e2325971 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -305,8 +305,6 @@ config DEBUG_ENTRY
Some of these sanity checks may slow down kernel entries and
exits or otherwise impact performance.
- This is currently used to help test NMI code.
-
If unsure, say N.
config DEBUG_NMI_SELFTEST
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index d3033183ed70..3227fb6a5b94 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -490,6 +490,56 @@ ENTRY(irq_entries_start)
.endr
END(irq_entries_start)
+.macro DEBUG_ENTRY_ASSERT_IRQS_OFF
+#ifdef CONFIG_DEBUG_ENTRY
+ pushfq
+ testl $X86_EFLAGS_IF, (%rsp)
+ jz 693f
+ ud2
+693:
+ addq $8, %rsp
+#endif
+.endm
+
+/*
+ * Enters the IRQ stack if we're not already using it. NMI-safe. Clobbers
+ * flags and puts old RSP into old_rsp, and leaves all other GPRs alone.
+ * Requires kernel GSBASE.
+ *
+ * The invariant is that, if irq_count != 1, then the IRQ stack is in use.
+ */
+.macro ENTER_IRQ_STACK old_rsp
+ DEBUG_ENTRY_ASSERT_IRQS_OFF
+ movq %rsp, \old_rsp
+ incl PER_CPU_VAR(irq_count)
+
+ /*
+ * Right now, if we just incremented irq_count to zero, we've
+ * claimed the IRQ stack but we haven't switched to it yet.
+ * Anything that can interrupt us here without using IST
+ * must be *extremely* careful to limit its stack usage.
+ */
+
+ cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp
+ pushq \old_rsp
+.endm
+
+/*
+ * Undoes ENTER_IRQ_STACK.
+ */
+.macro LEAVE_IRQ_STACK
+ DEBUG_ENTRY_ASSERT_IRQS_OFF
+ /* We need to be off the IRQ stack before decrementing irq_count. */
+ popq %rsp
+
+ /*
+ * As in ENTER_IRQ_STACK, irq_count == 0, we are still claiming
+ * the irq stack but we're not on it.
+ */
+
+ decl PER_CPU_VAR(irq_count)
+.endm
+
/*
* Interrupt entry/exit.
*
@@ -518,17 +568,7 @@ END(irq_entries_start)
#endif
1:
- /*
- * Save previous stack pointer, optionally switch to interrupt stack.
- * irq_count is used to check if a CPU is already on an interrupt stack
- * or not. While this is essentially redundant with preempt_count it is
- * a little cheaper to use a separate counter in the PDA (short of
- * moving irq_enter into assembly, which would be too much work)
- */
- movq %rsp, %rdi
- incl PER_CPU_VAR(irq_count)
- cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp
- pushq %rdi
+ ENTER_IRQ_STACK old_rsp=%rdi
/* We entered an interrupt context - irqs are off: */
TRACE_IRQS_OFF
@@ -548,10 +588,8 @@ common_interrupt:
ret_from_intr:
DISABLE_INTERRUPTS(CLBR_NONE)
TRACE_IRQS_OFF
- decl PER_CPU_VAR(irq_count)
- /* Restore saved previous stack */
- popq %rsp
+ LEAVE_IRQ_STACK
testb $3, CS(%rsp)
jz retint_kernel
@@ -863,14 +901,9 @@ bad_gs:
/* Call softirq on interrupt stack. Interrupts are off. */
ENTRY(do_softirq_own_stack)
- pushq %rbp
- mov %rsp, %rbp
- incl PER_CPU_VAR(irq_count)
- cmove PER_CPU_VAR(irq_stack_ptr), %rsp
- push %rbp /* frame pointer backlink */
+ ENTER_IRQ_STACK old_rsp=%r11
call __do_softirq
- leaveq
- decl PER_CPU_VAR(irq_count)
+ LEAVE_IRQ_STACK
ret
END(do_softirq_own_stack)
@@ -889,25 +922,21 @@ idtentry xen_hypervisor_callback xen_do_hypervisor_callback has_error_code=0
* So, on entry to the handler we detect whether we interrupted an
* existing activation in its critical region -- if so, we pop the current
* activation and restart the handler using the previous one.
+ *
+ * XXX: I have no idea what this comment is talking about. --luto
*/
ENTRY(xen_do_hypervisor_callback) /* do_hypervisor_callback(struct *pt_regs) */
-
+ ENTER_IRQ_STACK old_rsp=%r11
/*
* Since we don't modify %rdi, evtchn_do_upall(struct *pt_regs) will
* see the correct pointer to the pt_regs
*/
- movq %rdi, %rsp /* we don't return, adjust the stack frame */
-11: incl PER_CPU_VAR(irq_count)
- movq %rsp, %rbp
- cmovzq PER_CPU_VAR(irq_stack_ptr), %rsp
- pushq %rbp /* frame pointer backlink */
call xen_evtchn_do_upcall
- popq %rsp
- decl PER_CPU_VAR(irq_count)
+ LEAVE_IRQ_STACK
#ifndef CONFIG_PREEMPT
call xen_maybe_preempt_hcall
#endif
- jmp error_exit
+ ret
END(xen_do_hypervisor_callback)
/*
diff --git a/arch/x86/kernel/process_64.c b/arch/x86/kernel/process_64.c
index 0831ba3bcf95..70aa5917456e 100644
--- a/arch/x86/kernel/process_64.c
+++ b/arch/x86/kernel/process_64.c
@@ -280,6 +280,10 @@ __switch_to(struct task_struct *prev_p, struct task_struct *next_p)
unsigned fsindex, gsindex;
fpu_switch_t fpu_switch;
+#ifdef CONFIG_DEBUG_ENTRY
+ WARN_ON_ONCE(this_cpu_read(irq_count) != -1);
+#endif
+
fpu_switch = switch_fpu_prepare(prev_fpu, next_fpu, cpu);
/* We must save %fs and %gs before load_TLS() because
--
2.4.3
next prev parent reply other threads:[~2015-07-25 5:57 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-07-25 5:57 [PATCH v2 0/3] x86_64: Make int3 non-magical Andy Lutomirski
2015-07-25 5:57 ` Andy Lutomirski [this message]
2015-07-26 6:05 ` [PATCH v2 1/3] x86/entry/64: Refactor IRQ stacks and make them NMI-safe Borislav Petkov
2015-07-26 18:50 ` Andy Lutomirski
2015-07-28 9:41 ` Borislav Petkov
2015-08-17 11:18 ` Andy Shevchenko
2015-08-19 15:50 ` Andy Lutomirski
2015-07-25 5:57 ` [PATCH v2 2/3] x86/entry/64: Teach idtentry to use the IRQ stack Andy Lutomirski
2015-07-28 9:50 ` Borislav Petkov
2015-07-25 5:57 ` [PATCH v2 3/3] x86/entry/64: Move #BP from IST to " Andy Lutomirski
2015-07-28 9:54 ` Borislav Petkov
2015-07-29 17:57 ` Andy Lutomirski
2015-07-30 4:19 ` Borislav Petkov
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=13a3ceb8d36e5fa77b3b9e7a2006245a72b25f5d.1437803711.git.luto@kernel.org \
--to=luto@kernel.org \
--cc=bp@alien8.de \
--cc=brgerst@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=w@1wt.eu \
--cc=x86@kernel.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
all inboxes | Powered by JetHome®