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

["K.Prasad" <prasad@linux.vnet.ibm.com>]

On Mon, May 18, 2009 at 12:30:41PM -0400, Alan Stern wrote:
> On Mon, 18 May 2009, K.Prasad wrote:
> 
> > > > > > +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?
> > > 
> > 
> > Testing hbp_kernel_pos should be sufficient for PPC64 with just one
> > breakpoint register. However the above code is more extensible to other
> > PowerPC implementations which have more than one breakpoint register.
> 
> Then maybe you don't want to test this at all.  Just compare the dar 
> value with each of the breakpoint addresses.  That's more like what the 
> x86 code does.
> 
> Alan Stern
>

Comparing the DAR register value with each breakpoint address is
required to determine if the exception is the cause of a breakpoint hit
and I've added the code to hw_breakpoint_handler(). With this check in
place, I find that using hbp_kernel_pos to determine kernel/user space
origin is much easier (as you suggested) and the code is modified
accordingly.

Please find the changes in the new patchset being sent.

Thanks,
K.Prasad