[PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[K.Prasad]

While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
user-space debuggers (like GDB) who may want to use it. Hence we introduce a
new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
"features" member of "struct ppc_debug_info" to advertise support for the
same on Book3E PowerPC processors.

Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/ptrace.h |    1 +
 arch/powerpc/kernel/ptrace.c      |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 48223f9..cf014f9 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -380,6 +380,7 @@ struct ppc_debug_info {
 #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x0000000000000002
 #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x0000000000000004
 #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x0000000000000008
+#define PPC_DEBUG_FEATURE_DATA_BP_EXACT		0x0000000000000010
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 18d28b6..71db5a6 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1636,6 +1636,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
 		dbginfo.features |=
 				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
+				   PPC_DEBUG_FEATURE_DATA_BP_EXACT |
 				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
 #endif
 #else /* !CONFIG_PPC_ADV_DEBUG_REGS */
-- 
1.7.4.1

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[David Gibson]

On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> 
> While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> "features" member of "struct ppc_debug_info" to advertise support for the
> same on Book3E PowerPC processors.

I thought the idea was that the BP_EXACT mode was the default - if the
new interface was supported at all, then BP_EXACT was always
supported.  So, why do you need a new flag?

> 
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
> ---
>  arch/powerpc/include/asm/ptrace.h |    1 +
>  arch/powerpc/kernel/ptrace.c      |    1 +
>  2 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
> index 48223f9..cf014f9 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -380,6 +380,7 @@ struct ppc_debug_info {
>  #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x0000000000000002
>  #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x0000000000000004
>  #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x0000000000000008
> +#define PPC_DEBUG_FEATURE_DATA_BP_EXACT		0x0000000000000010
>  
>  #ifndef __ASSEMBLY__
>  
> diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
> index 18d28b6..71db5a6 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -1636,6 +1636,7 @@ long arch_ptrace(struct task_struct *child, long request,
>  #ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
>  		dbginfo.features |=
>  				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_DATA_BP_EXACT |
>  				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
>  #endif
>  #else /* !CONFIG_PPC_ADV_DEBUG_REGS */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[K.Prasad]

On Tue, Aug 23, 2011 at 03:09:31PM +1000, David Gibson wrote:
> On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> > 
> > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > "features" member of "struct ppc_debug_info" to advertise support for the
> > same on Book3E PowerPC processors.
> 
> I thought the idea was that the BP_EXACT mode was the default - if the
> new interface was supported at all, then BP_EXACT was always
> supported.  So, why do you need a new flag?
> 

Yes, BP_EXACT was always supported but not advertised through
PPC_PTRACE_GETHWDBGINFO. We're now doing that.

Thanks,
K.Prasad

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[David Gibson]

On Tue, Aug 23, 2011 at 02:57:56PM +0530, K.Prasad wrote:
> On Tue, Aug 23, 2011 at 03:09:31PM +1000, David Gibson wrote:
> > On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> > > 
> > > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > > "features" member of "struct ppc_debug_info" to advertise support for the
> > > same on Book3E PowerPC processors.
> > 
> > I thought the idea was that the BP_EXACT mode was the default - if the
> > new interface was supported at all, then BP_EXACT was always
> > supported.  So, why do you need a new flag?
> > 
> 
> Yes, BP_EXACT was always supported but not advertised through
> PPC_PTRACE_GETHWDBGINFO. We're now doing that.

I can see that.  But you haven't answered why.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[Thiago Jung Bauermann]

On Wed, 2011-08-24 at 14:00 +1000, David Gibson wrote:
> On Tue, Aug 23, 2011 at 02:57:56PM +0530, K.Prasad wrote:
> > On Tue, Aug 23, 2011 at 03:09:31PM +1000, David Gibson wrote:
> > > On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> > > > 
> > > > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > > > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > > > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > > > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > > > "features" member of "struct ppc_debug_info" to advertise support for the
> > > > same on Book3E PowerPC processors.
> > > 
> > > I thought the idea was that the BP_EXACT mode was the default - if the
> > > new interface was supported at all, then BP_EXACT was always
> > > supported.  So, why do you need a new flag?
> > > 
> > 
> > Yes, BP_EXACT was always supported but not advertised through
> > PPC_PTRACE_GETHWDBGINFO. We're now doing that.
> 
> I can see that.  But you haven't answered why.

BookS doesn't support BP_EXACT, that's why I suggested this flag.

A BP_EXACT watchpoint triggers only when there's a memory access exactly
at the given address. It doesn't trigger when there's (for example) a
4-byte write at an address immediately before which also changes the
memory contents of the byte watched by the BP_EXACT watchpoint. a ranged
watchpoint would trigger, so the semantics are different.

As a general rule, GDB only sets ranged watchpoints and only uses
BP_EXACT ones when the user sets a flag. I want GDB to fail when the
user sets the flag on BookS since it can't provide the feature.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[David Gibson]

On Wed, Aug 24, 2011 at 09:41:43PM -0300, Thiago Jung Bauermann wrote:
> On Wed, 2011-08-24 at 14:00 +1000, David Gibson wrote:
> > On Tue, Aug 23, 2011 at 02:57:56PM +0530, K.Prasad wrote:
> > > On Tue, Aug 23, 2011 at 03:09:31PM +1000, David Gibson wrote:
> > > > On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> > > > > 
> > > > > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > > > > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > > > > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > > > > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > > > > "features" member of "struct ppc_debug_info" to advertise support for the
> > > > > same on Book3E PowerPC processors.
> > > > 
> > > > I thought the idea was that the BP_EXACT mode was the default - if the
> > > > new interface was supported at all, then BP_EXACT was always
> > > > supported.  So, why do you need a new flag?
> > > > 
> > > 
> > > Yes, BP_EXACT was always supported but not advertised through
> > > PPC_PTRACE_GETHWDBGINFO. We're now doing that.
> > 
> > I can see that.  But you haven't answered why.
> 
> BookS doesn't support BP_EXACT, that's why I suggested this flag.

Surely you can support it with exactly the same sort of filtering
you're using for the 8-byte ranges now?

> A BP_EXACT watchpoint triggers only when there's a memory access exactly
> at the given address. It doesn't trigger when there's (for example) a
> 4-byte write at an address immediately before which also changes the
> memory contents of the byte watched by the BP_EXACT watchpoint. a ranged
> watchpoint would trigger, so the semantics are different.
> 
> As a general rule, GDB only sets ranged watchpoints and only uses
> BP_EXACT ones when the user sets a flag. I want GDB to fail when the
> user sets the flag on BookS since it can't provide the feature.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[Thiago Jung Bauermann]

On Fri, 2011-08-26 at 14:41 +1000, David Gibson wrote:
> On Wed, Aug 24, 2011 at 09:41:43PM -0300, Thiago Jung Bauermann wrote:
> > On Wed, 2011-08-24 at 14:00 +1000, David Gibson wrote:
> > > On Tue, Aug 23, 2011 at 02:57:56PM +0530, K.Prasad wrote:
> > > > On Tue, Aug 23, 2011 at 03:09:31PM +1000, David Gibson wrote:
> > > > > On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> > > > > > 
> > > > > > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > > > > > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > > > > > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > > > > > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > > > > > "features" member of "struct ppc_debug_info" to advertise support for the
> > > > > > same on Book3E PowerPC processors.
> > > > > 
> > > > > I thought the idea was that the BP_EXACT mode was the default - if the
> > > > > new interface was supported at all, then BP_EXACT was always
> > > > > supported.  So, why do you need a new flag?
> > > > > 
> > > > 
> > > > Yes, BP_EXACT was always supported but not advertised through
> > > > PPC_PTRACE_GETHWDBGINFO. We're now doing that.
> > > 
> > > I can see that.  But you haven't answered why.
> > 
> > BookS doesn't support BP_EXACT, that's why I suggested this flag.
> 
> Surely you can support it with exactly the same sort of filtering
> you're using for the 8-byte ranges now?

Yes, but to detect that the processor doesn't support BP_EXACT in
hardware I'd have to send a ptrace request, and have it rejected. Only
then I'd step back and simulate one with ranges. Considering that it's
easy and backwards compatible to add a new flag to signal that BP_EXACT
is not supported, I don't know why it would be better to go with the
more convoluted process.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[David Gibson]

On Tue, Aug 30, 2011 at 09:27:41PM -0300, Thiago Jung Bauermann wrote:
> On Fri, 2011-08-26 at 14:41 +1000, David Gibson wrote:
> > On Wed, Aug 24, 2011 at 09:41:43PM -0300, Thiago Jung Bauermann wrote:
> > > On Wed, 2011-08-24 at 14:00 +1000, David Gibson wrote:
> > > > On Tue, Aug 23, 2011 at 02:57:56PM +0530, K.Prasad wrote:
> > > > > On Tue, Aug 23, 2011 at 03:09:31PM +1000, David Gibson wrote:
> > > > > > On Fri, Aug 19, 2011 at 01:23:38PM +0530, K.Prasad wrote:
> > > > > > > 
> > > > > > > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > > > > > > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > > > > > > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > > > > > > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > > > > > > "features" member of "struct ppc_debug_info" to advertise support for the
> > > > > > > same on Book3E PowerPC processors.
> > > > > > 
> > > > > > I thought the idea was that the BP_EXACT mode was the default - if the
> > > > > > new interface was supported at all, then BP_EXACT was always
> > > > > > supported.  So, why do you need a new flag?
> > > > > > 
> > > > > 
> > > > > Yes, BP_EXACT was always supported but not advertised through
> > > > > PPC_PTRACE_GETHWDBGINFO. We're now doing that.
> > > > 
> > > > I can see that.  But you haven't answered why.
> > > 
> > > BookS doesn't support BP_EXACT, that's why I suggested this flag.
> > 
> > Surely you can support it with exactly the same sort of filtering
> > you're using for the 8-byte ranges now?
> 
> Yes, but to detect that the processor doesn't support BP_EXACT in
> hardware I'd have to send a ptrace request, and have it rejected. Only
> then I'd step back and simulate one with ranges. Considering that it's
> easy and backwards compatible to add a new flag to signal that BP_EXACT
> is not supported, I don't know why it would be better to go with the
> more convoluted process.

No, I'm saying why not implement BP_EXACT on server.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[PATCH 0/2] Changes to PowerPC ptrace flags using watchpoints - v2

[K.Prasad]

Hi David,
	Please find a revised version of the patchset which have
incorporated the various suggestions made by you.

Kindly review the patches and let me know if they look fine.

Changelog - v2
--------------
v1 posted at
http://lists.ozlabs.org/pipermail/linuxppc-dev/2011-August/092463.html

- Introduction of a new version number for the hw-breakpoint structures
  dropped.
- The ptrace flag operations are expected to make more precise requests
  and suitable error return codes have been added for incorrect
  requests.
- Modification of an existing breakpoint is not possible. Request an
  delete, followed by set breakpoint request instead.

Thanks,
K.Prasad

[PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags

[K.Prasad]

PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG are
PowerPC specific ptrace flags that use the watchpoint register. While they are
targeted primarily towards BookE users, user-space applications such as GDB
have started using them for BookS too. This patch enables the use of generic
hardware breakpoint interfaces for these new flags.

Apart from the usual benefits of using generic hw-breakpoint interfaces, these
changes allow debuggers (such as GDB) to use a common set of ptrace flags for
their watchpoint needs and allow more precise breakpoint specification (length
of the variable can be specified).

Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 Documentation/powerpc/ptrace.txt |   16 ++++++++
 arch/powerpc/kernel/ptrace.c     |   77 +++++++++++++++++++++++++++++++++++---
 2 files changed, 87 insertions(+), 6 deletions(-)

diff --git a/Documentation/powerpc/ptrace.txt b/Documentation/powerpc/ptrace.txt
index f4a5499..f2a7a39 100644
--- a/Documentation/powerpc/ptrace.txt
+++ b/Documentation/powerpc/ptrace.txt
@@ -127,6 +127,22 @@ Some examples of using the structure to:
   p.addr2           = (uint64_t) end_range;
   p.condition_value = 0;
 
+- set a watchpoint in server processors (BookS)
+
+  p.version         = 1;
+  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_RW;
+  p.addr_mode       = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
+  or
+  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
+
+  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
+  p.addr            = (uint64_t) begin_range;
+  /* For PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE addr2 needs to be specified, where
+   * addr2 - addr <= 8 Bytes.
+   */
+  p.addr2           = (uint64_t) end_range;
+  p.condition_value = 0;
+
 3. PTRACE_DELHWDEBUG
 
 Takes an integer which identifies an existing breakpoint or watchpoint
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 05b7dd2..cd41c78 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1339,6 +1339,12 @@ static int set_dac_range(struct task_struct *child,
 static long ppc_set_hwdebug(struct task_struct *child,
 		     struct ppc_hw_breakpoint *bp_info)
 {
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	int ret, len = 0;
+	struct thread_struct *thread = &(child->thread);
+	struct perf_event *bp;
+	struct perf_event_attr attr;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 #ifndef CONFIG_PPC_ADV_DEBUG_REGS
 	unsigned long dabr;
 #endif
@@ -1382,13 +1388,9 @@ static long ppc_set_hwdebug(struct task_struct *child,
 	 */
 	if ((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) == 0 ||
 	    (bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) != 0 ||
-	    bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT ||
 	    bp_info->condition_mode != PPC_BREAKPOINT_CONDITION_NONE)
 		return -EINVAL;
 
-	if (child->thread.dabr)
-		return -ENOSPC;
-
 	if ((unsigned long)bp_info->addr >= TASK_SIZE)
 		return -EIO;
 
@@ -1398,15 +1400,63 @@ static long ppc_set_hwdebug(struct task_struct *child,
 		dabr |= DABR_DATA_READ;
 	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
 		dabr |= DABR_DATA_WRITE;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	if (ptrace_get_breakpoints(child) < 0)
+		return -ESRCH;
 
-	child->thread.dabr = dabr;
+	/*
+	 * Check if the request is for 'range' breakpoints. We can
+	 * support it if range < 8 bytes.
+	 */
+	if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE) {
+		len = bp_info->addr2 - bp_info->addr;
+	} else if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT) {
+		ptrace_put_breakpoints(child);
+		return -EINVAL;
+	}
+	bp = thread->ptrace_bps[0];
+	if (bp) {
+		ptrace_put_breakpoints(child);
+		return -ENOSPC;
+	}
+
+	/* Create a new breakpoint request if one doesn't exist already */
+	hw_breakpoint_init(&attr);
+	attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN;
+	attr.bp_len = len;
+	arch_bp_generic_fields(dabr & (DABR_DATA_WRITE | DABR_DATA_READ),
+								&attr.bp_type);
+
+	thread->ptrace_bps[0] = bp = register_user_hw_breakpoint(&attr,
+					       ptrace_triggered, NULL, child);
+	if (IS_ERR(bp)) {
+		thread->ptrace_bps[0] = NULL;
+		ptrace_put_breakpoints(child);
+		return PTR_ERR(bp);
+	}
 
+	ptrace_put_breakpoints(child);
+	return 1;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+
+	if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
+		return -EINVAL;
+
+	if (child->thread.dabr)
+		return -ENOSPC;
+
+	child->thread.dabr = dabr;
 	return 1;
 #endif /* !CONFIG_PPC_ADV_DEBUG_DVCS */
 }
 
 static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
 {
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	int ret = 0;
+	struct thread_struct *thread = &(child->thread);
+	struct perf_event *bp;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 #ifdef CONFIG_PPC_ADV_DEBUG_REGS
 	int rc;
 
@@ -1426,10 +1476,25 @@ static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
 #else
 	if (data != 1)
 		return -EINVAL;
+
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+	if (ptrace_get_breakpoints(child) < 0)
+		return -ESRCH;
+
+	bp = thread->ptrace_bps[0];
+	if (bp) {
+		unregister_hw_breakpoint(bp);
+		thread->ptrace_bps[0] = NULL;
+	} else
+		ret = -ENOENT;
+	ptrace_put_breakpoints(child);
+	return ret;
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
 	if (child->thread.dabr == 0)
 		return -ENOENT;
 
 	child->thread.dabr = 0;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
 
 	return 0;
 #endif
@@ -1560,7 +1625,7 @@ long arch_ptrace(struct task_struct *child, long request,
 		dbginfo.data_bp_alignment = 4;
 #endif
 		dbginfo.sizeof_condition = 0;
-		dbginfo.features = 0;
+		dbginfo.features = PPC_DEBUG_FEATURE_DATA_BP_RANGE;
 #endif /* CONFIG_PPC_ADV_DEBUG_REGS */
 
 		if (!access_ok(VERIFY_WRITE, datavp,
-- 
1.7.4.1

[PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[K.Prasad]

While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
user-space debuggers (like GDB) who may want to use it. Hence we introduce a
new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
"features" member of "struct ppc_debug_info" to advertise support for the
same on Book3E PowerPC processors.

Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/ptrace.h |    1 +
 arch/powerpc/kernel/ptrace.c      |    1 +
 2 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 48223f9..cf014f9 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -380,6 +380,7 @@ struct ppc_debug_info {
 #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x0000000000000002
 #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x0000000000000004
 #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x0000000000000008
+#define PPC_DEBUG_FEATURE_DATA_BP_EXACT		0x0000000000000010
 
 #ifndef __ASSEMBLY__
 
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 18d28b6..71db5a6 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1636,6 +1636,7 @@ long arch_ptrace(struct task_struct *child, long request,
 #ifdef CONFIG_PPC_ADV_DEBUG_DAC_RANGE
 		dbginfo.features |=
 				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
+				   PPC_DEBUG_FEATURE_DATA_BP_EXACT |
 				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
 #endif
 #else /* !CONFIG_PPC_ADV_DEBUG_REGS */

Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags

[David Gibson]

On Thu, Dec 08, 2011 at 04:49:48PM +0530, K.Prasad wrote:
> PPC_PTRACE_GETHWDBGINFO, PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG are
> PowerPC specific ptrace flags that use the watchpoint register. While they are
> targeted primarily towards BookE users, user-space applications such as GDB
> have started using them for BookS too. This patch enables the use of generic
> hardware breakpoint interfaces for these new flags.
> 
> Apart from the usual benefits of using generic hw-breakpoint interfaces, these
> changes allow debuggers (such as GDB) to use a common set of ptrace flags for
> their watchpoint needs and allow more precise breakpoint specification (length
> of the variable can be specified).
> 
> Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>

Ok, I think it's finally ready to go.

Acked-by: David Gibson <david@gibson.dropbear.id.au>

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[David Gibson]

On Thu, Dec 08, 2011 at 04:53:30PM +0530, K.Prasad wrote:
> While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> "features" member of "struct ppc_debug_info" to advertise support for the
> same on Book3E PowerPC processors.

Hrm.  I had assumed the reason there wasn't a feature bit for EXACT
originally was that EXACT breakpoints were *always* supposed to be
supported by the new interface.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[K.Prasad]

On Wed, Dec 21, 2011 at 11:55:02AM +1100, David Gibson wrote:
> On Thu, Dec 08, 2011 at 04:53:30PM +0530, K.Prasad wrote:
> > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > "features" member of "struct ppc_debug_info" to advertise support for the
> > same on Book3E PowerPC processors.
> 
> Hrm.  I had assumed the reason there wasn't a feature bit for EXACT
> originally was that EXACT breakpoints were *always* supposed to be
> supported by the new interface.
>

Okay. Although BookS doesn't support EXACT breakpoints, it is possible
(after the introduction of new hw-breakpoint interfaces) to request for
a breakpoint of length 1 Byte.

The hw-breakpoint infrastructure would take care of filtering the
extraneous interrupts arising out of accesses in the neighbourhood, in
such a case.

David,
	Can you Ack this patch too, if you find the changes acceptable?

Thanks,
K.Prasad
 

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[David Gibson]

On Thu, Dec 22, 2011 at 03:04:47PM +0530, K.Prasad wrote:
> On Wed, Dec 21, 2011 at 11:55:02AM +1100, David Gibson wrote:
> > On Thu, Dec 08, 2011 at 04:53:30PM +0530, K.Prasad wrote:
> > > While PPC_PTRACE_SETHWDEBUG ptrace flag in PowerPC accepts
> > > PPC_BREAKPOINT_MODE_EXACT mode of breakpoint, the same is not intimated to the
> > > user-space debuggers (like GDB) who may want to use it. Hence we introduce a
> > > new PPC_DEBUG_FEATURE_DATA_BP_EXACT flag which will be populated on the
> > > "features" member of "struct ppc_debug_info" to advertise support for the
> > > same on Book3E PowerPC processors.
> > 
> > Hrm.  I had assumed the reason there wasn't a feature bit for EXACT
> > originally was that EXACT breakpoints were *always* supposed to be
> > supported by the new interface.
> >
> 
> Okay. Although BookS doesn't support EXACT breakpoints, it is possible
> (after the introduction of new hw-breakpoint interfaces) to request for
> a breakpoint of length 1 Byte.

Hrm.  An EXACT breakpoint is not exactly the same as a range
breakpoint of length 1 (consider unaligned accesses).  But despite
that, it should be possible to implement exact breakpoints on Book3S
server hardware with some software filtering.

And since that leaves no hardware that *can't* implement exact
breakpoints (directly or indirectly), I'm not yet convinced of the
need for a flag bit.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag

[Thiago Jung Bauermann]

On Mon, 2012-01-16 at 19:18 +1100, David Gibson wrote:
> Hrm.  An EXACT breakpoint is not exactly the same as a range
> breakpoint of length 1 (consider unaligned accesses).  But despite
> that, it should be possible to implement exact breakpoints on Book3S
> server hardware with some software filtering.
> 
> And since that leaves no hardware that *can't* implement exact
> breakpoints (directly or indirectly), I'm not yet convinced of the
> need for a flag bit.

I agree.

-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center