From: "K.Prasad" <prasad@linux•vnet.ibm.com>
To: Josh Boyer <jwboyer@linux•vnet.ibm.com>
Cc: linuxppc-dev@ozlabs•org,
Benjamin Herrenschmidt <benh@au1•ibm.com>,
paulus@samba•org
Subject: Re: [RFC] Hardware Breakpoint interfaces implementation for PPC64
Date: Wed, 13 May 2009 01:55:45 +0530 [thread overview]
Message-ID: <20090512202545.GE6033@in.ibm.com> (raw)
In-Reply-To: <20090512115149.GA1885@yoda.jdub.homelinux.org>
On Tue, May 12, 2009 at 07:51:49AM -0400, Josh Boyer wrote:
> On Tue, May 12, 2009 at 01:33:55AM +0530, K.Prasad wrote:
> >Hi PPC Dev folks,
> > Please find a patch below that implements the proposed Hardware
> >Breakpoint interfaces for PPC64 architecture.
> >
> >As a brief introduction, the proposed Hardware Breakpoint
> >infrastructure provides generic in-kernel interfaces to which users
> >from kernel- and user-space can request for breakpoint registers. An
> >ftrace plugin that can trace accesses to data variables is also part
> >of the generic HW Breakpoint interface patchset. The latest submission
> >for this patchset along with an x86 implementation can be found here:
> >http://lkml.org/lkml/2009/5/11/159.
> >
> >The following are the salient features of the PPC64 patch.
> >
> >- Arch-specific definitions for kernel and user-space requests are
> > defined in arch/powerpc/kernel/hw_breakpoint.c
> >- Ptrace is converted to use the HW Breakpoint interfaces
> >- The ftrace plugin called ksym_tracer is tested to work fine. For
> > instance when tracing pid_max kernel variable, the following was
> > obtained as output
> >
> ># cat trace
> ># tracer: ksym_tracer
> >#
> ># TASK-PID CPU# Symbol Type Function
> ># | | | | |
> >bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0x78/0x10c
> >bash 4502 3 pid_max RW .do_proc_dointvec_minmax_conv+0xa0/0x10c
> >bash 4502 3 pid_max RW .alloc_pid+0x8c/0x4a4
> >
> >There are however a few limitations/caveats of the patch as identified
> >below:
> >
> >- The patch is currently implemented only for PPC64 architecture. Other
> > architectures (especially Book-E implementations are expected to
> > happen in due course).
>
> Does this mean you will work on transitioning Book-E implementations, or that
> you expect the Book-E maintainers to? I'm just curious. The code as written
> relies heavily on the DABR/MSR setup that ppc64 has and Book-E unfortunately
> doesn't follow that at all.
>
> Book-E also allows for more than one HW breakpoint, which means you're growing
> the thread_struct by 32-bytes to support 4 of them. 64-bytes if this ever
> supports DAC events. Have you thought at all about a way to support this
> without carrying around the data in the thread struct?
>
> <snip>
>
The idea behind embedding the physical debug register values in
thread_struct is to use its synchronisation mechanisms for HW Breakpoint
related fields too. These values were originally maintained in a
separate structure whose pointer lay in thread_struct but was modified
based on a suggestion from Ingo Molnar (here:
http://lkml.org/lkml/2009/3/10/210).
I do see that Book-E processors will have severe memory footprint
constraints (in embedded environment) and if the maintainers carry a
different perspective (than the one cited above), the relevant fields
can be migrated to a new structure whose pointer will be embedded in
task_struct. The generic code may have to carry some #ifdefs though.
> >Index: linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> >===================================================================
> >--- linux-2.6-tip.hbkpt.orig/arch/powerpc/mm/fault.c
> >+++ linux-2.6-tip.hbkpt/arch/powerpc/mm/fault.c
> >@@ -137,6 +137,12 @@ int __kprobes do_page_fault(struct pt_re
> > error_code &= 0x48200000;
> > else
> > is_write = error_code & DSISR_ISSTORE;
> >+
> >+ if (error_code & DSISR_DABRMATCH) {
> >+ /* DABR match */
> >+ do_dabr(regs, address, error_code);
> >+ return 0;
> >+ }
> > #else
> > is_write = error_code & ESR_DST;
> > #endif /* CONFIG_4xx || CONFIG_BOOKE */
> >@@ -151,14 +157,6 @@ int __kprobes do_page_fault(struct pt_re
> > if (!user_mode(regs) && (address >= TASK_SIZE))
> > return SIGSEGV;
> >
> >-#if !(defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
> >- if (error_code & DSISR_DABRMATCH) {
> >- /* DABR match */
> >- do_dabr(regs, address, error_code);
> >- return 0;
> >- }
> >-#endif /* !(CONFIG_4xx || CONFIG_BOOKE)*/
> >-
> > if (in_atomic() || mm == NULL) {
> > if (!user_mode(regs))
> > return SIGSEGV;
>
>
> I don't understand why this was changed, and the changelog doesn't highlight it.
>
> josh
The intention is to capture the exception much before kprobes and xmon
do. The HW Breakpoint exception handler will return NOTIFY_DONE if the
exception doesn't belong to it and doesn't harm the rest. kprobes has
been tested to work fine alongwith HW Breakpoints.
I will add a description about this change in the next iteration of the
patch.
Thanks,
K.Prasad
next prev parent reply other threads:[~2009-05-12 20:25 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-05-11 20:03 [RFC] Hardware Breakpoint interfaces implementation for PPC64 K.Prasad
2009-05-12 0:56 ` Michael Neuling
2009-05-12 2:48 ` Michael Neuling
2009-05-12 20:01 ` K.Prasad
2009-05-12 11:51 ` Josh Boyer
2009-05-12 16:47 ` Scott Wood
2009-05-12 20:28 ` K.Prasad
2009-05-12 20:25 ` K.Prasad [this message]
2009-05-13 2:57 ` David Gibson
2009-05-13 3:00 ` David Gibson
2009-05-14 18:52 ` K.Prasad
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20090512202545.GE6033@in.ibm.com \
--to=prasad@linux$(echo .)vnet.ibm.com \
--cc=benh@au1$(echo .)ibm.com \
--cc=jwboyer@linux$(echo .)vnet.ibm.com \
--cc=linuxppc-dev@ozlabs$(echo .)org \
--cc=paulus@samba$(echo .)org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox