mirror of https://lore.kernel.org/lkml/
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome