[RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Dave Kleikamp]

These patches implement an extention to the ptrace interface proposed by
Thiago Bauermann and the the PowerPC gdb team.

GDB intends to support the following hardware debug features of BookE
processors:

4 hardware breakpoints (IAC)
2 hardware watchpoints (read, write and read-write) (DAC)
2 value conditions for the hardware watchpoints (DVC)

For that, we need to extend ptrace so that GDB can query and set these
resources. Since we're extending, we're trying to create an interface
that's extendable and that covers both BookE and server processors, so
that GDB doesn't need to special-case each of them. We propose the
following 3 new ptrace requests described below.

There have been discussions of a generic hardware debug interface for the
kernel which would hopefully contemplate all the functionality below and
supersede it.  But we need something that works now, and which enables GDB
to be simpler and work with both Server and Embedded processors without
special cases.

1. PTRACE_PPC_GETHWDEBUGINFO

Query for GDB to discover the hardware debug features. The main info to
be returned here is the minimum alignment for the hardware watchpoints.
BookE processors don't have restrictions here, but server processors have
an 8-byte alignment restriction for hardware watchpoints. We'd like to avoid
adding special cases to GDB based on what it sees in AUXV.

Since we're at it, we added other useful info that the kernel can return to
GDB: this query will return the number of hardware breakpoints, hardware
watchpoints and whether it supports a range of addresses and a condition.
The query will fill the following structure provided by the requesting process:

struct ppc_debug_info {
       unit32_t version;
       unit32_t num_instruction_bps;
       unit32_t num_data_bps;
       unit32_t num_condition_regs;
       unit32_t data_bp_alignment;
       unit32_t sizeof_condition; /* size of the DVC register */
       uint64_t features; /* bitmask of the individual flags */
};

features will have bits indicating whether there is support for:

#define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
#define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
#define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
#define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8

2. PTRACE_SETHWDEBUG

Sets a hardware breakpoint or watchpoint, according to the provided structure:

struct ppc_hw_breakpoint {
        uint32_t version;
#define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
#define PPC_BREAKPOINT_TRIGGER_READ     0x2
#define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
        uint32_t trigger_type;       /* only some combinations allowed */
#define PPC_BREAKPOINT_MODE_EXACT               0x0
#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
#define PPC_BREAKPOINT_MODE_MASK                0x3
        uint32_t addr_mode;          /* address match mode */

#define PPC_BREAKPOINT_CONDITION_NONE   0x0
#define PPC_BREAKPOINT_CONDITION_AND    0x1
#define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for the same thing as above */
#define PPC_BREAKPOINT_CONDITION_OR     0x2
#define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
#define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable bits */
#define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
        uint32_t condition_mode;     /* break/watchpoint condition flags */

        uint64_t addr;
        uint64_t addr2;
        uint64_t condition_value;
};

A request specifies one event, not necessarily just one register to be set.
For instance, if the request is for a watchpoint with a condition, both the
DAC and DVC registers will be set in the same request.

With this GDB can ask for all kinds of hardware breakpoints and watchpoints
that the BookE supports. COMEFROM breakpoints available in server processors
are not contemplated, but that is out of the scope of this work.

ptrace will return an integer (handle) uniquely identifying the breakpoint or
watchpoint just created. This integer will be used in the PTRACE_DELHWDEBUG
request to ask for its removal. Return -ENOSPC if the requested breakpoint
can't be allocated on the registers.

Some examples of using the structure to:

- set a breakpoint in the first breakpoint register

  p.version         = PPC_DEBUG_CURRENT_VERSION;
  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
  p.addr            = (uint64_t) address;
  p.addr2           = 0;
  p.condition_value = 0;

- set a watchpoint which triggers on reads in the second watchpoint register

  p.version         = PPC_DEBUG_CURRENT_VERSION;
  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_READ;
  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
  p.addr            = (uint64_t) address;
  p.addr2           = 0;
  p.condition_value = 0;

- set a watchpoint which triggers only with a specific value

  p.version         = PPC_DEBUG_CURRENT_VERSION;
  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_READ;
  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
  p.condition_mode  = PPC_BREAKPOINT_CONDITION_AND | PPC_BREAKPOINT_CONDITION_BE_ALL;
  p.addr            = (uint64_t) address;
  p.addr2           = 0;
  p.condition_value = (uint64_t) condition;

- set a ranged hardware breakpoint

  p.version         = PPC_DEBUG_CURRENT_VERSION;
  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
  p.addr_mode       = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
  p.addr            = (uint64_t) begin_range;
  p.addr2           = (uint64_t) end_range;
  p.condition_value = 0;

3. PTRACE_DELHWDEBUG

Takes an integer which identifies an existing breakpoint or watchpoint
(i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
corresponding breakpoint or watchpoint..

-- 
Dave Kleikamp
IBM Linux Technology Center

[RFC:PATCH 01/03] powerpc: Extended ptrace interface

[Dave Kleikamp]

powerpc: Extended ptrace interface

From: Torez Smith <lnxtorez@linux.vnet.ibm.com>

Add a new extended ptrace interface so that user-space has a single
interface for powerpc, without having to know the specific layout
of the debug registers.

Implement:
PPC_PTRACE_GETHWDEBUGINFO
PPC_PTRACE_SETHWDEBUG
PPC_PTRACE_DELHWDEBUG

Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
Cc: David Gibson <dwg@au1.ibm.com>
Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
---

 arch/powerpc/include/asm/ptrace.h |   75 ++++++++++++++++++++++++++++++++
 arch/powerpc/kernel/ptrace.c      |   88 +++++++++++++++++++++++++++++++++++++
 2 files changed, 163 insertions(+), 0 deletions(-)


diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h
index 8c34149..7ae887b 100644
--- a/arch/powerpc/include/asm/ptrace.h
+++ b/arch/powerpc/include/asm/ptrace.h
@@ -24,6 +24,12 @@
  * 2 of the License, or (at your option) any later version.
  */
 
+#ifdef __KERNEL__
+#include <linux/types.h>
+#else
+#include <stdint.h>
+#endif
+
 #ifndef __ASSEMBLY__
 
 struct pt_regs {
@@ -292,4 +298,73 @@ extern void user_disable_single_step(struct task_struct *);
 
 #define PTRACE_SINGLEBLOCK	0x100	/* resume execution until next branch */
 
+#define PPC_PTRACE_GETHWDBGINFO	0x89
+#define PPC_PTRACE_SETHWDEBUG	0x88
+#define PPC_PTRACE_DELHWDEBUG	0x87
+
+#ifndef __ASSEMBLY__
+
+struct ppc_debug_info {
+	uint32_t version;		/* Only version 1 exists to date */
+	uint32_t num_instruction_bps;
+	uint32_t num_data_bps;
+	uint32_t num_condition_regs;
+	uint32_t data_bp_alignment;
+	uint32_t sizeof_condition;	/* size of the DVC register */
+	uint64_t features;
+};
+
+#endif /* __ASSEMBLY__ */
+
+/*
+ * features will have bits indication whether there is support for:
+ */
+#define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
+#define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
+#define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
+#define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
+
+#ifndef __ASSEMBLY__
+
+struct ppc_hw_breakpoint {
+	uint32_t version;		/* currently, version must be 1 */
+	uint32_t trigger_type;		/* only some combinations allowed */
+	uint32_t addr_mode;		/* address match mode */
+	uint32_t condition_mode;	/* break/watchpoint condition flags */
+	uint64_t addr;			/* break/watchpoint address */
+	uint64_t addr2;			/* range end or mask */
+	uint64_t condition_value;	/* contents of the DVC register */
+};
+
+#endif /* __ASSEMBLY__ */
+
+/*
+ * Trigger Type
+ */
+#define PPC_BREAKPOINT_TRIGGER_EXECUTE	0x1
+#define PPC_BREAKPOINT_TRIGGER_READ	0x2
+#define PPC_BREAKPOINT_TRIGGER_WRITE	0x4
+#define PPC_BREAKPOINT_TRIGGER_RW	0x6
+
+/*
+ * Address Mode
+ */
+#define PPC_BREAKPOINT_MODE_EXACT		0x0
+#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE	0x1
+#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE	0x2
+#define PPC_BREAKPOINT_MODE_MASK		0x3
+
+/*
+ * Condition Mode
+ */
+#define PPC_BREAKPOINT_CONDITION_NONE	0x0
+#define PPC_BREAKPOINT_CONDITION_AND	0x1
+#define PPC_BREAKPOINT_CONDITION_EXACT	0x1
+#define PPC_BREAKPOINT_CONDITION_OR	0x2
+#define PPC_BREAKPOINT_CONDITION_AND_OR	0x3
+#define PPC_BREAKPOINT_CONDITION_BE_ALL	0x00ff0000
+#define PPC_BREAKPOINT_CONDITION_BE_SHIFT	16
+#define PPC_BREAKPOINT_CONDITION_BE(n)	\
+	(1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
+
 #endif /* _ASM_POWERPC_PTRACE_H */
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index ef14988..6be2ce0 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -839,6 +839,50 @@ void ptrace_disable(struct task_struct *child)
 	user_disable_single_step(child);
 }
 
+static long ppc_set_hwdebug(struct task_struct *child,
+		     struct ppc_hw_breakpoint *bp_info)
+{
+	/*
+	 * We currently support one data breakpoint
+	 */
+	if (((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) == 0) ||
+	    ((bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) != 0) ||
+	    (bp_info->trigger_type != PPC_BREAKPOINT_TRIGGER_WRITE) ||
+	    (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;
+
+	child->thread.dabr = (unsigned long)bp_info->addr;
+#ifdef CONFIG_BOOKE
+	child->thread.dbcr0 = DBCR0_IDM;
+	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
+		child->thread.dbcr0 |= DBSR_DAC1R;
+	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
+		child->thread.dbcr0 |= DBSR_DAC1W;
+	child->thread.regs->msr |= MSR_DE;
+#endif
+	return 1;
+}
+
+static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
+{
+	if ((data != 1) || (child->thread.dabr == 0))
+		return -EINVAL;
+
+	child->thread.dabr = 0;
+#ifdef CONFIG_BOOKE
+	child->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
+	child->thread.regs->msr &= ~MSR_DE;
+#endif
+	return 0;
+}
+
 /*
  * Here are the old "legacy" powerpc specific getregs/setregs ptrace calls,
  * we mark them as obsolete now, they will be removed in a future version
@@ -932,6 +976,50 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 		break;
 	}
 
+	case PPC_PTRACE_GETHWDBGINFO: {
+		struct ppc_debug_info dbginfo;
+
+		dbginfo.version = 1;
+		dbginfo.num_instruction_bps = 0;
+		dbginfo.num_data_bps = 1;
+		dbginfo.num_condition_regs = 0;
+#ifdef CONFIG_PPC64
+		dbginfo.data_bp_alignment = 8;
+#else
+		dbginfo.data_bp_alignment = 0;
+#endif
+		dbginfo.sizeof_condition = 0;
+		dbginfo.features = 0;
+
+		if (!access_ok(VERIFY_WRITE, data,
+			       sizeof(struct ppc_debug_info)))
+			return -EFAULT;
+		ret = __copy_to_user((struct ppc_debug_info __user *)data,
+				     &dbginfo, sizeof(struct ppc_debug_info)) ?
+		      -EFAULT : 0;
+		break;
+	}
+
+	case PPC_PTRACE_SETHWDEBUG: {
+		struct ppc_hw_breakpoint bp_info;
+
+		if (!access_ok(VERIFY_READ, data,
+			       sizeof(struct ppc_hw_breakpoint)))
+			return -EFAULT;
+		ret = __copy_from_user(&bp_info,
+				       (struct ppc_hw_breakpoint __user *)data,
+				       sizeof(struct ppc_hw_breakpoint)) ?
+		      -EFAULT : 0;
+		if (!ret)
+			ret = ppc_set_hwdebug(child, &bp_info);
+		break;
+	}
+
+	case PPC_PTRACE_DELHWDEBUG: {
+		ret = ppc_del_hwdebug(child, addr, data);
+		break;
+	}
+
 	case PTRACE_GET_DEBUGREG: {
 		ret = -EINVAL;
 		/* We only support one DABR and no IABRS at the moment */

-- 
Dave Kleikamp
IBM Linux Technology Center

[RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Dave Kleikamp]

powerpc: Add definitions for Debug Registers on BookE Platforms

From: Torez Smith <lnxtorez@linux.vnet.ibm.com>

This patch adds additional definitions for BookE Debug Registers
to the reg_booke.h header file.

Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
Cc: David Gibson <dwg@au1.ibm.com>
Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
---

 arch/powerpc/include/asm/processor.h |   30 +++++-
 arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
 2 files changed, 178 insertions(+), 28 deletions(-)


diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 9eed29e..1393307 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -161,9 +161,35 @@ struct thread_struct {
 #ifdef CONFIG_PPC32
 	void		*pgdir;		/* root of page-table tree */
 #endif
-#if defined(CONFIG_4xx) || defined (CONFIG_BOOKE)
-	unsigned long	dbcr0;		/* debug control register values */
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+	/*
+	 * The following help to manage the use of Debug Control Registers
+	 * om the BookE platforms.
+	 */
+	unsigned long	dbcr0;
 	unsigned long	dbcr1;
+	unsigned long	dbcr2;
+	/*
+	 * The stored value of the DBSR register will be the value at the
+	 * last debug interrupt. This register can only be read from the
+	 * user (will never be written to) and has value while helping to
+	 * describe the reason for the last debug trap.  Torez
+	 */
+	unsigned long	dbsr;
+	/*
+	 * The following will contain addresses used by debug applications
+	 * to help trace and trap on particular address locations.
+	 * The bits in the Debug Control Registers above help define which
+	 * of the following registers will contain valid data and/or addresses.
+	 */
+	unsigned long	iac1;
+	unsigned long	iac2;
+	unsigned long	iac3;
+	unsigned long	iac4;
+	unsigned long	dac1;
+	unsigned long	dac2;
+	unsigned long	dvc1;
+	unsigned long	dvc2;
 #endif
 	/* FP and VSX 0-31 register set */
 	double		fpr[32][TS_FPRWIDTH];
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 3bf7835..7f8c71f 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -248,6 +248,8 @@
 #define DBSR_RET	0x00008000	/* Return Debug Event */
 #define DBSR_CIRPT	0x00000040	/* Critical Interrupt Taken Event */
 #define DBSR_CRET	0x00000020	/* Critical Return Debug Event */
+#define DBSR_IAC12ATS	0x00000002	/* Instr Address Compare 1/2 Toggle */
+#define DBSR_IAC34ATS	0x00000001	/* Instr Address Compare 3/4 Toggle */
 #endif
 #ifdef CONFIG_40x
 #define DBSR_IC		0x80000000	/* Instruction Completion */
@@ -294,25 +296,68 @@
 #define DBCR0_IC	0x08000000	/* Instruction Completion */
 #define DBCR0_ICMP	DBCR0_IC
 #define DBCR0_BT	0x04000000	/* Branch Taken */
-#define DBCR0_BRT	DBCR0_BT
 #define DBCR0_EDE	0x02000000	/* Exception Debug Event */
 #define DBCR0_IRPT	DBCR0_EDE
 #define DBCR0_TDE	0x01000000	/* TRAP Debug Event */
-#define DBCR0_IA1	0x00800000	/* Instr Addr compare 1 enable */
-#define DBCR0_IAC1	DBCR0_IA1
-#define DBCR0_IA2	0x00400000	/* Instr Addr compare 2 enable */
-#define DBCR0_IAC2	DBCR0_IA2
-#define DBCR0_IA12	0x00200000	/* Instr Addr 1-2 range enable */
-#define DBCR0_IA12X	0x00100000	/* Instr Addr 1-2 range eXclusive */
-#define DBCR0_IA3	0x00080000	/* Instr Addr compare 3 enable */
-#define DBCR0_IAC3	DBCR0_IA3
-#define DBCR0_IA4	0x00040000	/* Instr Addr compare 4 enable */
-#define DBCR0_IAC4	DBCR0_IA4
-#define DBCR0_IA34	0x00020000	/* Instr Addr 3-4 range Enable */
-#define DBCR0_IA34X	0x00010000	/* Instr Addr 3-4 range eXclusive */
-#define DBCR0_IA12T	0x00008000	/* Instr Addr 1-2 range Toggle */
-#define DBCR0_IA34T	0x00004000	/* Instr Addr 3-4 range Toggle */
+#define DBCR0_IAC1	0x00800000	/* Instr Addr compare 1 enable */
+#define DBCR0_IAC2	0x00400000	/* Instr Addr compare 2 enable */
+#define DBCR0_IAC12M	0x00300000	/* Instr Addr 1-2 range enable */
+#define DBCR0_IAC12M_R	0x00100000	/* Instr Addr 1-2 Reserved state */
+#define DBCR0_IAC12M_I	0x00200000	/* Instr Addr 1-2 range Inclusive */
+#define DBCR0_IAC12M_X	0x00300000	/* Instr Addr 1-2 range eXclusive */
+#define DBCR0_IAC3	0x00080000	/* Instr Addr compare 3 enable */
+#define DBCR0_IAC4	0x00040000	/* Instr Addr compare 4 enable */
+#define DBCR0_IAC34	0x00020000	/* Instr Addr 3-4 range Enable */
+#define DBCR0_IAC34X	0x00010000	/* Instr Addr 3-4 range eXclusive */
+#define DBCR0_IAC12T	0x00008000	/* Instr Addr 1-2 range Toggle */
+#define DBCR0_IAC34T	0x00004000	/* Instr Addr 3-4 range Toggle */
 #define DBCR0_FT	0x00000001	/* Freeze Timers on debug event */
+
+#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
+				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
+#define DBCR0_BASE_REG_VALUE	0
+
+#define dbcr_iac_range(task)	((task)->thread.dbcr0)
+#define DBCR_IAC12M	DBCR0_IAC12M
+#define DBCR_IAC12M_I	DBCR0_IAC12M_I
+#define DBCR_IAC12M_X	DBCR0_IAC12M_X
+#define DBCR_IAC34M	DBCR0_IAC34M
+#define DBCR_IAC34M_I	DBCR0_IAC34M_I
+#define DBCR_IAC34M_X	DBCR0_IAC34M_X
+
+/* Bit definitions related to the DBCR1. */
+#define DBCR1_D1R	0x80000000	/* DAC1 Read Debug Event */
+#define DBCR1_DAC1R	DBCR1_D1R
+#define DBCR1_D2R	0x40000000	/* DAC2 Read Debug Event */
+#define DBCR1_DAC2R	DBCR1_D2R
+#define DBCR1_D1W	0x20000000	/* DAC1 Write Debug Event */
+#define DBCR1_DAC1W	DBCR1_D1W
+#define DBCR1_D2W	0x10000000	/* DAC2 Write Debug Event */
+#define DBCR1_DAC2W	DBCR1_D2W
+
+#define DBCR1_USER_DEBUG	(DBCR1_DAC1R | DBCR1_DAC2R | DBCR1_DAC1W | \
+				 DBCR1_DAC2W)
+#define DBCR1_BASE_REG_VALUE	0
+
+#define dbcr_dac(task)	((task)->thread.dbcr1)
+#define DBCR_DAC1R	DBCR1_DAC1R
+#define DBCR_DAC1W	DBCR1_DAC1W
+#define DBCR_DAC2R	DBCR1_DAC2R
+#define DBCR_DAC2W	DBCR1_DAC2W
+
+#define DBCR2_USER_DEBUG	0
+#define DBCR2_BASE_REG_VALUE	0
+
+/*
+ * Are there any active Debug Events represented in the
+ * Debug Control Registers?
+ */
+#define DBCR0_ACTIVE_EVENTS	(DBCR0_ICMP | DBCR0_IAC1 | DBCR0_IAC2 | \
+				 DBCR0_IAC3 | DBCR0_IAC4)
+#define DBCR1_ACTIVE_EVENTS	(DBCR1_D1R | DBCR1_D2R | DBCR1_D1W | DBCR1_D2W)
+#define DBCR_ACTIVE_EVENTS(dbcr0, dbcr1)  (((dbcr0) & DBCR0_ACTIVE_EVENTS) || \
+					   ((dbcr1) & DBCR1_ACTIVE_EVENTS))
+
 #elif defined(CONFIG_BOOKE)
 #define DBCR0_EDM	0x80000000	/* External Debug Mode */
 #define DBCR0_IDM	0x40000000	/* Internal Debug Mode */
@@ -324,8 +369,7 @@
 #define DBCR0_RST_NONE	0x00000000	/* No Reset */
 #define DBCR0_ICMP	0x08000000	/* Instruction Completion */
 #define DBCR0_IC	DBCR0_ICMP
-#define DBCR0_BRT	0x04000000	/* Branch Taken */
-#define DBCR0_BT	DBCR0_BRT
+#define DBCR0_BT	0x04000000	/* Branch Taken */
 #define DBCR0_IRPT	0x02000000	/* Exception Debug Event */
 #define DBCR0_TDE	0x01000000	/* TRAP Debug Event */
 #define DBCR0_TIE	DBCR0_TDE
@@ -342,19 +386,99 @@
 #define DBCR0_CRET	0x00000020	/* Critical Return Debug Event */
 #define DBCR0_FT	0x00000001	/* Freeze Timers on debug event */
 
+#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
+				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \
+				 DBCR0_DAC1R | DBCR0_DAC1W  | DBCR0_DAC2R | \
+				 DBCR0_DAC2W)
+#define DBCR0_BASE_REG_VALUE	0
+
+#define dbcr_dac(task)	((task)->thread.dbcr0)
+#define DBCR_DAC1R	DBCR0_DAC1R
+#define DBCR_DAC1W	DBCR0_DAC1W
+#define DBCR_DAC2R	DBCR0_DAC2R
+#define DBCR_DAC2W	DBCR0_DAC2W
+
 /* Bit definitions related to the DBCR1. */
-#define DBCR1_IAC12M	0x00800000	/* Instr Addr 1-2 range enable */
-#define DBCR1_IAC12MX	0x00C00000	/* Instr Addr 1-2 range eXclusive */
-#define DBCR1_IAC12AT	0x00010000	/* Instr Addr 1-2 range Toggle */
-#define DBCR1_IAC34M	0x00000080	/* Instr Addr 3-4 range enable */
-#define DBCR1_IAC34MX	0x000000C0	/* Instr Addr 3-4 range eXclusive */
-#define DBCR1_IAC34AT	0x00000001	/* Instr Addr 3-4 range Toggle */
+#define DBCR1_IAC1US	0xC0000000	/* Instr Addr Cmp 1 Sup/User   */
+#define DBCR1_IAC1ER	0x30000000	/* Instr Addr Cmp 1 Eff/Real */
+#define DBCR1_IAC1ER_01	0x10000000	/* reserved */
+#define DBCR1_IAC1ER_10	0x20000000	/* Instr Addr Cmp 1 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC1ER_11	0x30000000	/* Instr Addr Cmp 1 Eff/Real MSR[IS]=1 */
+#define DBCR1_IAC2US	0x0C000000	/* Instr Addr Cmp 2 Sup/User   */
+#define DBCR1_IAC2ER	0x03000000	/* Instr Addr Cmp 2 Eff/Real */
+#define DBCR1_IAC2ER_01	0x01000000	/* reserved */
+#define DBCR1_IAC2ER_10	0x02000000	/* Instr Addr Cmp 2 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC2ER_11	0x03000000	/* Instr Addr Cmp 2 Eff/Real MSR[IS]=1 */
+#define DBCR1_IAC12M	0x00C00000	/* Instr Addr 1-2 range enable */
+#define DBCR1_IAC12M_R	0x00400000	/* Instr Addr 1-2 reserved state */
+#define DBCR1_IAC12M_I	0x00800000	/* Instr Addr 1-2 range inclusive */
+#define DBCR1_IAC12M_X	0x00C00000	/* Instr Addr 1-2 range eXclusive */
+#define DBCR1_IAC12A_T	0x00010000	/* Instr Addr 1-2 range Toggle */
+#define DBCR1_IAC3US	0x0000C000	/* Instr Addr Cmp 3 Sup/User   */
+#define DBCR1_IAC3ER	0x00003000	/* Instr Addr Cmp 3 Eff/Real */
+#define DBCR1_IAC3ER_01	0x00001000	/* reserved */
+#define DBCR1_IAC3ER_10	0x00002000	/* Instr Addr Cmp 3 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC3ER_11	0x00003000	/* Instr Addr Cmp 3 Eff/Real MSR[IS]=1 */
+#define DBCR1_IAC4US	0x00000C00	/* Instr Addr Cmp 4 Sup/User   */
+#define DBCR1_IAC4ER	0x00000300	/* Instr Addr Cmp 4 Eff/Real */
+#define DBCR1_IAC4ER_01	0x00000100	/* Instr Addr Cmp 4 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC4ER_10	0x00000200	/* Instr Addr Cmp 4 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC4ER_11	0x00000300	/* Instr Addr Cmp 4 Eff/Real MSR[IS]=1 */
+#define DBCR1_IAC34M	0x000000C0	/* Instr Addr 3-4 range enable */
+#define DBCR1_IAC34M_R	0x00000040	/* Instr Addr 3-4 reserved state */
+#define DBCR1_IAC34M_I	0x00000080	/* Instr Addr 3-4 range inclusive */
+#define DBCR1_IAC34M_X	0x000000C0	/* Instr Addr 3-4 range eXclusive */
+#define DBCR1_IAC34A_T	0x00000001	/* Instr Addr 3-4 range Toggle */
+
+#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
+#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 | \
+				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
+				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
+				 DBCR1_IAC4US | DBCR1_IAC4ER_10)
+
+#define dbcr_iac_range(task)	((task)->thread.dbcr1)
+#define DBCR_IAC12M	DBCR1_IAC12M
+#define DBCR_IAC12M_I	DBCR1_IAC12M_I
+#define DBCR_IAC12M_X	DBCR1_IAC12M_X
+#define DBCR_IAC34M	DBCR1_IAC34M
+#define DBCR_IAC34M_I	DBCR1_IAC34M_I
+#define DBCR_IAC34M_X	DBCR1_IAC34M_X
 
 /* Bit definitions related to the DBCR2. */
-#define DBCR2_DAC12M	0x00800000	/* DAC 1-2 range enable */
-#define DBCR2_DAC12MX	0x00C00000	/* DAC 1-2 range eXclusive */
+#define DBCR2_DAC1US	0xC0000000	/* Data Addr Cmp 1 Sup/User   */
+#define DBCR2_DAC1ER	0x30000000	/* Data Addr Cmp 1 Eff/Real */
+#define DBCR2_DAC2US	0x00000000	/* Data Addr Cmp 2 Sup/User   */
+#define DBCR2_DAC2ER	0x00000000	/* Data Addr Cmp 2 Eff/Real */
+#define DBCR2_DAC12MODE	0x00C00000	/* DAC 1-2 Mode Bits */
+#define DBCR2_DAC12MASK	0x00400000	/* DAC 1-2 Mask mode*/
+#define DBCR2_DAC12R	0x00800000	/* DAC 1-2 range enable */
+#define DBCR2_DAC12RX	0x00C00000	/* DAC 1-2 range eXclusive */
 #define DBCR2_DAC12A	0x00200000	/* DAC 1-2 Asynchronous */
