From mboxrd@z Thu Jan 1 00:00:00 1970 In-Reply-To: <410726E9.9050804@acm.org> References: <410726E9.9050804@acm.org> Mime-Version: 1.0 (Apple Message framework v618) Content-Type: text/plain; charset=US-ASCII; format=flowed Message-Id: <50EB8FA3-E09D-11D8-9538-000393DBC2E8@freescale.com> Cc: From: Kumar Gala Subject: Re: Patch for debug setcontext Date: Wed, 28 Jul 2004 08:52:07 -0500 To: Corey Minyard Sender: owner-linuxppc-dev@lists.linuxppc.org List-Id: On Jul 27, 2004, at 11:09 PM, Corey Minyard wrote: > The attached patch adds the ability to do a setcontext and set up debug > information in the kernel. It lets you set a context and turn on > single-stepping and branch tracing. > I am using this to enable an in-application debugger. Basically, you > can insert traps and cause a signal (or cause a signal via other > means). From the signal handler you can single step or branch trace > using this patch. The single-stepping is important so that an > instruction may be emulated in a different location so the trap doesn't > have to be replaced. It is also used to step threads out of critical > areas in some cases. > In the future, even cooler things could be added to this framework like > support for setting up hardware breakpoints. I have done this in the > past in another job and it can be quite handy. > I have tested this on 440GP and 750. The patch is relative to stock > 2.6.7. > The most problematic part of this patch is probably the 4xx part using > the values in the dbcr0 register to figure out if it needs to be > loaded. I did it this way because it seems the easiest way to do it > since you can't just use PT_PTRACE to know if the register needs to be > loaded. I didn't see any problems but I don't know if there are any > races that might cause problems with this method. > Signed-off-by: Corey Minyard > > Index: linux-ipmi/arch/ppc/kernel/entry.S > =================================================================== > --- linux-ipmi.orig/arch/ppc/kernel/entry.S 2004-06-22 > 13:31:41.000000000 -0500 > +++ linux-ipmi/arch/ppc/kernel/entry.S 2004-07-27 21:14:55.000000000 > -0500 > @@ -111,8 +111,10 @@ > addi r11,r1,STACK_FRAME_OVERHEAD > stw r11,PT_REGS(r12) > #if defined(CONFIG_40x) || defined(CONFIG_BOOKE) > - lwz r12,PTRACE-THREAD(r12) > - andi. r12,r12,PT_PTRACED > + /* 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,DBCR0_IC@h > beq+ 3f > /* From user and task is ptraced - load up global dbcr0 */ > li r12,-1 /* clear all pending debug events */ > @@ -242,9 +244,10 @@ > bne- syscall_exit_work > syscall_exit_cont: > #ifdef CONFIG_4xx > - /* If the process has its own DBCR0 value, load it up */ > - lwz r0,PTRACE(r2) > - andi. r0,r0,PT_PTRACED > + /* 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,DBCR0_IC@h > bnel- load_4xx_dbcr0 > #endif > stwcx. r0,0,r1 /* to clear the reservation */ > @@ -590,9 +593,10 @@ > restore_user: > #ifdef CONFIG_4xx > - /* Check whether this process has its own DBCR0 value */ > - lwz r0,PTRACE(r2) > - andi. r0,r0,PT_PTRACED > + /* 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,DBCR0_IC@h > bnel- load_4xx_dbcr0 > #endif > @@ -867,14 +871,13 @@ > * having first saved away the global DBCR0. > */ > load_4xx_dbcr0: > - mfmsr r0 /* first disable debug exceptions */ > - rlwinm r0,r0,0,~MSR_DE > - mtmsr r0 > + mfmsr r10 /* first disable debug exceptions */ > + rlwinm r10,r10,0,~MSR_DE > + mtmsr r10 > isync > mfspr r10,SPRN_DBCR0 > lis r11,global_dbcr0@ha > addi r11,r11,global_dbcr0@l > - lwz r0,THREAD+THREAD_DBCR0(r2) > stw r10,0(r11) > mtspr SPRN_DBCR0,r0 > lwz r10,4(r11) > Index: linux-ipmi/arch/ppc/kernel/misc.S > =================================================================== > --- linux-ipmi.orig/arch/ppc/kernel/misc.S 2004-06-22 > 13:31:41.000000000 -0500 > +++ linux-ipmi/arch/ppc/kernel/misc.S 2004-07-27 15:14:06.000000000 > -0500 > @@ -1385,7 +1385,7 @@ > .long sys_fstatfs64 > .long ppc_fadvise64_64 > .long sys_ni_syscall /* 255 - rtas (used on ppc64) */ > - .long sys_ni_syscall /* 256 reserved for sys_debug_setcontext */ > + .long sys_debug_setcontext > .long sys_ni_syscall /* 257 reserved for vserver */ > .long sys_ni_syscall /* 258 reserved for new sys_remap_file_pages */ > .long sys_ni_syscall /* 259 reserved for new sys_mbind */ > Index: linux-ipmi/arch/ppc/kernel/signal.c > =================================================================== > --- linux-ipmi.orig/arch/ppc/kernel/signal.c 2004-06-22 > 13:28:05.000000000 -0500 > +++ linux-ipmi/arch/ppc/kernel/signal.c 2004-07-27 21:12:25.000000000 > -0500 > @@ -462,6 +462,96 @@ > return 0; > } > +int sys_debug_setcontext(struct ucontext __user *ctx, > + int ndbg, struct sig_dbg_op *dbg, > + int r6, int r7, int r8, > + struct pt_regs *regs) > +{ > + struct sig_dbg_op op; > + int i; > + unsigned long new_msr = regs->msr; > +#if defined(CONFIG_4xx) > + unsigned long new_dbcr0 = current->thread.dbcr0; > +#endif I'm guessing, this should really be #if defined(CONFIG_4xx) || defined(CONFIG_BOOKE) There are a number of similar cases later in this code as well. > + > + for (i=0; i + if (__copy_from_user(&op, dbg, sizeof(op))) > + return -EFAULT; > + switch (op.dbg_type) { > + case SIG_DBG_SINGLE_STEPPING: > +#if defined(CONFIG_4xx) > + if (op.dbg_value) { > + new_msr |= MSR_DE; > + new_dbcr0 |= (DBCR0_IDM | DBCR0_IC); > + } else { > + new_msr &= ~MSR_DE; > + new_dbcr0 &= ~(DBCR0_IDM | DBCR0_IC); > + } > +#else > + if (op.dbg_value) > + new_msr |= MSR_SE; > + else > + new_msr &= ~MSR_SE; > +#endif > + break; > + case SIG_DBG_BRANCH_TRACING: > +#if defined(CONFIG_4xx) > + return -EINVAL; > +#else > + if (op.dbg_value) > + new_msr |= MSR_BE; > + else > + new_msr &= ~MSR_BE; > +#endif > + break; > + > + default: > + return -EINVAL; > + } > + } > + > + /* We wait until here to actually install the values in the > + registers so if we fail in the above loop, it will not > + affect the contents of these registers. After this point, > + failure is a problem, anyway, and it's very unlikely unless > + the user is really doing something wrong. */ > + regs->msr = new_msr; > +#if defined(CONFIG_4xx) > + current->thread.dbcr0 = new_dbcr0; > +#endif > + > + /* > + * If we get a fault copying the context into the kernel's > + * image of the user's registers, we can't just return -EFAULT > + * because the user's registers will be corrupted. For instance > + * the NIP value may have been updated but not some of the > + * other registers. Given that we have done the verify_area > + * and successfully read the first and last bytes of the region > + * above, this should only happen in an out-of-memory situation > + * or if another thread unmaps the region containing the context. > + * We kill the task with a SIGSEGV in this situation. > + */ > + if (do_setcontext(ctx, regs, 1)) { > + force_sig(SIGSEGV, current); > + goto out; > + } > + > + /* > + * It's not clear whether or why it is desirable to save the > + * sigaltstack setting on signal delivery and restore it on > + * signal return. But other architectures do this and we have > + * always done it up until now so it is probably better not to > + * change it. -- paulus > + */ > + do_sigaltstack(&ctx->uc_stack, NULL, regs->gpr[1]); > + > + sigreturn_exit(regs); > + /* doesn't actually return back to here */ > + > + out: > + return 0; > +} > + > /* > * OK, we're invoking a handler > */ > Index: linux-ipmi/arch/ppc/kernel/traps.c > =================================================================== > --- linux-ipmi.orig/arch/ppc/kernel/traps.c 2004-06-22 > 13:31:41.000000000 -0500 > +++ linux-ipmi/arch/ppc/kernel/traps.c 2004-07-27 15:55:49.000000000 > -0500 > @@ -516,7 +516,7 @@ > void > SingleStepException(struct pt_regs *regs) > { > - regs->msr &= ~MSR_SE; /* Turn off 'trace' bit */ > + regs->msr &= ~(MSR_SE | MSR_BE); /* Turn off 'trace' bits */ > if (debugger_sstep(regs)) > return; > _exception(SIGTRAP, regs, TRAP_TRACE, 0); > Index: linux-ipmi/include/asm-ppc/signal.h > =================================================================== > --- linux-ipmi.orig/include/asm-ppc/signal.h 2003-12-17 > 20:58:08.000000000 -0600 > +++ linux-ipmi/include/asm-ppc/signal.h 2004-07-27 15:06:55.000000000 > -0500 > @@ -153,4 +153,23 @@ > #define ptrace_signal_deliver(regs, cookie) do { } while (0) > #endif /* __KERNEL__ */ > +/* > + * These are parameters to dbg_sigreturn syscall. They enable or > + * disable certain debugging things that can be done from signal > + * handlers. The dbg_sigreturn syscall *must* be called from a > + * SA_SIGINFO signal so the ucontext can be passed to it. It takes an > + * array of struct sig_dbg_op, which has the debug operations to > + * perform before returning from the signal. > + */ > +struct sig_dbg_op { > + int dbg_type; > + unsigned long dbg_value; > +}; > + > +/* Enable or disable single-stepping. The value sets the state. */ > +#define SIG_DBG_SINGLE_STEPPING 1 > + > +/* Enable or disable branch tracing. The value sets the state. */ > +#define SIG_DBG_BRANCH_TRACING 2 > + > #endif > Index: linux-ipmi/include/asm-ppc/unistd.h > =================================================================== > --- linux-ipmi.orig/include/asm-ppc/unistd.h 2004-06-22 > 13:32:19.000000000 -0500 > +++ linux-ipmi/include/asm-ppc/unistd.h 2004-07-27 15:04:30.000000000 > -0500 > @@ -260,7 +260,7 @@ > #define __NR_fstatfs64 253 > #define __NR_fadvise64_64 254 > #define __NR_rtas 255 > -/* Number 256 is reserved for sys_debug_setcontext */ > +#define __NR_sys_debug_setcontext 256 > /* Number 257 is reserved for vserver */ > /* Number 258 is reserved for new sys_remap_file_pages */ > /* Number 259 is reserved for new sys_mbind */ ** Sent via the linuxppc-dev mail list. See http://lists.linuxppc.org/