lkml.org 
[lkml]   [2022]   [Nov]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v2 3/5] LoongArch: Add kretprobe support
    On Mon, Nov 14, 2022 at 4:32 PM Jinyang He <hejinyang@loongson.cn> wrote:
    >
    > On 2022/11/14 下午2:50, Huacai Chen wrote:
    >
    > > On Mon, Nov 14, 2022 at 2:11 PM Jinyang He <hejinyang@loongson.cn> wrote:
    > >> On 2022/11/14 下午12:43, Huacai Chen wrote:
    > >>
    > >>> Hi, Tiezhu and Jinyang,
    > >>>
    > >>> On Wed, Sep 28, 2022 at 8:50 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote:
    > >>>> Use the generic kretprobe trampoline handler to add kretprobe
    > >>>> support for LoongArch.
    > >>>>
    > >>>> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn>
    > >>>> ---
    > >>>> arch/loongarch/Kconfig | 1 +
    > >>>> arch/loongarch/kernel/Makefile | 2 +-
    > >>>> arch/loongarch/kernel/kprobes.c | 24 ++++++++
    > >>>> arch/loongarch/kernel/kprobes_trampoline.S | 97 ++++++++++++++++++++++++++++++
    > >>>> 4 files changed, 123 insertions(+), 1 deletion(-)
    > >>>> create mode 100644 arch/loongarch/kernel/kprobes_trampoline.S
    > >>>>
    > >>>> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
    > >>>> index 8debd70..877be6a 100644
    > >>>> --- a/arch/loongarch/Kconfig
    > >>>> +++ b/arch/loongarch/Kconfig
    > >>>> @@ -95,6 +95,7 @@ config LOONGARCH
    > >>>> select HAVE_IRQ_EXIT_ON_IRQ_STACK
    > >>>> select HAVE_IRQ_TIME_ACCOUNTING
    > >>>> select HAVE_KPROBES
    > >>>> + select HAVE_KRETPROBES
    > >>>> select HAVE_MOD_ARCH_SPECIFIC
    > >>>> select HAVE_NMI
    > >>>> select HAVE_PCI
    > >>>> diff --git a/arch/loongarch/kernel/Makefile b/arch/loongarch/kernel/Makefile
    > >>>> index ff98d8a..48f50607 100644
    > >>>> --- a/arch/loongarch/kernel/Makefile
    > >>>> +++ b/arch/loongarch/kernel/Makefile
    > >>>> @@ -33,6 +33,6 @@ obj-$(CONFIG_UNWINDER_PROLOGUE) += unwind_prologue.o
    > >>>>
    > >>>> obj-$(CONFIG_PERF_EVENTS) += perf_event.o perf_regs.o
    > >>>>
    > >>>> -obj-$(CONFIG_KPROBES) += kprobes.o
    > >>>> +obj-$(CONFIG_KPROBES) += kprobes.o kprobes_trampoline.o
    > >>>>
    > >>>> CPPFLAGS_vmlinux.lds := $(KBUILD_CFLAGS)
    > >>>> diff --git a/arch/loongarch/kernel/kprobes.c b/arch/loongarch/kernel/kprobes.c
    > >>>> index c11f6e0..ca3f1dc 100644
    > >>>> --- a/arch/loongarch/kernel/kprobes.c
    > >>>> +++ b/arch/loongarch/kernel/kprobes.c
    > >>>> @@ -306,6 +306,30 @@ int __init arch_populate_kprobe_blacklist(void)
    > >>>> (unsigned long)__irqentry_text_end);
    > >>>> }
    > >>>>
    > >>>> +/* Called from __kretprobe_trampoline */
    > >>>> +void __used *trampoline_probe_handler(struct pt_regs *regs)
    > >>>> +{
    > >>>> + return (void *)kretprobe_trampoline_handler(regs, NULL);
    > >>>> +}
    > >>>> +NOKPROBE_SYMBOL(trampoline_probe_handler);
    > >>>> +
    > >>>> +void arch_prepare_kretprobe(struct kretprobe_instance *ri,
    > >>>> + struct pt_regs *regs)
    > >>>> +{
    > >>>> + ri->ret_addr = (kprobe_opcode_t *)regs->regs[1];
    > >>>> + ri->fp = NULL;
    > >>>> +
    > >>>> + /* Replace the return addr with trampoline addr */
    > >>>> + regs->regs[1] = (unsigned long)&__kretprobe_trampoline;
    > >>>> +}
    > >>>> +NOKPROBE_SYMBOL(arch_prepare_kretprobe);
    > >>>> +
    > >>>> +int arch_trampoline_kprobe(struct kprobe *p)
    > >>>> +{
    > >>>> + return 0;
    > >>>> +}
    > >>>> +NOKPROBE_SYMBOL(arch_trampoline_kprobe);
    > >>>> +
    > >>>> int __init arch_init_kprobes(void)
    > >>>> {
    > >>>> return 0;
    > >>>> diff --git a/arch/loongarch/kernel/kprobes_trampoline.S b/arch/loongarch/kernel/kprobes_trampoline.S
    > >>>> new file mode 100644
    > >>>> index 0000000..9888ab8
    > >>>> --- /dev/null
    > >>>> +++ b/arch/loongarch/kernel/kprobes_trampoline.S
    > >>>> @@ -0,0 +1,97 @@
    > >>>> +/* SPDX-License-Identifier: GPL-2.0+ */
    > >>>> +#include <linux/linkage.h>
    > >>>> +#include <asm/stackframe.h>
    > >>>> +
    > >>>> + .text
    > >>>> +
    > >>>> + .macro save_all_base_regs
    > >>>> + cfi_st zero, PT_R0
    > >>>> + cfi_st ra, PT_R1
    > >>>> + cfi_st tp, PT_R2
    > >>>> + cfi_st a0, PT_R4
    > >>>> + cfi_st a1, PT_R5
    > >>>> + cfi_st a2, PT_R6
    > >>>> + cfi_st a3, PT_R7
    > >>>> + cfi_st a4, PT_R8
    > >>>> + cfi_st a5, PT_R9
    > >>>> + cfi_st a6, PT_R10
    > >>>> + cfi_st a7, PT_R11
    > >>>> + cfi_st t0, PT_R12
    > >>>> + cfi_st t1, PT_R13
    > >>>> + cfi_st t2, PT_R14
    > >>>> + cfi_st t3, PT_R15
    > >>>> + cfi_st t4, PT_R16
    > >>>> + cfi_st t5, PT_R17
    > >>>> + cfi_st t6, PT_R18
    > >>>> + cfi_st t7, PT_R19
    > >>>> + cfi_st t8, PT_R20
    > >>>> + cfi_st u0, PT_R21
    > >>>> + cfi_st fp, PT_R22
    > >>>> + cfi_st s0, PT_R23
    > >>>> + cfi_st s1, PT_R24
    > >>>> + cfi_st s2, PT_R25
    > >>>> + cfi_st s3, PT_R26
    > >>>> + cfi_st s4, PT_R27
    > >>>> + cfi_st s5, PT_R28
    > >>>> + cfi_st s6, PT_R29
    > >>>> + cfi_st s7, PT_R30
    > >>>> + cfi_st s8, PT_R31
    > >>>> + addi.d t0, sp, PT_SIZE
    > >>>> + LONG_S t0, sp, PT_R3
    > >>>> + csrrd t0, LOONGARCH_CSR_CRMD
    > >>>> + andi t0, t0, 0x7 /* extract bit[1:0] PLV, bit[2] IE */
    > >>>> + LONG_S t0, sp, PT_PRMD
    > >>>> + .endm
    > >>>> +
    > >>>> + .macro restore_all_base_regs
    > >>>> + cfi_ld zero, PT_R0
    > >>>> + cfi_ld tp, PT_R2
    > >>>> + cfi_ld a0, PT_R4
    > >>>> + cfi_ld a1, PT_R5
    > >>>> + cfi_ld a2, PT_R6
    > >>>> + cfi_ld a3, PT_R7
    > >>>> + cfi_ld a4, PT_R8
    > >>>> + cfi_ld a5, PT_R9
    > >>>> + cfi_ld a6, PT_R10
    > >>>> + cfi_ld a7, PT_R11
    > >>>> + cfi_ld t0, PT_R12
    > >>>> + cfi_ld t1, PT_R13
    > >>>> + cfi_ld t2, PT_R14
    > >>>> + cfi_ld t3, PT_R15
    > >>>> + cfi_ld t4, PT_R16
    > >>>> + cfi_ld t5, PT_R17
    > >>>> + cfi_ld t6, PT_R18
    > >>>> + cfi_ld t7, PT_R19
    > >>>> + cfi_ld t8, PT_R20
    > >>>> + cfi_ld u0, PT_R21
    > >>>> + cfi_ld fp, PT_R22
    > >>>> + cfi_ld s0, PT_R23
    > >>>> + cfi_ld s1, PT_R24
    > >>>> + cfi_ld s2, PT_R25
    > >>>> + cfi_ld s3, PT_R26
    > >>>> + cfi_ld s4, PT_R27
    > >>>> + cfi_ld s5, PT_R28
    > >>>> + cfi_ld s6, PT_R29
    > >>>> + cfi_ld s7, PT_R30
    > >>>> + cfi_ld s8, PT_R31
    > >>>> + LONG_L t0, sp, PT_PRMD
    > >>>> + li.d t1, 0x7 /* mask bit[1:0] PLV, bit[2] IE */
    > >>>> + csrxchg t0, t1, LOONGARCH_CSR_CRMD
    > >>>> + .endm
    > >>> Do you think we need to save and restore all regs here?
    > >>>
    > >>> Huacai
    > >> Hi, Huacai,
    > >>
    > >>
    > >> Note that it is not function context. In the original kprobe design, it is
    > >> triggered by 'break' and then trap into exception with all pt_regs saved.
    > >> The all pt_regs will be visible to the user. So I think in this version
    > >> we should also support all regs to user. BTW, due to all exceptions is
    > >> trapped by 'break' something in pt_regs is not needed, like estat,
    > >> badvaddr and so on.
    > > OK, but I still have some questions:
    > > 1, Why $r0 need save/restore?
    >
    > Surely $r0 can be not saved, as now we do not have strange purpose
    > to make PT_R0 as a flag.
    >
    >
    > > 2, Why save $r1 but not restore?
    > My wrong idea is $r1 should be saved at CSR_ERA, to plays it like
    > exception happened. But its value always equal the address of
    > __kretprobe_trampoline. The kretprobe is something like fgraph. The real
    > return address is returned by trampoline_probe_handler. And at present,
    > the real return address is replaced in pt_regs->csr_era in
    > __kretprobe_trampoline_handler(). So the $r1 saved in CSR_ERA will
    > be destroied at __kretprobe_trampoline_handler() actually.
    > That's why $r1 saved also is not needed.
    >
    > And both way to get return address from return value or get return address
    >
    > from pt_regs is same on LoongArch because arch_kretprobe_fixup_return()
    >
    > does nothing. But I think get return address from pt_regs is more reliable.
    >
    >
    > > 3, What is the purpose of CRMD magic?
    >
    > PT_CRMD magic is just exception context. It gives us a chance e.g.
    > set ie off at func head, and ie on at func return.
    ARM64 and RISC-V don't have such magics, so maybe they are unneeded?

    Huacai
    >
    >
    > Thanks,
    >
    > Jinyang
    >
    > >
    > >>>> +
    > >>>> +SYM_CODE_START(__kretprobe_trampoline)
    > >>>> + addi.d sp, sp, -PT_SIZE
    > >>>> + save_all_base_regs
    > >>>> +
    > >>>> + move a0, sp /* pt_regs */
    > >>>> +
    > >>>> + bl trampoline_probe_handler
    > >>>> +
    > >>>> + /* use the result as the return-address */
    > >>>> + move ra, a0
    > >>>> +
    > >>>> + restore_all_base_regs
    > >>>> + addi.d sp, sp, PT_SIZE
    > >>>> +
    > >>>> + jr ra
    > >>>> +SYM_CODE_END(__kretprobe_trampoline)
    > >>>> --
    > >>>> 2.1.0
    > >>>>
    > >>>>
    >

    \
     
     \ /
      Last update: 2022-11-14 09:52    [W:3.722 / U:0.068 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site