-#endif
+#define DBCR2_DVC1M	0x000C0000	/* Data Value Comp 1 Mode */
+#define DBCR2_DVC1M_SHIFT	18	/* # of bits to shift DBCR2_DVC1M */
+#define DBCR2_DVC2M	0x00030000	/* Data Value Comp 2 Mode */
+#define DBCR2_DVC2M_SHIFT	16	/* # of bits to shift DBCR2_DVC2M */
+#define DBCR2_DVC1BE	0x00000F00	/* Data Value Comp 1 Byte */
+#define DBCR2_DVC1BE_SHIFT	8	/* # of bits to shift DBCR2_DVC1BE */
+#define DBCR2_DVC2BE	0x0000000F	/* Data Value Comp 2 Byte */
+#define DBCR2_DVC2BE_SHIFT	0	/* # of bits to shift DBCR2_DVC2BE */
+
+#define DBCR2_USER_DEBUG	(DBCR2_DAC12MODE | DBCR2_DVC1M | DBCR2_DVC2M | \
+				 DBCR2_DVC1BE | DBCR2_DVC2BE)
+#define DBCR2_BASE_REG_VALUE	(DBCR2_DAC1US | DBCR2_DAC2US)
+
+/*
+ * Are there any active Debug Events represented in the
+ * Debug Control Registers?
+ */
+#define DBCR0_ACTIVE_EVENTS  (DBCR0_ICMP | DBCR0_IAC1 | DBCR0_IAC2 | \
+			      DBCR0_IAC3 | DBCR0_IAC4 | DBCR0_DAC1R | \
+			      DBCR0_DAC1W  | DBCR0_DAC2R | DBCR0_DAC2W)
+#define DBCR1_ACTIVE_EVENTS	0
+
+#define DBCR_ACTIVE_EVENTS(dbcr0, dbcr1)  (((dbcr0) & DBCR0_ACTIVE_EVENTS) || \
+					   ((dbcr1) & DBCR1_ACTIVE_EVENTS))
+#endif /* #elif defined(CONFIG_BOOKE) */
 
 /* Bit definitions related to the TCR. */
 #define TCR_WP(x)	(((x)&0x3)<<30)	/* WDT Period */

-- 
Dave Kleikamp
IBM Linux Technology Center

[RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Dave Kleikamp]

powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

From: Torez Smith <lnxtorez@linux.vnet.ibm.com>

This patch defines context switch and trap related functionality
for BookE specific Debug Registers. It adds support to ptrace()
for setting and getting BookE related Debug Registers

Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
Cc: David Gibson <dwg@au1.ibm.com>
Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
---

 arch/powerpc/include/asm/system.h |    2 
 arch/powerpc/kernel/process.c     |  109 ++++++++-
 arch/powerpc/kernel/ptrace.c      |  435 ++++++++++++++++++++++++++++++++++---
 arch/powerpc/kernel/signal.c      |    6 -
 arch/powerpc/kernel/signal_32.c   |    8 +
 arch/powerpc/kernel/traps.c       |   86 ++++++-
 6 files changed, 564 insertions(+), 82 deletions(-)


diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
index bb8e006..474bf23 100644
--- a/arch/powerpc/include/asm/system.h
+++ b/arch/powerpc/include/asm/system.h
@@ -114,6 +114,8 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
 extern int set_dabr(unsigned long dabr);
 extern void do_dabr(struct pt_regs *regs, unsigned long address,
 		    unsigned long error_code);
+extern void do_send_trap(struct pt_regs *regs, unsigned long address,
+			 unsigned long error_code, int signal_code, int errno);
 extern void print_backtrace(unsigned long *);
 extern void show_regs(struct pt_regs * regs);
 extern void flush_instruction_cache(void);
diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index c930ac3..a0dbb09 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -245,6 +245,24 @@ void discard_lazy_cpu_state(void)
 }
 #endif /* CONFIG_SMP */
 
+void do_send_trap(struct pt_regs *regs, unsigned long address,
+		  unsigned long error_code, int signal_code, int errno)
+{
+	siginfo_t info;
+
+	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
+			11, SIGSEGV) == NOTIFY_STOP)
+		return;
+
+	/* Deliver the signal to userspace */
+	info.si_signo = SIGTRAP;
+	info.si_errno = errno;
+	info.si_code = signal_code;
+	info.si_addr = (void __user *)address;
+	force_sig_info(SIGTRAP, &info, current);
+}
+
+#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
 void do_dabr(struct pt_regs *regs, unsigned long address,
 		    unsigned long error_code)
 {
@@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
 	if (debugger_dabr_match(regs))
 		return;
 
-	/* Clear the DAC and struct entries.  One shot trigger */
-#if defined(CONFIG_BOOKE)
-	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
-							| DBCR0_IDM));
-#endif
-
 	/* Clear the DABR */
 	set_dabr(0);
 
@@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
 	info.si_addr = (void __user *)address;
 	force_sig_info(SIGTRAP, &info, current);
 }
+#endif
 
 static DEFINE_PER_CPU(unsigned long, current_dabr);
 
+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+/*
+ * Set the debug registers back to their default "safe" values.
+ */
+static void set_debug_reg_defaults(struct thread_struct *thread)
+{
+	thread->iac1 = thread->iac2 = thread->iac3 = thread->iac4 = 0;
+	thread->dac1 = thread->dac2 = 0;
+	thread->dvc1 = thread->dvc2 = 0;
+	/*
+	 * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
+	 * the exception of the reserved bits should be cleared out
+	 * and set to 0.
+	 *
+	 * For the DBCR0 register, the reserved bits are bits 17:30.
+	 * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
+	 * And, bits 10:11 for DBCR2.
+	 */
+	thread->dbcr0 = DBCR0_BASE_REG_VALUE;
+	/*
+	 * First clear all "non reserved" bits from DBCR1 then initialize reg
+	 * to force User/Supervisor bits to b11 (user-only MSR[PR]=1) and
+	 * Effective/Real * bits to b10 (trap only if IS==0)
+	 */
+	thread->dbcr1 = DBCR1_BASE_REG_VALUE;
+	/*
+	 * Force Data Address Compare User/Supervisor bits to be User-only
+	 * (0b11 MSR[PR]=1) and set all other bits in DBCR2 register to be 0.
+	 * This sets the Data Address Compare Effective/Real bits to be 0b00
+	 * (Effective, MSR[DS]=don't care).
+	 */
+	thread->dbcr2 = DBCR2_BASE_REG_VALUE;
+}
+
+static void prime_debug_regs(struct thread_struct *thread)
+{
+	mtspr(SPRN_IAC1, thread->iac1);
+	mtspr(SPRN_IAC2, thread->iac2);
+	mtspr(SPRN_IAC3, thread->iac3);
+	mtspr(SPRN_IAC4, thread->iac4);
+	mtspr(SPRN_DAC1, thread->dac1);
+	mtspr(SPRN_DAC2, thread->dac2);
+	mtspr(SPRN_DVC1, thread->dvc1);
+	mtspr(SPRN_DVC2, thread->dvc2);
+	mtspr(SPRN_DBCR0, thread->dbcr0);
+	mtspr(SPRN_DBCR1, thread->dbcr1);
+	mtspr(SPRN_DBCR2, thread->dbcr2);
+}
+/*
+ * Unless neither the old or new thread are making use of the
+ * debug registers, set the debug registers from the values
+ * stored in the new thread.
+ */
+static void switch_booke_debug_regs(struct thread_struct *new_thread)
+{
+	if ((current->thread.dbcr0 & DBCR0_IDM)
+		|| (new_thread->dbcr0 & DBCR0_IDM))
+			prime_debug_regs(new_thread);
+}
+#endif
+
 int set_dabr(unsigned long dabr)
 {
 	__get_cpu_var(current_dabr) = dabr;
@@ -284,7 +358,7 @@ int set_dabr(unsigned long dabr)
 		return ppc_md.set_dabr(dabr);
 
 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
-#if defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
 	mtspr(SPRN_DAC1, dabr);
 #elif defined(CONFIG_PPC_BOOK3S)
 	mtspr(SPRN_DABR, dabr);
@@ -371,10 +445,8 @@ struct task_struct *__switch_to(struct task_struct *prev,
 
 #endif /* CONFIG_SMP */
 
-#if defined(CONFIG_BOOKE)
-	/* If new thread DAC (HW breakpoint) is the same then leave it */
-	if (new->thread.dabr)
-		set_dabr(new->thread.dabr);
+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+	switch_booke_debug_regs(&new->thread);
 #else
 	if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
 		set_dabr(new->thread.dabr);
@@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
 	printk("  CR: %08lx  XER: %08lx\n", regs->ccr, regs->xer);
 	trap = TRAP(regs);
 	if (trap == 0x300 || trap == 0x600)
-#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
 #else
 		printk("DAR: "REG", DSISR: "REG"\n", regs->dar, regs->dsisr);
@@ -568,14 +640,19 @@ void flush_thread(void)
 
 	discard_lazy_cpu_state();
 
+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
+	/*
+	 * flush_thread() is called on exec() to reset the
+	 * thread's status. Set all debug regs back to their
+	 * default values....Torez
+	 */
+	set_debug_reg_defaults(&current->thread);
+#else
 	if (current->thread.dabr) {
 		current->thread.dabr = 0;
 		set_dabr(0);
-
-#if defined(CONFIG_BOOKE)
-		current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W);
-#endif
 	}
+#endif
 }
 
 void
diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c
index 6be2ce0..6710a69 100644
--- a/arch/powerpc/kernel/ptrace.c
+++ b/arch/powerpc/kernel/ptrace.c
@@ -737,17 +737,25 @@ void user_disable_single_step(struct task_struct *task)
 	struct pt_regs *regs = task->thread.regs;
 
 	if (regs != NULL) {
-#if defined(CONFIG_BOOKE)
-		/* If DAC don't clear DBCRO_IDM or MSR_DE */
-		if (task->thread.dabr)
-			task->thread.dbcr0 &= ~(DBCR0_IC | DBCR0_BT);
-		else {
-			task->thread.dbcr0 &= ~(DBCR0_IC | DBCR0_BT | DBCR0_IDM);
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+		/*
+		 * The logic to disable single stepping should be as
+		 * simple as turning off the Instruction Complete flag.
+		 * And, after doing so, if all debug flags are off, turn
+		 * off DBCR0(IDM) and MSR(DE) .... Torez
+		 */
+		task->thread.dbcr0 &= ~DBCR0_IC;
+		/*
+		 * Test to see if any of the DBCR_ACTIVE_EVENTS bits are set.
+		 */
+		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
+					task->thread.dbcr1)) {
+			/*
+			 * All debug events were off.....
+			 */
+			task->thread.dbcr0 &= ~DBCR0_IDM;
 			regs->msr &= ~MSR_DE;
 		}
-#elif defined(CONFIG_40x)
-		task->thread.dbcr0 &= ~(DBCR0_IC | DBCR0_BT | DBCR0_IDM);
-		regs->msr &= ~MSR_DE;
 #else
 		regs->msr &= ~(MSR_SE | MSR_BE);
 #endif
@@ -769,8 +777,7 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 	if ((data & ~0x7UL) >= TASK_SIZE)
 		return -EIO;
 
-#ifndef CONFIG_BOOKE
-
+#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
 	/* For processors using DABR (i.e. 970), the bottom 3 bits are flags.
 	 *  It was assumed, on previous implementations, that 3 bits were
 	 *  passed together with the data address, fitting the design of the
@@ -789,21 +796,22 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 
 	/* Move contents to the DABR register */
 	task->thread.dabr = data;
-
-#endif
-#if defined(CONFIG_BOOKE)
-
+#else
 	/* As described above, it was assumed 3 bits were passed with the data
 	 *  address, but we will assume only the mode bits will be passed
 	 *  as to not cause alignment restrictions for DAC-based processors.
 	 */
 
 	/* DAC's hold the whole address without any mode flags */
-	task->thread.dabr = data & ~0x3UL;
-
-	if (task->thread.dabr == 0) {
-		task->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
-		task->thread.regs->msr &= ~MSR_DE;
+	task->thread.dac1 = data & ~0x3UL;
+
+	if (task->thread.dac1 == 0) {
+		dbcr_dac(task) &= ~(DBCR_DAC1R | DBCR_DAC1W);
+		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
+					task->thread.dbcr1)) {
+			task->thread.regs->msr &= ~MSR_DE;
+			task->thread.dbcr0 &= ~DBCR0_IDM;
+		}
 		return 0;
 	}
 
@@ -814,15 +822,15 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
 
 	/* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
 	   register */
-	task->thread.dbcr0 = DBCR0_IDM;
+	task->thread.dbcr0 |= DBCR0_IDM;
 
 	/* Check for write and read flags and set DBCR0
 	   accordingly */
+	dbcr_dac(task) &= ~(DBCR_DAC1R|DBCR_DAC1W);
 	if (data & 0x1UL)
-		task->thread.dbcr0 |= DBSR_DAC1R;
+		dbcr_dac(task) |= DBCR_DAC1R;
 	if (data & 0x2UL)
-		task->thread.dbcr0 |= DBSR_DAC1W;
-
+		dbcr_dac(task) |= DBCR_DAC1W;
 	task->thread.regs->msr |= MSR_DE;
 #endif
 	return 0;
@@ -839,11 +847,324 @@ void ptrace_disable(struct task_struct *child)
 	user_disable_single_step(child);
 }
 
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+static long set_intruction_bp(struct task_struct *child,
+			      struct ppc_hw_breakpoint *bp_info)
+{
+	int slots_needed;
+	int slot;
+	int free_slot = 0;
+
+	/*
+	 * Find an avalailable slot for the breakpoint.
+	 * If possible, reserve consecutive slots, 1 & 2, for a range
+	 * breakpoint.  (Can this be done simpler?)
+	 */
+	if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT)
+		slots_needed = 1;
+	else
+		slots_needed = 2;
+
+	if ((child->thread.dbcr0 & DBCR0_IAC1) == 0) {
+		if (slots_needed == 1) {
+			if (child->thread.dbcr0 & DBCR0_IAC2) {
+				slot = 1;
+				goto found;
+			}
+			/* Try to save slots 1 & 2 for range */
+			free_slot = 1;
+		} else
+			if ((child->thread.dbcr0 & DBCR0_IAC2) == 0) {
+				slot = 1;
+				goto found;
+			}
+	} else if ((slots_needed == 1) &&
+		   ((child->thread.dbcr0 & DBCR0_IAC2) == 0)) {
+		slot = 2;
+		goto found;
+	}
+	if ((child->thread.dbcr0 & DBCR0_IAC3) == 0) {
+		if (slots_needed == 1) {
+			slot = 3;
+			goto found;
+		}
+		if ((child->thread.dbcr0 & DBCR0_IAC4) == 0) {
+			slot = 3;
+			goto found;
+		}
+		return -ENOSPC;
+	} else if (slots_needed == 2)
+		return -ENOSPC;
+	if ((child->thread.dbcr0 & DBCR0_IAC4) == 0) {
+		slot = 4;
+	} else if (free_slot)
+		slot = free_slot;
+	else
+		return -ENOSPC;
+found:
+	if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT) {
+		switch (slot) {
+			case 1:
+				child->thread.iac1 = bp_info->addr;
+				child->thread.dbcr0 |= DBCR0_IAC1;
+				break;
+			case 2:
+				child->thread.iac2 = bp_info->addr;
+				child->thread.dbcr0 |= DBCR0_IAC2;
+				break;
+			case 3:
+				child->thread.iac3 = bp_info->addr;
+				child->thread.dbcr0 |= DBCR0_IAC3;
+				break;
+			case 4:
+				child->thread.iac4 = bp_info->addr;
+				child->thread.dbcr0 |= DBCR0_IAC4;
+				break;
+		}
+	} else if (slot == 1) {
+		child->thread.iac1 = bp_info->addr;
+		child->thread.iac2 = bp_info->addr2;
+		child->thread.dbcr0 |= (DBCR0_IAC1 | DBCR0_IAC2);
+		if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
+			dbcr_iac_range(child) |= DBCR_IAC12M_X;
+		else
+			dbcr_iac_range(child) |= DBCR_IAC12M_I;
+	} else { /* slot == 3 */
+		child->thread.iac3 = bp_info->addr;
+		child->thread.iac4 = bp_info->addr2;
+		child->thread.dbcr0 |= (DBCR0_IAC3 | DBCR0_IAC4);
+		if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
+			dbcr_iac_range(child) |= DBCR_IAC34M_X;
+		else
+			dbcr_iac_range(child) |= DBCR_IAC34M_I;
+	}
+	child->thread.dbcr0 |= DBCR0_IDM;
+	child->thread.regs->msr |= MSR_DE;
+
+	return slot;
+}
+
+static int del_instruction_bp(struct task_struct *child, int slot)
+{
+	switch (slot) {
+	case 1:
+		if (dbcr_iac_range(child) & DBCR_IAC12M) {
+			/* address range - clear slots 1 & 2 */
+			child->thread.iac2 = 0;
+			child->thread.dbcr0 &= ~DBCR0_IAC2;
+			dbcr_iac_range(child) &= ~DBCR_IAC12M;
+		}
+		child->thread.iac1 = 0;
+		child->thread.dbcr0 &= ~DBCR0_IAC1;
+		break;
+	case 2:
+		if (dbcr_iac_range(child) & DBCR_IAC12M)
+			/* used in a range */
+			return -EINVAL;
+		child->thread.iac2 = 0;
+		child->thread.dbcr0 &= ~DBCR0_IAC2;
+		break;
+	case 3:
+		if (dbcr_iac_range(child) & DBCR_IAC34M) {
+			/* address range - clear slots 3 & 4 */
+			child->thread.iac4 = 0;
+			child->thread.dbcr0 &= ~DBCR0_IAC4;
+			dbcr_iac_range(child) &= ~DBCR_IAC34M;
+		}
+		child->thread.iac3 = 0;
+		child->thread.dbcr0 &= ~DBCR0_IAC3;
+		break;
+	case 4:
+		if (dbcr_iac_range(child) & DBCR_IAC34M)
+			/* Used in a range */
+			return -EINVAL;
+		child->thread.iac4 = 0;
+		child->thread.dbcr0 &= ~DBCR0_IAC4;
+		break;
+	default:
+		return -EINVAL;
+	}
+	return 0;
+}
+
+static int set_dac(struct task_struct *child, struct ppc_hw_breakpoint *bp_info)
+{
+	int byte_enable =
+		(bp_info->condition_mode >> PPC_BREAKPOINT_CONDITION_BE_SHIFT)
+		& 0xf;
+	int condition_mode =
+		bp_info->condition_mode & PPC_BREAKPOINT_CONDITION_AND_OR;
+	int slot;
+
+	if (byte_enable && (condition_mode == 0))
+		return -EINVAL;
+
+	if (bp_info->addr >= TASK_SIZE)
+		return -EIO;
+
+	if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) == 0) {
+		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
+			dbcr_dac(child) |= DBCR_DAC1R;
+		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
+			dbcr_dac(child) |= DBCR_DAC1W;
+		child->thread.dac1 = (unsigned long)bp_info->addr;
+#ifdef CONFIG_BOOKE
+		if (byte_enable) {
+			child->thread.dvc1 =
+				(unsigned long)bp_info->condition_value;
+			child->thread.dbcr2 |=
+				((byte_enable << DBCR2_DVC1BE_SHIFT) |
+				 (condition_mode << DBCR2_DVC1M_SHIFT));
+		}
+#endif
+		slot = 1;
+	} else if ((dbcr_dac(child) & (DBCR_DAC2R | DBCR_DAC2W)) == 0) {
+		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
+			dbcr_dac(child) |= DBCR_DAC2R;
+		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
+			dbcr_dac(child) |= DBCR_DAC2W;
+		child->thread.dac2 = (unsigned long)bp_info->addr;
+#ifdef CONFIG_BOOKE
+		if (byte_enable) {
+			child->thread.dvc2 =
+				(unsigned long)bp_info->condition_value;
+			child->thread.dbcr2 |=
+				((byte_enable << DBCR2_DVC2BE_SHIFT) |
+				 (condition_mode << DBCR2_DVC2M_SHIFT));
+		}
+#endif
+		slot = 2;
+	} else
+		return -ENOSPC;
+	child->thread.dbcr0 |= DBCR0_IDM;
+	child->thread.regs->msr |= MSR_DE;
+
+	return slot + 4;
+}
+
+static int del_dac(struct task_struct *child, int slot)
+{
+	if (slot == 1) {
+#ifdef CONFIG_BOOKE
+		if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
+			child->thread.dac1 = 0;
+			child->thread.dac2 = 0;
+			child->thread.dbcr0 &= ~(DBCR0_DAC1R | DBCR0_DAC1W |
+						 DBCR0_DAC2R | DBCR0_DAC2W);
+			child->thread.dbcr2 &= ~DBCR2_DAC12MODE;
+			return 0;
+		}
+		child->thread.dbcr2 &= ~(DBCR2_DVC1M | DBCR2_DVC1BE);
+		child->thread.dvc1 = 0;
+#endif
+		child->thread.dac1 = 0;
+		dbcr_dac(child) &= ~(DBCR_DAC1R | DBCR_DAC1W);
+	} else if (slot == 2) {
+#ifdef CONFIG_BOOKE
+		if (child->thread.dbcr2 & DBCR2_DAC12MODE)
+			/* Part of a range */
+			return -EINVAL;
+		child->thread.dbcr2 &= ~(DBCR2_DVC2M | DBCR2_DVC2BE);
+		child->thread.dvc2 = 0;
+#endif
+		child->thread.dac2 = 0;
+		dbcr_dac(child) &= ~(DBCR_DAC2R | DBCR_DAC2W);
+	} else
+		return -EINVAL;
+
+	return 0;
+}
+#endif /* CONFIG_40x || CONFIG_BOOKE */
+
+#ifdef CONFIG_BOOKE
+static int set_dac_range(struct task_struct *child,
+			 struct ppc_hw_breakpoint *bp_info)
+{
+	int mode = bp_info->addr_mode & PPC_BREAKPOINT_MODE_MASK;
+
+	/* We don't allow range watchpoints to be used with DVC */
+	if (bp_info->condition_mode && PPC_BREAKPOINT_CONDITION_BE_ALL)
+		return -EINVAL;
+
+	if (bp_info->addr >= TASK_SIZE)
+		return -EIO;
+
+	if (child->thread.dbcr0 &
+	    (DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W))
+		return -ENOSPC;
+
+	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
+		child->thread.dbcr0 |= (DBCR0_DAC1R | DBCR0_DAC2R | DBCR0_IDM);
+	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
+		child->thread.dbcr0 |= (DBCR0_DAC1W | DBCR0_DAC2W | DBCR0_IDM);
+	child->thread.dac1 = bp_info->addr;
+	child->thread.dac2 = bp_info->addr2;
+	if (mode == PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE)
+		child->thread.dbcr2  |= DBCR2_DAC12R;
+	else if (mode == PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
+		child->thread.dbcr2  |= DBCR2_DAC12RX;
+	else	/* PPC_BREAKPOINT_MODE_MASK */
+		child->thread.dbcr2  |= DBCR2_DAC12MASK;
+	child->thread.regs->msr |= MSR_DE;
+
+	return 5;
+}
+#endif /* CONFIG_BOOKE */
+
 static long ppc_set_hwdebug(struct task_struct *child,
 		     struct ppc_hw_breakpoint *bp_info)
 {
+	if (bp_info->version != 1)
+		return -ENOTSUPP;
+
+#ifdef CONFIG_BOOKE
+	/*
+	 * Check for invalid flags and combinations
+	 */
+	if ((bp_info->trigger_type == 0) ||
+	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
+				       PPC_BREAKPOINT_TRIGGER_RW)) ||
+	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
+	    (bp_info->condition_mode &
+	     ~(PPC_BREAKPOINT_CONDITION_AND_OR |
+	       PPC_BREAKPOINT_CONDITION_BE_ALL)))
+		return -EINVAL;
+
+	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
+		if (bp_info->trigger_type != PPC_BREAKPOINT_TRIGGER_EXECUTE)
+			/* At least another bit was set */
+			return -EINVAL;
+		return set_intruction_bp(child, bp_info);
+	}
+
+	if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT)
+		return set_dac(child, bp_info);
+
+	return set_dac_range(child, bp_info);
+#elif defined(CONFIG_40x)
+	/*
+	 * Check for invalid flags and combinations
+	 */
+	if ((bp_info->trigger_type == 0) ||
+	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
+				       PPC_BREAKPOINT_TRIGGER_RW)) ||
+	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
+	    (bp_info->condition_mode != PPC_BREAKPOINT_CONDITION_NONE))
+		return -EINVAL;
+
+	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
+		if (bp_info->trigger_type != PPC_BREAKPOINT_TRIGGER_EXECUTE)
+			/* At least another bit was set */
+			return -EINVAL;
+		return set_intruction_bp(child, bp_info);
+	}
+	if (bp_info->addr_mode != PPC_BREAKPOINT_MODE_EXACT)
+		return -EINVAL;
+
+	return set_dac(child, bp_info);
+#else
 	/*
-	 * We currently support one data breakpoint
+	 * We only support one data breakpoint
 	 */
 	if (((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) == 0) ||
 	    ((bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) != 0) ||
@@ -859,28 +1180,35 @@ static long ppc_set_hwdebug(struct task_struct *child,
 		return -EIO;
 
 	child->thread.dabr = (unsigned long)bp_info->addr;
-#ifdef CONFIG_BOOKE
-	child->thread.dbcr0 = DBCR0_IDM;
-	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
-		child->thread.dbcr0 |= DBSR_DAC1R;
-	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
-		child->thread.dbcr0 |= DBSR_DAC1W;
-	child->thread.regs->msr |= MSR_DE;
-#endif
 	return 1;
+#endif
 }
 
 static long ppc_del_hwdebug(struct task_struct *child, long addr, long data)
 {
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+	int rc;
+
+	if (data <= 4)
+		rc = del_instruction_bp(child, (int)data);
+	else
+		rc = del_dac(child, (int)data - 4);
+
+	if (!rc) {
+		if (!DBCR_ACTIVE_EVENTS(child->thread.dbcr0,
+					child->thread.dbcr1)) {
+			child->thread.dbcr0 &= ~DBCR0_IDM;
+			child->thread.regs->msr &= ~MSR_DE;
+		}
+	}
+	return rc;
+#else
 	if ((data != 1) || (child->thread.dabr == 0))
 		return -EINVAL;
 
 	child->thread.dabr = 0;
-#ifdef CONFIG_BOOKE
-	child->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
-	child->thread.regs->msr &= ~MSR_DE;
-#endif
 	return 0;
+#endif
 }
 
 /*
@@ -980,16 +1308,36 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 		struct ppc_debug_info dbginfo;
 
 		dbginfo.version = 1;
+#ifdef CONFIG_BOOKE
+		dbginfo.num_instruction_bps = 4;
+		dbginfo.num_data_bps = 2;
+		dbginfo.num_condition_regs = 2;
+		dbginfo.data_bp_alignment = 0;
+		dbginfo.sizeof_condition = 4;
+		dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
+				   PPC_DEBUG_FEATURE_INSN_BP_MASK |
+				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
+				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
+#elif defined(CONFIG_40x)
+		/*
+		 * I don't know how the DVCs work on 40x, I'm not going
+		 * to support it now. -- Shaggy
+		 */
+		dbginfo.num_instruction_bps = 4;
+		dbginfo.num_data_bps = 2;
+		dbginfo.num_condition_regs = 0;
+		dbginfo.data_bp_alignment = 0;
+		dbginfo.sizeof_condition = 0;
+		dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
+				   PPC_DEBUG_FEATURE_INSN_BP_MASK;
+#else
 		dbginfo.num_instruction_bps = 0;
 		dbginfo.num_data_bps = 1;
 		dbginfo.num_condition_regs = 0;
-#ifdef CONFIG_PPC64
 		dbginfo.data_bp_alignment = 8;
-#else
-		dbginfo.data_bp_alignment = 0;
-#endif
 		dbginfo.sizeof_condition = 0;
 		dbginfo.features = 0;
+#endif
 
 		if (!access_ok(VERIFY_WRITE, data,
 			       sizeof(struct ppc_debug_info)))
@@ -1025,8 +1373,13 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
 		/* We only support one DABR and no IABRS at the moment */
 		if (addr > 0)
 			break;
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+		ret = put_user(child->thread.dac1,
+			       (unsigned long __user *)data);
+#else
 		ret = put_user(child->thread.dabr,
 			       (unsigned long __user *)data);
+#endif
 		break;
 	}
 
