Hi, 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). I've refreshed the patch to a recent stable release (2.6.25.1, still apllies cleanly on 2.6.25.4), made the patch compatible with both 4xx and ppc64 processor designs, fixed some masks that didn't seem correct (the ones setting hw watch read/write modes) and refactored some of the code. Though, i'm still not happy enough with the patch as i think we could improve it a bit further. Some points i consider worth of attention: 1) There is a do_dac(...) implementation inside arch/powerpc/kernel/traps.c. I don't feel this is correct. I see that the 64-bit counterpart, do_dabr(...), is implemented inside arch/powerpc/mm/fault.c. Due to do_dac(...) being implemented inside traps.c, we need to externalize the declaration for "get_dac(...)" on "include/asm-[powerpc|ppc]/system.h" so it's made visible to that scope. We could use mfspr(...) to get the register's contents directly, but then i wouldn't make sense to have get_dac(...) in the first place. Maybe moving the do_dac(...) code to arch/powerpc/mm/fault.c would make more sense since we seem to have the address already, and won't need to call get_dac(...) to get it. 2) The change to make set_debugreg(...) and get_debugreg(...) transparent for both DAC-driven and DABR-driven processors is OK. But that shouldn't require us to externalize the declaration of set_debugreg(...) in order to use it in arch/powerpc/kernel/traps.c right? Maybe this has some relationship with the above point. 3) Maybe it would be better to come up with a way to merge both DABR and DAC/DBCR0 logic in order to get rid of dozens of processor-specific #ifdef's? This could be a bit more complex since it would require re-writing good portions of code. 4) Should i use CONFIG_40x ou CONFIG_4xx instead? Would CONFIG_4xx automatically include 40x's? I'm mainly targetting 4xx's here, though 40x's should be similar except for 403. 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 appreciate any comments about these items and the patch itself. Best regards. Luis
Index: linux-2.6.25.1/arch/powerpc/kernel/process.c =================================================================== --- linux-2.6.25.1.orig/arch/powerpc/kernel/process.c 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/arch/powerpc/kernel/process.c 2008-05-21 07:26:55.000000000 -0700 @@ -219,6 +219,28 @@ } #endif /* CONFIG_SPE */ +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE) + +/* Add support for HW Debug breakpoint. Use DAC register. */ +int set_dac(unsigned long dac) +{ + mtspr(SPRN_DAC1, dac); + + return 0; +} +unsigned int get_dac() +{ + return mfspr(SPRN_DAC1); +} +int set_dbcr0(unsigned long dbcr) +{ + mtspr(SPRN_DBCR0, dbcr); + + return 0; +} + +#endif + #ifndef CONFIG_SMP /* * If we are doing lazy switching of CPU state (FP, altivec or SPE), @@ -330,6 +352,13 @@ 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.dac) + set_dac(new->thread.dac); +#endif + new_thread = &new->thread; old_thread = ¤t->thread; @@ -515,6 +544,16 @@ discard_lazy_cpu_state(); +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE) + if (current->thread.dac) { + current->thread.dac = 0; + /* Clear debug control. */ + current->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W); + + set_dac(0); + } +#endif + if (current->thread.dabr) { current->thread.dabr = 0; set_dabr(0); Index: linux-2.6.25.1/arch/powerpc/kernel/ptrace.c =================================================================== --- linux-2.6.25.1.orig/arch/powerpc/kernel/ptrace.c 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/arch/powerpc/kernel/ptrace.c 2008-05-21 08:23:34.000000000 -0700 @@ -629,9 +629,15 @@ { 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.dac) + 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 +646,83 @@ 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; + + #if defined(CONFIG_40x) || defined(CONFIG_BOOKE) + ret = put_user(task->thread.dac, + (unsigned long __user *)data); + #else + ret = put_user(task->thread.dabr, + (unsigned long __user *)data); + #endif + + 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) + + /* Read or Write bits must be set. */ + if (!(data & 0x3UL)) + return -EINVAL; + + /* 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. */ + task->thread.dac = data & ~0x7UL; + + if (task->thread.dac == 0) { + task->thread.dbcr0 &= ~(DBSR_DAC1R | DBSR_DAC1W | DBCR0_IDM); + task->thread.regs->msr &= ~MSR_DE; + return 0; + } + + /* 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 & 1) + task->thread.dbcr0 |= DBSR_DAC1R; + if (data & 2) + task->thread.dbcr0 |= DBSR_DAC1W; + + task->thread.regs->msr |= MSR_DE; +#endif return 0; } @@ -763,11 +830,10 @@ case PTRACE_GET_DEBUGREG: { ret = -EINVAL; - /* We only support one DABR and no IABRS at the moment */ + /* We only support one DAC or DABR at the moment. */ if (addr > 0) break; - ret = put_user(child->thread.dabr, - (unsigned long __user *)data); + ret = ptrace_get_debugreg(child, data); break; } Index: linux-2.6.25.1/arch/powerpc/kernel/signal.c =================================================================== --- linux-2.6.25.1.orig/arch/powerpc/kernel/signal.c 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/arch/powerpc/kernel/signal.c 2008-05-21 07:26:55.000000000 -0700 @@ -148,6 +148,17 @@ set_dabr(current->thread.dabr); if (is32) { +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE) + /* + * Reenable the DAC before delivering the signal to + * user space. The DAC will have been cleared if it + * triggered inside the kernel. + */ + if (current->thread.dac) { + set_dac(current->thread.dac); + set_dbcr0(current->thread.dbcr0); + } +#endif if (ka.sa.sa_flags & SA_SIGINFO) ret = handle_rt_signal32(signr, &ka, &info, oldset, regs); Index: linux-2.6.25.1/arch/powerpc/kernel/traps.c =================================================================== --- linux-2.6.25.1.orig/arch/powerpc/kernel/traps.c 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/arch/powerpc/kernel/traps.c 2008-05-21 09:08:49.000000000 -0700 @@ -54,6 +54,9 @@ #endif #include <asm/kexec.h> +extern int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, + unsigned long data); + #if defined(CONFIG_DEBUGGER) || defined(CONFIG_KEXEC) int (*__debugger)(struct pt_regs *regs); int (*__debugger_ipi)(struct pt_regs *regs); @@ -61,6 +64,7 @@ int (*__debugger_sstep)(struct pt_regs *regs); int (*__debugger_iabr_match)(struct pt_regs *regs); int (*__debugger_dabr_match)(struct pt_regs *regs); +int (*__debugger_dac_match)(struct pt_regs *regs); int (*__debugger_fault_handler)(struct pt_regs *regs); EXPORT_SYMBOL(__debugger); @@ -69,6 +73,7 @@ EXPORT_SYMBOL(__debugger_sstep); EXPORT_SYMBOL(__debugger_iabr_match); EXPORT_SYMBOL(__debugger_dabr_match); +EXPORT_SYMBOL(__debugger_dac_match); EXPORT_SYMBOL(__debugger_fault_handler); #endif @@ -253,6 +258,27 @@ } #endif +#if defined(CONFIG_40x) || defined(CONFIG_BOOKE) + +static void do_dac(struct pt_regs *regs, unsigned long address, + unsigned long error_code) +{ + siginfo_t info; + + /* Clear the DAC and struct entries. One shot trigger. */ + mtspr(SPRN_DBCR0, mfspr(SPRN_DBCR0) & ~(DBSR_DAC1R | DBSR_DAC1W + | DBCR0_IDM)); + set_dac(0); + ptrace_set_debugreg(current, 0, 0); + + /* Deliver 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 /* * I/O accesses can cause machine checks on powermacs. * Check if the NIP corresponds to the address of a sync @@ -1045,6 +1071,20 @@ return; } _exception(SIGTRAP, regs, TRAP_TRACE, 0); + } 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)); + } + /* Setup and send the trap to the handler. */ + do_dac(regs, get_dac(), debug_status); } } #endif /* CONFIG_4xx || CONFIG_BOOKE */ Index: linux-2.6.25.1/include/asm-powerpc/processor.h =================================================================== --- linux-2.6.25.1.orig/include/asm-powerpc/processor.h 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/include/asm-powerpc/processor.h 2008-05-21 07:26:55.000000000 -0700 @@ -149,6 +149,7 @@ #if defined(CONFIG_4xx) || defined (CONFIG_BOOKE) unsigned long dbcr0; /* debug control register values */ unsigned long dbcr1; + unsigned long dac; /* Data Address Compare register */ #endif double fpr[32]; /* Complete floating point set */ struct { /* fpr ... fpscr must be contiguous */ Index: linux-2.6.25.1/include/asm-powerpc/system.h =================================================================== --- linux-2.6.25.1.orig/include/asm-powerpc/system.h 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/include/asm-powerpc/system.h 2008-05-21 08:26:10.000000000 -0700 @@ -73,6 +73,7 @@ extern int (*__debugger_sstep)(struct pt_regs *regs); extern int (*__debugger_iabr_match)(struct pt_regs *regs); extern int (*__debugger_dabr_match)(struct pt_regs *regs); +extern int (*__debugger_dac_match)(struct pt_regs *regs); extern int (*__debugger_fault_handler)(struct pt_regs *regs); #define DEBUGGER_BOILERPLATE(__NAME) \ @@ -89,6 +90,7 @@ DEBUGGER_BOILERPLATE(debugger_sstep) DEBUGGER_BOILERPLATE(debugger_iabr_match) DEBUGGER_BOILERPLATE(debugger_dabr_match) +DEBUGGER_BOILERPLATE(debugger_dac_match) DEBUGGER_BOILERPLATE(debugger_fault_handler) #else @@ -98,10 +100,17 @@ static inline int debugger_sstep(struct pt_regs *regs) { return 0; } static inline int debugger_iabr_match(struct pt_regs *regs) { return 0; } static inline int debugger_dabr_match(struct pt_regs *regs) { return 0; } +static inline int debugger_dac_match(struct pt_regs *regs) { return 0; } static inline int debugger_fault_handler(struct pt_regs *regs) { return 0; } #endif extern int set_dabr(unsigned long dabr); + +/* These are 4XX-specific functions for debugging register manipulations. */ +extern int set_dac(unsigned long dac); +extern unsigned int get_dac(void); +extern int set_dbcr0(unsigned long dbcr); + extern void print_backtrace(unsigned long *); extern void show_regs(struct pt_regs * regs); extern void flush_instruction_cache(void); Index: linux-2.6.25.1/arch/powerpc/kernel/entry_32.S =================================================================== --- linux-2.6.25.1.orig/arch/powerpc/kernel/entry_32.S 2008-05-21 07:25:45.000000000 -0700 +++ linux-2.6.25.1/arch/powerpc/kernel/entry_32.S 2008-05-21 07:26:55.000000000 -0700 @@ -112,7 +112,7 @@ /* Check to see if the dbcr0 register is set up to debug. Use the single-step bit to do this. */ lwz r12,THREAD_DBCR0(r12) - andis. r12,r12,[EMAIL PROTECTED] + andis. r12,r12,(DBCR0_IC | 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 */ @@ -241,7 +241,7 @@ /* If the process has its own DBCR0 value, load it up. The single step bit tells us that dbcr0 should be loaded. */ lwz r0,THREAD+THREAD_DBCR0(r2) - andis. r10,r0,[EMAIL PROTECTED] + andis. r10,r0,(DBCR0_IC | DBSR_DAC1R | DBSR_DAC1W)@h bnel- load_dbcr0 #endif #ifdef CONFIG_44x @@ -669,7 +669,7 @@ /* Check whether this process has its own DBCR0 value. The single step bit tells us that dbcr0 should be loaded. */ lwz r0,THREAD+THREAD_DBCR0(r2) - andis. r10,r0,[EMAIL PROTECTED] + andis. r10,r0,(DBCR0_IC | DBSR_DAC1R | DBSR_DAC1W)@h bnel- load_dbcr0 #endif Index: linux-2.6.25.1/include/asm-ppc/system.h =================================================================== --- linux-2.6.25.1.orig/include/asm-ppc/system.h 2008-05-01 14:45:25.000000000 -0700 +++ linux-2.6.25.1/include/asm-ppc/system.h 2008-05-21 08:26:28.000000000 -0700 @@ -56,6 +56,12 @@ extern void hard_reset_now(void); extern void poweroff_now(void); extern int set_dabr(unsigned long dabr); + +/* These are 4XX-specific functions for debugging register manipulations. */ +extern int set_dac(unsigned long dac); +extern unsigned int get_dac(void); +extern int set_dbcr0(unsigned long dbcr); + #ifdef CONFIG_6xx extern long _get_L2CR(void); extern long _get_L3CR(void);
_______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev