Re: [PATCH v2 3/5] LoongArch: Add kretprobe support
From: Jinyang He
Date: Mon Nov 14 2022 - 04:27:06 EST
On 2022/11/14 下午4:51, Huacai Chen wrote:
On Mon, Nov 14, 2022 at 4:32 PM Jinyang He <hejinyang@xxxxxxxxxxx> wrote:
On 2022/11/14 下午2:50, Huacai Chen wrote:
On Mon, Nov 14, 2022 at 2:11 PM Jinyang He <hejinyang@xxxxxxxxxxx> 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@xxxxxxxxxxx> wrote:
Use the generic kretprobe trampoline handler to add kretprobe
support for LoongArch.
Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx>
---
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