On Thu, 2008-05-22 at 13:51 +1000, Paul Mackerras wrote:
> Luis Machado writes:
>
> > This is a patch that has been sitting idle for quite some time. I
> > decided to move it further because it is something useful. It was
> > originally written by Michel Darneille, based off of 2.6.16.
> >
> > The original patch, though, was not compatible with the current DABR
> > logic. DABR's are used to implement hardware watchpoint support for
> > ppc64 processors (i.e. 970, Power5 etc). 4xx's have a different
> > debugging register layout and needs to be handled differently (they two
> > registers: DAC and DBCR0).
>
> Yes, they are different, but they do essentially the same thing, so I
> think we should try and unify the handling of the two. Maybe you
> could rename set_dabr() to set_hw_watchpoint() or something and make
> it set DABR on 64-bit and "classic" 32-bit processors, and DAC on
> 4xx/Book E processors.
>
> Likewise, I don't think we need both a "dabr" field and a "dac" field
> in the thread_struct - one should do. Rename "dabr" to something else
> if you feel that the "dabr" name is too ppc64-specific. And I don't
> think we need both __debugger_dabr_match and __debugger_dac_match.
>
> > 5) This is something i'm worried about for future features. We currently
> > have a way to support only Hardware Watchpoints, but not Hardware
> > Breakpoints (on 64-bit processors that have a IABR register or 32-bit
> > processors carrying the IAC register). Looking at the code, we don't
> > differentiate a watchpoint from a breakpoint request. A ptrace call has
> > currently 3 arguments: REQUEST, ADDR and DATA. We use REQUEST and DATA
> > to set a hardware watchpoint. Maybe we could use the ADDR parameter to
> > set a hardware breakpoint? Or use it to tell the kernel whether we want
> > a hardware watchpoint or hardware breakpoint and then pass the address
> > of the instruction/data through the DATA parameter? What do you think?
>
> I would think there would be a different REQUEST value to mean "set a
> hardware breakpoint". Roland McGrath (cc'd) might be able to tell us
> what other architectures do.
>
> Paul.
Paul,
Follows a re-worked patch that unifies the handling of hardware
watchpoint structures for DABR-based and DAC-based processors.
The flow is exactly the same for DABR-based processors.
As for the DAC-based code, i've eliminated the "dac" field from the
thread structure, since now we use the "dabr" field to represent DAC1 of
the DAC-based processors. As a consequence, i removed all the
occurrences of "dac" throughout the code, using "dabr" in those cases.
The function "set_dabr" sets the correct register (DABR OR DAC) for each
specific processor now, instead of only setting the value for DABR-based
processors.
"do_dac" was merged with "do_dabr" as they were mostly equivalent pieces
of code. I've moved "do_dabr" to "arch/powerpc/kernel/process.c" so it
is visible for DAC-based code as well.
Tested on a Taishan 440GX with success.
Is it OK to leave it as "dabr" for both DABR and DAC? What do you think
of the patch overall?
Thanks,
Best regards,
Luis
Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/process.c
===================================================================
--- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/process.c 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/arch/powerpc/kernel/process.c 2008-06-20
02:51:16.000000000 -0700
@@ -241,6 +241,35 @@
}
#endif /* CONFIG_SMP */
+void do_dabr(struct pt_regs *regs, unsigned long address,
+ unsigned long error_code)
+{
+ siginfo_t info;
+
+#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
+ if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
+ 11, SIGSEGV) == NOTIFY_STOP)
+ return;
+
+ if (debugger_dabr_match(regs))
+ return;
+#else
+ /* Clear the DAC and struct entries. One shot trigger. */
+ mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W
+ | DBCR0_IDM));
+#endif
+
+ /* Clear the DABR */
+ set_dabr(0);
+
+ /* Deliver the signal to userspace */
+ info.si_signo = SIGTRAP;
+ info.si_errno = 0;
+ info.si_code = TRAP_HWBKPT;
+ info.si_addr = (void __user *)address;
+ force_sig_info(SIGTRAP, &info, current);
+}
+
static DEFINE_PER_CPU(unsigned long, current_dabr);
int set_dabr(unsigned long dabr)
@@ -256,6 +285,11 @@
#if defined(CONFIG_PPC64) || defined(CONFIG_6xx)
mtspr(SPRN_DABR, dabr);
#endif
+
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+ mtspr(SPRN_DAC1, dabr);
+#endif
+
return 0;
}
@@ -330,6 +364,12 @@
if (unlikely(__get_cpu_var(current_dabr) != new->thread.dabr))
set_dabr(new->thread.dabr);
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+ /* If new thread DAC (HW breakpoint) is the same then leave it. */
+ if (new->thread.dabr)
+ set_dabr(new->thread.dabr);
+#endif
+
new_thread = &new->thread;
old_thread = ¤t->thread;
@@ -518,6 +558,10 @@
if (current->thread.dabr) {
current->thread.dabr = 0;
set_dabr(0);
+
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+ current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W);
+#endif
}
}
Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/ptrace.c
===================================================================
--- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/ptrace.c 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/arch/powerpc/kernel/ptrace.c 2008-06-20
05:10:59.000000000 -0700
@@ -616,7 +616,7 @@
if (regs != NULL) {
#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
- task->thread.dbcr0 = DBCR0_IDM | DBCR0_IC;
+ task->thread.dbcr0 |= DBCR0_IDM | DBCR0_IC;
regs->msr |= MSR_DE;
#else
regs->msr |= MSR_SE;
@@ -629,9 +629,16 @@
{
struct pt_regs *regs = task->thread.regs;
+
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+ /* If DAC then do not single step, skip. */
+ if (task->thread.dabr)
+ return;
+#endif
+
if (regs != NULL) {
#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
- task->thread.dbcr0 = 0;
+ task->thread.dbcr0 &= ~(DBCR0_IC | DBCR0_IDM);
regs->msr &= ~MSR_DE;
#else
regs->msr &= ~MSR_SE;
@@ -640,22 +647,79 @@
clear_tsk_thread_flag(task, TIF_SINGLESTEP);
}
-static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
+static int ptrace_get_debugreg(struct task_struct *task, unsigned long data)
+{
+ int ret;
+
+ ret = put_user(task->thread.dabr,
+ (unsigned long __user *)data);
+ return ret;
+}
+
+int ptrace_set_debugreg(struct task_struct *task, unsigned long addr,
unsigned long data)
{
- /* We only support one DABR and no IABRS at the moment */
+ /* For ppc64 we support one DABR and no IABR's at the moment (ppc64).
+ For embedded processors we support one DAC and no IAC's
+ at the moment. */
if (addr > 0)
return -EINVAL;
- /* The bottom 3 bits are flags */
if ((data & ~0x7UL) >= TASK_SIZE)
return -EIO;
- /* Ensure translation is on */
+#ifdef CONFIG_PPC64
+
+ /* 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
+ DABR register, as follows:
+
+ bit 0: Read flag
+ bit 1: Write flag
+ bit 2: Breakpoint translation
+
+ Thus, we use them here as so. */
+
+ /* Ensure breakpoint translation bit is set. */
if (data && !(data & DABR_TRANSLATION))
return -EIO;
+ /* Move contents to the DABR register. */
task->thread.dabr = data;
+
+#endif
+#if defined(CONFIG_40x) || defined(CONFIG_BOOKE)
+
+ /* As described above, we assume 3 bits are passed with the data
+ address, so mask them so we can set the DAC register with the
+ correct address. */
+ /* 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;
+ return 0;
+ }
+
+ /* Read or Write bits must be set. */
+
+ if (!(data & 0x3UL))
+ return -EINVAL;
+
+ /* Set the Internal Debugging flag (IDM bit 1) for the DBCR0
+ register. */
+ task->thread.dbcr0 = DBCR0_IDM;
+
+ /* Check for write and read flags and set DBCR0 accordingly. */
+ if (data & 0x1UL)
+ task->thread.dbcr0 |= DBSR_DAC1R;
+ if (data & 0x2UL)
+ task->thread.dbcr0 |= DBSR_DAC1W;
+
+ task->thread.regs->msr |= MSR_DE;
+#endif
return 0;
}
Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/signal.c
===================================================================
--- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/signal.c 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/arch/powerpc/kernel/signal.c 2008-06-20
02:51:16.000000000 -0700
@@ -144,8 +144,12 @@
* 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_40x) || defined(CONFIG_BOOKE)
+ mtspr(SPRN_DBCR0, current->thread.dbcr0);
+#endif
+ }
if (is32) {
if (ka.sa.sa_flags & SA_SIGINFO)
Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/traps.c
===================================================================
--- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/traps.c 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/arch/powerpc/kernel/traps.c 2008-06-20
02:54:37.000000000 -0700
@@ -1045,6 +1045,21 @@
return;
}
_exception(SIGTRAP, regs, TRAP_TRACE, 0);
+ } else if (debug_status & (DBSR_DAC1R | DBSR_DAC1W)) {
+ regs->msr &= ~MSR_DE;
+ printk("\nWatchpoint Triggered\n");
+ 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));
+ }
+ /* Setup and send the trap to the handler. */
+ do_dabr(regs, mfspr(SPRN_DAC1), debug_status);
}
}
#endif /* CONFIG_4xx || CONFIG_BOOKE */
Index: linux-2.6.25-oldpatch/arch/powerpc/kernel/entry_32.S
===================================================================
--- linux-2.6.25-oldpatch.orig/arch/powerpc/kernel/entry_32.S 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/arch/powerpc/kernel/entry_32.S 2008-06-20
02:51:16.000000000 -0700
@@ -112,7 +112,7 @@
/* Check to see if the dbcr0 register is set up to debug. Use the
internal debug mode bit to do this. */
lwz r12,THREAD_DBCR0(r12)
- andis. r12,r12,[EMAIL PROTECTED]
+ andis. r12,r12,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
beq+ 3f
/* From user and task is ptraced - load up global dbcr0 */
li r12,-1 /* clear all pending debug events */
@@ -248,7 +248,7 @@
/* If the process has its own DBCR0 value, load it up. The internal
debug mode bit tells us that dbcr0 should be loaded. */
lwz r0,THREAD+THREAD_DBCR0(r2)
- andis. r10,r0,[EMAIL PROTECTED]
+ andis. r10,r0,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
bnel- load_dbcr0
#endif
#ifdef CONFIG_44x
@@ -676,7 +676,7 @@
/* Check whether this process has its own DBCR0 value. The internal
debug mode bit tells us that dbcr0 should be loaded. */
lwz r0,THREAD+THREAD_DBCR0(r2)
- andis. r10,r0,[EMAIL PROTECTED]
+ andis. r10,r0,(DBCR0_IDM | DBSR_DAC1R | DBSR_DAC1W)@h
bnel- load_dbcr0
#endif
Index: linux-2.6.25-oldpatch/arch/powerpc/mm/fault.c
===================================================================
--- linux-2.6.25-oldpatch.orig/arch/powerpc/mm/fault.c 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/arch/powerpc/mm/fault.c 2008-06-20
02:51:16.000000000 -0700
@@ -100,31 +100,6 @@
return 0;
}
-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
-static void do_dabr(struct pt_regs *regs, unsigned long address,
- unsigned long error_code)
-{
- siginfo_t info;
-
- if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
- 11, SIGSEGV) == NOTIFY_STOP)
- return;
-
- if (debugger_dabr_match(regs))
- return;
-
- /* Clear the DABR */
- set_dabr(0);
-
- /* Deliver the signal to userspace */
- info.si_signo = SIGTRAP;
- info.si_errno = 0;
- info.si_code = TRAP_HWBKPT;
- info.si_addr = (void __user *)address;
- force_sig_info(SIGTRAP, &info, current);
-}
-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
-
/*
* For 600- and 800-family processors, the error_code parameter is DSISR
* for a data fault, SRR1 for an instruction fault. For 400-family processors
Index: linux-2.6.25-oldpatch/include/asm-powerpc/system.h
===================================================================
--- linux-2.6.25-oldpatch.orig/include/asm-powerpc/system.h 2008-06-20
02:48:19.000000000 -0700
+++ linux-2.6.25-oldpatch/include/asm-powerpc/system.h 2008-06-20
02:51:16.000000000 -0700
@@ -103,6 +103,8 @@
#endif
extern int set_dabr(unsigned long dabr);
+extern void do_dabr(struct pt_regs *regs, unsigned long address,
+ unsigned long error_code);
extern void print_backtrace(unsigned long *);
extern void show_regs(struct pt_regs * regs);
extern void flush_instruction_cache(void);
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