diff --git a/arch/powerpc/kernel/signal.c b/arch/powerpc/kernel/signal.c
index 00b5078..94df779 100644
--- a/arch/powerpc/kernel/signal.c
+++ b/arch/powerpc/kernel/signal.c
@@ -140,17 +140,15 @@ static int do_signal_pending(sigset_t *oldset, struct pt_regs *regs)
 		return 0;               /* no signals delivered */
 	}
 
+#if !(defined(CONFIG_BOOKE) || defined(CONFIG_40x))
         /*
 	 * Reenable the DABR before delivering the signal to
 	 * user space. The DABR will have been cleared if it
 	 * triggered inside the kernel.
 	 */
-	if (current->thread.dabr) {
+	if (current->thread.dabr)
 		set_dabr(current->thread.dabr);
-#if defined(CONFIG_BOOKE)
-		mtspr(SPRN_DBCR0, current->thread.dbcr0);
 #endif
-	}
 
 	if (is32) {
         	if (ka.sa.sa_flags & SA_SIGINFO)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index d670429..6cc6e81 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -1092,8 +1092,12 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
 				new_msr |= MSR_DE;
 				new_dbcr0 |= (DBCR0_IDM | DBCR0_IC);
 			} else {
-				new_msr &= ~MSR_DE;
-				new_dbcr0 &= ~(DBCR0_IDM | DBCR0_IC);
+				new_dbcr0 &= ~DBCR0_IC;
+				if (!DBCR_ACTIVE_EVENTS(new_dbcr0,
+						current->thread.dbcr1)) {
+					new_msr &= ~MSR_DE;
+					new_dbcr0 &= ~DBCR0_IDM;
+				}
 			}
 #else
 			if (op.dbg_value)
diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
index a81c743..d919571 100644
--- a/arch/powerpc/kernel/traps.c
+++ b/arch/powerpc/kernel/traps.c
@@ -1016,9 +1016,63 @@ void SoftwareEmulation(struct pt_regs *regs)
 #endif /* CONFIG_8xx */
 
 #if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+static void handle_debug(struct pt_regs *regs, unsigned long debug_status)
+{
+	int changed = 0;
+	/*
+	 * Determine the cause of the debug event, clear the
+	 * event flags and send a trap to the handler. Torez
+	 */
+	if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
+		dbcr_dac(current) &= ~(DBCR_DAC1R | DBCR_DAC1W);
+		do_send_trap(regs, mfspr(SPRN_DAC1), debug_status, TRAP_HWBKPT,
+			     5);
+		changed |= 0x01;
+	}  else if (debug_status & (DBSR_DAC2R | DBSR_DAC2W)) {
+		dbcr_dac(current) &= ~(DBCR_DAC2R | DBCR_DAC2W);
+		do_send_trap(regs, mfspr(SPRN_DAC2), debug_status, TRAP_HWBKPT,
+			     6);
+		changed |= 0x01;
+	}  else if (debug_status & DBSR_IAC1) {
+		current->thread.dbcr0 &= ~DBCR0_IAC1;
+		do_send_trap(regs, mfspr(SPRN_IAC1), debug_status, TRAP_HWBKPT,
+			     1);
+		changed |= 0x01;
+	}  else if (debug_status & DBSR_IAC2) {
+		current->thread.dbcr0 &= ~DBCR0_IAC2;
+		do_send_trap(regs, mfspr(SPRN_IAC2), debug_status, TRAP_HWBKPT,
+			     2);
+		changed |= 0x01;
+	}  else if (debug_status & DBSR_IAC3) {
+		current->thread.dbcr0 &= ~DBCR0_IAC3;
+		do_send_trap(regs, mfspr(SPRN_IAC3), debug_status, TRAP_HWBKPT,
+			     3);
+		changed |= 0x01;
+	}  else if (debug_status & DBSR_IAC4) {
+		current->thread.dbcr0 &= ~DBCR0_IAC4;
+		do_send_trap(regs, mfspr(SPRN_IAC4), debug_status, TRAP_HWBKPT,
+			     4);
+		changed |= 0x01;
+	}
+	/*
+	 * At the point this routine was called, the MSR(DE) was turned off.
+	 * Check all other debug flags and see if that bit needs to be turned
+	 * back on or not.
+	 */
+	if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0, current->thread.dbcr1))
+		regs->msr |= MSR_DE;
+	else
+		/* Make sure the IDM flag is off */
+		current->thread.dbcr0 &= ~DBCR0_IDM;
+
+	if (changed & 0x01)
+		mtspr(SPRN_DBCR0, current->thread.dbcr0);
+}
 
 void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status)
 {
+	current->thread.dbsr = debug_status;
+
 	/* Hack alert: On BookE, Branch Taken stops on the branch itself, while
 	 * on server, it stops on the target of the branch. In order to simulate
 	 * the server behaviour, we thus restart right away with a single step
@@ -1062,27 +1116,21 @@ void __kprobes DebugException(struct pt_regs *regs, unsigned long debug_status)
 		if (debugger_sstep(regs))
 			return;
 
-		if (user_mode(regs))
-			current->thread.dbcr0 &= ~(DBCR0_IC);
-
-		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
-	} else if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
-		regs->msr &= ~MSR_DE;
-
 		if (user_mode(regs)) {
-			current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W |
-								DBCR0_IDM);
-		} else {
-			/* Disable DAC interupts */
-			mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R |
-						DBSR_DAC1W | DBCR0_IDM));
-
-			/* Clear the DAC event */
-			mtspr(SPRN_DBSR, (DBSR_DAC1R | DBSR_DAC1W));
+			current->thread.dbcr0 &= ~DBCR0_IC;
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+			if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0,
+					       current->thread.dbcr1))
+				regs->msr |= MSR_DE;
+			else
+				/* Make sure the IDM bit is off */
+				current->thread.dbcr0 &= ~DBCR0_IDM;
+#endif
 		}
-		/* Setup and send the trap to the handler */
-		do_dabr(regs, mfspr(SPRN_DAC1), debug_status);
-	}
+
+		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
+	} else
+		handle_debug(regs, debug_status);
 }
 #endif /* CONFIG_4xx || CONFIG_BOOKE */
 

-- 
Dave Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Josh Boyer]

On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
>diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
>index 9eed29e..1393307 100644
>--- a/arch/powerpc/include/asm/processor.h
>+++ b/arch/powerpc/include/asm/processor.h
>@@ -161,9 +161,35 @@ struct thread_struct {
> #ifdef CONFIG_PPC32
> 	void		*pgdir;		/* root of page-table tree */
> #endif
>-#if defined(CONFIG_4xx) || defined (CONFIG_BOOKE)
>-	unsigned long	dbcr0;		/* debug control register values */
>+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
>+	/*
>+	 * The following help to manage the use of Debug Control Registers
>+	 * om the BookE platforms.
>+	 */
>+	unsigned long	dbcr0;
> 	unsigned long	dbcr1;
>+	unsigned long	dbcr2;

This is wrong for 405.  405 only has dbcr0 and dbcr1.  I don't see why you'd
change the #define values to be more explicit and then include things that
don't make sense.

>+	/*
>+	 * The stored value of the DBSR register will be the value at the
>+	 * last debug interrupt. This register can only be read from the
>+	 * user (will never be written to) and has value while helping to
>+	 * describe the reason for the last debug trap.  Torez
>+	 */
>+	unsigned long	dbsr;
>+	/*
>+	 * The following will contain addresses used by debug applications
>+	 * to help trace and trap on particular address locations.
>+	 * The bits in the Debug Control Registers above help define which
>+	 * of the following registers will contain valid data and/or addresses.
>+	 */
>+	unsigned long	iac1;
>+	unsigned long	iac2;
>+	unsigned long	iac3;
>+	unsigned long	iac4;
>+	unsigned long	dac1;
>+	unsigned long	dac2;
>+	unsigned long	dvc1;
>+	unsigned long	dvc2;
> #endif

Without digging much, I'm wondering if we could just use a pointer to a debug
register structure here instead of growing struct thread more.

> 	/* FP and VSX 0-31 register set */
> 	double		fpr[32][TS_FPRWIDTH];
>diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
>index 3bf7835..7f8c71f 100644
>--- a/arch/powerpc/include/asm/reg_booke.h
>+++ b/arch/powerpc/include/asm/reg_booke.h
>@@ -248,6 +248,8 @@
> #define DBSR_RET	0x00008000	/* Return Debug Event */
> #define DBSR_CIRPT	0x00000040	/* Critical Interrupt Taken Event */
> #define DBSR_CRET	0x00000020	/* Critical Return Debug Event */
>+#define DBSR_IAC12ATS	0x00000002	/* Instr Address Compare 1/2 Toggle */
>+#define DBSR_IAC34ATS	0x00000001	/* Instr Address Compare 3/4 Toggle */
> #endif
> #ifdef CONFIG_40x
> #define DBSR_IC		0x80000000	/* Instruction Completion */
>@@ -294,25 +296,68 @@
> #define DBCR0_IC	0x08000000	/* Instruction Completion */
> #define DBCR0_ICMP	DBCR0_IC
> #define DBCR0_BT	0x04000000	/* Branch Taken */
>-#define DBCR0_BRT	DBCR0_BT
> #define DBCR0_EDE	0x02000000	/* Exception Debug Event */
> #define DBCR0_IRPT	DBCR0_EDE
> #define DBCR0_TDE	0x01000000	/* TRAP Debug Event */
>-#define DBCR0_IA1	0x00800000	/* Instr Addr compare 1 enable */
>-#define DBCR0_IAC1	DBCR0_IA1
>-#define DBCR0_IA2	0x00400000	/* Instr Addr compare 2 enable */
>-#define DBCR0_IAC2	DBCR0_IA2
>-#define DBCR0_IA12	0x00200000	/* Instr Addr 1-2 range enable */
>-#define DBCR0_IA12X	0x00100000	/* Instr Addr 1-2 range eXclusive */
>-#define DBCR0_IA3	0x00080000	/* Instr Addr compare 3 enable */
>-#define DBCR0_IAC3	DBCR0_IA3
>-#define DBCR0_IA4	0x00040000	/* Instr Addr compare 4 enable */
>-#define DBCR0_IAC4	DBCR0_IA4
>-#define DBCR0_IA34	0x00020000	/* Instr Addr 3-4 range Enable */
>-#define DBCR0_IA34X	0x00010000	/* Instr Addr 3-4 range eXclusive */
>-#define DBCR0_IA12T	0x00008000	/* Instr Addr 1-2 range Toggle */
>-#define DBCR0_IA34T	0x00004000	/* Instr Addr 3-4 range Toggle */
>+#define DBCR0_IAC1	0x00800000	/* Instr Addr compare 1 enable */
>+#define DBCR0_IAC2	0x00400000	/* Instr Addr compare 2 enable */
>+#define DBCR0_IAC12M	0x00300000	/* Instr Addr 1-2 range enable */
>+#define DBCR0_IAC12M_R	0x00100000	/* Instr Addr 1-2 Reserved state */
>+#define DBCR0_IAC12M_I	0x00200000	/* Instr Addr 1-2 range Inclusive */
>+#define DBCR0_IAC12M_X	0x00300000	/* Instr Addr 1-2 range eXclusive */
>+#define DBCR0_IAC3	0x00080000	/* Instr Addr compare 3 enable */
>+#define DBCR0_IAC4	0x00040000	/* Instr Addr compare 4 enable */
>+#define DBCR0_IAC34	0x00020000	/* Instr Addr 3-4 range Enable */
>+#define DBCR0_IAC34X	0x00010000	/* Instr Addr 3-4 range eXclusive */
>+#define DBCR0_IAC12T	0x00008000	/* Instr Addr 1-2 range Toggle */
>+#define DBCR0_IAC34T	0x00004000	/* Instr Addr 3-4 range Toggle */

A lot of this seems to just be cleanup... would it be possible to factor out
the cleanup parts so that the additions are easier to review?

> #define DBCR0_FT	0x00000001	/* Freeze Timers on debug event */
>+
>+#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
>+				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
>+#define DBCR0_BASE_REG_VALUE	0
>+
>+#define dbcr_iac_range(task)	((task)->thread.dbcr0)
>+#define DBCR_IAC12M	DBCR0_IAC12M
>+#define DBCR_IAC12M_I	DBCR0_IAC12M_I
>+#define DBCR_IAC12M_X	DBCR0_IAC12M_X
>+#define DBCR_IAC34M	DBCR0_IAC34M
>+#define DBCR_IAC34M_I	DBCR0_IAC34M_I
>+#define DBCR_IAC34M_X	DBCR0_IAC34M_X
>+
>+/* Bit definitions related to the DBCR1. */
>+#define DBCR1_D1R	0x80000000	/* DAC1 Read Debug Event */
>+#define DBCR1_DAC1R	DBCR1_D1R
>+#define DBCR1_D2R	0x40000000	/* DAC2 Read Debug Event */
>+#define DBCR1_DAC2R	DBCR1_D2R
>+#define DBCR1_D1W	0x20000000	/* DAC1 Write Debug Event */
>+#define DBCR1_DAC1W	DBCR1_D1W
>+#define DBCR1_D2W	0x10000000	/* DAC2 Write Debug Event */
>+#define DBCR1_DAC2W	DBCR1_D2W
>+
>+#define DBCR1_USER_DEBUG	(DBCR1_DAC1R | DBCR1_DAC2R | DBCR1_DAC1W | \
>+				 DBCR1_DAC2W)
>+#define DBCR1_BASE_REG_VALUE	0
>+
>+#define dbcr_dac(task)	((task)->thread.dbcr1)
>+#define DBCR_DAC1R	DBCR1_DAC1R
>+#define DBCR_DAC1W	DBCR1_DAC1W
>+#define DBCR_DAC2R	DBCR1_DAC2R
>+#define DBCR_DAC2W	DBCR1_DAC2W
>+
>+#define DBCR2_USER_DEBUG	0
>+#define DBCR2_BASE_REG_VALUE	0

Why are these defined for 405?

>+/*
>+ * Are there any active Debug Events represented in the
>+ * Debug Control Registers?
>+ */
>+#define DBCR0_ACTIVE_EVENTS	(DBCR0_ICMP | DBCR0_IAC1 | DBCR0_IAC2 | \
>+				 DBCR0_IAC3 | DBCR0_IAC4)
>+#define DBCR1_ACTIVE_EVENTS	(DBCR1_D1R | DBCR1_D2R | DBCR1_D1W | DBCR1_D2W)
>+#define DBCR_ACTIVE_EVENTS(dbcr0, dbcr1)  (((dbcr0) & DBCR0_ACTIVE_EVENTS) || \
>+					   ((dbcr1) & DBCR1_ACTIVE_EVENTS))
>+
> #elif defined(CONFIG_BOOKE)
> #define DBCR0_EDM	0x80000000	/* External Debug Mode */
> #define DBCR0_IDM	0x40000000	/* Internal Debug Mode */
>@@ -324,8 +369,7 @@
> #define DBCR0_RST_NONE	0x00000000	/* No Reset */
> #define DBCR0_ICMP	0x08000000	/* Instruction Completion */
> #define DBCR0_IC	DBCR0_ICMP
>-#define DBCR0_BRT	0x04000000	/* Branch Taken */
>-#define DBCR0_BT	DBCR0_BRT
>+#define DBCR0_BT	0x04000000	/* Branch Taken */

This seems like just cleanup of DBCR0_BRT?

> #define DBCR0_IRPT	0x02000000	/* Exception Debug Event */
> #define DBCR0_TDE	0x01000000	/* TRAP Debug Event */
> #define DBCR0_TIE	DBCR0_TDE
>@@ -342,19 +386,99 @@
> #define DBCR0_CRET	0x00000020	/* Critical Return Debug Event */
> #define DBCR0_FT	0x00000001	/* Freeze Timers on debug event */
>
>+#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
>+				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4 | \
>+				 DBCR0_DAC1R | DBCR0_DAC1W  | DBCR0_DAC2R | \
>+				 DBCR0_DAC2W)
>+#define DBCR0_BASE_REG_VALUE	0
>+
>+#define dbcr_dac(task)	((task)->thread.dbcr0)
>+#define DBCR_DAC1R	DBCR0_DAC1R
>+#define DBCR_DAC1W	DBCR0_DAC1W
>+#define DBCR_DAC2R	DBCR0_DAC2R
>+#define DBCR_DAC2W	DBCR0_DAC2W
>+
> /* Bit definitions related to the DBCR1. */

I'll try to review these a bit later.  Changing #defines makes for hard patch
review :)

josh

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Josh Boyer]

On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
>powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
>
>From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>
>This patch defines context switch and trap related functionality
>for BookE specific Debug Registers. It adds support to ptrace()
>for setting and getting BookE related Debug Registers
>
>Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
>Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
>Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
>Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
>Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
>Cc: David Gibson <dwg@au1.ibm.com>
>Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
>---
>
> arch/powerpc/include/asm/system.h |    2 
> arch/powerpc/kernel/process.c     |  109 ++++++++-
> arch/powerpc/kernel/ptrace.c      |  435 ++++++++++++++++++++++++++++++++++---
> arch/powerpc/kernel/signal.c      |    6 -
> arch/powerpc/kernel/signal_32.c   |    8 +
> arch/powerpc/kernel/traps.c       |   86 ++++++-
> 6 files changed, 564 insertions(+), 82 deletions(-)
>
>
>diff --git a/arch/powerpc/include/asm/system.h b/arch/powerpc/include/asm/system.h
>index bb8e006..474bf23 100644
>--- a/arch/powerpc/include/asm/system.h
>+++ b/arch/powerpc/include/asm/system.h
>@@ -114,6 +114,8 @@ static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; }
> extern int set_dabr(unsigned long dabr);
> extern void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code);
>+extern void do_send_trap(struct pt_regs *regs, unsigned long address,
>+			 unsigned long error_code, int signal_code, int errno);
> extern void print_backtrace(unsigned long *);
> extern void show_regs(struct pt_regs * regs);
> extern void flush_instruction_cache(void);
>diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>index c930ac3..a0dbb09 100644
>--- a/arch/powerpc/kernel/process.c
>+++ b/arch/powerpc/kernel/process.c
>@@ -245,6 +245,24 @@ void discard_lazy_cpu_state(void)
> }
> #endif /* CONFIG_SMP */
>
>+void do_send_trap(struct pt_regs *regs, unsigned long address,
>+		  unsigned long error_code, int signal_code, int errno)
>+{
>+	siginfo_t info;
>+
>+	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>+			11, SIGSEGV) == NOTIFY_STOP)
>+		return;
>+
>+	/* Deliver the signal to userspace */
>+	info.si_signo = SIGTRAP;
>+	info.si_errno = errno;
>+	info.si_code = signal_code;
>+	info.si_addr = (void __user *)address;
>+	force_sig_info(SIGTRAP, &info, current);
>+}
>+
>+#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code)
> {
>@@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
> 	if (debugger_dabr_match(regs))
> 		return;
>
>-	/* Clear the DAC and struct entries.  One shot trigger */
>-#if defined(CONFIG_BOOKE)
>-	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
>-							| DBCR0_IDM));
>-#endif
>-
> 	/* Clear the DABR */
> 	set_dabr(0);
>
>@@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
> 	info.si_addr = (void __user *)address;
> 	force_sig_info(SIGTRAP, &info, current);
> }
>+#endif
>
> static DEFINE_PER_CPU(unsigned long, current_dabr);
>
>+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
>+/*
>+ * Set the debug registers back to their default "safe" values.
>+ */
>+static void set_debug_reg_defaults(struct thread_struct *thread)
>+{
>+	thread->iac1 = thread->iac2 = thread->iac3 = thread->iac4 = 0;
>+	thread->dac1 = thread->dac2 = 0;
>+	thread->dvc1 = thread->dvc2 = 0;
>+	/*
>+	 * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
>+	 * the exception of the reserved bits should be cleared out
>+	 * and set to 0.
>+	 *
>+	 * For the DBCR0 register, the reserved bits are bits 17:30.
>+	 * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
>+	 * And, bits 10:11 for DBCR2.
>+	 */
>+	thread->dbcr0 = DBCR0_BASE_REG_VALUE;
>+	/*
>+	 * First clear all "non reserved" bits from DBCR1 then initialize reg
>+	 * to force User/Supervisor bits to b11 (user-only MSR[PR]=1) and
>+	 * Effective/Real * bits to b10 (trap only if IS==0)
>+	 */
>+	thread->dbcr1 = DBCR1_BASE_REG_VALUE;
>+	/*
>+	 * Force Data Address Compare User/Supervisor bits to be User-only
>+	 * (0b11 MSR[PR]=1) and set all other bits in DBCR2 register to be 0.
>+	 * This sets the Data Address Compare Effective/Real bits to be 0b00
>+	 * (Effective, MSR[DS]=don't care).
>+	 */
>+	thread->dbcr2 = DBCR2_BASE_REG_VALUE;
>+}
>+
>+static void prime_debug_regs(struct thread_struct *thread)
>+{
>+	mtspr(SPRN_IAC1, thread->iac1);
>+	mtspr(SPRN_IAC2, thread->iac2);
>+	mtspr(SPRN_IAC3, thread->iac3);
>+	mtspr(SPRN_IAC4, thread->iac4);
>+	mtspr(SPRN_DAC1, thread->dac1);
>+	mtspr(SPRN_DAC2, thread->dac2);
>+	mtspr(SPRN_DVC1, thread->dvc1);
>+	mtspr(SPRN_DVC2, thread->dvc2);
>+	mtspr(SPRN_DBCR0, thread->dbcr0);
>+	mtspr(SPRN_DBCR1, thread->dbcr1);
>+	mtspr(SPRN_DBCR2, thread->dbcr2);

405 has no DBCR2, so I doubt setting it via mtspr would be good.  Does this
compile for 40x and did you test it at all?

Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
CONFIG_PPC_BOOK3E_64 which is also inaccurate.  I don't see that fixed in any
of the other patches.  Are you missing a patch, or was this generated against
and old kernel or something?

>@@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
> 	printk("  CR: %08lx  XER: %08lx\n", regs->ccr, regs->xer);
> 	trap = TRAP(regs);
> 	if (trap == 0x300 || trap == 0x600)
>-#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
>+#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
> #else
> 		printk("DAR: "REG", DSISR: "REG"\n", regs->dar, regs->dsisr);

This hunk seems unnecessary.

I need to find some time to sit down and look over the rest still.  I do have
some questions on how/if this will interact correctly with an older GDB still,
and some of the things I've pointed out make me wonder how much this has all
been tested and on what platforms.

josh

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Josh Boyer]

On Thu, Dec 10, 2009 at 12:27:11PM -0500, Josh Boyer wrote:
>On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
>>+static void prime_debug_regs(struct thread_struct *thread)
>>+{
>>+	mtspr(SPRN_IAC1, thread->iac1);
>>+	mtspr(SPRN_IAC2, thread->iac2);
>>+	mtspr(SPRN_IAC3, thread->iac3);
>>+	mtspr(SPRN_IAC4, thread->iac4);
>>+	mtspr(SPRN_DAC1, thread->dac1);
>>+	mtspr(SPRN_DAC2, thread->dac2);
>>+	mtspr(SPRN_DVC1, thread->dvc1);
>>+	mtspr(SPRN_DVC2, thread->dvc2);
>>+	mtspr(SPRN_DBCR0, thread->dbcr0);
>>+	mtspr(SPRN_DBCR1, thread->dbcr1);
>>+	mtspr(SPRN_DBCR2, thread->dbcr2);
>
>405 has no DBCR2, so I doubt setting it via mtspr would be good.  Does this
>compile for 40x and did you test it at all?
>
>Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
>CONFIG_PPC_BOOK3E_64 which is also inaccurate.

I mis-read the code on this part.  SPRN_DBCR2 gets defined unconditionally,
so Book-E should be fine.  Still seems broken on 405 though.

josh

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Dave Kleikamp]

On Thu, 2009-12-10 at 12:41 -0500, Josh Boyer wrote:
> On Thu, Dec 10, 2009 at 12:27:11PM -0500, Josh Boyer wrote:
> >On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
> >>+static void prime_debug_regs(struct thread_struct *thread)
> >>+{
> >>+	mtspr(SPRN_IAC1, thread->iac1);
> >>+	mtspr(SPRN_IAC2, thread->iac2);
> >>+	mtspr(SPRN_IAC3, thread->iac3);
> >>+	mtspr(SPRN_IAC4, thread->iac4);
> >>+	mtspr(SPRN_DAC1, thread->dac1);
> >>+	mtspr(SPRN_DAC2, thread->dac2);
> >>+	mtspr(SPRN_DVC1, thread->dvc1);
> >>+	mtspr(SPRN_DVC2, thread->dvc2);
> >>+	mtspr(SPRN_DBCR0, thread->dbcr0);
> >>+	mtspr(SPRN_DBCR1, thread->dbcr1);
> >>+	mtspr(SPRN_DBCR2, thread->dbcr2);
> >
> >405 has no DBCR2, so I doubt setting it via mtspr would be good.  Does this
> >compile for 40x and did you test it at all?
> >
> >Also, looking at the current kernel it seems SPRN_DBCR2 is only defined for
> >CONFIG_PPC_BOOK3E_64 which is also inaccurate.
> 
> I mis-read the code on this part.  SPRN_DBCR2 gets defined unconditionally,
> so Book-E should be fine.  Still seems broken on 405 though.

Agreed.  I'll rushed through the 405 stuff a bit.  I'll scrutinize and
test it before I post again.

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 01/03] powerpc: Extended ptrace interface

