A checkpointed task image may specify a value for the DABR (Data Access Breakpoint Register). The restart code needs to validate this value before making any changes to the current task.
ptrace_set_debugreg encapsulates the bounds checking and platform dependencies of programming the DABR. Split this into "validate" (debugreg_valid) and "update" (debugreg_update) functions, and make them available for use outside of the ptrace code. Also ptrace_set_debugreg has extern linkage, but no users outside of ptrace.c. Make it static. Signed-off-by: Nathan Lynch <n...@pobox.com> --- arch/powerpc/include/asm/ptrace.h | 6 +++ arch/powerpc/kernel/ptrace.c | 68 ++++++++++++++++++++---------------- 2 files changed, 44 insertions(+), 30 deletions(-) Is something like this okay to carry in the checkpoint/restart patches? diff --git a/arch/powerpc/include/asm/ptrace.h b/arch/powerpc/include/asm/ptrace.h index c9c678f..1b3a5f0 100644 --- a/arch/powerpc/include/asm/ptrace.h +++ b/arch/powerpc/include/asm/ptrace.h @@ -81,6 +81,8 @@ struct pt_regs { #ifndef __ASSEMBLY__ +#include <linux/types.h> + #define instruction_pointer(regs) ((regs)->nip) #define user_stack_pointer(regs) ((regs)->gpr[1]) #define regs_return_value(regs) ((regs)->gpr[3]) @@ -138,6 +140,10 @@ do { \ extern void user_enable_single_step(struct task_struct *); extern void user_disable_single_step(struct task_struct *); +/* for reprogramming DABR/DAC during restart of a checkpointed task */ +extern bool debugreg_valid(unsigned long val); +extern void debugreg_update(struct task_struct *task, unsigned long val); + #endif /* __ASSEMBLY__ */ #endif /* __KERNEL__ */ diff --git a/arch/powerpc/kernel/ptrace.c b/arch/powerpc/kernel/ptrace.c index 3635be6..60259c6 100644 --- a/arch/powerpc/kernel/ptrace.c +++ b/arch/powerpc/kernel/ptrace.c @@ -735,22 +735,13 @@ void user_disable_single_step(struct task_struct *task) clear_tsk_thread_flag(task, TIF_SINGLESTEP); } -int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, - unsigned long data) +bool debugreg_valid(unsigned long val) { - /* 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 in dabr are flags */ - if ((data & ~0x7UL) >= TASK_SIZE) - return -EIO; + if ((val & ~0x7UL) >= TASK_SIZE) + return false; #ifndef 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 @@ -764,47 +755,64 @@ int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, */ /* 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_BOOKE) - + if (val && !(val & DABR_TRANSLATION)) + return false; +#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. */ + /* Read or Write bits must be set */ + if (!(val & 0x3UL)) + return -EINVAL; +#endif + return true; +} + +void debugreg_update(struct task_struct *task, unsigned long val) +{ +#ifndef CONFIG_BOOKE + task->thread.dabr = val; +#else /* DAC's hold the whole address without any mode flags */ - task->thread.dabr = data & ~0x3UL; + task->thread.dabr = val & ~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) + if (val & 0x1UL) task->thread.dbcr0 |= DBSR_DAC1R; - if (data & 0x2UL) + if (val & 0x2UL) task->thread.dbcr0 |= DBSR_DAC1W; task->thread.regs->msr |= MSR_DE; #endif +} + +static int ptrace_set_debugreg(struct task_struct *task, unsigned long addr, + unsigned long data) +{ + /* 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; + + if (!debugreg_valid(data)) + return -EIO; + + debugreg_update(task, data); + return 0; } -- 1.6.0.6 _______________________________________________ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev