Re: [PATCH 1/4] LoongArch: Add kprobe support

[Tiezhu Yang]

Hi Jinyang,

On 09/22/2022 10:19 AM, Jinyang He wrote:
> On 09/20/2022 11:37 AM, Tiezhu Yang wrote:
>
> > Kprobes allows you to trap at almost any kernel address and
> > execute a callback function, this commit adds kprobe support
> > for LoongArch.
> >
> > Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
> > ---
> >   arch/loongarch/Kconfig               |   1 +
> >   arch/loongarch/include/asm/inst.h    |  31 ++++
> >   arch/loongarch/include/asm/kprobes.h |  44 +++++
> >   arch/loongarch/include/asm/ptrace.h  |   1 +
> >   arch/loongarch/kernel/Makefile       |   2 +
> >   arch/loongarch/kernel/inst.c         | 107 ++++++++++++
> >   arch/loongarch/kernel/kprobes.c      | 309

...

> > +void simu_pc(struct pt_regs *regs, union loongarch_instruction insn)
> > +{
> > +    unsigned long pc = regs->csr_era;
> > +    unsigned int rd = insn.reg1i20_format.rd;
> > +    unsigned int imm = insn.reg1i20_format.immediate;
> > +
> > +    switch (insn.reg1i20_format.opcode) {
> > +    case pcaddi_op:
> > +        regs->regs[rd] = pc + sign_extended(imm << 2, 21);
> > +        break;
> > +    case pcaddu12i_op:
> > +        regs->regs[rd] = pc + sign_extended(imm << 12, 31);
> > +        break;
> > +    case pcaddu18i_op:
> > +        regs->regs[rd] = pc + sign_extended(imm << 18, 37);
> > +        break;
> > +    case pcalau12i_op:
> > +        regs->regs[rd] = pc + sign_extended(imm << 12, 31);
> > +        regs->regs[rd] &= ~((1 << 12) - 1);
> > +        break;
> > +    }
> > +
> > +    regs->csr_era += LOONGARCH_INSN_SIZE;
> > +}
> Hi, Tiezhu,
>
> The simu_pc and the simu_branch should make sure the instruction op is
> excepted.
> I think returning with errno or adding notes to explain that is needed.

OK, will do it in v2.

> BTW, I honored these arch-specific simulation work I used to contribute
> in the old world, although the codes has been optimized. Could you add
> my sign-off?

OK, no problem, as we discussed offline, I will split it into a
single patch and add your credit, thanks for your work.

> > diff --git a/arch/loongarch/kernel/kprobes.c
> > b/arch/loongarch/kernel/kprobes.c

...

> > +int kprobe_exceptions_notify(struct notifier_block *self,
> > +                 unsigned long val, void *data)
> > +{
> > +    struct die_args *args = (struct die_args *)data;
> > +    int ret = NOTIFY_DONE;
> > +
> > +    switch (val) {
> > +    case DIE_BREAK:
> > +        if (kprobe_pre_handler(args->regs))
> > +            ret = NOTIFY_STOP;
> > +        break;
> > +    case DIE_SSTEPBP:
> > +        if (kprobe_post_handler(args->regs))
> > +            ret = NOTIFY_STOP;
> > +        break;
> > +    case DIE_PAGE_FAULT:
> > +        preempt_disable();
> > +        if (kprobe_running()
> > +            && kprobe_fault_handler(args->regs, args->trapnr))
> > +            ret = NOTIFY_STOP;
> > +        preempt_enable();
> > +        break;
> The DIE_PAGE_FAULT not be notified in do_page_fault() now, and the newly
> version to call kprobe_fault_handler is
> do_page_fault()->kprobe_page_fault()
> ->kprobe_fault_handler().

OK, will modify it in v2, thanks for your review.

Thanks,
Tiezhu