Linux Documentation
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	Ingo Molnar <mingo@kernel.org>,
	x86@kernel.org, Jinchao Wang <wangjinchao600@gmail.com>,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Ian Rogers <irogers@google.com>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, linux-perf-users@vger.kernel.org
Subject: Re: [PATCH v11 01/11] x86/hw_breakpoints: Make DR7 updates NMI safe
Date: Tue, 4 Aug 2026 07:55:17 +0900	[thread overview]
Message-ID: <20260804075517.1c1c9e2dc6ff66e0e73f946d@kernel.org> (raw)
In-Reply-To: <20260803065744.GU49951@noisy.programming.kicks-ass.net>

On Mon, 3 Aug 2026 08:57:44 +0200
Peter Zijlstra <peterz@infradead.org> wrote:

> On Sun, Aug 02, 2026 at 05:18:37PM +0900, Masami Hiramatsu (Google) wrote:
> > From: Jinchao Wang <wangjinchao600@gmail.com>
> > 
> > Hardware breakpoint installation and removal run with IRQs disabled, but
> > an NMI can still enter the same code through KGDB. The interrupted
> > operation and the NMI can consequently claim the same slot or overwrite
> > each other's DR7 state.
> 
> Is KGDB really the only way to trip this? Mostly I think we let KGDB
> have the pieces if it does something 'funny'.

Hmm, I think even if so, this detection is better to be handled in
hw_breakpoint layer. And I plan to use this from kprobe events,
which is also a kind of NMI.

> 
> > diff --git a/arch/x86/kernel/hw_breakpoint.c b/arch/x86/kernel/hw_breakpoint.c
> > index f846c15f21ca..9ef24b55737f 100644
> > --- a/arch/x86/kernel/hw_breakpoint.c
> > +++ b/arch/x86/kernel/hw_breakpoint.c
> > @@ -40,6 +40,9 @@
> >  DEFINE_PER_CPU(unsigned long, cpu_dr7);
> >  EXPORT_PER_CPU_SYMBOL(cpu_dr7);
> >  
> > +/* Sequence number of the per-CPU DR7 state. */
> > +DEFINE_PER_CPU(unsigned int, cpu_dr7_seq);
> 
> > diff --git a/arch/x86/kernel/nmi.c b/arch/x86/kernel/nmi.c
> > index 3c9f60d6ca5a..f55a0cbd5927 100644
> > --- a/arch/x86/kernel/nmi.c
> > +++ b/arch/x86/kernel/nmi.c
> > @@ -532,10 +532,13 @@ enum nmi_states {
> >  static DEFINE_PER_CPU(enum nmi_states, nmi_state);
> >  static DEFINE_PER_CPU(unsigned long, nmi_cr2);
> >  static DEFINE_PER_CPU(unsigned long, nmi_dr7);
> > +static DEFINE_PER_CPU(unsigned int, nmi_dr7_seq);
> 
> This is weird, why have two distinct sequence numbers for dr7?

nmi_dr7_seq is for sequence number of operation, which is for
detecting dr7 overwrite in NMI.
nmi_dr7 is for saving the DR7.

Thanks,


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

  reply	other threads:[~2026-08-03 22:55 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-02  8:18 [PATCH v11 00/11] tracing: wprobe: x86: Add wprobe for watchpoint Masami Hiramatsu (Google)
2026-08-02  8:18 ` [PATCH v11 01/11] x86/hw_breakpoints: Make DR7 updates NMI safe Masami Hiramatsu (Google)
2026-08-03  6:57   ` Peter Zijlstra
2026-08-03 22:55     ` Masami Hiramatsu [this message]
2026-08-03 23:07       ` Peter Zijlstra
2026-08-02  8:18 ` [PATCH v11 02/11] x86/hw_breakpoints: Add arch_modify_local_hw_breakpoint_addr() API Masami Hiramatsu (Google)
2026-08-02  8:18 ` [PATCH v11 03/11] HWBP: Add modify_local_hw_breakpoint_addr() API Masami Hiramatsu (Google)
2026-08-02  8:19 ` [PATCH v11 04/11] tracing: wprobe: Add watchpoint probe event based on hardware breakpoint Masami Hiramatsu (Google)
2026-08-03  7:03   ` Peter Zijlstra
2026-08-03 23:00     ` Masami Hiramatsu
2026-08-03 23:08       ` Peter Zijlstra
2026-08-02  8:19 ` [PATCH v11 05/11] x86: hw_breakpoint: Add a kconfig to clarify when a breakpoint fires Masami Hiramatsu (Google)
2026-08-02  8:19 ` [PATCH v11 06/11] selftests: tracing: Add a basic testcase for wprobe Masami Hiramatsu (Google)
2026-08-02  8:19 ` [PATCH v11 07/11] selftests: tracing: Add syntax " Masami Hiramatsu (Google)
2026-08-02  8:19 ` [PATCH v11 08/11] tracing: wprobe: Add wprobe event trigger Masami Hiramatsu (Google)
2026-08-03  0:52   ` Masami Hiramatsu
2026-08-02  8:20 ` [PATCH v11 09/11] selftests: ftrace: Add wprobe trigger testcase Masami Hiramatsu (Google)
2026-08-02  8:20 ` [PATCH v11 10/11] tracing/wprobe: Support BTF typecast in fetchargs Masami Hiramatsu (Google)
2026-08-02  8:20 ` [PATCH v11 11/11] tracing/wprobe: Support BTF typecast in wprobe trigger command Masami Hiramatsu (Google)
2026-08-03 12:33 ` [PATCH v11 00/11] tracing: wprobe: x86: Add wprobe for watchpoint Masami Hiramatsu

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=20260804075517.1c1c9e2dc6ff66e0e73f946d@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=irogers@google.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=mingo@kernel.org \
    --cc=peterz@infradead.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=wangjinchao600@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox