All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: tglx@linutronix.de, luto@amacapital.net, peterz@infradead.org
Cc: linux-kernel@vger.kernel.org, x86@kernel.org,
	Lai Jiangshan <laijs@linux.alibaba.com>,
	sean.j.christopherson@intel.com, andrew.cooper3@citrix.com,
	daniel.thompson@linaro.org, Andy Lutomirski <luto@kernel.org>
Subject: [PATCH 4/6] x86/entry: Optimize local_db_save() for virt
Date: Thu, 28 May 2020 22:19:41 +0200	[thread overview]
Message-ID: <20200528202328.588432451@infradead.org> (raw)
In-Reply-To: 20200528201937.038455891@infradead.org

Because DRn access is 'difficult' with virt; but the DR7 read is
cheaper than a cacheline miss on native, add a virt specific
fast path to local_db_save(), such that when breakpoints are not in
use we avoid touching DRn entirely.

Suggested-by: Andy Lutomirski <luto@kernel.org>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
 arch/x86/include/asm/debugreg.h |    6 ++++++
 arch/x86/kernel/hw_breakpoint.c |   26 ++++++++++++++++++++++----
 arch/x86/kvm/vmx/nested.c       |    2 ++
 3 files changed, 30 insertions(+), 4 deletions(-)

--- a/arch/x86/include/asm/debugreg.h
+++ b/arch/x86/include/asm/debugreg.h
@@ -115,6 +115,12 @@ static inline void debug_stack_usage_dec
 
 static __always_inline void local_db_save(unsigned long *dr7)
 {
+	if (static_cpu_has(X86_FEATURE_HYPERVISOR) && !hw_breakpoint_active()) {
+		*dr7 = 0;
+		barrier();
+		return;
+	}
+
 	get_debugreg(*dr7, 7);
 	if (*dr7)
 		set_debugreg(0, 7);
--- a/arch/x86/kernel/hw_breakpoint.c
+++ b/arch/x86/kernel/hw_breakpoint.c
@@ -97,6 +97,8 @@ int arch_install_hw_breakpoint(struct pe
 	unsigned long *dr7;
 	int i;
 
+	lockdep_assert_irqs_disabled();
+
 	for (i = 0; i < HBP_NUM; i++) {
 		struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
 
@@ -115,6 +117,12 @@ int arch_install_hw_breakpoint(struct pe
 	dr7 = this_cpu_ptr(&cpu_dr7);
 	*dr7 |= encode_dr7(i, info->len, info->type);
 
+	/*
+	 * Ensure we first write cpu_dr7 before we set the DR7 register.
+	 * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+	 */
+	barrier();
+
 	set_debugreg(*dr7, 7);
 	if (info->mask)
 		set_dr_addr_mask(info->mask, i);
@@ -134,9 +142,11 @@ int arch_install_hw_breakpoint(struct pe
 void arch_uninstall_hw_breakpoint(struct perf_event *bp)
 {
 	struct arch_hw_breakpoint *info = counter_arch_bp(bp);
-	unsigned long *dr7;
+	unsigned long dr7;
 	int i;
 
+	lockdep_assert_irqs_disabled();
+
 	for (i = 0; i < HBP_NUM; i++) {
 		struct perf_event **slot = this_cpu_ptr(&bp_per_reg[i]);
 
@@ -149,12 +159,20 @@ void arch_uninstall_hw_breakpoint(struct
 	if (WARN_ONCE(i == HBP_NUM, "Can't find any breakpoint slot"))
 		return;
 
-	dr7 = this_cpu_ptr(&cpu_dr7);
-	*dr7 &= ~__encode_dr7(i, info->len, info->type);
+	dr7 = this_cpu_read(cpu_dr7);
+	dr7 &= ~__encode_dr7(i, info->len, info->type);
 
-	set_debugreg(*dr7, 7);
+	set_debugreg(dr7, 7);
 	if (info->mask)
 		set_dr_addr_mask(0, i);
+
+	/*
+	 * Ensure the write to cpu_dr7 is after we've set the DR7 register.
+	 * This ensures an NMI never see cpu_dr7 0 when DR7 is not.
+	 */
+	barrier();
+
+	this_cpu_write(cpu_dr7, dr7);
 }
 
 static int arch_bp_generic_len(int x86_len)
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -3027,6 +3027,8 @@ static int nested_vmx_check_vmentry_hw(s
 
 	/*
 	 * VMExit clears RFLAGS.IF and DR7, even on a consistency check.
+	 * XXX how is this not broken? access to cpu_dr7 ought to be with
+	 * IRQs disabled.
 	 */
 	local_irq_enable();
 	if (hw_breakpoint_active())



  parent reply	other threads:[~2020-05-28 20:27 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-28 20:19 [PATCH 0/6] x86/entry: disallow #DB more Peter Zijlstra
2020-05-28 20:19 ` [PATCH 1/6] x86/entry: Introduce local_db_{save,restore}() Peter Zijlstra
2020-05-28 20:52   ` Andrew Cooper
2020-05-28 21:15     ` Peter Zijlstra
2020-05-28 21:33       ` Peter Zijlstra
2020-05-29 17:28         ` Andy Lutomirski
2020-05-29 19:02           ` Peter Zijlstra
2020-05-28 21:36       ` Andrew Cooper
2020-05-28 20:19 ` [PATCH 2/6] x86/entry, nmi: Disable #DB Peter Zijlstra
2020-05-28 20:19 ` [PATCH 3/6] x86/entry, mce: Disallow #DB during #MC Peter Zijlstra
2020-05-28 20:19 ` Peter Zijlstra [this message]
2020-05-29 17:24   ` [PATCH 4/6] x86/entry: Optimize local_db_save() for virt Sean Christopherson
2020-05-28 20:19 ` [PATCH 5/6] x86/entry: Remove debug IDT frobbing Peter Zijlstra
2020-05-28 20:19 ` [RFC][PATCH 6/6] x86/entry: Remove DBn stacks Peter Zijlstra
2020-05-28 22:35   ` Lai Jiangshan
2020-05-28 22:37     ` Peter Zijlstra
2020-05-28 22:42 ` [PATCH 0/6] x86/entry: disallow #DB more Lai Jiangshan
2020-05-28 22:48   ` Peter Zijlstra
2020-05-28 23:05     ` Lai Jiangshan
2020-05-29  8:00       ` Peter Zijlstra

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=20200528202328.588432451@infradead.org \
    --to=peterz@infradead.org \
    --cc=andrew.cooper3@citrix.com \
    --cc=daniel.thompson@linaro.org \
    --cc=laijs@linux.alibaba.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=luto@kernel.org \
    --cc=sean.j.christopherson@intel.com \
    --cc=tglx@linutronix.de \
    --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.