Re: ppc_set_hwdebug vs ptrace_set_debugreg

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

On Wed, Dec 01, 2010 at 10:07:58AM +0530, K.Prasad wrote:
> On Mon, Nov 29, 2010 at 11:15:51AM +0100, Andreas Schwab wrote:
> > "K.Prasad" <prasad@linux.vnet.ibm.com> writes:
> > 
> > > Although ppc_set_hwdebug() can set DABR through set_dabr() in
> > > arch/powerpc/kernel/process.c, it is good to have it converted to use
> > > register_user_hw_breakpoint().
> > 
> > What do you mean with "good to have"?  It doesn't work without it unless
> > I disable PERF_EVENTS (which is the only way to disable
> > HAVE_HW_BREAKPOINT).
> > 
> > Andreas.
> >
> 
> Let me see if I can cook up a patch for this i.e. make set_dabr() invoke
> register_user_hw_breakpoint() when CONFIG_PPC_BOOK3S is defined; before I
> head out on my vacation (starting second week of this month).
> 
> Thanks,
> K.Prasad
>

Hi,
	Can you check if the following patch (compile tested only) works
for you?

Thanks,
K.Prasad
  
Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 arch/powerpc/kernel/ptrace.c |   34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Index: linux-2.6.set_hwdebug/arch/powerpc/kernel/ptrace.c
===================================================================
--- linux-2.6.set_hwdebug.orig/arch/powerpc/kernel/ptrace.c
+++ linux-2.6.set_hwdebug/arch/powerpc/kernel/ptrace.c
@@ -1316,6 +1316,11 @@ static int set_dac_range(struct task_str
 static long ppc_set_hwdebug(struct task_struct *child,
 		     struct ppc_hw_breakpoint *bp_info)
 {
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+
 	if (bp_info->version != 1)
 		return -ENOTSUPP;
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
@@ -1362,10 +1367,29 @@ static long ppc_set_hwdebug(struct task_
 
 	if (child->thread.dabr)
 		return -ENOSPC;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	if (child->thread->ptrace_bps[0])
+		return -ENOSPC;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 
 	if ((unsigned long)bp_info->addr >= TASK_SIZE)
 		return -EIO;
 
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	/* Create a new breakpoint request if one doesn't exist already */
+	hw_breakpoint_init(&attr);
+	attr.bp_addr = bp_info->addr & ~HW_BREAKPOINT_ALIGN;
+	arch_bp_generic_fields(bp_info->addr &
+				(DABR_DATA_WRITE | DABR_DATA_READ),
+							&attr.bp_type);
+
+	bp = register_user_hw_breakpoint(&attr, ptrace_triggered, task);
+	if (IS_ERR(bp))
+		return PTR_ERR(bp);
+
+	child->thread.ptrace_bps[0] = bp;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+
 	child->thread.dabr = (unsigned long)bp_info->addr;
 
 	return 1;
@@ -1395,6 +1419,16 @@ static long ppc_del_hwdebug(struct task_
 		return -EINVAL;
 	if (child->thread.dabr == 0)
 		return -ENOENT;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	/*
+	 * There is no way by which address in ptrace_bps[0] and thread.dabr
+	 * can be different. So we don't explicitly check if they're the same
+	 */
+	if (child->thread.ptrace_bps[0]) {
+		unregister_hw_breakpoint(child->thread.ptrace_bps[0]);
+		child->thread.ptrace_bps[0] = NULL;
+	}
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 
 	child->thread.dabr = 0;