LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Roland McGrath <roland@redhat.com>
To: Alan Stern <stern@rowland.harvard.edu>
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: Thu, 22 Feb 2007 18:19:03 -0800 (PST)	[thread overview]
Message-ID: <20070223021903.41B5D1800E4@magilla.sf.frob.com> (raw)
In-Reply-To: Alan Stern's message of  Wednesday, 21 February 2007 15:35:13 -0500 <Pine.LNX.4.44L0.0702211458250.3435-100000@iolanthe.rowland.org>

> 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).

You should not actually need to disable user watchpoints, because in data
watchpoints the exception comes after the instruction completes.  Only for
instruction watchpoints does the exception come before the instruction
executes, and no user watchpoints can be in the address range containing
kernel code.  

SIGTRAP both doesn't queue, and doesn't give %dr6 values in its siginfo_t.
All user watchpoints will be handled via the signal; this is the only way
ptrace can report them, and is also the utrace way of doing things.
do_debug can happen inside kernel code, and tracing of user-level tasks can
only safely do anything at the point just before returning to user mode,
where signals are handled.  So, getting to send_sigtrap in do_debug is
enough to say "one or more user debug exceptions happened".  The %dr6 value
that collects in the thread state to be seen by ptrace, or by utrace-based
things using your new facility, needs to collect all the %dr6 bits that
were set by the hardware and weren't consumed by kernel-level tracing.  An
eventual utrace-based thing might in fact have some other way to tie in so
that the event details could just be in some call made by do_debug and not
recorded in the thread's virtual %dr6 value.  But at least for ptrace, they
should collect there if it becomes possible for more than one exception to
happen while in kernel mode or in a single user instruction.  (A single
instruction can cause multiple exceptions at the hardware level.)

> 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?

> To work with ptrace, the new scheme will completely virtualize the debug
> registers.  So for example, a ptrace call might request a debugpoint in
> dr0, but it could end up that the physical register used is really dr2
> instead.  The various bits in dr6 and dr7 will be mapped in such a way
> that the entire procedure is transparent to the user.  Debugpoints 
> registered in kernelspace or by utrace won't care, of course.

I think that's a fine idea.  

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).  

> There are two things I am uncertain about: vm86 mode and kprobes.  I don't
> know anything about how either of them works.  

I know about kprobes.  I don't know about vm86, but I can read the code.


Thanks,
Roland

  parent reply	other threads:[~2007-02-23  2:19 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 [this message]
2007-02-23 16:55             ` Alan Stern
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=20070223021903.41B5D1800E4@magilla.sf.frob.com \
    --to=roland@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=prasanna@in.ibm.com \
    --cc=stern@rowland.harvard.edu \
    --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).