Re: [PATCH v11 01/11] x86/hw_breakpoints: Make DR7 updates NMI safe

[Google]

On Tue, 4 Aug 2026 01:07:23 +0200
Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:

> On Tue, Aug 04, 2026 at 07:55:17AM +0900, Masami Hiramatsu wrote:
> > On Mon, 3 Aug 2026 08:57:44 +0200
> > Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> > 
> > > On Sun, Aug 02, 2026 at 05:18:37PM +0900, Masami Hiramatsu (Google) wrote:
> > > > From: Jinchao Wang <wangjinchao600@xxxxxxxxx>
> > > > 
> > > > 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.
> 
> I mean cpu_dr7_seq and nmi_dr7_seq.

Oops, Indeed. Let me fix it. Thanks!



-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>