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: Fri,  9 Feb 2007 02:21:24 -0800 (PST)	[thread overview]
Message-ID: <20070209102124.1DF3618003C@magilla.sf.frob.com> (raw)
In-Reply-To: Alan Stern's message of  Wednesday, 7 February 2007 14:22:13 -0500 <Pine.LNX.4.44L0.0702071356580.4502-100000@iolanthe.rowland.org>

> All right.  However this means thread_struct will have to grow in order to
> hold each task's debug-register allocations and priorities.  Would that be
> acceptable?  (This might be a good reason to keep the number of bits
> down.)

Well, noone seems to mind the wasted debugreg[5..6] words. ;-) 

I'm inclined to make thread_struct smaller than it is now by making it
indirect (debugreg[8] -> one pointer).  It feels like this would be pretty
safe now that we have TIF_DEBUG anyway.  Already nothing should be looking
at the field when TIF_DEBUG isn't set.

> Another question: How can a program using the ptrace API ever give up a
> debug-register allocation?  Disabling the register isn't sufficient; a
> user program should be able to store a watchpoint address in dr1, enable
> it in dr7, disable it in dr7, and then re-enable it in the expectation
> that the address stored in dr1 hasn't been overwritten.  As far as I can
> see, ptrace-type allocations have to be permanent (until the task exits or
> execs, or uses some other to-be-determined API to do the de-allocation.)

I hadn't really thought about this before, but it's pretty straightforward.
Each allocation is for one of %dr[0-3] and for its associated bits in %dr7.
%dr0 and bits 0,1,16-19; %dr1 and bits 2,3,20-23; etc.
%dr7 & (0x000f0003 << N) for %drN, I guess it is.
((((1 << DR_CONTROL_SIZE) - 1) << DR_CONTROL_SHIFT) |
 ((1 << DR_ENABLE_SIZE) - 1)) << N, I should say.

Each allocation owns those 38 bits (70 bits on x86_64).  When all those
bits are zero, the allocation is not active and might as well not be there
(except for whatever semantics you might want to have about an allocation's
lifetime as distinct from the event of resetting its contents).  

gdb already works this way: when it removes a watchpoint, it clears drN to
zero when it zeros all the corresponding bits in dr7.  (In fact it's in a
separate call after changing dr7, because of the ptrace interface.)

Your question made me think about the %dr6 issue, too, which I also hadn't
thought about before.  It looks like your patch handles this correctly, but
it's a subtle point that I think warrants some comments in the code.  When
userland inserts a watchpoint and it's hit, it gets a SIGTRAP via do_debug
and eventually looks at dr6 (via ptrace) to see what happened.  Kernel
watchpoints that come along after the user signal is generated can clobber
the CPU %dr6 with new hits, but userland should not perceive this.  This
works out because what userland sees is thread.debugreg[6], the only place
that sets it is do_debug, and a kwatch hit bails out before changing it.
Any other new users of the debugreg sharing interface need to be cognizant
of the %dr6 issue to avoid stepping on old ptrace uses.


Thanks,
Roland

  parent reply	other threads:[~2007-02-09 10:21 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 [this message]
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
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=20070209102124.1DF3618003C@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).