2022-11-23 14:25:01

by Song Shuai

[permalink] [raw]
Subject: [PATCH 0/2] riscv/ftrace: add WITH_DIRECT_CALLS support

This series adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
SAMPLE_FTRACE_DIRECT and SAMPLE_FTRACE_DIRECT_MULTI are also included
here as the samples for testing DIRECT_CALLS related interface.

First, select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide
register_ftrace_direct[_multi] interfaces allowing user to register
the customed trampoline (direct_caller) as the mcount for one or
more target functions. And modify_ftrace_direct[_multi] are also
provided for modify direct_caller.

At the same time, the samples in ./samples/ftrace/ can be built
as kerenl module for testing these interfaces with SAMPLE_FTRACE_DIRECT
and SAMPLE_FTRACE_DIRECT_MULTI selected.

Second, to make the direct_caller and the other ftrace hooks
(eg. function/fgraph tracer, k[ret]probes) co-exist, a temporary register
are nominated to store the address of direct_caller in ftrace_regs_caller.
After the setting of the address direct_caller by direct_ops->func and
the RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
by the `jr` inst.

The following tests have been passed in my local qemu-riscv64 virt machine.

1. tests with CONFIG_FTRACE_STARTUP_TEST
2. tests of samples/ftrace/ftrace*.ko
3. manual tests with any combination of the following hooks
- function/function_graph tracer
- ftrace*.ko
- kprobe/kretprobe

For your reference, here is the log when function tracer, kretprobe and
ftrace-direct-too.ko co-hooks the handle_mm_fault function.

```
[root@stage4 tracing]# echo handle_mm_fault > set_ftrace_filter
[root@stage4 tracing]# echo 'r:myr handle_mm_fault' > kprobe_events
[root@stage4 tracing]# echo function > current_tracer
[root@stage4 tracing]# echo 1 > events/kprobes/myr/enable
[root@stage4 tracing]# insmod /root/ftrace-direct-too.ko
[root@stage4 tracing]#
[root@stage4 tracing]# cat trace | tail
cat-388 [000] ...1. 583.051438: myr: (do_page_fault+0x16c/0x5f2 <- handle_mm_fault)
cat-388 [000] ...2. 583.057930: handle_mm_fault <-do_page_fault
cat-388 [000] ..... 583.057990: my_direct_func: handle mm fault vma=000000002d9fe19c address=ffffffae9b7000 flags=215
cat-388 [000] ...1. 583.058284: myr: (do_page_fault+0x16c/0x5f2 <- handle_mm_fault)
tail-389 [001] ...2. 583.059062: handle_mm_fault <-do_page_fault
tail-389 [001] ..... 583.059104: my_direct_func: handle mm fault vma=0000000017f3c48e address=aaaaaabebf3000 flags=215
tail-389 [001] ...1. 583.059325: myr: (do_page_fault+0x16c/0x5f2 <- handle_mm_fault)
tail-389 [001] ...2. 583.060371: handle_mm_fault <-do_page_fault
tail-389 [001] ..... 583.060410: my_direct_func: handle mm fault vma=0000000017f3c48e address=aaaaaabebf1000 flags=255
tail-389 [001] ...1. 583.060996: myr: (do_page_fault+0x16c/0x5f2 <- handle_mm_fault)
```

Note1:

This series is based on (riscv/ftrace: make ftrace_caller call ftrace_graph_func)
in this repo -> https://github.com/guoren83/linux/commits/ftrace_fixup_v3 .

Note2:

The checkpatch.pl will output some warnings on this series, like this

```
WARNING: Prefer using '"%s...", __func__' to using 'my_direct_func2', this function's name, in a string
111: FILE: samples/ftrace/ftrace-direct-multi-modify.c:48:
+" call my_direct_func2\n"
```

The reason is that checkpatch depends on patch context providing the
function name. In the above warning, my_direct_func2 has some codeline
distance with the changed trunk, so its declaration doesn't come into
the patch, and then the warning jumps out.

You may notice the location of `my_ip` variable changes in the 2nd patch.
I did that for reducing the warnings to some extent. But killing all the
warnings will makes the patch less readable, so I stopped here.

--
Song

Song Shuai (2):
riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
samples/ftrace: add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]

arch/riscv/Kconfig | 3 ++
arch/riscv/include/asm/ftrace.h | 6 ++++
arch/riscv/kernel/mcount-dyn.S | 4 +++
samples/ftrace/ftrace-direct-modify.c | 35 ++++++++++++++++++-
samples/ftrace/ftrace-direct-multi-modify.c | 37 +++++++++++++++++++++
samples/ftrace/ftrace-direct-multi.c | 22 ++++++++++++
samples/ftrace/ftrace-direct-too.c | 26 +++++++++++++++
samples/ftrace/ftrace-direct.c | 22 ++++++++++++
8 files changed, 154 insertions(+), 1 deletion(-)

--
2.20.1


2022-11-23 14:43:27

by Song Shuai

[permalink] [raw]
Subject: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.

select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
register_ftrace_direct[_multi] interfaces allowing users to register
the customed trampoline (direct_caller) as the mcount for one or
more target functions. And modify_ftrace_direct[_multi] are also
provided for modifying direct_caller.

To make the direct_caller and the other ftrace hooks (eg. function/fgraph
tracer, k[ret]probes) co-exist, a temporary register is nominated to
store the address of direct_caller in ftrace_regs_caller. After the
setting of the address direct_caller by direct_ops->func and the
RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
by the `jr` inst.

Signed-off-by: Song Shuai <[email protected]>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/ftrace.h | 6 ++++++
arch/riscv/kernel/mcount-dyn.S | 4 ++++
3 files changed, 11 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 39ec8d628cf6..d083ec08d0b6 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -278,6 +278,7 @@ config ARCH_RV64I
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
+ select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 01bebb28eabe..be4d57566139 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -114,6 +114,12 @@ struct ftrace_regs;
void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs);
#define ftrace_graph_func ftrace_graph_func
+
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+ regs->t1 = addr;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */

#endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index 466c6ef217b1..b89c85a58569 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
ENTRY(ftrace_regs_caller)
SAVE_ABI_REGS 1
+ REG_S x0, PT_T1(sp)
PREPARE_ARGS

ftrace_regs_call:
@@ -241,7 +242,10 @@ ftrace_regs_call:


RESTORE_ABI_REGS 1
+ bnez t1,.Ldirect
jr t0
+.Ldirect:
+ jr t1
ENDPROC(ftrace_regs_caller)

ENTRY(ftrace_caller)
--
2.20.1

2022-11-23 15:23:59

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Hi Song,

On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
>
> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
>
> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> register_ftrace_direct[_multi] interfaces allowing users to register
> the customed trampoline (direct_caller) as the mcount for one or
> more target functions. And modify_ftrace_direct[_multi] are also
> provided for modifying direct_caller.
>
> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> store the address of direct_caller in ftrace_regs_caller. After the
> setting of the address direct_caller by direct_ops->func and the
> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> by the `jr` inst.
>
> Signed-off-by: Song Shuai <[email protected]>
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/ftrace.h | 6 ++++++
> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 39ec8d628cf6..d083ec08d0b6 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -278,6 +278,7 @@ config ARCH_RV64I
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER
> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 01bebb28eabe..be4d57566139 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -114,6 +114,12 @@ struct ftrace_regs;
> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs);
> #define ftrace_graph_func ftrace_graph_func
> +
> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> +{
> + regs->t1 = addr;
How about regs->t0 = addr; ?
And delete all mcount-dyn.S modification.
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> index 466c6ef217b1..b89c85a58569 100644
> --- a/arch/riscv/kernel/mcount-dyn.S
> +++ b/arch/riscv/kernel/mcount-dyn.S
> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> ENTRY(ftrace_regs_caller)
> SAVE_ABI_REGS 1
> + REG_S x0, PT_T1(sp)
> PREPARE_ARGS
>
> ftrace_regs_call:
> @@ -241,7 +242,10 @@ ftrace_regs_call:
>
>
> RESTORE_ABI_REGS 1
> + bnez t1,.Ldirect
> jr t0
> +.Ldirect:
> + jr t1
> ENDPROC(ftrace_regs_caller)
>
> ENTRY(ftrace_caller)
> --
> 2.20.1
>


--
Best Regards
Guo Ren

2022-11-23 15:24:26

by Song Shuai

[permalink] [raw]
Subject: [PATCH 2/2] samples/ftrace: add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]

select HAVE_SAMPLE_FTRACE_DIRECT and HAVE_SAMPLE_FTRACE_DIRECT_MULTI
for ARCH_RV64I in arch/riscv/Kconfig. And add riscv asm code for
the ftrace-direct*.c files in samples/ftrace/.

Signed-off-by: Song Shuai <[email protected]>
---
arch/riscv/Kconfig | 2 ++
samples/ftrace/ftrace-direct-modify.c | 35 ++++++++++++++++++-
samples/ftrace/ftrace-direct-multi-modify.c | 37 +++++++++++++++++++++
samples/ftrace/ftrace-direct-multi.c | 22 ++++++++++++
samples/ftrace/ftrace-direct-too.c | 26 +++++++++++++++
samples/ftrace/ftrace-direct.c | 22 ++++++++++++
6 files changed, 143 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d083ec08d0b6..572973103d73 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -279,6 +279,8 @@ config ARCH_RV64I
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+ select HAVE_SAMPLE_FTRACE_DIRECT
+ select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index 39146fa83e20..5582d000d803 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -17,10 +17,43 @@ void my_direct_func2(void)
trace_printk("my direct func2\n");
}

+static unsigned long my_ip = (unsigned long)schedule;
+
extern void my_tramp1(void *);
extern void my_tramp2(void *);

-static unsigned long my_ip = (unsigned long)schedule;
+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:\n"
+" addi sp,sp,-16\n"
+" sd t0,0(sp)\n"
+" sd ra,8(sp)\n"
+" call my_direct_func1\n"
+" ld t0,0(sp)\n"
+" ld ra,8(sp)\n"
+" addi sp,sp,16\n"
+" jr t0\n"
+" .size my_tramp1, .-my_tramp1\n"
+
+" .type my_tramp2, @function\n"
+" .globl my_tramp2\n"
+" my_tramp2:\n"
+" addi sp,sp,-16\n"
+" sd t0,0(sp)\n"
+" sd ra,8(sp)\n"
+" call my_direct_func2\n"
+" ld t0,0(sp)\n"
+" ld ra,8(sp)\n"
+" addi sp,sp,16\n"
+" jr t0\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */

#ifdef CONFIG_X86_64

diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
index 65aa94d96f4e..43e02d39c652 100644
--- a/samples/ftrace/ftrace-direct-multi-modify.c
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -20,6 +20,43 @@ void my_direct_func2(unsigned long ip)
extern void my_tramp1(void *);
extern void my_tramp2(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func1\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp1, .-my_tramp1\n"
+
+" .type my_tramp2, @function\n"
+" .globl my_tramp2\n"
+" my_tramp2:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func2\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct-multi.c b/samples/ftrace/ftrace-direct-multi.c
index 41ded7c615c7..df91339d3095 100644
--- a/samples/ftrace/ftrace-direct-multi.c
+++ b/samples/ftrace/ftrace-direct-multi.c
@@ -15,6 +15,28 @@ void my_direct_func(unsigned long ip)

extern void my_tramp(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index 6690468c5cc2..fe3b8c4f3336 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -17,6 +17,32 @@ void my_direct_func(struct vm_area_struct *vma,

extern void my_tramp(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:\n"
+" addi sp,sp,-40\n"
+" sd a0,0(sp)\n"
+" sd a1,8(sp)\n"
+" sd a2,16(sp)\n"
+" sd t0,24(sp)\n"
+" sd ra,32(sp)\n"
+" call my_direct_func\n"
+" ld a0,0(sp)\n"
+" ld a1,8(sp)\n"
+" ld a2,16(sp)\n"
+" ld t0,24(sp)\n"
+" ld ra,32(sp)\n"
+" addi sp,sp,40\n"
+" jr t0\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index e8f1e440b9b8..0784024b6653 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -14,6 +14,28 @@ void my_direct_func(struct task_struct *p)

extern void my_tramp(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
--
2.20.1

2022-11-23 17:30:52

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
>
> Cool job, thx.
>
> On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
>>
>> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
>>
>> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
>> register_ftrace_direct[_multi] interfaces allowing users to register
>> the customed trampoline (direct_caller) as the mcount for one or
>> more target functions. And modify_ftrace_direct[_multi] are also
>> provided for modifying direct_caller.
>>
>> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
>> tracer, k[ret]probes) co-exist, a temporary register is nominated to
>> store the address of direct_caller in ftrace_regs_caller. After the
>> setting of the address direct_caller by direct_ops->func and the
>> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
>> by the `jr` inst.
>>
>> Signed-off-by: Song Shuai <[email protected]>
>> ---
>> arch/riscv/Kconfig | 1 +
>> arch/riscv/include/asm/ftrace.h | 6 ++++++
>> arch/riscv/kernel/mcount-dyn.S | 4 ++++
>> 3 files changed, 11 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 39ec8d628cf6..d083ec08d0b6 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -278,6 +278,7 @@ config ARCH_RV64I
>> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
>> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
>> select HAVE_FUNCTION_GRAPH_TRACER
>> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
>> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
>> index 01bebb28eabe..be4d57566139 100644
>> --- a/arch/riscv/include/asm/ftrace.h
>> +++ b/arch/riscv/include/asm/ftrace.h
>> @@ -114,6 +114,12 @@ struct ftrace_regs;
>> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>> struct ftrace_ops *op, struct ftrace_regs *fregs);
>> #define ftrace_graph_func ftrace_graph_func
>> +
>> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
>> +{
>> + regs->t1 = addr;
>
> How about regs->t0 = addr; ?
> And delete all mcount-dyn.S modification.
>
The direct_caller has the same program layout as the ftrace_caller, which means
the reg t0 will never be changed when direct_caller returns.

If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
direct_caller will enter the dead loop.

Actually the reg t0 always saves the address of function entry with 8B
offset, it should only
changed by the IPMODIFY ops instead of the direct_ops.
>>
>> +}
>> +
>> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>>
>> #endif /* __ASSEMBLY__ */
>> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
>> index 466c6ef217b1..b89c85a58569 100644
>> --- a/arch/riscv/kernel/mcount-dyn.S
>> +++ b/arch/riscv/kernel/mcount-dyn.S
>> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
>> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>> ENTRY(ftrace_regs_caller)
>> SAVE_ABI_REGS 1
>> + REG_S x0, PT_T1(sp)
>> PREPARE_ARGS
>>
>> ftrace_regs_call:
>> @@ -241,7 +242,10 @@ ftrace_regs_call:
>>
>>
>> RESTORE_ABI_REGS 1
>> + bnez t1,.Ldirect
>> jr t0
>> +.Ldirect:
>> + jr t1
>> ENDPROC(ftrace_regs_caller)
>>
>> ENTRY(ftrace_caller)
>> --
>> 2.20.1
>>
>
>
> --
> Best Regards
> Guo Ren

2022-11-24 02:40:18

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
>
> Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> >
> > Cool job, thx.
> >
> > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> >>
> >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> >>
> >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> >> register_ftrace_direct[_multi] interfaces allowing users to register
> >> the customed trampoline (direct_caller) as the mcount for one or
> >> more target functions. And modify_ftrace_direct[_multi] are also
> >> provided for modifying direct_caller.
> >>
> >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> >> store the address of direct_caller in ftrace_regs_caller. After the
> >> setting of the address direct_caller by direct_ops->func and the
> >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> >> by the `jr` inst.
> >>
> >> Signed-off-by: Song Shuai <[email protected]>
> >> ---
> >> arch/riscv/Kconfig | 1 +
> >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> >> 3 files changed, 11 insertions(+)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index 39ec8d628cf6..d083ec08d0b6 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> >> select HAVE_FUNCTION_GRAPH_TRACER
> >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> >> index 01bebb28eabe..be4d57566139 100644
> >> --- a/arch/riscv/include/asm/ftrace.h
> >> +++ b/arch/riscv/include/asm/ftrace.h
> >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> >> #define ftrace_graph_func ftrace_graph_func
> >> +
> >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> >> +{
> >> + regs->t1 = addr;
> >
> > How about regs->t0 = addr; ?
> > And delete all mcount-dyn.S modification.
> >
> The direct_caller has the same program layout as the ftrace_caller, which means
> the reg t0 will never be changed when direct_caller returns.
>
> If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> direct_caller will enter the dead loop.
?

ftrace_regs_caller->call_direct_funcs->
arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0.

Only call_direct_funcs call arch_ftrace_set_direct_caller !

static void call_direct_funcs(unsigned long ip, unsigned long pip,
struct ftrace_ops *ops, struct ftrace_regs *fregs)
{
struct pt_regs *regs = ftrace_get_regs(fregs);
unsigned long addr;

addr = ftrace_find_rec_direct(ip);
if (!addr)
return;

arch_ftrace_set_direct_caller(regs, addr);
}

>
> Actually the reg t0 always saves the address of function entry with 8B
> offset, it should only
> changed by the IPMODIFY ops instead of the direct_ops.
> >>
> >> +}
> >> +
> >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >>
> >> #endif /* __ASSEMBLY__ */
> >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> >> index 466c6ef217b1..b89c85a58569 100644
> >> --- a/arch/riscv/kernel/mcount-dyn.S
> >> +++ b/arch/riscv/kernel/mcount-dyn.S
> >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >> ENTRY(ftrace_regs_caller)
> >> SAVE_ABI_REGS 1
> >> + REG_S x0, PT_T1(sp)
> >> PREPARE_ARGS
> >>
> >> ftrace_regs_call:
> >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> >>
> >>
> >> RESTORE_ABI_REGS 1
> >> + bnez t1,.Ldirect
> >> jr t0
> >> +.Ldirect:
> >> + jr t1
> >> ENDPROC(ftrace_regs_caller)
> >>
> >> ENTRY(ftrace_caller)
> >> --
> >> 2.20.1
> >>
> >
> >
> > --
> > Best Regards
> > Guo Ren



--
Best Regards
Guo Ren

2022-11-24 03:32:55

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Song Shuai <[email protected]> 于2022年11月24日周四 02:52写道:
>
> Guo Ren <[email protected]> 于2022年11月24日周四 02:08写道:
> >
> > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > >
> > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > >
> > > > Cool job, thx.
> > > >
> > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > >>
> > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > >>
> > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > >> provided for modifying direct_caller.
> > > >>
> > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > >> setting of the address direct_caller by direct_ops->func and the
> > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > >> by the `jr` inst.
> > > >>
> > > >> Signed-off-by: Song Shuai <[email protected]>
> > > >> ---
> > > >> arch/riscv/Kconfig | 1 +
> > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > >> 3 files changed, 11 insertions(+)
> > > >>
> > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > >> --- a/arch/riscv/Kconfig
> > > >> +++ b/arch/riscv/Kconfig
> > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > >> index 01bebb28eabe..be4d57566139 100644
> > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > >> #define ftrace_graph_func ftrace_graph_func
> > > >> +
> > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > >> +{
> > > >> + regs->t1 = addr;
> > > >
> > > > How about regs->t0 = addr; ?
> > > > And delete all mcount-dyn.S modification.
> > > >
> > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > the reg t0 will never be changed when direct_caller returns.
> > >
> > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > direct_caller will enter the dead loop.
> > ?
> >
> > ftrace_regs_caller->call_direct_funcs->
> > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0.
> >
> > Only call_direct_funcs call arch_ftrace_set_direct_caller !
> >
> > static void call_direct_funcs(unsigned long ip, unsigned long pip,
> > struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > {
> > struct pt_regs *regs = ftrace_get_regs(fregs);
> > unsigned long addr;
> >
> > addr = ftrace_find_rec_direct(ip);
> > if (!addr)
> > return;
> >
> > arch_ftrace_set_direct_caller(regs, addr);
> > }
> >
> When direct_caller and function tracer co-hook a function, the simple
> diagram is like this:
>
> ```
> func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> function_trace_call // write ringbuffer
> |
> -> call_direct_funcs // regs->t1 = direct_caller
> -> t1 !=0 && jr t1 // goto direct_caller
> ```
>
```
f -- regs_caller -- list_func
| | -- function_trace_call
| | -- call_direct_funcs // t1 = addr
|-- t1 !=0 && jr t1 // goto direct_caller
```
This diagram looks better.
> And the direct_caller has a similar implement as ftrace_caller.
>
> ```
> direct_caller:
> add sp,sp,-?
> sd t0,?(sp)
> sd ra,?(sp)
> call foo
> ld t0,?(sp)
> ld ra,?(sp)
> add sp,sp,?
> jr t0 // <- back to function entry
> ```
>
> If we change regs->t0 as direct_caller and execute `jr t0` directly,
> the direct_caller will always jump to itself due to its last `jr` inst.
>
> ```
> func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> function_trace_call // write ringbuffer
> |
> -> call_direct_funcs // regs->t0 = direct_caller
> -> jr t0 // goto direct_caller
>
> direct_caller:
> ...
> sd t0,?(sp)
> ...
> ld t0,?(s0)
> ...
> jr t0 // goto direct_caller always
> ```
>
> Hope I made it clear.
> > >
> > > Actually the reg t0 always saves the address of function entry with 8B
> > > offset, it should only
> > > changed by the IPMODIFY ops instead of the direct_ops.
> > > >>
> > > >> +}
> > > >> +
> > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > >>
> > > >> #endif /* __ASSEMBLY__ */
> > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > >> index 466c6ef217b1..b89c85a58569 100644
> > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > >> ENTRY(ftrace_regs_caller)
> > > >> SAVE_ABI_REGS 1
> > > >> + REG_S x0, PT_T1(sp)
> > > >> PREPARE_ARGS
> > > >>
> > > >> ftrace_regs_call:
> > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > >>
> > > >>
> > > >> RESTORE_ABI_REGS 1
> > > >> + bnez t1,.Ldirect
> > > >> jr t0
> > > >> +.Ldirect:
> > > >> + jr t1
> > > >> ENDPROC(ftrace_regs_caller)
> > > >>
> > > >> ENTRY(ftrace_caller)
> > > >> --
> > > >> 2.20.1
> > > >>
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> Thanks,
> Song
Thanks,
Song

2022-11-24 03:40:07

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Guo Ren <[email protected]> 于2022年11月24日周四 02:08写道:
>
> On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> >
> > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > >
> > > Cool job, thx.
> > >
> > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > >>
> > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > >>
> > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > >> the customed trampoline (direct_caller) as the mcount for one or
> > >> more target functions. And modify_ftrace_direct[_multi] are also
> > >> provided for modifying direct_caller.
> > >>
> > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > >> store the address of direct_caller in ftrace_regs_caller. After the
> > >> setting of the address direct_caller by direct_ops->func and the
> > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > >> by the `jr` inst.
> > >>
> > >> Signed-off-by: Song Shuai <[email protected]>
> > >> ---
> > >> arch/riscv/Kconfig | 1 +
> > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > >> 3 files changed, 11 insertions(+)
> > >>
> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > >> --- a/arch/riscv/Kconfig
> > >> +++ b/arch/riscv/Kconfig
> > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > >> select HAVE_FUNCTION_GRAPH_TRACER
> > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > >> index 01bebb28eabe..be4d57566139 100644
> > >> --- a/arch/riscv/include/asm/ftrace.h
> > >> +++ b/arch/riscv/include/asm/ftrace.h
> > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > >> #define ftrace_graph_func ftrace_graph_func
> > >> +
> > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > >> +{
> > >> + regs->t1 = addr;
> > >
> > > How about regs->t0 = addr; ?
> > > And delete all mcount-dyn.S modification.
> > >
> > The direct_caller has the same program layout as the ftrace_caller, which means
> > the reg t0 will never be changed when direct_caller returns.
> >
> > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > direct_caller will enter the dead loop.
> ?
>
> ftrace_regs_caller->call_direct_funcs->
> arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0.
>
> Only call_direct_funcs call arch_ftrace_set_direct_caller !
>
> static void call_direct_funcs(unsigned long ip, unsigned long pip,
> struct ftrace_ops *ops, struct ftrace_regs *fregs)
> {
> struct pt_regs *regs = ftrace_get_regs(fregs);
> unsigned long addr;
>
> addr = ftrace_find_rec_direct(ip);
> if (!addr)
> return;
>
> arch_ftrace_set_direct_caller(regs, addr);
> }
>
When direct_caller and function tracer co-hook a function, the simple
diagram is like this:

```
func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
function_trace_call // write ringbuffer
|
-> call_direct_funcs // regs->t1 = direct_caller
-> t1 !=0 && jr t1 // goto direct_caller
```

And the direct_caller has a similar implement as ftrace_caller.

```
direct_caller:
add sp,sp,-?
sd t0,?(sp)
sd ra,?(sp)
call foo
ld t0,?(sp)
ld ra,?(sp)
add sp,sp,?
jr t0 // <- back to function entry
```

If we change regs->t0 as direct_caller and execute `jr t0` directly,
the direct_caller will always jump to itself due to its last `jr` inst.

```
func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
function_trace_call // write ringbuffer
|
-> call_direct_funcs // regs->t0 = direct_caller
-> jr t0 // goto direct_caller

direct_caller:
...
sd t0,?(sp)
...
ld t0,?(s0)
...
jr t0 // goto direct_caller always
```

Hope I made it clear.
> >
> > Actually the reg t0 always saves the address of function entry with 8B
> > offset, it should only
> > changed by the IPMODIFY ops instead of the direct_ops.
> > >>
> > >> +}
> > >> +
> > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > >>
> > >> #endif /* __ASSEMBLY__ */
> > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > >> index 466c6ef217b1..b89c85a58569 100644
> > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > >> ENTRY(ftrace_regs_caller)
> > >> SAVE_ABI_REGS 1
> > >> + REG_S x0, PT_T1(sp)
> > >> PREPARE_ARGS
> > >>
> > >> ftrace_regs_call:
> > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > >>
> > >>
> > >> RESTORE_ABI_REGS 1
> > >> + bnez t1,.Ldirect
> > >> jr t0
> > >> +.Ldirect:
> > >> + jr t1
> > >> ENDPROC(ftrace_regs_caller)
> > >>
> > >> ENTRY(ftrace_caller)
> > >> --
> > >> 2.20.1
> > >>
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
>
>
>
> --
> Best Regards
> Guo Ren
Thanks,
Song

2022-11-24 04:08:31

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Thu, Nov 24, 2022 at 11:09 AM Song Shuai <[email protected]> wrote:
>
> Song Shuai <[email protected]> 于2022年11月24日周四 02:52写道:
> >
> > Guo Ren <[email protected]> 于2022年11月24日周四 02:08写道:
> > >
> > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > > >
> > > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > > >
> > > > > Cool job, thx.
> > > > >
> > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > > >>
> > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > > >>
> > > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > > >> provided for modifying direct_caller.
> > > > >>
> > > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > > >> setting of the address direct_caller by direct_ops->func and the
> > > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > > >> by the `jr` inst.
> > > > >>
> > > > >> Signed-off-by: Song Shuai <[email protected]>
> > > > >> ---
> > > > >> arch/riscv/Kconfig | 1 +
> > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > > >> 3 files changed, 11 insertions(+)
> > > > >>
> > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > > >> --- a/arch/riscv/Kconfig
> > > > >> +++ b/arch/riscv/Kconfig
> > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > > >> index 01bebb28eabe..be4d57566139 100644
> > > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > > >> #define ftrace_graph_func ftrace_graph_func
> > > > >> +
> > > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > > >> +{
> > > > >> + regs->t1 = addr;
> > > > >
> > > > > How about regs->t0 = addr; ?
> > > > > And delete all mcount-dyn.S modification.
> > > > >
> > > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > > the reg t0 will never be changed when direct_caller returns.
> > > >
> > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > > direct_caller will enter the dead loop.
> > > ?
> > >
> > > ftrace_regs_caller->call_direct_funcs->
> > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0.
> > >
> > > Only call_direct_funcs call arch_ftrace_set_direct_caller !
> > >
> > > static void call_direct_funcs(unsigned long ip, unsigned long pip,
> > > struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > > {
> > > struct pt_regs *regs = ftrace_get_regs(fregs);
> > > unsigned long addr;
> > >
> > > addr = ftrace_find_rec_direct(ip);
> > > if (!addr)
> > > return;
> > >
> > > arch_ftrace_set_direct_caller(regs, addr);
> > > }
> > >
> > When direct_caller and function tracer co-hook a function, the simple
> > diagram is like this:
> >
> > ```
> > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> > function_trace_call // write ringbuffer
> > |
> > -> call_direct_funcs // regs->t1 = direct_caller
> > -> t1 !=0 && jr t1 // goto direct_caller
> > ```
> >
> ```
> f -- regs_caller -- list_func
> | | -- function_trace_call
> | | -- call_direct_funcs // t1 = addr
> |-- t1 !=0 && jr t1 // goto direct_caller
Cool diagram! Thx, but we still need a discussion:
f -- regs_caller
| -- SAVE_ABI_REGS 1
| -- list_func
| | -- function_trace_call
| | -- call_direct_funcs // t1 = addr
| -- RESTORE_ABI_REGS 1
|-- t1 !=0 && jr t1 // goto direct_caller
If you set t1 non-zero, then we always go to direct_caller. I think
the code is equal to set t0=addr.
| | -- call_direct_funcs // t0 = addr
| -- RESTORE_ABI_REGS 1
|-- jr t0 // goto direct_caller

I think the only problem happens in the below non-existent situation:
f -- regs_caller
| -- SAVE_ABI_REGS 1
| -- list_func
| | -- call_direct_funcs // t0 = addr
| | -- function_trace_call //t0 contains
direct_caller instead
| -- RESTORE_ABI_REGS 1
|-- jr t0 // goto direct_caller

The key issue is whether going to direct_caller correctly depends on
call_direct_funcs called, right?

> ```
> This diagram looks better.
> > And the direct_caller has a similar implement as ftrace_caller.
> >
> > ```
> > direct_caller:
> > add sp,sp,-?
> > sd t0,?(sp)
> > sd ra,?(sp)
> > call foo
> > ld t0,?(sp)
> > ld ra,?(sp)
> > add sp,sp,?
> > jr t0 // <- back to function entry
> > ```
> >
> > If we change regs->t0 as direct_caller and execute `jr t0` directly,
> > the direct_caller will always jump to itself due to its last `jr` inst.
> >
> > ```
> > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> > function_trace_call // write ringbuffer
> > |
> > -> call_direct_funcs // regs->t0 = direct_caller
> > -> jr t0 // goto direct_caller
> >
> > direct_caller:
> > ...
> > sd t0,?(sp)
> > ...
> > ld t0,?(s0)
> > ...
> > jr t0 // goto direct_caller always
> > ```
> >
> > Hope I made it clear.
> > > >
> > > > Actually the reg t0 always saves the address of function entry with 8B
> > > > offset, it should only
> > > > changed by the IPMODIFY ops instead of the direct_ops.
> > > > >>
> > > > >> +}
> > > > >> +
> > > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > >>
> > > > >> #endif /* __ASSEMBLY__ */
> > > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > > >> index 466c6ef217b1..b89c85a58569 100644
> > > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > >> ENTRY(ftrace_regs_caller)
> > > > >> SAVE_ABI_REGS 1
> > > > >> + REG_S x0, PT_T1(sp)
> > > > >> PREPARE_ARGS
> > > > >>
> > > > >> ftrace_regs_call:
> > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > > >>
> > > > >>
> > > > >> RESTORE_ABI_REGS 1
> > > > >> + bnez t1,.Ldirect
> > > > >> jr t0
> > > > >> +.Ldirect:
> > > > >> + jr t1
> > > > >> ENDPROC(ftrace_regs_caller)
> > > > >>
> > > > >> ENTRY(ftrace_caller)
> > > > >> --
> > > > >> 2.20.1
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > Best Regards
> > > > > Guo Ren
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> > Thanks,
> > Song
> Thanks,
> Song



--
Best Regards
Guo Ren

2022-11-24 12:07:21

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Guo Ren <[email protected]> 于2022年11月24日周四 03:40写道:
>
> On Thu, Nov 24, 2022 at 11:09 AM Song Shuai <[email protected]> wrote:
> >
> > Song Shuai <[email protected]> 于2022年11月24日周四 02:52写道:
> > >
> > > Guo Ren <[email protected]> 于2022年11月24日周四 02:08写道:
> > > >
> > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > > > >
> > > > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > > > >
> > > > > > Cool job, thx.
> > > > > >
> > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > > > >>
> > > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > > > >>
> > > > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > > > >> provided for modifying direct_caller.
> > > > > >>
> > > > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > > > >> setting of the address direct_caller by direct_ops->func and the
> > > > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > > > >> by the `jr` inst.
> > > > > >>
> > > > > >> Signed-off-by: Song Shuai <[email protected]>
> > > > > >> ---
> > > > > >> arch/riscv/Kconfig | 1 +
> > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > > > >> 3 files changed, 11 insertions(+)
> > > > > >>
> > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > > > >> --- a/arch/riscv/Kconfig
> > > > > >> +++ b/arch/riscv/Kconfig
> > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > > > >> index 01bebb28eabe..be4d57566139 100644
> > > > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > > > >> #define ftrace_graph_func ftrace_graph_func
> > > > > >> +
> > > > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > > > >> +{
> > > > > >> + regs->t1 = addr;
> > > > > >
> > > > > > How about regs->t0 = addr; ?
> > > > > > And delete all mcount-dyn.S modification.
> > > > > >
> > > > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > > > the reg t0 will never be changed when direct_caller returns.
> > > > >
> > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > > > direct_caller will enter the dead loop.
> > > > ?
> > > >
> > > > ftrace_regs_caller->call_direct_funcs->
> > > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0.
> > > >
> > > > Only call_direct_funcs call arch_ftrace_set_direct_caller !
> > > >
> > > > static void call_direct_funcs(unsigned long ip, unsigned long pip,
> > > > struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > > > {
> > > > struct pt_regs *regs = ftrace_get_regs(fregs);
> > > > unsigned long addr;
> > > >
> > > > addr = ftrace_find_rec_direct(ip);
> > > > if (!addr)
> > > > return;
> > > >
> > > > arch_ftrace_set_direct_caller(regs, addr);
> > > > }
> > > >
> > > When direct_caller and function tracer co-hook a function, the simple
> > > diagram is like this:
> > >
> > > ```
> > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> > > function_trace_call // write ringbuffer
> > > |
> > > -> call_direct_funcs // regs->t1 = direct_caller
> > > -> t1 !=0 && jr t1 // goto direct_caller
> > > ```
> > >
> > ```
> > f -- regs_caller -- list_func
> > | | -- function_trace_call
> > | | -- call_direct_funcs // t1 = addr
> > |-- t1 !=0 && jr t1 // goto direct_caller
> Cool diagram! Thx, but we still need a discussion:
> f -- regs_caller
> | -- SAVE_ABI_REGS 1
> | -- list_func
> | | -- function_trace_call
> | | -- call_direct_funcs // t1 = addr
> | -- RESTORE_ABI_REGS 1
> |-- t1 !=0 && jr t1 // goto direct_caller
> If you set t1 non-zero, then we always go to direct_caller. I think
> the code is equal to set t0=addr.
If only focusing on the whole ftrace_regs_caller code, you're right.

But we should also take direct_caller code into the consideration,
if using t0 here, direct_caller will always return to itself as I
described before.
> | | -- call_direct_funcs // t0 = addr
> | -- RESTORE_ABI_REGS 1
> |-- jr t0 // goto direct_caller
>
> I think the only problem happens in the below non-existent situation:
> f -- regs_caller
> | -- SAVE_ABI_REGS 1
> | -- list_func
> | | -- call_direct_funcs // t0 = addr
> | | -- function_trace_call //t0 contains
> direct_caller instead
function_trace_call is one type of global_ops->func which doesn't take care
of the regs->t0.

But the func of IPMODIFY ops (eg. livepatch, kprobe with posthandler)
will change regs->epc which will then be restored to t0 in ftrace_regs_caller.
And then the address of direct_caller will be covered in this situation.
I'm not very clear about the process of the livepatch and kprobe in RISCV
but I think we should keep t0 for their ipmodifying.
> | -- RESTORE_ABI_REGS 1
> |-- jr t0 // goto direct_caller
>
> The key issue is whether going to direct_caller correctly depends on
> call_direct_funcs called, right?
Yes, in other words, call_direct_func informs ftrace_regs_caller that
there is a direct caller stored in t1, please jump to it first.
>
> > ```
> > This diagram looks better.
> > > And the direct_caller has a similar implement as ftrace_caller.
> > >
> > > ```
> > > direct_caller:
> > > add sp,sp,-?
> > > sd t0,?(sp)
> > > sd ra,?(sp)
> > > call foo
> > > ld t0,?(sp)
> > > ld ra,?(sp)
> > > add sp,sp,?
> > > jr t0 // <- back to function entry
> > > ```
> > >
> > > If we change regs->t0 as direct_caller and execute `jr t0` directly,
> > > the direct_caller will always jump to itself due to its last `jr` inst.
--- Here is what I described in the previous email.
> > >
> > > ```
> > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> > > function_trace_call // write ringbuffer
> > > |
> > > -> call_direct_funcs // regs->t0 = direct_caller
> > > -> jr t0 // goto direct_caller
> > >
> > > direct_caller:
> > > ...
> > > sd t0,?(sp)
> > > ...
> > > ld t0,?(s0)
> > > ...
> > > jr t0 // goto direct_caller always
> > > ```
> > >
> > > Hope I made it clear.
> > > > >
> > > > > Actually the reg t0 always saves the address of function entry with 8B
> > > > > offset, it should only
> > > > > changed by the IPMODIFY ops instead of the direct_ops.
> > > > > >>
> > > > > >> +}
> > > > > >> +
> > > > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > >>
> > > > > >> #endif /* __ASSEMBLY__ */
> > > > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > > > >> index 466c6ef217b1..b89c85a58569 100644
> > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > >> ENTRY(ftrace_regs_caller)
> > > > > >> SAVE_ABI_REGS 1
> > > > > >> + REG_S x0, PT_T1(sp)
> > > > > >> PREPARE_ARGS
> > > > > >>
> > > > > >> ftrace_regs_call:
> > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > > > >>
> > > > > >>
> > > > > >> RESTORE_ABI_REGS 1
> > > > > >> + bnez t1,.Ldirect
> > > > > >> jr t0
> > > > > >> +.Ldirect:
> > > > > >> + jr t1
> > > > > >> ENDPROC(ftrace_regs_caller)
> > > > > >>
> > > > > >> ENTRY(ftrace_caller)
> > > > > >> --
> > > > > >> 2.20.1
> > > > > >>
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best Regards
> > > > > > Guo Ren
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> > > Thanks,
> > > Song
> > Thanks,
> > Song
>
>
>
> --
> Best Regards
> Guo Ren
Thanks,
Song

2022-11-24 15:35:13

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Thu, Nov 24, 2022 at 7:49 PM Song Shuai <[email protected]> wrote:
>
> Guo Ren <[email protected]> 于2022年11月24日周四 03:40写道:
> >
> > On Thu, Nov 24, 2022 at 11:09 AM Song Shuai <[email protected]> wrote:
> > >
> > > Song Shuai <[email protected]> 于2022年11月24日周四 02:52写道:
> > > >
> > > > Guo Ren <[email protected]> 于2022年11月24日周四 02:08写道:
> > > > >
> > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > > > > >
> > > > > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > > > > >
> > > > > > > Cool job, thx.
> > > > > > >
> > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > > > > >>
> > > > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > > > > >>
> > > > > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > > > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > > > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > > > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > > > > >> provided for modifying direct_caller.
> > > > > > >>
> > > > > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > > > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > > > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > > > > >> setting of the address direct_caller by direct_ops->func and the
> > > > > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > > > > >> by the `jr` inst.
> > > > > > >>
> > > > > > >> Signed-off-by: Song Shuai <[email protected]>
> > > > > > >> ---
> > > > > > >> arch/riscv/Kconfig | 1 +
> > > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > > > > >> 3 files changed, 11 insertions(+)
> > > > > > >>
> > > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > > > > >> --- a/arch/riscv/Kconfig
> > > > > > >> +++ b/arch/riscv/Kconfig
> > > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > > > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > > > > >> index 01bebb28eabe..be4d57566139 100644
> > > > > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > > > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > > > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > > > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > > > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > > > > >> #define ftrace_graph_func ftrace_graph_func
> > > > > > >> +
> > > > > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > > > > >> +{
> > > > > > >> + regs->t1 = addr;
> > > > > > >
> > > > > > > How about regs->t0 = addr; ?
> > > > > > > And delete all mcount-dyn.S modification.
> > > > > > >
> > > > > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > > > > the reg t0 will never be changed when direct_caller returns.
> > > > > >
> > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > > > > direct_caller will enter the dead loop.
> > > > > ?
> > > > >
> > > > > ftrace_regs_caller->call_direct_funcs->
> > > > > arch_ftrace_set_direct_caller-> ftrace_regs_caller jr t0.
> > > > >
> > > > > Only call_direct_funcs call arch_ftrace_set_direct_caller !
> > > > >
> > > > > static void call_direct_funcs(unsigned long ip, unsigned long pip,
> > > > > struct ftrace_ops *ops, struct ftrace_regs *fregs)
> > > > > {
> > > > > struct pt_regs *regs = ftrace_get_regs(fregs);
> > > > > unsigned long addr;
> > > > >
> > > > > addr = ftrace_find_rec_direct(ip);
> > > > > if (!addr)
> > > > > return;
> > > > >
> > > > > arch_ftrace_set_direct_caller(regs, addr);
> > > > > }
> > > > >
> > > > When direct_caller and function tracer co-hook a function, the simple
> > > > diagram is like this:
> > > >
> > > > ```
> > > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> > > > function_trace_call // write ringbuffer
> > > > |
> > > > -> call_direct_funcs // regs->t1 = direct_caller
> > > > -> t1 !=0 && jr t1 // goto direct_caller
> > > > ```
> > > >
> > > ```
> > > f -- regs_caller -- list_func
> > > | | -- function_trace_call
> > > | | -- call_direct_funcs // t1 = addr
> > > |-- t1 !=0 && jr t1 // goto direct_caller
> > Cool diagram! Thx, but we still need a discussion:
> > f -- regs_caller
> > | -- SAVE_ABI_REGS 1
> > | -- list_func
> > | | -- function_trace_call
> > | | -- call_direct_funcs // t1 = addr
> > | -- RESTORE_ABI_REGS 1
> > |-- t1 !=0 && jr t1 // goto direct_caller
> > If you set t1 non-zero, then we always go to direct_caller. I think
> > the code is equal to set t0=addr.
> If only focusing on the whole ftrace_regs_caller code, you're right.
Yes, that's the problem I have; I didn't look at the sample code.

t0 -> ftrace caller
t1 -> my_tramp1 (direct caller)
ra -> original func return addr.

Okay, I would include these patches in v4.

Reviewed-by: Guo Ren <[email protected]>


>
> But we should also take direct_caller code into the consideration,
> if using t0 here, direct_caller will always return to itself as I
> described before.
> > | | -- call_direct_funcs // t0 = addr
> > | -- RESTORE_ABI_REGS 1
> > |-- jr t0 // goto direct_caller
> >
> > I think the only problem happens in the below non-existent situation:
> > f -- regs_caller
> > | -- SAVE_ABI_REGS 1
> > | -- list_func
> > | | -- call_direct_funcs // t0 = addr
> > | | -- function_trace_call //t0 contains
> > direct_caller instead
> function_trace_call is one type of global_ops->func which doesn't take care
> of the regs->t0.
>
> But the func of IPMODIFY ops (eg. livepatch, kprobe with posthandler)
> will change regs->epc which will then be restored to t0 in ftrace_regs_caller.
> And then the address of direct_caller will be covered in this situation.
> I'm not very clear about the process of the livepatch and kprobe in RISCV
> but I think we should keep t0 for their ipmodifying.
> > | -- RESTORE_ABI_REGS 1
> > |-- jr t0 // goto direct_caller
> >
> > The key issue is whether going to direct_caller correctly depends on
> > call_direct_funcs called, right?
> Yes, in other words, call_direct_func informs ftrace_regs_caller that
> there is a direct caller stored in t1, please jump to it first.
> >
> > > ```
> > > This diagram looks better.
> > > > And the direct_caller has a similar implement as ftrace_caller.
> > > >
> > > > ```
> > > > direct_caller:
> > > > add sp,sp,-?
> > > > sd t0,?(sp)
> > > > sd ra,?(sp)
> > > > call foo
> > > > ld t0,?(sp)
> > > > ld ra,?(sp)
> > > > add sp,sp,?
> > > > jr t0 // <- back to function entry
> > > > ```
> > > >
> > > > If we change regs->t0 as direct_caller and execute `jr t0` directly,
> > > > the direct_caller will always jump to itself due to its last `jr` inst.
> --- Here is what I described in the previous email.
> > > >
> > > > ```
> > > > func -> ftrace_regs_caller -> arch_ftrace_ops_list_func ->
> > > > function_trace_call // write ringbuffer
> > > > |
> > > > -> call_direct_funcs // regs->t0 = direct_caller
> > > > -> jr t0 // goto direct_caller
> > > >
> > > > direct_caller:
> > > > ...
> > > > sd t0,?(sp)
> > > > ...
> > > > ld t0,?(s0)
> > > > ...
> > > > jr t0 // goto direct_caller always
> > > > ```
> > > >
> > > > Hope I made it clear.
> > > > > >
> > > > > > Actually the reg t0 always saves the address of function entry with 8B
> > > > > > offset, it should only
> > > > > > changed by the IPMODIFY ops instead of the direct_ops.
> > > > > > >>
> > > > > > >> +}
> > > > > > >> +
> > > > > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > > >>
> > > > > > >> #endif /* __ASSEMBLY__ */
> > > > > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > > > > >> index 466c6ef217b1..b89c85a58569 100644
> > > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > > >> ENTRY(ftrace_regs_caller)
> > > > > > >> SAVE_ABI_REGS 1
> > > > > > >> + REG_S x0, PT_T1(sp)
> > > > > > >> PREPARE_ARGS
> > > > > > >>
> > > > > > >> ftrace_regs_call:
> > > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > > > > >>
> > > > > > >>
> > > > > > >> RESTORE_ABI_REGS 1
> > > > > > >> + bnez t1,.Ldirect
> > > > > > >> jr t0
> > > > > > >> +.Ldirect:
> > > > > > >> + jr t1
> > > > > > >> ENDPROC(ftrace_regs_caller)
> > > > > > >>
> > > > > > >> ENTRY(ftrace_caller)
> > > > > > >> --
> > > > > > >> 2.20.1
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best Regards
> > > > > > > Guo Ren
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best Regards
> > > > > Guo Ren
> > > > Thanks,
> > > > Song
> > > Thanks,
> > > Song
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> Thanks,
> Song



--
Best Regards
Guo Ren

2022-11-24 15:41:31

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
>
> Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> >
> > Cool job, thx.
> >
> > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> >>
> >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> >>
> >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> >> register_ftrace_direct[_multi] interfaces allowing users to register
> >> the customed trampoline (direct_caller) as the mcount for one or
> >> more target functions. And modify_ftrace_direct[_multi] are also
> >> provided for modifying direct_caller.
> >>
> >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> >> store the address of direct_caller in ftrace_regs_caller. After the
> >> setting of the address direct_caller by direct_ops->func and the
> >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> >> by the `jr` inst.
> >>
> >> Signed-off-by: Song Shuai <[email protected]>
> >> ---
> >> arch/riscv/Kconfig | 1 +
> >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> >> 3 files changed, 11 insertions(+)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index 39ec8d628cf6..d083ec08d0b6 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> >> select HAVE_FUNCTION_GRAPH_TRACER
> >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> >> index 01bebb28eabe..be4d57566139 100644
> >> --- a/arch/riscv/include/asm/ftrace.h
> >> +++ b/arch/riscv/include/asm/ftrace.h
> >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> >> #define ftrace_graph_func ftrace_graph_func
> >> +
> >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> >> +{
> >> + regs->t1 = addr;
> >
> > How about regs->t0 = addr; ?
> > And delete all mcount-dyn.S modification.
> >
> The direct_caller has the same program layout as the ftrace_caller, which means
> the reg t0 will never be changed when direct_caller returns.
>
> If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> direct_caller will enter the dead loop.
>
> Actually the reg t0 always saves the address of function entry with 8B
> offset, it should only
> changed by the IPMODIFY ops instead of the direct_ops.
How about:
static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
unsigned long addr)
{
regs->t1 = regs->t0;
regs->t0 = addr;

direct_caller:
add sp,sp,-?
sd t1,?(sp)
sd ra,?(sp)
call foo
ld t1,?(sp)
ld ra,?(sp)
add sp,sp,?
jr t1 // <- back to function entry

And delete all mcount-dyn.S modification.

> >>
> >> +}
> >> +
> >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >>
> >> #endif /* __ASSEMBLY__ */
> >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> >> index 466c6ef217b1..b89c85a58569 100644
> >> --- a/arch/riscv/kernel/mcount-dyn.S
> >> +++ b/arch/riscv/kernel/mcount-dyn.S
> >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >> ENTRY(ftrace_regs_caller)
> >> SAVE_ABI_REGS 1
> >> + REG_S x0, PT_T1(sp)
> >> PREPARE_ARGS
> >>
> >> ftrace_regs_call:
> >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> >>
> >>
> >> RESTORE_ABI_REGS 1
> >> + bnez t1,.Ldirect
> >> jr t0
> >> +.Ldirect:
> >> + jr t1
> >> ENDPROC(ftrace_regs_caller)
> >>
> >> ENTRY(ftrace_caller)
> >> --
> >> 2.20.1
> >>
> >
> >
> > --
> > Best Regards
> > Guo Ren



--
Best Regards
Guo Ren

2022-11-25 02:37:04

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Guo Ren <[email protected]> 于2022年11月24日周四 15:31写道:
>
> On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> >
> > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > >
> > > Cool job, thx.
> > >
> > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > >>
> > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > >>
> > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > >> the customed trampoline (direct_caller) as the mcount for one or
> > >> more target functions. And modify_ftrace_direct[_multi] are also
> > >> provided for modifying direct_caller.
> > >>
> > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > >> store the address of direct_caller in ftrace_regs_caller. After the
> > >> setting of the address direct_caller by direct_ops->func and the
> > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > >> by the `jr` inst.
> > >>
> > >> Signed-off-by: Song Shuai <[email protected]>
> > >> ---
> > >> arch/riscv/Kconfig | 1 +
> > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > >> 3 files changed, 11 insertions(+)
> > >>
> > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > >> --- a/arch/riscv/Kconfig
> > >> +++ b/arch/riscv/Kconfig
> > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > >> select HAVE_FUNCTION_GRAPH_TRACER
> > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > >> index 01bebb28eabe..be4d57566139 100644
> > >> --- a/arch/riscv/include/asm/ftrace.h
> > >> +++ b/arch/riscv/include/asm/ftrace.h
> > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > >> #define ftrace_graph_func ftrace_graph_func
> > >> +
> > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > >> +{
> > >> + regs->t1 = addr;
> > >
> > > How about regs->t0 = addr; ?
> > > And delete all mcount-dyn.S modification.
> > >
> > The direct_caller has the same program layout as the ftrace_caller, which means
> > the reg t0 will never be changed when direct_caller returns.
> >
> > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > direct_caller will enter the dead loop.
> >
> > Actually the reg t0 always saves the address of function entry with 8B
> > offset, it should only
> > changed by the IPMODIFY ops instead of the direct_ops.
> How about:
> static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
> unsigned long addr)
> {
> regs->t1 = regs->t0;
> regs->t0 = addr;
>
> direct_caller:
> add sp,sp,-?
> sd t1,?(sp)
direct_caller also serves as the first trampoline as ftrace_caller, like this:
```
func -- direct_caller
-- ftrace_[regs]_caller
```
So the t1 in this line has to be t0 to save the PC.
> sd ra,?(sp)
> call foo
> ld t1,?(sp)
And this line.
> ld ra,?(sp)
> add sp,sp,?
> jr t1 // <- back to function entry
>
> And delete all mcount-dyn.S modification.
>
> > >>
> > >> +}
> > >> +
> > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > >>
> > >> #endif /* __ASSEMBLY__ */
> > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > >> index 466c6ef217b1..b89c85a58569 100644
> > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > >> ENTRY(ftrace_regs_caller)
> > >> SAVE_ABI_REGS 1
> > >> + REG_S x0, PT_T1(sp)
> > >> PREPARE_ARGS
> > >>
> > >> ftrace_regs_call:
> > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > >>
> > >>
> > >> RESTORE_ABI_REGS 1
> > >> + bnez t1,.Ldirect
> > >> jr t0
> > >> +.Ldirect:
> > >> + jr t1
> > >> ENDPROC(ftrace_regs_caller)
> > >>
> > >> ENTRY(ftrace_caller)
> > >> --
> > >> 2.20.1
> > >>
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
>
>
>
> --
> Best Regards
> Guo Ren
Thanks,
Song

2022-11-25 03:37:22

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <[email protected]> wrote:
>
> Guo Ren <[email protected]> 于2022年11月24日周四 15:31写道:
> >
> > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > >
> > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > >
> > > > Cool job, thx.
> > > >
> > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > >>
> > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > >>
> > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > >> provided for modifying direct_caller.
> > > >>
> > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > >> setting of the address direct_caller by direct_ops->func and the
> > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > >> by the `jr` inst.
> > > >>
> > > >> Signed-off-by: Song Shuai <[email protected]>
> > > >> ---
> > > >> arch/riscv/Kconfig | 1 +
> > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > >> 3 files changed, 11 insertions(+)
> > > >>
> > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > >> --- a/arch/riscv/Kconfig
> > > >> +++ b/arch/riscv/Kconfig
> > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > >> index 01bebb28eabe..be4d57566139 100644
> > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > >> #define ftrace_graph_func ftrace_graph_func
> > > >> +
> > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > >> +{
> > > >> + regs->t1 = addr;
> > > >
> > > > How about regs->t0 = addr; ?
> > > > And delete all mcount-dyn.S modification.
> > > >
> > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > the reg t0 will never be changed when direct_caller returns.
> > >
> > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > direct_caller will enter the dead loop.
> > >
> > > Actually the reg t0 always saves the address of function entry with 8B
> > > offset, it should only
> > > changed by the IPMODIFY ops instead of the direct_ops.
> > How about:
> > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
> > unsigned long addr)
> > {
> > regs->t1 = regs->t0;
> > regs->t0 = addr;
> >
> > direct_caller:
> > add sp,sp,-?
> > sd t1,?(sp)
> direct_caller also serves as the first trampoline as ftrace_caller, like this:
> ```
> func -- direct_caller
> -- ftrace_[regs]_caller
> ```
> So the t1 in this line has to be t0 to save the PC.

direct_caller:
add sp,sp,-?
sd t1,?(sp)
sd t0, ?(so)
sd ra,?(sp)
mov t0, t1
call foo
ld t0,?(sp)
ld t1,?(sp)
ld ra,?(sp)
add sp,sp,?
jr t1 // <- back to function entry



> > sd ra,?(sp)
> > call foo
> > ld t1,?(sp)
> And this line.
> > ld ra,?(sp)
> > add sp,sp,?
> > jr t1 // <- back to function entry
> >
> > And delete all mcount-dyn.S modification.
> >
> > > >>
> > > >> +}
> > > >> +
> > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > >>
> > > >> #endif /* __ASSEMBLY__ */
> > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > >> index 466c6ef217b1..b89c85a58569 100644
> > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > >> ENTRY(ftrace_regs_caller)
> > > >> SAVE_ABI_REGS 1
> > > >> + REG_S x0, PT_T1(sp)
> > > >> PREPARE_ARGS
> > > >>
> > > >> ftrace_regs_call:
> > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > >>
> > > >>
> > > >> RESTORE_ABI_REGS 1
> > > >> + bnez t1,.Ldirect
> > > >> jr t0
> > > >> +.Ldirect:
> > > >> + jr t1
> > > >> ENDPROC(ftrace_regs_caller)
> > > >>
> > > >> ENTRY(ftrace_caller)
> > > >> --
> > > >> 2.20.1
> > > >>
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> Thanks,
> Song



--
Best Regards
Guo Ren

2022-11-25 07:00:14

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Guo Ren <[email protected]> 于2022年11月25日周五 03:10写道:
>
> On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <[email protected]> wrote:
> >
> > Guo Ren <[email protected]> 于2022年11月24日周四 15:31写道:
> > >
> > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > > >
> > > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > > >
> > > > > Cool job, thx.
> > > > >
> > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > > >>
> > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > > >>
> > > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > > >> provided for modifying direct_caller.
> > > > >>
> > > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > > >> setting of the address direct_caller by direct_ops->func and the
> > > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > > >> by the `jr` inst.
> > > > >>
> > > > >> Signed-off-by: Song Shuai <[email protected]>
> > > > >> ---
> > > > >> arch/riscv/Kconfig | 1 +
> > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > > >> 3 files changed, 11 insertions(+)
> > > > >>
> > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > > >> --- a/arch/riscv/Kconfig
> > > > >> +++ b/arch/riscv/Kconfig
> > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > > >> index 01bebb28eabe..be4d57566139 100644
> > > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > > >> #define ftrace_graph_func ftrace_graph_func
> > > > >> +
> > > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > > >> +{
> > > > >> + regs->t1 = addr;
> > > > >
> > > > > How about regs->t0 = addr; ?
> > > > > And delete all mcount-dyn.S modification.
> > > > >
> > > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > > the reg t0 will never be changed when direct_caller returns.
> > > >
> > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > > direct_caller will enter the dead loop.
> > > >
> > > > Actually the reg t0 always saves the address of function entry with 8B
> > > > offset, it should only
> > > > changed by the IPMODIFY ops instead of the direct_ops.
> > > How about:
> > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
> > > unsigned long addr)
> > > {
> > > regs->t1 = regs->t0;
> > > regs->t0 = addr;
> > >
> > > direct_caller:
> > > add sp,sp,-?
> > > sd t1,?(sp)
> > direct_caller also serves as the first trampoline as ftrace_caller, like this:
> > ```
> > func -- direct_caller
> > -- ftrace_[regs]_caller
> > ```
> > So the t1 in this line has to be t0 to save the PC.
>
> direct_caller:
> add sp,sp,-?
> sd t1,?(sp)
> sd t0, ?(so)
> sd ra,?(sp)
> mov t0, t1
This foo is the tracing function along with the direct_caller,
and it has the same parameters as the target function.
So the t0 or t1 here means nothing for this foo function.

No offense, but what's the purpose of this mv inst?
> call foo
> ld t0,?(sp)
> ld t1,?(sp)
> ld ra,?(sp)
> add sp,sp,?
> jr t1 // <- back to function entry
When direct_caller works as the first trampoline
the content of t1 here means nothing for the target function, neither
PC nor PIP.
>
>
> > > sd ra,?(sp)
> > > call foo
> > > ld t1,?(sp)
> > And this line.
> > > ld ra,?(sp)
> > > add sp,sp,?
> > > jr t1 // <- back to function entry
> > >
> > > And delete all mcount-dyn.S modification.
> > >
> > > > >>
> > > > >> +}
> > > > >> +
> > > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > >>
> > > > >> #endif /* __ASSEMBLY__ */
> > > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > > >> index 466c6ef217b1..b89c85a58569 100644
> > > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > >> ENTRY(ftrace_regs_caller)
> > > > >> SAVE_ABI_REGS 1
> > > > >> + REG_S x0, PT_T1(sp)
> > > > >> PREPARE_ARGS
> > > > >>
> > > > >> ftrace_regs_call:
> > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > > >>
> > > > >>
> > > > >> RESTORE_ABI_REGS 1
> > > > >> + bnez t1,.Ldirect
> > > > >> jr t0
> > > > >> +.Ldirect:
> > > > >> + jr t1
> > > > >> ENDPROC(ftrace_regs_caller)
> > > > >>
> > > > >> ENTRY(ftrace_caller)
> > > > >> --
> > > > >> 2.20.1
> > > > >>
> > > > >
> > > > >
> > > > > --
> > > > > Best Regards
> > > > > Guo Ren
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> > Thanks,
> > Song
>
>
>
> --
> Best Regards
> Guo Ren

2022-11-28 10:55:06

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Fri, Nov 25, 2022 at 2:33 PM Song Shuai <[email protected]> wrote:
>
> Guo Ren <[email protected]> 于2022年11月25日周五 03:10写道:
> >
> > On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <[email protected]> wrote:
> > >
> > > Guo Ren <[email protected]> 于2022年11月24日周四 15:31写道:
> > > >
> > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > > > >
> > > > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > > > >
> > > > > > Cool job, thx.
> > > > > >
> > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > > > >>
> > > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > > > >>
> > > > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > > > >> provided for modifying direct_caller.
> > > > > >>
> > > > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > > > >> setting of the address direct_caller by direct_ops->func and the
> > > > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > > > >> by the `jr` inst.
> > > > > >>
> > > > > >> Signed-off-by: Song Shuai <[email protected]>
> > > > > >> ---
> > > > > >> arch/riscv/Kconfig | 1 +
> > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > > > >> 3 files changed, 11 insertions(+)
> > > > > >>
> > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > > > >> --- a/arch/riscv/Kconfig
> > > > > >> +++ b/arch/riscv/Kconfig
> > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > > > >> index 01bebb28eabe..be4d57566139 100644
> > > > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > > > >> #define ftrace_graph_func ftrace_graph_func
> > > > > >> +
> > > > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > > > >> +{
> > > > > >> + regs->t1 = addr;
> > > > > >
> > > > > > How about regs->t0 = addr; ?
> > > > > > And delete all mcount-dyn.S modification.
> > > > > >
> > > > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > > > the reg t0 will never be changed when direct_caller returns.
> > > > >
> > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > > > direct_caller will enter the dead loop.
> > > > >
> > > > > Actually the reg t0 always saves the address of function entry with 8B
> > > > > offset, it should only
> > > > > changed by the IPMODIFY ops instead of the direct_ops.
> > > > How about:
> > > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
> > > > unsigned long addr)
> > > > {
> > > > regs->t1 = regs->t0;
> > > > regs->t0 = addr;
> > > >
> > > > direct_caller:
> > > > add sp,sp,-?
> > > > sd t1,?(sp)
> > > direct_caller also serves as the first trampoline as ftrace_caller, like this:
> > > ```
> > > func -- direct_caller
> > > -- ftrace_[regs]_caller
> > > ```
> > > So the t1 in this line has to be t0 to save the PC.
> >
> > direct_caller:
> > add sp,sp,-?
> > sd t1,?(sp)
> > sd t0, ?(so)
> > sd ra,?(sp)
> > mov t0, t1
> This foo is the tracing function along with the direct_caller,
> and it has the same parameters as the target function.
> So the t0 or t1 here means nothing for this foo function.
>
> No offense, but what's the purpose of this mv inst?
Here is my modification: [1]
[1]: https://lore.kernel.org/linux-riscv/[email protected]/

I've tested like this:
mount -t debugfs none /sys/kernel/debug/
cd /sys/kernel/debug/tracing/
echo handle_mm_fault > set_ftrace_filter
echo 'r:myr handle_mm_fault' > kprobe_events
echo function > current_tracer
echo 1 > events/kprobes/myr/enable
insmod /root/ftrace/ftrace-direct-too.ko
cat trace

cat-137 [000] ..... 3132.231948: handle_mm_fault
<-do_page_fault
cat-137 [000] ..... 3132.231973: my_direct_func:
handle mm fault vma=000000001ba58b17 address=aaaaaaaab689c8 flags=254
cat-137 [000] ..... 3132.232622: myr:
(do_page_fault+0x176/0x424 <- handle_mm_fault)
cat-137 [000] ..... 3132.232724: handle_mm_fault
<-do_page_fault
cat-137 [000] ..... 3132.232750: my_direct_func:
handle mm fault vma=000000001ba58b17 address=aaaaaaaab7df9c flags=254
cat-137 [000] ..... 3132.233385: myr:
(do_page_fault+0x176/0x424 <- handle_mm_fault)
cat-137 [000] ..... 3132.233744: handle_mm_fault
<-do_page_fault
cat-137 [000] ..... 3132.233772: my_direct_func:
handle mm fault vma=000000001ba58b17 address=aaaaaaaab2b2c8 flags=354
cat-137 [000] ..... 3132.234392: myr:
(do_page_fault+0x176/0x424 <- handle_mm_fault)
cat-137 [000] ..... 3132.234480: handle_mm_fault
<-do_page_fault
cat-137 [000] ..... 3132.234505: my_direct_func:
handle mm fault vma=000000001ba58b17 address=aaaaaaaab5afc0 flags=354
cat-137 [000] ..... 3132.235149: myr:
(do_page_fault+0x176/0x424 <- handle_mm_fault)
cat-137 [000] ..... 3132.235263: handle_mm_fault
<-do_page_fault
cat-137 [000] ..... 3132.235289: my_direct_func:
handle mm fault vma=0000000071a096de address=fffffff7fac530 flags=254
cat-137 [000] ..... 3132.235893: myr:
(do_page_fault+0x176/0x424 <- handle_mm_fault)


> > call foo
> > ld t0,?(sp)
> > ld t1,?(sp)
> > ld ra,?(sp)
> > add sp,sp,?
> > jr t1 // <- back to function entry
> When direct_caller works as the first trampoline
> the content of t1 here means nothing for the target function, neither
> PC nor PIP.
> >
> >
> > > > sd ra,?(sp)
> > > > call foo
> > > > ld t1,?(sp)
> > > And this line.
> > > > ld ra,?(sp)
> > > > add sp,sp,?
> > > > jr t1 // <- back to function entry
> > > >
> > > > And delete all mcount-dyn.S modification.
> > > >
> > > > > >>
> > > > > >> +}
> > > > > >> +
> > > > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > >>
> > > > > >> #endif /* __ASSEMBLY__ */
> > > > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > > > >> index 466c6ef217b1..b89c85a58569 100644
> > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > >> ENTRY(ftrace_regs_caller)
> > > > > >> SAVE_ABI_REGS 1
> > > > > >> + REG_S x0, PT_T1(sp)
> > > > > >> PREPARE_ARGS
> > > > > >>
> > > > > >> ftrace_regs_call:
> > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > > > >>
> > > > > >>
> > > > > >> RESTORE_ABI_REGS 1
> > > > > >> + bnez t1,.Ldirect
> > > > > >> jr t0
> > > > > >> +.Ldirect:
> > > > > >> + jr t1
> > > > > >> ENDPROC(ftrace_regs_caller)
> > > > > >>
> > > > > >> ENTRY(ftrace_caller)
> > > > > >> --
> > > > > >> 2.20.1
> > > > > >>
> > > > > >
> > > > > >
> > > > > > --
> > > > > > Best Regards
> > > > > > Guo Ren
> > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> > > Thanks,
> > > Song
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren



--
Best Regards
Guo Ren

2022-11-28 10:55:30

by Guo Ren

[permalink] [raw]
Subject: [PATCH 1/2] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

From: Song Shuai <[email protected]>

This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.

select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
register_ftrace_direct[_multi] interfaces allowing users to register
the customed trampoline (direct_caller) as the mcount for one or
more target functions. And modify_ftrace_direct[_multi] are also
provided for modifying direct_caller.

To make the direct_caller and the other ftrace hooks (eg. function/fgraph
tracer, k[ret]probes) co-exist, a temporary register is nominated to
store the address of direct_caller in ftrace_regs_caller. After the
setting of the address direct_caller by direct_ops->func and the
RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
by the `jr` inst.

Signed-off-by: Song Shuai <[email protected]>
Co-developed-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
---
Remove modification of mcount-dyn.S. (Guo Ren)
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/ftrace.h | 26 ++++++++++++++++++++++++++
2 files changed, 27 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 1d0e5838b11b..2828537abfcd 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -278,6 +278,7 @@ config ARCH_RV64I
select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
+ select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 84f856a3286e..d2c159cdfa5c 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -114,6 +114,32 @@ struct ftrace_regs;
void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
struct ftrace_ops *op, struct ftrace_regs *fregs);
#define ftrace_graph_func ftrace_graph_func
+
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+ /*
+ * Backup origin detour target (t0) in t1, and set t0 with addr.
+ *
+ * Here is the sample code for riscv direct_caller:
+ *
+ * addi sp,sp,-16
+ * move t0,t1
+ * ^^^^^^^^^^
+ * sd t0,0(sp)
+ * sd ra,8(sp)
+ * call my_direct_func1
+ * ld t0,0(sp)
+ * ld ra,8(sp)
+ * addi sp,sp,16
+ * jr t0
+ *
+ * (Set t0 with t1 for continuous detour, because t1 contains origin detour
+ * target)
+ */
+ regs->t1 = regs->t0;
+ regs->t0 = addr;
+}
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */

#endif /* __ASSEMBLY__ */
--
2.36.1

2022-11-28 11:26:39

by Guo Ren

[permalink] [raw]
Subject: [PATCH 2/2] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]

From: Song Shuai <[email protected]>

select HAVE_SAMPLE_FTRACE_DIRECT and HAVE_SAMPLE_FTRACE_DIRECT_MULTI
for ARCH_RV64I in arch/riscv/Kconfig. And add riscv asm code for
the ftrace-direct*.c files in samples/ftrace/.

Signed-off-by: Song Shuai <[email protected]>
Co-developed-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
---
Below is modified by Guo Ren:

diff --git a/samples/ftrace/ftrace-direct-modify.c
b/samples/ftrace/ftrace-direct-modify.c
index 5582d000d803..d970b46ecc4e 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -29,6 +29,7 @@ asm (" .pushsection .text, \"ax\",
@progbits\n"
" .globl my_tramp1\n"
" my_tramp1:\n"
" addi sp,sp,-16\n"
+" move t0,t1\n"
" sd t0,0(sp)\n"
" sd ra,8(sp)\n"
" call my_direct_func1\n"
diff --git a/samples/ftrace/ftrace-direct-multi-modify.c
b/samples/ftrace/ftrace-direct-multi-modify.c
index 43e02d39c652..9534e18fa6ed 100644
--- a/samples/ftrace/ftrace-direct-multi-modify.c
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -28,6 +28,7 @@ asm (" .pushsection .text, \"ax\",
@progbits\n"
" my_tramp1:\n"
" addi sp,sp,-24\n"
" sd a0,0(sp)\n"
+" move t0,t1\n"
" sd t0,8(sp)\n"
" sd ra,16(sp)\n"
" call my_direct_func1\n"
diff --git a/samples/ftrace/ftrace-direct-multi.c
b/samples/ftrace/ftrace-direct-multi.c
index df91339d3095..2111478b0463 100644
--- a/samples/ftrace/ftrace-direct-multi.c
+++ b/samples/ftrace/ftrace-direct-multi.c
@@ -23,6 +23,7 @@ asm (" .pushsection .text, \"ax\",
@progbits\n"
" my_tramp:\n"
" addi sp,sp,-24\n"
" sd a0,0(sp)\n"
+" move t0,t1\n"
" sd t0,8(sp)\n"
" sd ra,16(sp)\n"
" call my_direct_func\n"
diff --git a/samples/ftrace/ftrace-direct-too.c
b/samples/ftrace/ftrace-direct-too.c
index fe3b8c4f3336..e80981d3dc0a 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -27,6 +27,7 @@ asm (" .pushsection .text, \"ax\",
@progbits\n"
" sd a0,0(sp)\n"
" sd a1,8(sp)\n"
" sd a2,16(sp)\n"
+" move t0,t1\n"
" sd t0,24(sp)\n"
" sd ra,32(sp)\n"
" call my_direct_func\n"
diff --git a/samples/ftrace/ftrace-direct.c
b/samples/ftrace/ftrace-direct.c
index 0784024b6653..b6cae1b5cb66 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -22,6 +22,7 @@ asm (" .pushsection .text, \"ax\",
@progbits\n"
" my_tramp:\n"
" addi sp,sp,-24\n"
" sd a0,0(sp)\n"
+" move t0,t1\n"
" sd t0,8(sp)\n"
" sd ra,16(sp)\n"
" call my_direct_func\n"
---
arch/riscv/Kconfig | 2 ++
samples/ftrace/ftrace-direct-modify.c | 34 ++++++++++++++++++
samples/ftrace/ftrace-direct-multi-modify.c | 38 +++++++++++++++++++++
samples/ftrace/ftrace-direct-multi.c | 23 +++++++++++++
samples/ftrace/ftrace-direct-too.c | 27 +++++++++++++++
samples/ftrace/ftrace-direct.c | 23 +++++++++++++
6 files changed, 147 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 2828537abfcd..0a1da03e88c2 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -279,6 +279,8 @@ config ARCH_RV64I
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+ select HAVE_SAMPLE_FTRACE_DIRECT
+ select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
select HAVE_FUNCTION_GRAPH_TRACER
select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index 39146fa83e20..6a633ff4cf6d 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -22,6 +22,40 @@ extern void my_tramp2(void *);

static unsigned long my_ip = (unsigned long)schedule;

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:\n"
+" addi sp,sp,-16\n"
+" move t0,t1\n"
+" sd t0,0(sp)\n"
+" sd ra,8(sp)\n"
+" call my_direct_func1\n"
+" ld t0,0(sp)\n"
+" ld ra,8(sp)\n"
+" addi sp,sp,16\n"
+" jr t0\n"
+" .size my_tramp1, .-my_tramp1\n"
+
+" .type my_tramp2, @function\n"
+" .globl my_tramp2\n"
+" my_tramp2:\n"
+" addi sp,sp,-16\n"
+" sd t0,0(sp)\n"
+" sd ra,8(sp)\n"
+" call my_direct_func2\n"
+" ld t0,0(sp)\n"
+" ld ra,8(sp)\n"
+" addi sp,sp,16\n"
+" jr t0\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
index 65aa94d96f4e..9534e18fa6ed 100644
--- a/samples/ftrace/ftrace-direct-multi-modify.c
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -20,6 +20,44 @@ void my_direct_func2(unsigned long ip)
extern void my_tramp1(void *);
extern void my_tramp2(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp1, @function\n"
+" .globl my_tramp1\n"
+" my_tramp1:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" move t0,t1\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func1\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp1, .-my_tramp1\n"
+
+" .type my_tramp2, @function\n"
+" .globl my_tramp2\n"
+" my_tramp2:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func2\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp2, .-my_tramp2\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct-multi.c b/samples/ftrace/ftrace-direct-multi.c
index 41ded7c615c7..2111478b0463 100644
--- a/samples/ftrace/ftrace-direct-multi.c
+++ b/samples/ftrace/ftrace-direct-multi.c
@@ -15,6 +15,29 @@ void my_direct_func(unsigned long ip)

extern void my_tramp(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" move t0,t1\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
index 6690468c5cc2..e80981d3dc0a 100644
--- a/samples/ftrace/ftrace-direct-too.c
+++ b/samples/ftrace/ftrace-direct-too.c
@@ -17,6 +17,33 @@ void my_direct_func(struct vm_area_struct *vma,

extern void my_tramp(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:\n"
+" addi sp,sp,-40\n"
+" sd a0,0(sp)\n"
+" sd a1,8(sp)\n"
+" sd a2,16(sp)\n"
+" move t0,t1\n"
+" sd t0,24(sp)\n"
+" sd ra,32(sp)\n"
+" call my_direct_func\n"
+" ld a0,0(sp)\n"
+" ld a1,8(sp)\n"
+" ld a2,16(sp)\n"
+" ld t0,24(sp)\n"
+" ld ra,32(sp)\n"
+" addi sp,sp,40\n"
+" jr t0\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
index e8f1e440b9b8..b6cae1b5cb66 100644
--- a/samples/ftrace/ftrace-direct.c
+++ b/samples/ftrace/ftrace-direct.c
@@ -14,6 +14,29 @@ void my_direct_func(struct task_struct *p)

extern void my_tramp(void *);

+#ifdef CONFIG_RISCV
+
+asm (" .pushsection .text, \"ax\", @progbits\n"
+" .type my_tramp, @function\n"
+" .globl my_tramp\n"
+" my_tramp:\n"
+" addi sp,sp,-24\n"
+" sd a0,0(sp)\n"
+" move t0,t1\n"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" call my_direct_func\n"
+" ld a0,0(sp)\n"
+" ld t0,8(sp)\n"
+" ld ra,16(sp)\n"
+" addi sp,sp,24\n"
+" jr t0\n"
+" .size my_tramp, .-my_tramp\n"
+" .popsection\n"
+);
+
+#endif /* CONFIG_RISCV */
+
#ifdef CONFIG_X86_64

#include <asm/ibt.h>
--
2.36.1

2022-11-28 13:04:34

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Abandoned

On Mon, Nov 28, 2022 at 6:12 PM <[email protected]> wrote:
>
> From: Song Shuai <[email protected]>
>
> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
>
> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> register_ftrace_direct[_multi] interfaces allowing users to register
> the customed trampoline (direct_caller) as the mcount for one or
> more target functions. And modify_ftrace_direct[_multi] are also
> provided for modifying direct_caller.
>
> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> store the address of direct_caller in ftrace_regs_caller. After the
> setting of the address direct_caller by direct_ops->func and the
> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> by the `jr` inst.
>
> Signed-off-by: Song Shuai <[email protected]>
> Co-developed-by: Guo Ren <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> ---
> Remove modification of mcount-dyn.S. (Guo Ren)
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/ftrace.h | 26 ++++++++++++++++++++++++++
> 2 files changed, 27 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 1d0e5838b11b..2828537abfcd 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -278,6 +278,7 @@ config ARCH_RV64I
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER
> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 84f856a3286e..d2c159cdfa5c 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -114,6 +114,32 @@ struct ftrace_regs;
> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs);
> #define ftrace_graph_func ftrace_graph_func
> +
> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> +{
> + /*
> + * Backup origin detour target (t0) in t1, and set t0 with addr.
> + *
> + * Here is the sample code for riscv direct_caller:
> + *
> + * addi sp,sp,-16
> + * move t0,t1
> + * ^^^^^^^^^^
> + * sd t0,0(sp)
> + * sd ra,8(sp)
> + * call my_direct_func1
> + * ld t0,0(sp)
> + * ld ra,8(sp)
> + * addi sp,sp,16
> + * jr t0
> + *
> + * (Set t0 with t1 for continuous detour, because t1 contains origin detour
> + * target)
> + */
> + regs->t1 = regs->t0;
> + regs->t0 = addr;
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>
> #endif /* __ASSEMBLY__ */
> --
> 2.36.1
>


--
Best Regards
Guo Ren

2022-11-28 13:30:50

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Mon, Nov 28, 2022 at 6:17 PM Guo Ren <[email protected]> wrote:
>
> On Fri, Nov 25, 2022 at 2:33 PM Song Shuai <[email protected]> wrote:
> >
> > Guo Ren <[email protected]> 于2022年11月25日周五 03:10写道:
> > >
> > > On Fri, Nov 25, 2022 at 9:53 AM Song Shuai <[email protected]> wrote:
> > > >
> > > > Guo Ren <[email protected]> 于2022年11月24日周四 15:31写道:
> > > > >
> > > > > On Thu, Nov 24, 2022 at 1:27 AM Song Shuai <[email protected]> wrote:
> > > > > >
> > > > > > Guo Ren <[email protected]> 于2022年11月23日周三 23:02写道:
> > > > > > >
> > > > > > > Cool job, thx.
> > > > > > >
> > > > > > > On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> > > > > > >>
> > > > > > >> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> > > > > > >>
> > > > > > >> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > > > > > >> register_ftrace_direct[_multi] interfaces allowing users to register
> > > > > > >> the customed trampoline (direct_caller) as the mcount for one or
> > > > > > >> more target functions. And modify_ftrace_direct[_multi] are also
> > > > > > >> provided for modifying direct_caller.
> > > > > > >>
> > > > > > >> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > > > > > >> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > > > > > >> store the address of direct_caller in ftrace_regs_caller. After the
> > > > > > >> setting of the address direct_caller by direct_ops->func and the
> > > > > > >> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > > > > > >> by the `jr` inst.
> > > > > > >>
> > > > > > >> Signed-off-by: Song Shuai <[email protected]>
> > > > > > >> ---
> > > > > > >> arch/riscv/Kconfig | 1 +
> > > > > > >> arch/riscv/include/asm/ftrace.h | 6 ++++++
> > > > > > >> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > > > > > >> 3 files changed, 11 insertions(+)
> > > > > > >>
> > > > > > >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > > > > >> index 39ec8d628cf6..d083ec08d0b6 100644
> > > > > > >> --- a/arch/riscv/Kconfig
> > > > > > >> +++ b/arch/riscv/Kconfig
> > > > > > >> @@ -278,6 +278,7 @@ config ARCH_RV64I
> > > > > > >> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > > > > > >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > > > > > >> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > > > > >> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > > > > > >> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > > > > > >> select HAVE_FUNCTION_GRAPH_TRACER
> > > > > > >> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > > > > > >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > > > > > >> index 01bebb28eabe..be4d57566139 100644
> > > > > > >> --- a/arch/riscv/include/asm/ftrace.h
> > > > > > >> +++ b/arch/riscv/include/asm/ftrace.h
> > > > > > >> @@ -114,6 +114,12 @@ struct ftrace_regs;
> > > > > > >> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > > > > > >> struct ftrace_ops *op, struct ftrace_regs *fregs);
> > > > > > >> #define ftrace_graph_func ftrace_graph_func
> > > > > > >> +
> > > > > > >> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > > > > > >> +{
> > > > > > >> + regs->t1 = addr;
> > > > > > >
> > > > > > > How about regs->t0 = addr; ?
> > > > > > > And delete all mcount-dyn.S modification.
> > > > > > >
> > > > > > The direct_caller has the same program layout as the ftrace_caller, which means
> > > > > > the reg t0 will never be changed when direct_caller returns.
> > > > > >
> > > > > > If regs->t0 changes here and ftrace_regs_caller executes `jr t0`,
> > > > > > direct_caller will enter the dead loop.
> > > > > >
> > > > > > Actually the reg t0 always saves the address of function entry with 8B
> > > > > > offset, it should only
> > > > > > changed by the IPMODIFY ops instead of the direct_ops.
> > > > > How about:
> > > > > static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs,
> > > > > unsigned long addr)
> > > > > {
> > > > > regs->t1 = regs->t0;
> > > > > regs->t0 = addr;
> > > > >
> > > > > direct_caller:
> > > > > add sp,sp,-?
> > > > > sd t1,?(sp)
> > > > direct_caller also serves as the first trampoline as ftrace_caller, like this:
> > > > ```
> > > > func -- direct_caller
> > > > -- ftrace_[regs]_caller
> > > > ```
> > > > So the t1 in this line has to be t0 to save the PC.
> > >
> > > direct_caller:
> > > add sp,sp,-?
> > > sd t1,?(sp)
> > > sd t0, ?(so)
> > > sd ra,?(sp)
> > > mov t0, t1
> > This foo is the tracing function along with the direct_caller,
> > and it has the same parameters as the target function.
> > So the t0 or t1 here means nothing for this foo function.
> >
> > No offense, but what's the purpose of this mv inst?
> Here is my modification: [1]
> [1]: https://lore.kernel.org/linux-riscv/[email protected]/
>
> I've tested like this:
> mount -t debugfs none /sys/kernel/debug/
> cd /sys/kernel/debug/tracing/
> echo handle_mm_fault > set_ftrace_filter
> echo 'r:myr handle_mm_fault' > kprobe_events
> echo function > current_tracer
> echo 1 > events/kprobes/myr/enable
> insmod /root/ftrace/ftrace-direct-too.ko
> cat trace
>
> cat-137 [000] ..... 3132.231948: handle_mm_fault
> <-do_page_fault
> cat-137 [000] ..... 3132.231973: my_direct_func:
> handle mm fault vma=000000001ba58b17 address=aaaaaaaab689c8 flags=254
> cat-137 [000] ..... 3132.232622: myr:
> (do_page_fault+0x176/0x424 <- handle_mm_fault)
> cat-137 [000] ..... 3132.232724: handle_mm_fault
> <-do_page_fault
> cat-137 [000] ..... 3132.232750: my_direct_func:
> handle mm fault vma=000000001ba58b17 address=aaaaaaaab7df9c flags=254
> cat-137 [000] ..... 3132.233385: myr:
> (do_page_fault+0x176/0x424 <- handle_mm_fault)
> cat-137 [000] ..... 3132.233744: handle_mm_fault
> <-do_page_fault
> cat-137 [000] ..... 3132.233772: my_direct_func:
> handle mm fault vma=000000001ba58b17 address=aaaaaaaab2b2c8 flags=354
> cat-137 [000] ..... 3132.234392: myr:
> (do_page_fault+0x176/0x424 <- handle_mm_fault)
> cat-137 [000] ..... 3132.234480: handle_mm_fault
> <-do_page_fault
> cat-137 [000] ..... 3132.234505: my_direct_func:
> handle mm fault vma=000000001ba58b17 address=aaaaaaaab5afc0 flags=354
> cat-137 [000] ..... 3132.235149: myr:
> (do_page_fault+0x176/0x424 <- handle_mm_fault)
> cat-137 [000] ..... 3132.235263: handle_mm_fault
> <-do_page_fault
> cat-137 [000] ..... 3132.235289: my_direct_func:
> handle mm fault vma=0000000071a096de address=fffffff7fac530 flags=254
> cat-137 [000] ..... 3132.235893: myr:
> (do_page_fault+0x176/0x424 <- handle_mm_fault)
I got a fail with no ftrace enabled. Yes, your patch is correct.
# insmod /root/ftrace/ftrace-direct-too.ko
[ 13.586821]
[ 13.586970] **********************************************************
[ 13.587186] ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **
[ 13.587396] ** **
[ 13.587594] ** trace_printk() being used. Allocating extra memory. **
[ 13.587804] ** **
[ 13.588240] ** This means that this is a DEBUG kernel and it is **
[ 13.588488] ** unsafe for production use. **
[ 13.588735] ** **
[ 13.588966] ** If you see this message and you are not debugging **
[ 13.589200] ** the kernel, report this immediately to your vendor! **
[ 13.589435] ** **
[ 13.589682] ** NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE NOTICE **
[ 13.589922] **********************************************************
[ 13.632876] Unable to handle kernel access to user memory without
uaccess routines at virtual address 0000000000000000
[ 13.633717] Oops [#1]
[ 13.633858] Modules linked in: ftrace_direct_too
[ 13.634294] CPU: 0 PID: 121 Comm: sh Not tainted
6.1.0-rc1-00016-g459bb40f7d97 #388
[ 13.634652] Hardware name: riscv-virtio,qemu (DT)
[ 13.634962] epc : 0x0
[ 13.635313] ra : do_page_fault+0x176/0x424
[ 13.635548] epc : 0000000000000000 ra : ffffffff8000c8d0 sp :
ff2000000083be80
[ 13.635819] gp : ffffffff815dc778 tp : ff600000033a9680 t0 :
0000000000000000
[ 13.636144] t1 : 0000000000000000 t2 : 0000000000000000 s0 :
ff2000000083bee0
[ 13.636427] s1 : ff2000000083bee0 a0 : ff600000038cb660 a1 :
00fffffff7e74790
[ 13.636725] a2 : 0000000000000255 a3 : 0000000000000000 a4 :
0000000000000000
[ 13.637022] a5 : 0000000000000000 a6 : 0000000000000000 a7 :
0000000000000000
[ 13.637308] s2 : 00fffffff7e74790 s3 : 000000000000000f s4 :
ff6000000319b400
[ 13.637600] s5 : ff600000033a9680 s6 : ff600000038cb660 s7 :
ff6000000319b460
[ 13.637889] s8 : 0000000000000255 s9 : 0000000000000001 s10:
00fffffffffff4dc
[ 13.638172] s11: 0000000000000004 t3 : 0000000000000000 t4 :
0000000000000000
[ 13.638453] t5 : 0000000000000000 t6 : 0000000000000000
[ 13.638679] status: 0000000200000120 badaddr: 0000000000000000
cause: 000000000000000c
[ 13.640035] ---[ end trace 0000000000000000 ]---


>
>
> > > call foo
> > > ld t0,?(sp)
> > > ld t1,?(sp)
> > > ld ra,?(sp)
> > > add sp,sp,?
> > > jr t1 // <- back to function entry
> > When direct_caller works as the first trampoline
> > the content of t1 here means nothing for the target function, neither
> > PC nor PIP.
> > >
> > >
> > > > > sd ra,?(sp)
> > > > > call foo
> > > > > ld t1,?(sp)
> > > > And this line.
> > > > > ld ra,?(sp)
> > > > > add sp,sp,?
> > > > > jr t1 // <- back to function entry
> > > > >
> > > > > And delete all mcount-dyn.S modification.
> > > > >
> > > > > > >>
> > > > > > >> +}
> > > > > > >> +
> > > > > > >> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > > >>
> > > > > > >> #endif /* __ASSEMBLY__ */
> > > > > > >> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > > > > > >> index 466c6ef217b1..b89c85a58569 100644
> > > > > > >> --- a/arch/riscv/kernel/mcount-dyn.S
> > > > > > >> +++ b/arch/riscv/kernel/mcount-dyn.S
> > > > > > >> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > > > > > >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > > > > > >> ENTRY(ftrace_regs_caller)
> > > > > > >> SAVE_ABI_REGS 1
> > > > > > >> + REG_S x0, PT_T1(sp)
> > > > > > >> PREPARE_ARGS
> > > > > > >>
> > > > > > >> ftrace_regs_call:
> > > > > > >> @@ -241,7 +242,10 @@ ftrace_regs_call:
> > > > > > >>
> > > > > > >>
> > > > > > >> RESTORE_ABI_REGS 1
> > > > > > >> + bnez t1,.Ldirect
> > > > > > >> jr t0
> > > > > > >> +.Ldirect:
> > > > > > >> + jr t1
> > > > > > >> ENDPROC(ftrace_regs_caller)
> > > > > > >>
> > > > > > >> ENTRY(ftrace_caller)
> > > > > > >> --
> > > > > > >> 2.20.1
> > > > > > >>
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > Best Regards
> > > > > > > Guo Ren
> > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Best Regards
> > > > > Guo Ren
> > > > Thanks,
> > > > Song
> > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
>
>
>
> --
> Best Regards
> Guo Ren



--
Best Regards
Guo Ren

2022-11-28 13:37:16

by Song Shuai

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

Guo Ren <[email protected]> 于2022年11月28日周一 13:17写道:
>
> On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
> >
> > This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
> >
> > select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> > register_ftrace_direct[_multi] interfaces allowing users to register
> > the customed trampoline (direct_caller) as the mcount for one or
> > more target functions. And modify_ftrace_direct[_multi] are also
> > provided for modifying direct_caller.
> >
> > To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> > tracer, k[ret]probes) co-exist, a temporary register is nominated to
> > store the address of direct_caller in ftrace_regs_caller. After the
> > setting of the address direct_caller by direct_ops->func and the
> > RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> > by the `jr` inst.
> >
> > Signed-off-by: Song Shuai <[email protected]>
> > ---
> > arch/riscv/Kconfig | 1 +
> > arch/riscv/include/asm/ftrace.h | 6 ++++++
> > arch/riscv/kernel/mcount-dyn.S | 4 ++++
> > 3 files changed, 11 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 39ec8d628cf6..d083ec08d0b6 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -278,6 +278,7 @@ config ARCH_RV64I
> > select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > select HAVE_FUNCTION_GRAPH_TRACER
> > select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > index 01bebb28eabe..be4d57566139 100644
> > --- a/arch/riscv/include/asm/ftrace.h
> > +++ b/arch/riscv/include/asm/ftrace.h
> > @@ -114,6 +114,12 @@ struct ftrace_regs;
> > void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> > struct ftrace_ops *op, struct ftrace_regs *fregs);
> > #define ftrace_graph_func ftrace_graph_func
> > +
> > +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > +{
> > + regs->t1 = addr;
> > +}
> > +
> > #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >
> > #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > index 466c6ef217b1..b89c85a58569 100644
> > --- a/arch/riscv/kernel/mcount-dyn.S
> > +++ b/arch/riscv/kernel/mcount-dyn.S
> > @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > ENTRY(ftrace_regs_caller)
> > SAVE_ABI_REGS 1
> > + REG_S x0, PT_T1(sp)
> ENTRY(ftrace_regs_caller)
> + move t1, zero
> SAVE_ABI_REGS 1
> PREPARE_ARGS
>
> Shall we use a move here, instead?
That seems ok to me. Just move it.
>
> > PREPARE_ARGS
> >
> > ftrace_regs_call:
> > @@ -241,7 +242,10 @@ ftrace_regs_call:
> >
> >
> > RESTORE_ABI_REGS 1
> > + bnez t1,.Ldirect
> > jr t0
> > +.Ldirect:
> > + jr t1
> > ENDPROC(ftrace_regs_caller)
> >
> > ENTRY(ftrace_caller)
> > --
> > 2.20.1
> >
>
>
> --
> Best Regards
> Guo Ren



--
Thanks,
Song

2022-11-28 14:00:48

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 1/2] riscv/ftrace: add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

On Wed, Nov 23, 2022 at 10:20 PM Song Shuai <[email protected]> wrote:
>
> This patch adds DYNAMIC_FTRACE_WITH_DIRECT_CALLS support for RISC-V.
>
> select the DYNAMIC_FTRACE_WITH_DIRECT_CALLS to provide the
> register_ftrace_direct[_multi] interfaces allowing users to register
> the customed trampoline (direct_caller) as the mcount for one or
> more target functions. And modify_ftrace_direct[_multi] are also
> provided for modifying direct_caller.
>
> To make the direct_caller and the other ftrace hooks (eg. function/fgraph
> tracer, k[ret]probes) co-exist, a temporary register is nominated to
> store the address of direct_caller in ftrace_regs_caller. After the
> setting of the address direct_caller by direct_ops->func and the
> RESTORE_REGS in ftrace_regs_caller, direct_caller will be jumped to
> by the `jr` inst.
>
> Signed-off-by: Song Shuai <[email protected]>
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/ftrace.h | 6 ++++++
> arch/riscv/kernel/mcount-dyn.S | 4 ++++
> 3 files changed, 11 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 39ec8d628cf6..d083ec08d0b6 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -278,6 +278,7 @@ config ARCH_RV64I
> select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER
> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 01bebb28eabe..be4d57566139 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -114,6 +114,12 @@ struct ftrace_regs;
> void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> struct ftrace_ops *op, struct ftrace_regs *fregs);
> #define ftrace_graph_func ftrace_graph_func
> +
> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> +{
> + regs->t1 = addr;
> +}
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>
> #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> index 466c6ef217b1..b89c85a58569 100644
> --- a/arch/riscv/kernel/mcount-dyn.S
> +++ b/arch/riscv/kernel/mcount-dyn.S
> @@ -233,6 +233,7 @@ ENDPROC(ftrace_caller)
> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> ENTRY(ftrace_regs_caller)
> SAVE_ABI_REGS 1
> + REG_S x0, PT_T1(sp)
ENTRY(ftrace_regs_caller)
+ move t1, zero
SAVE_ABI_REGS 1
PREPARE_ARGS

Shall we use a move here, instead?

> PREPARE_ARGS
>
> ftrace_regs_call:
> @@ -241,7 +242,10 @@ ftrace_regs_call:
>
>
> RESTORE_ABI_REGS 1
> + bnez t1,.Ldirect
> jr t0
> +.Ldirect:
> + jr t1
> ENDPROC(ftrace_regs_caller)
>
> ENTRY(ftrace_caller)
> --
> 2.20.1
>


--
Best Regards
Guo Ren

2022-11-28 14:42:53

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH 2/2] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]

Abandoned.

On Mon, Nov 28, 2022 at 6:12 PM <[email protected]> wrote:
>
> From: Song Shuai <[email protected]>
>
> select HAVE_SAMPLE_FTRACE_DIRECT and HAVE_SAMPLE_FTRACE_DIRECT_MULTI
> for ARCH_RV64I in arch/riscv/Kconfig. And add riscv asm code for
> the ftrace-direct*.c files in samples/ftrace/.
>
> Signed-off-by: Song Shuai <[email protected]>
> Co-developed-by: Guo Ren <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> ---
> Below is modified by Guo Ren:
>
> diff --git a/samples/ftrace/ftrace-direct-modify.c
> b/samples/ftrace/ftrace-direct-modify.c
> index 5582d000d803..d970b46ecc4e 100644
> --- a/samples/ftrace/ftrace-direct-modify.c
> +++ b/samples/ftrace/ftrace-direct-modify.c
> @@ -29,6 +29,7 @@ asm (" .pushsection .text, \"ax\",
> @progbits\n"
> " .globl my_tramp1\n"
> " my_tramp1:\n"
> " addi sp,sp,-16\n"
> +" move t0,t1\n"
> " sd t0,0(sp)\n"
> " sd ra,8(sp)\n"
> " call my_direct_func1\n"
> diff --git a/samples/ftrace/ftrace-direct-multi-modify.c
> b/samples/ftrace/ftrace-direct-multi-modify.c
> index 43e02d39c652..9534e18fa6ed 100644
> --- a/samples/ftrace/ftrace-direct-multi-modify.c
> +++ b/samples/ftrace/ftrace-direct-multi-modify.c
> @@ -28,6 +28,7 @@ asm (" .pushsection .text, \"ax\",
> @progbits\n"
> " my_tramp1:\n"
> " addi sp,sp,-24\n"
> " sd a0,0(sp)\n"
> +" move t0,t1\n"
> " sd t0,8(sp)\n"
> " sd ra,16(sp)\n"
> " call my_direct_func1\n"
> diff --git a/samples/ftrace/ftrace-direct-multi.c
> b/samples/ftrace/ftrace-direct-multi.c
> index df91339d3095..2111478b0463 100644
> --- a/samples/ftrace/ftrace-direct-multi.c
> +++ b/samples/ftrace/ftrace-direct-multi.c
> @@ -23,6 +23,7 @@ asm (" .pushsection .text, \"ax\",
> @progbits\n"
> " my_tramp:\n"
> " addi sp,sp,-24\n"
> " sd a0,0(sp)\n"
> +" move t0,t1\n"
> " sd t0,8(sp)\n"
> " sd ra,16(sp)\n"
> " call my_direct_func\n"
> diff --git a/samples/ftrace/ftrace-direct-too.c
> b/samples/ftrace/ftrace-direct-too.c
> index fe3b8c4f3336..e80981d3dc0a 100644
> --- a/samples/ftrace/ftrace-direct-too.c
> +++ b/samples/ftrace/ftrace-direct-too.c
> @@ -27,6 +27,7 @@ asm (" .pushsection .text, \"ax\",
> @progbits\n"
> " sd a0,0(sp)\n"
> " sd a1,8(sp)\n"
> " sd a2,16(sp)\n"
> +" move t0,t1\n"
> " sd t0,24(sp)\n"
> " sd ra,32(sp)\n"
> " call my_direct_func\n"
> diff --git a/samples/ftrace/ftrace-direct.c
> b/samples/ftrace/ftrace-direct.c
> index 0784024b6653..b6cae1b5cb66 100644
> --- a/samples/ftrace/ftrace-direct.c
> +++ b/samples/ftrace/ftrace-direct.c
> @@ -22,6 +22,7 @@ asm (" .pushsection .text, \"ax\",
> @progbits\n"
> " my_tramp:\n"
> " addi sp,sp,-24\n"
> " sd a0,0(sp)\n"
> +" move t0,t1\n"
> " sd t0,8(sp)\n"
> " sd ra,16(sp)\n"
> " call my_direct_func\n"
> ---
> arch/riscv/Kconfig | 2 ++
> samples/ftrace/ftrace-direct-modify.c | 34 ++++++++++++++++++
> samples/ftrace/ftrace-direct-multi-modify.c | 38 +++++++++++++++++++++
> samples/ftrace/ftrace-direct-multi.c | 23 +++++++++++++
> samples/ftrace/ftrace-direct-too.c | 27 +++++++++++++++
> samples/ftrace/ftrace-direct.c | 23 +++++++++++++
> 6 files changed, 147 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 2828537abfcd..0a1da03e88c2 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -279,6 +279,8 @@ config ARCH_RV64I
> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> + select HAVE_SAMPLE_FTRACE_DIRECT
> + select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
> select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> select HAVE_FUNCTION_GRAPH_TRACER
> select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
> index 39146fa83e20..6a633ff4cf6d 100644
> --- a/samples/ftrace/ftrace-direct-modify.c
> +++ b/samples/ftrace/ftrace-direct-modify.c
> @@ -22,6 +22,40 @@ extern void my_tramp2(void *);
>
> static unsigned long my_ip = (unsigned long)schedule;
>
> +#ifdef CONFIG_RISCV
> +
> +asm (" .pushsection .text, \"ax\", @progbits\n"
> +" .type my_tramp1, @function\n"
> +" .globl my_tramp1\n"
> +" my_tramp1:\n"
> +" addi sp,sp,-16\n"
> +" move t0,t1\n"
> +" sd t0,0(sp)\n"
> +" sd ra,8(sp)\n"
> +" call my_direct_func1\n"
> +" ld t0,0(sp)\n"
> +" ld ra,8(sp)\n"
> +" addi sp,sp,16\n"
> +" jr t0\n"
> +" .size my_tramp1, .-my_tramp1\n"
> +
> +" .type my_tramp2, @function\n"
> +" .globl my_tramp2\n"
> +" my_tramp2:\n"
> +" addi sp,sp,-16\n"
> +" sd t0,0(sp)\n"
> +" sd ra,8(sp)\n"
> +" call my_direct_func2\n"
> +" ld t0,0(sp)\n"
> +" ld ra,8(sp)\n"
> +" addi sp,sp,16\n"
> +" jr t0\n"
> +" .size my_tramp2, .-my_tramp2\n"
> +" .popsection\n"
> +);
> +
> +#endif /* CONFIG_RISCV */
> +
> #ifdef CONFIG_X86_64
>
> #include <asm/ibt.h>
> diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
> index 65aa94d96f4e..9534e18fa6ed 100644
> --- a/samples/ftrace/ftrace-direct-multi-modify.c
> +++ b/samples/ftrace/ftrace-direct-multi-modify.c
> @@ -20,6 +20,44 @@ void my_direct_func2(unsigned long ip)
> extern void my_tramp1(void *);
> extern void my_tramp2(void *);
>
> +#ifdef CONFIG_RISCV
> +
> +asm (" .pushsection .text, \"ax\", @progbits\n"
> +" .type my_tramp1, @function\n"
> +" .globl my_tramp1\n"
> +" my_tramp1:\n"
> +" addi sp,sp,-24\n"
> +" sd a0,0(sp)\n"
> +" move t0,t1\n"
> +" sd t0,8(sp)\n"
> +" sd ra,16(sp)\n"
> +" call my_direct_func1\n"
> +" ld a0,0(sp)\n"
> +" ld t0,8(sp)\n"
> +" ld ra,16(sp)\n"
> +" addi sp,sp,24\n"
> +" jr t0\n"
> +" .size my_tramp1, .-my_tramp1\n"
> +
> +" .type my_tramp2, @function\n"
> +" .globl my_tramp2\n"
> +" my_tramp2:\n"
> +" addi sp,sp,-24\n"
> +" sd a0,0(sp)\n"
> +" sd t0,8(sp)\n"
> +" sd ra,16(sp)\n"
> +" call my_direct_func2\n"
> +" ld a0,0(sp)\n"
> +" ld t0,8(sp)\n"
> +" ld ra,16(sp)\n"
> +" addi sp,sp,24\n"
> +" jr t0\n"
> +" .size my_tramp2, .-my_tramp2\n"
> +" .popsection\n"
> +);
> +
> +#endif /* CONFIG_RISCV */
> +
> #ifdef CONFIG_X86_64
>
> #include <asm/ibt.h>
> diff --git a/samples/ftrace/ftrace-direct-multi.c b/samples/ftrace/ftrace-direct-multi.c
> index 41ded7c615c7..2111478b0463 100644
> --- a/samples/ftrace/ftrace-direct-multi.c
> +++ b/samples/ftrace/ftrace-direct-multi.c
> @@ -15,6 +15,29 @@ void my_direct_func(unsigned long ip)
>
> extern void my_tramp(void *);
>
> +#ifdef CONFIG_RISCV
> +
> +asm (" .pushsection .text, \"ax\", @progbits\n"
> +" .type my_tramp, @function\n"
> +" .globl my_tramp\n"
> +" my_tramp:\n"
> +" addi sp,sp,-24\n"
> +" sd a0,0(sp)\n"
> +" move t0,t1\n"
> +" sd t0,8(sp)\n"
> +" sd ra,16(sp)\n"
> +" call my_direct_func\n"
> +" ld a0,0(sp)\n"
> +" ld t0,8(sp)\n"
> +" ld ra,16(sp)\n"
> +" addi sp,sp,24\n"
> +" jr t0\n"
> +" .size my_tramp, .-my_tramp\n"
> +" .popsection\n"
> +);
> +
> +#endif /* CONFIG_RISCV */
> +
> #ifdef CONFIG_X86_64
>
> #include <asm/ibt.h>
> diff --git a/samples/ftrace/ftrace-direct-too.c b/samples/ftrace/ftrace-direct-too.c
> index 6690468c5cc2..e80981d3dc0a 100644
> --- a/samples/ftrace/ftrace-direct-too.c
> +++ b/samples/ftrace/ftrace-direct-too.c
> @@ -17,6 +17,33 @@ void my_direct_func(struct vm_area_struct *vma,
>
> extern void my_tramp(void *);
>
> +#ifdef CONFIG_RISCV
> +
> +asm (" .pushsection .text, \"ax\", @progbits\n"
> +" .type my_tramp, @function\n"
> +" .globl my_tramp\n"
> +" my_tramp:\n"
> +" addi sp,sp,-40\n"
> +" sd a0,0(sp)\n"
> +" sd a1,8(sp)\n"
> +" sd a2,16(sp)\n"
> +" move t0,t1\n"
> +" sd t0,24(sp)\n"
> +" sd ra,32(sp)\n"
> +" call my_direct_func\n"
> +" ld a0,0(sp)\n"
> +" ld a1,8(sp)\n"
> +" ld a2,16(sp)\n"
> +" ld t0,24(sp)\n"
> +" ld ra,32(sp)\n"
> +" addi sp,sp,40\n"
> +" jr t0\n"
> +" .size my_tramp, .-my_tramp\n"
> +" .popsection\n"
> +);
> +
> +#endif /* CONFIG_RISCV */
> +
> #ifdef CONFIG_X86_64
>
> #include <asm/ibt.h>
> diff --git a/samples/ftrace/ftrace-direct.c b/samples/ftrace/ftrace-direct.c
> index e8f1e440b9b8..b6cae1b5cb66 100644
> --- a/samples/ftrace/ftrace-direct.c
> +++ b/samples/ftrace/ftrace-direct.c
> @@ -14,6 +14,29 @@ void my_direct_func(struct task_struct *p)
>
> extern void my_tramp(void *);
>
> +#ifdef CONFIG_RISCV
> +
> +asm (" .pushsection .text, \"ax\", @progbits\n"
> +" .type my_tramp, @function\n"
> +" .globl my_tramp\n"
> +" my_tramp:\n"
> +" addi sp,sp,-24\n"
> +" sd a0,0(sp)\n"
> +" move t0,t1\n"
> +" sd t0,8(sp)\n"
> +" sd ra,16(sp)\n"
> +" call my_direct_func\n"
> +" ld a0,0(sp)\n"
> +" ld t0,8(sp)\n"
> +" ld ra,16(sp)\n"
> +" addi sp,sp,24\n"
> +" jr t0\n"
> +" .size my_tramp, .-my_tramp\n"
> +" .popsection\n"
> +);
> +
> +#endif /* CONFIG_RISCV */
> +
> #ifdef CONFIG_X86_64
>
> #include <asm/ibt.h>
> --
> 2.36.1
>


--
Best Regards
Guo Ren