All of lore.kernel.org
 help / color / mirror / Atom feed
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
 

  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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.