On Thu, 07 Jun 2018 17:07:00 +0530
"Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx> wrote:
> Masami Hiramatsu wrote:
> > Don't call the ->break_handler() from the arm kprobes code,
> ^^^ powerpc
>
> > because it was only used by jprobes which got removed.
> >
> > This also makes skip_singlestep() a static function since
> > only ftrace-kprobe.c is using this function.
> >
> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> > Cc: Paul Mackerras <paulus@xxxxxxxxx>
> > Cc: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
> > Cc: "Naveen N. Rao" <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
> > Cc: linuxppc-dev@xxxxxxxxxxxxxxxx
> > ---
> > arch/powerpc/include/asm/kprobes.h | 10 ----------
> > arch/powerpc/kernel/kprobes-ftrace.c | 16 +++-------------
> > arch/powerpc/kernel/kprobes.c | 31 +++++++++++--------------------
> > 3 files changed, 14 insertions(+), 43 deletions(-)
>
> With 2 small comments...
2 ? or 1 ?
> Acked-by: Naveen N. Rao <naveen.n.rao@xxxxxxxxxxxxxxxxxx>
>
> - Naveen
>
> >
> > diff --git a/arch/powerpc/include/asm/kprobes.h b/arch/powerpc/include/asm/kprobes.h
> > index 674036db558b..785c464b6588 100644
> > --- a/arch/powerpc/include/asm/kprobes.h
> > +++ b/arch/powerpc/include/asm/kprobes.h
> > @@ -102,16 +102,6 @@ extern int kprobe_exceptions_notify(struct notifier_block *self,
> > extern int kprobe_fault_handler(struct pt_regs *regs, int trapnr);
> > extern int kprobe_handler(struct pt_regs *regs);
> > extern int kprobe_post_handler(struct pt_regs *regs);
> > -#ifdef CONFIG_KPROBES_ON_FTRACE
> > -extern int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
> > - struct kprobe_ctlblk *kcb);
> > -#else
> > -static inline int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
> > - struct kprobe_ctlblk *kcb)
> > -{
> > - return 0;
> > -}
> > -#endif
> > #else
> > static inline int kprobe_handler(struct pt_regs *regs) { return 0; }
> > static inline int kprobe_post_handler(struct pt_regs *regs) { return 0; }
> > diff --git a/arch/powerpc/kernel/kprobes-ftrace.c b/arch/powerpc/kernel/kprobes-ftrace.c
> > index 1b316331c2d9..3869b0e5d5c7 100644
> > --- a/arch/powerpc/kernel/kprobes-ftrace.c
> > +++ b/arch/powerpc/kernel/kprobes-ftrace.c
> > @@ -26,8 +26,8 @@
> > #include <linux/ftrace.h>
> >
> > static nokprobe_inline
> > -int __skip_singlestep(struct kprobe *p, struct pt_regs *regs,
> > - struct kprobe_ctlblk *kcb, unsigned long orig_nip)
> > +int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
> > + struct kprobe_ctlblk *kcb, unsigned long orig_nip)
> > {
> > /*
> > * Emulate singlestep (and also recover regs->nip)
> > @@ -44,16 +44,6 @@ int __skip_singlestep(struct kprobe *p, struct pt_regs *regs,
> > return 1;
> > }
> >
> > -int skip_singlestep(struct kprobe *p, struct pt_regs *regs,
> > - struct kprobe_ctlblk *kcb)
> > -{
> > - if (kprobe_ftrace(p))
> > - return __skip_singlestep(p, regs, kcb, 0);
> > - else
> > - return 0;
> > -}
> > -NOKPROBE_SYMBOL(skip_singlestep);
> > -
> > /* Ftrace callback handler for kprobes */
> > void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> > struct ftrace_ops *ops, struct pt_regs *regs)
> > @@ -82,7 +72,7 @@ void kprobe_ftrace_handler(unsigned long nip, unsigned long parent_nip,
> > __this_cpu_write(current_kprobe, p);
> > kcb->kprobe_status = KPROBE_HIT_ACTIVE;
> > if (!p->pre_handler || !p->pre_handler(p, regs))
> > - __skip_singlestep(p, regs, kcb, orig_nip);
> > + skip_singlestep(p, regs, kcb, orig_nip);
>
> We can probably get rid of skip_singlestep() completely along with
> orig_nip since instructions are always 4 bytes on powerpc. So, the
> changes we do to nip should help to recover the value automatically.
Good point! Yes, skip_singlestep() is no more exported, so we just consolidate
it into kprobe_ftrace_handler() for simplifying operation.
Thank you!
>
> - Naveen
>
>
--
Masami Hiramatsu <mhiramat@xxxxxxxxxx>
