lkml.org 
[lkml]   [2009]   [May]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [Patch 1/2] Improvements and minor fixes to HW Breakpoint interface
    On Wed, May 20, 2009 at 10:04:09AM -0400, Alan Stern wrote:
    > On Wed, 20 May 2009, K.Prasad wrote:
    >
    > > --- linux-2.6-tip.hbkpt.orig/arch/x86/kernel/hw_breakpoint.c
    > > +++ linux-2.6-tip.hbkpt/arch/x86/kernel/hw_breakpoint.c
    > > @@ -296,6 +296,19 @@ void arch_flush_thread_hw_breakpoint(str
    > >
    > > /*
    > > * Handle debug exception notifications.
    > > + *
    > > + * Return value is either NOTIFY_STOP or NOTIFY_DONE as explained below.
    > > + *
    > > + * NOTIFY_DONE returned if one of the following conditions is true.
    > > + * i) When the causative address is from user-space and the exception
    > > + * is a valid one i.e. not triggered a result of lazy debug register
    > --------------------^----^ missing ',' characters
    > ---------------------------------------^ missing "as"
    >
    > > + * switching
    > > + * ii) When there are more bits than trap<n> set in DR6 register (such
    > > + * as BD, BS or BT) indicating that more than one debug condition is
    > > + * met and requires some more action in do_debug().
    > > + *
    > > + * NOTIFY_STOP returned for all other cases
    > > + *
    > > */
    > > int __kprobes hw_breakpoint_handler(struct die_args *args)
    > > {
    > > @@ -346,8 +359,10 @@ int __kprobes hw_breakpoint_handler(stru
    > > * or due to the delay between updates of hbp_kernel_pos
    > > * and this_hbp_kernel.
    > > */
    > > - if (!bp)
    > > + if (!bp) {
    > > + rc = NOTIFY_STOP;
    > > continue;
    > > + }
    > >
    > > (bp->triggered)(bp, args->regs);
    >
    > This is the same mistake as before. If rc had previously been set to
    > NOTIFY_DONE then you will unconditionally change it back to
    > NOTIFY_STOP. You should make the code do what the comment says, set rc
    > to NOTIFY_DONE when a non-NULL user breakpoint occurs:
    >
    > if (i >= hbp_kernel_pos) {
    > bp = per_cpu(this_hbp_kernel[i], cpu);
    > } else {
    > bp = current->thread.hbp[i];
    > if (bp)
    > rc = NOTIFY_DONE;
    > }
    > /*
    > * bp can be NULL due to lazy debug register switching
    > * or due to the delay between updates of hbp_kernel_pos
    > * and this_hbp_kernel.
    > */
    > if (!bp)
    > continue;
    >
    > (bp->triggered)(bp, args->regs);
    >
    >
    > The rest of the changes are okay.
    >
    > Alan Stern
    >

    Thanks. I am pasting the hw_breakpoint_handler() function completely
    below (as opposed to patch form which may be difficult to follow). Upon
    your consent with the changes, I intend to send a consolidated patchset
    to Ingo (merging the patches over hw_breakpoint.c) for inclusion in -tip
    tree.

    I have also moved the "(*dr6_p) &= ~(DR_TRAP0 << i)" before the "if (!bp)"
    check.

    /*
    * Handle debug exception notifications.
    *
    * Return value is either NOTIFY_STOP or NOTIFY_DONE as explained below.
    *
    * NOTIFY_DONE returned if one of the following conditions is true.
    * i) When the causative address is from user-space and the exception
    * is a valid one, i.e. not triggered as a result of lazy debug register
    * switching
    * ii) When there are more bits than trap<n> set in DR6 register (such
    * as BD, BS or BT) indicating that more than one debug condition is
    * met and requires some more action in do_debug().
    *
    * NOTIFY_STOP returned for all other cases
    *
    */
    int __kprobes hw_breakpoint_handler(struct die_args *args)
    {
    int i, cpu, rc = NOTIFY_STOP;
    struct hw_breakpoint *bp;
    unsigned long dr7, dr6;
    unsigned long *dr6_p;

    /* The DR6 value is pointed by args->err */
    dr6_p = (unsigned long *)ERR_PTR(args->err);
    dr6 = *dr6_p;

    /* Do an early return if no trap bits are set in DR6 */
    if ((dr6 & DR_TRAP_BITS) == 0)
    return NOTIFY_DONE;

    /* Lazy debug register switching */
    if (!test_tsk_thread_flag(current, TIF_DEBUG))
    arch_uninstall_thread_hw_breakpoint();

    get_debugreg(dr7, 7);
    /* Disable breakpoints during exception handling */
    set_debugreg(0UL, 7);
    /*
    * Assert that local interrupts are disabled
    * Reset the DRn bits in the virtualized register value.
    * The ptrace trigger routine will add in whatever is needed.
    */
    current->thread.debugreg6 &= ~DR_TRAP_BITS;
    cpu = get_cpu();

    /* Handle all the breakpoints that were triggered */
    for (i = 0; i < HBP_NUM; ++i) {
    if (likely(!(dr6 & (DR_TRAP0 << i))))
    continue;
    /*
    * Find the corresponding hw_breakpoint structure and
    * invoke its triggered callback.
    */
    if (i >= hbp_kernel_pos)
    bp = per_cpu(this_hbp_kernel[i], cpu);
    else {
    bp = current->thread.hbp[i];
    if (bp)
    rc = NOTIFY_DONE;
    }

    /*
    * Reset the 'i'th TRAP bit in dr6 to denote completion of
    * exception handling
    */
    (*dr6_p) &= ~(DR_TRAP0 << i);
    /*
    * bp can be NULL due to lazy debug register switching
    * or due to the delay between updates of hbp_kernel_pos
    * and this_hbp_kernel.
    */
    if (!bp)
    continue;

    (bp->triggered)(bp, args->regs);
    }
    if (dr6 & (~DR_TRAP_BITS))
    rc = NOTIFY_DONE;

    set_debugreg(dr7, 7);
    put_cpu_no_resched();
    return rc;
    }

    Thanks,
    K.Prasad


    \
     
     \ /
      Last update: 2009-05-20 18:47    [W:0.030 / U:30.528 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site