[David Gibson]

On Thu, Dec 10, 2009 at 01:57:15PM -0200, Dave Kleikamp wrote:
> powerpc: Extended ptrace interface
> 
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> 
> Add a new extended ptrace interface so that user-space has a single
> interface for powerpc, without having to know the specific layout
> of the debug registers.


> Implement:
> PPC_PTRACE_GETHWDEBUGINFO
> PPC_PTRACE_SETHWDEBUG
> PPC_PTRACE_DELHWDEBUG
> 
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>

Apart from the data breakpoint alignment for 32-bit systems, all the
comments below are trivial nits, so:

Acked-by: David Gibson <dwg@au1.ibm.com>

[snip]
> +/*
> + * Trigger Type
> + */
> +#define PPC_BREAKPOINT_TRIGGER_EXECUTE	0x1
> +#define PPC_BREAKPOINT_TRIGGER_READ	0x2
> +#define PPC_BREAKPOINT_TRIGGER_WRITE	0x4
> +#define PPC_BREAKPOINT_TRIGGER_RW	0x6

For a little extra safety, I'd tend towards defining the RW constant
in terms of the READ and WRITE constants.

> +
> +/*
> + * Address Mode
> + */
> +#define PPC_BREAKPOINT_MODE_EXACT		0x0
> +#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE	0x1
> +#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE	0x2
> +#define PPC_BREAKPOINT_MODE_MASK		0x3
> +
> +/*
> + * Condition Mode
> + */
> +#define PPC_BREAKPOINT_CONDITION_NONE	0x0
> +#define PPC_BREAKPOINT_CONDITION_AND	0x1
> +#define PPC_BREAKPOINT_CONDITION_EXACT	0x1

And likewuse define EXACT in terms of AND.

> +#define PPC_BREAKPOINT_CONDITION_OR	0x2
> +#define PPC_BREAKPOINT_CONDITION_AND_OR	0x3
> +#define PPC_BREAKPOINT_CONDITION_BE_ALL	0x00ff0000
> +#define PPC_BREAKPOINT_CONDITION_BE_SHIFT	16
> +#define PPC_BREAKPOINT_CONDITION_BE(n)	\
> +	(1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))

[snip]
> +	case PPC_PTRACE_GETHWDBGINFO: {
> +		struct ppc_debug_info dbginfo;
> +
> +		dbginfo.version = 1;
> +		dbginfo.num_instruction_bps = 0;
> +		dbginfo.num_data_bps = 1;
> +		dbginfo.num_condition_regs = 0;
> +#ifdef CONFIG_PPC64
> +		dbginfo.data_bp_alignment = 8;
> +#else
> +		dbginfo.data_bp_alignment = 0;

Uh.. this looks wrong.  Surely it should be 4.

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

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[David Gibson]

On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> powerpc: Add definitions for Debug Registers on BookE Platforms
> 
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> 
> This patch adds additional definitions for BookE Debug Registers
> to the reg_booke.h header file.
> 
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>

As with patch 1/3, none of the comments below is anything that
couldn't be fixed up after merging.  So,

Acked-by: David Gibson <dwg@au1.ibm.com>

> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
> 
>  arch/powerpc/include/asm/processor.h |   30 +++++-
>  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
>  2 files changed, 178 insertions(+), 28 deletions(-)

[snip]
> +	/*
> +	 * The following will contain addresses used by debug applications
> +	 * to help trace and trap on particular address locations.
> +	 * The bits in the Debug Control Registers above help define which
> +	 * of the following registers will contain valid data and/or addresses.
> +	 */
> +	unsigned long	iac1;
> +	unsigned long	iac2;
> +	unsigned long	iac3;
> +	unsigned long	iac4;
> +	unsigned long	dac1;
> +	unsigned long	dac2;
> +	unsigned long	dvc1;
> +	unsigned long	dvc2;

I think you'd make the logic in patch 3 substantially easier, if you
defined these as
	unsigned long iac[4];
	unsigned long dac[2];
	unsigned long dvc[2];
instead of as individual structure members.

[snip]
> +#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> +				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> +#define DBCR0_BASE_REG_VALUE	0

These constants are left over from when the interface allowed
more-or-less direct access to the debug regs.  I don't think the
USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
just used in the load_default function, and might as well be inline
there.

[snip]
> +
> +#define dbcr_iac_range(task)	((task)->thread.dbcr0)

Hrm, I think the way these macros work to do the 40x vs. BookE
abstration is kind of ugly.  But an unequivocally better way doesn't
immediately occur to me.

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

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Dave Kleikamp]

On Fri, 2009-12-11 at 11:53 +1100, David Gibson wrote:
> On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> > powerpc: Add definitions for Debug Registers on BookE Platforms
> > 
> > From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> > 
> > This patch adds additional definitions for BookE Debug Registers
> > to the reg_booke.h header file.
> > 
> > Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> 
> As with patch 1/3, none of the comments below is anything that
> couldn't be fixed up after merging.  So,
> 
> Acked-by: David Gibson <dwg@au1.ibm.com>
> 
> > Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> > Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> > Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> > Cc: David Gibson <dwg@au1.ibm.com>
> > Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> > ---
> > 
> >  arch/powerpc/include/asm/processor.h |   30 +++++-
> >  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
> >  2 files changed, 178 insertions(+), 28 deletions(-)
> 
> [snip]
> > +	/*
> > +	 * The following will contain addresses used by debug applications
> > +	 * to help trace and trap on particular address locations.
> > +	 * The bits in the Debug Control Registers above help define which
> > +	 * of the following registers will contain valid data and/or addresses.
> > +	 */
> > +	unsigned long	iac1;
> > +	unsigned long	iac2;
> > +	unsigned long	iac3;
> > +	unsigned long	iac4;
> > +	unsigned long	dac1;
> > +	unsigned long	dac2;
> > +	unsigned long	dvc1;
> > +	unsigned long	dvc2;
> 
> I think you'd make the logic in patch 3 substantially easier, if you
> defined these as
> 	unsigned long iac[4];
> 	unsigned long dac[2];
> 	unsigned long dvc[2];
> instead of as individual structure members.

I'll give that a look.  You're probably right.

> [snip]
> > +#define DBCR0_USER_DEBUG	(DBCR0_IDM | DBCR0_ICMP | DBCR0_IAC1 | \
> > +				 DBCR0_IAC2 | DBCR0_IAC3 | DBCR0_IAC4)
> > +#define DBCR0_BASE_REG_VALUE	0
> 
> These constants are left over from when the interface allowed
> more-or-less direct access to the debug regs.  I don't think the
> USER_DEBUG constant is used at all any more, and the BASE_REG_VALUE is
> just used in the load_default function, and might as well be inline
> there.

Right.

> [snip]
> > +
> > +#define dbcr_iac_range(task)	((task)->thread.dbcr0)
> 
> Hrm, I think the way these macros work to do the 40x vs. BookE
> abstration is kind of ugly.  But an unequivocally better way doesn't
> immediately occur to me.

Without this, the ifdef's were horrendous.  I'm open to renaming this or
redefining it to be more intuitive if anyone has a better idea.
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Kumar Gala]

On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> These patches implement an extention to the ptrace interface proposed =
by
> Thiago Bauermann and the the PowerPC gdb team.
>=20
> GDB intends to support the following hardware debug features of BookE
> processors:
>=20
> 4 hardware breakpoints (IAC)
> 2 hardware watchpoints (read, write and read-write) (DAC)
> 2 value conditions for the hardware watchpoints (DVC)
>=20
> For that, we need to extend ptrace so that GDB can query and set these
> resources. Since we're extending, we're trying to create an interface
> that's extendable and that covers both BookE and server processors, so
> that GDB doesn't need to special-case each of them. We propose the
> following 3 new ptrace requests described below.
>=20
> There have been discussions of a generic hardware debug interface for =
the
> kernel which would hopefully contemplate all the functionality below =
and
> supersede it.  But we need something that works now, and which enables =
GDB
> to be simpler and work with both Server and Embedded processors =
without
> special cases.
>=20
> 1. PTRACE_PPC_GETHWDEBUGINFO
>=20
> Query for GDB to discover the hardware debug features. The main info =
to
> be returned here is the minimum alignment for the hardware =
watchpoints.
> BookE processors don't have restrictions here, but server processors =
have
> an 8-byte alignment restriction for hardware watchpoints. We'd like to =
avoid
> adding special cases to GDB based on what it sees in AUXV.
>=20
> Since we're at it, we added other useful info that the kernel can =
return to
> GDB: this query will return the number of hardware breakpoints, =
hardware
> watchpoints and whether it supports a range of addresses and a =
condition.
> The query will fill the following structure provided by the requesting =
process:
>=20
> struct ppc_debug_info {
>       unit32_t version;
>       unit32_t num_instruction_bps;
>       unit32_t num_data_bps;
>       unit32_t num_condition_regs;
>       unit32_t data_bp_alignment;
>       unit32_t sizeof_condition; /* size of the DVC register */
>       uint64_t features; /* bitmask of the individual flags */
> };
>=20
> features will have bits indicating whether there is support for:
>=20
> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8

Is GDB smart enough to deal w/no condition_regs?  On some Book-E devices =
we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the features?

- k=

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Kumar Gala]

On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> These patches implement an extention to the ptrace interface proposed =
by
> Thiago Bauermann and the the PowerPC gdb team.
>=20
> GDB intends to support the following hardware debug features of BookE
> processors:
>=20
> 4 hardware breakpoints (IAC)
> 2 hardware watchpoints (read, write and read-write) (DAC)
> 2 value conditions for the hardware watchpoints (DVC)
>=20
> For that, we need to extend ptrace so that GDB can query and set these
> resources. Since we're extending, we're trying to create an interface
> that's extendable and that covers both BookE and server processors, so
> that GDB doesn't need to special-case each of them. We propose the
> following 3 new ptrace requests described below.
>=20
> There have been discussions of a generic hardware debug interface for =
the
> kernel which would hopefully contemplate all the functionality below =
and
> supersede it.  But we need something that works now, and which enables =
GDB
> to be simpler and work with both Server and Embedded processors =
without
> special cases.
>=20
> 1. PTRACE_PPC_GETHWDEBUGINFO
>=20
> Query for GDB to discover the hardware debug features. The main info =
to
> be returned here is the minimum alignment for the hardware =
watchpoints.
> BookE processors don't have restrictions here, but server processors =
have
> an 8-byte alignment restriction for hardware watchpoints. We'd like to =
avoid
> adding special cases to GDB based on what it sees in AUXV.
>=20
> Since we're at it, we added other useful info that the kernel can =
return to
> GDB: this query will return the number of hardware breakpoints, =
hardware
> watchpoints and whether it supports a range of addresses and a =
condition.
> The query will fill the following structure provided by the requesting =
process:
>=20
> struct ppc_debug_info {
>       unit32_t version;
>       unit32_t num_instruction_bps;
>       unit32_t num_data_bps;
>       unit32_t num_condition_regs;
>       unit32_t data_bp_alignment;
>       unit32_t sizeof_condition; /* size of the DVC register */
>       uint64_t features; /* bitmask of the individual flags */
> };
>=20
> features will have bits indicating whether there is support for:
>=20
> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
>=20
> 2. PTRACE_SETHWDEBUG
>=20
> Sets a hardware breakpoint or watchpoint, according to the provided =
structure:
>=20
> struct ppc_hw_breakpoint {
>        uint32_t version;
> #define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
> #define PPC_BREAKPOINT_TRIGGER_READ     0x2
> #define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
>        uint32_t trigger_type;       /* only some combinations allowed =
*/
> #define PPC_BREAKPOINT_MODE_EXACT               0x0
> #define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
> #define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
> #define PPC_BREAKPOINT_MODE_MASK                0x3
>        uint32_t addr_mode;          /* address match mode */
>=20
> #define PPC_BREAKPOINT_CONDITION_NONE   0x0
> #define PPC_BREAKPOINT_CONDITION_AND    0x1
> #define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for =
the same thing as above */
> #define PPC_BREAKPOINT_CONDITION_OR     0x2
> #define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
> #define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable =
bits */
> #define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
>        uint32_t condition_mode;     /* break/watchpoint condition =
flags */
>=20
>        uint64_t addr;
>        uint64_t addr2;
>        uint64_t condition_value;
> };
>=20
> A request specifies one event, not necessarily just one register to be =
set.
> For instance, if the request is for a watchpoint with a condition, =
both the
> DAC and DVC registers will be set in the same request.
>=20
> With this GDB can ask for all kinds of hardware breakpoints and =
watchpoints
> that the BookE supports. COMEFROM breakpoints available in server =
processors
> are not contemplated, but that is out of the scope of this work.
>=20
> ptrace will return an integer (handle) uniquely identifying the =
breakpoint or
> watchpoint just created. This integer will be used in the =
PTRACE_DELHWDEBUG
> request to ask for its removal. Return -ENOSPC if the requested =
breakpoint
> can't be allocated on the registers.
>=20
> Some examples of using the structure to:
>=20
> - set a breakpoint in the first breakpoint register
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>  p.addr            =3D (uint64_t) address;
>  p.addr2           =3D 0;
>  p.condition_value =3D 0;
>=20
> - set a watchpoint which triggers on reads in the second watchpoint =
register
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>  p.addr            =3D (uint64_t) address;
>  p.addr2           =3D 0;
>  p.condition_value =3D 0;
>=20
> - set a watchpoint which triggers only with a specific value
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_AND | =
PPC_BREAKPOINT_CONDITION_BE_ALL;
>  p.addr            =3D (uint64_t) address;
>  p.addr2           =3D 0;
>  p.condition_value =3D (uint64_t) condition;
>=20
> - set a ranged hardware breakpoint
>=20
>  p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>  p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>  p.addr_mode       =3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
>  p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>  p.addr            =3D (uint64_t) begin_range;
>  p.addr2           =3D (uint64_t) end_range;
>  p.condition_value =3D 0;
>=20
> 3. PTRACE_DELHWDEBUG
>=20
> Takes an integer which identifies an existing breakpoint or watchpoint
> (i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
> corresponding breakpoint or watchpoint..

This is a good write up.  We should have it as a commit message for the =
first patch.

- k

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Dave Kleikamp]

On Thu, 2009-12-10 at 20:23 -0600, Kumar Gala wrote:
> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> 
> > These patches implement an extention to the ptrace interface proposed by
> > Thiago Bauermann and the the PowerPC gdb team.
> > 
> > GDB intends to support the following hardware debug features of BookE
> > processors:
> > 
> > 4 hardware breakpoints (IAC)
> > 2 hardware watchpoints (read, write and read-write) (DAC)
> > 2 value conditions for the hardware watchpoints (DVC)
> > 
> > For that, we need to extend ptrace so that GDB can query and set these
> > resources. Since we're extending, we're trying to create an interface
> > that's extendable and that covers both BookE and server processors, so
> > that GDB doesn't need to special-case each of them. We propose the
> > following 3 new ptrace requests described below.
> > 
> > There have been discussions of a generic hardware debug interface for the
> > kernel which would hopefully contemplate all the functionality below and
> > supersede it.  But we need something that works now, and which enables GDB
> > to be simpler and work with both Server and Embedded processors without
> > special cases.
> > 
> > 1. PTRACE_PPC_GETHWDEBUGINFO
> > 
> > Query for GDB to discover the hardware debug features. The main info to
> > be returned here is the minimum alignment for the hardware watchpoints.
> > BookE processors don't have restrictions here, but server processors have
> > an 8-byte alignment restriction for hardware watchpoints. We'd like to avoid
> > adding special cases to GDB based on what it sees in AUXV.
> > 
> > Since we're at it, we added other useful info that the kernel can return to
> > GDB: this query will return the number of hardware breakpoints, hardware
> > watchpoints and whether it supports a range of addresses and a condition.
> > The query will fill the following structure provided by the requesting process:
> > 
> > struct ppc_debug_info {
> >       unit32_t version;
> >       unit32_t num_instruction_bps;
> >       unit32_t num_data_bps;
> >       unit32_t num_condition_regs;
> >       unit32_t data_bp_alignment;
> >       unit32_t sizeof_condition; /* size of the DVC register */
> >       uint64_t features; /* bitmask of the individual flags */
> > };
> > 
> > features will have bits indicating whether there is support for:
> > 
> > #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> > #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> > #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> > #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
> 
> Is GDB smart enough to deal w/no condition_regs?  On some Book-E
> devices we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the
> features?

I had discussed it with the gdb team.  I could easily add a feature
flag, but it would be equivalent to num_condition_regs > 0.  I don't
have a strong opinion either way.
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Dave Kleikamp]

On Thu, 2009-12-10 at 20:24 -0600, Kumar Gala wrote:
> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> 
> > These patches implement an extention to the ptrace interface proposed by
> > Thiago Bauermann and the the PowerPC gdb team.
> > 
> > GDB intends to support the following hardware debug features of BookE
> > processors:
> > 
> > 4 hardware breakpoints (IAC)
> > 2 hardware watchpoints (read, write and read-write) (DAC)
> > 2 value conditions for the hardware watchpoints (DVC)
> > 
> > For that, we need to extend ptrace so that GDB can query and set these
> > resources. Since we're extending, we're trying to create an interface
> > that's extendable and that covers both BookE and server processors, so
> > that GDB doesn't need to special-case each of them. We propose the
> > following 3 new ptrace requests described below.
> > 
> > There have been discussions of a generic hardware debug interface for the
> > kernel which would hopefully contemplate all the functionality below and
> > supersede it.  But we need something that works now, and which enables GDB
> > to be simpler and work with both Server and Embedded processors without
> > special cases.
> > 
> > 1. PTRACE_PPC_GETHWDEBUGINFO
> > 
> > Query for GDB to discover the hardware debug features. The main info to
> > be returned here is the minimum alignment for the hardware watchpoints.
> > BookE processors don't have restrictions here, but server processors have
> > an 8-byte alignment restriction for hardware watchpoints. We'd like to avoid
> > adding special cases to GDB based on what it sees in AUXV.
> > 
> > Since we're at it, we added other useful info that the kernel can return to
> > GDB: this query will return the number of hardware breakpoints, hardware
> > watchpoints and whether it supports a range of addresses and a condition.
> > The query will fill the following structure provided by the requesting process:
> > 
> > struct ppc_debug_info {
> >       unit32_t version;
> >       unit32_t num_instruction_bps;
> >       unit32_t num_data_bps;
> >       unit32_t num_condition_regs;
> >       unit32_t data_bp_alignment;
> >       unit32_t sizeof_condition; /* size of the DVC register */
> >       uint64_t features; /* bitmask of the individual flags */
> > };
> > 
> > features will have bits indicating whether there is support for:
> > 
> > #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> > #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> > #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> > #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
> > 
> > 2. PTRACE_SETHWDEBUG
> > 
> > Sets a hardware breakpoint or watchpoint, according to the provided structure:
> > 
> > struct ppc_hw_breakpoint {
> >        uint32_t version;
> > #define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
> > #define PPC_BREAKPOINT_TRIGGER_READ     0x2
> > #define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
> >        uint32_t trigger_type;       /* only some combinations allowed */
> > #define PPC_BREAKPOINT_MODE_EXACT               0x0
> > #define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
> > #define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
> > #define PPC_BREAKPOINT_MODE_MASK                0x3
> >        uint32_t addr_mode;          /* address match mode */
> > 
> > #define PPC_BREAKPOINT_CONDITION_NONE   0x0
> > #define PPC_BREAKPOINT_CONDITION_AND    0x1
> > #define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for the same thing as above */
> > #define PPC_BREAKPOINT_CONDITION_OR     0x2
> > #define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
> > #define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable bits */
> > #define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
> >        uint32_t condition_mode;     /* break/watchpoint condition flags */
> > 
> >        uint64_t addr;
> >        uint64_t addr2;
> >        uint64_t condition_value;
> > };
> > 
> > A request specifies one event, not necessarily just one register to be set.
> > For instance, if the request is for a watchpoint with a condition, both the
> > DAC and DVC registers will be set in the same request.
> > 
> > With this GDB can ask for all kinds of hardware breakpoints and watchpoints
> > that the BookE supports. COMEFROM breakpoints available in server processors
> > are not contemplated, but that is out of the scope of this work.
> > 
> > ptrace will return an integer (handle) uniquely identifying the breakpoint or
> > watchpoint just created. This integer will be used in the PTRACE_DELHWDEBUG
> > request to ask for its removal. Return -ENOSPC if the requested breakpoint
> > can't be allocated on the registers.
> > 
> > Some examples of using the structure to:
> > 
> > - set a breakpoint in the first breakpoint register
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> >  p.addr            = (uint64_t) address;
> >  p.addr2           = 0;
> >  p.condition_value = 0;
> > 
> > - set a watchpoint which triggers on reads in the second watchpoint register
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_READ;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> >  p.addr            = (uint64_t) address;
> >  p.addr2           = 0;
> >  p.condition_value = 0;
> > 
> > - set a watchpoint which triggers only with a specific value
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_READ;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_EXACT;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_AND | PPC_BREAKPOINT_CONDITION_BE_ALL;
> >  p.addr            = (uint64_t) address;
> >  p.addr2           = 0;
> >  p.condition_value = (uint64_t) condition;
> > 
> > - set a ranged hardware breakpoint
> > 
> >  p.version         = PPC_DEBUG_CURRENT_VERSION;
> >  p.trigger_type    = PPC_BREAKPOINT_TRIGGER_EXECUTE;
> >  p.addr_mode       = PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
> >  p.condition_mode  = PPC_BREAKPOINT_CONDITION_NONE;
> >  p.addr            = (uint64_t) begin_range;
> >  p.addr2           = (uint64_t) end_range;
> >  p.condition_value = 0;
> > 
> > 3. PTRACE_DELHWDEBUG
> > 
> > Takes an integer which identifies an existing breakpoint or watchpoint
> > (i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
> > corresponding breakpoint or watchpoint..
> 
> This is a good write up.  We should have it as a commit message for the first patch.

I have to give credit to Thiago for this.

Would it be worth adding to Documentation/powerpc/ ?
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Kumar Gala]

On Dec 10, 2009, at 8:29 PM, Dave Kleikamp wrote:

> On Thu, 2009-12-10 at 20:24 -0600, Kumar Gala wrote:
>> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
>>=20
>>> These patches implement an extention to the ptrace interface =
proposed by
>>> Thiago Bauermann and the the PowerPC gdb team.
>>>=20
>>> GDB intends to support the following hardware debug features of =
BookE
>>> processors:
>>>=20
>>> 4 hardware breakpoints (IAC)
>>> 2 hardware watchpoints (read, write and read-write) (DAC)
>>> 2 value conditions for the hardware watchpoints (DVC)
>>>=20
>>> For that, we need to extend ptrace so that GDB can query and set =
these
>>> resources. Since we're extending, we're trying to create an =
interface
>>> that's extendable and that covers both BookE and server processors, =
so
>>> that GDB doesn't need to special-case each of them. We propose the
>>> following 3 new ptrace requests described below.
>>>=20
>>> There have been discussions of a generic hardware debug interface =
for the
>>> kernel which would hopefully contemplate all the functionality below =
and
>>> supersede it.  But we need something that works now, and which =
enables GDB
>>> to be simpler and work with both Server and Embedded processors =
without
>>> special cases.
>>>=20
>>> 1. PTRACE_PPC_GETHWDEBUGINFO
>>>=20
>>> Query for GDB to discover the hardware debug features. The main info =
to
>>> be returned here is the minimum alignment for the hardware =
watchpoints.
>>> BookE processors don't have restrictions here, but server processors =
have
>>> an 8-byte alignment restriction for hardware watchpoints. We'd like =
to avoid
>>> adding special cases to GDB based on what it sees in AUXV.
>>>=20
>>> Since we're at it, we added other useful info that the kernel can =
return to
>>> GDB: this query will return the number of hardware breakpoints, =
hardware
>>> watchpoints and whether it supports a range of addresses and a =
condition.
>>> The query will fill the following structure provided by the =
requesting process:
>>>=20
>>> struct ppc_debug_info {
>>>      unit32_t version;
>>>      unit32_t num_instruction_bps;
>>>      unit32_t num_data_bps;
>>>      unit32_t num_condition_regs;
>>>      unit32_t data_bp_alignment;
>>>      unit32_t sizeof_condition; /* size of the DVC register */
>>>      uint64_t features; /* bitmask of the individual flags */
>>> };
>>>=20
>>> features will have bits indicating whether there is support for:
>>>=20
>>> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
>>> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
>>> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
>>> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
>>>=20
>>> 2. PTRACE_SETHWDEBUG
>>>=20
>>> Sets a hardware breakpoint or watchpoint, according to the provided =
structure:
>>>=20
>>> struct ppc_hw_breakpoint {
>>>       uint32_t version;
>>> #define PPC_BREAKPOINT_TRIGGER_EXECUTE  0x1
>>> #define PPC_BREAKPOINT_TRIGGER_READ     0x2
>>> #define PPC_BREAKPOINT_TRIGGER_WRITE    0x4
>>>       uint32_t trigger_type;       /* only some combinations allowed =
*/
>>> #define PPC_BREAKPOINT_MODE_EXACT               0x0
>>> #define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE     0x1
>>> #define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE     0x2
>>> #define PPC_BREAKPOINT_MODE_MASK                0x3
>>>       uint32_t addr_mode;          /* address match mode */
>>>=20
>>> #define PPC_BREAKPOINT_CONDITION_NONE   0x0
>>> #define PPC_BREAKPOINT_CONDITION_AND    0x1
>>> #define PPC_BREAKPOINT_CONDITION_EXACT  0x1	/* different name for =
the same thing as above */
>>> #define PPC_BREAKPOINT_CONDITION_OR     0x2
>>> #define PPC_BREAKPOINT_CONDITION_AND_OR 0x3
>>> #define PPC_BREAKPOINT_CONDITION_BE_ALL 0x00ff0000	/* byte enable =
bits */
>>> #define PPC_BREAKPOINT_CONDITION_BE(n)  (1<<((n)+16))
>>>       uint32_t condition_mode;     /* break/watchpoint condition =
flags */
>>>=20
>>>       uint64_t addr;
>>>       uint64_t addr2;
>>>       uint64_t condition_value;
>>> };
>>>=20
>>> A request specifies one event, not necessarily just one register to =
be set.
>>> For instance, if the request is for a watchpoint with a condition, =
both the
>>> DAC and DVC registers will be set in the same request.
>>>=20
>>> With this GDB can ask for all kinds of hardware breakpoints and =
watchpoints
>>> that the BookE supports. COMEFROM breakpoints available in server =
processors
>>> are not contemplated, but that is out of the scope of this work.
>>>=20
>>> ptrace will return an integer (handle) uniquely identifying the =
breakpoint or
>>> watchpoint just created. This integer will be used in the =
PTRACE_DELHWDEBUG
>>> request to ask for its removal. Return -ENOSPC if the requested =
breakpoint
>>> can't be allocated on the registers.
>>>=20
>>> Some examples of using the structure to:
>>>=20
>>> - set a breakpoint in the first breakpoint register
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>>> p.addr            =3D (uint64_t) address;
>>> p.addr2           =3D 0;
>>> p.condition_value =3D 0;
>>>=20
>>> - set a watchpoint which triggers on reads in the second watchpoint =
register
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>>> p.addr            =3D (uint64_t) address;
>>> p.addr2           =3D 0;
>>> p.condition_value =3D 0;
>>>=20
>>> - set a watchpoint which triggers only with a specific value
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_READ;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_EXACT;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_AND | =
PPC_BREAKPOINT_CONDITION_BE_ALL;
>>> p.addr            =3D (uint64_t) address;
>>> p.addr2           =3D 0;
>>> p.condition_value =3D (uint64_t) condition;
>>>=20
>>> - set a ranged hardware breakpoint
>>>=20
>>> p.version         =3D PPC_DEBUG_CURRENT_VERSION;
>>> p.trigger_type    =3D PPC_BREAKPOINT_TRIGGER_EXECUTE;
>>> p.addr_mode       =3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE;
>>> p.condition_mode  =3D PPC_BREAKPOINT_CONDITION_NONE;
>>> p.addr            =3D (uint64_t) begin_range;
>>> p.addr2           =3D (uint64_t) end_range;
>>> p.condition_value =3D 0;
>>>=20
>>> 3. PTRACE_DELHWDEBUG
>>>=20
>>> Takes an integer which identifies an existing breakpoint or =
watchpoint
>>> (i.e., the value returned from PTRACE_SETHWDEBUG), and deletes the
>>> corresponding breakpoint or watchpoint..
>>=20
>> This is a good write up.  We should have it as a commit message for =
the first patch.
>=20
> I have to give credit to Thiago for this.
>=20
> Would it be worth adding to Documentation/powerpc/ ?

