* ppc_set_hwdebug vs ptrace_set_debugreg @ 2010-11-27 19:36 Andreas Schwab 2010-11-29 7:22 ` K.Prasad 0 siblings, 1 reply; 11+ messages in thread From: Andreas Schwab @ 2010-11-27 19:36 UTC (permalink / raw) To: linuxppc-dev; +Cc: Dave Kleikamp, K.Prasad Why does ptrace_set_debugreg call register_user_hw_breakpoint, but ppc_set_hwdebug doesn't? Shouldn't ppc_set_hwdebug set the DABR_DATA_(READ|WRITE|TRANSLATION) bits in the dabr? Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-11-27 19:36 ppc_set_hwdebug vs ptrace_set_debugreg Andreas Schwab @ 2010-11-29 7:22 ` K.Prasad 2010-11-29 10:15 ` Andreas Schwab 0 siblings, 1 reply; 11+ messages in thread From: K.Prasad @ 2010-11-29 7:22 UTC (permalink / raw) To: Andreas Schwab; +Cc: linuxppc-dev, Dave Kleikamp On Sat, Nov 27, 2010 at 08:36:30PM +0100, Andreas Schwab wrote: > Why does ptrace_set_debugreg call register_user_hw_breakpoint, but > ppc_set_hwdebug doesn't? Shouldn't ppc_set_hwdebug set the > DABR_DATA_(READ|WRITE|TRANSLATION) bits in the dabr? > > Andreas. The hw-breakpoint interfaces were initially planned for the old ptrace option PTRACE_SET_DEBUGREG,while, the newer ptrace options are mostly to exploit the advanced debug features of Book3E processors. Although ppc_set_hwdebug() can set DABR through set_dabr() in arch/powerpc/kernel/process.c, it is good to have it converted to use register_user_hw_breakpoint(). This was planned to be done alongside the conversion of all ptrace options enabled by CONFIG_PPC_ADV_DEBUG_REGS, which is yet to be done. Are you looking to use debug registers through perf, or somesuch, that you need register_user_hw_breakpoint() to be used for these new ptrace flags? Thanks, K.Prasad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-11-29 7:22 ` K.Prasad @ 2010-11-29 10:15 ` Andreas Schwab 2010-12-01 4:37 ` K.Prasad 0 siblings, 1 reply; 11+ messages in thread From: Andreas Schwab @ 2010-11-29 10:15 UTC (permalink / raw) To: prasad; +Cc: linuxppc-dev, Dave Kleikamp "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > Although ppc_set_hwdebug() can set DABR through set_dabr() in > arch/powerpc/kernel/process.c, it is good to have it converted to use > register_user_hw_breakpoint(). What do you mean with "good to have"? It doesn't work without it unless I disable PERF_EVENTS (which is the only way to disable HAVE_HW_BREAKPOINT). Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-11-29 10:15 ` Andreas Schwab @ 2010-12-01 4:37 ` K.Prasad 2010-12-13 8:26 ` K.Prasad 0 siblings, 1 reply; 11+ messages in thread From: K.Prasad @ 2010-12-01 4:37 UTC (permalink / raw) To: Andreas Schwab; +Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju On Mon, Nov 29, 2010 at 11:15:51AM +0100, Andreas Schwab wrote: > "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > > > Although ppc_set_hwdebug() can set DABR through set_dabr() in > > arch/powerpc/kernel/process.c, it is good to have it converted to use > > register_user_hw_breakpoint(). > > What do you mean with "good to have"? It doesn't work without it unless > I disable PERF_EVENTS (which is the only way to disable > HAVE_HW_BREAKPOINT). > > Andreas. > Let me see if I can cook up a patch for this i.e. make set_dabr() invoke register_user_hw_breakpoint() when CONFIG_PPC_BOOK3S is defined; before I head out on my vacation (starting second week of this month). Thanks, K.Prasad ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-12-01 4:37 ` K.Prasad @ 2010-12-13 8:26 ` K.Prasad 2010-12-13 19:05 ` Andreas Schwab 0 siblings, 1 reply; 11+ messages in thread From: K.Prasad @ 2010-12-13 8:26 UTC (permalink / raw) To: Andreas Schwab Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras On Wed, Dec 01, 2010 at 10:07:58AM +0530, K.Prasad wrote: > On Mon, Nov 29, 2010 at 11:15:51AM +0100, Andreas Schwab wrote: > > "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > > > > > Although ppc_set_hwdebug() can set DABR through set_dabr() in > > > arch/powerpc/kernel/process.c, it is good to have it converted to use > > > register_user_hw_breakpoint(). > > > > What do you mean with "good to have"? It doesn't work without it unless > > I disable PERF_EVENTS (which is the only way to disable > > HAVE_HW_BREAKPOINT). > > > > Andreas. > > > > Let me see if I can cook up a patch for this i.e. make set_dabr() invoke > register_user_hw_breakpoint() when CONFIG_PPC_BOOK3S is defined; before I > head out on my vacation (starting second week of this month). > > Thanks, > K.Prasad > Hi, Can you check if the following patch (compile tested only) works for you? Thanks, K.Prasad Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com> --- arch/powerpc/kernel/ptrace.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) Index: linux-2.6.set_hwdebug/arch/powerpc/kernel/ptrace.c =================================================================== --- linux-2.6.set_hwdebug.orig/arch/powerpc/kernel/ptrace.c +++ linux-2.6.set_hwdebug/arch/powerpc/kernel/ptrace.c @@ -1316,6 +1316,11 @@ static int set_dac_range(struct task_str static long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_info) { +#ifdef CONFIG_HAVE_HW_BREAKPOINT + struct perf_event *bp; + struct perf_event_attr attr; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ + if (bp_info->version != 1) return -ENOTSUPP; #ifdef CONFIG_PPC_ADV_DEBUG_REGS @@ -1362,10 +1367,29 @@ static long ppc_set_hwdebug(struct task_ if (child->thread.dabr) return -ENOSPC; +#ifdef CONFIG_HAVE_HW_BREAKPOINT + if (child->thread->ptrace_bps[0]) + return -ENOSPC; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ if ((unsigned long)bp_info->addr >= TASK_SIZE) return -EIO; +#ifdef CONFIG_HAVE_HW_BREAKPOINT + /* Create a new breakpoint request if one doesn't exist already */ + hw_breakpoint_init(&attr); + attr.bp_addr = bp_info->addr & ~HW_BREAKPOINT_ALIGN; + arch_bp_generic_fields(bp_info->addr & + (DABR_DATA_WRITE | DABR_DATA_READ), + &attr.bp_type); + + bp = register_user_hw_breakpoint(&attr, ptrace_triggered, task); + if (IS_ERR(bp)) + return PTR_ERR(bp); + + child->thread.ptrace_bps[0] = bp; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ + child->thread.dabr = (unsigned long)bp_info->addr; return 1; @@ -1395,6 +1419,16 @@ static long ppc_del_hwdebug(struct task_ return -EINVAL; if (child->thread.dabr == 0) return -ENOENT; +#ifdef CONFIG_HAVE_HW_BREAKPOINT + /* + * There is no way by which address in ptrace_bps[0] and thread.dabr + * can be different. So we don't explicitly check if they're the same + */ + if (child->thread.ptrace_bps[0]) { + unregister_hw_breakpoint(child->thread.ptrace_bps[0]); + child->thread.ptrace_bps[0] = NULL; + } +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ child->thread.dabr = 0; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-12-13 8:26 ` K.Prasad @ 2010-12-13 19:05 ` Andreas Schwab 2010-12-14 12:54 ` K.Prasad 0 siblings, 1 reply; 11+ messages in thread From: Andreas Schwab @ 2010-12-13 19:05 UTC (permalink / raw) To: prasad; +Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > +#ifdef CONFIG_HAVE_HW_BREAKPOINT > + /* Create a new breakpoint request if one doesn't exist already */ > + hw_breakpoint_init(&attr); > + attr.bp_addr = bp_info->addr & ~HW_BREAKPOINT_ALIGN; > + arch_bp_generic_fields(bp_info->addr & > + (DABR_DATA_WRITE | DABR_DATA_READ), > + &attr.bp_type); > + > + bp = register_user_hw_breakpoint(&attr, ptrace_triggered, task); > + if (IS_ERR(bp)) > + return PTR_ERR(bp); > + > + child->thread.ptrace_bps[0] = bp; > +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ > + > child->thread.dabr = (unsigned long)bp_info->addr; That cannot work, see <http://permalink.gmane.org/gmane.linux.ports.ppc64.devel/71418>. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-12-13 19:05 ` Andreas Schwab @ 2010-12-14 12:54 ` K.Prasad 2010-12-14 18:59 ` Andreas Schwab 2010-12-16 17:07 ` Andreas Schwab 0 siblings, 2 replies; 11+ messages in thread From: K.Prasad @ 2010-12-14 12:54 UTC (permalink / raw) To: Andreas Schwab Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras On Mon, Dec 13, 2010 at 08:05:36PM +0100, Andreas Schwab wrote: > "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > > > +#ifdef CONFIG_HAVE_HW_BREAKPOINT > > + /* Create a new breakpoint request if one doesn't exist already */ > > + hw_breakpoint_init(&attr); > > + attr.bp_addr = bp_info->addr & ~HW_BREAKPOINT_ALIGN; > > + arch_bp_generic_fields(bp_info->addr & > > + (DABR_DATA_WRITE | DABR_DATA_READ), > > + &attr.bp_type); > > + > > + bp = register_user_hw_breakpoint(&attr, ptrace_triggered, task); > > + if (IS_ERR(bp)) > > + return PTR_ERR(bp); > > + > > + child->thread.ptrace_bps[0] = bp; > > +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ > > + > > child->thread.dabr = (unsigned long)bp_info->addr; > > That cannot work, see > <http://permalink.gmane.org/gmane.linux.ports.ppc64.devel/71418>. > Ok. The above patch makes it a bit easy. How about the revised patch below? It is only compile-tested; have you got a quick test case that I can run? Enable PPC_PTRACE_SETHWDEBUG and PPC_PTRACE_DELHWDEBUG to use the generic hardware breakpoint interfaces. This helps prevent conflict for the use of DABR register in the absence of CONFIG_PPC_ADV_DEBUG_REGS and when PTRACE_SET_DEBUGREG/PTRACE_GET_DEBUGREG flags are used by ptrace. Signed-off-by: K.Prasad <prasad@linux.vnet.ibm.com> --- arch/powerpc/kernel/ptrace.c | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) Index: linux-2.6.set_hwdebug/arch/powerpc/kernel/ptrace.c =================================================================== --- linux-2.6.set_hwdebug.orig/arch/powerpc/kernel/ptrace.c +++ linux-2.6.set_hwdebug/arch/powerpc/kernel/ptrace.c @@ -1316,6 +1316,10 @@ static int set_dac_range(struct task_str static long ppc_set_hwdebug(struct task_struct *child, struct ppc_hw_breakpoint *bp_info) { +#ifdef CONFIG_HAVE_HW_BREAKPOINT + struct perf_event *bp; + struct perf_event_attr attr; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ #ifndef CONFIG_PPC_ADV_DEBUG_REGS unsigned long dabr; #endif @@ -1365,6 +1369,10 @@ static long ppc_set_hwdebug(struct task_ if (child->thread.dabr) return -ENOSPC; +#ifdef CONFIG_HAVE_HW_BREAKPOINT + if (child->thread.ptrace_bps[0]) + return -ENOSPC; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ if ((unsigned long)bp_info->addr >= TASK_SIZE) return -EIO; @@ -1376,6 +1384,20 @@ static long ppc_set_hwdebug(struct task_ if (bp_info->trigger_type & PPC_BREAKPOINT_TRIGGER_WRITE) dabr |= DABR_DATA_WRITE; +#ifdef CONFIG_HAVE_HW_BREAKPOINT + /* Create a new breakpoint request if one doesn't exist already */ + hw_breakpoint_init(&attr); + attr.bp_addr = dabr & ~HW_BREAKPOINT_ALIGN; + arch_bp_generic_fields(dabr & (DABR_DATA_WRITE | DABR_DATA_READ), + &attr.bp_type); + + bp = register_user_hw_breakpoint(&attr, ptrace_triggered, child); + if (IS_ERR(bp)) + return PTR_ERR(bp); + + child->thread.ptrace_bps[0] = bp; +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ + child->thread.dabr = dabr; return 1; @@ -1405,6 +1427,16 @@ static long ppc_del_hwdebug(struct task_ return -EINVAL; if (child->thread.dabr == 0) return -ENOENT; +#ifdef CONFIG_HAVE_HW_BREAKPOINT + /* + * There is no way by which address in ptrace_bps[0] and thread.dabr + * can be different. So we don't explicitly check if they're the same + */ + if (child->thread.ptrace_bps[0]) { + unregister_hw_breakpoint(child->thread.ptrace_bps[0]); + child->thread.ptrace_bps[0] = NULL; + } +#endif /* CONFIG_HAVE_HW_BREAKPOINT */ child->thread.dabr = 0; ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-12-14 12:54 ` K.Prasad @ 2010-12-14 18:59 ` Andreas Schwab 2010-12-16 17:07 ` Andreas Schwab 1 sibling, 0 replies; 11+ messages in thread From: Andreas Schwab @ 2010-12-14 18:59 UTC (permalink / raw) To: prasad; +Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > How about the revised patch below? It is only compile-tested; have you > got a quick test case that I can run? Try the watchpoint tests in gdb. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-12-14 12:54 ` K.Prasad 2010-12-14 18:59 ` Andreas Schwab @ 2010-12-16 17:07 ` Andreas Schwab 2011-01-02 12:54 ` K.Prasad 1 sibling, 1 reply; 11+ messages in thread From: Andreas Schwab @ 2010-12-16 17:07 UTC (permalink / raw) To: prasad; +Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > How about the revised patch below? It is only compile-tested; have you > got a quick test case that I can run? It crashes the kernel when running the watch-vfork test. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2010-12-16 17:07 ` Andreas Schwab @ 2011-01-02 12:54 ` K.Prasad 2011-01-02 14:58 ` Andreas Schwab 0 siblings, 1 reply; 11+ messages in thread From: K.Prasad @ 2011-01-02 12:54 UTC (permalink / raw) To: Andreas Schwab Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras On Thu, Dec 16, 2010 at 06:07:47PM +0100, Andreas Schwab wrote: > "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > > > How about the revised patch below? It is only compile-tested; have you > > got a quick test case that I can run? > > It crashes the kernel when running the watch-vfork test. > > Andreas. > Hi Andreas, I tried running it multiple times but saw no crash (or error messages in dmesg). Can you send me the crash logs? What's the behaviour when the testcase is run on an unpatched kernel? The watch-vfork test actually fails on my system (4 unexpected failures) irrespective of the kernel containing the patch or not. Thanks, K.Prasad P.S.: I'd been on vacation and couldn't look at this issue during then. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: ppc_set_hwdebug vs ptrace_set_debugreg 2011-01-02 12:54 ` K.Prasad @ 2011-01-02 14:58 ` Andreas Schwab 0 siblings, 0 replies; 11+ messages in thread From: Andreas Schwab @ 2011-01-02 14:58 UTC (permalink / raw) To: prasad; +Cc: linuxppc-dev, Dave Kleikamp, Srikar Dronamraju, Paul Mackerras "K.Prasad" <prasad@linux.vnet.ibm.com> writes: > The watch-vfork test actually fails on my system (4 unexpected failures) It should pass all four tests. If gdb cannot even set a watchpoint it cannot trigger the crash, of course. Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-01-02 14:58 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-11-27 19:36 ppc_set_hwdebug vs ptrace_set_debugreg Andreas Schwab 2010-11-29 7:22 ` K.Prasad 2010-11-29 10:15 ` Andreas Schwab 2010-12-01 4:37 ` K.Prasad 2010-12-13 8:26 ` K.Prasad 2010-12-13 19:05 ` Andreas Schwab 2010-12-14 12:54 ` K.Prasad 2010-12-14 18:59 ` Andreas Schwab 2010-12-16 17:07 ` Andreas Schwab 2011-01-02 12:54 ` K.Prasad 2011-01-02 14:58 ` Andreas Schwab
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).