LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Alan Stern <stern@rowland.harvard.edu>
To: Roland McGrath <roland@redhat.com>
Cc: Prasanna S Panchamukhi <prasanna@in.ibm.com>,
	Kernel development list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] Kwatch: kernel watchpoints using CPU debug registers
Date: Fri, 23 Feb 2007 11:55:01 -0500 (EST)	[thread overview]
Message-ID: <Pine.LNX.4.44L0.0702231137190.6241-100000@netrider.rowland.org> (raw)
In-Reply-To: <20070223021903.41B5D1800E4@magilla.sf.frob.com>

On Thu, 22 Feb 2007, Roland McGrath wrote:

> > Yes, you are wrong -- although perhaps you shouldn't be.
> > 
> > The current version of do_debug() clears dr7 when a debug interrupt is 
> > fielded.  As a result, if a system call touches two watchpoint addresses 
> > in userspace only the first access will be noticed.
> 
> Ah, I see.  I think it would indeed be nice to fix this.
> 
> > This is probably a bug in do_debug().  It would be better to disable each
> > individual userspace watchpoint as it is triggered (or even not to disable
> > it at all).  dr7 would be restored when the SIGTRAP is delivered.  (But
> > what if the user is blocking or ignoring SIGTRAP?)
> 
> The user blocking or ignoring it doesn't come up, because it's a
> force_sig_info call.  However, a debugger will indeed swallow the signal
> through ptrace/utrace means.  In ptrace, the dr7 is always going to get
> reset because there will always be a context switch out and back in that
> does it.  But with utrace it's now possible to swallow the signal and keep
> going without a context switch (e.g. a breakpoint that is just doing
> logging but not stopping).  So perhaps we should have a TIF_RESTORE_DR7
> that goes into _TIF_WORK_MASK and gets handled in do_notify_resume
> (or maybe it's TIF_HWBKPT).

I think the best approach will be not to reset dr7 at all.  Then there 
won't be any need to worry about restoring it.  Leaving a userspace 
watchpoint enabled while running in the kernel ought not to matter; a 
system call shouldn't touch any address in userspace more than once or 
twice.

> > I've worked out a plan for implementing combined user/kernel mode
> > breakpoints and watchpoints (call them "debugpoints" as a catch-all
> > term).  It should work transparently with ptrace and should accomodate
> > whatever scheme utrace decides to adopt.  There won't need to be a
> > separate kwatch facility on top of it; the new combined facility will
> > handle debugpoints in both userspace and kernelspace.
> 
> That sounds great.  I'm not thrilled with the name "debugpoint", I have to
> tell you.  The hardware documentation calls all these things "breakpoints",
> and I think "data breakpoint" and "instruction breakpoint" are pretty good
> terms.  How about "hwbkpt" for the facility API?

Okay, I'll change the name.

> The one caveat I have here is that I don't want ptrace (via utrace) to have
> to supply the usual structure.  I probably only think this because it would
> be a pain for the ptrace/utrace implementation to find a place to stick it.
> But I have a rationalization.  The old ptrace interface, and the
> utrace_regset for debugregs, is not really a "debugpoint user" in the sense
> you're defining it.  It's an access to the "raw" debugregs as part of the
> thread's virtual CPU context.  You can use ptrace to set a watchpoint, then
> detach ptrace, and the thread will get a SIGTRAP later though there is no
> remnant at that point of the debugger interface that made it come about.
> For the degenerate case of medium-high priority with no handler callbacks
> (that should instead be an error at registration time if no slot is free),
> you shouldn't really need any per-caller storage (there can only be one
> such caller per slot).  

My idea was to put 4 hwbkpt structures in thread_struct, so they would
always be available for use by ptrace.  However it turned out not to be
feasible to replace the debugreg array with something more sophisticated,
because of conflicting declarations and problems with the ordering of
#includes.  So instead I have been forced to replace debugreg[] with a
pointer to a structure which can be allocated as needed.

This raises the possibility that a PTRACE syscall might fail because the 
allocation fails.  Hopefully that won't be an issue?