That would also work.

- k=

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Kumar Gala]

On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> +#define DBCR1_IAC1US	0xC0000000	/* Instr Addr Cmp 1 Sup/User   =
*/
> +#define DBCR1_IAC1ER	0x30000000	/* Instr Addr Cmp 1 Eff/Real */
> +#define DBCR1_IAC1ER_01	0x10000000	/* reserved */
> +#define DBCR1_IAC1ER_10	0x20000000	/* Instr Addr Cmp 1 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC1ER_11	0x30000000	/* Instr Addr Cmp 1 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC2US	0x0C000000	/* Instr Addr Cmp 2 Sup/User   =
*/
> +#define DBCR1_IAC2ER	0x03000000	/* Instr Addr Cmp 2 Eff/Real */
> +#define DBCR1_IAC2ER_01	0x01000000	/* reserved */
> +#define DBCR1_IAC2ER_10	0x02000000	/* Instr Addr Cmp 2 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC2ER_11	0x03000000	/* Instr Addr Cmp 2 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC12M	0x00C00000	/* Instr Addr 1-2 range enable =
*/
> +#define DBCR1_IAC12M_R	0x00400000	/* Instr Addr 1-2 =
reserved state */
> +#define DBCR1_IAC12M_I	0x00800000	/* Instr Addr 1-2 range =
inclusive */
> +#define DBCR1_IAC12M_X	0x00C00000	/* Instr Addr 1-2 range =
eXclusive */
> +#define DBCR1_IAC12A_T	0x00010000	/* Instr Addr 1-2 range =
Toggle */
> +#define DBCR1_IAC3US	0x0000C000	/* Instr Addr Cmp 3 Sup/User   =
*/
> +#define DBCR1_IAC3ER	0x00003000	/* Instr Addr Cmp 3 Eff/Real */
> +#define DBCR1_IAC3ER_01	0x00001000	/* reserved */
> +#define DBCR1_IAC3ER_10	0x00002000	/* Instr Addr Cmp 3 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC3ER_11	0x00003000	/* Instr Addr Cmp 3 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC4US	0x00000C00	/* Instr Addr Cmp 4 Sup/User   =
*/
> +#define DBCR1_IAC4ER	0x00000300	/* Instr Addr Cmp 4 Eff/Real */
> +#define DBCR1_IAC4ER_01	0x00000100	/* Instr Addr Cmp 4 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC4ER_10	0x00000200	/* Instr Addr Cmp 4 =
Eff/Real MSR[IS]=3D0 */
> +#define DBCR1_IAC4ER_11	0x00000300	/* Instr Addr Cmp 4 =
Eff/Real MSR[IS]=3D1 */
> +#define DBCR1_IAC34M	0x000000C0	/* Instr Addr 3-4 range enable =
*/
> +#define DBCR1_IAC34M_R	0x00000040	/* Instr Addr 3-4 =
reserved state */
> +#define DBCR1_IAC34M_I	0x00000080	/* Instr Addr 3-4 range =
inclusive */
> +#define DBCR1_IAC34M_X	0x000000C0	/* Instr Addr 3-4 range =
eXclusive */
> +#define DBCR1_IAC34A_T	0x00000001	/* Instr Addr 3-4 range =
Toggle */
> +
> +#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
> +#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 | \
> +				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
> +				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
> +				 DBCR1_IAC4US | DBCR1_IAC4ER_10)

We are we using MSR[IS] IS=3D0, why not just any Eff address?  In the =
future we might have user as IS =3D 1, and kernel as IS =3D 0.

- k=

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Kumar Gala]

On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> These patches implement an extention to the ptrace interface proposed =
by
> Thiago Bauermann and the the PowerPC gdb team.
>=20
> GDB intends to support the following hardware debug features of BookE
> processors:
>=20
> 4 hardware breakpoints (IAC)
> 2 hardware watchpoints (read, write and read-write) (DAC)
> 2 value conditions for the hardware watchpoints (DVC)
>=20
> For that, we need to extend ptrace so that GDB can query and set these
> resources. Since we're extending, we're trying to create an interface
> that's extendable and that covers both BookE and server processors, so
> that GDB doesn't need to special-case each of them. We propose the
> following 3 new ptrace requests described below.
>=20
> There have been discussions of a generic hardware debug interface for =
the
> kernel which would hopefully contemplate all the functionality below =
and
> supersede it.  But we need something that works now, and which enables =
GDB
> to be simpler and work with both Server and Embedded processors =
without
> special cases.

What do we do in EDM mode?  We need a flag somewhere to determine if HW =
supports conveying DBCR0[EDM] and if it does which of the ptrace calls =
fails?

- k=

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Kumar Gala]

On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
>=20
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>=20
> This patch defines context switch and trap related functionality
> for BookE specific Debug Registers. It adds support to ptrace()
> for setting and getting BookE related Debug Registers
>=20
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
>=20
> arch/powerpc/include/asm/system.h |    2=20
> arch/powerpc/kernel/process.c     |  109 ++++++++-
> arch/powerpc/kernel/ptrace.c      |  435 =
++++++++++++++++++++++++++++++++++---
> arch/powerpc/kernel/signal.c      |    6 -
> arch/powerpc/kernel/signal_32.c   |    8 +
> arch/powerpc/kernel/traps.c       |   86 ++++++-
> 6 files changed, 564 insertions(+), 82 deletions(-)
>=20
>=20
> diff --git a/arch/powerpc/include/asm/system.h =
b/arch/powerpc/include/asm/system.h
> index bb8e006..474bf23 100644
> --- a/arch/powerpc/include/asm/system.h
> +++ b/arch/powerpc/include/asm/system.h
> @@ -114,6 +114,8 @@ static inline int debugger_fault_handler(struct =
pt_regs *regs) { return 0; }
> extern int set_dabr(unsigned long dabr);
> extern void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code);
> +extern void do_send_trap(struct pt_regs *regs, unsigned long address,
> +			 unsigned long error_code, int signal_code, int =
errno);
> extern void print_backtrace(unsigned long *);
> extern void show_regs(struct pt_regs * regs);
> extern void flush_instruction_cache(void);
> diff --git a/arch/powerpc/kernel/process.c =
b/arch/powerpc/kernel/process.c
> index c930ac3..a0dbb09 100644
> --- a/arch/powerpc/kernel/process.c
> +++ b/arch/powerpc/kernel/process.c
> @@ -245,6 +245,24 @@ void discard_lazy_cpu_state(void)
> }
> #endif /* CONFIG_SMP */
>=20
> +void do_send_trap(struct pt_regs *regs, unsigned long address,
> +		  unsigned long error_code, int signal_code, int errno)
> +{
> +	siginfo_t info;
> +
> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> +		return;
> +
> +	/* Deliver the signal to userspace */
> +	info.si_signo =3D SIGTRAP;
> +	info.si_errno =3D errno;
> +	info.si_code =3D signal_code;
> +	info.si_addr =3D (void __user *)address;
> +	force_sig_info(SIGTRAP, &info, current);
> +}
> +
> +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> void do_dabr(struct pt_regs *regs, unsigned long address,
> 		    unsigned long error_code)
> {
> @@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long =
address,
> 	if (debugger_dabr_match(regs))
> 		return;
>=20
> -	/* Clear the DAC and struct entries.  One shot trigger */
> -#if defined(CONFIG_BOOKE)
> -	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> -							| DBCR0_IDM));
> -#endif
> -
> 	/* Clear the DABR */
> 	set_dabr(0);
>=20
> @@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long =
address,
> 	info.si_addr =3D (void __user *)address;
> 	force_sig_info(SIGTRAP, &info, current);
> }
> +#endif
>=20
> static DEFINE_PER_CPU(unsigned long, current_dabr);
>=20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +/*
> + * Set the debug registers back to their default "safe" values.
> + */
> +static void set_debug_reg_defaults(struct thread_struct *thread)
> +{
> +	thread->iac1 =3D thread->iac2 =3D thread->iac3 =3D thread->iac4 =
=3D 0;
> +	thread->dac1 =3D thread->dac2 =3D 0;
> +	thread->dvc1 =3D thread->dvc2 =3D 0;
> +	/*
> +	 * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
> +	 * the exception of the reserved bits should be cleared out
> +	 * and set to 0.
> +	 *
> +	 * For the DBCR0 register, the reserved bits are bits 17:30.
> +	 * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
> +	 * And, bits 10:11 for DBCR2.
> +	 */
> +	thread->dbcr0 =3D DBCR0_BASE_REG_VALUE;

This seems to always be 0, why have a special #define for it.

> +	/*
> +	 * First clear all "non reserved" bits from DBCR1 then =
initialize reg
> +	 * to force User/Supervisor bits to b11 (user-only MSR[PR]=3D1) =
and
> +	 * Effective/Real * bits to b10 (trap only if IS=3D=3D0)
> +	 */
> +	thread->dbcr1 =3D DBCR1_BASE_REG_VALUE;
> +	/*
> +	 * Force Data Address Compare User/Supervisor bits to be =
User-only
> +	 * (0b11 MSR[PR]=3D1) and set all other bits in DBCR2 register =
to be 0.
> +	 * This sets the Data Address Compare Effective/Real bits to be =
0b00
> +	 * (Effective, MSR[DS]=3Ddon't care).
> +	 */
> +	thread->dbcr2 =3D DBCR2_BASE_REG_VALUE;
> +}
> +
> +static void prime_debug_regs(struct thread_struct *thread)
> +{
> +	mtspr(SPRN_IAC1, thread->iac1);
> +	mtspr(SPRN_IAC2, thread->iac2);
> +	mtspr(SPRN_IAC3, thread->iac3);
> +	mtspr(SPRN_IAC4, thread->iac4);
> +	mtspr(SPRN_DAC1, thread->dac1);
> +	mtspr(SPRN_DAC2, thread->dac2);
> +	mtspr(SPRN_DVC1, thread->dvc1);
> +	mtspr(SPRN_DVC2, thread->dvc2);
> +	mtspr(SPRN_DBCR0, thread->dbcr0);
> +	mtspr(SPRN_DBCR1, thread->dbcr1);
> +	mtspr(SPRN_DBCR2, thread->dbcr2);

We should probably look at dbginfo.num_condition_regs, =
dbginfo.num_instruction_bps, & dbginfo.num_data_bps and set these =
accordingly.

> +}
> +/*
> + * Unless neither the old or new thread are making use of the
> + * debug registers, set the debug registers from the values
> + * stored in the new thread.
> + */
> +static void switch_booke_debug_regs(struct thread_struct *new_thread)
> +{
> +	if ((current->thread.dbcr0 & DBCR0_IDM)
> +		|| (new_thread->dbcr0 & DBCR0_IDM))
> +			prime_debug_regs(new_thread);
> +}
> +#endif
> +
> int set_dabr(unsigned long dabr)
> {
> 	__get_cpu_var(current_dabr) =3D dabr;
> @@ -284,7 +358,7 @@ int set_dabr(unsigned long dabr)
> 		return ppc_md.set_dabr(dabr);
>=20
> 	/* XXX should we have a CPU_FTR_HAS_DABR ? */
> -#if defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> 	mtspr(SPRN_DAC1, dabr);
> #elif defined(CONFIG_PPC_BOOK3S)
> 	mtspr(SPRN_DABR, dabr);
> @@ -371,10 +445,8 @@ struct task_struct *__switch_to(struct =
task_struct *prev,
>=20
> #endif /* CONFIG_SMP */
>=20
> -#if defined(CONFIG_BOOKE)
> -	/* If new thread DAC (HW breakpoint) is the same then leave it =
*/
> -	if (new->thread.dabr)
> -		set_dabr(new->thread.dabr);
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	switch_booke_debug_regs(&new->thread);
> #else
> 	if (unlikely(__get_cpu_var(current_dabr) !=3D new->thread.dabr))
> 		set_dabr(new->thread.dabr);
> @@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
> 	printk("  CR: %08lx  XER: %08lx\n", regs->ccr, regs->xer);
> 	trap =3D TRAP(regs);
> 	if (trap =3D=3D 0x300 || trap =3D=3D 0x600)
> -#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> 		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, =
regs->dsisr);
> #else
> 		printk("DAR: "REG", DSISR: "REG"\n", regs->dar, =
regs->dsisr);
> @@ -568,14 +640,19 @@ void flush_thread(void)
>=20
> 	discard_lazy_cpu_state();
>=20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	/*
> +	 * flush_thread() is called on exec() to reset the
> +	 * thread's status. Set all debug regs back to their
> +	 * default values....Torez
> +	 */
> +	set_debug_reg_defaults(&current->thread);
> +#else
> 	if (current->thread.dabr) {
> 		current->thread.dabr =3D 0;
> 		set_dabr(0);
> -
> -#if defined(CONFIG_BOOKE)
> -		current->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W);
> -#endif
> 	}
> +#endif
> }
>=20
> void
> diff --git a/arch/powerpc/kernel/ptrace.c =
b/arch/powerpc/kernel/ptrace.c
> index 6be2ce0..6710a69 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -737,17 +737,25 @@ void user_disable_single_step(struct task_struct =
*task)
> 	struct pt_regs *regs =3D task->thread.regs;
>=20
> 	if (regs !=3D NULL) {
> -#if defined(CONFIG_BOOKE)
> -		/* If DAC don't clear DBCRO_IDM or MSR_DE */
> -		if (task->thread.dabr)
> -			task->thread.dbcr0 &=3D ~(DBCR0_IC | DBCR0_BT);
> -		else {
> -			task->thread.dbcr0 &=3D ~(DBCR0_IC | DBCR0_BT | =
DBCR0_IDM);
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +		/*
> +		 * The logic to disable single stepping should be as
> +		 * simple as turning off the Instruction Complete flag.
> +		 * And, after doing so, if all debug flags are off, turn
> +		 * off DBCR0(IDM) and MSR(DE) .... Torez
> +		 */
> +		task->thread.dbcr0 &=3D ~DBCR0_IC;
> +		/*
> +		 * Test to see if any of the DBCR_ACTIVE_EVENTS bits are =
set.
> +		 */
> +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> +					task->thread.dbcr1)) {
> +			/*
> +			 * All debug events were off.....
> +			 */
> +			task->thread.dbcr0 &=3D ~DBCR0_IDM;
> 			regs->msr &=3D ~MSR_DE;
> 		}
> -#elif defined(CONFIG_40x)
> -		task->thread.dbcr0 &=3D ~(DBCR0_IC | DBCR0_BT | =
DBCR0_IDM);
> -		regs->msr &=3D ~MSR_DE;
> #else
> 		regs->msr &=3D ~(MSR_SE | MSR_BE);
> #endif
> @@ -769,8 +777,7 @@ int ptrace_set_debugreg(struct task_struct *task, =
unsigned long addr,
> 	if ((data & ~0x7UL) >=3D TASK_SIZE)
> 		return -EIO;
>=20
> -#ifndef CONFIG_BOOKE
> -
> +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> 	/* For processors using DABR (i.e. 970), the bottom 3 bits are =
flags.
> 	 *  It was assumed, on previous implementations, that 3 bits =
were
> 	 *  passed together with the data address, fitting the design of =
the
> @@ -789,21 +796,22 @@ int ptrace_set_debugreg(struct task_struct =
*task, unsigned long addr,
>=20
> 	/* Move contents to the DABR register */
> 	task->thread.dabr =3D data;
> -
> -#endif
> -#if defined(CONFIG_BOOKE)
> -
> +#else
> 	/* As described above, it was assumed 3 bits were passed with =
the data
> 	 *  address, but we will assume only the mode bits will be =
passed
> 	 *  as to not cause alignment restrictions for DAC-based =
processors.
> 	 */
>=20
> 	/* DAC's hold the whole address without any mode flags */
> -	task->thread.dabr =3D data & ~0x3UL;
> -
> -	if (task->thread.dabr =3D=3D 0) {
> -		task->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | =
DBCR0_IDM);
> -		task->thread.regs->msr &=3D ~MSR_DE;
> +	task->thread.dac1 =3D data & ~0x3UL;
> +
> +	if (task->thread.dac1 =3D=3D 0) {
> +		dbcr_dac(task) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> +					task->thread.dbcr1)) {
> +			task->thread.regs->msr &=3D ~MSR_DE;
> +			task->thread.dbcr0 &=3D ~DBCR0_IDM;
> +		}
> 		return 0;
> 	}
>=20
> @@ -814,15 +822,15 @@ int ptrace_set_debugreg(struct task_struct =
*task, unsigned long addr,
>=20
> 	/* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
> 	   register */
> -	task->thread.dbcr0 =3D DBCR0_IDM;
> +	task->thread.dbcr0 |=3D DBCR0_IDM;
>=20
> 	/* Check for write and read flags and set DBCR0
> 	   accordingly */
> +	dbcr_dac(task) &=3D ~(DBCR_DAC1R|DBCR_DAC1W);
> 	if (data & 0x1UL)
> -		task->thread.dbcr0 |=3D DBSR_DAC1R;
> +		dbcr_dac(task) |=3D DBCR_DAC1R;
> 	if (data & 0x2UL)
> -		task->thread.dbcr0 |=3D DBSR_DAC1W;
> -
> +		dbcr_dac(task) |=3D DBCR_DAC1W;
> 	task->thread.regs->msr |=3D MSR_DE;
> #endif
> 	return 0;
> @@ -839,11 +847,324 @@ void ptrace_disable(struct task_struct *child)
> 	user_disable_single_step(child);
> }
>=20
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +static long set_intruction_bp(struct task_struct *child,
> +			      struct ppc_hw_breakpoint *bp_info)
> +{
> +	int slots_needed;
> +	int slot;
> +	int free_slot =3D 0;
> +
> +	/*
> +	 * Find an avalailable slot for the breakpoint.
> +	 * If possible, reserve consecutive slots, 1 & 2, for a range
> +	 * breakpoint.  (Can this be done simpler?)
> +	 */
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		slots_needed =3D 1;
> +	else
> +		slots_needed =3D 2;
> +
> +	if ((child->thread.dbcr0 & DBCR0_IAC1) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			if (child->thread.dbcr0 & DBCR0_IAC2) {
> +				slot =3D 1;
> +				goto found;
> +			}
> +			/* Try to save slots 1 & 2 for range */
> +			free_slot =3D 1;
> +		} else
> +			if ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0) =
{
> +				slot =3D 1;
> +				goto found;
> +			}
> +	} else if ((slots_needed =3D=3D 1) &&
> +		   ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0)) {
> +		slot =3D 2;
> +		goto found;
> +	}
> +	if ((child->thread.dbcr0 & DBCR0_IAC3) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		return -ENOSPC;
> +	} else if (slots_needed =3D=3D 2)
> +		return -ENOSPC;
> +	if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +		slot =3D 4;
> +	} else if (free_slot)
> +		slot =3D free_slot;
> +	else
> +		return -ENOSPC;

Need to factor in if # of IACs is only 2.

> +found:
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT) {
> +		switch (slot) {
> +			case 1:
> +				child->thread.iac1 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC1;
> +				break;
> +			case 2:
> +				child->thread.iac2 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC2;
> +				break;
> +			case 3:
> +				child->thread.iac3 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC3;
> +				break;
> +			case 4:
> +				child->thread.iac4 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC4;
> +				break;
> +		}
> +	} else if (slot =3D=3D 1) {
> +		child->thread.iac1 =3D bp_info->addr;
> +		child->thread.iac2 =3D bp_info->addr2;
> +		child->thread.dbcr0 |=3D (DBCR0_IAC1 | DBCR0_IAC2);
> +		if (bp_info->addr_mode =3D=3D =
PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_I;
> +	} else { /* slot =3D=3D 3 */
> +		child->thread.iac3 =3D bp_info->addr;
> +		child->thread.iac4 =3D bp_info->addr2;
> +		child->thread.dbcr0 |=3D (DBCR0_IAC3 | DBCR0_IAC4);
> +		if (bp_info->addr_mode =3D=3D =
PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_I;
> +	}
> +	child->thread.dbcr0 |=3D DBCR0_IDM;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return slot;
> +}
> +
> +static int del_instruction_bp(struct task_struct *child, int slot)
> +{
> +	switch (slot) {
> +	case 1:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M) {
> +			/* address range - clear slots 1 & 2 */
> +			child->thread.iac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC12M;
> +		}
> +		child->thread.iac1 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC1;
> +		break;
> +	case 2:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M)
> +			/* used in a range */
> +			return -EINVAL;
> +		child->thread.iac2 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +		break;
> +	case 3:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M) {
> +			/* address range - clear slots 3 & 4 */
> +			child->thread.iac4 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC34M;
> +		}
> +		child->thread.iac3 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC3;
> +		break;
> +	case 4:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M)
> +			/* Used in a range */
> +			return -EINVAL;
> +		child->thread.iac4 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int set_dac(struct task_struct *child, struct =
ppc_hw_breakpoint *bp_info)
> +{
> +	int byte_enable =3D
> +		(bp_info->condition_mode >> =
PPC_BREAKPOINT_CONDITION_BE_SHIFT)
> +		& 0xf;
> +	int condition_mode =3D
> +		bp_info->condition_mode & =
PPC_BREAKPOINT_CONDITION_AND_OR;
> +	int slot;
> +
> +	if (byte_enable && (condition_mode =3D=3D 0))
> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) =3D=3D 0) {
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +			dbcr_dac(child) |=3D DBCR_DAC1R;
> +		if (bp_info->trigger_type & =
PPC_BREAKPOINT_TRIGGER_WRITE)
> +			dbcr_dac(child) |=3D DBCR_DAC1W;
> +		child->thread.dac1 =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE
> +		if (byte_enable) {
> +			child->thread.dvc1 =3D
> +				(unsigned long)bp_info->condition_value;
> +			child->thread.dbcr2 |=3D
> +				((byte_enable << DBCR2_DVC1BE_SHIFT) |
> +				 (condition_mode << DBCR2_DVC1M_SHIFT));
> +		}
> +#endif
> +		slot =3D 1;
> +	} else if ((dbcr_dac(child) & (DBCR_DAC2R | DBCR_DAC2W)) =3D=3D =
0) {
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +			dbcr_dac(child) |=3D DBCR_DAC2R;
> +		if (bp_info->trigger_type & =
PPC_BREAKPOINT_TRIGGER_WRITE)
> +			dbcr_dac(child) |=3D DBCR_DAC2W;
> +		child->thread.dac2 =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE
> +		if (byte_enable) {
> +			child->thread.dvc2 =3D
> +				(unsigned long)bp_info->condition_value;
> +			child->thread.dbcr2 |=3D
> +				((byte_enable << DBCR2_DVC2BE_SHIFT) |
> +				 (condition_mode << DBCR2_DVC2M_SHIFT));
> +		}
> +#endif
> +		slot =3D 2;
> +	} else
> +		return -ENOSPC;
> +	child->thread.dbcr0 |=3D DBCR0_IDM;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return slot + 4;
> +}
> +
> +static int del_dac(struct task_struct *child, int slot)
> +{
> +	if (slot =3D=3D 1) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
> +			child->thread.dac1 =3D 0;
> +			child->thread.dac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~(DBCR0_DAC1R | =
DBCR0_DAC1W |
> +						 DBCR0_DAC2R | =
DBCR0_DAC2W);
> +			child->thread.dbcr2 &=3D ~DBCR2_DAC12MODE;
> +			return 0;
> +		}
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC1M | DBCR2_DVC1BE);
> +		child->thread.dvc1 =3D 0;
> +#endif
> +		child->thread.dac1 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +	} else if (slot =3D=3D 2) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE)
> +			/* Part of a range */
> +			return -EINVAL;
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC2M | DBCR2_DVC2BE);
> +		child->thread.dvc2 =3D 0;
> +#endif
> +		child->thread.dac2 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC2R | DBCR_DAC2W);
> +	} else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +#endif /* CONFIG_40x || CONFIG_BOOKE */
> +
> +#ifdef CONFIG_BOOKE
> +static int set_dac_range(struct task_struct *child,
> +			 struct ppc_hw_breakpoint *bp_info)
> +{
> +	int mode =3D bp_info->addr_mode & PPC_BREAKPOINT_MODE_MASK;
> +
> +	/* We don't allow range watchpoints to be used with DVC */
> +	if (bp_info->condition_mode && PPC_BREAKPOINT_CONDITION_BE_ALL)
> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if (child->thread.dbcr0 &
> +	    (DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W))
> +		return -ENOSPC;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1R | DBCR0_DAC2R | =
DBCR0_IDM);
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1W | DBCR0_DAC2W | =
DBCR0_IDM);
> +	child->thread.dac1 =3D bp_info->addr;
> +	child->thread.dac2 =3D bp_info->addr2;
> +	if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12R;
> +	else if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12RX;
> +	else	/* PPC_BREAKPOINT_MODE_MASK */
> +		child->thread.dbcr2  |=3D DBCR2_DAC12MASK;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return 5;
> +}
> +#endif /* CONFIG_BOOKE */
> +
> static long ppc_set_hwdebug(struct task_struct *child,
> 		     struct ppc_hw_breakpoint *bp_info)
> {
> +	if (bp_info->version !=3D 1)
> +		return -ENOTSUPP;
> +
> +#ifdef CONFIG_BOOKE
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode &
> +	     ~(PPC_BREAKPOINT_CONDITION_AND_OR |
> +	       PPC_BREAKPOINT_CONDITION_BE_ALL)))
> +		return -EINVAL;

