Messages in this thread | | | Subject | Re: [PATCH v2 3/5] LoongArch: Add kretprobe support | From | Jinyang He <> | Date | Mon, 14 Nov 2022 17:26:54 +0800 |
| |
On 2022/11/14 下午4:51, Huacai Chen wrote:
> 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? > ARM64 have noted "Construct a useful saved PSTATE" in kprobes_trampoline.S.
And it seems we provide more interfaces than RISC-V. ?
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 >>>>>> >>>>>>
| |