From: "K.Prasad" <prasad@linux.vnet.ibm.com>
To: Alan Stern <stern@rowland.harvard.edu>,
Roland McGrath <roland@redhat.com>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
mingo@elte.hu, jason.wessel@windriver.com, avi@qumranet.com,
richardj_moore@uk.ibm.com
Subject: Re: [RFC Patch 3/9] Modifying generic debug exception to use virtual debug registers
Date: Thu, 4 Dec 2008 17:43:21 +0530 [thread overview]
Message-ID: <20081204121320.GA5207@in.ibm.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.0810181106100.1469-100000@netrider.rowland.org>
On Sat, Oct 18, 2008 at 11:21:53AM -0400, Alan Stern wrote:
> On Fri, 17 Oct 2008, Roland McGrath wrote:
>
> > Current Intel manuals say, "Certain debug exceptions may clear bits 0-3.
> > The remaining cntents of the DR6 register are never cleared by the processor."
> >
> > Your experiments told us that "certain debug exceptions" includes at least
> > the data breakpoint hits. I assume that what it really means is all the
> > exceptions that set one of those four bits, i.e. ones due to DR[0-3] use.
> > Perhaps someone from Intel can clarify exactly what it means.
> >
> > > So this means that do_debug shouldn't modify the four low bits in vdr6.
> > [...]
> >
> > Right.
> >
> > > I don't know how we should handle the BT (debug trap) and BS
> > > (single-step exception) bits. Maybe the kprobes code can take care of
> > > them.
> >
> > BT is for task switch with TSS.T set. I don't think that can ever happen
> > in Linux, since we don't use hardware task-switching. If at all, maybe in
> > vm86 mode. I don't think there's a way to do it just from user_ldt.
> >
> > I think BS (DR_STEP) should get set in vdr6 only when a SIGTRAP is
> > generated for the exception. It should never get cleared by the system,
> > only by PTRACE_POKEUSR. That is consistent with what we get now, AFAICT.
> >
> > I don't think kprobes should "take care of" DR_STEP. It should eat a
> > DR_STEP that it's responsible for, and leave any others alone. i.e.,
> > CONFIG_KPROBE=n must not break the normal bookkeeping.
> >
> > IIRC there can be one do_debug trap that's for both a breakpoint register
> > hit and a single-step (TF), with DR_STEP plus DR_TRAPn both set at once.
> > To handle that too, I think this will work:
> >
> > do_debug does:
> >
> > get_debugreg(condition, 6);
> > set_debugreg(0, 6);
> >
> > Make sure the hw_breakpoint notifier is before the kprobes notifier.
> > hw_breakpoint is responsible for the low 4 bits of vdr6 and leaves its
> > other bits alone. It returns NOTIFY_STOP iff both this hit is not a ptrace
> > hit and hardware %db6 (args->err) has no other nonreserved bits set.
>
> Ah yes, it's coming back to me now. The handler routines see the
> original hardware DR6 contents in args->err. They want to turn off the
> bits corresponding to events they take care of, leaving the remaining
> bits intact. When the notifier chain is finished, any bits still left
> in args->err have to be acted on by do_debug, by or'ing them into vdr6.
>
> The problem is that, owing to the way the code is structured, this
> can't be done. args->err is local to notify_die, so any changes made
> to its value are not available in do_debug.
>
> > kprobes stays as it, returns NOTIFY_STOP iff it's swallowing the step.
> > do_debug stays mostly the same, replace:
> >
> > tsk->thread.debugreg6 = condition;
> >
> > with:
> >
> > tsk->thread.vdr6 &= DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3;
> > tsk->thread.vdr6 |= condition & ~(DR_TRAP0|DR_TRAP1|DR_TRAP2|DR_TRAP3);
>
> No, this can't be right. Or if it is, it's just by coincidence. What
> we really want to do is:
>
> tsk->thread.vdr6 |= args->err;
>
> after notify_die() returns. Unfortunately this is impossible unless we
> change things around. For example, instead of passing condition as an
> argument to notify_die(), we could pass (long) &condition and change
> the notifier routines to use (* (unsigned *) (args->err)) instead of
> args->err.
>
> > > kprobes stays as it, returns NOTIFY_STOP iff it's swallowing the step.
> >
> > Oops, I think this breaks if there was also a ptrace db[0-3] hit in the
> > same exception. In that case, kprobes would need to not return NOTIFY_STOP
> > when it otherwise would, if thread.vdr6 has low bits set.
>
> What should happen is kprobes returns NOTIFY_STOP if there are no
> unreserved bits still set in args->err -- or (* (unsigned *)
> (args->err)) -- when it is ready to return.
>
> Alan Stern
>
Given that kprobes and HW Breakpoint exceptions multiplex only the
DIE_DEBUG notifier, kprobes tries to handle and eat-up the exception
with a NOTIFY_STOP only if it is in the context of kprobes i.e. when
kprobe_running() is true; which leaves no room for interference with a
user-space single-stepping request.
Thanks,
K.Prasad
next prev parent reply other threads:[~2008-12-04 12:15 UTC|newest]
Thread overview: 36+ messages / expand[flat|nested] mbox.gz Atom feed top
2008-10-08 19:20 [RFC Patch 0/9] Hardware Breakpoint interfaces - v2 K.Prasad
2008-10-08 19:23 ` [RFC Patch 1/9] Introducing generic hardware breakpoint handler interfaces K.Prasad
2008-10-16 2:49 ` Roland McGrath
2008-10-16 3:45 ` K.Prasad
2008-10-18 0:34 ` Roland McGrath
2008-10-16 14:38 ` Alan Stern
2008-10-17 23:58 ` Roland McGrath
2008-10-18 15:23 ` Alan Stern
2008-10-08 19:23 ` [RFC Patch 2/9] x86 architecture implementation of Hardware Breakpoint interfaces K.Prasad
2008-10-16 2:57 ` Roland McGrath
2008-10-08 19:24 ` [RFC Patch 3/9] Modifying generic debug exception to use virtual debug registers K.Prasad
2008-10-16 0:25 ` Roland McGrath
2008-10-16 14:12 ` Alan Stern
2008-10-16 19:22 ` Roland McGrath
2008-10-17 15:55 ` Alan Stern
2008-10-17 23:24 ` Roland McGrath
2008-10-17 23:27 ` Roland McGrath
2008-10-18 15:21 ` Alan Stern
2008-12-04 12:13 ` K.Prasad [this message]
2008-10-08 19:24 ` [RFC Patch 4/9] Modify kprobe exception handler to recognise single-stepping by HW Breakpoint handler K.Prasad
2008-10-08 19:25 ` [RFC Patch 5/9] Use wrapper routines around debug registers in processor related functions K.Prasad
2008-10-08 19:25 ` [RFC Patch 6/9] Use virtual debug registers in process/thread handling code K.Prasad
2008-10-16 1:44 ` Roland McGrath
2008-10-16 14:27 ` Alan Stern
2008-10-18 0:08 ` Roland McGrath
2008-10-18 15:34 ` Alan Stern
2008-12-03 4:54 ` Roland McGrath
2008-12-04 1:05 ` Roland McGrath
2008-12-04 12:23 ` K.Prasad
2008-10-08 19:26 ` [RFC Patch 7/9] Modify signal handling code to refrain from re-enabling HW Breakpoints K.Prasad
2008-10-08 19:26 ` [RFC Patch 8/9] Modify Ptrace to use wrapper routines to access breakpoint registers K.Prasad
2008-10-16 1:44 ` Roland McGrath
2008-12-04 17:30 ` K.Prasad
2008-10-08 19:27 ` [RFC Patch 9/9] Cleanup HW Breakpoint registers before kexec K.Prasad
-- strict thread matches above, loose matches on Subject: below --
2008-12-04 19:08 [RFC Patch 0/9] Hardware Breakpoint interfaces - v2 K.Prasad
2008-12-04 19:12 ` [RFC Patch 3/9] Modifying generic debug exception to use virtual debug registers K.Prasad
2008-10-07 11:38 [RFC Patch 0/9] Hardware Breakpoint interfaces K.Prasad
2008-10-07 11:42 ` [RFC Patch 3/9] Modifying generic debug exception to use virtual debug registers K.Prasad
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=20081204121320.GA5207@in.ibm.com \
--to=prasad@linux.vnet.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=avi@qumranet.com \
--cc=jason.wessel@windriver.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@elte.hu \
--cc=richardj_moore@uk.ibm.com \
--cc=roland@redhat.com \
--cc=stern@rowland.harvard.edu \
/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
Powered by JetHome