We should add a sanity check for bp_info->condition_mode !=3D =
PPC_BREAKPOINT_CONDITION_NONE if dbginfo.num_condition_regs =3D 0.

> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D =
PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return set_dac(child, bp_info);
> +
> +	return set_dac_range(child, bp_info);
> +#elif defined(CONFIG_40x)
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode !=3D =
PPC_BREAKPOINT_CONDITION_NONE))
> +		return -EINVAL;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D =
PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +	if (bp_info->addr_mode !=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return -EINVAL;
> +
> +	return set_dac(child, bp_info);
> +#else
> 	/*
> -	 * We currently support one data breakpoint
> +	 * We only support one data breakpoint
> 	 */
> 	if (((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) =3D=3D =
0) ||
> 	    ((bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) !=3D =
0) ||
> @@ -859,28 +1180,35 @@ static long ppc_set_hwdebug(struct task_struct =
*child,
> 		return -EIO;
>=20
> 	child->thread.dabr =3D (unsigned long)bp_info->addr;
> -#ifdef CONFIG_BOOKE
> -	child->thread.dbcr0 =3D DBCR0_IDM;
> -	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> -		child->thread.dbcr0 |=3D DBSR_DAC1R;
> -	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> -		child->thread.dbcr0 |=3D DBSR_DAC1W;
> -	child->thread.regs->msr |=3D MSR_DE;
> -#endif
> 	return 1;
> +#endif
> }
>=20
> static long ppc_del_hwdebug(struct task_struct *child, long addr, long =
data)
> {
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +	int rc;
> +
> +	if (data <=3D 4)
> +		rc =3D del_instruction_bp(child, (int)data);
> +	else
> +		rc =3D del_dac(child, (int)data - 4);
> +
> +	if (!rc) {
> +		if (!DBCR_ACTIVE_EVENTS(child->thread.dbcr0,
> +					child->thread.dbcr1)) {
> +			child->thread.dbcr0 &=3D ~DBCR0_IDM;
> +			child->thread.regs->msr &=3D ~MSR_DE;
> +		}
> +	}
> +	return rc;
> +#else
> 	if ((data !=3D 1) || (child->thread.dabr =3D=3D 0))
> 		return -EINVAL;
>=20
> 	child->thread.dabr =3D 0;
> -#ifdef CONFIG_BOOKE
> -	child->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> -	child->thread.regs->msr &=3D ~MSR_DE;
> -#endif
> 	return 0;
> +#endif
> }
>=20
> /*
> @@ -980,16 +1308,36 @@ long arch_ptrace(struct task_struct *child, =
long request, long addr, long data)
> 		struct ppc_debug_info dbginfo;
>=20
> 		dbginfo.version =3D 1;
> +#ifdef CONFIG_BOOKE
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 2;
> +		dbginfo.data_bp_alignment =3D 0;
> +		dbginfo.sizeof_condition =3D 4;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK |
> +				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
> +#elif defined(CONFIG_40x)
> +		/*
> +		 * I don't know how the DVCs work on 40x, I'm not going
> +		 * to support it now. -- Shaggy
> +		 */
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 0;
> +		dbginfo.data_bp_alignment =3D 0;
> +		dbginfo.sizeof_condition =3D 0;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK;
> +#else
> 		dbginfo.num_instruction_bps =3D 0;
> 		dbginfo.num_data_bps =3D 1;
> 		dbginfo.num_condition_regs =3D 0;
> -#ifdef CONFIG_PPC64
> 		dbginfo.data_bp_alignment =3D 8;
> -#else
> -		dbginfo.data_bp_alignment =3D 0;
> -#endif
> 		dbginfo.sizeof_condition =3D 0;
> 		dbginfo.features =3D 0;
> +#endif

This is a bit ugly and BOOKE 64 parts probably don't have the 8 byte =
alignment.

Should we push some of this into cputable?

>=20
> 		if (!access_ok(VERIFY_WRITE, data,
> 			       sizeof(struct ppc_debug_info)))
> @@ -1025,8 +1373,13 @@ long arch_ptrace(struct task_struct *child, =
long request, long addr, long data)
> 		/* We only support one DABR and no IABRS at the moment =
*/
> 		if (addr > 0)
> 			break;
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +		ret =3D put_user(child->thread.dac1,
> +			       (unsigned long __user *)data);
> +#else
> 		ret =3D put_user(child->thread.dabr,
> 			       (unsigned long __user *)data);
> +#endif
> 		break;
> 	}
>=20
> diff --git a/arch/powerpc/kernel/signal.c =
b/arch/powerpc/kernel/signal.c
> index 00b5078..94df779 100644
> --- a/arch/powerpc/kernel/signal.c
> +++ b/arch/powerpc/kernel/signal.c
> @@ -140,17 +140,15 @@ static int do_signal_pending(sigset_t *oldset, =
struct pt_regs *regs)
> 		return 0;               /* no signals delivered */
> 	}
>=20
> +#if !(defined(CONFIG_BOOKE) || defined(CONFIG_40x))
>         /*
> 	 * Reenable the DABR before delivering the signal to
> 	 * user space. The DABR will have been cleared if it
> 	 * triggered inside the kernel.
> 	 */
> -	if (current->thread.dabr) {
> +	if (current->thread.dabr)
> 		set_dabr(current->thread.dabr);
> -#if defined(CONFIG_BOOKE)
> -		mtspr(SPRN_DBCR0, current->thread.dbcr0);
> #endif
> -	}
>=20
> 	if (is32) {
>         	if (ka.sa.sa_flags & SA_SIGINFO)
> diff --git a/arch/powerpc/kernel/signal_32.c =
b/arch/powerpc/kernel/signal_32.c
> index d670429..6cc6e81 100644
> --- a/arch/powerpc/kernel/signal_32.c
> +++ b/arch/powerpc/kernel/signal_32.c
> @@ -1092,8 +1092,12 @@ int sys_debug_setcontext(struct ucontext __user =
*ctx,
> 				new_msr |=3D MSR_DE;
> 				new_dbcr0 |=3D (DBCR0_IDM | DBCR0_IC);
> 			} else {
> -				new_msr &=3D ~MSR_DE;
> -				new_dbcr0 &=3D ~(DBCR0_IDM | DBCR0_IC);
> +				new_dbcr0 &=3D ~DBCR0_IC;
> +				if (!DBCR_ACTIVE_EVENTS(new_dbcr0,
> +						current->thread.dbcr1)) =
{
> +					new_msr &=3D ~MSR_DE;
> +					new_dbcr0 &=3D ~DBCR0_IDM;
> +				}
> 			}
> #else
> 			if (op.dbg_value)
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index a81c743..d919571 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1016,9 +1016,63 @@ void SoftwareEmulation(struct pt_regs *regs)
> #endif /* CONFIG_8xx */
>=20
> #if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +static void handle_debug(struct pt_regs *regs, unsigned long =
debug_status)
> +{
> +	int changed =3D 0;
> +	/*
> +	 * Determine the cause of the debug event, clear the
> +	 * event flags and send a trap to the handler. Torez
> +	 */
> +	if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
> +		dbcr_dac(current) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +		do_send_trap(regs, mfspr(SPRN_DAC1), debug_status, =
TRAP_HWBKPT,
> +			     5);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & (DBSR_DAC2R | DBSR_DAC2W)) {
> +		dbcr_dac(current) &=3D ~(DBCR_DAC2R | DBCR_DAC2W);
> +		do_send_trap(regs, mfspr(SPRN_DAC2), debug_status, =
TRAP_HWBKPT,
> +			     6);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC1) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC1;
> +		do_send_trap(regs, mfspr(SPRN_IAC1), debug_status, =
TRAP_HWBKPT,
> +			     1);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC2) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +		do_send_trap(regs, mfspr(SPRN_IAC2), debug_status, =
TRAP_HWBKPT,
> +			     2);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC3) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC3;
> +		do_send_trap(regs, mfspr(SPRN_IAC3), debug_status, =
TRAP_HWBKPT,
> +			     3);
> +		changed |=3D 0x01;
> +	}  else if (debug_status & DBSR_IAC4) {
> +		current->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +		do_send_trap(regs, mfspr(SPRN_IAC4), debug_status, =
TRAP_HWBKPT,
> +			     4);
> +		changed |=3D 0x01;
> +	}
> +	/*
> +	 * At the point this routine was called, the MSR(DE) was turned =
off.
> +	 * Check all other debug flags and see if that bit needs to be =
turned
> +	 * back on or not.
> +	 */
> +	if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0, =
current->thread.dbcr1))
> +		regs->msr |=3D MSR_DE;
> +	else
> +		/* Make sure the IDM flag is off */
> +		current->thread.dbcr0 &=3D ~DBCR0_IDM;
> +
> +	if (changed & 0x01)
> +		mtspr(SPRN_DBCR0, current->thread.dbcr0);
> +}
>=20
> void __kprobes DebugException(struct pt_regs *regs, unsigned long =
debug_status)
> {
> +	current->thread.dbsr =3D debug_status;
> +
> 	/* Hack alert: On BookE, Branch Taken stops on the branch =
itself, while
> 	 * on server, it stops on the target of the branch. In order to =
simulate
> 	 * the server behaviour, we thus restart right away with a =
single step
> @@ -1062,27 +1116,21 @@ void __kprobes DebugException(struct pt_regs =
*regs, unsigned long debug_status)
> 		if (debugger_sstep(regs))
> 			return;
>=20
> -		if (user_mode(regs))
> -			current->thread.dbcr0 &=3D ~(DBCR0_IC);
> -
> -		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
> -	} else if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
> -		regs->msr &=3D ~MSR_DE;
> -
> 		if (user_mode(regs)) {
> -			current->thread.dbcr0 &=3D ~(DBSR_DAC1R | =
DBSR_DAC1W |
> -								=
DBCR0_IDM);
> -		} else {
> -			/* Disable DAC interupts */
> -			mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & =
~(DBSR_DAC1R |
> -						DBSR_DAC1W | =
DBCR0_IDM));
> -
> -			/* Clear the DAC event */
> -			mtspr(SPRN_DBSR, (DBSR_DAC1R | DBSR_DAC1W));
> +			current->thread.dbcr0 &=3D ~DBCR0_IC;
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +			if (DBCR_ACTIVE_EVENTS(current->thread.dbcr0,
> +					       current->thread.dbcr1))
> +				regs->msr |=3D MSR_DE;
> +			else
> +				/* Make sure the IDM bit is off */
> +				current->thread.dbcr0 &=3D ~DBCR0_IDM;
> +#endif
> 		}
> -		/* Setup and send the trap to the handler */
> -		do_dabr(regs, mfspr(SPRN_DAC1), debug_status);
> -	}
> +
> +		_exception(SIGTRAP, regs, TRAP_TRACE, regs->nip);
> +	} else
> +		handle_debug(regs, debug_status);
> }
> #endif /* CONFIG_4xx || CONFIG_BOOKE */
>=20
>=20
> --=20
> Dave Kleikamp
> IBM Linux Technology Center
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC:PATCH 01/03] powerpc: Extended ptrace interface

[Kumar Gala]

On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:

> powerpc: Extended ptrace interface
>=20
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>=20
> Add a new extended ptrace interface so that user-space has a single
> interface for powerpc, without having to know the specific layout
> of the debug registers.
>=20
> Implement:
> PPC_PTRACE_GETHWDEBUGINFO
> PPC_PTRACE_SETHWDEBUG
> PPC_PTRACE_DELHWDEBUG
>=20
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
>=20
> arch/powerpc/include/asm/ptrace.h |   75 =
++++++++++++++++++++++++++++++++
> arch/powerpc/kernel/ptrace.c      |   88 =
+++++++++++++++++++++++++++++++++++++
> 2 files changed, 163 insertions(+), 0 deletions(-)
>=20
>=20
> diff --git a/arch/powerpc/include/asm/ptrace.h =
b/arch/powerpc/include/asm/ptrace.h
> index 8c34149..7ae887b 100644
> --- a/arch/powerpc/include/asm/ptrace.h
> +++ b/arch/powerpc/include/asm/ptrace.h
> @@ -24,6 +24,12 @@
>  * 2 of the License, or (at your option) any later version.
>  */
>=20
> +#ifdef __KERNEL__
> +#include <linux/types.h>
> +#else
> +#include <stdint.h>
> +#endif
> +
> #ifndef __ASSEMBLY__
>=20
> struct pt_regs {
> @@ -292,4 +298,73 @@ extern void user_disable_single_step(struct =
task_struct *);
>=20
> #define PTRACE_SINGLEBLOCK	0x100	/* resume execution until next =
branch */
>=20
> +#define PPC_PTRACE_GETHWDBGINFO	0x89
> +#define PPC_PTRACE_SETHWDEBUG	0x88
> +#define PPC_PTRACE_DELHWDEBUG	0x87
> +
> +#ifndef __ASSEMBLY__
> +
> +struct ppc_debug_info {
> +	uint32_t version;		/* Only version 1 exists to date =
*/
> +	uint32_t num_instruction_bps;
> +	uint32_t num_data_bps;
> +	uint32_t num_condition_regs;
> +	uint32_t data_bp_alignment;
> +	uint32_t sizeof_condition;	/* size of the DVC register */
> +	uint64_t features;
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +/*
> + * features will have bits indication whether there is support for:
> + */
> +#define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> +#define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> +#define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> +#define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8

Pad these out.

#define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x0000000000000001
#define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x0000000000000002

etc..


> +
> +#ifndef __ASSEMBLY__
> +
> +struct ppc_hw_breakpoint {
> +	uint32_t version;		/* currently, version must be 1 =
*/
> +	uint32_t trigger_type;		/* only some combinations =
allowed */
> +	uint32_t addr_mode;		/* address match mode */
> +	uint32_t condition_mode;	/* break/watchpoint condition =
flags */
> +	uint64_t addr;			/* break/watchpoint address */
> +	uint64_t addr2;			/* range end or mask */
> +	uint64_t condition_value;	/* contents of the DVC register =
*/
> +};
> +
> +#endif /* __ASSEMBLY__ */
> +
> +/*
> + * Trigger Type
> + */
> +#define PPC_BREAKPOINT_TRIGGER_EXECUTE	0x1
> +#define PPC_BREAKPOINT_TRIGGER_READ	0x2
> +#define PPC_BREAKPOINT_TRIGGER_WRITE	0x4
> +#define PPC_BREAKPOINT_TRIGGER_RW	0x6

(ditto on the padding)

> +
> +/*
> + * Address Mode
> + */
> +#define PPC_BREAKPOINT_MODE_EXACT		0x0
> +#define PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE	0x1
> +#define PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE	0x2
> +#define PPC_BREAKPOINT_MODE_MASK		0x3
> +

(ditto on the padding)

> +/*
> + * Condition Mode
> + */
> +#define PPC_BREAKPOINT_CONDITION_NONE	0x0
> +#define PPC_BREAKPOINT_CONDITION_AND	0x1
> +#define PPC_BREAKPOINT_CONDITION_EXACT	0x1
> +#define PPC_BREAKPOINT_CONDITION_OR	0x2
> +#define PPC_BREAKPOINT_CONDITION_AND_OR	0x3

(ditto on the padding)

> +#define PPC_BREAKPOINT_CONDITION_BE_ALL	0x00ff0000
> +#define PPC_BREAKPOINT_CONDITION_BE_SHIFT	16
> +#define PPC_BREAKPOINT_CONDITION_BE(n)	\
> +	(1<<((n)+PPC_BREAKPOINT_CONDITION_BE_SHIFT))
> +
> #endif /* _ASM_POWERPC_PTRACE_H */
> diff --git a/arch/powerpc/kernel/ptrace.c =
b/arch/powerpc/kernel/ptrace.c
> index ef14988..6be2ce0 100644
> --- a/arch/powerpc/kernel/ptrace.c
> +++ b/arch/powerpc/kernel/ptrace.c
> @@ -839,6 +839,50 @@ void ptrace_disable(struct task_struct *child)
> 	user_disable_single_step(child);
> }
>=20
> +static long ppc_set_hwdebug(struct task_struct *child,
> +		     struct ppc_hw_breakpoint *bp_info)
> +{
> +	/*
> +	 * We currently support one data breakpoint
> +	 */
> +	if (((bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_RW) =3D=3D =
0) ||
> +	    ((bp_info->trigger_type & ~PPC_BREAKPOINT_TRIGGER_RW) !=3D =
0) ||
> +	    (bp_info->trigger_type !=3D PPC_BREAKPOINT_TRIGGER_WRITE) ||
> +	    (bp_info->addr_mode !=3D PPC_BREAKPOINT_MODE_EXACT) ||
> +	    (bp_info->condition_mode !=3D =
PPC_BREAKPOINT_CONDITION_NONE))
> +		return -EINVAL;
> +
> +	if (child->thread.dabr)
> +		return -ENOSPC;
> +
> +	if ((unsigned long)bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	child->thread.dabr =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE
> +	child->thread.dbcr0 =3D DBCR0_IDM;
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +		child->thread.dbcr0 |=3D DBSR_DAC1R;
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +		child->thread.dbcr0 |=3D DBSR_DAC1W;
> +	child->thread.regs->msr |=3D MSR_DE;
> +#endif
> +	return 1;
> +}
> +
> +static long ppc_del_hwdebug(struct task_struct *child, long addr, =
long data)
> +{
> +	if ((data !=3D 1) || (child->thread.dabr =3D=3D 0))
> +		return -EINVAL;
> +
> +	child->thread.dabr =3D 0;
> +#ifdef CONFIG_BOOKE
> +	child->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> +	child->thread.regs->msr &=3D ~MSR_DE;
> +#endif
> +	return 0;
> +}
> +
> /*
>  * Here are the old "legacy" powerpc specific getregs/setregs ptrace =
calls,
>  * we mark them as obsolete now, they will be removed in a future =
version
> @@ -932,6 +976,50 @@ long arch_ptrace(struct task_struct *child, long =
request, long addr, long data)
> 		break;
> 	}
>=20
> +	case PPC_PTRACE_GETHWDBGINFO: {
> +		struct ppc_debug_info dbginfo;
> +
> +		dbginfo.version =3D 1;
> +		dbginfo.num_instruction_bps =3D 0;
> +		dbginfo.num_data_bps =3D 1;
> +		dbginfo.num_condition_regs =3D 0;
> +#ifdef CONFIG_PPC64
> +		dbginfo.data_bp_alignment =3D 8;
> +#else
> +		dbginfo.data_bp_alignment =3D 0;
> +#endif
> +		dbginfo.sizeof_condition =3D 0;
> +		dbginfo.features =3D 0;
> +
> +		if (!access_ok(VERIFY_WRITE, data,
> +			       sizeof(struct ppc_debug_info)))
> +			return -EFAULT;
> +		ret =3D __copy_to_user((struct ppc_debug_info __user =
*)data,
> +				     &dbginfo, sizeof(struct =
ppc_debug_info)) ?
> +		      -EFAULT : 0;
> +		break;
> +	}
> +
> +	case PPC_PTRACE_SETHWDEBUG: {
> +		struct ppc_hw_breakpoint bp_info;
> +
> +		if (!access_ok(VERIFY_READ, data,
> +			       sizeof(struct ppc_hw_breakpoint)))
> +			return -EFAULT;
> +		ret =3D __copy_from_user(&bp_info,
> +				       (struct ppc_hw_breakpoint __user =
*)data,
> +				       sizeof(struct ppc_hw_breakpoint)) =
?
> +		      -EFAULT : 0;
> +		if (!ret)
> +			ret =3D ppc_set_hwdebug(child, &bp_info);
> +		break;
> +	}
> +
> +	case PPC_PTRACE_DELHWDEBUG: {
> +		ret =3D ppc_del_hwdebug(child, addr, data);
> +		break;
> +	}
> +
> 	case PTRACE_GET_DEBUGREG: {
> 		ret =3D -EINVAL;
> 		/* We only support one DABR and no IABRS at the moment =
*/
>=20
> --=20
> Dave Kleikamp
> IBM Linux Technology Center
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[David Gibson]

On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
> powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
>=20
> From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
>=20
> This patch defines context switch and trap related functionality
> for BookE specific Debug Registers. It adds support to ptrace()
> for setting and getting BookE related Debug Registers
>=20
> Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
> Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
> Cc: David Gibson <dwg@au1.ibm.com>
> Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
> ---
>=20
>  arch/powerpc/include/asm/system.h |    2=20
>  arch/powerpc/kernel/process.c     |  109 ++++++++-
>  arch/powerpc/kernel/ptrace.c      |  435 +++++++++++++++++++++++++++++++=
+++---
>  arch/powerpc/kernel/signal.c      |    6 -
>  arch/powerpc/kernel/signal_32.c   |    8 +
>  arch/powerpc/kernel/traps.c       |   86 ++++++-
>  6 files changed, 564 insertions(+), 82 deletions(-)

[snip]
> +void do_send_trap(struct pt_regs *regs, unsigned long address,
> +		  unsigned long error_code, int signal_code, int errno)
> +{
> +	siginfo_t info;
> +
> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
> +			11, SIGSEGV) =3D=3D NOTIFY_STOP)
> +		return;
> +
> +	/* Deliver the signal to userspace */
> +	info.si_signo =3D SIGTRAP;
> +	info.si_errno =3D errno;

We're using the errno siginfo field, but not for an errno, so possibly
the parameter should be called something else.

[snip]
> +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
>  void do_dabr(struct pt_regs *regs, unsigned long address,
>  		    unsigned long error_code)
>  {
> @@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long add=
ress,
>  	if (debugger_dabr_match(regs))
>  		return;
> =20
> -	/* Clear the DAC and struct entries.  One shot trigger */
> -#if defined(CONFIG_BOOKE)
> -	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> -							| DBCR0_IDM));
> -#endif
> -
>  	/* Clear the DABR */
>  	set_dabr(0);

Uh.. does this imply we're keeping the one-shot behaviour for
new-style breakpoints?  To me the interface really suggests they're
persistent, although dealing with the semantics of that at signal time
can get curly.

