[RFC Patch 2/6] Introduce PPC64 specific Hardware Breakpointinterfaces

Alan Stern stern at rowland.harvard.edu
Fri May 15 06:20:04 EST 2009


On Fri, 15 May 2009, K.Prasad wrote:

> I see that you're referring to this code in __switch_to() :
>         if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
>                 set_dabr(new->thread.dabr);
> 
> arch_install_thread_hw_breakpoint()<--switch_to_thread_hw_breakpoint()
> <--__switch_to() implementation is also similar.
> 
> In __switch_to(),
>                 if (unlikely(test_tsk_thread_flag(new, TIF_DEBUG)))
>                         switch_to_thread_hw_breakpoint(new);
> 
> happens only when TIF_DEBUG flag is set. This flag is cleared when the
> process unregisters any breakpoints it had requested earlier. So, the
> set_dabr() call is avoided for processes not using the debug register.

In the x86 code, shouldn't arch_update_user_hw_breakpoint set or clear
TIF_DEBUG, depending on whether or not there are any user breakpoints
remaining?

> > > +int __kprobes hw_breakpoint_handler(struct die_args *args)
> > > +{
> > > +	int rc = NOTIFY_STOP;
> > > +	struct hw_breakpoint *bp;
> > > +	struct pt_regs *regs = args->regs;
> > > +	unsigned long dar;
> > > +	int cpu, stepped, is_kernel;
> > > +
> > > +	/* Disable breakpoints during exception handling */
> > > +	set_dabr(0);
> > > +
> > > +	dar = regs->dar & (~HW_BREAKPOINT_ALIGN);
> > > +	is_kernel = (dar >= TASK_SIZE) ? 1 : 0;
> > 
> > is_kernel_addr() ?
> > 
> 
> Ok.

Shouldn't this test hbp_kernel_pos instead?

> > > +	if (is_kernel)
> > > +		bp = hbp_kernel[0];
> > > +	else {
> > > +		bp = current->thread.hbp[0];
> > > +		/* Lazy debug register switching */
> > > +		if (!bp)
> > > +			return rc;

Shouldn't this test be moved outside the "if" statement, as in the x86 
code?

Alan Stern




More information about the Linuxppc-dev mailing list