Alan Stern


  reply	other threads:[~2007-02-23 16:55 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20070207025008.1B11118005D@magilla.sf.frob.com>
2007-02-07 19:22 ` Alan Stern
2007-02-07 22:08   ` Bob Copeland
2007-02-09 10:21   ` Roland McGrath
2007-02-09 15:54     ` Alan Stern
2007-02-09 23:31       ` Roland McGrath
2007-02-10  4:32         ` Alan Stern
2007-02-18  3:03           ` Roland McGrath
2007-02-21 20:35         ` Alan Stern
2007-02-22 11:43           ` S. P. Prasanna
2007-02-23  2:19           ` Roland McGrath
2007-02-23 16:55             ` Alan Stern [this message]
2007-02-24  0:08               ` Roland McGrath
2007-03-02 17:19                 ` [RFC] hwbkpt: Hardware breakpoints (was Kwatch) Alan Stern
2007-03-05  7:01                   ` Roland McGrath
2007-03-05 13:36                     ` Christoph Hellwig
2007-03-05 16:16                       ` Alan Stern
2007-03-05 16:49                         ` Christoph Hellwig
2007-03-05 22:04                         ` Roland McGrath
2007-03-05 17:25                     ` Alan Stern
2007-03-06  3:13                       ` Roland McGrath
2007-03-06 15:23                         ` Alan Stern
2007-03-07  3:49                           ` Roland McGrath
2007-03-07 19:11                             ` Alan Stern
2007-03-09  6:52                               ` Roland McGrath
2007-03-09 18:40                                 ` Alan Stern
2007-03-13  8:00                                   ` Roland McGrath
2007-03-13 13:07                                     ` Alan Cox
2007-03-13 18:56                                     ` Alan Stern
2007-03-14  3:00                                       ` Roland McGrath
2007-03-14 19:11                                         ` Alan Stern
2007-03-28 21:39                                           ` Roland McGrath
2007-03-29 21:35                                             ` Alan Stern
2007-04-13 21:09                                             ` Alan Stern
2007-05-11 15:25                                             ` Alan Stern
2007-05-13 10:39                                               ` Roland McGrath
2007-05-14 15:42                                                 ` Alan Stern
2007-05-14 21:25                                                   ` Roland McGrath
2007-05-16 19:03                                                     ` Alan Stern
2007-05-23  8:47                                                       ` Roland McGrath
2007-06-01 19:39                                                         ` Alan Stern
2007-06-14  6:48                                                           ` Roland McGrath
2007-06-19 20:35                                                             ` Alan Stern
2007-06-25 10:52                                                               ` Roland McGrath
2007-06-25 15:36                                                                 ` Alan Stern
2007-06-26 20:49                                                                   ` Roland McGrath
2007-06-27  3:26                                                                     ` Alan Stern
2007-06-27 21:04                                                                       ` Roland McGrath
2007-06-29  3:00                                                                         ` Alan Stern
2007-07-11  6:59                                                                           ` Roland McGrath
2007-06-28  3:02                                                                       ` Roland McGrath
2007-06-25 11:32                                                               ` Roland McGrath
2007-06-25 15:37                                                                 ` Alan Stern
2007-06-25 20:51                                                                 ` Alan Stern
2007-06-26 18:17                                                                   ` Roland McGrath
2007-06-27  2:43                                                                     ` Alan Stern
2007-05-17 20:39                                                 ` Alan Stern
2007-03-16 21:07                                         ` Alan Stern
2007-03-22 19:44                                         ` Alan Stern
     [not found] <20070206042153.66AB418005D@magilla.sf.frob.com>
2007-02-06 19:58 ` [PATCH] Kwatch: kernel watchpoints using CPU debug registers Alan Stern
2007-02-07  2:56   ` Roland McGrath
2007-01-16 16:55 Alan Stern
2007-01-16 23:35 ` Christoph Hellwig
2007-01-17 16:33   ` Alan Stern
2007-01-17  9:44 ` Ingo Molnar
2007-01-17 16:17   ` Alan Stern
2007-01-18  0:12     ` Christoph Hellwig
2007-01-18  7:31       ` Ingo Molnar
2007-01-18 15:37         ` Alan Stern
2007-01-18 22:33         ` Christoph Hellwig
2007-02-06  4:25     ` Roland McGrath

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=Pine.LNX.4.44L0.0702231137190.6241-100000@netrider.rowland.org \
    --to=stern@rowland.harvard.edu \
    --cc=linux-kernel@vger.kernel.org \
    --cc=prasanna@in.ibm.com \
    --cc=roland@redhat.com \
    --subject='Re: [PATCH] Kwatch: kernel watchpoints using CPU debug registers' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).