> =20
> @@ -273,9 +285,71 @@ void do_dabr(struct pt_regs *regs, unsigned long add=
ress,
>  	info.si_addr =3D (void __user *)address;
>  	force_sig_info(SIGTRAP, &info, current);
>  }
> +#endif
> =20
>  static DEFINE_PER_CPU(unsigned long, current_dabr);
> =20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +/*
> + * Set the debug registers back to their default "safe" values.
> + */
> +static void set_debug_reg_defaults(struct thread_struct *thread)
> +{
> +	thread->iac1 =3D thread->iac2 =3D thread->iac3 =3D thread->iac4 =3D 0;
> +	thread->dac1 =3D thread->dac2 =3D 0;
> +	thread->dvc1 =3D thread->dvc2 =3D 0;
> +	/*
> +	 * reset the DBCR0, DBCR1 and DBCR2 registers. All bits with
> +	 * the exception of the reserved bits should be cleared out
> +	 * and set to 0.
> +	 *
> +	 * For the DBCR0 register, the reserved bits are bits 17:30.
> +	 * Reserved bits for DBCR1 are bits 10:14 and bits 26:30.
> +	 * And, bits 10:11 for DBCR2.
> +	 */
> +	thread->dbcr0 =3D DBCR0_BASE_REG_VALUE;

Since this is now the only place it's used, I'd pull the
BASE_REG_VALUE constant inline here.  Makes the actual definition sit
next to the describing comment which is a bonus.

> +	/*
> +	 * First clear all "non reserved" bits from DBCR1 then initialize reg
> +	 * to force User/Supervisor bits to b11 (user-only MSR[PR]=3D1) and
> +	 * Effective/Real * bits to b10 (trap only if IS=3D=3D0)
> +	 */
> +	thread->dbcr1 =3D DBCR1_BASE_REG_VALUE;
> +	/*
> +	 * Force Data Address Compare User/Supervisor bits to be User-only
> +	 * (0b11 MSR[PR]=3D1) and set all other bits in DBCR2 register to be 0.
> +	 * This sets the Data Address Compare Effective/Real bits to be 0b00
> +	 * (Effective, MSR[DS]=3Ddon't care).
> +	 */
> +	thread->dbcr2 =3D DBCR2_BASE_REG_VALUE;
> +}
> +
> +static void prime_debug_regs(struct thread_struct *thread)
> +{
> +	mtspr(SPRN_IAC1, thread->iac1);
> +	mtspr(SPRN_IAC2, thread->iac2);
> +	mtspr(SPRN_IAC3, thread->iac3);
> +	mtspr(SPRN_IAC4, thread->iac4);
> +	mtspr(SPRN_DAC1, thread->dac1);
> +	mtspr(SPRN_DAC2, thread->dac2);
> +	mtspr(SPRN_DVC1, thread->dvc1);
> +	mtspr(SPRN_DVC2, thread->dvc2);
> +	mtspr(SPRN_DBCR0, thread->dbcr0);
> +	mtspr(SPRN_DBCR1, thread->dbcr1);
> +	mtspr(SPRN_DBCR2, thread->dbcr2);

As Josh pointed out, you'll need to be a little more careful here to
only mtspr() registers which actually exist on the current platform.
We may be better off only implementing the new interface for BookE in
the first cut.

> +}
> +/*
> + * Unless neither the old or new thread are making use of the
> + * debug registers, set the debug registers from the values
> + * stored in the new thread.
> + */
> +static void switch_booke_debug_regs(struct thread_struct *new_thread)
> +{
> +	if ((current->thread.dbcr0 & DBCR0_IDM)
> +		|| (new_thread->dbcr0 & DBCR0_IDM))
> +			prime_debug_regs(new_thread);
> +}
> +#endif
> +
>  int set_dabr(unsigned long dabr)
>  {
>  	__get_cpu_var(current_dabr) =3D dabr;
> @@ -284,7 +358,7 @@ int set_dabr(unsigned long dabr)
>  		return ppc_md.set_dabr(dabr);
> =20
>  	/* XXX should we have a CPU_FTR_HAS_DABR ? */
> -#if defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
>  	mtspr(SPRN_DAC1, dabr);

Uh.. this would seem to be wrong.  set_dabr(0) is called from the
debug exception - but nowadays that could be tiggered by a DAC other
than DAC1.

>  #elif defined(CONFIG_PPC_BOOK3S)
>  	mtspr(SPRN_DABR, dabr);
> @@ -371,10 +445,8 @@ struct task_struct *__switch_to(struct task_struct *=
prev,
> =20
>  #endif /* CONFIG_SMP */
> =20
> -#if defined(CONFIG_BOOKE)
> -	/* If new thread DAC (HW breakpoint) is the same then leave it */
> -	if (new->thread.dabr)
> -		set_dabr(new->thread.dabr);
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	switch_booke_debug_regs(&new->thread);
>  #else
>  	if (unlikely(__get_cpu_var(current_dabr) !=3D new->thread.dabr))
>  		set_dabr(new->thread.dabr);
> @@ -514,7 +586,7 @@ void show_regs(struct pt_regs * regs)
>  	printk("  CR: %08lx  XER: %08lx\n", regs->ccr, regs->xer);
>  	trap =3D TRAP(regs);
>  	if (trap =3D=3D 0x300 || trap =3D=3D 0x600)
> -#if defined(CONFIG_4xx) || defined(CONFIG_BOOKE)
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
>  		printk("DEAR: "REG", ESR: "REG"\n", regs->dar, regs->dsisr);
>  #else
>  		printk("DAR: "REG", DSISR: "REG"\n", regs->dar, regs->dsisr);
> @@ -568,14 +640,19 @@ void flush_thread(void)
> =20
>  	discard_lazy_cpu_state();
> =20
> +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> +	/*
> +	 * flush_thread() is called on exec() to reset the
> +	 * thread's status. Set all debug regs back to their
> +	 * default values....Torez
> +	 */
> +	set_debug_reg_defaults(&current->thread);

Better to define this function as a nop on non-BookE.

[snip]
> +#else
>  	/* As described above, it was assumed 3 bits were passed with the data
>  	 *  address, but we will assume only the mode bits will be passed
>  	 *  as to not cause alignment restrictions for DAC-based processors.
>  	 */
> =20
>  	/* DAC's hold the whole address without any mode flags */
> -	task->thread.dabr =3D data & ~0x3UL;
> -
> -	if (task->thread.dabr =3D=3D 0) {
> -		task->thread.dbcr0 &=3D ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> -		task->thread.regs->msr &=3D ~MSR_DE;
> +	task->thread.dac1 =3D data & ~0x3UL;
> +
> +	if (task->thread.dac1 =3D=3D 0) {
> +		dbcr_dac(task) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> +					task->thread.dbcr1)) {
> +			task->thread.regs->msr &=3D ~MSR_DE;
> +			task->thread.dbcr0 &=3D ~DBCR0_IDM;
> +		}
>  		return 0;
>  	}

Ok, so effectively the old ptrace method of setting the DABR acts as a
bypass to set DAC1, rather than having the old interface being
implemented via the new interface.  This has some weirdness - you can
clobber a new-style breakpoint in DAC1 using the old interface, for
example.  Still, it might be the simplest approach for a first cut.

What *is* a problem though, is that this means that the SIGTRAP will
always give a slot number, even for a breakpoint established using the
old interface.  Part of the idea of encoding the registered breakpoint
number in the siginfo was to be able to distinguish between old-style
and new-style breakpoints at trap time.

[snip]
> +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> +static long set_intruction_bp(struct task_struct *child,
> +			      struct ppc_hw_breakpoint *bp_info)
> +{
> +	int slots_needed;
> +	int slot;
> +	int free_slot =3D 0;
> +
> +	/*
> +	 * Find an avalailable slot for the breakpoint.
> +	 * If possible, reserve consecutive slots, 1 & 2, for a range
> +	 * breakpoint.  (Can this be done simpler?)
> +	 */
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		slots_needed =3D 1;
> +	else
> +		slots_needed =3D 2;

Ugh.  This logic is pretty hard to follow.  I'm having trouble
convincing myself it gets all the cases right, although I haven't
found something definitely wrong.

> +	if ((child->thread.dbcr0 & DBCR0_IAC1) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			if (child->thread.dbcr0 & DBCR0_IAC2) {
> +				slot =3D 1;
> +				goto found;
> +			}
> +			/* Try to save slots 1 & 2 for range */
> +			free_slot =3D 1;
> +		} else
> +			if ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0) {
> +				slot =3D 1;
> +				goto found;
> +			}
> +	} else if ((slots_needed =3D=3D 1) &&
> +		   ((child->thread.dbcr0 & DBCR0_IAC2) =3D=3D 0)) {
> +		slot =3D 2;
> +		goto found;
> +	}
> +	if ((child->thread.dbcr0 & DBCR0_IAC3) =3D=3D 0) {
> +		if (slots_needed =3D=3D 1) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +			slot =3D 3;
> +			goto found;
> +		}
> +		return -ENOSPC;
> +	} else if (slots_needed =3D=3D 2)
> +		return -ENOSPC;
> +	if ((child->thread.dbcr0 & DBCR0_IAC4) =3D=3D 0) {
> +		slot =3D 4;
> +	} else if (free_slot)
> +		slot =3D free_slot;
> +	else
> +		return -ENOSPC;
> +found:
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT) {
> +		switch (slot) {
> +			case 1:
> +				child->thread.iac1 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC1;
> +				break;
> +			case 2:
> +				child->thread.iac2 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC2;
> +				break;
> +			case 3:
> +				child->thread.iac3 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC3;
> +				break;
> +			case 4:
> +				child->thread.iac4 =3D bp_info->addr;
> +				child->thread.dbcr0 |=3D DBCR0_IAC4;
> +				break;

By using an array instead of individual iac1..4 in the thread struct,
plus a suitable macro to generate the right bit, you could get rid of
this nasty switch.

> +		}
> +	} else if (slot =3D=3D 1) {
> +		child->thread.iac1 =3D bp_info->addr;
> +		child->thread.iac2 =3D bp_info->addr2;

You test that addr is a user address in the caller, but I don't think
you ever check addr2.

> +		child->thread.dbcr0 |=3D (DBCR0_IAC1 | DBCR0_IAC2);

Uh.. I thought for range breakpoints you only needed to enable the bit
for the first of the two IACs (plus the range enable bit, of course).

> +		if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC12M_I;
> +	} else { /* slot =3D=3D 3 */
> +		child->thread.iac3 =3D bp_info->addr;
> +		child->thread.iac4 =3D bp_info->addr2;
> +		child->thread.dbcr0 |=3D (DBCR0_IAC3 | DBCR0_IAC4);
> +		if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_X;
> +		else
> +			dbcr_iac_range(child) |=3D DBCR_IAC34M_I;
> +	}
> +	child->thread.dbcr0 |=3D DBCR0_IDM;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return slot;

Yeah, ok.  I think the idea of using the register number as the
breakpoint identifier is limiting and we'll need to do something else
eventually.  Nonetheless, since userspace should be treating it as
opaque, it will do for a first cut and we can rework this later.

> +}
> +
> +static int del_instruction_bp(struct task_struct *child, int slot)
> +{
> +	switch (slot) {
> +	case 1:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M) {
> +			/* address range - clear slots 1 & 2 */
> +			child->thread.iac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC12M;
> +		}
> +		child->thread.iac1 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC1;
> +		break;

Uh.. as far as I can tell, you won't return an error if you try to
clear a breakpoint from a slot that isn't used.  That seems like
incorrect behaviour.

> +	case 2:
> +		if (dbcr_iac_range(child) & DBCR_IAC12M)
> +			/* used in a range */
> +			return -EINVAL;
> +		child->thread.iac2 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC2;
> +		break;
> +	case 3:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M) {
> +			/* address range - clear slots 3 & 4 */
> +			child->thread.iac4 =3D 0;
> +			child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +			dbcr_iac_range(child) &=3D ~DBCR_IAC34M;
> +		}
> +		child->thread.iac3 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC3;
> +		break;
> +	case 4:
> +		if (dbcr_iac_range(child) & DBCR_IAC34M)
> +			/* Used in a range */
> +			return -EINVAL;
> +		child->thread.iac4 =3D 0;
> +		child->thread.dbcr0 &=3D ~DBCR0_IAC4;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
> +static int set_dac(struct task_struct *child, struct ppc_hw_breakpoint *=
bp_info)
> +{
> +	int byte_enable =3D
> +		(bp_info->condition_mode >> PPC_BREAKPOINT_CONDITION_BE_SHIFT)
> +		& 0xf;
> +	int condition_mode =3D
> +		bp_info->condition_mode & PPC_BREAKPOINT_CONDITION_AND_OR;

Using the fact that AND_OR is also equal to a suitable mask is a bit
overly subtle.  Better to use a separately defined mask constant.

> +	int slot;
> +
> +	if (byte_enable && (condition_mode =3D=3D 0))
> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) =3D=3D 0) {
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +			dbcr_dac(child) |=3D DBCR_DAC1R;
> +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +			dbcr_dac(child) |=3D DBCR_DAC1W;
> +		child->thread.dac1 =3D (unsigned long)bp_info->addr;
> +#ifdef CONFIG_BOOKE

Better to have a runtime feature bit test here, than use an #ifdef to
distinguish the 40x and BookE cases.  Plus, you should return an error
if the user attempts to use a feature not supported on this hardware,
which doesn't seem to happen here.

=18[snip]
> +static int del_dac(struct task_struct *child, int slot)
> +{

Again, you never seem to generate an error if you attempt to delete a
watchpoint that was never set.

> +	if (slot =3D=3D 1) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE) {
> +			child->thread.dac1 =3D 0;
> +			child->thread.dac2 =3D 0;
> +			child->thread.dbcr0 &=3D ~(DBCR0_DAC1R | DBCR0_DAC1W |
> +						 DBCR0_DAC2R | DBCR0_DAC2W);
> +			child->thread.dbcr2 &=3D ~DBCR2_DAC12MODE;
> +			return 0;
> +		}
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC1M | DBCR2_DVC1BE);
> +		child->thread.dvc1 =3D 0;

Since this will just clear fields which are never used on 40x, you
shouldn't need the ifdef.

> +#endif
> +		child->thread.dac1 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC1R | DBCR_DAC1W);
> +	} else if (slot =3D=3D 2) {
> +#ifdef CONFIG_BOOKE
> +		if (child->thread.dbcr2 & DBCR2_DAC12MODE)
> +			/* Part of a range */
> +			return -EINVAL;
> +		child->thread.dbcr2 &=3D ~(DBCR2_DVC2M | DBCR2_DVC2BE);
> +		child->thread.dvc2 =3D 0;
> +#endif
> +		child->thread.dac2 =3D 0;
> +		dbcr_dac(child) &=3D ~(DBCR_DAC2R | DBCR_DAC2W);
> +	} else
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +#endif /* CONFIG_40x || CONFIG_BOOKE */
> +
> +#ifdef CONFIG_BOOKE
> +static int set_dac_range(struct task_struct *child,
> +			 struct ppc_hw_breakpoint *bp_info)
> +{
> +	int mode =3D bp_info->addr_mode & PPC_BREAKPOINT_MODE_MASK;
> +
> +	/* We don't allow range watchpoints to be used with DVC */
> +	if (bp_info->condition_mode && PPC_BREAKPOINT_CONDITION_BE_ALL)

Uh.. that condition really doesn't look right.  First, surely it
should be a bitwise, not a logical and, second the comment is talking
about range watchpoints, but the condition is about the byte enable
bits.

> +		return -EINVAL;
> +
> +	if (bp_info->addr >=3D TASK_SIZE)
> +		return -EIO;
> +
> +	if (child->thread.dbcr0 &
> +	    (DBCR0_DAC1R | DBCR0_DAC1W | DBCR0_DAC2R | DBCR0_DAC2W))
> +		return -ENOSPC;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1R | DBCR0_DAC2R | DBCR0_IDM);
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> +		child->thread.dbcr0 |=3D (DBCR0_DAC1W | DBCR0_DAC2W | DBCR0_IDM);

Again, I thought only the DAC1 R/W bits were used for range watchpoints.

> +	child->thread.dac1 =3D bp_info->addr;
> +	child->thread.dac2 =3D bp_info->addr2;
> +	if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_INCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12R;
> +	else if (mode =3D=3D PPC_BREAKPOINT_MODE_RANGE_EXCLUSIVE)
> +		child->thread.dbcr2  |=3D DBCR2_DAC12RX;
> +	else	/* PPC_BREAKPOINT_MODE_MASK */
> +		child->thread.dbcr2  |=3D DBCR2_DAC12MASK;
> +	child->thread.regs->msr |=3D MSR_DE;
> +
> +	return 5;
> +}
> +#endif /* CONFIG_BOOKE */
> +
>  static long ppc_set_hwdebug(struct task_struct *child,
>  		     struct ppc_hw_breakpoint *bp_info)
>  {
> +	if (bp_info->version !=3D 1)
> +		return -ENOTSUPP;
> +
> +#ifdef CONFIG_BOOKE
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode &
> +	     ~(PPC_BREAKPOINT_CONDITION_AND_OR |
> +	       PPC_BREAKPOINT_CONDITION_BE_ALL)))
> +		return -EINVAL;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +
> +	if (bp_info->addr_mode =3D=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return set_dac(child, bp_info);
> +
> +	return set_dac_range(child, bp_info);
> +#elif defined(CONFIG_40x)
> +	/*
> +	 * Check for invalid flags and combinations
> +	 */
> +	if ((bp_info->trigger_type =3D=3D 0) ||
> +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> +	    (bp_info->condition_mode !=3D PPC_BREAKPOINT_CONDITION_NONE))
> +		return -EINVAL;
> +
> +	if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_EXECUTE) {
> +		if (bp_info->trigger_type !=3D PPC_BREAKPOINT_TRIGGER_EXECUTE)
> +			/* At least another bit was set */
> +			return -EINVAL;
> +		return set_intruction_bp(child, bp_info);
> +	}
> +	if (bp_info->addr_mode !=3D PPC_BREAKPOINT_MODE_EXACT)
> +		return -EINVAL;
> +
> +	return set_dac(child, bp_info);
> +#else
>  	/*
> -	 * We currently support one data breakpoint
> +	 * We only support one data breakpoint

Uh.. you've updated this comment to something which is still wrong
with the new code...

[snip]
> @@ -980,16 +1308,36 @@ long arch_ptrace(struct task_struct *child, long r=
equest, long addr, long data)
>  		struct ppc_debug_info dbginfo;
> =20
>  		dbginfo.version =3D 1;
> +#ifdef CONFIG_BOOKE
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 2;
> +		dbginfo.data_bp_alignment =3D 0;

Surely that can't be right, I thought there was always a data bp
alignment constraint.

> +		dbginfo.sizeof_condition =3D 4;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK |
> +				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
> +#elif defined(CONFIG_40x)
> +		/*
> +		 * I don't know how the DVCs work on 40x, I'm not going
> +		 * to support it now. -- Shaggy
> +		 */
> +		dbginfo.num_instruction_bps =3D 4;
> +		dbginfo.num_data_bps =3D 2;
> +		dbginfo.num_condition_regs =3D 0;
> +		dbginfo.data_bp_alignment =3D 0;
> +		dbginfo.sizeof_condition =3D 0;
> +		dbginfo.features =3D PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> +				   PPC_DEBUG_FEATURE_INSN_BP_MASK;
> +#else
>  		dbginfo.num_instruction_bps =3D 0;
>  		dbginfo.num_data_bps =3D 1;
>  		dbginfo.num_condition_regs =3D 0;
> -#ifdef CONFIG_PPC64
>  		dbginfo.data_bp_alignment =3D 8;

Uh, this isn't quite right.  32-bit classic PPC can have DABRs, so
!CONFIG_PPC64 does not imply 40x|BOOKE, which means we still need the
32-bit case here.

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

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[David Gibson]

On Thu, Dec 10, 2009 at 08:41:53PM -0600, Kumar Gala wrote:
[snip]
> > +#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
> > +#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 | \
> > +				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
> > +				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
> > +				 DBCR1_IAC4US | DBCR1_IAC4ER_10)
> 
> We are we using MSR[IS] IS=0, why not just any Eff address?  In the
> future we might have user as IS = 1, and kernel as IS = 0.

Since the user can't control that directly, we can update this when
and if we change our use of address spaces.

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

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Kumar Gala]

On Dec 10, 2009, at 9:28 PM, David Gibson wrote:

> On Thu, Dec 10, 2009 at 08:41:53PM -0600, Kumar Gala wrote:
> [snip]
>>> +#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
>>> +#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 =
| \
>>> +				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
>>> +				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
>>> +				 DBCR1_IAC4US | DBCR1_IAC4ER_10)
>>=20
>> We are we using MSR[IS] IS=3D0, why not just any Eff address?  In the
>> future we might have user as IS =3D 1, and kernel as IS =3D 0.
>=20
> Since the user can't control that directly, we can update this when
> and if we change our use of address spaces.

That's such a subtle issue (easy to forgot about and miss in the =
future).  Why do we need this now?

- k=

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Thiago Jung Bauermann]

On Fri 11 Dec 2009 00:27:36 Dave Kleikamp wrote:
> On Thu, 2009-12-10 at 20:23 -0600, Kumar Gala wrote:
> > On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> > > #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> > > #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> > > #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> > > #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
> >
> > Is GDB smart enough to deal w/no condition_regs?  On some Book-E
> > devices we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the
> > features?
> 
> I had discussed it with the gdb team.  I could easily add a feature
> flag, but it would be equivalent to num_condition_regs > 0.  I don't
> have a strong opinion either way.

The current GDB code we have here uses num_condition_regs > 0 to discover if 
DVCs are supported, so a PPC_DEBUG_FEATURE constant for that is redundant 
IMHO.
-- 
[]'s
Thiago Jung Bauermann
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Kumar Gala]

On Dec 11, 2009, at 2:07 PM, Thiago Jung Bauermann wrote:

> On Fri 11 Dec 2009 00:27:36 Dave Kleikamp wrote:
>> On Thu, 2009-12-10 at 20:23 -0600, Kumar Gala wrote:
>>> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
>>>> #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
>>>> #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
>>>> #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
>>>> #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
>>>=20
>>> Is GDB smart enough to deal w/no condition_regs?  On some Book-E
>>> devices we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in =
the
>>> features?
>>=20
>> I had discussed it with the gdb team.  I could easily add a feature
>> flag, but it would be equivalent to num_condition_regs > 0.  I don't
>> have a strong opinion either way.
>=20
> The current GDB code we have here uses num_condition_regs > 0 to =
discover if=20
> DVCs are supported, so a PPC_DEBUG_FEATURE constant for that is =
redundant=20
> IMHO.

That's fine, just want to make sure we are ok w/num_condition_regs =3D=3D =
0.

- k=

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Benjamin Herrenschmidt]

On Thu, 2009-12-10 at 20:24 -0600, Kumar Gala wrote:
> This is a good write up.  We should have it as a commit message for
> the first patch.

No, that should go into Documentation/powerpc/

Cheers,
Ben.

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[David Gibson]

On Fri, Dec 11, 2009 at 08:35:35AM -0600, Kumar Gala wrote:
> 
> On Dec 10, 2009, at 9:28 PM, David Gibson wrote:
> 
> > On Thu, Dec 10, 2009 at 08:41:53PM -0600, Kumar Gala wrote:
> > [snip]
> >>> +#define DBCR1_USER_DEBUG	(DBCR1_IAC12M | DBCR1_IAC34M)
> >>> +#define DBCR1_BASE_REG_VALUE	(DBCR1_IAC1US | DBCR1_IAC1ER_10 | \
> >>> +				 DBCR1_IAC2US | DBCR1_IAC2ER_10 | \
> >>> +				 DBCR1_IAC3US | DBCR1_IAC3ER_10 | \
> >>> +				 DBCR1_IAC4US | DBCR1_IAC4ER_10)
> >> 
> >> We are we using MSR[IS] IS=0, why not just any Eff address?  In the
> >> future we might have user as IS = 1, and kernel as IS = 0.
> > 
> > Since the user can't control that directly, we can update this when
> > and if we change our use of address spaces.
> 
> That's such a subtle issue (easy to forgot about and miss in the
> future).  Why do we need this now?

Um.. I guess we don't really.  I was thinking of this default value a)
in the context of the old explicit register setting interface, where
we wanted to force what userspace could and could not set here and b)
half in the context of a system with real mode, in which we certainly
wouldn't want userspace to be able to set real address breakpoints.

I don't think it matters that much.  And, if we use AS1 for userspace
in the future, we arguably still want to change this to make it _01,
so that userspace can't set breakpoints in the kernel AS (although I
guess as long as we force the PR state in which the breakpoint
triggers, it doesn't really matter).

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

[RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Dave Kleikamp]

powerpc: Add definitions for Debug Registers on BookE Platforms

From: Torez Smith <lnxtorez@linux.vnet.ibm.com>

This patch adds additional definitions for BookE Debug Registers
to the reg_booke.h header file.

Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
Acked-by: David Gibson <dwg@au1.ibm.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: Josh Boyer <jwboyer@linux.vnet.ibm.com>
Cc: Kumar Gala <galak@kernel.crashing.org>
Cc: Sergio Durigan Junior <sergiodj@br.ibm.com>
Cc: Thiago Jung Bauermann <bauerman@br.ibm.com>
Cc: linuxppc-dev list <Linuxppc-dev@ozlabs.org>
---

 arch/powerpc/include/asm/processor.h |   32 +++++++++++
 arch/powerpc/include/asm/reg_booke.h |   96 ++++++++++++++++++++++++++++++++++
 2 files changed, 125 insertions(+), 3 deletions(-)


diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
index 9eed29e..ab0ee94 100644
--- a/arch/powerpc/include/asm/processor.h
+++ b/arch/powerpc/include/asm/processor.h
@@ -161,9 +161,37 @@ struct thread_struct {
 #ifdef CONFIG_PPC32
 	void		*pgdir;		/* root of page-table tree */
 #endif
-#if defined(CONFIG_4xx) || defined (CONFIG_BOOKE)
-	unsigned long	dbcr0;		/* debug control register values */
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+	/*
+	 * The following help to manage the use of Debug Control Registers
+	 * om the BookE platforms.
+	 */
+	unsigned long	dbcr0;
 	unsigned long	dbcr1;
+#ifdef CONFIG_BOOKE
+	unsigned long	dbcr2;
+#endif
+	/*
+	 * The stored value of the DBSR register will be the value at the
+	 * last debug interrupt. This register can only be read from the
+	 * user (will never be written to) and has value while helping to
+	 * describe the reason for the last debug trap.  Torez
+	 */
+	unsigned long	dbsr;
+	/*
+	 * The following will contain addresses used by debug applications
+	 * to help trace and trap on particular address locations.
+	 * The bits in the Debug Control Registers above help define which
+	 * of the following registers will contain valid data and/or addresses.
+	 */
+	unsigned long	iac1;
+	unsigned long	iac2;
+	unsigned long	iac3;
+	unsigned long	iac4;
+	unsigned long	dac1;
+	unsigned long	dac2;
+	unsigned long	dvc1;
+	unsigned long	dvc2;
 #endif
 	/* FP and VSX 0-31 register set */
 	double		fpr[32][TS_FPRWIDTH];
diff --git a/arch/powerpc/include/asm/reg_booke.h b/arch/powerpc/include/asm/reg_booke.h
index 3bf7835..8808d30 100644
--- a/arch/powerpc/include/asm/reg_booke.h
+++ b/arch/powerpc/include/asm/reg_booke.h
@@ -248,6 +248,8 @@
 #define DBSR_RET	0x00008000	/* Return Debug Event */
 #define DBSR_CIRPT	0x00000040	/* Critical Interrupt Taken Event */
 #define DBSR_CRET	0x00000020	/* Critical Return Debug Event */
+#define DBSR_IAC12ATS	0x00000002	/* Instr Address Compare 1/2 Toggle */
+#define DBSR_IAC34ATS	0x00000001	/* Instr Address Compare 3/4 Toggle */
 #endif
 #ifdef CONFIG_40x
 #define DBSR_IC		0x80000000	/* Instruction Completion */
@@ -313,6 +315,38 @@
 #define DBCR0_IA12T	0x00008000	/* Instr Addr 1-2 range Toggle */
 #define DBCR0_IA34T	0x00004000	/* Instr Addr 3-4 range Toggle */
 #define DBCR0_FT	0x00000001	/* Freeze Timers on debug event */
+
+#define dbcr_iac_range(task)	((task)->thread.dbcr0)
+#define DBCR_IAC12I	DBCR0_IA12			/* Range Inclusive */
+#define DBCR_IAC12X	(DBCR0_IA12 | DBCR0_IA12X)	/* Range Exclusive */
+#define DBCR_IAC12MODE	(DBCR0_IA12 | DBCR0_IA12X)	/* IAC 1-2 Mode Bits */
+#define DBCR_IAC34I	DBCR0_IA34			/* Range Inclusive */
+#define DBCR_IAC34X	(DBCR0_IA34 | DBCR0_IA34X)	/* Range Exclusive */
+#define DBCR_IAC34MODE	(DBCR0_IA34 | DBCR0_IA34X)	/* IAC 3-4 Mode Bits */
+
+/* Bit definitions related to the DBCR1. */
+#define DBCR1_DAC1R	0x80000000	/* DAC1 Read Debug Event */
+#define DBCR1_DAC2R	0x40000000	/* DAC2 Read Debug Event */
+#define DBCR1_DAC1W	0x20000000	/* DAC1 Write Debug Event */
+#define DBCR1_DAC2W	0x10000000	/* DAC2 Write Debug Event */
+
+#define dbcr_dac(task)	((task)->thread.dbcr1)
+#define DBCR_DAC1R	DBCR1_DAC1R
+#define DBCR_DAC1W	DBCR1_DAC1W
+#define DBCR_DAC2R	DBCR1_DAC2R
+#define DBCR_DAC2W	DBCR1_DAC2W
+
+/*
+ * Are there any active Debug Events represented in the
+ * Debug Control Registers?
+ */
+#define DBCR0_ACTIVE_EVENTS	(DBCR0_ICMP | DBCR0_IAC1 | DBCR0_IAC2 | \
+				 DBCR0_IAC3 | DBCR0_IAC4)
+#define DBCR1_ACTIVE_EVENTS	(DBCR1_DAC1R | DBCR1_DAC2R | \
+				 DBCR1_DAC1W | DBCR1_DAC2W)
+#define DBCR_ACTIVE_EVENTS(dbcr0, dbcr1)  (((dbcr0) & DBCR0_ACTIVE_EVENTS) || \
+					   ((dbcr1) & DBCR1_ACTIVE_EVENTS))
+
 #elif defined(CONFIG_BOOKE)
 #define DBCR0_EDM	0x80000000	/* External Debug Mode */
 #define DBCR0_IDM	0x40000000	/* Internal Debug Mode */
