Re: [Patch][hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Mon, May 14, 2012 at 10:53:42AM +1000, Benjamin Herrenschmidt wrote: > On Fri, 2012-05-11 at 14:13 +0530, K.Prasad wrote: > > > +#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 */ > > "ret" is unused in that function, causing a warning which breaks the > build since we have -Werror. I'm fixing that locally but be more careful > next time please. > > Ben. Sorry about that warning, I clearly missed that. I guess you do a custom build with -Werror flag enabled, unlike what the normal Makefile does? Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch][hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
Hi Ben,
Please find a patch that introduces generic hw-breakpoint
interfaces for a couple of ptrace flags used by server-class processors.
This patch has been reviewed by the community and acked-by David Gibson
last year
(http://thread.gmane.org/gmane.linux.ports.ppc.embedded/47473/focus=47760)
but did not get submitted to you for inclusion (the patch
applies to the latest linux-2.6 tree, with a few line adjustments).
While originally a patchset of two, I'm submitting only the first (and
independent) patch due to disagreements over the 2/2 patch.
Kindly consider this patch for inclusion into the relevant PowerPC tree.
Thanks,
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
Acked-by: David Gibson
---
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_ALIG
Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag
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
---
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 0x0002
#define PPC_DEBUG_FEATURE_DATA_BP_RANGE0x0004
#define PPC_DEBUG_FEATURE_DATA_BP_MASK 0x0008
+#define PPC_DEBUG_FEATURE_DATA_BP_EXACT0x0010
#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 */
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
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
---
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)
+
[PATCH 0/2] Changes to PowerPC ptrace flags using watchpoints - v2
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Wed, Dec 07, 2011 at 05:01:57PM -0200, Thiago Jung Bauermann wrote:
> On Thu, 2011-12-01 at 15:50 +0530, K.Prasad wrote:
> > On Mon, Nov 28, 2011 at 02:11:11PM +1100, David Gibson wrote:
> > > [snip]
> > > On Wed, Oct 12, 2011 at 11:09:48PM +0530, K.Prasad wrote:
> > > > 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;
> > >
> > > You should probably document the alignment constraint on the address
> > > here, too.
> > >
> >
> > Alignment constraints will be learnt by the user-space during runtime.
> > We provide that as part of 'struct ppc_debug_info' in
> > 'data_bp_alignment' field.
> >
> > While the alignment is always 8-bytes for BookS, I think userspace
> > should be left to learn it through PTRACE_PPC_GETHWDEBUGINFO.
>
> Right. In particular, BookE doesn't have alignment constraints.
>
Okay.
> > > > + attr.bp_len = len;
> > > > + ret = modify_user_hw_breakpoint(bp, &attr);
> > > > + if (ret) {
> > > > + ptrace_put_breakpoints(child);
> > > > + return ret;
> > > > + }
> > >
> > > If a bp already exists, you're modifying it. I thought the semantics
> > > of the new interface meant that you shoul return ENOSPC in this case,
> > > and a DEL would be necessary before adding another breakpoint.
> > >
> >
> > I'm not too sure what would be the desired behaviour for this interface,
> > either way is fine with me. I'd like to hear from the GDB folks (copied
> > in this email) to know what would please them.
>
> ENOSPC should be returned. The interface doesn't have provisions for
> modifying breakpoints. The client should delete/create instead of trying
> to modify.
>
> Since PTRACE_PPC_GETHWDEBUGINFO returns the number of available
> breakpoint registers, the client shouldn't (and GDB doesn't) try to set
> more breakpoints than possible.
>
Okay, I will modify the code accordingly.
> > > > @@ -1426,10 +1488,24 @@ 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;
> > > > + }
> > > > + ptrace_put_breakpoints(child);
> > > > + return 0;
> > >
> > > Shouldn't DEL return an error if there is no existing bp.
> > >
> >
> > Same comment as above. We'd like to know what behaviour would help the
> > GDB use this interface better as there's no right or wrong way here.
>
> GDB expects DEL to return ENOENT is there's no existing bp.
>
Fine, here too. We'll return a -ENOENT here.
Thanks for your comments.
-- K.Prasad
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Mon, Nov 28, 2011 at 02:11:11PM +1100, David Gibson wrote:
> [snip]
> On Wed, Oct 12, 2011 at 11:09:48PM +0530, K.Prasad wrote:
> > > > + if (bp) {
> > > > + attr = bp->attr;
> > > > + attr.bp_addr = (unsigned long)bp_info->addr &
> > > > ~HW_BREAKPOINT_ALIGN;
> > > > + arch_bp_generic_fields(dabr &
> > > > + (DABR_DATA_WRITE |
> > > > DABR_DATA_READ),
> > > > + &attr.bp_type);
> > > > + attr.bp_len = len;
> > >
> > > If gdb is using the new breakpoint interface, surely it should just
> > > use it, rather than doing this bit frobbing as in the old SET_DABR
> > > call.
> > >
> >
> > I understand that you wanted to avoid this duplication of effort in terms
> > of encoding and decoding the breakpoint type from
> > PPC_BREAKPOINT_TRIGGER_READ to DABR_DATA_READ to HW_BREAKPOINT_R.
> >
> > However HW_BREAKPOINT_R is a generic definition used across
> > architectures, DABR_DATA_READ is used in the !CONFIG_HAVE_HW_BREAKPOINT
> > case while PPC_BREAKPOINT_TRIGGER_READ is used in
> > CONFIG_PPC_ADV_DEBUG_REGS case.
> >
> > While we could define PPC_BREAKPOINT_TRIGGER_READ and DABR_DATA_READ to
> > the same value it may not result in any code savings (since the bit
> > translation is done for !CONFIG_HAVE_HW_BREAKPOINT case anyway). So, I
> > think it is best left the way it is.
>
> That's not what I'm suggesting. What I'm saying is that ig userspace
> is using the new generic interface, then it should just set the
> bp_type field, and it should not use the DABR_DATA_{READ,WRITE} bits.
> The DABR_DATA bits should *only* be processed in the legacy interface,
> never in the generic interface.
>
The DABR_DATA_{READ,WRITE} bits are neither set by the user, nor
expected by the hw-breakpoint interface. It is an intermediate code used
to re-use the arch_bp_generic_fields function. We could convert directly
from PPC_BREAKPOINT_TRIGGER_READ to HW_BREAKPOINT_R (using a switch-case)
but that may not result in any code savings.
DABR_DATA_{READ,WRITE} is indeed legacy and cannot be set by user-space
for a PPC_PTRACE_SETHWDEBUG + CONFIG_HAVE_HW_BREAKPOINT combination.
[snipped]
> > 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;
>
> You should probably document the alignment constraint on the address
> here, too.
>
Alignment constraints will be learnt by the user-space during runtime.
We provide that as part of 'struct ppc_debug_info' in
'data_bp_alignment' field.
While the alignment is always 8-bytes for BookS, I think userspace
should be left to learn it through PTRACE_PPC_GETHWDEBUGINFO.
> > + /* 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..be5dc57 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
>
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Wed, Oct 12, 2011 at 02:33:59PM +1100, David Gibson wrote:
> On Fri, Sep 16, 2011 at 12:57:10PM +0530, K.Prasad wrote:
> > On Fri, Aug 26, 2011 at 03:05:52PM +0530, K.Prasad wrote:
> > > On Wed, Aug 24, 2011 at 01:59:39PM +1000, David Gibson wrote:
> > > > On Tue, Aug 23, 2011 at 02:55:13PM +0530, K.Prasad wrote:
> > > > > On Tue, Aug 23, 2011 at 03:08:50PM +1000, David Gibson wrote:
> > > > > > On Fri, Aug 19, 2011 at 01:21:36PM +0530, K.Prasad wrote:
[snipped]
> > [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace
> > flags
> >
> > 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. The version number of the associated data structures
> > "ppc_hw_breakpoint" and "ppc_debug_info" is incremented to denote new
> > semantics.
>
> Above pargraph needs revision.
>
>
Sure, done.
> > 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).
> >
> > [Edjunior: Identified an issue in the patch with the sanity check for
> > version
> > numbers]
> >
> > Tested-by: Edjunior Barbosa Machado
> > Signed-off-by: K.Prasad
> >
> > diff --git a/Documentation/powerpc/ptrace.txt
> > b/Documentation/powerpc/ptrace.txt
> > index f4a5499..04656ec 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_RANGE_EXACT;
>
> MODE_RANGE_EXACT? Shouldn't that just be MODE_EXACT?
>
That was silly. Thanks for pointing it out.
> > +
> > + 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..2449495 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,83 @@ static long ppc_set_hwdebug(struct task_struct
> &
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Fri, Aug 26, 2011 at 03:05:52PM +0530, K.Prasad wrote: > On Wed, Aug 24, 2011 at 01:59:39PM +1000, David Gibson wrote: > > On Tue, Aug 23, 2011 at 02:55:13PM +0530, K.Prasad wrote: > > > On Tue, Aug 23, 2011 at 03:08:50PM +1000, David Gibson wrote: > > > > On Fri, Aug 19, 2011 at 01:21:36PM +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. The version number of the associated data structures > > > > > "ppc_hw_breakpoint" and "ppc_debug_info" is incremented to denote new > > > > > semantics. > > > > > > > > So, the structure itself doesn't seem to have been extended. I don't > > > > understand what the semantic difference is - your patch comment needs > > > > to explain this clearly. > > > > > > > > > > We had a request to extend the structure but thought it was dangerous to > > > do so. For instance if the user-space used version1 of the structure, > > > while kernel did a copy_to_user() pertaining to version2, then we'd run > > > into problems. Unfortunately the ptrace flags weren't designed to accept > > > a version number as input from the user through the > > > PPC_PTRACE_GETHWDBGINFO flag (which would have solved this issue). > > > > I still don't follow you. > > > > Two things here. > > One, the change of semantics warranted an increment of the version > number. The new semantics accepts PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE on > BookS, while the old version number did not. I've added a small comment > in the code to this effect. > > Two, regarding changes in the "ppc_hw_breakpoint" and "ppc_debug_info" > structures - we would like to add more members to it if we can (GDB has a > pending request to add more members to it). However the problem foreseen > is that there could be a mismatch between the versions of the structure > used by the user vs kernel-space i.e. if a new version of the structure, > known to the kernel, had an extra member while the user-space still had > the old version, then it becomes dangerous because the __copy_to_user > function would overflow the buffer size in user-space. > > This could have been avoided if PPC_PTRACE_GETHWDBGINFO was originally > designed to accept a version number (and provide corresponding > "struct ppc_debug_info") rather than send a populated "ppc_debug_info" > structure along with the version number. > Based on further discussions with the code-reviewer (David Gibson ), it was decided that incrementing the version number for the proposed changes is unnecessary as the patch only introduces new features but not a change in semantics. Please find a new version of the patch where the version number is retained as 1, along with the other planned changes. Thanks, K.Prasad [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags 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. The version number of the associated data structures "ppc_hw_breakpoint" and "ppc_debug_info" is incremented to denote new semantics. 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). [Edjunior: Identified an issue in the patch with the sanity check for version numbers] Tested-by: Edjunior Barbosa Machado Signed-off-by: K.Prasad diff --git a/Documentation/powerpc/ptrace.txt b/Documentation/powerpc/ptrace.txt index f4a5499..04656ec 100644 --- a/Documentation/powerpc/ptrace.txt +++ b/Documentation/powerpc/ptrace.txt @@ -127,6 +127,22 @@ Some examples of
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Wed, Aug 24, 2011 at 01:59:39PM +1000, David Gibson wrote: > On Tue, Aug 23, 2011 at 02:55:13PM +0530, K.Prasad wrote: > > On Tue, Aug 23, 2011 at 03:08:50PM +1000, David Gibson wrote: > > > On Fri, Aug 19, 2011 at 01:21:36PM +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. The version number of the associated data structures > > > > "ppc_hw_breakpoint" and "ppc_debug_info" is incremented to denote new > > > > semantics. > > > > > > So, the structure itself doesn't seem to have been extended. I don't > > > understand what the semantic difference is - your patch comment needs > > > to explain this clearly. > > > > > > > We had a request to extend the structure but thought it was dangerous to > > do so. For instance if the user-space used version1 of the structure, > > while kernel did a copy_to_user() pertaining to version2, then we'd run > > into problems. Unfortunately the ptrace flags weren't designed to accept > > a version number as input from the user through the > > PPC_PTRACE_GETHWDBGINFO flag (which would have solved this issue). > > I still don't follow you. > Two things here. One, the change of semantics warranted an increment of the version number. The new semantics accepts PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE on BookS, while the old version number did not. I've added a small comment in the code to this effect. Two, regarding changes in the "ppc_hw_breakpoint" and "ppc_debug_info" structures - we would like to add more members to it if we can (GDB has a pending request to add more members to it). However the problem foreseen is that there could be a mismatch between the versions of the structure used by the user vs kernel-space i.e. if a new version of the structure, known to the kernel, had an extra member while the user-space still had the old version, then it becomes dangerous because the __copy_to_user function would overflow the buffer size in user-space. This could have been avoided if PPC_PTRACE_GETHWDBGINFO was originally designed to accept a version number (and provide corresponding "struct ppc_debug_info") rather than send a populated "ppc_debug_info" structure along with the version number. > > I'll add a comment w.r.t change in semantics - such as the ability to > > accept 'range' breakpoints in BookS. > > > > > > 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). > > > > > > What is the mechanism for implementing the range breakpoint on book3s? > > > > > > > The hw-breakpoint interface, accepts length as an argument in BookS (any > > value <= 8 Bytes) and would filter out extraneous interrupts arising out > > of accesses outside the range comprising inside > > hw_breakpoint_handler function. > > > > We put that ability to use here. > > Ah, so in hardware the breakpoints are always 8 bytes long, but you > filter out false hits on a shorter range? Of course, the utility of > range breakpoints is questionable when length <=8, but the start must > be aligned on an 8-byte boundary. > Yes, we ensure that through + attr.bp_addr = (unsigned long)bp_info->addr & ~HW_BREAKPOINT_ALIGN; > [snip] > > > > if ((unsigned long)bp_info->addr >= TASK_SIZE) > > > > return -EIO; > > > > > > > > @@ -1398,15 +1400,86 @@ 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 (bp_info->version == 1) > > > > +
Re: [PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag
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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
On Tue, Aug 23, 2011 at 03:08:50PM +1000, David Gibson wrote:
> On Fri, Aug 19, 2011 at 01:21:36PM +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. The version number of the associated data structures
> > "ppc_hw_breakpoint" and "ppc_debug_info" is incremented to denote new
> > semantics.
>
> So, the structure itself doesn't seem to have been extended. I don't
> understand what the semantic difference is - your patch comment needs
> to explain this clearly.
>
We had a request to extend the structure but thought it was dangerous to
do so. For instance if the user-space used version1 of the structure,
while kernel did a copy_to_user() pertaining to version2, then we'd run
into problems. Unfortunately the ptrace flags weren't designed to accept
a version number as input from the user through the
PPC_PTRACE_GETHWDBGINFO flag (which would have solved this issue).
I'll add a comment w.r.t change in semantics - such as the ability to
accept 'range' breakpoints in BookS.
> > 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).
>
> What is the mechanism for implementing the range breakpoint on book3s?
>
The hw-breakpoint interface, accepts length as an argument in BookS (any
value <= 8 Bytes) and would filter out extraneous interrupts arising out
of accesses outside the range comprising inside
hw_breakpoint_handler function.
We put that ability to use here.
> > [Edjunior: Identified an issue in the patch with the sanity check for
> > version
> > numbers]
> >
> > Tested-by: Edjunior Barbosa Machado
> > Signed-off-by: K.Prasad
> > ---
> > Documentation/powerpc/ptrace.txt | 16 ++
> > arch/powerpc/kernel/ptrace.c | 104
> > +++---
> > 2 files changed, 112 insertions(+), 8 deletions(-)
> >
> > diff --git a/Documentation/powerpc/ptrace.txt
> > b/Documentation/powerpc/ptrace.txt
> > index f4a5499..97301ae 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) using version 2
> > +
> > + p.version = 2;
> > + p.trigger_type= PPC_BREAKPOINT_TRIGGER_RW;
> > + p.addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
> > + or
> > + p.addr_mode = PPC_BREAKPOINT_MODE_RANGE_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..18d28b6 100644
> > --- a/arch/powerpc/kernel/ptrace.c
> > +++ b/arch/powerpc/kernel/ptrace.c
> > @@ -1339,11 +1339,17 @@ 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 */
>
> I'm confused. This compiled before on book3s, and I don't see any
> changes to Makefile or Kconfig in the patch that will result in this
> code compiling when it previously didn't Why are these new guards
> added?
>
The code is guarded using the CONFIG_ flags for two reasons.
a) We don't want the
[PATCH 2/2] [PowerPC Book3E] Introduce new ptrace debug feature flag
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
---
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 0x0002
#define PPC_DEBUG_FEATURE_DATA_BP_RANGE0x0004
#define PPC_DEBUG_FEATURE_DATA_BP_MASK 0x0008
+#define PPC_DEBUG_FEATURE_DATA_BP_EXACT0x0010
#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
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[PATCH 1/2] [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags
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. The version number of the associated data structures
"ppc_hw_breakpoint" and "ppc_debug_info" is incremented to denote new semantics.
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).
[Edjunior: Identified an issue in the patch with the sanity check for version
numbers]
Tested-by: Edjunior Barbosa Machado
Signed-off-by: K.Prasad
---
Documentation/powerpc/ptrace.txt | 16 ++
arch/powerpc/kernel/ptrace.c | 104 +++---
2 files changed, 112 insertions(+), 8 deletions(-)
diff --git a/Documentation/powerpc/ptrace.txt b/Documentation/powerpc/ptrace.txt
index f4a5499..97301ae 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) using version 2
+
+ p.version = 2;
+ p.trigger_type= PPC_BREAKPOINT_TRIGGER_RW;
+ p.addr_mode = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
+ or
+ p.addr_mode = PPC_BREAKPOINT_MODE_RANGE_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..18d28b6 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -1339,11 +1339,17 @@ 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
- if (bp_info->version != 1)
+ if ((bp_info->version != 1) && (bp_info->version != 2))
return -ENOTSUPP;
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
/*
@@ -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,86 @@ 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 (bp_info->version == 1)
+ goto version_one;
+ if (ptrace_get_breakpoints(child) < 0)
+ return -ESRCH;
- child->thread.dabr = dabr;
+ bp = thread->ptrace_bps[0];
+ if (!bp_info->addr) {
+ if (bp) {
+ unregister_hw_breakpoint(bp);
+ thread->ptrace_bps[0] = NULL;
+ }
+ ptrace_put_breakpoints(child);
+ return 0;
+ }
+ /*
+* 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;
+ }
+ if (bp) {
+ attr = bp->attr;
+ attr.bp_addr = (unsigned long)bp_info->addr &
~HW_BREAKPOINT_ALIGN;
+ arch_bp_generic_fields(dabr &
+
[PATCH 0/2] Changes to PowerPC ptrace flags using watchpoints
Hi All, Please find a set of two patches that introduce generic hw-breakpoint interfaces for a couple of ptrace flags used by server-class processors and another change to advertise a feature to user-space hitherto available and used, but not claimed as supported. GDB, which recently began using the PPC_PTRACE_GETHWDBGINFO/PPC_PTRACE_SETHWDEBUG/PPC_PTRACE_DELHWDEBUG flags on BookS processors is presently unable to set watchpoints. The changes in Patch1, will fix that issue and help it use a common set of code across BookE and BookS. K.Prasad (2): [hw-breakpoint] Use generic hw-breakpoint interfaces for new PPC ptrace flags [PowerPC Book3E] Introduce new ptrace debug feature flag Documentation/powerpc/ptrace.txt | 16 ++ arch/powerpc/include/asm/ptrace.h |1 + arch/powerpc/kernel/ptrace.c | 105 ++--- 3 files changed, 114 insertions(+), 8 deletions(-) Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/6] hw_breakpoints: Migrate breakpoint conditional build under new config
On Mon, Jul 04, 2011 at 03:29:14PM +0200, Frederic Weisbecker wrote: > On Mon, Jul 04, 2011 at 06:57:46PM +0530, K.Prasad wrote: > > On Tue, May 24, 2011 at 11:52:23PM +0200, Frederic Weisbecker wrote: > > > Migrate conditional hw_breakpoint code compilation under > > > the new config to prepare for letting the user chose whether > > > or not to build this feature > > > > > > > Making the hardware breakpoint patches modular has always been a goal. > > I've looked at the PowerPC parts of the code and they look harmless. > > > > Acked-by: K.Prasad > > Great! > > I'll push that soon, thanks guys for your acks! Meanwhile, I was testing hardware breakpoints through perf and found that monitoring a given address fails when using 'perf record' (returns -ENOSPC) while 'perf stat' and watchpoint through gdb works fine (see logs below). Has this behaviour been reported for other perf counters? Thanks, K.Prasad # tools/perf/perf --version perf version 3.0.0-rc5 # # grep pid_max /proc/kallsyms 81a25010 D pid_max 81a25014 D pid_max_min 81a25018 D pid_max_max # # uname -a Linux llm37.in.ibm.com 3.0.0-rc5 #1 SMP Mon Jul 4 22:24:02 IST 2011 x86_64 x86_64 x86_64 GNU/Linux # # tools/perf/perf stat -e mem:0x81a25010:rw make kernel/futex.o CHK include/linux/version.h CHK include/generated/utsrelease.h CALLscripts/checksyscalls.sh make[1]: `kernel/futex.o' is up to date. Performance counter stats for 'make kernel/futex.o': 188 mem:0x81a25010:rw 10.734957333 seconds time elapsed # tools/perf/perf record -e mem:0x81a25010:rw make kernel/futex.o Error: sys_perf_event_open() syscall returned with 28 (No space left on device). /bin/dmesg may provide additional information. Fatal: No CONFIG_PERF_EVENTS=y kernel support configured? ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 4/6] hw_breakpoints: Breakpoints arch ability don't need perf events
On Tue, May 24, 2011 at 11:52:25PM +0200, Frederic Weisbecker wrote: > The breakpoint support ability in an arch is not related > to the fact perf events is built or not. HAVE_HW_BREAKPOINT > only shows an ability so this dependency makes no sense > anymore. Archs that select HAVE_HW_BREAKPOINT already > ensure that perf event is built. > > Remove that dependency. > > Signed-off-by: Frederic Weisbecker > Cc: Ingo Molnar > Cc: Peter Zijlstra > Cc: Will Deacon > Cc: Prasad > Cc: Paul Mundt > --- > arch/Kconfig |1 - > 1 files changed, 0 insertions(+), 1 deletions(-) > > diff --git a/arch/Kconfig b/arch/Kconfig > index f78c2be..ce4be89 100644 > --- a/arch/Kconfig > +++ b/arch/Kconfig > @@ -149,7 +149,6 @@ config HAVE_DEFAULT_NO_SPIN_MUTEXES > > config HAVE_HW_BREAKPOINT > bool > - depends on PERF_EVENTS > > config HAVE_MIXED_BREAKPOINTS_REGS > bool > -- Just a thought you might want to consider... The need to keep the ability (HAVE_HW_BREAKPOINT) and the user-choice to enable hardware breakpoints (through HW_BREAKPOINT) in separate config options isn't very clear to me (and is a bit confusing with very similar names). Why not make HAVE_HW_BREAKPOINT selectable by the user (which in turn would turn on PERF_EVENTS) for a given architecture? Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/6] hw_breakpoints: Migrate breakpoint conditional build under new config
On Tue, May 24, 2011 at 11:52:23PM +0200, Frederic Weisbecker wrote: > Migrate conditional hw_breakpoint code compilation under > the new config to prepare for letting the user chose whether > or not to build this feature > > Signed-off-by: Frederic Weisbecker > Acked-by: Will Deacon > Cc: Ingo Molnar > Cc: Peter Zijlstra > Cc: Prasad > Cc: Paul Mundt > --- > arch/arm/include/asm/hw_breakpoint.h |4 ++-- > arch/arm/include/asm/processor.h |2 +- > arch/arm/kernel/Makefile |2 +- > arch/arm/kernel/entry-header.S |2 +- > arch/arm/kernel/ptrace.c |4 ++-- > arch/powerpc/include/asm/cputable.h |4 ++-- > arch/powerpc/include/asm/hw_breakpoint.h |6 +++--- > arch/powerpc/include/asm/processor.h |4 ++-- > arch/powerpc/kernel/Makefile |2 +- > arch/powerpc/kernel/process.c| 18 +- > arch/powerpc/kernel/ptrace.c | 13 +++-- > arch/powerpc/lib/Makefile|2 +- > arch/sh/kernel/Makefile |2 +- > arch/sh/kernel/cpu/sh4a/Makefile |2 +- > include/linux/hw_breakpoint.h|6 +++--- > include/linux/perf_event.h |4 ++-- > include/linux/ptrace.h |6 +++--- > include/linux/sched.h|2 +- > kernel/events/Makefile |2 +- > kernel/events/core.c |4 ++-- > kernel/ptrace.c |4 ++-- > samples/Kconfig |2 +- > 22 files changed, 49 insertions(+), 48 deletions(-) > Making the hardware breakpoint patches modular has always been a goal. I've looked at the PowerPC parts of the code and they look harmless. Acked-by: K.Prasad Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/5] powerpc, hw_breakpoints: Fix racy access to ptrace breakpoints
On Fri, Apr 22, 2011 at 03:16:27PM +0200, Frederic Weisbecker wrote: > (resend with ppc list in cc) > > While the tracer accesses ptrace breakpoints, the child task may > concurrently exit due to a SIGKILL and thus release its breakpoints > at the same time. We can then dereference some freed pointers. > > To fix this, hold a reference on the child breakpoints before > manipulating them. > > Reported-by: Oleg Nesterov > Signed-off-by: Frederic Weisbecker > Cc: Ingo Molnar > Cc: Benjamin Herrenschmidt > Cc: Peter Zijlstra > Cc: Will Deacon > Cc: Prasad > Cc: Paul Mundt > Cc: v2.6.33.. > --- > arch/powerpc/kernel/ptrace.c |3 +++ > 1 files changed, 3 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c > index 55613e3..4edeeb3 100644 > --- a/arch/powerpc/kernel/ptrace.c > +++ b/arch/powerpc/kernel/ptrace.c > @@ -1591,7 +1591,10 @@ long arch_ptrace(struct task_struct *child, long > request, > } > > case PTRACE_SET_DEBUGREG: > + if (ptrace_get_breakpoints(child) < 0) > + return -ESRCH; > ret = ptrace_set_debugreg(child, addr, data); > + ptrace_put_breakpoints(child); > break; > > #ifdef CONFIG_PPC64 > -- > 1.7.3.2 > Hi Frederic, Looks fine to me. Acked-by: K.Prasad Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
On Mon, Feb 07, 2011 at 04:13:37PM +1100, Benjamin Herrenschmidt wrote:
> On Mon, 2011-02-07 at 10:22 +0530, K.Prasad wrote:
> > Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around
> > the
> > definition of 'set_debug_reg_defaults'.
>
> Can you send a patch ?
>
Pasted below.
> > There's indeed too much sprinkling of #ifdefs in the code, but most of
> > it would go away when the BookE code also uses the generic
> > hw-breakpoint interfaces.
>
> What's your status for those patches ?
>
The last I checked, I knew that the version of patchset I sent out
(linuxppc-dev message id: 20100629165152.ga8...@in.ibm.com)
didn't actually work when tested on hardware (it was just compile tested
before sending). It needs substantial amount of time to get it in shape,
which I'm unfortunately lacking now (plenty of priority items in hand that
needs dedicated attention).
Thanks,
K.Prasad
Fix the error in spelling the config option for hw-breakpoints and fix
the build issue that follows.
Signed-off by: K.Prasad
---
arch/powerpc/kernel/process.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
Index: linux-2.6.set_hwdebug/arch/powerpc/kernel/process.c
===
--- linux-2.6.set_hwdebug.orig/arch/powerpc/kernel/process.c
+++ linux-2.6.set_hwdebug/arch/powerpc/kernel/process.c
@@ -353,6 +353,7 @@ static void switch_booke_debug_regs(stru
prime_debug_regs(new_thread);
}
#else /* !CONFIG_PPC_ADV_DEBUG_REGS */
+#ifndef CONFIG_HAVE_HW_BREAKPOINT
static void set_debug_reg_defaults(struct thread_struct *thread)
{
if (thread->dabr) {
@@ -360,6 +361,7 @@ static void set_debug_reg_defaults(struc
set_dabr(0);
}
}
+#endif /* !CONFIG_HAVE_HW_BREAKPOINT */
#endif /* CONFIG_PPC_ADV_DEBUG_REGS */
int set_dabr(unsigned long dabr)
@@ -670,11 +672,11 @@ void flush_thread(void)
{
discard_lazy_cpu_state();
-#ifdef CONFIG_HAVE_HW_BREAKPOINTS
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
flush_ptrace_hw_breakpoint(current);
-#else /* CONFIG_HAVE_HW_BREAKPOINTS */
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
set_debug_reg_defaults(¤t->thread);
-#endif /* CONFIG_HAVE_HW_BREAKPOINTS */
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
}
void
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
On Mon, Feb 07, 2011 at 02:10:39PM +1100, Benjamin Herrenschmidt wrote: > On Mon, 2011-02-07 at 08:26 +0530, K.Prasad wrote: > > On Mon, Feb 07, 2011 at 09:54:13AM +1100, Benjamin Herrenschmidt wrote: > > > A typo in the #ifdef statement makes us never call it > > > in flush_thread() > > > > > > > I wish it never compiled for such typos :-) > > > > > > > Signed-off-by: Benjamin Herrenschmidt > > Acked-by: K.Prasad > > Interestingly, that 'fix' now breaks the build: > > cc1: warnings being treated as errors > /home/benh/linux-powerpc-test/arch/powerpc/kernel/process.c:356: error: > 'set_debug_reg_defaults' defined but not used > > This file is is becoming an absolute mess of ifdef's in large part due > to the new BookE debug stuff and your HW breakpoint stuff... Any chance > you and Shaggy see if you can improve that situation a bit ? > > Cheers, > Ben. > > Okay! Another wrapper of "#ifndef CONFIG_HAVE_HW_BREAKPOINT" around the definition of 'set_debug_reg_defaults'. There's indeed too much sprinkling of #ifdefs in the code, but most of it would go away when the BookE code also uses the generic hw-breakpoint interfaces. Given the advanced debug features that BookE supports, it's unfortunately not that straight-forward (needs additions to generic hw-breakpoint infrastructure). Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] powerpc: Fix call to flush_ptrace_hw_breakpoint()
On Mon, Feb 07, 2011 at 09:54:13AM +1100, Benjamin Herrenschmidt wrote:
> A typo in the #ifdef statement makes us never call it
> in flush_thread()
>
I wish it never compiled for such typos :-)
> Signed-off-by: Benjamin Herrenschmidt
Acked-by: K.Prasad
> ---
>
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 7a1d5cb..c4e4eab 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -670,11 +670,11 @@ void flush_thread(void)
> {
> discard_lazy_cpu_state();
>
> -#ifdef CONFIG_HAVE_HW_BREAKPOINTS
> +#ifdef CONFIG_HAVE_HW_BREAKPOINT
> flush_ptrace_hw_breakpoint(current);
> -#else /* CONFIG_HAVE_HW_BREAKPOINTS */
> +#else /* CONFIG_HAVE_HW_BREAKPOINT */
> set_debug_reg_defaults(¤t->thread);
> -#endif /* CONFIG_HAVE_HW_BREAKPOINTS */
> +#endif /* CONFIG_HAVE_HW_BREAKPOINT */
> }
>
> void
>
>
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc_set_hwdebug vs ptrace_set_debugreg
On Thu, Dec 16, 2010 at 06:07:47PM +0100, Andreas Schwab wrote: > "K.Prasad" writes: > > > How about the revised patch below? It is only compile-tested; have you > > got a quick test case that I can run? > > It crashes the kernel when running the watch-vfork test. > > Andreas. > Hi Andreas, I tried running it multiple times but saw no crash (or error messages in dmesg). Can you send me the crash logs? What's the behaviour when the testcase is run on an unpatched kernel? The watch-vfork test actually fails on my system (4 unexpected failures) irrespective of the kernel containing the patch or not. Thanks, K.Prasad P.S.: I'd been on vacation and couldn't look at this issue during then. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc_set_hwdebug vs ptrace_set_debugreg
On Mon, Dec 13, 2010 at 08:05:36PM +0100, Andreas Schwab wrote:
> "K.Prasad" writes:
>
> > +#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;
>
> That cannot work, see
> <http://permalink.gmane.org/gmane.linux.ports.ppc64.devel/71418>.
>
Ok. The above patch makes it a bit easy.
How about the revised patch below? It is only compile-tested; have you
got a quick test case that I can run?
Enable PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG to use the generic
hardware breakpoint interfaces. This helps prevent conflict for the use of
DABR register in the absence of CONFIG_PPC_ADV_DEBUG_REGS and when
PTRACE_SET_DEBUGREG/PTRACE_GET_DEBUGREG flags are used by ptrace.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/ptrace.c | 32
1 file changed, 32 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,10 @@ 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 */
#ifndef CONFIG_PPC_ADV_DEBUG_REGS
unsigned long dabr;
#endif
@@ -1365,6 +1369,10 @@ 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;
@@ -1376,6 +1384,20 @@ static long ppc_set_hwdebug(struct task_
if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
dabr |= DABR_DATA_WRITE;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ /* Create a new breakpoint request if one doesn't exist already */
+ hw_breakpoint_init(&attr);
+ attr.bp_addr = dabr & ~HW_BREAKPOINT_ALIGN;
+ arch_bp_generic_fields(dabr & (DABR_DATA_WRITE | DABR_DATA_READ),
+ &attr.bp_type);
+
+ bp = register_user_hw_breakpoint(&attr, ptrace_triggered, child);
+ if (IS_ERR(bp))
+ return PTR_ERR(bp);
+
+ child->thread.ptrace_bps[0] = bp;
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+
child->thread.dabr = dabr;
return 1;
@@ -1405,6 +1427,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;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc_set_hwdebug vs ptrace_set_debugreg
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" 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
---
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;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc_set_hwdebug vs ptrace_set_debugreg
On Mon, Nov 29, 2010 at 11:15:51AM +0100, Andreas Schwab wrote: > "K.Prasad" 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 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: ppc_set_hwdebug vs ptrace_set_debugreg
On Sat, Nov 27, 2010 at 08:36:30PM +0100, Andreas Schwab wrote: > Why does ptrace_set_debugreg call register_user_hw_breakpoint, but > ppc_set_hwdebug doesn't? Shouldn't ppc_set_hwdebug set the > DABR_DATA_(READ|WRITE|TRANSLATION) bits in the dabr? > > Andreas. The hw-breakpoint interfaces were initially planned for the old ptrace option PTRACE_SET_DEBUGREG,while, the newer ptrace options are mostly to exploit the advanced debug features of Book3E processors. 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(). This was planned to be done alongside the conversion of all ptrace options enabled by CONFIG_PPC_ADV_DEBUG_REGS, which is yet to be done. Are you looking to use debug registers through perf, or somesuch, that you need register_user_hw_breakpoint() to be used for these new ptrace flags? Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 02/13] powerpc/book3e: Hack to get gdb moving along on Book3E 64-bit
On Fri, Jul 09, 2010 at 04:16:44PM +1000, Benjamin Herrenschmidt wrote:
Hi,
A few questions and some trivial comments below.
> Our handling of debug interrupts on Book3E 64-bit is not quite
> the way it should be just yet. This is a workaround to let gdb
> work at least for now. We ensure that when context switching,
> we set the appropriate DBCR0 value for the new task. We also
> make sure that we turn off MSR[DE] within the kernel, and set
> it as part of the bits that get set when going back to userspace.
>
I think I'm missing the code where MSR_DE is set before returning
to user-space? I just found one instance where MSR_USER64 (which now
includes MSR_DE) is used (in start_thread()). If not set, we'll lose the
interrupts caused in IDM too.
> In the long run, we will probably set the userspace DBCR0 on the
> exception exit code path and ensure we have some proper kernel
> value to set on the way into the kernel, a bit like ppc32 does,
> but that will take more work.
The effort to port ppc32 BookIII E debug register usage to use generic
hw-breakpoint interfaces (linuxppc-dev message-id:
20100629165152.ga8...@in.ibm.com), in its final form, should cleanup
most of this code. Even the hook switch_booke_debug_regs() in
__switch_to() should be done away.
> Signed-off-by: Benjamin Herrenschmidt
> ---
> arch/powerpc/include/asm/reg_booke.h |4 ++--
> arch/powerpc/kernel/process.c| 22 ++
> 2 files changed, 24 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/reg_booke.h
> b/arch/powerpc/include/asm/reg_booke.h
> index 2360317..66dc6f0 100644
> --- a/arch/powerpc/include/asm/reg_booke.h
> +++ b/arch/powerpc/include/asm/reg_booke.h
> @@ -29,8 +29,8 @@
> #if defined(CONFIG_PPC_BOOK3E_64)
> #define MSR_ MSR_ME | MSR_CE
> #define MSR_KERNEL MSR_ | MSR_CM
> -#define MSR_USER32 MSR_ | MSR_PR | MSR_EE
> -#define MSR_USER64 MSR_USER32 | MSR_CM
> +#define MSR_USER32 MSR_ | MSR_PR | MSR_EE | MSR_DE
> +#define MSR_USER64 MSR_USER32 | MSR_CM | MSR_DE
MSR_DE is included twice in MSR_USER64 (once through MSR_USER32).
> #elif defined (CONFIG_40x)
> #define MSR_KERNEL (MSR_ME|MSR_RI|MSR_IR|MSR_DR|MSR_CE)
> #define MSR_USER (MSR_KERNEL|MSR_PR|MSR_EE)
> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
> index 1e78453..551f671 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -477,6 +477,28 @@ struct task_struct *__switch_to(struct task_struct *prev,
> new_thread = &new->thread;
> old_thread = ¤t->thread;
>
> +#if defined(CONFIG_PPC_BOOK3E_64)
> + /* XXX Current Book3E code doesn't deal with kernel side DBCR0,
You may want to use the style
/*
* ..
> + * we always hold the user values, so we set it now.
> + *
> + * However, we ensure the kernel MSR:DE is appropriately cleared too
> + * to avoid spurrious single step exceptions in the kernel.
^^^spurious^^^
> + *
> + * This will have to change to merge with the ppc32 code at some point,
> + * but I don't like much what ppc32 is doing today so there's some
> + * thinking needed there
> + */
> + if ((new_thread->dbcr0 | old_thread->dbcr0) & DBCR0_IDM) {
> + u32 dbcr0;
thread->dbcr0 is defined as "unsigned long" in processor.h however "u32 dbcr0"
here must be fine (given that DBCR0 uses 32-bits in ppc32 and uses only 32:63
bits in BOOKIIIE_64). Should dbcr<0-n> be made u32, given that there
will be no 64-bit long value to store (or am I missing something)?
Thanks,
K.Prasad
> +
> + mtmsr(mfmsr() & ~MSR_DE);
> + isync();
> + dbcr0 = mfspr(SPRN_DBCR0);
> + dbcr0 = (dbcr0 & DBCR0_EDM) | new_thread->dbcr0;
> + mtspr(SPRN_DBCR0, dbcr0);
> + }
> +#endif /* CONFIG_PPC64_BOOK3E */
> +
> #ifdef CONFIG_PPC64
> /*
>* Collect processor utilization data per process
> --
> 1.6.3.3
>
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC Patch 1/1] Implement hardware breakpoint interfaces for PowerPC BookE processors
Introduce support for generic hw-breakpoint interfaces for PowerPC
BookIII E processors.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |2
arch/powerpc/include/asm/cputable.h|4
arch/powerpc/include/asm/hw_breakpoint_booke.h | 46 ++
arch/powerpc/kernel/Makefile |4
arch/powerpc/kernel/hw_breakpoint_booke.c | 429 +
arch/powerpc/kernel/process.c |3
arch/powerpc/kernel/ptrace.c | 51 ++
arch/powerpc/kernel/traps.c|9
include/linux/perf_event.h |4
kernel/trace/trace_ksym.c |4
10 files changed, 554 insertions(+), 2 deletions(-)
Index: linux-2.6.bookE/arch/powerpc/include/asm/hw_breakpoint_booke.h
===
--- /dev/null
+++ linux-2.6.bookE/arch/powerpc/include/asm/hw_breakpoint_booke.h
@@ -0,0 +1,46 @@
+#ifndef_I386_HW_BREAKPOINT_H
+#define_I386_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#define__ARCH_HW_BREAKPOINT_H
+
+struct arch_hw_breakpoint {
+ unsigned intlen;
+ unsigned long address;
+ unsigned long type;
+};
+
+#include
+#include
+#include
+
+/* Breakpoint length beyond which we should use 'range' breakpoints */
+#define DAC_LEN 8
+#define HW_BREAKPOINT_ALIGN 0x3
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+
+struct perf_event;
+struct perf_sample_data;
+struct pmu;
+
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+extern void hw_breakpoint_handler(struct pt_regs *regs,
+ unsigned long debug_status);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+
+extern struct pmu perf_ops_bp;
+
+#endif /* __KERNEL__ */
+#endif /* _I386_HW_BREAKPOINT_H */
+
Index: linux-2.6.bookE/arch/powerpc/kernel/hw_breakpoint_booke.c
===
--- /dev/null
+++ linux-2.6.bookE/arch/powerpc/kernel/hw_breakpoint_booke.c
@@ -0,0 +1,429 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers for PowerPC BookIII E. Derived from
+ * "arch/powerpc/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * Stores the breakpoints currently in use on each debug register for each CPU
+ * register for each cpus
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
+
+int hw_breakpoint_weight(struct perf_event *bp)
+{
+ return (bp->attr.bp_len > DAC_LEN) ? 2 : 1;
+}
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint. Eventually we enable it in the debug control register.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ int i;
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ unsigned long dbcr0 = mfspr(SPRN_DBCR0);
+
+ for (i = 0; i < HBP_NUM; i++) {
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg[i]);
+
+ if (*slot)
+ continue;
+ *slot = bp;
+ mtspr(SPRN_DAC1, info->address);
+ /* Clean the 'type' fields from DBCR0 to erase old values */
+ dbcr0 &= ~(DBCR0_DAC1W | DBCR0_DAC1R | DB
[RFC Patch 0/1] hw-breakpoint interfaces for BookE - ver II
Hi All, Please find a new version of the patch that implements generic hw-breakpoint interfaces for PowerPC BookIII E. While there are several improvements over the previous version in terms of code placement, support for ptrace and fixes for incorrect issues, the patch remains compile-tested (only)(against 44x/iss476-smp_defconfig config) and is yet to be tested on hardware (for functionality). Changelog Version II (Version I:linuxppc-dev message-id: 20100427164029.ga8...@in.ibm.com) - - ptrace requests (through PTRACE_SET_DEBUGREG and PTRACE_GET_DEBUGREG) are now supported. - Changes in original arch/powerpc/kernel/hw_breakpoint.c have been percolated down to hw_breakpoint_booke.c (such as introduction of thread_change_pc(), arch_unregister_hw_breakpoint() and changes to hw_breakpoint_handler()) along with some code re-arrangement (for brevity/clarity). The usage of CONFIG (in this patch) option doesn't largely represent a form that it would turn into after enabling the use of DVCs, DACs by non-ptrace breakpoint requests. They should turn correct as versions of this patch progress. Kindly let me know your comments. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXII
On Tue, Jun 15, 2010 at 11:54:59AM +1000, Paul Mackerras wrote: > On Fri, Jun 04, 2010 at 12:21:45PM +0530, K.Prasad wrote: > > > Meanwhile I tested the per-cpu breakpoints with the new emulate_step > > patch (refer linuxppc-dev message-id: > > 20100602112903.gb30...@brick.ozlabs.ibm.com) and they continue to fail > > due to emulate_step() failure, in my case, on a "lwz r0,0(r28)" > > instruction. > > You need to pass the instruction word to emulate_step(), not the > instruction address. Also you need to have the full GPR set > available. The patch below fixes these problems. I'll fold these > changes into your patch 2/5. > > Paul. The breakpoints that used to fail before (due to emulate_step() failure) are now working fine upon applying this patch. Please include this patch along with my 2/5 as indicated by you. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 5/5] PPC64-HWBKPT: Discard extraneous interrupt due to accesses outside symbol length
On Thu, Jun 10, 2010 at 10:40:24PM +1000, Paul Mackerras wrote:
> On Wed, Jun 09, 2010 at 03:55:59PM +0530, K.Prasad wrote:
>
> > + if (!((bp->attr.bp_addr <= dar) &&
> > +(dar <= (bp->attr.bp_addr + bp->attr.bp_len {
> > + /*
> > +* This exception is triggered not because of a memory access
> > +* on the monitored variable but in the double-word address
> > +* range in which it is contained. We will consume this
> > +* exception, considering it as 'noise'.
> > +*/
> > + info->extraneous_interrupt = true;
> > + }
>
> Ummm, don't you need to add "else info->extraneous_interrupt = false;"
> here? I don't see anywhere that you ever clear it otherwise.
>
> Also, I think you need to do the "if (!info->extraneous_interrupt)"
> check around the call to perf_bp_event() later on in
> hw_breakpoint_handler() as well as around the call in
> single_step_dabr_instruction().
>
> Paul.
True, I've added the check before perf_bp_event() (wherever it was
missing before) in the patchset ver XXIV.
Thanks,
K.Prasad
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 5/5] PPC64-HWBKPT: Discard extraneous interrupt due to accesses outside symbol length
Many a times, the requested breakpoint length can be less than the fixed
breakpoint length i.e. 8 bytes supported by PowerPC BookIII S. This could lead
to extraneous interrupts resulting in false breakpoint notifications. The patch
below detects and discards such interrupts for non-ptrace requests (we don't
want to change ptrace behaviour for fear of breaking compatability).
[Suggestions from Paul Mackerras to add a new flag in
'struct arch_hw_breakpoint' to identify extraneous interrupts]
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |1 +
arch/powerpc/kernel/hw_breakpoint.c | 23 +--
2 files changed, 22 insertions(+), 2 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -202,6 +202,7 @@ int __kprobes hw_breakpoint_handler(stru
struct pt_regs *regs = args->regs;
int stepped = 1;
struct arch_hw_breakpoint *info;
+ unsigned long dar = regs->dar;
/* Disable breakpoints during exception handling */
set_dabr(0);
@@ -232,6 +233,22 @@ int __kprobes hw_breakpoint_handler(stru
goto out;
}
+ /*
+* Verify if dar lies within the address range occupied by the symbol
+* being watched to filter extraneous exceptions.
+*/
+ if (!((bp->attr.bp_addr <= dar) &&
+(dar <= (bp->attr.bp_addr + bp->attr.bp_len {
+ /*
+* This exception is triggered not because of a memory access
+* on the monitored variable but in the double-word address
+* range in which it is contained. We will consume this
+* exception, considering it as 'noise'.
+*/
+ info->extraneous_interrupt = true;
+ } else
+ info->extraneous_interrupt = false;
+
/* Do not emulate user-space instructions, instead single-step them */
if (user_mode(regs)) {
bp->ctx->task->thread.last_hit_ubp = bp;
@@ -255,7 +272,8 @@ int __kprobes hw_breakpoint_handler(stru
* As a policy, the callback is invoked in a 'trigger-after-execute'
* fashion
*/
- perf_bp_event(bp, regs);
+ if (!info->extraneous_interrupt)
+ perf_bp_event(bp, regs);
set_dabr(info->address | info->type | DABR_TRANSLATION);
out:
@@ -286,7 +304,8 @@ int __kprobes single_step_dabr_instructi
* We shall invoke the user-defined callback function in the single
* stepping handler to confirm to 'trigger-after-execute' semantics
*/
- perf_bp_event(bp, regs);
+ if (!bp_info->extraneous_interrupt)
+ perf_bp_event(bp, regs);
/*
* Do not disable MSR_SE if the process was already in
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -27,6 +27,7 @@
#ifdef CONFIG_HAVE_HW_BREAKPOINT
struct arch_hw_breakpoint {
+ boolextraneous_interrupt;
u8 len; /* length of the target data symbol */
int type;
unsigned long address;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 4/5] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery.
Restore hw-breakpoints if the user-context is altered in the signal handler.
[With inputs from Paul Mackerras which helped identify a
need to restore breakpoints before handling a signal delivered in certain
cases]
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |3 +++
arch/powerpc/kernel/hw_breakpoint.c | 18 ++
arch/powerpc/kernel/signal.c |3 +++
3 files changed, 24 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -65,9 +65,12 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
#else /* CONFIG_HAVE_HW_BREAKPOINT */
static inline void hw_breakpoint_disable(void) { }
+static inline void thread_change_pc(struct task_struct *tsk,
+ struct pt_regs *regs) { }
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -174,6 +174,24 @@ int arch_validate_hwbkpt_settings(struct
}
/*
+ * Restores the breakpoint on the debug registers.
+ * Invoke this function if it is known that the execution context is about to
+ * change to cause loss of MSR_SE settings.
+ */
+void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
+{
+ struct arch_hw_breakpoint *info;
+
+ if (likely(!tsk->thread.last_hit_ubp))
+ return;
+
+ info = counter_arch_bp(tsk->thread.last_hit_ubp);
+ regs->msr &= ~MSR_SE;
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+ tsk->thread.last_hit_ubp = NULL;
+}
+
+/*
* Handle debug exception notifications.
*/
int __kprobes hw_breakpoint_handler(struct die_args *args)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
@@ -149,6 +150,8 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+ /* Re-enable the breakpoints for the signal stack */
+ thread_change_pc(current, regs);
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 3/5] PPC64-HWBKPT: Handle concurrent alignment interrupts
An alignment interrupt may intervene between a DSI/hw-breakpoint exception
and the single-step exception. Enable the alignment interrupt (through
modifications to emulate_single_step()) to notify the single-step exception
handler for proper restoration of hw-breakpoints.
[The need to invoke single-step handler after alignment interrupt pointed out
by Paul Mackerras ]
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/traps.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
@@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
void __kprobes single_step_exception(struct pt_regs *regs)
{
- regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
+ clear_single_step(regs);
if (notify_die(DIE_SSTEP, "single_step", regs, 5,
5, SIGTRAP) == NOTIFY_STOP)
@@ -621,10 +621,8 @@ void __kprobes single_step_exception(str
*/
static void emulate_single_step(struct pt_regs *regs)
{
- if (single_stepping(regs)) {
- clear_single_step(regs);
- _exception(SIGTRAP, regs, TRAP_TRACE, 0);
- }
+ if (single_stepping(regs))
+ single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/5] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC BookIII S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
[Suggestions from Paul Mackerras to
- emulate_step() all non-task bound breakpoints and single-step only the
per-task breakpoints
- perform arch-specific cleanup before unregistration through
arch_unregister_hw_breakpoint()
]
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 73 +++
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 320 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c| 14 +
arch/powerpc/kernel/ptrace.c | 64 ++
arch/powerpc/lib/Makefile|1
10 files changed, 489 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,73 @@
+/*
+ * PowerPC BookIII S hardware breakpoint definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010, IBM Corporation.
+ * Author: K.Prasad
+ *
+ */
+
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
+static inline void hw_breakpoint_disable(void) { }
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,320 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#inc
[Patch 1/5] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
Acked-by: Frederic Weisbecker
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXIV
Hi Paul, Please find a new set of patches with changes as described below. The patches have been tested for kernel- and user-mode breakpoints (after applying the two patches you sent, refer message-id: 20100603004758.ga19...@brick.ozlabs.ibm.com and message-id: 20100615015459.ga30...@drongo) and are found to work fine! Changelog - ver XXIV (Version XXII: 20100609102446.ga20...@in.ibm.com) - A lockdep issue seen due to invocation of unregister_hw_breakpoint() in exception context is now fixed (perf_event_disable() is used instead). - Fixed improper handling of extraneous exceptions (through clearing of extraneous exception flag and preventing the invocation of callback). Kindly let me know if you have any further comments. Thanks, K.Prasad Changelog - ver XXIII (Version XXII: 20100528063924.ga8...@in.ibm.com) - Detection of extraneous breakpoint exceptions is now done using a boolean flag in 'struct arch_hw_breakpoint'. - A dangling put_cpu() (remnant from previous patch versions) in arch_unregister_hw_breakpoint() is now removed. Changelog - ver XXII (Version XXI: linuxppc-dev ref:20100525091314.ga29...@in.ibm.com) - Extraneous breakpoint exceptions are now properly handled; causative instruction will be single-stepped and debug register values restored. - Restoration of breakpoints during signal handling through thread_change_pc() had defects which are now fixed. - Breakpoints are flushed through flush_ptrace_hw_breakpoint() call in both flush_thread() and prepare_to_copy() functions. 'ptrace_bps[]' and 'last_hit_ubp' members are now promptly cleaned-up. - Single-step exception is now conditionally emulated upon hitting alignment_exception. - Rebased to commit 31f46717997a83bdf6db0dd04810c0a329eb3148 of linux-2.6 tree. Changelog - ver XXI (Version XX: linuxppc-dev ref:20100524103136.ga8...@in.ibm.com) - Decision to emulate an instruction is now based on whether the causative instruction is in user_mode() or not. - Breakpoints don't have to be cleared during sigreturn. A 'double-hit' on hw_breakpoint_handler() is harmless for non-ptrace instructions. - Minor changes to aid code brevity. Changelog - ver XX (Version XIX: linuxppc-dev ref: 20100524040137.ga20...@in.ibm.com) - Non task-bound breakpoints will only be emulated. Breakpoint will be unregistered with a warning if emulation fails. Changelog - ver XIX (Version XVIII: linuxppc-dev ref: 20100512033055.ga6...@in.ibm.com) - Increased coverage of breakpoints during concurrent alignment_exception and signal handling (which previously had 'blind-spots'). - Support for kernel-thread breakpoints and kernel-space breakpoints inside the context of a user-space process. - Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby necessitating minor changes to arch_validate_hwbkpt_settings(). Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - Slight code restructuring for brevity and coding-style corrections. - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - hw-breakpoint restoration variables are cleaned-up before unregistration through arch_unregister_hw_breakpoint(). - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Mov
Re: [Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXII
On Mon, Jun 07, 2010 at 09:25:59PM +1000, Paul Mackerras wrote: > On Mon, Jun 07, 2010 at 12:33:51PM +0530, K.Prasad wrote: > > > Given that 'ptrace_bps' is used only for ptrace originated breakpoints > > and that we return early i.e. before detecting extraneous interrupts > > in hw_breakpoint_handler() (as shown above) they shouldn't overlap each > > other. The following comment in hw_breakpoint_handler() explains the > > same. > > /* > > * To prevent invocation of perf_event_bp(), we shall overload > > * thread.ptrace_bps[] pointer (unused for non-ptrace > > * exceptions) to flag an extraneous interrupt which must be > > * skipped. > > */ > > My point is that while we are using ptrace_bps[0] to mark a non-ptrace > breakpoint that we're single-stepping, some other process could be > ptracing this process and could get into ptrace_set_debugreg() and > would think that the process already has a ptrace breakpoint and call > modify_user_hw_breakpoint() when it should be calling > register_user_hw_breakpoint(). Or this process could die and so we > call flush_ptrace_hw_breakpoint() and it incorrectly thinks we have a > ptrace breakpoint. > > If there is a reason why we can be quite sure that while we are using > current->thread.ptrace_bps[0] in this way, ptrace_set_debugreg() can > never get called with this task as the ptracee, and nor can > flush_ptrace_hw_breakpoint() get called on this task, then maybe it's > safe. But it's not at all obviously safe. So I'd very much rather we > just use an extra flag somewhere, that isn't used elsewhere for > anything else, so we can convince ourselves that it is all correct > without having to look at lots of different pieces of code. > > There are 3 bytes of padding in struct arch_hw_breakpoint; couldn't we > use one of them as a "not really hit" flag? > > Paul. > ___ I get your reasoning now; ptrace_bps[] re-use will cause failures under these circumstances. I've sent a new version of the patchset which adds a new flag in 'struct arch_hw_breakpoint' (I was always thinking of 'struct thread_struct' before and was scared to introduce another new member in it, thereby leading me to incorrectly optimise using ptrace_bps) to flag extraneous_interrupt (Given that it's your idea I've added your signed-off too). Kindly let me know your comments, if any. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 5/5] PPC64-HWBKPT: Discard extraneous interrupt due to accesses outside symbol length
Many a times, the requested breakpoint length can be less than the fixed
breakpoint length i.e. 8 bytes supported by PowerPC BookIII S. This could lead
to extraneous interrupts resulting in false breakpoint notifications. The patch
below detects and discards such interrupts for non-ptrace requests (we don't
want to change ptrace behaviour for fear of breaking compatability).
Signed-off-by: K.Prasad
Signed-off-by: Paul Mackerras
---
arch/powerpc/include/asm/hw_breakpoint.h |1 +
arch/powerpc/kernel/hw_breakpoint.c | 19 ++-
2 files changed, 19 insertions(+), 1 deletion(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -202,6 +202,7 @@ int __kprobes hw_breakpoint_handler(stru
struct pt_regs *regs = args->regs;
int stepped = 1;
struct arch_hw_breakpoint *info;
+ unsigned long dar = regs->dar;
/* Disable breakpoints during exception handling */
set_dabr(0);
@@ -232,6 +233,21 @@ int __kprobes hw_breakpoint_handler(stru
goto out;
}
+ /*
+* Verify if dar lies within the address range occupied by the symbol
+* being watched to filter extraneous exceptions.
+*/
+ if (!((bp->attr.bp_addr <= dar) &&
+(dar <= (bp->attr.bp_addr + bp->attr.bp_len {
+ /*
+* This exception is triggered not because of a memory access
+* on the monitored variable but in the double-word address
+* range in which it is contained. We will consume this
+* exception, considering it as 'noise'.
+*/
+ info->extraneous_interrupt = true;
+ }
+
/* Do not emulate user-space instructions, instead single-step them */
if (user_mode(regs)) {
bp->ctx->task->thread.last_hit_ubp = bp;
@@ -286,7 +302,8 @@ int __kprobes single_step_dabr_instructi
* We shall invoke the user-defined callback function in the single
* stepping handler to confirm to 'trigger-after-execute' semantics
*/
- perf_bp_event(bp, regs);
+ if (!bp_info->extraneous_interrupt)
+ perf_bp_event(bp, regs);
/*
* Do not disable MSR_SE if the process was already in
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -27,6 +27,7 @@
#ifdef CONFIG_HAVE_HW_BREAKPOINT
struct arch_hw_breakpoint {
+ boolextraneous_interrupt;
u8 len; /* length of the target data symbol */
int type;
unsigned long address;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 4/5] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery.
Restore hw-breakpoints if the user-context is altered in the signal handler.
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |3 +++
arch/powerpc/kernel/hw_breakpoint.c | 18 ++
arch/powerpc/kernel/signal.c |3 +++
3 files changed, 24 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -65,9 +65,12 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
#else /* CONFIG_HAVE_HW_BREAKPOINT */
static inline void hw_breakpoint_disable(void) { }
+static inline void thread_change_pc(struct task_struct *tsk,
+ struct pt_regs *regs) { }
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -174,6 +174,24 @@ int arch_validate_hwbkpt_settings(struct
}
/*
+ * Restores the breakpoint on the debug registers.
+ * Invoke this function if it is known that the execution context is about to
+ * change to cause loss of MSR_SE settings.
+ */
+void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
+{
+ struct arch_hw_breakpoint *info;
+
+ if (likely(!tsk->thread.last_hit_ubp))
+ return;
+
+ info = counter_arch_bp(tsk->thread.last_hit_ubp);
+ regs->msr &= ~MSR_SE;
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+ tsk->thread.last_hit_ubp = NULL;
+}
+
+/*
* Handle debug exception notifications.
*/
int __kprobes hw_breakpoint_handler(struct die_args *args)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
@@ -149,6 +150,8 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+ /* Re-enable the breakpoints for the signal stack */
+ thread_change_pc(current, regs);
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 3/5] PPC64-HWBKPT: Handle concurrent alignment interrupts
An alignment interrupt may intervene between a DSI/hw-breakpoint exception
and the single-step exception. Enable the alignment interrupt (through
modifications to emulate_single_step()) to notify the single-step exception
handler for proper restoration of hw-breakpoints.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/traps.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
@@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
void __kprobes single_step_exception(struct pt_regs *regs)
{
- regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
+ clear_single_step(regs);
if (notify_die(DIE_SSTEP, "single_step", regs, 5,
5, SIGTRAP) == NOTIFY_STOP)
@@ -621,10 +621,8 @@ void __kprobes single_step_exception(str
*/
static void emulate_single_step(struct pt_regs *regs)
{
- if (single_stepping(regs)) {
- clear_single_step(regs);
- _exception(SIGTRAP, regs, TRAP_TRACE, 0);
- }
+ if (single_stepping(regs))
+ single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/5] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC BookIII S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 73 +++
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 320 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c| 14 +
arch/powerpc/kernel/ptrace.c | 64 ++
arch/powerpc/lib/Makefile|1
10 files changed, 489 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,73 @@
+/*
+ * PowerPC BookIII S hardware breakpoint definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010, IBM Corporation.
+ * Author: K.Prasad
+ *
+ */
+
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
+static inline void hw_breakpoint_disable(void) { }
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,320 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE
[Patch 1/5] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
Acked-by: Frederic Weisbecker
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXIII
Hi All, Please find a new version of the patchset that implement hardware breakpoint interfaces for the PowerPC BookIII S processor. The changes are few and as described below. Changelog - ver XXIII (Version XXII: 20100528063924.ga8...@in.ibm.com) - Detection of extraneous breakpoint exceptions is now done using a boolean flag in 'struct arch_hw_breakpoint'. - A dangling put_cpu() (remnant from previous patch versions) in arch_unregister_hw_breakpoint() is now removed. Kindly let me know your comments. Thanks, K.Prasad Changelog - ver XXII (Version XXI: linuxppc-dev ref:20100525091314.ga29...@in.ibm.com) - Extraneous breakpoint exceptions are now properly handled; causative instruction will be single-stepped and debug register values restored. - Restoration of breakpoints during signal handling through thread_change_pc() had defects which are now fixed. - Breakpoints are flushed through flush_ptrace_hw_breakpoint() call in both flush_thread() and prepare_to_copy() functions. 'ptrace_bps[]' and 'last_hit_ubp' members are now promptly cleaned-up. - Single-step exception is now conditionally emulated upon hitting alignment_exception. - Rebased to commit 31f46717997a83bdf6db0dd04810c0a329eb3148 of linux-2.6 tree. Changelog - ver XXI (Version XX: linuxppc-dev ref:20100524103136.ga8...@in.ibm.com) - Decision to emulate an instruction is now based on whether the causative instruction is in user_mode() or not. - Breakpoints don't have to be cleared during sigreturn. A 'double-hit' on hw_breakpoint_handler() is harmless for non-ptrace instructions. - Minor changes to aid code brevity. Changelog - ver XX (Version XIX: linuxppc-dev ref: 20100524040137.ga20...@in.ibm.com) - Non task-bound breakpoints will only be emulated. Breakpoint will be unregistered with a warning if emulation fails. Changelog - ver XIX (Version XVIII: linuxppc-dev ref: 20100512033055.ga6...@in.ibm.com) - Increased coverage of breakpoints during concurrent alignment_exception and signal handling (which previously had 'blind-spots'). - Support for kernel-thread breakpoints and kernel-space breakpoints inside the context of a user-space process. - Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby necessitating minor changes to arch_validate_hwbkpt_settings(). Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - Slight code restructuring for brevity and coding-style corrections. - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - hw-breakpoint restoration variables are cleaned-up before unregistration through arch_unregister_hw_breakpoint(). - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver X
Re: [Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXII
On Fri, Jun 04, 2010 at 07:06:48PM +1000, Paul Mackerras wrote:
> On Fri, Jun 04, 2010 at 12:21:45PM +0530, K.Prasad wrote:
>
> > Meanwhile I tested the per-cpu breakpoints with the new emulate_step
> > patch (refer linuxppc-dev message-id:
> > 20100602112903.gb30...@brick.ozlabs.ibm.com) and they continue to fail
> > due to emulate_step() failure, in my case, on a "lwz r0,0(r28)"
> > instruction.
>
> Strange, what was in r28? The emulator should handle that instruction.
>
It must be containing one of the offsets of "struct tracer" which is a
parameter to the function trace_selftest_startup_ksym(). Basically this
function does a selftest over hw-breakpoints by placing read-write
breakpoint on a dummy global-variable and performs read-write access
thereupon. The lwz instruction which fails here corresponds to the
instruction that does read-write. A complete disassemble of the function
upto the failing instruction (at address) is pasted at the end of this
mail.
> > About the latest patchset, given that we chose to ignore extraneous
> > interrupts for non-ptrace breakpoints, I thought that re-using
> > current->thread.ptrace_bps as a flag would be efficient than
> > introducing
> > a new member in 'struct thread_struct' to do the same. I'm not sure
> > if
> > you foresee any issues with that.
>
> I just wonder what provides exclusion between its use as a flag and
> its use to hold a real ptrace breakpoint. As far as I can see nothing
> does. If there is something, it's off in some other source file,
> unless I'm missing something. And in that case there should be a bit
> fat comment explaining why it's safe.
>
The hw_breakpoint_handler() goes like this:
int __kprobes hw_breakpoint_handler(struct die_args *args)
{
...
/*
* Return early after invoking user-callback function without restoring
* DABR if the breakpoint is from ptrace which always operates in
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
* generated in do_dabr().
*/
if (is_ptrace_bp) {
perf_bp_event(bp, regs);
rc = NOTIFY_DONE;
goto out;
}
/*
* Verify if dar lies within the address range occupied by the symbol
* being watched to filter extraneous exceptions.
*/
if (!((bp->attr.bp_addr <= dar) &&
(dar <= (bp->attr.bp_addr + bp->attr.bp_len {
/*
* This exception is triggered not because of a memory access
* on the monitored variable but in the double-word address
* range in which it is contained. We will consume this
* exception, considering it as 'noise'.
*/
is_extraneous_interrupt = true;
}
...
}
Given that 'ptrace_bps' is used only for ptrace originated breakpoints
and that we return early i.e. before detecting extraneous interrupts
in hw_breakpoint_handler() (as shown above) they shouldn't overlap each
other. The following comment in hw_breakpoint_handler() explains the
same.
/*
* To prevent invocation of perf_event_bp(), we shall overload
* thread.ptrace_bps[] pointer (unused for non-ptrace
* exceptions) to flag an extraneous interrupt which must be
* skipped.
*/
Thanks,
K.Prasad
(gdb) disass trace_selftest_startup_ksym
Dump of assembler code for function trace_selftest_startup_ksym:
0xc0125580 : mflrr0
0xc0125584 : std r26,-48(r1)
0xc0125588 : std r27,-40(r1)
0xc012558c :std r28,-32(r1)
0xc0125590 :std r29,-24(r1)
0xc0125594 :std r30,-16(r1)
0xc0125598 :std r31,-8(r1)
0xc012559c :std r0,16(r1)
0xc01255a0 :stdur1,-176(r1)
0xc01255a4 :mr r31,r1
0xc01255a8 :ld r30,-17784(r2)
0xc01255ac :mr r26,r4
0xc01255b0 :mr r27,r3
0xc01255b4 :bl
0xc0125510
0xc01255b8 :li r4,3
0xc01255bc :mr. r29,r3
0xc01255c0 :mr r5,r29
0xc01255c4 :beq
0xc01255e0
0xc01255c8 :ld r3,-31520(r30)
0xc01255cc :ld r4,0(r27)
0xc01255d0 :bl
0xc009c560
0xc01255d4 :nop
0xc01255d8 :b
0xc0125690
0xc01255dc :nop
0xc01255e0 :ld r28,-31512(r30)
0xc01255e4 : ld r3,-31504(r30)
0xc01255e8 : stw r29,0(r28)
0xc01255ec : mr r5,r28
0xc01255f0 : bl
Re: [Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXII
On Wed, Jun 02, 2010 at 09:33:16PM +1000, Paul Mackerras wrote: > On Fri, May 28, 2010 at 12:09:24PM +0530, K.Prasad wrote: > > > Please find a new set of patches that have the following changes. > > Thanks. There are a couple of minor things still remaining (dangling > put_cpu in arch_unregister_hw_breakpoint, plus I don't think reusing > current->thread.ptrace_bps the way you did in patch 5/5 is a good > idea), but I think at this stage I'll put them in a tree together > with my latest emulate_step version and then push them to Ben H and/or > Ingo Molnar once I've done some testing. > > Paul. Hi Paul, Thanks for agreeing to put the patchset into a tree and push it to the appropriate maintainers. Meanwhile I tested the per-cpu breakpoints with the new emulate_step patch (refer linuxppc-dev message-id: 20100602112903.gb30...@brick.ozlabs.ibm.com) and they continue to fail due to emulate_step() failure, in my case, on a "lwz r0,0(r28)" instruction. About the latest patchset, given that we chose to ignore extraneous interrupts for non-ptrace breakpoints, I thought that re-using current->thread.ptrace_bps as a flag would be efficient than introducing a new member in 'struct thread_struct' to do the same. I'm not sure if you foresee any issues with that. If so, I'd like to send a new patch (rather than a new version of the complete patchset) to fix it along with the dangling put_cpu() in arch_unregister_hw_breakpoint() (I forgot to remove parts of the code between versions XIX and XX). Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC PATCH] powerpc: Emulate most load and store instructions in emulate_step()
On Thu, May 20, 2010 at 10:49:55PM +1000, Paul Mackerras wrote: > This extends the emulate_step() function to handle most of the load > and store instructions implemented on current 64-bit server processors. > The aim is to handle all the load and store instructions used in the > kernel, so this handles the Altivec/VMX lvx and stvx and the VSX > lxv2dx and stxv2dx instructions (implemented in POWER7). > Can the emulate_step() function be used on BookIII E processors as well (arch/powerpc/kernel/kprobes.c invokes it irrespective of the host processor though)? If yes, we can use it with hw_breakpoint_handler() of BookE processors (like what is done on the PPC64 counterpart). > The new code can emulate user mode instructions, and checks the > effective address for a load or store if the saved state is for > user mode. It doesn't handle little-endian mode at present. > > For floating-point, Altivec/VMX and VSX instructions, it checks > that the saved MSR has the enable bit for the relevant facility > set, and if so, assumes that the FP/VMX/VSX registers contain > valid state, and does loads or stores directly to/from the > FP/VMX/VSX registers, using assembly helpers in ldstfp.S. > Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 3/4] PPC64-HWBKPT: Handle concurrent alignment interrupts
On Thu, May 27, 2010 at 04:20:44PM +1000, Paul Mackerras wrote:
> On Tue, May 25, 2010 at 02:44:35PM +0530, K.Prasad wrote:
>
> > An alignment interrupt may intervene between a DSI/hw-breakpoint exception
> > and the single-step exception. Enable the alignment interrupt (through
> > modifications to emulate_single_step()) to notify the single-step exception
> > handler for proper restoration of hw-breakpoints.
> >
> > Signed-off-by: K.Prasad
> > ---
> > arch/powerpc/kernel/traps.c |7 ++-
> > 1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
> > ===
> > --- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
> > +++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
> > @@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
> >
> > void __kprobes single_step_exception(struct pt_regs *regs)
> > {
> > - regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
> > + clear_single_step(regs);
> >
> > if (notify_die(DIE_SSTEP, "single_step", regs, 5,
> > 5, SIGTRAP) == NOTIFY_STOP)
> > @@ -621,10 +621,7 @@ void __kprobes single_step_exception(str
> > */
> > static void emulate_single_step(struct pt_regs *regs)
> > {
> > - if (single_stepping(regs)) {
> > - clear_single_step(regs);
> > - _exception(SIGTRAP, regs, TRAP_TRACE, 0);
> > - }
> > + single_step_exception(regs);
> > }
>
> We still need the if (single_stepping(regs)) in emulate_single_step.
> We don't want to send the process a SIGTRAP every time it gets an
> alignment interrupt. :)
>
> Paul.
Agreed, and made changes to that effect in version XXII (as seen in
patch linuxppc-dev message-id: 20100528064017.gd8...@in.ibm.com).
Thanks,
K.Prasad
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 2/4] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC BookIII S
On Thu, May 27, 2010 at 04:19:40PM +1000, Paul Mackerras wrote:
> On Tue, May 25, 2010 at 02:44:20PM +0530, K.Prasad wrote:
>
> > Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
> > processors. These interfaces help arbitrate requests from various users and
> > schedules them as appropriate.
>
> A few comments on the code below...
>
I've posted a new patchset that addresses almost all of your comments.
Please find them here: linuxppc-dev message-id:
20100528063924.ga8...@in.ibm.com
> > +int __kprobes hw_breakpoint_handler(struct die_args *args)
> > +{
> > + bool is_ptrace_bp = false;
> > + int rc = NOTIFY_STOP;
> > + struct perf_event *bp;
> > + struct pt_regs *regs = args->regs;
> > + unsigned long dar = regs->dar;
> > + int stepped = 1;
> > + struct arch_hw_breakpoint *info;
> > +
> > + /* Disable breakpoints during exception handling */
> > + set_dabr(0);
> > + /*
> > +* The counter may be concurrently released but that can only
> > +* occur from a call_rcu() path. We can then safely fetch
> > +* the breakpoint, use its callback, touch its counter
> > +* while we are in an rcu_read_lock() path.
> > +*/
> > + rcu_read_lock();
> > +
> > + bp = __get_cpu_var(bp_per_reg);
> > + if (!bp)
> > + goto out;
> > + info = counter_arch_bp(bp);
> > + is_ptrace_bp = (bp->overflow_handler == ptrace_triggered) ?
> > + true : false;
> > +
> > + /*
> > +* Verify if dar lies within the address range occupied by the symbol
> > +* being watched to filter extraneous exceptions.
> > +*/
> > + if (!((bp->attr.bp_addr <= dar) &&
> > + (dar <= (bp->attr.bp_addr + bp->attr.bp_len))) &&
> > + (!is_ptrace_bp))
> > + /*
> > +* This exception is triggered not because of a memory access on
> > +* the monitored variable but in the double-word address range
> > +* in which it is contained. We will consume this exception,
> > +* considering it as 'noise'.
> > +*/
> > + goto restore_bp;
>
> At this point we have to do the single-stepping, because the NIP is
> still pointing at the instruction that caused the exception, and if we
> just return to it with DABR set we won't make any progress, we'll just
> take the same exception again immediately.
>
I don't know how I convinced myself earlier that this would work :-)
Given that the instructions needs to be emulated in the same manner as
others, I've re-used the ptrace_bps[] member in 'thread_struct' as a
flag to indicate such breakpoints. This will be later checked in
single_step_dabr_instruction() to prevent invocation of
perf_event_bp().
> > +/*
> > + * Handle single-step exceptions following a DABR hit.
> > + */
> > +int __kprobes single_step_dabr_instruction(struct die_args *args)
> > +{
> > + struct pt_regs *regs = args->regs;
> > + struct perf_event *bp = NULL;
> > + struct arch_hw_breakpoint *bp_info;
> > +
> > + bp = current->thread.last_hit_ubp;
> > + /*
> > +* Check if we are single-stepping as a result of a
> > +* previous HW Breakpoint exception
> > +*/
> > + if (!bp)
> > + return NOTIFY_DONE;
> > +
> > + bp_info = counter_arch_bp(bp);
> > +
> > + /*
> > +* We shall invoke the user-defined callback function in the single
> > +* stepping handler to confirm to 'trigger-after-execute' semantics
> > +*/
> > + perf_bp_event(bp, regs);
> > +
> > + /*
> > +* Do not disable MSR_SE if the process was already in
> > +* single-stepping mode.
> > +*/
> > + if (!test_thread_flag(TIF_SINGLESTEP))
> > + regs->msr &= ~MSR_SE;
> > +
> > + set_dabr(bp_info->address | bp_info->type | DABR_TRANSLATION);
> > + return NOTIFY_STOP;
> > +}
>
> Nowhere in here do we reset current->thread.last_hit_ubp, yet other
> parts of the code assume that .last_hit_ubp != NULL means that we are
> currently single-stepping. I think we need to clear .last_hit_ubp
> here.
>
True, made the change.
> > Index: linux-2.6.ppc64_test/arch/powerpc/kernel/process.c
> > ===
> > --- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/process.c
> > +++ linux-2.6.ppc64_test/arch/powerpc/kernel/process.c
>
[Patch 5/5] PPC64-HWBKPT: Discard extraneous interrupt due to accesses outside symbol length
Many a times, the requested breakpoint length can be less than the fixed
breakpoint length i.e. 8 bytes supported by PowerPC BookIII S. This could lead
to extraneous interrupts resulting in false breakpoint notifications. The patch
below detects and discards such interrupts for non-ptrace requests (we don't
want to change ptrace behaviour for fear of breaking compatability).
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/hw_breakpoint.c | 39
1 file changed, 35 insertions(+), 4 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -198,12 +198,13 @@ void thread_change_pc(struct task_struct
*/
int __kprobes hw_breakpoint_handler(struct die_args *args)
{
- bool is_ptrace_bp = false;
+ bool is_extraneous_interrupt = false, is_ptrace_bp = false;
int rc = NOTIFY_STOP;
struct perf_event *bp;
struct pt_regs *regs = args->regs;
int stepped = 1;
struct arch_hw_breakpoint *info;
+ unsigned long dar = regs->dar;
/* Disable breakpoints during exception handling */
set_dabr(0);
@@ -234,9 +235,33 @@ int __kprobes hw_breakpoint_handler(stru
goto out;
}
+ /*
+* Verify if dar lies within the address range occupied by the symbol
+* being watched to filter extraneous exceptions.
+*/
+ if (!((bp->attr.bp_addr <= dar) &&
+(dar <= (bp->attr.bp_addr + bp->attr.bp_len {
+ /*
+* This exception is triggered not because of a memory access
+* on the monitored variable but in the double-word address
+* range in which it is contained. We will consume this
+* exception, considering it as 'noise'.
+*/
+ is_extraneous_interrupt = true;
+ }
+
/* Do not emulate user-space instructions, instead single-step them */
if (user_mode(regs)) {
- bp->ctx->task->thread.last_hit_ubp = bp;
+ /*
+* To prevent invocation of perf_event_bp(), we shall overload
+* thread.ptrace_bps[] pointer (unused for non-ptrace
+* exceptions) to flag an extraneous interrupt which must be
+* skipped.
+*/
+ if (is_extraneous_interrupt)
+ bp->ctx->task->thread.ptrace_bps[0] = bp;
+ else
+ bp->ctx->task->thread.last_hit_ubp = bp;
regs->msr |= MSR_SE;
goto out;
}
@@ -274,7 +299,12 @@ int __kprobes single_step_dabr_instructi
struct perf_event *bp = NULL;
struct arch_hw_breakpoint *bp_info;
- bp = current->thread.last_hit_ubp;
+ if (current->thread.last_hit_ubp)
+ bp = current->thread.last_hit_ubp;
+ else {
+ bp = current->thread.ptrace_bps[0];
+ current->thread.ptrace_bps[0] = NULL;
+ }
/*
* Check if we are single-stepping as a result of a
* previous HW Breakpoint exception
@@ -288,7 +318,8 @@ int __kprobes single_step_dabr_instructi
* We shall invoke the user-defined callback function in the single
* stepping handler to confirm to 'trigger-after-execute' semantics
*/
- perf_bp_event(bp, regs);
+ if (bp == current->thread.last_hit_ubp)
+ perf_bp_event(bp, regs);
/*
* Do not disable MSR_SE if the process was already in
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 4/5] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery.
Restore hw-breakpoints if the user-context is altered in the signal handler.
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |3 +++
arch/powerpc/kernel/hw_breakpoint.c | 18 ++
arch/powerpc/kernel/signal.c |3 +++
3 files changed, 24 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -65,9 +65,12 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs);
#else /* CONFIG_HAVE_HW_BREAKPOINT */
static inline void hw_breakpoint_disable(void) { }
+static inline void thread_change_pc(struct task_struct *tsk,
+ struct pt_regs *regs) { }
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -176,6 +176,24 @@ int arch_validate_hwbkpt_settings(struct
}
/*
+ * Restores the breakpoint on the debug registers.
+ * Invoke this function if it is known that the execution context is about to
+ * change to cause loss of MSR_SE settings.
+ */
+void thread_change_pc(struct task_struct *tsk, struct pt_regs *regs)
+{
+ struct arch_hw_breakpoint *info;
+
+ if (likely(!tsk->thread.last_hit_ubp))
+ return;
+
+ info = counter_arch_bp(tsk->thread.last_hit_ubp);
+ regs->msr &= ~MSR_SE;
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+ tsk->thread.last_hit_ubp = NULL;
+}
+
+/*
* Handle debug exception notifications.
*/
int __kprobes hw_breakpoint_handler(struct die_args *args)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
@@ -149,6 +150,8 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+ /* Re-enable the breakpoints for the signal stack */
+ thread_change_pc(current, regs);
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 3/5] PPC64-HWBKPT: Handle concurrent alignment interrupts
An alignment interrupt may intervene between a DSI/hw-breakpoint exception
and the single-step exception. Enable the alignment interrupt (through
modifications to emulate_single_step()) to notify the single-step exception
handler for proper restoration of hw-breakpoints.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/traps.c |8 +++-
1 file changed, 3 insertions(+), 5 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
@@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
void __kprobes single_step_exception(struct pt_regs *regs)
{
- regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
+ clear_single_step(regs);
if (notify_die(DIE_SSTEP, "single_step", regs, 5,
5, SIGTRAP) == NOTIFY_STOP)
@@ -621,10 +621,8 @@ void __kprobes single_step_exception(str
*/
static void emulate_single_step(struct pt_regs *regs)
{
- if (single_stepping(regs)) {
- clear_single_step(regs);
- _exception(SIGTRAP, regs, TRAP_TRACE, 0);
- }
+ if (single_stepping(regs))
+ single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/5] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC BookIII S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 73 +++
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 322 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c| 14 +
arch/powerpc/kernel/ptrace.c | 64 ++
arch/powerpc/lib/Makefile|1
10 files changed, 491 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,73 @@
+/*
+ * PowerPC BookIII S hardware breakpoint definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010, IBM Corporation.
+ * Author: K.Prasad
+ *
+ */
+
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
+static inline void hw_breakpoint_disable(void) { }
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,322 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE
[Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXII
Hi All, Please find a new set of patches that have the following changes. Changelog - ver XXII (Version XXI: linuxppc-dev ref:20100525091314.ga29...@in.ibm.com) - Extraneous breakpoint exceptions are now properly handled; causative instruction will be single-stepped and debug register values restored. - Restoration of breakpoints during signal handling through thread_change_pc() had defects which are now fixed. - Breakpoints are flushed through flush_ptrace_hw_breakpoint() call in both flush_thread() and prepare_to_copy() functions. 'ptrace_bps[]' and 'last_hit_ubp' members are now promptly cleaned-up. - Single-step exception is now conditionally emulated upon hitting alignment_exception. - Rebased to commit 31f46717997a83bdf6db0dd04810c0a329eb3148 of linux-2.6 tree. Kindly let me know your comments for the same. Thanks, K.Prasad Changelog - ver XXI (Version XX: linuxppc-dev ref:20100524103136.ga8...@in.ibm.com) - Decision to emulate an instruction is now based on whether the causative instruction is in user_mode() or not. - Breakpoints don't have to be cleared during sigreturn. A 'double-hit' on hw_breakpoint_handler() is harmless for non-ptrace instructions. - Minor changes to aid code brevity. Changelog - ver XX (Version XIX: linuxppc-dev ref: 20100524040137.ga20...@in.ibm.com) - Non task-bound breakpoints will only be emulated. Breakpoint will be unregistered with a warning if emulation fails. Changelog - ver XIX (Version XVIII: linuxppc-dev ref: 20100512033055.ga6...@in.ibm.com) - Increased coverage of breakpoints during concurrent alignment_exception and signal handling (which previously had 'blind-spots'). - Support for kernel-thread breakpoints and kernel-space breakpoints inside the context of a user-space process. - Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby necessitating minor changes to arch_validate_hwbkpt_settings(). Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - Slight code restructuring for brevity and coding-style corrections. - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - hw-breakpoint restoration variables are cleaned-up before unregistration through arch_unregister_hw_breakpoint(). - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this ch
[Patch 1/5] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
Acked-by: Frederic Weisbecker
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 1/4] Allow arch-specific cleanup before breakpoint unregistration
On Wed, May 26, 2010 at 07:23:15PM +0200, Frederic Weisbecker wrote: > On Wed, May 26, 2010 at 10:47:42PM +0530, K.Prasad wrote: > > On Wed, May 26, 2010 at 10:54:41AM +0100, David Howells wrote: > > > K.Prasad wrote: > > > > > > > > My understanding is weak function definitions must appear in a > > > > > different C > > > > > file than their call sites to work on some toolchains. > > > > > > > > > > > > > Atleast, there are quite a few precedents inside the Linux kernel for > > > > __weak functions being invoked from the file in which they are defined > > > > (arch_hwblk_init, arch_enable_nonboot_cpus_begin and hw_perf_disable to > > > > name a few). > > > > Moreover the online GCC docs haven't any such constraints mentioned. > > > > > > I've seen problems in this area. gcc sometimes inlines a weak function > > > that's > > > in the same file as the call point. > > > > > > > We've seen such behaviour even otherwiseeven with noinline attribute > > in place. I'm not sure if this gcc fix > > (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16922) helped correct the > > behaviour, but the lesson has been to not trust a function to be > > inlined/remain non-inline consistently. > > > If we can't put the call to the function in the same file of its weak > definition, then perf is totally screwed. > > And in fact it makes __weak basically useless and unusable. I guess > that happened in old gcc versions that have been fixed now. > > Anyway, I'm personally fine with this patch (you can put my hack > if you want). > I guess you meant "Acked-by:" :-) Thanks, I'll add the same. --K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 1/4] Allow arch-specific cleanup before breakpoint unregistration
On Wed, May 26, 2010 at 10:47:42PM +0530, K.Prasad wrote: > On Wed, May 26, 2010 at 10:54:41AM +0100, David Howells wrote: > > K.Prasad wrote: > > > > > > My understanding is weak function definitions must appear in a > > > > different C > > > > file than their call sites to work on some toolchains. > > > > > > > > > > Atleast, there are quite a few precedents inside the Linux kernel for > > > __weak functions being invoked from the file in which they are defined > > > (arch_hwblk_init, arch_enable_nonboot_cpus_begin and hw_perf_disable to > > > name a few). > > > Moreover the online GCC docs haven't any such constraints mentioned. > > > > I've seen problems in this area. gcc sometimes inlines a weak function > > that's > > in the same file as the call point. > > > > We've seen such behaviour even otherwiseeven with noinline attribute > in place. I'm not sure if this gcc fix > (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16922) helped correct the Looks like I cited the wrong bug. The appropriate one is http://gcc.gnu.org/bugzilla/show_bug.cgi?id=34563. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 1/4] Allow arch-specific cleanup before breakpoint unregistration
On Wed, May 26, 2010 at 10:54:41AM +0100, David Howells wrote: > K.Prasad wrote: > > > > My understanding is weak function definitions must appear in a different C > > > file than their call sites to work on some toolchains. > > > > > > > Atleast, there are quite a few precedents inside the Linux kernel for > > __weak functions being invoked from the file in which they are defined > > (arch_hwblk_init, arch_enable_nonboot_cpus_begin and hw_perf_disable to > > name a few). > > Moreover the online GCC docs haven't any such constraints mentioned. > > I've seen problems in this area. gcc sometimes inlines a weak function that's > in the same file as the call point. > We've seen such behaviour even otherwiseeven with noinline attribute in place. I'm not sure if this gcc fix (http://gcc.gnu.org/bugzilla/show_bug.cgi?id=16922) helped correct the behaviour, but the lesson has been to not trust a function to be inlined/remain non-inline consistently. > David Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 1/4] Allow arch-specific cleanup before breakpoint unregistration
On Tue, May 25, 2010 at 06:39:19AM -0500, Millton Miller wrote:
> On Tue, 25 May 2010 at 14:43:56 +0530, K.Prasad wrote:
> > Certain architectures (such as PowerPC Book III S) have a need to cleanup
> > data-structures before the breakpoint is unregistered. This patch introduces
> > an arch-specific hook in release_bp_slot() along with a weak definition in
> > the form of a stub funciton.
> >
> > Signed-off-by: K.Prasad
> > ---
> > kernel/hw_breakpoint.c | 12
> > 1 file changed, 12 insertions(+)
>
>
> My understanding is weak function definitions must appear in a different C
> file than their call sites to work on some toolchains.
>
Atleast, there are quite a few precedents inside the Linux kernel for
__weak functions being invoked from the file in which they are defined
(arch_hwblk_init, arch_enable_nonboot_cpus_begin and hw_perf_disable to
name a few).
Moreover the online GCC docs haven't any such constraints mentioned.
> Andrew, can you confirm the above statement?
>
> > Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
> > ===
> > --- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
> > +++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
> > @@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
> > }
> >
> > /*
> > + * Function to perform processor-specific cleanup during unregistration
> > + */
> > +__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
> > +{
> > + /*
> > +* A weak stub function here for those archs that don't define
> > +* it inside arch/.../kernel/hw_breakpoint.c
> > +*/
> > +}
> > +
> > +/*
> > * Contraints to check before allowing this new breakpoint counter:
> > *
> > * == Non-pinned counter == (Considered as pinned for now)
> > @@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
> > {
> > mutex_lock(&nr_bp_mutex);
> >
> > + arch_unregister_hw_breakpoint(bp);
> > __release_bp_slot(bp);
> >
> > mutex_unlock(&nr_bp_mutex);
> >
>
>
> Since the weak version is empty, should it just be delcared (in
> a header, put the comment there) and not defined?
>
The initial thinking behind defining it in the .c file was, for one,
the function need not be moved (from .h to .c) when other architectures
have a need to populate them. Secondly, given that powerpc (which has a
'strong' definition for arch_unregister_hw_breakpoint()) includes the
header file (in which this can be moved to) I wasn't sure about
possible conflicts.
> milton
> ___
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Thanks,
K.Prasad
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery.
Restore hw-breakpoints if the user-context is altered in the signal handler.
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |2 ++
arch/powerpc/kernel/hw_breakpoint.c | 16
arch/powerpc/kernel/signal.c |3 +++
arch/powerpc/kernel/signal_32.c |2 ++
arch/powerpc/kernel/signal_64.c |2 ++
5 files changed, 25 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -65,9 +65,11 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void thread_change_pc(struct task_struct *tsk);
#else /* CONFIG_HAVE_HW_BREAKPOINT */
static inline void hw_breakpoint_disable(void) { }
+static inline void thread_change_pc(struct task_struct *tsk) { }
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -176,6 +176,22 @@ int arch_validate_hwbkpt_settings(struct
}
/*
+ * Restores the breakpoint on the debug registers.
+ * Invoke this function if it is known that the execution context is about to
+ * change to cause loss of MSR_SE settings.
+ */
+void thread_change_pc(struct task_struct *tsk)
+{
+ struct arch_hw_breakpoint *info;
+
+ if (likely(!tsk->thread.last_hit_ubp))
+ return;
+
+ info = counter_arch_bp(tsk->thread.last_hit_ubp);
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+}
+
+/*
* Handle debug exception notifications.
*/
int __kprobes hw_breakpoint_handler(struct die_args *args)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
@@ -149,6 +150,8 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+/* Re-enable the breakpoints for the signal stack */
+ thread_change_pc(current);
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_64.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "signal.h"
@@ -312,6 +313,7 @@ int sys_swapcontext(struct ucontext __us
|| __copy_to_user(&old_ctx->uc_sigmask,
¤t->blocked, sizeof(sigset_t)))
return -EFAULT;
+ thread_change_pc(current);
}
if (new_ctx == NULL)
return 0;
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_32.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_32.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_32.c
@@ -42,6 +42,7 @@
#include
#include
#include
+#include
#ifdef CONFIG_PPC64
#include "ppc32.h"
#include
@@ -996,6 +997,7 @@ long sys_swapcontext(struct ucontext __u
|| put_sigset_t(&old_ctx->uc_sigmask, ¤t->blocked)
|| __put_user(to_user_ptr(mctx), &old_ctx->uc_regs))
return -EFAULT;
+ thread_change_pc(current);
}
if (new_ctx == NULL)
return 0;
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 3/4] PPC64-HWBKPT: Handle concurrent alignment interrupts
An alignment interrupt may intervene between a DSI/hw-breakpoint exception
and the single-step exception. Enable the alignment interrupt (through
modifications to emulate_single_step()) to notify the single-step exception
handler for proper restoration of hw-breakpoints.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/traps.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
@@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
void __kprobes single_step_exception(struct pt_regs *regs)
{
- regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
+ clear_single_step(regs);
if (notify_die(DIE_SSTEP, "single_step", regs, 5,
5, SIGTRAP) == NOTIFY_STOP)
@@ -621,10 +621,7 @@ void __kprobes single_step_exception(str
*/
static void emulate_single_step(struct pt_regs *regs)
{
- if (single_stepping(regs)) {
- clear_single_step(regs);
- _exception(SIGTRAP, regs, TRAP_TRACE, 0);
- }
+ single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/4] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC BookIII S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 73 ++
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 338 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c|6
arch/powerpc/kernel/ptrace.c | 64 +
arch/powerpc/lib/Makefile|1
10 files changed, 499 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,73 @@
+/*
+ * PowerPC BookIII S hardware breakpoint definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010, IBM Corporation.
+ * Author: K.Prasad
+ *
+ */
+
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#else /* CONFIG_HAVE_HW_BREAKPOINT */
+static inline void hw_breakpoint_disable(void) { }
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,338 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE
[Patch 1/4] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/4] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XXI
Hi All, Please find a new version of the hw-breakpoint patchset with changes as described below. The patchset, passes when tested against breakpoints caused by user-space instructions but fails against kernel-space instructions (as a result of emulate_step() failure). They should begin to work fine with further enhancements to emulate_step(). Changelog - ver XXI (Version XIX: linuxppc-dev ref:20100524103136.ga8...@in.ibm.com) - Decision to emulate an instruction is now based on whether the causative instruction is in user_mode() or not. - Breakpoints don't have to be cleared during sigreturn. A 'double-hit' on hw_breakpoint_handler() is harmless for non-ptrace instructions. - Minor changes to enhance code brevity. Kindly let me know your comments. Thanks, K.Prasad Changelog - ver XX (Version XIX: linuxppc-dev ref: 20100524040137.ga20...@in.ibm.com) - Non task-bound breakpoints will only be emulated. Breakpoint will be unregistered with a warning if emulation fails. Changelog - ver XIX (Version XVIII: linuxppc-dev ref: 20100512033055.ga6...@in.ibm.com) - Increased coverage of breakpoints during concurrent alignment_exception and signal handling (which previously had 'blind-spots'). - Support for kernel-thread breakpoints and kernel-space breakpoints inside the context of a user-space process. - Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby necessitating minor changes to arch_validate_hwbkpt_settings(). Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - Slight code restructuring for brevity and coding-style corrections. - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - hw-breakpoint restoration variables are cleaned-up before unregistration through arch_unregister_hw_breakpoint(). - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this change is the non-triggering of hw-breakpoints while single-stepping kernel through KGDB or Xmon. - Minor code-cleanups and addition of comments in hw_breakpoint_handler() and single_step_dabr_instruction(). - Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6 Changelog - ver XIII (Version XII: linuxppc-dev ref: 20100121084640.ga3...@in.ibm.com) - Fixed a bug for user-space breakpoints (triggered following the failure
Re: [Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Thu, May 20, 2010 at 11:10:03PM +1000, Paul Mackerras wrote:
> On Thu, May 20, 2010 at 09:36:03AM +0530, K.Prasad wrote:
>
(Had this mail composed along with the patchset...but mail server issues
caused delay in sending this...)
Hi Paul,
While we continue to discuss some of the design decisions, I
thought I'd ready up a patchset to capture the changes we agreed upon to
(lest we miss them). I have sent a new version of the patchset here:
linuxppc-dev message-id: 20100524040137.ga20...@in.ibm.com.
Please see more responses below.
> > > Right. However, the thread is running the signal handler without the
> > > DABR being set, which is unfortunate.
> > >
> >
> > In order to keep the breakpoint active during signal handling, a
> > PowerPC specific signal handling code, say do_signal_pending() in
> > arch/powerpc/kernel/signal.c, must be tapped to check for/restore
> > the breakpoint for the process (if it exists).
>
> What I would suggest is something slightly different: anything that
> causes the thread to change where it's executing -- signal delivery,
> modification of NIP with ptrace -- should cancel the single-step and
> reinstall the breakpoint in the DABR. In other words we just forget
> that we hit the breakpoint, and rely on hitting it again if we ever
> get back to that instruction. I think that is by far the most
> reliable approach.
>
> That means that the hw-breakpoint code needs to export a function
> called, say, thread_change_pc(), which is called whenever we're
> changing a thread's userspace PC (NIP) value. If the hw-breakpoint
> code has put that thread into single-step mode, we cancel the
> single-step and if the thread is current, set DABR.
>
I have made changes to signal-handling code on the suggested lines (as
seen here: linuxppc-dev message-id:20100524040342.ge20...@in.ibm.com)
wherein the debug registers are populated before signal-delivery and
cleaned during signal-return.
However handling of nested interrupts, where second exception is taken
inside the signal handler is still flimsy and the system would experience
two hw-breakpoint exceptions. To overcome the same, we will need a flag in
'struct thread_struct' or perhaps in 'struct arch_hw_breakpoint' to
indicate a breakpoint previously taken in signal-handling context. Given
that the repurcussions of a double-hit are not dangerous, and unsure of
how an addition to thread_struct might be received, I've skipped those
changes for now.
> > I'm afraid if this is more complexity than we want to handle in the
> > first patchset. I agree that this will create a 'blind-spot' of code
> > which cannot not be monitored using breakpoints and may limit debugging
> > efforts (specially for memory corruption); however suspecting that signal
> > handlers (especially those that don't return to original instruction)
> > would be rare, I feel that this could be a 'feature' that can be brought
> > later-in. What do you think?
>
> I think the thread_change_pc() approach is quite a bit simpler, and I
> think it's better to get this right at the outset rather than have it
> cause bugs later on, when we've all forgotten all the curly
> details. :)
Yes, the details are mostly captured in the latest patchset. Had to make
some 'bold' changes to overcome the issues though :-)
>
> > > Imagine this scenario: we get the DABR match, set MSR_SE and return to
> > > the task. In the meantime another higher-priority task has become
> > > runnable and our need_resched flag is set, so we call schedule() on
> > > the way back out to usermode. The other task runs and then blocks and
> > > our task gets scheduled again. As part of the context switch,
> > > arch_install_hw_breakpoint() will get called and will set DABR. So
> > > we'll return to usermode with both DABR and MSE_SE set.
> > >
> >
> > I didn't foresee such a possibility. I think this can be handled by
> > putting a check in arch_install_hw_breakpoint() as shown below:
> >
> > int arch_install_hw_breakpoint(struct perf_event *bp)
> > {
> > struct arch_hw_breakpoint *info = counter_arch_bp(bp);
> > struct perf_event **slot = &__get_cpu_var(bp_per_reg);
> >
> > *slot = bp;
> > if (!current->thread.last_hit_ubp)
> > set_dabr(info->address | info->type | DABR_TRANSLATION);
> > return 0;
> > }
>
> Yes, basically, but don't we need to handle per-cpu breakpoints as
> well? That is, we only want the extra check if this breakpoint is a
> per-task breakpoint. Or am I not seeing enough context her
[Patch 1/4] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/4] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XX
Hi All, Here's a quick release of the next version of the patchset with a small, yet significant changelog. Please let me know the comments, if any. Changelog - ver XX (Version XIX: linuxppc-dev ref: 20100524040137.ga20...@in.ibm.com) - Non task-bound breakpoints will only be emulated. Breakpoint will be unregistered with a warning if emulation fails. Thanks, K.Prasad Changelog - ver XIX (Version XVIII: linuxppc-dev ref: 20100512033055.ga6...@in.ibm.com) - Increased coverage of breakpoints during concurrent alignment_exception and signal handling (which previously had 'blind-spots'). - Support for kernel-thread breakpoints and kernel-space breakpoints inside the context of a user-space process. - Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby necessitating minor changes to arch_validate_hwbkpt_settings(). Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - hw-breakpoint restoration variables are cleaned-up before unregistration through a new function hook arch_unregister_hw_breakpoint(). - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. - Slight code restructuring for brevity and style corrections. Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this change is the non-triggering of hw-breakpoints while single-stepping kernel through KGDB or Xmon. - Minor code-cleanups and addition of comments in hw_breakpoint_handler() and single_step_dabr_instruction(). - Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6 Changelog - ver XIII (Version XII: linuxppc-dev ref: 20100121084640.ga3...@in.ibm.com) - Fixed a bug for user-space breakpoints (triggered following the failure of a breakpoint request). - Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of linux-2.6 Changelog - ver XII (Version XI: linuxppc-dev ref: 20100119091234.ga9...@in.ibm.com) - Unset MSR_SE only if kernel was not previously in single-step mode. - Pre-emption is now enabled before returning from the hw-breakpoint exception handler. - Variables to track the source of single-step exception (breakpoint from kernel, user-space vs single-stepping due to other requests) are added. - Extraneous hw-breakpoint exceptions (due to memory accesses lying outside monitored symbol length) is now done for both kernel and us
[Patch 4/4] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery and clear them during sigreturn() syscall.
Limitation: Nested hw-breakpoint exceptions (where second exception is raised
inside signal context) will cause a 'double-hit' i.e. the first breakpoint
exception will be taken twice.
Restore hw-breakpoints if the user-context is altered in the signal handler
(causing loss of MSR_SE). Side-effect: 'Double-hit' of breakpoint if the
instruction pointer is unaltered in the new context.
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |3 +++
arch/powerpc/kernel/hw_breakpoint.c | 28
arch/powerpc/kernel/signal.c |8
arch/powerpc/kernel/signal_32.c | 10 ++
arch/powerpc/kernel/signal_64.c |7 +++
5 files changed, 56 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -43,6 +43,9 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void sighandler_install_bp(struct task_struct *tsk);
+extern void sigreturn_uninstall_bp(struct task_struct *tsk);
+extern void thread_change_pc(struct task_struct *tsk, unsigned long msr);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -175,6 +175,34 @@ int arch_validate_hwbkpt_settings(struct
return 0;
}
+void sighandler_install_bp(struct task_struct *tsk)
+{
+ struct arch_hw_breakpoint *info;
+
+ if (likely(!tsk->thread.last_hit_ubp))
+ return;
+
+ info = counter_arch_bp(tsk->thread.last_hit_ubp);
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+}
+
+void sigreturn_uninstall_bp(struct task_struct *tsk)
+{
+ if (unlikely(tsk->thread.last_hit_ubp))
+ set_dabr(0);
+}
+
+void thread_change_pc(struct task_struct *tsk, unsigned long new_msr)
+{
+ /*
+* Do not bother to restore breakpoints if single-stepping is not
+* cleared. single_step_dabr_instruction() will handle it if MSR_SE
+* is set.
+*/
+ if (!(new_msr & MSR_SE))
+ sighandler_install_bp(tsk);
+}
+
/*
* Handle debug exception notifications.
*/
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
@@ -149,6 +150,13 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ /*
+* Re-enable the breakpoints (if it was previously cleared in
+* hw_breakpoint_handler()) for the signal stack.
+*/
+ sighandler_install_bp(current);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_64.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "signal.h"
@@ -312,6 +313,9 @@ int sys_swapcontext(struct ucontext __us
|| __copy_to_user(&old_ctx->uc_sigmask,
¤t->blocked, sizeof(sigset_t)))
return -EFAULT;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ thread_change_pc(current, new_msr);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
}
if (new_ctx == NULL)
return 0;
@@ -364,6 +368,9 @@ int sys_rt_sigreturn(unsigned long r3, u
if (__copy_from_user(&set, &uc->uc_sigmask, sizeof(set)))
goto badframe;
restore_sigmask(&set);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ sigreturn_uninstall_bp(current);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (restore_sigcontext(regs, NULL, 1, &uc->uc_mcontext))
[Patch 3/4] PPC64-HWBKPT: Handle concurrent alignment interrupts
An alignment interrupt may intervene between a DSI/hw-breakpoint exception
and the single-step exception. Enable the alignment interrupt (through
modifications to emulate_single_step()) to notify the single-step exception
handler for proper restoration of hw-breakpoints.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/traps.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
@@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
void __kprobes single_step_exception(struct pt_regs *regs)
{
- regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
+ clear_single_step(regs);
if (notify_die(DIE_SSTEP, "single_step", regs, 5,
5, SIGTRAP) == NOTIFY_STOP)
@@ -621,10 +621,7 @@ void __kprobes single_step_exception(str
*/
static void emulate_single_step(struct pt_regs *regs)
{
- if (single_stepping(regs)) {
- clear_single_step(regs);
- _exception(SIGTRAP, regs, TRAP_TRACE, 0);
- }
+ single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/4] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC Book III S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 49
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 345 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c|6
arch/powerpc/kernel/ptrace.c | 64 +
arch/powerpc/lib/Makefile|1
include/linux/hw_breakpoint.h|1
11 files changed, 483 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,49 @@
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,345 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+ *slot = bp;
+
+ /*
+* Do not install DABR values if the instruction must be single-stepped.
+* If so, DABR will be populated in single_step_dabr_instruction().
+*/
+ if (current->thread.last_hit_ubp != bp)
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+
+ return 0;
+}
+
+/*
+ * Uninstall the breakpoint contained in the given counter.
Re: [RFC Patch 2/5] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC Book III S
On Mon, May 24, 2010 at 09:32:25AM +0530, K.Prasad wrote:
> Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
> processors. These interfaces help arbitrate requests from various users and
> schedules them as appropriate.
>
> +/*
> + * Handle debug exception notifications.
> + */
> +int __kprobes hw_breakpoint_handler(struct die_args *args)
> +{
> + bool is_kernel, is_ptrace_bp = false;
> + int rc = NOTIFY_STOP;
> + struct perf_event *bp;
> + struct pt_regs *regs = args->regs;
> + unsigned long dar = regs->dar;
> + int stepped = 1;
> + struct arch_hw_breakpoint *info;
> +
> + /* Disable breakpoints during exception handling */
> + set_dabr(0);
> + /*
> + * The counter may be concurrently released but that can only
> + * occur from a call_rcu() path. We can then safely fetch
> + * the breakpoint, use its callback, touch its counter
> + * while we are in an rcu_read_lock() path.
> + */
> + rcu_read_lock();
> +
> + bp = __get_cpu_var(bp_per_reg);
> + if (!bp)
> + goto out;
> + info = counter_arch_bp(bp);
> + is_kernel = is_kernel_addr(bp->attr.bp_addr);
> + is_ptrace_bp = (bp->overflow_handler == ptrace_triggered) ?
> + true : false;
> +
> + /*
> + * Verify if dar lies within the address range occupied by the symbol
> + * being watched to filter extraneous exceptions.
> + */
> + if (!((bp->attr.bp_addr <= dar) &&
> + (dar <= (bp->attr.bp_addr + bp->attr.bp_len))) &&
> + (!is_ptrace_bp))
> + /*
> + * This exception is triggered not because of a memory access on
> + * the monitored variable but in the double-word address range
> + * in which it is contained. We will consume this exception,
> + * considering it as 'noise'.
> + */
> + goto restore_bp;
> +
> + /*
> + * Return early after invoking user-callback function without restoring
> + * DABR if the breakpoint is from ptrace which always operates in
> + * one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
> + * generated in do_dabr().
> + */
> + if (is_ptrace_bp) {
> + perf_bp_event(bp, regs);
> + rc = NOTIFY_DONE;
> + goto out;
> + }
> +
> + /*
> + * Do not emulate user-space instructions from kernel-space,
> + * instead single-step them.
> + */
> + if (!is_kernel) {
> + bp->ctx->task->thread.last_hit_ubp = bp;
> + regs->msr |= MSR_SE;
> + goto out;
> + }
> +
> + stepped = emulate_step(regs, regs->nip);
> + /* emulate_step() could not execute it, single-step them */
> + if (stepped == 0) {
As I was responding to one of the previous mails, I realised that I
had not made changes here as Paul Mackerras had suggested
(reference linuxppc-dev message-id:
20100520131003.gb29...@brick.ozlabs.ibm.com) i.e. uninstall breakpoint
if single-stepping failed.
I'll quickly send out a revised patch as a reply to this one. Regrets
for the confusion caused.
Thanks,
K.Prasad
> + regs->msr |= MSR_SE;
> + __get_cpu_var(last_hit_bp) = bp;
> + goto out;
> + }
> + /*
> + * As a policy, the callback is invoked in a 'trigger-after-execute'
> + * fashion
> + */
> + perf_bp_event(bp, regs);
> +
> +restore_bp:
> + set_dabr(info->address | info->type | DABR_TRANSLATION);
> +out:
> + rcu_read_unlock();
> + return rc;
> +}
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC Patch 5/5] PPC64-HWBKPT: Enable proper distinction of per-task and per-cpu breakpoints
Per-task and per-cpu breakpoints have to be unambiguously identified for
proper restoration of hw-breakpoints. The notion of pure user- and kernel-
space breakpoints is antiquated. Store 'pid' of the process against which
the perf-counter was requested to help proper distinction.
This helps seamless handling of kernel-space breakpoints within the context
of a user-space process and breakpoints for kernel-threads.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/hw_breakpoint.c | 24 +---
include/linux/perf_event.h |1 +
kernel/perf_event.c |9 ++---
3 files changed, 24 insertions(+), 10 deletions(-)
Index: linux-2.6.ppc64_test/include/linux/perf_event.h
===
--- linux-2.6.ppc64_test.orig/include/linux/perf_event.h
+++ linux-2.6.ppc64_test/include/linux/perf_event.h
@@ -698,6 +698,7 @@ struct perf_event {
int oncpu;
int cpu;
+ pid_t pid;
struct list_headowner_entry;
struct task_struct *owner;
Index: linux-2.6.ppc64_test/kernel/perf_event.c
===
--- linux-2.6.ppc64_test.orig/kernel/perf_event.c
+++ linux-2.6.ppc64_test/kernel/perf_event.c
@@ -4684,6 +4684,7 @@ static const struct pmu *sw_perf_event_i
static struct perf_event *
perf_event_alloc(struct perf_event_attr *attr,
int cpu,
+ pid_t pid,
struct perf_event_context *ctx,
struct perf_event *group_leader,
struct perf_event *parent_event,
@@ -4717,6 +4718,7 @@ perf_event_alloc(struct perf_event_attr
mutex_init(&event->mmap_mutex);
event->cpu = cpu;
+ event->pid = pid;
event->attr = *attr;
event->group_leader = group_leader;
event->pmu = NULL;
@@ -5015,7 +5017,7 @@ SYSCALL_DEFINE5(perf_event_open,
goto err_put_context;
}
- event = perf_event_alloc(&attr, cpu, ctx, group_leader,
+ event = perf_event_alloc(&attr, cpu, pid, ctx, group_leader,
NULL, NULL, GFP_KERNEL);
err = PTR_ERR(event);
if (IS_ERR(event))
@@ -5090,7 +5092,7 @@ perf_event_create_kernel_counter(struct
goto err_exit;
}
- event = perf_event_alloc(attr, cpu, ctx, NULL,
+ event = perf_event_alloc(attr, cpu, pid, ctx, NULL,
NULL, overflow_handler, GFP_KERNEL);
if (IS_ERR(event)) {
err = PTR_ERR(event);
@@ -5142,7 +5144,8 @@ inherit_event(struct perf_event *parent_
parent_event = parent_event->parent;
child_event = perf_event_alloc(&parent_event->attr,
- parent_event->cpu, child_ctx,
+ parent_event->cpu, child->pid,
+ child_ctx,
group_leader, parent_event,
NULL, GFP_KERNEL);
if (IS_ERR(child_event))
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -221,7 +221,7 @@ void thread_change_pc(struct task_struct
*/
int __kprobes hw_breakpoint_handler(struct die_args *args)
{
- bool is_kernel, is_ptrace_bp = false;
+ bool is_kernel, is_taskbound_bp, is_ptrace_bp = false;
int rc = NOTIFY_STOP;
struct perf_event *bp;
struct pt_regs *regs = args->regs;
@@ -246,6 +246,7 @@ int __kprobes hw_breakpoint_handler(stru
is_kernel = is_kernel_addr(bp->attr.bp_addr);
is_ptrace_bp = (bp->overflow_handler == ptrace_triggered) ?
true : false;
+ is_taskbound_bp = (bp->pid > 0) ? true : false;
/*
* Verify if dar lies within the address range occupied by the symbol
@@ -288,7 +289,14 @@ int __kprobes hw_breakpoint_handler(stru
/* emulate_step() could not execute it, single-step them */
if (stepped == 0) {
regs->msr |= MSR_SE;
- __get_cpu_var(last_hit_bp) = bp;
+ /*
+* Kernel-space addresses can also be bound to a task. If so,
+* store the breakpoint in its 'thread_struct'
+*/
+ if (is_taskbound_bp)
+ bp->ctx->task->thread.last_hit_ubp = bp;
+ else
+ __get_cpu_var(last_hit_bp
[RFC Patch 4/5] PPC64-HWBKPT: Enable hw-breakpoints while handling intervening signals
A signal delivered between a hw_breakpoint_handler() and the
single_step_dabr_instruction() will not have the breakpoint active during
signal handling (since breakpoint will not be restored through single-stepping
due to absence of MSR_SE bit on the signal frame). Enable breakpoints before
signal delivery and clear them during sigreturn() syscall.
Limitation: Nested hw-breakpoint exceptions (where second exception is raised
inside signal context) will cause a 'double-hit' i.e. the first breakpoint
exception will be taken twice.
Restore hw-breakpoints if the user-context is altered in the signal handler
(causing loss of MSR_SE).
Side-effect: 'Double-hit' of breakpoint if the instruction pointer is
unaltered in the new context.
Signed-off-by: K.Prasad
---
arch/powerpc/include/asm/hw_breakpoint.h |3 +++
arch/powerpc/kernel/hw_breakpoint.c | 28
arch/powerpc/kernel/signal.c |8
arch/powerpc/kernel/signal_32.c | 10 ++
arch/powerpc/kernel/signal_64.c |7 +++
5 files changed, 56 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/hw_breakpoint.h
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -43,6 +43,9 @@ static inline void hw_breakpoint_disable
{
set_dabr(0);
}
+extern void sighandler_install_bp(struct task_struct *tsk);
+extern void sigreturn_uninstall_bp(struct task_struct *tsk);
+extern void thread_change_pc(struct task_struct *tsk, unsigned long msr);
#endif /* CONFIG_HAVE_HW_BREAKPOINT */
#endif /* __KERNEL__ */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -188,6 +188,34 @@ int arch_validate_hwbkpt_settings(struct
return 0;
}
+void sighandler_install_bp(struct task_struct *tsk)
+{
+ struct arch_hw_breakpoint *info;
+
+ if (likely(!tsk->thread.last_hit_ubp))
+ return;
+
+ info = counter_arch_bp(tsk->thread.last_hit_ubp);
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+}
+
+void sigreturn_uninstall_bp(struct task_struct *tsk)
+{
+ if (unlikely(tsk->thread.last_hit_ubp))
+ set_dabr(0);
+}
+
+void thread_change_pc(struct task_struct *tsk, unsigned long new_msr)
+{
+ /*
+* Do not bother to restore breakpoints if single-stepping is not
+* cleared. single_step_dabr_instruction() will handle it if MSR_SE
+* is set.
+*/
+ if (!(new_msr & MSR_SE))
+ sighandler_install_bp(tsk);
+}
+
/*
* Handle debug exception notifications.
*/
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal.c
@@ -11,6 +11,7 @@
#include
#include
+#include
#include
#include
@@ -149,6 +150,13 @@ static int do_signal_pending(sigset_t *o
if (current->thread.dabr)
set_dabr(current->thread.dabr);
#endif
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ /*
+* Re-enable the breakpoints (if it was previously cleared in
+* hw_breakpoint_handler()) for the signal stack.
+*/
+ sighandler_install_bp(current);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/signal_64.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/signal_64.c
@@ -33,6 +33,7 @@
#include
#include
#include
+#include
#include "signal.h"
@@ -312,6 +313,9 @@ int sys_swapcontext(struct ucontext __us
|| __copy_to_user(&old_ctx->uc_sigmask,
¤t->blocked, sizeof(sigset_t)))
return -EFAULT;
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ thread_change_pc(current, new_msr);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
}
if (new_ctx == NULL)
return 0;
@@ -364,6 +368,9 @@ int sys_rt_sigreturn(unsigned long r3, u
if (__copy_from_user(&set, &uc->uc_sigmask, sizeof(set)))
goto badframe;
restore_sigmask(&set);
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+ sigreturn_uninstall_bp(current);
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
if (restore_sigcontext(regs, NULL, 1, &uc->uc_mcontext))
[RFC Patch 3/5] PPC64-HWBKPT: Handle concurrent alignment interrupts
An alignment interrupt may intervene between a DSI/hw-breakpoint exception
and the single-step exception. Enable the alignment interrupt (through
modifications to emulate_single_step()) to notify the single-step exception
handler for proper restoration of hw-breakpoints.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/traps.c |7 ++-
1 file changed, 2 insertions(+), 5 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/traps.c
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/traps.c
@@ -602,7 +602,7 @@ void RunModeException(struct pt_regs *re
void __kprobes single_step_exception(struct pt_regs *regs)
{
- regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */
+ clear_single_step(regs);
if (notify_die(DIE_SSTEP, "single_step", regs, 5,
5, SIGTRAP) == NOTIFY_STOP)
@@ -621,10 +621,7 @@ void __kprobes single_step_exception(str
*/
static void emulate_single_step(struct pt_regs *regs)
{
- if (single_stepping(regs)) {
- clear_single_step(regs);
- _exception(SIGTRAP, regs, TRAP_TRACE, 0);
- }
+ single_step_exception(regs);
}
static inline int __parse_fpscr(unsigned long fpscr)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC Patch 2/5] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC Book III S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 49
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 365 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c|6
arch/powerpc/kernel/ptrace.c | 64 +
arch/powerpc/lib/Makefile|1
include/linux/hw_breakpoint.h|1
11 files changed, 503 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,49 @@
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,365 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2010 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Store the 'bp' that caused the hw-breakpoint exception just before we
+ * single-step. Use it to distinguish a single-step exception (due to a
+ * previous hw-breakpoint exception) from a normal one
+ */
+static DEFINE_PER_CPU(struct perf_event *, last_hit_bp);
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+ *slot = bp;
+
+ /*
+* Do not install DABR values if the instruction must be single-stepped.
+
[RFC Patch 1/5] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -242,6 +242,17 @@ toggle_bp_slot(struct perf_event *bp, bo
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -339,6 +350,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[RFC Patch 0/5] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XIX
Hi All, Please find a new version of the patches that implement hw-breakpoints for PowerPC Book III S. This patchset has some major additions to hw-breakpoint exception handling that help reliable restoration of breakpoints + increased coverage of breakpoints for signal handlers. The patches have been marked RFC, given that changes through patches 4/5 and 5/5 haven't been tested in particular and that they make modifications to the generic framework too. Kindly let me know your comments. Changelog - ver XIX (Version XVIII: linuxppc-dev ref: 20100512033055.ga6...@in.ibm.com) - Increased coverage of breakpoints during concurrent alignment_exception and signal handling (which previously had 'blind-spots'). - Support for kernel-thread breakpoints and kernel-space breakpoints inside the context of a user-space process. - Patches re-based to commit f4b87dee923342505e1ddba8d34ce9de33e75050, thereby necessitating minor changes to arch_validate_hwbkpt_settings(). Thanks, K.Prasad Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - hw-breakpoint restoration variables are cleaned-up before unregistration through a new function hook arch_unregister_hw_breakpoint(). - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. - Slight code restructuring for brevity and style corrections. Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this change is the non-triggering of hw-breakpoints while single-stepping kernel through KGDB or Xmon. - Minor code-cleanups and addition of comments in hw_breakpoint_handler() and single_step_dabr_instruction(). - Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6 Changelog - ver XIII (Version XII: linuxppc-dev ref: 20100121084640.ga3...@in.ibm.com) - Fixed a bug for user-space breakpoints (triggered following the failure of a breakpoint request). - Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of linux-2.6 Changelog - ver XII (Version XI: linuxppc-dev ref: 20100119091234.ga9...@in.ibm.com) - Unset MSR_SE only if kernel was not previously in single-step mode. - Pre-emption is now enabled before returning from the hw-breakpoint exception handler. - Variables to track the source of single-step exception (breakpoint from kernel, user-space vs single-stepping due to other requests) are added. - Extraneous h
Re: [Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Mon, May 17, 2010 at 10:32:41PM +1000, Paul Mackerras wrote: > On Fri, May 14, 2010 at 12:25:31PM +0530, K.Prasad wrote: > > > Okay. I will re-use single_step_exception() after modifications; it > > appearsto have no in-kernel users for it. > > It's called from exceptions-64s.S, head_32.S and head_8xx.S in > arch/powerpc/kernel. > > > > Suppose the address at which the data breakpoint has been unmapped, > > > and the process has a handler for the SIGSEGV signal. When we try to > > > single-step the load or store, we will get a DSI (0x300) interrupt, > > > call into do_page_fault, and end up sending the process a SIGSEGV. > > > That will invoke the signal handler, which can then do anything it > > > likes. It can do a blocking system call, it can longjmp() back into > > > its main event, or it can return from the signal handler. Only in the > > > last case will it retry the load or store, and then only if the signal > > > handler hasn't modified the NIP value in the signal frame. That's > > > what I mean by "doesn't return to the instruction". > > > > > > > At the outset, this seemed to be a scary thing to happen; but turns out > > to be harmful only to the extent of generating a false hw-breakpoint > > exception in certain cases. A case-by-case basis analysis reveals thus: > > > > Consider an instruction stream i1, i2, i3, ... iN, where i1 has > > finished execution and i2 is about to be executed but has generated a > > DSI interrupt with the above-mentioned conditions i.e. DSISR indicates a > > DABR match + Page-Table entry not found. Now according to do_hash_page > > in exception-64s.S (as pasted below), do_page_fault() and do_dabr() are > > invoked one after the other. > > > > _STATIC(do_hash_page) > > std r3,_DAR(r1) > > std r4,_DSISR(r1) > > > > andis. r0,r4,0xa410/* weird error? */ > > bne-handle_page_fault /* if not, try to insert a HPTE */ > > andis. r0,r4,dsisr_dabrma...@h > > bne-handle_dabr_fault > > Note that bne is not a procedure call; we'll actually get two DSIs in > this scenario. But I don't think that matters. Also note that the > branch to handle_page_fault here is not for the HPTE-not-found case; > it's for the unusual errors. So we'll handle the HPTE insertion after > handling the DABR match. > Okay. > > Thus, when control returns to user-space to instruction 'i2', the > > hw_breakpoint_handler() has completed execution, and a SIGSEGV is pending > > to be delivered and single-stepping enabled MSR_SE is set. Upon return to > > user-space, the handler for SIGSEGV is executed and it may perform one of > > the following (as you stated previously): > > (a) Make a blocking syscall, eventually yielding the CPU to a new thread > > (b) Jump to a different instruction in user-space, say iN, and not complete > > the execution of instruction i2 at all. > > (c) Return to instruction i2 and complete the execution. > > > > In case of (a), the context-switches should not affect the ability to > > single-step the instruction when the thread is eventually scheduled to > > run. The thread, when scheduled onto the CPU will complete signal > > handling, return to execute instruction i2, cause single-step exception, > > restore breakpoints and run smoothly thereafter. > > Right. However, the thread is running the signal handler without the > DABR being set, which is unfortunate. > In order to keep the breakpoint active during signal handling, a PowerPC specific signal handling code, say do_signal_pending() in arch/powerpc/kernel/signal.c, must be tapped to check for/restore the breakpoint for the process (if it exists). Then, single_step_dabr_instruction() must be suitably modified to not clear the current->thread.last_hit_ubp pointer if breakpoint has been taken in a nested condition i.e. a breakpoint exception in signal-handler which was preceded by a previous breakpoint exception in normal user-space stack. A flag to denote such a condition would be required in 'struct thread_struct'. I'm afraid if this is more complexity than we want to handle in the first patchset. I agree that this will create a 'blind-spot' of code which cannot not be monitored using breakpoints and may limit debugging efforts (specially for memory corruption); however suspecting that signal handlers (especially those that don't return to original instruction) would be rare, I feel that this could be a 'feature' that can be brought later-in. What do you think? > > In case
Re: [Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Wed, May 12, 2010 at 04:32:47PM +1000, Paul Mackerras wrote: > On Wed, May 05, 2010 at 02:03:03AM +0530, K.Prasad wrote: > > > It is true that the breakpoint exceptions will go amiss following the > > alignment exception, and be restored when the thread single-steps due > > to other requests causing undesirable effects. (Borrowing from some of > > the discussions I had with BenH, earlier) There can be two ways of > > changing the implementation to counter it: > > > > - To sense that the impending exception (alignment, page-fault, > > single-step) is a successor to a hw-breakpoint exception (and that > > restoration of debug register values is necessary), somewhere early in > > exceptions-64s.S and jump to a common handler, say > > do_single_step_dabr() which does a majority of > > single_step_dabr_instruction(). > > - To modify emulate_single_step() to also do a notify_die(DIE_SSTEP,...) > > in addition to its existing code. This would invoke > > single_step_dabr_instruction() where the breakpoints can be restored. > > I thought you would change the explicit regs->msr modification in > single_step_exception() to clear_single_step(), then just make > emulate_single_step() call single_step_exception(). > Okay. I will re-use single_step_exception() after modifications; it appearsto have no in-kernel users for it. (single_step_exception() clears MSR more than what clear_single_step() does, it shouldn't matter though). > > I must admit that it is not clear to me when you say "doesn't return to > > the instruction" and "instruction has been changed underneath". Are you > > Suppose the address at which the data breakpoint has been unmapped, > and the process has a handler for the SIGSEGV signal. When we try to > single-step the load or store, we will get a DSI (0x300) interrupt, > call into do_page_fault, and end up sending the process a SIGSEGV. > That will invoke the signal handler, which can then do anything it > likes. It can do a blocking system call, it can longjmp() back into > its main event, or it can return from the signal handler. Only in the > last case will it retry the load or store, and then only if the signal > handler hasn't modified the NIP value in the signal frame. That's > what I mean by "doesn't return to the instruction". > At the outset, this seemed to be a scary thing to happen; but turns out to be harmful only to the extent of generating a false hw-breakpoint exception in certain cases. A case-by-case basis analysis reveals thus: Consider an instruction stream i1, i2, i3, ... iN, where i1 has finished execution and i2 is about to be executed but has generated a DSI interrupt with the above-mentioned conditions i.e. DSISR indicates a DABR match + Page-Table entry not found. Now according to do_hash_page in exception-64s.S (as pasted below), do_page_fault() and do_dabr() are invoked one after the other. _STATIC(do_hash_page) std r3,_DAR(r1) std r4,_DSISR(r1) andis. r0,r4,0xa410/* weird error? */ bne-handle_page_fault /* if not, try to insert a HPTE */ andis. r0,r4,dsisr_dabrma...@h bne-handle_dabr_fault Thus, when control returns to user-space to instruction 'i2', the hw_breakpoint_handler() has completed execution, and a SIGSEGV is pending to be delivered and single-stepping enabled MSR_SE is set. Upon return to user-space, the handler for SIGSEGV is executed and it may perform one of the following (as you stated previously): (a) Make a blocking syscall, eventually yielding the CPU to a new thread (b) Jump to a different instruction in user-space, say iN, and not complete the execution of instruction i2 at all. (c) Return to instruction i2 and complete the execution. In case of (a), the context-switches should not affect the ability to single-step the instruction when the thread is eventually scheduled to run. The thread, when scheduled onto the CPU will complete signal handling, return to execute instruction i2, cause single-step exception, restore breakpoints and run smoothly thereafter. In case of (b), the new instruction iN is single-stepped, the breakpoint values are restored and the hw-breakpoint exception callback is invoked after iN is executed. The user of this breakpoint i.e. the caller of register_user_hw_breakpoint() who had placed a breakpoint on addressed accessed by instruction i2 will be confused to find that an unrelated instruction (which may not be a load/store) has caused the breakpoint. If so desired, we may adopt the 'trigger-before-execute' semantics for user-space breakpoints wherein the hw-breakpoint callback (through perf_bp_event()) is invoked in hw_breakpoint_handler() itself. This would indicate to the us
Re: [Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Wed, May 05, 2010 at 02:03:03AM +0530, K.Prasad wrote: > On Mon, May 03, 2010 at 04:23:30PM +1000, Paul Mackerras wrote: > > On Wed, Apr 14, 2010 at 09:18:27AM +0530, K.Prasad wrote: > > [snipped] > > It has been pointed out to me before (Roland's mail Ref:linuxppc-dev > message-id: 20100119100335.3eb62...@magilla.sf.frob.com) that there will > be too many corner cases that will be difficult to foresee, however your > above list appears to be exhaustive. While the alternatives to this being > a fallback to one-shot breakpoints (thereby leading to confusing > hw-breakpoint interface semantics), this is an attempt to generate > continuous and 'trigger-after-execute' (for non-ptrace requests) > breakpoint exceptions. I believe that, with the addressal of concerns > cited above, the resultant patchset would be one that achieves the > stated design goals with no loss to existing functionality. > Hi Paul, > I intend to send out another version of this patchset with fixes as > described in my replies above (unless I hear objections to it :-)). > Meanwhile, a little sickness had kept me away from working on this patchset. I have now posted a new version of the same here () which contains changes as described above. A few more changes to the patch is impending post merger of Frederic's patches (which are now in -tip) into mainline (ref: commit 73266fc1df2f94cf72b3beba3eee3b88ed0b0664 to 777d0411cd1e384115985dac5ccd42031e3eee2b); mainly due to the new ability for a per-task breakpoint to request kernel-space breakpoints (the notion of kernel- vs user-bp would also become obsolete, it is better to call them per-cpu vs per-task breakpoints). Also, I find that possibility of a kernel-thread specific breakpoint (which can migrate across CPUs) has not been thought and implemented well in this patch (will be much easier after merger of Frederic's patch). I would prefer to have atleast some version of the patch included in mainline before bringing in support for the same. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 3/3] PPC64-HWBKPT: Implement hw-breakpoints for PowerPC Book III S
Implement perf-events based hw-breakpoint interfaces for PowerPC Book III S
processors. These interfaces help arbitrate requests from various users and
schedules them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 45 +++
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 354 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c|6
arch/powerpc/kernel/ptrace.c | 64 +
arch/powerpc/kernel/traps.c |3
arch/powerpc/lib/Makefile|1
include/linux/hw_breakpoint.h|1
12 files changed, 491 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,45 @@
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_bp_generic_fields(int type, int *gen_bp_type);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp,
+ struct task_struct *tsk);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,354 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2009 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Store the 'bp' that caused the hw-breakpoint exception just before we
+ * single-step. Use it to distinguish a single-step exception (due to a
+ * previous hw-breakpoint exception) from a normal one
+ */
+static DEFINE_PER_CPU(struct perf_event *, last_hit_bp);
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+ *slot = bp;
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+ return 0;
+}
+
+/*
+ * Uninstall the breakpoint contained in the g
[Patch 2/3] Allow arch-specific cleanup before breakpoint unregistration
Certain architectures (such as PowerPC Book III S) have a need to cleanup
data-structures before the breakpoint is unregistered. This patch introduces
an arch-specific hook in release_bp_slot() along with a weak definition in
the form of a stub funciton.
Signed-off-by: K.Prasad
---
kernel/hw_breakpoint.c | 12
1 file changed, 12 insertions(+)
Index: linux-2.6.ppc64_test/kernel/hw_breakpoint.c
===
--- linux-2.6.ppc64_test.orig/kernel/hw_breakpoint.c
+++ linux-2.6.ppc64_test/kernel/hw_breakpoint.c
@@ -203,6 +203,17 @@ static void toggle_bp_slot(struct perf_e
}
/*
+ * Function to perform processor-specific cleanup during unregistration
+ */
+__weak void arch_unregister_hw_breakpoint(struct perf_event *bp)
+{
+ /*
+* A weak stub function here for those archs that don't define
+* it inside arch/.../kernel/hw_breakpoint.c
+*/
+}
+
+/*
* Contraints to check before allowing this new breakpoint counter:
*
* == Non-pinned counter == (Considered as pinned for now)
@@ -280,6 +291,7 @@ void release_bp_slot(struct perf_event *
{
mutex_lock(&nr_bp_mutex);
+ arch_unregister_hw_breakpoint(bp);
__release_bp_slot(bp);
mutex_unlock(&nr_bp_mutex);
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 1/3] PPC64-HWBKPT: Disable interrupts for data breakpoint exceptions
Data address breakpoint exceptions are currently handled along with page-faults
which require interrupts to remain in enabled state. Since exception handling
for data breakpoints aren't pre-empt safe, we handle them separately.
Signed-off-by: K.Prasad
Acked-by: Paul Mackerras
---
arch/powerpc/kernel/exceptions-64s.S | 13 -
arch/powerpc/mm/fault.c |5 +++--
2 files changed, 15 insertions(+), 3 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/exceptions-64s.S
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S
@@ -735,8 +735,11 @@ _STATIC(do_hash_page)
std r3,_DAR(r1)
std r4,_DSISR(r1)
- andis. r0,r4,0xa450/* weird error? */
+ andis. r0,r4,0xa410/* weird error? */
bne-handle_page_fault /* if not, try to insert a HPTE */
+ andis. r0,r4,dsisr_dabrma...@h
+ bne-handle_dabr_fault
+
BEGIN_FTR_SECTION
andis. r0,r4,0x0020/* Is it a segment table fault? */
bne-do_ste_alloc/* If so handle it */
@@ -823,6 +826,14 @@ END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ISER
bl .raw_local_irq_restore
b 11f
+/* We have a data breakpoint exception - handle it */
+handle_dabr_fault:
+ ld r4,_DAR(r1)
+ ld r5,_DSISR(r1)
+ addir3,r1,STACK_FRAME_OVERHEAD
+ bl .do_dabr
+ b .ret_from_except_lite
+
/* Here we have a page fault that hash_page can't handle. */
handle_page_fault:
ENABLE_INTS
Index: linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/mm/fault.c
+++ linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
@@ -151,13 +151,14 @@ int __kprobes do_page_fault(struct pt_re
if (!user_mode(regs) && (address >= TASK_SIZE))
return SIGSEGV;
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
+#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE) || \
+defined(CONFIG_PPC_BOOK3S_64))
if (error_code & DSISR_DABRMATCH) {
/* DABR match */
do_dabr(regs, address, error_code);
return 0;
}
-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
+#endif
if (in_atomic() || mm == NULL) {
if (!user_mode(regs))
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/3] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XVIII
Hi Ben/Paul, Please find a new version of the patchset that implements hw-breakpoint interfaces for PowerPC Book III S. The changes in this version over the previous version are as listed below, and are the result of addressing comments received for the previous version. Changelog - ver XVIII (Version XVII: linuxppc-dev ref: 20100414034340.ga6...@in.ibm.com) - hw-breakpoint restoration variables are cleaned-up before unregistration through a new function hook arch_unregister_hw_breakpoint(). - emulate_single_step() now notifies DIE_SSTEP to registered handlers; causes single_step_dabr_instruction() to be invoked after alignment_exception. - SIGTRAP is no longer generated for non-ptrace user-space breakpoints. - Slight code restructuring for brevity and style corrections. Kindly accept them to be a part of -next tree. Thanks, K.Prasad Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5 (found no patch induced failures). Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this change is the non-triggering of hw-breakpoints while single-stepping kernel through KGDB or Xmon. - Minor code-cleanups and addition of comments in hw_breakpoint_handler() and single_step_dabr_instruction(). - Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6 Changelog - ver XIII (Version XII: linuxppc-dev ref: 20100121084640.ga3...@in.ibm.com) - Fixed a bug for user-space breakpoints (triggered following the failure of a breakpoint request). - Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of linux-2.6 Changelog - ver XII (Version XI: linuxppc-dev ref: 20100119091234.ga9...@in.ibm.com) - Unset MSR_SE only if kernel was not previously in single-step mode. - Pre-emption is now enabled before returning from the hw-breakpoint exception handler. - Variables to track the source of single-step exception (breakpoint from kernel, user-space vs single-stepping due to other requests) are added. - Extraneous hw-breakpoint exceptions (due to memory accesses lying outside monitored symbol length) is now done for both kernel and user-space (previously only user-space). - single_step_dabr_instruction() now returns NOTIFY_DONE if kernel was in single-step mode even before the hw-breakpoint. This enables other users of single-step mode to be notified of the exception. - User-space instructions are not emulated from kernel-space, they are instead single-stepped. Changelog - ver XI -- (Version X: linuxppc-dev ref: 20091211160144.ga23...@in.ibm.com) - Conditionally unset MSR_SE in the single-step handler - Added comments to explain the duration and need for pre-empti
Re: [Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Mon, May 03, 2010 at 04:23:30PM +1000, Paul Mackerras wrote:
> On Wed, Apr 14, 2010 at 09:18:27AM +0530, K.Prasad wrote:
>
> > Implement perf-events based hw-breakpoint interfaces for PPC64 processors.
> > These interfaces help arbitrate requests from various users and schedules
> > them as appropriate.
>
> [snip]
>
Hi Paul,
Thanks for the detailed review. Please find my replies inline.
> > --- /dev/null
> > +++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
> > @@ -0,0 +1,45 @@
> > +#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
> > +#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
> > +
> > +#ifdef __KERNEL__
> > +#define__ARCH_HW_BREAKPOINT_H
>
> This symbol doesn't seem to be referenced anywhere. Is it really
> necessary to define it? I know x86 and sh do, but maybe it's a
> leftover there.
>
Yes, either I use _PPC_BOOK3S_64_HW_BREAKPOINT_H or
__ARCH_HW_BREAKPOINT_H to prevent recursive definitions, not both. I will
remove __ARCH_HW_BREAKPOINT_H.
> > ===
> > --- /dev/null
> > +++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
>
> > + switch (bp->attr.bp_type) {
> > + case HW_BREAKPOINT_R:
> > + info->type = DABR_DATA_READ;
> > + break;
> > + case HW_BREAKPOINT_W:
> > + info->type = DABR_DATA_WRITE;
> > + break;
> > + case HW_BREAKPOINT_R | HW_BREAKPOINT_W:
> > + info->type = (DABR_DATA_READ | DABR_DATA_WRITE);
> > + break;
> > + default:
> > + return ret;
> > + }
>
> You have code like this in several places -- maybe this should be
> encapsulated in a helper function. Also, I think it could be written
> more compactly.
>
Actually this particular code-snippet differs slightly from two such
'similar' switch-cases found in ptrace_set_debugreg() - which I will
replace with a helper function. The above-cited switch-case statement
which maps the generic breakpoint types to arch-specific bit-encodings
are used only once (as above) and will not help code-reduction if moved
to a separate function.
> > + /*
> > +* Return early after invoking user-callback function without restoring
> > +* DABR if the breakpoint is from ptrace which always operates in
> > +* one-shot mode
> > +*/
> > + if (is_ptrace_bp == true) {
> > + perf_bp_event(bp, regs);
> > + rc = NOTIFY_DONE;
> > + goto out;
> > + }
>
> As far as I can see, returning NOTIFY_DONE won't tell do_dabr() that
> you have handled the exception, and it will go on to do the
> debugger_dabr_match() call and generate a signal to the current
> process. Is that what you want? If so a comment would be useful.
>
Yes, the debugger_dabr_match() will return early without doing much and
the signal is generated for the ptrace-ed process. I will append the
comment to appear as below:
/*
* Return early after invoking user-callback function without restoring
* DABR if the breakpoint is from ptrace which always operates in
* one-shot mode. The ptrace-ed process will receive the SIGTRAP signal
* generated in do_dabr().
*/
> Also, the " == true" part is completely redundant. Normal kernel
> style would be if (is_ptrace_bp) { ..., and similarly the
> (is_ptrace_bp == false) above should be !is_ptrace_bp.
>
It is an untidy habit that I've had, personally, for
increased readability! I will remove such redundancies from the code.
> > +
> > + /*
> > +* Do not emulate user-space instructions from kernel-space,
> > +* instead single-step them.
> > +*/
> > + if (is_kernel == false) {
> > + current->thread.last_hit_ubp = bp;
> > + regs->msr |= MSR_SE;
> > + goto out;
> > + }
>
> I'm a bit worried about what could happen from here on. We go back
> out to userspace and try to execute the load or store. Besides
> executing successfully and taking the trace interrupt, there are
> several other possibilities:
>
The aim, during design time, has been to reliably restore the breakpoint
values immediately after execution of the instruction or in the
worst-case, to cause loss of breakpoints rather than ignore the other
sources of exceptions. Please find more responses below.
> - we could get an alignment interrupt
> - we could get a page fault (page not present or protection violation)
> - we could get a MMU hash table miss (unlikely since the low-level
> code calls hash_page before do_dabr, but poss
[RFC Patch 1/1] Implement hw-breakpoint interfaces for BookE processors
Implement hardware breakpoint interfaces for PowerPC BookE processors
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |2
arch/powerpc/include/asm/cputable.h|4
arch/powerpc/include/asm/hw_breakpoint_booke.h | 42 +++
arch/powerpc/kernel/Makefile |4
arch/powerpc/kernel/hw_breakpoint_booke.c | 326 +
arch/powerpc/kernel/ptrace.c |8
arch/powerpc/kernel/traps.c| 11
include/linux/perf_event.h |4
8 files changed, 398 insertions(+), 3 deletions(-)
Index: linux-2.6.bookE/arch/powerpc/include/asm/hw_breakpoint_booke.h
===
--- /dev/null
+++ linux-2.6.bookE/arch/powerpc/include/asm/hw_breakpoint_booke.h
@@ -0,0 +1,42 @@
+#ifndef_I386_HW_BREAKPOINT_H
+#define_I386_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#define__ARCH_HW_BREAKPOINT_H
+
+struct arch_hw_breakpoint {
+ u8 len;
+ unsigned long address;
+ unsigned long type;
+};
+
+#include
+#include
+#include
+
+/* Breakpoint length beyond which we should use 'range' breakpoints */
+#define DAC_LEN 8
+
+static inline int hw_breakpoint_slots(int type)
+{
+ return HBP_NUM;
+}
+
+struct perf_event;
+struct pmu;
+
+extern int arch_check_bp_in_kernelspace(struct perf_event *bp);
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+extern void hw_breakpoint_handler(struct pt_regs *regs,
+ unsigned long debug_status);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+
+extern struct pmu perf_ops_bp;
+
+#endif /* __KERNEL__ */
+#endif /* _I386_HW_BREAKPOINT_H */
+
Index: linux-2.6.bookE/arch/powerpc/kernel/hw_breakpoint_booke.c
===
--- /dev/null
+++ linux-2.6.bookE/arch/powerpc/kernel/hw_breakpoint_booke.c
@@ -0,0 +1,326 @@
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Store the 'bp' that caused the hw-breakpoint exception just before we
+ * single-step. Use it to distinguish a single-step exception (due to a
+ * previous hw-breakpoint exception) from a normal one
+ */
+static DEFINE_PER_CPU(struct perf_event *, last_hit_bp);
+
+/*
+ * Save the debug registers to restore them after single-stepping the
+ * instruction that caused the debug exception
+ */
+static DEFINE_PER_CPU(unsigned long, last_hit_dac[2]);
+static DEFINE_PER_CPU(unsigned long, last_hit_dbcr0);
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for each cpus
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg[HBP_NUM]);
+
+int hw_breakpoint_weight(struct perf_event *bp)
+{
+ return (bp->attr.bp_len > DAC_LEN) ? 2 : 1;
+}
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint. Eventually we enable it in the debug control register.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ bool range_bp;
+ int i;
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ unsigned long dbcr0 = mfspr(SPRN_DBCR0);
+
+ range_bp = (info->len > DAC_LEN) ? true : false;
+ for (i = 0; i < HBP_NUM; i++) {
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg[i]);
+
+ if (*slot)
+ continue;
+ *slot = bp;
+ mtspr(SPRN_DAC1, info->address);
+ /* Clean the 'type' fields to erase past values */
+ dbcr0 &= ~(DBCR0_DAC2W | DBCR0_DAC2R);
+
+ mtspr(SPRN_DBCR0, dbcr0 |
+ (info->type << (HBP_NUM - i)) | DBCR0_IDM);
+ /*
+* Use DAC2 register in 'range' mode if the length of the
+* breakpoint request is 'large'
+*/
+ if (unlikely(range_bp)) {
+ if (i > (HBP_NUM - hw_breakpoint_weight(bp))) {
+ *slot = NULL;
+ mtspr(SPRN_DBCR0, dbcr0);
+ return -EBUSY;
+ }
+ (*slot)++;
+ i++;
+ /*
+* In 'range' m
[RFC Patch 0/1] [hw-bkpt BookE] hw-breakpoint interfaces for BookE - ver I
Hi All, Please find a patch that implements hardware-breakpoint interfaces for BookE processors. The patches are under continuous development and are sent to receive early comments. For the moment, they are (only) compile tested (with ppc64e_defconfig), further testing will accompany the ongoing development. A few notes about the patchset, as below: - The patch is designed with reference to BookIII-E type processors specification (having two DAC/DVC registers). - Instruction breakpoint requests are not implemented through the generic breakpoint interfaces. Such requests are still possible for user-space through ptrace. - Breakpoint exceptions are designed to 'trigger-after-execute', although the processors raise the exception before instruction execution. To achieve this, the causative insruction is single-stepped over and the breakpoint handler is invoked in the ICMP exception handler. - The patches are dependant on the recent submissions (not yet integrated into mainline) that bring support for hw-breakpoint weight (patchset from Frederic Weisbecker LKML ref:1271999639-23605-1-git-send-regression-fweis...@gmail.com) and PPC64 hw-breakpoint support (linuxppc-dev ref:20100414034340.ga6...@in.ibm.com). Here are a few items identified to work upon in the successive versions. TO DO -- - Modify ptrace requests to use the generic hw-breakpoint interfaces (PTRACE__DEBUGREG, PTRACE_SETHWDEBUG) - Explore intergration of BookE and BookS code intergration (hw_breakpoint.c and hw_breakpoint_booke.c) - Code clean-up and reduction. Kindly let me know about comments/suggestions, if any. Thank You, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [Patch 1/1] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Wed, Apr 07, 2010 at 06:03:31PM +1000, Benjamin Herrenschmidt wrote: > Ok so too many problems with your last patch, I didn't have time to fix > them all, so it's not going into -next this week. > > Please, test with a variety of defconfigs (iseries, cell, g5 for > example), and especially with CONFIG_PERF_EVENTS not set. There are > issues in the generic header for that (though I'm told some people are > working on a fix). > > Basically, we need something like CONFIG_HW_BREAKPOINTS that is set > (maybe optionally, maybe not) with CONFIG_PERF_EVENTS and > CONFIG_HAVE_HW_BREAKPOINT. Then you can use that to properly fix the > ifdef'ing in include/linux/hw_breakpoints.h, and fix the various other > cases in powerpc code where you are testing for the wrong thing. > > Cheers, > Ben. > Hi Ben, I've sent a new patch (linuxppc-dev message-id ref: 20100414034340.ga6...@in.ibm.com) that builds against the defconfigs for various architectures pointed out by you (I did see quite a few errors that aren't induced by the patch). The source tree is buildable even without CONFIG_PERF_EVENTS, and is limited to scope using CONFIG_HAVE_HW_BREAKPOINT. At this stage, I didnot find a need for a seperate CONFIG_HW_BREAKPOINTS though. Let me know what you think. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
Implement perf-events based hw-breakpoint interfaces for PPC64 processors.
These interfaces help arbitrate requests from various users and schedules
them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 45
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 324 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c|6
arch/powerpc/kernel/ptrace.c | 81 +++
arch/powerpc/lib/Makefile|1
include/linux/hw_breakpoint.h|1
11 files changed, 475 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,45 @@
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#define__ARCH_HW_BREAKPOINT_H
+#ifdef CONFIG_HAVE_HW_BREAKPOINT
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp,
+ struct task_struct *tsk);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#endif /* CONFIG_HAVE_HW_BREAKPOINT */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,324 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2009 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Store the 'bp' that caused the hw-breakpoint exception just before we
+ * single-step. Use it to distinguish a single-step exception (due to a
+ * previous hw-breakpoint exception) from a normal one
+ */
+static DEFINE_PER_CPU(struct perf_event *, last_hit_bp);
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+ *slot = bp;
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+ return 0;
+}
+
+/*
+ * Uninstall the breakpoint contained in the given counter.
+ *
+ * First we search the debug address register it uses and then w
[Patch 1/2] PPC64-HWBKPT: Disable interrupts for data breakpoint exceptions
Data address breakpoint exceptions are currently handled along with page-faults
which require interrupts to remain in enabled state. Since exception handling
for data breakpoints aren't pre-empt safe, we handle them separately.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/exceptions-64s.S | 13 -
arch/powerpc/mm/fault.c |5 +++--
2 files changed, 15 insertions(+), 3 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/exceptions-64s.S
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S
@@ -735,8 +735,11 @@ _STATIC(do_hash_page)
std r3,_DAR(r1)
std r4,_DSISR(r1)
- andis. r0,r4,0xa450/* weird error? */
+ andis. r0,r4,0xa410/* weird error? */
bne-handle_page_fault /* if not, try to insert a HPTE */
+ andis. r0,r4,dsisr_dabrma...@h
+ bne-handle_dabr_fault
+
BEGIN_FTR_SECTION
andis. r0,r4,0x0020/* Is it a segment table fault? */
bne-do_ste_alloc/* If so handle it */
@@ -823,6 +826,14 @@ END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ISER
bl .raw_local_irq_restore
b 11f
+/* We have a data breakpoint exception - handle it */
+handle_dabr_fault:
+ ld r4,_DAR(r1)
+ ld r5,_DSISR(r1)
+ addir3,r1,STACK_FRAME_OVERHEAD
+ bl .do_dabr
+ b .ret_from_except_lite
+
/* Here we have a page fault that hash_page can't handle. */
handle_page_fault:
ENABLE_INTS
Index: linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/mm/fault.c
+++ linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
@@ -151,13 +151,14 @@ int __kprobes do_page_fault(struct pt_re
if (!user_mode(regs) && (address >= TASK_SIZE))
return SIGSEGV;
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
+#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE) || \
+defined(CONFIG_PPC_BOOK3S_64))
if (error_code & DSISR_DABRMATCH) {
/* DABR match */
do_dabr(regs, address, error_code);
return 0;
}
-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
+#endif
if (in_atomic() || mm == NULL) {
if (!user_mode(regs))
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/2] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XVII
Hi Ben, Please find a new version of the patchset. It contains small, yet significant number of changes to address the build issues you pointed out (details of which are listed in the changelog below). Changelog - ver XVII (Version XVI: linuxppc-dev ref: 20100330095809.ga14...@in.ibm.com) - CONFIG_HAVE_HW_BREAKPOINT is now used to define the scope of the new code (in lieu of CONFIG_PPC_BOOK3S_64). - CONFIG_HAVE_HW_BREAKPOINT is now dependant upon CONFIG_PERF_EVENTS and CONFIG_PPC_BOOK3S_64 (to overcome build failures under certain configs). - Included a target in arch/powerpc/lib/Makefile to build sstep.o when HAVE_HW_BREAKPOINT. - Added a dummy definition for hw_breakpoint_disable() when !HAVE_HW_BREAKPOINT. - Tested builds under defconfigs for ppc64, cell and g5. Found no patch induced failures. Kindly accept them to be a part of -next tree. Thanks, K.Prasad Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this change is the non-triggering of hw-breakpoints while single-stepping kernel through KGDB or Xmon. - Minor code-cleanups and addition of comments in hw_breakpoint_handler() and single_step_dabr_instruction(). - Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6 Changelog - ver XIII (Version XII: linuxppc-dev ref: 20100121084640.ga3...@in.ibm.com) - Fixed a bug for user-space breakpoints (triggered following the failure of a breakpoint request). - Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of linux-2.6 Changelog - ver XII (Version XI: linuxppc-dev ref: 20100119091234.ga9...@in.ibm.com) - Unset MSR_SE only if kernel was not previously in single-step mode. - Pre-emption is now enabled before returning from the hw-breakpoint exception handler. - Variables to track the source of single-step exception (breakpoint from kernel, user-space vs single-stepping due to other requests) are added. - Extraneous hw-breakpoint exceptions (due to memory accesses lying outside monitored symbol length) is now done for both kernel and user-space (previously only user-space). - single_step_dabr_instruction() now returns NOTIFY_DONE if kernel was in single-step mode even before the hw-breakpoint. This enables other users of single-step mode to be notified of the exception. - User-space instructions are not emulated from kernel-space, they are instead single-stepped. Changelog - ver XI -- (Version X: linuxppc-dev ref: 20091211160144.ga23...@in.ibm.com) - Conditionally unset MSR_SE in the single-step handler - Added comments to explain the duration and need for pre-emption disable following hw-breakpoint exception. Changelog - ver X -- - Re-write the PPC64 patches for the new implementation of hw-breakpoints that uses the perf-layer. - Rebased to commit 7622fc234190a37d4e9fe3ed944a2b61a63fca03 of -tip. Changelog - ver IX --- - Invocation of user-defined callback will be 'trigger-after-execute' (except for ptrace). - Creation of a new global per-CPU breakpoint structure to help invocation of user-defined callback from single-step handler. (Changes made now) - Validation before registration will fail only if the address does no
Re: [RFC Patch 1/2] PPC64-HWBKPT: Disable interrupts for data breakpoint exceptions
On Tue, Mar 30, 2010 at 04:32:25PM +1100, Benjamin Herrenschmidt wrote: > On Tue, 2010-03-30 at 16:24 +1100, Paul Mackerras wrote: > > On Tue, Mar 23, 2010 at 07:37:02PM +0530, K.Prasad wrote: > > > > > Index: linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S > > > === > > > --- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/exceptions-64s.S > > > +++ linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S > > > @@ -735,6 +735,9 @@ _STATIC(do_hash_page) > > > std r3,_DAR(r1) > > > std r4,_DSISR(r1) > > > > > > + andis. r0,r4,0x0040/* Data Address Breakpoint match? */ > > > > Minor comment: why not dsisr_dabrma...@h instead of 0x0040? > > > > > + bne-handle_dabr_fault > > > + > > > andis. r0,r4,0xa450/* weird error? */ > > > bne-handle_page_fault /* if not, try to insert a HPTE */ > > > BEGIN_FTR_SECTION > > > @@ -823,6 +826,15 @@ END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ISER > > > bl .raw_local_irq_restore > > > b 11f > > I would move your new test to the "weird error" case (ie, after the bne- > handle_page_fault) to avoid hitting the fast path. > Done that...so basically the branch to handle_page_fault will happen only if 0xa410 matches. The changes can be seen here: linuxppc-dev: message-id: 20100330095925.gb14...@in.ibm.com. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC Patch 1/2] PPC64-HWBKPT: Disable interrupts for data breakpoint exceptions
On Tue, Mar 30, 2010 at 04:24:42PM +1100, Paul Mackerras wrote: > On Tue, Mar 23, 2010 at 07:37:02PM +0530, K.Prasad wrote: > > > Index: linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S > > === > > --- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/exceptions-64s.S > > +++ linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S > > @@ -735,6 +735,9 @@ _STATIC(do_hash_page) > > std r3,_DAR(r1) > > std r4,_DSISR(r1) > > > > + andis. r0,r4,0x0040/* Data Address Breakpoint match? */ > > Minor comment: why not dsisr_dabrma...@h instead of 0x0040? > Sure...I didn't realise that the upper 16-bits could be extracted as shown aboveI've implemented the suggestion in the next version of the patch sent here: linuxppc-dev message-id:20100330095809.ga14...@in.ibm.com. > > + bne-handle_dabr_fault > > + > > andis. r0,r4,0xa450/* weird error? */ > > bne-handle_page_fault /* if not, try to insert a HPTE */ > > BEGIN_FTR_SECTION > > @@ -823,6 +826,15 @@ END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ISER > > bl .raw_local_irq_restore > > b 11f > > > > +/* We have a data breakpoint exception - handle it */ > > +handle_dabr_fault: > > + /* Populate the pt_regs structure */ > > Another minor comment: that comment isn't accurate since you're not > putting anything in the pt_regs, just getting arguments to do_dabr > from it. > Thanks for pointing it out...it has been removed. Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [RFC Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Mon, Mar 29, 2010 at 02:53:36PM -0500, Dave Kleikamp wrote: > On Mon, 2010-03-29 at 17:01 +0530, K.Prasad wrote: > > On Fri, Mar 26, 2010 at 04:11:45PM -0500, Dave Kleikamp wrote: > > > On Tue, 2010-03-23 at 19:37 +0530, K.Prasad wrote: > > > > plain text document attachment (ppc64_hbkpt_02) > > > > Implement perf-events based hw-breakpoint interfaces for PPC64 > > > > processors. > > > > These interfaces help arbitrate requests from various users and > > > > schedules > > > > them as appropriate. > > > > > > > > Signed-off-by: K.Prasad > > > > > > SNIP > > > > > > > Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/cputable.h > > > > === > > > > --- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/cputable.h > > > > +++ linux-2.6.ppc64_test/arch/powerpc/include/asm/cputable.h > > > > @@ -511,6 +511,13 @@ static inline int cpu_has_feature(unsign > > > > & feature); > > > > } > > > > > > > > +#define CPU_FTR_HAS_DABR (defined(CONFIG_PPC64) && \ > > > > + !defined(CONFIG_PPC_ADV_DEBUG_REGS)) > > > > +#ifdef CPU_FTR_HAS_DABR > > > > +/* Number of physical HW breakpoint registers */ > > > > +#define HBP_NUM 1 > > > > +#endif > > > > + > > > > #endif /* !__ASSEMBLY__ */ > > > > > > > > #endif /* __KERNEL__ */ > > > > > > These new defines don't really correlate to the cpu table. One would > > > expect cpu_has_feature(CPU_FTR_HAS_DABR) to have meaning, but it would > > > have to be defined similar to the other CPU_FTR_ constants, and or-ed > > > with CPU_FTRS_ALWAYS (when appropriate). > > > > > [snipped] > > There are a few issues with such an approach: > > i) Two such fields would be required in 'struct cpu_spec' - one for > > instruction breakpoints and other for data. > > ii) As pointed out by you below, hbp_num or num_hw_brkpts would always > > be assigned to the compile time constant HBP_NUM (hence a variable is not > > required to store it). > > iii) HBP_NUM still cannot be entirely removed as it is used by generic > > kernel/hw_breakpoint.c code (and is used by x86 code as well). > > > > I think the simplest approach would be to have the following entry in > > cputable.h (and get away with the rest of the additions seen in patch > > ver XV) > > > > #ifdef CONFIG_PPC_BOOK3S_64 > > #define HBP_NUM 1 > > #endif > > > > The next version of the patch should contain changes to that effect > > (assuming I hear no objections). > > I just don't think this belongs in cputable.h. Why not put this in > hw_breakpoint.h? > HBP_NUM was originally defined in hw_breakpoint.h (until ver XIV) but was moved to cputable.h as it introduces a duplicate definition in processor.h (which is how it is done in the x86 implementation). linux/hw_breakpoint.h inclusion in processor.h, to circumvent the duplication leads to circular dependancies w.r.t. declarations. I think leaving it in cputable.h (as in version XVI of the patch sent here: message-id:20100330095809.ga14...@in.ibm.com) gives us a much cleaner implementation (than x86 which has duplicate HBP_NUM definitions). Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
Implement perf-events based hw-breakpoint interfaces for PPC64 processors.
These interfaces help arbitrate requests from various users and schedules
them as appropriate.
Signed-off-by: K.Prasad
---
arch/powerpc/Kconfig |1
arch/powerpc/include/asm/cputable.h |4
arch/powerpc/include/asm/hw_breakpoint.h | 50
arch/powerpc/include/asm/processor.h |8
arch/powerpc/kernel/Makefile |1
arch/powerpc/kernel/hw_breakpoint.c | 324 +++
arch/powerpc/kernel/machine_kexec_64.c |3
arch/powerpc/kernel/process.c|6
arch/powerpc/kernel/ptrace.c | 81 +++
9 files changed, 478 insertions(+)
Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,50 @@
+#ifndef _PPC_BOOK3S_64_HW_BREAKPOINT_H
+#define _PPC_BOOK3S_64_HW_BREAKPOINT_H
+
+#ifdef __KERNEL__
+#define__ARCH_HW_BREAKPOINT_H
+#ifdef CONFIG_PPC_BOOK3S_64
+
+struct arch_hw_breakpoint {
+ u8 len; /* length of the target data symbol */
+ int type;
+ unsigned long address;
+};
+
+#include
+#include
+#include
+
+struct perf_event;
+struct pmu;
+struct perf_sample_data;
+
+#define HW_BREAKPOINT_ALIGN 0x7
+/* Maximum permissible length of any HW Breakpoint */
+#define HW_BREAKPOINT_LEN 0x8
+
+extern int arch_validate_hwbkpt_settings(struct perf_event *bp,
+ struct task_struct *tsk);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+ unsigned long val, void *data);
+int arch_install_hw_breakpoint(struct perf_event *bp);
+void arch_uninstall_hw_breakpoint(struct perf_event *bp);
+void hw_breakpoint_pmu_read(struct perf_event *bp);
+extern void flush_ptrace_hw_breakpoint(struct task_struct *tsk);
+
+extern struct pmu perf_ops_bp;
+extern void ptrace_triggered(struct perf_event *bp, int nmi,
+ struct perf_sample_data *data, struct pt_regs *regs);
+static inline void hw_breakpoint_disable(void)
+{
+ set_dabr(0);
+}
+
+#else
+static inline void hw_breakpoint_disable(void)
+{
+ /* Function is defined only on PPC_BOOK3S_64 for now */
+}
+#endif /* CONFIG_PPC_BOOK3S_64 */
+#endif /* __KERNEL__ */
+#endif /* _PPC_BOOK3S_64_HW_BREAKPOINT_H */
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
===
--- /dev/null
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/hw_breakpoint.c
@@ -0,0 +1,324 @@
+/*
+ * HW_breakpoint: a unified kernel/user-space hardware breakpoint facility,
+ * using the CPU's debug registers. Derived from
+ * "arch/x86/kernel/hw_breakpoint.c"
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+ *
+ * Copyright 2009 IBM Corporation
+ * Author: K.Prasad
+ *
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+#include
+
+/*
+ * Store the 'bp' that caused the hw-breakpoint exception just before we
+ * single-step. Use it to distinguish a single-step exception (due to a
+ * previous hw-breakpoint exception) from a normal one
+ */
+static DEFINE_PER_CPU(struct perf_event *, last_hit_bp);
+
+/*
+ * Stores the breakpoints currently in use on each breakpoint address
+ * register for every cpu
+ */
+static DEFINE_PER_CPU(struct perf_event *, bp_per_reg);
+
+/*
+ * Install a perf counter breakpoint.
+ *
+ * We seek a free debug address register and use it for this
+ * breakpoint.
+ *
+ * Atomic: we hold the counter->ctx->lock and we only handle variables
+ * and registers local to this cpu.
+ */
+int arch_install_hw_breakpoint(struct perf_event *bp)
+{
+ struct arch_hw_breakpoint *info = counter_arch_bp(bp);
+ struct perf_event **slot = &__get_cpu_var(bp_per_reg);
+
+ *slot = bp;
+ set_dabr(info->address | info->type | DABR_TRANSLATION);
+ return 0;
+}
+
+/*
+ * Uninstall the breakpoint contained in the given counter.
+ *
+ * First we search the debug address re
[Patch 1/2] PPC64-HWBKPT: Disable interrupts for data breakpoint exceptions
Data address breakpoint exceptions are currently handled along with page-faults
which require interrupts to remain in enabled state. Since exception handling
for data breakpoints aren't pre-empt safe, we handle them separately.
Signed-off-by: K.Prasad
---
arch/powerpc/kernel/exceptions-64s.S | 13 -
arch/powerpc/mm/fault.c |5 +++--
2 files changed, 15 insertions(+), 3 deletions(-)
Index: linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/kernel/exceptions-64s.S
+++ linux-2.6.ppc64_test/arch/powerpc/kernel/exceptions-64s.S
@@ -735,8 +735,11 @@ _STATIC(do_hash_page)
std r3,_DAR(r1)
std r4,_DSISR(r1)
- andis. r0,r4,0xa450/* weird error? */
+ andis. r0,r4,0xa410/* weird error? */
bne-handle_page_fault /* if not, try to insert a HPTE */
+ andis. r0,r4,dsisr_dabrma...@h
+ bne-handle_dabr_fault
+
BEGIN_FTR_SECTION
andis. r0,r4,0x0020/* Is it a segment table fault? */
bne-do_ste_alloc/* If so handle it */
@@ -823,6 +826,14 @@ END_FW_FTR_SECTION_IFCLR(FW_FEATURE_ISER
bl .raw_local_irq_restore
b 11f
+/* We have a data breakpoint exception - handle it */
+handle_dabr_fault:
+ ld r4,_DAR(r1)
+ ld r5,_DSISR(r1)
+ addir3,r1,STACK_FRAME_OVERHEAD
+ bl .do_dabr
+ b .ret_from_except_lite
+
/* Here we have a page fault that hash_page can't handle. */
handle_page_fault:
ENABLE_INTS
Index: linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
===
--- linux-2.6.ppc64_test.orig/arch/powerpc/mm/fault.c
+++ linux-2.6.ppc64_test/arch/powerpc/mm/fault.c
@@ -151,13 +151,14 @@ int __kprobes do_page_fault(struct pt_re
if (!user_mode(regs) && (address >= TASK_SIZE))
return SIGSEGV;
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
+#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE) || \
+defined(CONFIG_PPC_BOOK3S_64))
if (error_code & DSISR_DABRMATCH) {
/* DABR match */
do_dabr(regs, address, error_code);
return 0;
}
-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
+#endif
if (in_atomic() || mm == NULL) {
if (!user_mode(regs))
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
[Patch 0/2] PPC64-HWBKPT: Hardware Breakpoint interfaces - ver XVI
Hi All, Please find the version XVI of the patches that implement hardware-breakpoint interfaces for PPC64 Server machines (with one DABR). The changes over the previous version are as listed below. Changelog - ver XVI (Version XV: linuxppc-dev ref: 20100323140639.ga21...@in.ibm.com) - Used a new config option CONFIG_PPC_BOOK3S_64 (in lieu of CONFIG_PPC64/CPU_FTR_HAS_DABR) to limit the scope of the new code. - Disabled breakpoints before kexec of the machine using hw_breakpoint_disable(). - Minor optimisation in exception-64s.S to check for data breakpoint exceptions in DSISR finally (after check for other causes) + changes in code comments and representation of DSISR_DABRMATCH constant. - Rebased to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6. Let me know if you have any comments. Thanks, K.Prasad Changelog - ver XV (Version XIV: linuxppc-dev ref: 20100308181232.ga3...@in.ibm.com) - Additional patch to disable interrupts during data breakpoint exception handling. - Moved HBP_NUM definition to cputable.h under a new CPU_FTR definition (CPU_FTR_HAS_DABR). - Filtering of extraneous exceptions (due to accesses outside symbol length) is by-passed for ptrace requests. - Removed flush_ptrace_hw_breakpoint() from __switch_to() due to incorrect coding placement. - Changes to code comments as per community reviews for previous version. - Minor coding-style changes in hw_breakpoint.c as per review comments. - Re-based to commit ae6be51ed01d6c4aaf249a207b4434bc7785853b of linux-2.6 Changelog - ver XIV (Version XIII: linuxppc-dev ref: 20100215055605.gb3...@in.ibm.com) - Removed the 'name' field from 'struct arch_hw_breakpoint'. - All callback invocations through bp->overflow_handler() are replaced with perf_bp_event(). - Removed the check for pre-existing single-stepping mode in hw_breakpoint_handler() as this check is unreliable while in kernel-space. Side effect of this change is the non-triggering of hw-breakpoints while single-stepping kernel through KGDB or Xmon. - Minor code-cleanups and addition of comments in hw_breakpoint_handler() and single_step_dabr_instruction(). - Re-based to commit 25cf84cf377c0aae5dbcf937ea89bc7893db5176 of linux-2.6 Changelog - ver XIII (Version XII: linuxppc-dev ref: 20100121084640.ga3...@in.ibm.com) - Fixed a bug for user-space breakpoints (triggered following the failure of a breakpoint request). - Re-based on commit 724e6d3fe8003c3f60bf404bf22e4e331327c596 of linux-2.6 Changelog - ver XII (Version XI: linuxppc-dev ref: 20100119091234.ga9...@in.ibm.com) - Unset MSR_SE only if kernel was not previously in single-step mode. - Pre-emption is now enabled before returning from the hw-breakpoint exception handler. - Variables to track the source of single-step exception (breakpoint from kernel, user-space vs single-stepping due to other requests) are added. - Extraneous hw-breakpoint exceptions (due to memory accesses lying outside monitored symbol length) is now done for both kernel and user-space (previously only user-space). - single_step_dabr_instruction() now returns NOTIFY_DONE if kernel was in single-step mode even before the hw-breakpoint. This enables other users of single-step mode to be notified of the exception. - User-space instructions are not emulated from kernel-space, they are instead single-stepped. Changelog - ver XI -- (Version X: linuxppc-dev ref: 20091211160144.ga23...@in.ibm.com) - Conditionally unset MSR_SE in the single-step handler - Added comments to explain the duration and need for pre-emption disable following hw-breakpoint exception. Changelog - ver X -- - Re-write the PPC64 patches for the new implementation of hw-breakpoints that uses the perf-layer. - Rebased to commit 7622fc234190a37d4e9fe3ed944a2b61a63fca03 of -tip. Changelog - ver IX --- - Invocation of user-defined callback will be 'trigger-after-execute' (except for ptrace). - Creation of a new global per-CPU breakpoint structure to help invocation of user-defined callback from single-step handler. (Changes made now) - Validation before registration will fail only if the address does not match the kernel symbol's (if specified) resolved address (through kallsyms_lookup_name()). - 'symbolsize' value is expected to within the range contained by the symbol's starting address and the end of a double-word boundary (8 Bytes). - PPC64's arch-dependant code is now aware of 'cpumask' in 'struct hw_breakpoint' and can accomodate requests for a subset of CPUs in the system. - Introduced arch_disable_hw_breakpoint() required for _hw_breakpoint() APIs. Changelog - ver VIII --- - Reverting changes to allow one-shot breakpoints only for ptrace requests. - Minor chang
Re: [RFC Patch 2/2] PPC64-HWBKPT: Implement hw-breakpoints for PPC64
On Fri, Mar 26, 2010 at 04:11:45PM -0500, Dave Kleikamp wrote: > On Tue, 2010-03-23 at 19:37 +0530, K.Prasad wrote: > > plain text document attachment (ppc64_hbkpt_02) > > Implement perf-events based hw-breakpoint interfaces for PPC64 processors. > > These interfaces help arbitrate requests from various users and schedules > > them as appropriate. > > > > Signed-off-by: K.Prasad > > SNIP > > > Index: linux-2.6.ppc64_test/arch/powerpc/include/asm/cputable.h > > === > > --- linux-2.6.ppc64_test.orig/arch/powerpc/include/asm/cputable.h > > +++ linux-2.6.ppc64_test/arch/powerpc/include/asm/cputable.h > > @@ -511,6 +511,13 @@ static inline int cpu_has_feature(unsign > > & feature); > > } > > > > +#define CPU_FTR_HAS_DABR (defined(CONFIG_PPC64) && \ > > + !defined(CONFIG_PPC_ADV_DEBUG_REGS)) > > +#ifdef CPU_FTR_HAS_DABR > > +/* Number of physical HW breakpoint registers */ > > +#define HBP_NUM 1 > > +#endif > > + > > #endif /* !__ASSEMBLY__ */ > > > > #endif /* __KERNEL__ */ > > These new defines don't really correlate to the cpu table. One would > expect cpu_has_feature(CPU_FTR_HAS_DABR) to have meaning, but it would > have to be defined similar to the other CPU_FTR_ constants, and or-ed > with CPU_FTRS_ALWAYS (when appropriate). > The code can be changed as below: #if (defined(CONFIG_PPC64) && !defined(CONFIG_PPC_ADV_DEBUG_REGS)) #define CPU_FTR_HAS_DABR 1 /* Number of physical HW breakpoint registers */ #define HBP_NUM 1 #endif However, a config option CONFIG_PPC_BOOK3S_64 (I just found) whose scope includes only 64-bit server processors (having one DABR) to be the most suitable. I think it must be used in lieu of introducing a new CPU_FTR_HAS_DABR definition in cputable.h > Similarly, I would expect the cpu_spec structure to have a new field, > hbp_num, which is initialized in cputable.c. Maybe a longer name would > be better, num_hw_brkpts? > There are a few issues with such an approach: i) Two such fields would be required in 'struct cpu_spec' - one for instruction breakpoints and other for data. ii) As pointed out by you below, hbp_num or num_hw_brkpts would always be assigned to the compile time constant HBP_NUM (hence a variable is not required to store it). iii) HBP_NUM still cannot be entirely removed as it is used by generic kernel/hw_breakpoint.c code (and is used by x86 code as well). I think the simplest approach would be to have the following entry in cputable.h (and get away with the rest of the additions seen in patch ver XV) #ifdef CONFIG_PPC_BOOK3S_64 #define HBP_NUM 1 #endif The next version of the patch should contain changes to that effect (assuming I hear no objections). > When I added the PPC_ADV_DEBUG config options for the bookE features, I > didn't see an immediate need to clutter the cputable since their values > are fixed at compile time. We should be consistent with these, but It is even more true with ppc64-server processors, where the number of debug registers (denoted by HBP_NUM) is fixed to 1 (unlike BookE where the DACs can be used in standalone or as a pair of registers). > unless we are going to determine any of these at run-time, I don't know > that they belong in the cpu table. > > Thanks, > Shaggy > -- Thanks, K.Prasad ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
