diff mbox

[RFC,1/6] Prepare the PowerPC platform for HW Breakpoint infrastructure

Message ID 20090514134335.GB14229@in.ibm.com (mailing list archive)
State Changes Requested, archived
Headers show

Commit Message

K.Prasad May 14, 2009, 1:43 p.m. UTC 
Prepare the PowerPC code for HW Breakpoint infrastructure patches by including
relevant constant definitions and function declarations.

Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/hw_breakpoint.h |   49 +++++
 arch/powerpc/include/asm/processor.h     |    1 
 arch/powerpc/include/asm/reg.h           |    4

Comments

Benjamin Herrenschmidt May 18, 2009, 3:35 a.m. UTC | #1 
On Thu, 2009-05-14 at 19:13 +0530, K.Prasad wrote:
> plain text document attachment (ppc64_prepare_code_01)
> Prepare the PowerPC code for HW Breakpoint infrastructure patches by including
> relevant constant definitions and function declarations.

Hi !

Some comments below...

> +#define   HBP_NUM	1	/* Number of physical HW breakpoint registers */

Can you use a more verbose constant ? reg.h is included everywhere so
the risk of collision is high.

>  #define   DABR_TRANSLATION	(1UL << 2)
>  #define   DABR_DATA_WRITE	(1UL << 1)
>  #define   DABR_DATA_READ	(1UL << 0)
> +#define   DABR_DATA_RW		(3UL << 0)

Do you really need that ? It's just DABR_DATA_WRITE | DABR_DATA_READ :-)

Cheers,
Ben.
K.Prasad May 18, 2009, 4:15 p.m. UTC | #2 
On Mon, May 18, 2009 at 01:35:32PM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2009-05-14 at 19:13 +0530, K.Prasad wrote:
> > plain text document attachment (ppc64_prepare_code_01)
> > Prepare the PowerPC code for HW Breakpoint infrastructure patches by including
> > relevant constant definitions and function declarations.
> 
> Hi !
> 
> Some comments below...
> 
> > +#define   HBP_NUM	1	/* Number of physical HW breakpoint registers */
> 
> Can you use a more verbose constant ? reg.h is included everywhere so
> the risk of collision is high.
>

This constant is used by the generic HW Breakpoint code in
kernel/hw_breakpoint.c too and renaming it here will require changes
there too, while I don't see any existing name-space clashes.

Instead the definition can be moved into arch/powerpc/include/asm/hw_breakpoint.h
and in arch/power/include/asm/processor.h. What do you think?
 
> >  #define   DABR_TRANSLATION	(1UL << 2)
> >  #define   DABR_DATA_WRITE	(1UL << 1)
> >  #define   DABR_DATA_READ	(1UL << 0)
> > +#define   DABR_DATA_RW		(3UL << 0)
> 
> Do you really need that ? It's just DABR_DATA_WRITE | DABR_DATA_READ :-)
> 

Yes, it looks silly. I will remove it. The HW_BREAKPOINT_RW defined in
arch/powerpc/include/asm/hw_breakpoint.h will instead be defined as:

#define HW_BREAKPOINT_RW (DABR_DATA_READ | DABR_DATA_WRITE)


> Cheers,
> Ben.
>

Thanks,
K.Prasad
diff mbox

Patch

Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
===================================================================
--- /dev/null
+++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/hw_breakpoint.h
@@ -0,0 +1,49 @@ 
+#ifndef	_PPC64_HW_BREAKPOINT_H
+#define	_PPC64_HW_BREAKPOINT_H
+
+#ifdef	__KERNEL__
+#define	__ARCH_HW_BREAKPOINT_H
+
+struct arch_hw_breakpoint {
+	char		*name; /* Contains name of the symbol to set bkpt */
+	unsigned long	address;
+	u8		type;
+};
+
+#include <linux/kdebug.h>
+#include <asm/reg.h>
+#include <asm-generic/hw_breakpoint.h>
+
+#define HW_BREAKPOINT_READ DABR_DATA_READ
+#define HW_BREAKPOINT_WRITE DABR_DATA_WRITE
+#define HW_BREAKPOINT_RW DABR_DATA_RW
+
+#define HW_BREAKPOINT_ALIGN 0x7
+#define HW_BREAKPOINT_LEN INSTRUCTION_LEN
+
+extern struct hw_breakpoint *hbp_kernel[HBP_NUM];
+extern unsigned int hbp_user_refcount[HBP_NUM];
+
+extern void arch_install_thread_hw_breakpoint(struct task_struct *tsk);
+extern void arch_uninstall_thread_hw_breakpoint(void);
+extern int arch_validate_hwbkpt_settings(struct hw_breakpoint *bp,
+						struct task_struct *tsk);
+extern void arch_update_user_hw_breakpoint(int pos, struct task_struct *tsk);
+extern void arch_flush_thread_hw_breakpoint(struct task_struct *tsk);
+extern void arch_update_kernel_hw_breakpoint(void *);
+extern int hw_breakpoint_exceptions_notify(struct notifier_block *unused,
+				     unsigned long val, void *data);
+
+extern void flush_thread_hw_breakpoint(struct task_struct *tsk);
+extern int copy_thread_hw_breakpoint(struct task_struct *tsk,
+		struct task_struct *child, unsigned long clone_flags);
+extern void switch_to_thread_hw_breakpoint(struct task_struct *tsk);
+
+static inline void hw_breakpoint_disable(void)
+{
+	set_dabr(0);
+}
+
+#endif	/* __KERNEL__ */
+#endif	/* _PPC64_HW_BREAKPOINT_H */
+
Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/processor.h
+++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/processor.h
@@ -177,6 +177,7 @@  struct thread_struct {
 #ifdef CONFIG_PPC64
 	unsigned long	start_tb;	/* Start purr when proc switched in */
 	unsigned long	accum_tb;	/* Total accumilated purr for process */
+	struct hw_breakpoint *hbp[HBP_NUM];
 #endif
 	unsigned long	dabr;		/* Data address breakpoint register */
 #ifdef CONFIG_ALTIVEC
Index: linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
===================================================================
--- linux-2.6-tip.hbkpt.orig/arch/powerpc/include/asm/reg.h
+++ linux-2.6-tip.hbkpt/arch/powerpc/include/asm/reg.h
@@ -26,6 +26,8 @@ 
 #include <asm/reg_8xx.h>
 #endif /* CONFIG_8xx */
 
+#define INSTRUCTION_LEN	4		/* Length of any instruction */
+
 #define MSR_SF_LG	63              /* Enable 64 bit mode */
 #define MSR_ISF_LG	61              /* Interrupt 64b mode valid on 630 */
 #define MSR_HV_LG 	60              /* Hypervisor state */
@@ -184,9 +186,11 @@ 
 #define   CTRL_TE	0x00c00000	/* thread enable */
 #define   CTRL_RUNLATCH	0x1
 #define SPRN_DABR	0x3F5	/* Data Address Breakpoint Register */
+#define   HBP_NUM	1	/* Number of physical HW breakpoint registers */
 #define   DABR_TRANSLATION	(1UL << 2)
 #define   DABR_DATA_WRITE	(1UL << 1)
 #define   DABR_DATA_READ	(1UL << 0)
+#define   DABR_DATA_RW		(3UL << 0)
 #define SPRN_DABR2	0x13D	/* e300 */
 #define SPRN_DABRX	0x3F7	/* Data Address Breakpoint Register Extension */
 #define   DABRX_USER	(1UL << 0)