@@ -342,19 +376,79 @@
 #define DBCR0_CRET	0x00000020	/* Critical Return Debug Event */
 #define DBCR0_FT	0x00000001	/* Freeze Timers on debug event */
 
+#define dbcr_dac(task)	((task)->thread.dbcr0)
+#define DBCR_DAC1R	DBCR0_DAC1R
+#define DBCR_DAC1W	DBCR0_DAC1W
+#define DBCR_DAC2R	DBCR0_DAC2R
+#define DBCR_DAC2W	DBCR0_DAC2W
+
 /* Bit definitions related to the DBCR1. */
+#define DBCR1_IAC1US	0xC0000000	/* Instr Addr Cmp 1 Sup/User   */
+#define DBCR1_IAC1ER	0x30000000	/* Instr Addr Cmp 1 Eff/Real */
+#define DBCR1_IAC1ER_01	0x10000000	/* reserved */
+#define DBCR1_IAC1ER_10	0x20000000	/* Instr Addr Cmp 1 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC1ER_11	0x30000000	/* Instr Addr Cmp 1 Eff/Real MSR[IS]=1 */
+#define DBCR1_IAC2US	0x0C000000	/* Instr Addr Cmp 2 Sup/User   */
+#define DBCR1_IAC2ER	0x03000000	/* Instr Addr Cmp 2 Eff/Real */
+#define DBCR1_IAC2ER_01	0x01000000	/* reserved */
+#define DBCR1_IAC2ER_10	0x02000000	/* Instr Addr Cmp 2 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC2ER_11	0x03000000	/* Instr Addr Cmp 2 Eff/Real MSR[IS]=1 */
 #define DBCR1_IAC12M	0x00800000	/* Instr Addr 1-2 range enable */
 #define DBCR1_IAC12MX	0x00C00000	/* Instr Addr 1-2 range eXclusive */
 #define DBCR1_IAC12AT	0x00010000	/* Instr Addr 1-2 range Toggle */
+#define DBCR1_IAC3US	0x0000C000	/* Instr Addr Cmp 3 Sup/User   */
+#define DBCR1_IAC3ER	0x00003000	/* Instr Addr Cmp 3 Eff/Real */
+#define DBCR1_IAC3ER_01	0x00001000	/* reserved */
+#define DBCR1_IAC3ER_10	0x00002000	/* Instr Addr Cmp 3 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC3ER_11	0x00003000	/* Instr Addr Cmp 3 Eff/Real MSR[IS]=1 */
+#define DBCR1_IAC4US	0x00000C00	/* Instr Addr Cmp 4 Sup/User   */
+#define DBCR1_IAC4ER	0x00000300	/* Instr Addr Cmp 4 Eff/Real */
+#define DBCR1_IAC4ER_01	0x00000100	/* Instr Addr Cmp 4 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC4ER_10	0x00000200	/* Instr Addr Cmp 4 Eff/Real MSR[IS]=0 */
+#define DBCR1_IAC4ER_11	0x00000300	/* Instr Addr Cmp 4 Eff/Real MSR[IS]=1 */
 #define DBCR1_IAC34M	0x00000080	/* Instr Addr 3-4 range enable */
 #define DBCR1_IAC34MX	0x000000C0	/* Instr Addr 3-4 range eXclusive */
 #define DBCR1_IAC34AT	0x00000001	/* Instr Addr 3-4 range Toggle */
 
+#define dbcr_iac_range(task)	((task)->thread.dbcr1)
+#define DBCR_IAC12I	DBCR1_IAC12M	/* Range Inclusive */
+#define DBCR_IAC12X	DBCR1_IAC12MX	/* Range Exclusive */
+#define DBCR_IAC12MODE	DBCR1_IAC12MX	/* IAC 1-2 Mode Bits */
+#define DBCR_IAC34I	DBCR1_IAC34M	/* Range Inclusive */
+#define DBCR_IAC34X	DBCR1_IAC34MX	/* Range Exclusive */
+#define DBCR_IAC34MODE	DBCR1_IAC34MX	/* IAC 3-4 Mode Bits */
+
 /* Bit definitions related to the DBCR2. */
+#define DBCR2_DAC1US	0xC0000000	/* Data Addr Cmp 1 Sup/User   */
+#define DBCR2_DAC1ER	0x30000000	/* Data Addr Cmp 1 Eff/Real */
+#define DBCR2_DAC2US	0x00000000	/* Data Addr Cmp 2 Sup/User   */
+#define DBCR2_DAC2ER	0x00000000	/* Data Addr Cmp 2 Eff/Real */
 #define DBCR2_DAC12M	0x00800000	/* DAC 1-2 range enable */
+#define DBCR2_DAC12MM	0x00400000	/* DAC 1-2 Mask mode*/
 #define DBCR2_DAC12MX	0x00C00000	/* DAC 1-2 range eXclusive */
+#define DBCR2_DAC12MODE	0x00C00000	/* DAC 1-2 Mode Bits */
 #define DBCR2_DAC12A	0x00200000	/* DAC 1-2 Asynchronous */
-#endif
+#define DBCR2_DVC1M	0x000C0000	/* Data Value Comp 1 Mode */
+#define DBCR2_DVC1M_SHIFT	18	/* # of bits to shift DBCR2_DVC1M */
+#define DBCR2_DVC2M	0x00030000	/* Data Value Comp 2 Mode */
+#define DBCR2_DVC2M_SHIFT	16	/* # of bits to shift DBCR2_DVC2M */
+#define DBCR2_DVC1BE	0x00000F00	/* Data Value Comp 1 Byte */
+#define DBCR2_DVC1BE_SHIFT	8	/* # of bits to shift DBCR2_DVC1BE */
+#define DBCR2_DVC2BE	0x0000000F	/* Data Value Comp 2 Byte */
+#define DBCR2_DVC2BE_SHIFT	0	/* # of bits to shift DBCR2_DVC2BE */
+
+/*
+ * Are there any active Debug Events represented in the
+ * Debug Control Registers?
+ */
+#define DBCR0_ACTIVE_EVENTS  (DBCR0_ICMP | DBCR0_IAC1 | DBCR0_IAC2 | \
+			      DBCR0_IAC3 | DBCR0_IAC4 | DBCR0_DAC1R | \
+			      DBCR0_DAC1W  | DBCR0_DAC2R | DBCR0_DAC2W)
+#define DBCR1_ACTIVE_EVENTS	0
+
+#define DBCR_ACTIVE_EVENTS(dbcr0, dbcr1)  (((dbcr0) & DBCR0_ACTIVE_EVENTS) || \
+					   ((dbcr1) & DBCR1_ACTIVE_EVENTS))
+#endif /* #elif defined(CONFIG_BOOKE) */
 
 /* Bit definitions related to the TCR. */
 #define TCR_WP(x)	(((x)&0x3)<<30)	/* WDT Period */

-- 
Dave Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Dave Kleikamp]

On Thu, 2009-12-10 at 12:07 -0500, Josh Boyer wrote:
> On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:

> >+	/*
> >+	 * The stored value of the DBSR register will be the value at the
> >+	 * last debug interrupt. This register can only be read from the
> >+	 * user (will never be written to) and has value while helping to
> >+	 * describe the reason for the last debug trap.  Torez
> >+	 */
> >+	unsigned long	dbsr;
> >+	/*
> >+	 * The following will contain addresses used by debug applications
> >+	 * to help trace and trap on particular address locations.
> >+	 * The bits in the Debug Control Registers above help define which
> >+	 * of the following registers will contain valid data and/or addresses.
> >+	 */
> >+	unsigned long	iac1;
> >+	unsigned long	iac2;
> >+	unsigned long	iac3;
> >+	unsigned long	iac4;
> >+	unsigned long	dac1;
> >+	unsigned long	dac2;
> >+	unsigned long	dvc1;
> >+	unsigned long	dvc2;
> > #endif
> 
> Without digging much, I'm wondering if we could just use a pointer to a debug
> register structure here instead of growing struct thread more.

I didn't do this (yet).  I'm not sure if it is worth the trouble.  Don't
hesitate to change my mind on this.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 02/03] powerpc: Add definitions for Debug Registers on BookE Platforms

[Dave Kleikamp]

On Fri, 2009-12-11 at 11:53 +1100, David Gibson wrote:
> On Thu, Dec 10, 2009 at 01:57:21PM -0200, Dave Kleikamp wrote:
> > powerpc: Add definitions for Debug Registers on BookE Platforms
> > 
> > From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> > 
> > This patch adds additional definitions for BookE Debug Registers
> > to the reg_booke.h header file.
> > 
> > Signed-off-by: Torez Smith  <lnxtorez@linux.vnet.ibm.com>
> > Signed-off-by: Dave Kleikamp <shaggy@linux.vnet.ibm.com>
 
> >  arch/powerpc/include/asm/processor.h |   30 +++++-
> >  arch/powerpc/include/asm/reg_booke.h |  176 +++++++++++++++++++++++++++++-----
> >  2 files changed, 178 insertions(+), 28 deletions(-)
> 
> [snip]
> > +	/*
> > +	 * The following will contain addresses used by debug applications
> > +	 * to help trace and trap on particular address locations.
> > +	 * The bits in the Debug Control Registers above help define which
> > +	 * of the following registers will contain valid data and/or addresses.
> > +	 */
> > +	unsigned long	iac1;
> > +	unsigned long	iac2;
> > +	unsigned long	iac3;
> > +	unsigned long	iac4;
> > +	unsigned long	dac1;
> > +	unsigned long	dac2;
> > +	unsigned long	dvc1;
> > +	unsigned long	dvc2;
> 
> I think you'd make the logic in patch 3 substantially easier, if you
> defined these as
> 	unsigned long iac[4];
> 	unsigned long dac[2];
> 	unsigned long dvc[2];
> instead of as individual structure members.

I've cleaned up the logic a bit without having to change this.  Any
further simplification of the code would also involve abstracting the
#defines further and I'm not sure it would make anything more clear.

> [snip]
> > +
> > +#define dbcr_iac_range(task)	((task)->thread.dbcr0)
> 
> Hrm, I think the way these macros work to do the 40x vs. BookE
> abstration is kind of ugly.  But an unequivocally better way doesn't
> immediately occur to me.

I haven't changed this.  I don't have a better solution.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Dave Kleikamp]

On Thu, 2009-12-10 at 20:50 -0600, Kumar Gala wrote:
> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> 
> > powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
> > 
> > From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> > 
> > This patch defines context switch and trap related functionality
> > for BookE specific Debug Registers. It adds support to ptrace()
> > for setting and getting BookE related Debug Registers

snip

> > +static void prime_debug_regs(struct thread_struct *thread)
> > +{
> > +	mtspr(SPRN_IAC1, thread->iac1);
> > +	mtspr(SPRN_IAC2, thread->iac2);
> > +	mtspr(SPRN_IAC3, thread->iac3);
> > +	mtspr(SPRN_IAC4, thread->iac4);
> > +	mtspr(SPRN_DAC1, thread->dac1);
> > +	mtspr(SPRN_DAC2, thread->dac2);
> > +	mtspr(SPRN_DVC1, thread->dvc1);
> > +	mtspr(SPRN_DVC2, thread->dvc2);
> > +	mtspr(SPRN_DBCR0, thread->dbcr0);
> > +	mtspr(SPRN_DBCR1, thread->dbcr1);
> > +	mtspr(SPRN_DBCR2, thread->dbcr2);
> 
> We should probably look at dbginfo.num_condition_regs, dbginfo.num_instruction_bps, & dbginfo.num_data_bps and set these accordingly.

All I did here is make dbcr2 depend on CONFIG_BOOKE.  For the time
being, there's no run-time checks on this, only compile-time.

> > +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
> > +static long set_intruction_bp(struct task_struct *child,
> > +			      struct ppc_hw_breakpoint *bp_info)
> > +{
> > +	int slots_needed;
> > +	int slot;
> > +	int free_slot = 0;
> > +
> > +	/*
> > +	 * Find an avalailable slot for the breakpoint.
> > +	 * If possible, reserve consecutive slots, 1 & 2, for a range
> > +	 * breakpoint.  (Can this be done simpler?)
> > +	 */
> > +	if (bp_info->addr_mode == PPC_BREAKPOINT_MODE_EXACT)
> > +		slots_needed = 1;
> > +	else
> > +		slots_needed = 2;
> > +
> > +	if ((child->thread.dbcr0 & DBCR0_IAC1) == 0) {
> > +		if (slots_needed == 1) {
> > +			if (child->thread.dbcr0 & DBCR0_IAC2) {
> > +				slot = 1;
> > +				goto found;
> > +			}
> > +			/* Try to save slots 1 & 2 for range */
> > +			free_slot = 1;
> > +		} else
> > +			if ((child->thread.dbcr0 & DBCR0_IAC2) == 0) {
> > +				slot = 1;
> > +				goto found;
> > +			}
> > +	} else if ((slots_needed == 1) &&
> > +		   ((child->thread.dbcr0 & DBCR0_IAC2) == 0)) {
> > +		slot = 2;
> > +		goto found;
> > +	}
> > +	if ((child->thread.dbcr0 & DBCR0_IAC3) == 0) {
> > +		if (slots_needed == 1) {
> > +			slot = 3;
> > +			goto found;
> > +		}
> > +		if ((child->thread.dbcr0 & DBCR0_IAC4) == 0) {
> > +			slot = 3;
> > +			goto found;
> > +		}
> > +		return -ENOSPC;
> > +	} else if (slots_needed == 2)
> > +		return -ENOSPC;
> > +	if ((child->thread.dbcr0 & DBCR0_IAC4) == 0) {
> > +		slot = 4;
> > +	} else if (free_slot)
> > +		slot = free_slot;
> > +	else
> > +		return -ENOSPC;
> 
> Need to factor in if # of IACs is only 2.

What cpu has 2 IACs?

> > static long ppc_set_hwdebug(struct task_struct *child,
> > 		     struct ppc_hw_breakpoint *bp_info)
> > {
> > +	if (bp_info->version != 1)
> > +		return -ENOTSUPP;
> > +
> > +#ifdef CONFIG_BOOKE
> > +	/*
> > +	 * Check for invalid flags and combinations
> > +	 */
> > +	if ((bp_info->trigger_type == 0) ||
> > +	    (bp_info->trigger_type & ~(PPC_BREAKPOINT_TRIGGER_EXECUTE |
> > +				       PPC_BREAKPOINT_TRIGGER_RW)) ||
> > +	    (bp_info->addr_mode & ~PPC_BREAKPOINT_MODE_MASK) ||
> > +	    (bp_info->condition_mode &
> > +	     ~(PPC_BREAKPOINT_CONDITION_AND_OR |
> > +	       PPC_BREAKPOINT_CONDITION_BE_ALL)))
> > +		return -EINVAL;
> 
> We should add a sanity check for bp_info->condition_mode != PPC_BREAKPOINT_CONDITION_NONE if dbginfo.num_condition_regs = 0.

This falls into the run-time checks that I haven't implemented.  BOOKE
will not define dbginfo.num_condition_regs to be 0.

> > /*
> > @@ -980,16 +1308,36 @@ long arch_ptrace(struct task_struct *child, long request, long addr, long data)
> > 		struct ppc_debug_info dbginfo;
> > 
> > 		dbginfo.version = 1;
> > +#ifdef CONFIG_BOOKE
> > +		dbginfo.num_instruction_bps = 4;
> > +		dbginfo.num_data_bps = 2;
> > +		dbginfo.num_condition_regs = 2;
> > +		dbginfo.data_bp_alignment = 0;
> > +		dbginfo.sizeof_condition = 4;
> > +		dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> > +				   PPC_DEBUG_FEATURE_INSN_BP_MASK |
> > +				   PPC_DEBUG_FEATURE_DATA_BP_RANGE |
> > +				   PPC_DEBUG_FEATURE_DATA_BP_MASK;
> > +#elif defined(CONFIG_40x)
> > +		/*
> > +		 * I don't know how the DVCs work on 40x, I'm not going
> > +		 * to support it now. -- Shaggy
> > +		 */
> > +		dbginfo.num_instruction_bps = 4;
> > +		dbginfo.num_data_bps = 2;
> > +		dbginfo.num_condition_regs = 0;
> > +		dbginfo.data_bp_alignment = 0;
> > +		dbginfo.sizeof_condition = 0;
> > +		dbginfo.features = PPC_DEBUG_FEATURE_INSN_BP_RANGE |
> > +				   PPC_DEBUG_FEATURE_INSN_BP_MASK;
> > +#else
> > 		dbginfo.num_instruction_bps = 0;
> > 		dbginfo.num_data_bps = 1;
> > 		dbginfo.num_condition_regs = 0;
> > -#ifdef CONFIG_PPC64
> > 		dbginfo.data_bp_alignment = 8;
> > -#else
> > -		dbginfo.data_bp_alignment = 0;
> > -#endif
> > 		dbginfo.sizeof_condition = 0;
> > 		dbginfo.features = 0;
> > +#endif
> 
> This is a bit ugly and BOOKE 64 parts probably don't have the 8 byte alignment.
> 
> Should we push some of this into cputable?

I'm thinking about it.  It may clean up some of the ifdefs.  I'll give
it a try, and if it ends up cleaner, I'll submit a follow-up patch.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 03/03] powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace

[Dave Kleikamp]

On Fri, 2009-12-11 at 14:26 +1100, David Gibson wrote:
> On Thu, Dec 10, 2009 at 01:57:27PM -0200, Dave Kleikamp wrote:
> > powerpc: Add support for BookE Debug Reg. traps, exceptions and ptrace
> > 
> > From: Torez Smith <lnxtorez@linux.vnet.ibm.com>
> > 
> > This patch defines context switch and trap related functionality
> > for BookE specific Debug Registers. It adds support to ptrace()
> > for setting and getting BookE related Debug Registers

> [snip]
> > +#if !(defined(CONFIG_40x) || defined(CONFIG_BOOKE))
> >  void do_dabr(struct pt_regs *regs, unsigned long address,
> >  		    unsigned long error_code)
> >  {
> > @@ -257,12 +275,6 @@ void do_dabr(struct pt_regs *regs, unsigned long address,
> >  	if (debugger_dabr_match(regs))
> >  		return;
> >  
> > -	/* Clear the DAC and struct entries.  One shot trigger */
> > -#if defined(CONFIG_BOOKE)
> > -	mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
> > -							| DBCR0_IDM));
> > -#endif
> > -
> >  	/* Clear the DABR */
> >  	set_dabr(0);
> 
> Uh.. does this imply we're keeping the one-shot behaviour for
> new-style breakpoints?  To me the interface really suggests they're
> persistent, although dealing with the semantics of that at signal time
> can get curly.

I've left it as one-shot.  The gdb guys seem happy with that.

[snip]

> >  int set_dabr(unsigned long dabr)
> >  {
> >  	__get_cpu_var(current_dabr) = dabr;
> > @@ -284,7 +358,7 @@ int set_dabr(unsigned long dabr)
> >  		return ppc_md.set_dabr(dabr);
> >  
> >  	/* XXX should we have a CPU_FTR_HAS_DABR ? */
> > -#if defined(CONFIG_BOOKE)
> > +#if defined(CONFIG_BOOKE) || defined(CONFIG_40x)
> >  	mtspr(SPRN_DAC1, dabr);
> 
> Uh.. this would seem to be wrong.  set_dabr(0) is called from the
> debug exception - but nowadays that could be tiggered by a DAC other
> than DAC1.

Right now, the only place left that set_dabr() is called for BOOKE or
40x is from xmon.  I think that's already a problem in that it doesn't
play nicely with ptrace and it fails to set DBCR0_DAC1R or DBCR0_DAC1W.
We probably need an independent patch for that.


> [snip]
> > +#else
> >  	/* As described above, it was assumed 3 bits were passed with the data
> >  	 *  address, but we will assume only the mode bits will be passed
> >  	 *  as to not cause alignment restrictions for DAC-based processors.
> >  	 */
> >  
> >  	/* DAC's hold the whole address without any mode flags */
> > -	task->thread.dabr = data & ~0x3UL;
> > -
> > -	if (task->thread.dabr == 0) {
> > -		task->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM);
> > -		task->thread.regs->msr &= ~MSR_DE;
> > +	task->thread.dac1 = data & ~0x3UL;
> > +
> > +	if (task->thread.dac1 == 0) {
> > +		dbcr_dac(task) &= ~(DBCR_DAC1R | DBCR_DAC1W);
> > +		if (!DBCR_ACTIVE_EVENTS(task->thread.dbcr0,
> > +					task->thread.dbcr1)) {
> > +			task->thread.regs->msr &= ~MSR_DE;
> > +			task->thread.dbcr0 &= ~DBCR0_IDM;
> > +		}
> >  		return 0;
> >  	}
> 
> Ok, so effectively the old ptrace method of setting the DABR acts as a
> bypass to set DAC1, rather than having the old interface being
> implemented via the new interface.  This has some weirdness - you can
> clobber a new-style breakpoint in DAC1 using the old interface, for
> example.  Still, it might be the simplest approach for a first cut.

I really didn't consider a program using both the old and new interface.

> What *is* a problem though, is that this means that the SIGTRAP will
> always give a slot number, even for a breakpoint established using the
> old interface.  Part of the idea of encoding the registered breakpoint
> number in the siginfo was to be able to distinguish between old-style
> and new-style breakpoints at trap time.

Do we realistically expect both the old and new style breakpoints to be
used together?  I'm having trouble visualizing the scenerio.

> [snip]

> > +	int slot;
> > +
> > +	if (byte_enable && (condition_mode == 0))
> > +		return -EINVAL;
> > +
> > +	if (bp_info->addr >= TASK_SIZE)
> > +		return -EIO;
> > +
> > +	if ((dbcr_dac(child) & (DBCR_DAC1R | DBCR_DAC1W)) == 0) {
> > +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_READ)
> > +			dbcr_dac(child) |= DBCR_DAC1R;
> > +		if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE)
> > +			dbcr_dac(child) |= DBCR_DAC1W;
> > +		child->thread.dac1 = (unsigned long)bp_info->addr;
> > +#ifdef CONFIG_BOOKE
> 
> Better to have a runtime feature bit test here, than use an #ifdef to
> distinguish the 40x and BookE cases.  Plus, you should return an error
> if the user attempts to use a feature not supported on this hardware,
> which doesn't seem to happen here.

I'll take a look at making this a runtime feature.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Dave Kleikamp]

On Thu, 2009-12-10 at 20:23 -0600, Kumar Gala wrote:
> On Dec 10, 2009, at 9:57 AM, Dave Kleikamp wrote:
> 
> > These patches implement an extention to the ptrace interface proposed by
> > Thiago Bauermann and the the PowerPC gdb team.
> > 
> > GDB intends to support the following hardware debug features of BookE
> > processors:
> > 
> > 4 hardware breakpoints (IAC)
> > 2 hardware watchpoints (read, write and read-write) (DAC)
> > 2 value conditions for the hardware watchpoints (DVC)
> > 
> > For that, we need to extend ptrace so that GDB can query and set these
> > resources. Since we're extending, we're trying to create an interface
> > that's extendable and that covers both BookE and server processors, so
> > that GDB doesn't need to special-case each of them. We propose the
> > following 3 new ptrace requests described below.
> > 
> > There have been discussions of a generic hardware debug interface for the
> > kernel which would hopefully contemplate all the functionality below and
> > supersede it.  But we need something that works now, and which enables GDB
> > to be simpler and work with both Server and Embedded processors without
> > special cases.
> > 
> > 1. PTRACE_PPC_GETHWDEBUGINFO
> > 
> > Query for GDB to discover the hardware debug features. The main info to
> > be returned here is the minimum alignment for the hardware watchpoints.
> > BookE processors don't have restrictions here, but server processors have
> > an 8-byte alignment restriction for hardware watchpoints. We'd like to avoid
> > adding special cases to GDB based on what it sees in AUXV.
> > 
> > Since we're at it, we added other useful info that the kernel can return to
> > GDB: this query will return the number of hardware breakpoints, hardware
> > watchpoints and whether it supports a range of addresses and a condition.
> > The query will fill the following structure provided by the requesting process:
> > 
> > struct ppc_debug_info {
> >       unit32_t version;
> >       unit32_t num_instruction_bps;
> >       unit32_t num_data_bps;
> >       unit32_t num_condition_regs;
> >       unit32_t data_bp_alignment;
> >       unit32_t sizeof_condition; /* size of the DVC register */
> >       uint64_t features; /* bitmask of the individual flags */
> > };
> > 
> > features will have bits indicating whether there is support for:
> > 
> > #define PPC_DEBUG_FEATURE_INSN_BP_RANGE		0x1
> > #define PPC_DEBUG_FEATURE_INSN_BP_MASK		0x2
> > #define PPC_DEBUG_FEATURE_DATA_BP_RANGE		0x4
> > #define PPC_DEBUG_FEATURE_DATA_BP_MASK		0x8
> 
> Is GDB smart enough to deal w/no condition_regs?  On some Book-E
> devices we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the
> features?

I wasn't aware that the bookE devices had varying numbers of these
registers.  I guess I will have to make it a runtime option.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Dave Kleikamp]

On Thu, 2009-12-10 at 20:45 -0600, Kumar Gala wrote:

> What do we do in EDM mode?  We need a flag somewhere to determine if
> HW supports conveying DBCR0[EDM] and if it does which of the ptrace
> calls fails?

I really don't have a good answer to this.  I'm open to any and all
advice.

Shaggy
-- 
David Kleikamp
IBM Linux Technology Center

Re: [RFC:PATCH 00/03] powerpc: Expose BookE debug registers through extended ptrace interface

[Dave Kleikamp]

On Mon, 2010-01-18 at 16:34 -0600, Dave Kleikamp wrote:
> On Thu, 2009-12-10 at 20:23 -0600, Kumar Gala wrote:

> > Is GDB smart enough to deal w/no condition_regs?  On some Book-E
> > devices we have 2 IACs, 2 DACs, and 0 DVCs.  Does it need to be in the
> > features?
> 
> I wasn't aware that the bookE devices had varying numbers of these
> registers.  I guess I will have to make it a runtime option.

Kumar,

Can you tell me which bookE processors have 2 IAC's, and which have no
DVC's?

I think we still may be able to make these compile-time options as long
no two cpus that run on the same binary kernel vary in the number of
registers.  Right now I know the 403 only has 2 IAC's, and I don't
intend to expose the DVC's for the 40x processors anyway.

If they don't need to be run-time configurable, I think it would be
cleaner to define the number of each type of register in CONFIG_ flags
and put the logic into the Kconfig files.

Thanks,
Shaggy
-- 
David Kleikamp
IBM Linux Technology Center