From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Paul McKenney <paulmck@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
"Joel Fernandes (Google)" <joel@joelfernandes.org>,
"Steven Rostedt (VMware)" <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Frederic Weisbecker <frederic@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Brian Gerst <brgerst@gmail.com>, Juergen Gross <jgross@suse.com>,
Alexandre Chartre <alexandre.chartre@oracle.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>
Subject: [RESEND][patch V3 20/23] x86/kvm/svm: Handle hardirqs proper on guest enter/exit
Date: Fri, 20 Mar 2020 19:00:16 +0100 [thread overview]
Message-ID: <20200320180034.391111270@linutronix.de> (raw)
In-Reply-To: 20200320175956.033706968@linutronix.de
SVM hard interrupt disabled state handling is interesting. It disables
interrupts via CLGI and then invokes local_irq_enable(). This is done after
context tracking. local_irq_enable() can call into the tracer and lockdep
which requires to turn RCU on and off.
Replace it with the scheme which is used when exiting to user mode which
handles tracing and the preparatory lockdep step before context tracking
and the final lockdep step afterwards. Enable interrupts with a plain STI
in the assembly code instead.
Do the reverse operation when coming out of guest mode.
This allows to move the actual entry code into the .noinstr.text section
and verify correctness with objtool.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
---
arch/x86/kvm/svm.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5767,17 +5767,26 @@ static void svm_vcpu_run(struct kvm_vcpu
x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
/*
- * Tell context tracking that this CPU is about to enter guest
- * mode. This has to be after x86_spec_ctrl_set_guest() because
- * that can take locks (lockdep needs RCU) and calls into world and
- * some more.
+ * VMENTER enables interrupts (host state), but the kernel state is
+ * interrupts disabled when this is invoked. Also tell RCU about
+ * it. This is the same logic as for exit_to_user_mode().
+ *
+ * 1) Trace interrupts on state
+ * 2) Prepare lockdep with RCU on
+ * 3) Invoke context tracking if enabled to adjust RCU state
+ * 4) Tell lockdep that interrupts are enabled
+ *
+ * This has to be after x86_spec_ctrl_set_guest() because that can
+ * take locks (lockdep needs RCU) and calls into world and some
+ * more.
*/
+ __trace_hardirqs_on();
+ lockdep_hardirqs_on_prepare(CALLER_ADDR0);
guest_enter_irqoff();
-
- /* This is wrong vs. lockdep. Will be fixed in the next step */
- local_irq_enable();
+ lockdep_hardirqs_on(CALLER_ADDR0);
asm volatile (
+ "sti \n\t"
"push %%" _ASM_BP "; \n\t"
"mov %c[rbx](%[svm]), %%" _ASM_BX " \n\t"
"mov %c[rcx](%[svm]), %%" _ASM_CX " \n\t"
@@ -5838,7 +5847,8 @@ static void svm_vcpu_run(struct kvm_vcpu
"xor %%edx, %%edx \n\t"
"xor %%esi, %%esi \n\t"
"xor %%edi, %%edi \n\t"
- "pop %%" _ASM_BP
+ "pop %%" _ASM_BP "\n\t"
+ "cli \n\t"
:
: [svm]"a"(svm),
[vmcb]"i"(offsetof(struct vcpu_svm, vmcb_pa)),
@@ -5878,14 +5888,23 @@ static void svm_vcpu_run(struct kvm_vcpu
loadsegment(gs, svm->host.gs);
#endif
#endif
+
/*
- * Tell context tracking that this CPU is back.
+ * VMEXIT disables interrupts (host state, see the CLI in the ASM
+ * above), but tracing and lockdep have them in state 'on'. Same as
+ * enter_from_user_mode().
+ *
+ * 1) Tell lockdep that interrupts are disabled
+ * 2) Invoke context tracking if enabled to reactivate RCU
+ * 3) Trace interrupts off state
*
* This needs to be done before the below as native_read_msr()
* contains a tracepoint and x86_spec_ctrl_restore_host() calls
* into world and some more.
*/
+ lockdep_hardirqs_off(CALLER_ADDR0);
guest_exit_irqoff();
+ __trace_hardirqs_off();
/*
* We do not use IBRS in the kernel. If this vCPU has used the
@@ -5907,9 +5926,6 @@ static void svm_vcpu_run(struct kvm_vcpu
reload_tss(vcpu);
- /* This is wrong vs. lockdep. Will be fixed in the next step */
- local_irq_disable();
-
x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl);
vcpu->arch.cr2 = svm->vmcb->save.cr2;
WARNING: multiple messages have this Message-ID (diff)
From: Thomas Gleixner <tglx@linutronix.de>
To: LKML <linux-kernel@vger.kernel.org>
Cc: x86@kernel.org, Paul McKenney <paulmck@kernel.org>,
Josh Poimboeuf <jpoimboe@redhat.com>,
"Joel Fernandes (Google)" <joel@joelfernandes.org>,
"Steven Rostedt (VMware)" <rostedt@goodmis.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Alexei Starovoitov <ast@kernel.org>,
Frederic Weisbecker <frederic@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Brian Gerst <brgerst@gmail.com>, Juergen Gross <jgross@suse.com>,
Alexandre Chartre <alexandre.chartre@oracle.com>,
Tom Lendacky <thomas.lendacky@amd.com>,
Paolo Bonzini <pbonzini@redhat.com>,
kvm@vger.kernel.org, Peter Zijlstra <peterz@infradead.org>
Subject: [patch V3 20/23] x86/kvm/svm: Handle hardirqs proper on guest enter/exit
Date: Fri, 20 Mar 2020 19:00:16 +0100 [thread overview]
Message-ID: <20200320180034.391111270@linutronix.de> (raw)
In-Reply-To: 20200320175956.033706968@linutronix.de
SVM hard interrupt disabled state handling is interesting. It disables
interrupts via CLGI and then invokes local_irq_enable(). This is done after
context tracking. local_irq_enable() can call into the tracer and lockdep
which requires to turn RCU on and off.
Replace it with the scheme which is used when exiting to user mode which
handles tracing and the preparatory lockdep step before context tracking
and the final lockdep step afterwards. Enable interrupts with a plain STI
in the assembly code instead.
Do the reverse operation when coming out of guest mode.
This allows to move the actual entry code into the .noinstr.text section
and verify correctness with objtool.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: kvm@vger.kernel.org
---
arch/x86/kvm/svm.c | 40 ++++++++++++++++++++++++++++------------
1 file changed, 28 insertions(+), 12 deletions(-)
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -5767,17 +5767,26 @@ static void svm_vcpu_run(struct kvm_vcpu
x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
/*
- * Tell context tracking that this CPU is about to enter guest
- * mode. This has to be after x86_spec_ctrl_set_guest() because
- * that can take locks (lockdep needs RCU) and calls into world and
- * some more.
+ * VMENTER enables interrupts (host state), but the kernel state is
+ * interrupts disabled when this is invoked. Also tell RCU about
+ * it. This is the same logic as for exit_to_user_mode().
+ *
+ * 1) Trace interrupts on state
+ * 2) Prepare lockdep with RCU on
+ * 3) Invoke context tracking if enabled to adjust RCU state
+ * 4) Tell lockdep that interrupts are enabled
+ *
+ * This has to be after x86_spec_ctrl_set_guest() because that can
+ * take locks (lockdep needs RCU) and calls into world and some
+ * more.
*/
+ __trace_hardirqs_on();
+ lockdep_hardirqs_on_prepare(CALLER_ADDR0);
guest_enter_irqoff();
-
- /* This is wrong vs. lockdep. Will be fixed in the next step */
- local_irq_enable();
+ lockdep_hardirqs_on(CALLER_ADDR0);
asm volatile (
+ "sti \n\t"
"push %%" _ASM_BP "; \n\t"
"mov %c[rbx](%[svm]), %%" _ASM_BX " \n\t"
"mov %c[rcx](%[svm]), %%" _ASM_CX " \n\t"
@@ -5838,7 +5847,8 @@ static void svm_vcpu_run(struct kvm_vcpu
"xor %%edx, %%edx \n\t"
"xor %%esi, %%esi \n\t"
"xor %%edi, %%edi \n\t"
- "pop %%" _ASM_BP
+ "pop %%" _ASM_BP "\n\t"
+ "cli \n\t"
:
: [svm]"a"(svm),
[vmcb]"i"(offsetof(struct vcpu_svm, vmcb_pa)),
@@ -5878,14 +5888,23 @@ static void svm_vcpu_run(struct kvm_vcpu
loadsegment(gs, svm->host.gs);
#endif
#endif
+
/*
- * Tell context tracking that this CPU is back.
+ * VMEXIT disables interrupts (host state, see the CLI in the ASM
+ * above), but tracing and lockdep have them in state 'on'. Same as
+ * enter_from_user_mode().
+ *
+ * 1) Tell lockdep that interrupts are disabled
+ * 2) Invoke context tracking if enabled to reactivate RCU
+ * 3) Trace interrupts off state
*
* This needs to be done before the below as native_read_msr()
* contains a tracepoint and x86_spec_ctrl_restore_host() calls
* into world and some more.
*/
+ lockdep_hardirqs_off(CALLER_ADDR0);
guest_exit_irqoff();
+ __trace_hardirqs_off();
/*
* We do not use IBRS in the kernel. If this vCPU has used the
@@ -5907,9 +5926,6 @@ static void svm_vcpu_run(struct kvm_vcpu
reload_tss(vcpu);
- /* This is wrong vs. lockdep. Will be fixed in the next step */
- local_irq_disable();
-
x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl);
vcpu->arch.cr2 = svm->vmcb->save.cr2;
next prev parent reply other threads:[~2020-03-20 22:04 UTC|newest]
Thread overview: 67+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-20 17:59 [RESEND][patch V3 00/23] x86/entry: Consolidation part II (syscalls) Thomas Gleixner
2020-03-20 17:59 ` [patch " Thomas Gleixner
2020-03-20 17:59 ` [RESEND][patch V3 01/23] rcu: Dont acquire lock in NMI handler in rcu_nmi_enter_common() Thomas Gleixner
2020-03-20 17:59 ` [patch " Thomas Gleixner
2020-03-24 15:37 ` Frederic Weisbecker
2020-03-20 17:59 ` [RESEND][patch V3 02/23] rcu: Add comments marking transitions between RCU watching and not Thomas Gleixner
2020-03-20 17:59 ` [patch " Thomas Gleixner
2020-03-24 15:38 ` Frederic Weisbecker
2020-03-20 17:59 ` [RESEND][patch V3 03/23] vmlinux.lds.h: Create section for protection against instrumentation Thomas Gleixner
2020-03-20 17:59 ` [patch " Thomas Gleixner
2020-03-24 12:26 ` [RESEND][patch " Borislav Petkov
2020-04-03 8:08 ` Alexandre Chartre
2020-03-20 18:00 ` [RESEND][patch V3 04/23] kprobes: Prevent probes in .noinstr.text section Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-23 14:00 ` Masami Hiramatsu
2020-03-23 16:03 ` Thomas Gleixner
2020-03-24 5:49 ` Masami Hiramatsu
2020-03-24 9:47 ` Thomas Gleixner
2020-03-25 13:39 ` Masami Hiramatsu
2020-03-20 18:00 ` [RESEND][patch V3 05/23] tracing: Provide lockdep less trace_hardirqs_on/off() variants Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-04-03 8:34 ` [RESEND][patch " Alexandre Chartre
2020-03-20 18:00 ` [RESEND][patch V3 06/23] bug: Annotate WARN/BUG/stackfail as noinstr safe Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-04-02 21:01 ` [RESEND][patch " Josh Poimboeuf
2020-04-02 21:34 ` Peter Zijlstra
2020-04-02 21:43 ` Josh Poimboeuf
2020-04-02 21:49 ` Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 07/23] lockdep: Prepare for noinstr sections Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 08/23] x86/entry: Mark enter_from_user_mode() noinstr Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 09/23] x86/entry/common: Protect against instrumentation Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 10/23] x86/entry: Move irq tracing on syscall entry to C-code Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 11/23] x86/entry: Move irq flags tracing to prepare_exit_to_usermode() Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 12/23] context_tracking: Ensure that the critical path cannot be instrumented Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 13/23] lib/smp_processor_id: Move it into noinstr section Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 14/23] x86/speculation/mds: Mark mds_user_clear_cpu_buffers() __always_inline Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 15/23] x86/entry/64: Check IF in __preempt_enable_notrace() thunk Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 16/23] x86/entry/64: Mark ___preempt_schedule_notrace() thunk noinstr Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 17/23] rcu/tree: Mark the idle relevant functions noinstr Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-24 16:09 ` [RESEND][patch " Paul E. McKenney
2020-03-24 19:28 ` Thomas Gleixner
2020-03-24 19:58 ` Paul E. McKenney
2020-03-20 18:00 ` [RESEND][patch V3 18/23] x86/kvm: Move context tracking where it belongs Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 19/23] x86/kvm/vmx: Add hardirq tracing to guest enter/exit Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-24 23:03 ` [RESEND][patch " Peter Zijlstra
2020-03-24 23:21 ` Thomas Gleixner
2020-03-20 18:00 ` Thomas Gleixner [this message]
2020-03-20 18:00 ` [patch V3 20/23] x86/kvm/svm: Handle hardirqs proper on " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 21/23] context_tracking: Make guest_enter/exit_irqoff() .noinstr ready Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 22/23] x86/kvm/vmx: Move guest enter/exit into .noinstr.text Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
2020-03-20 18:00 ` [RESEND][patch V3 23/23] x86/kvm/svm: " Thomas Gleixner
2020-03-20 18:00 ` [patch " Thomas Gleixner
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=20200320180034.391111270@linutronix.de \
--to=tglx@linutronix.de \
--cc=alexandre.chartre@oracle.com \
--cc=ast@kernel.org \
--cc=brgerst@gmail.com \
--cc=frederic@kernel.org \
--cc=jgross@suse.com \
--cc=joel@joelfernandes.org \
--cc=jpoimboe@redhat.com \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=paulmck@kernel.org \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=rostedt@goodmis.org \
--cc=thomas.lendacky@amd.com \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.