riscv: Optimize function trace
v5 cover riscv: Optimize function trace
From: Guo Ren <[email protected]>
The previous ftrace detour implementation fc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT") contain three problems.
- The most horrible bug is preemption panic which found by Andy [1]. Let's disable preemption for ftrace first, and Andy could continue the ftrace preemption work.
- The "-fpatchable-function-entry= CFLAG" wasted code size !RISCV_ISA_C.
- The ftrace detour implementation wasted code size.
Patches 1,2,3 fixup above problems.
Patches 4,5,6,7 are the features based on reduced detour code patch, we include them in the series for test and maintenance.
You can directly try it with:
https://github.com/guoren83/linux/tree/ftrace_fixup_v5
Make function graph use ftrace directly [2]
===========================================
In RISC-V architecture, when we enable the ftrace_graph tracer on some functions, the function tracings on other functions will suffer extra graph tracing work. In essence, graph_ops isn't limited by its func_hash due to the global ftrace_graph_[regs]_call label. That should be corrected.
What inspires me is the commit 0c0593b45c9b ("x86/ftrace: Make function graph use ftrace directly") that uses graph_ops::func function to install return_hooker and makes the function called against its func_hash.
This series of patches makes function graph use ftrace directly for riscv.
If FTRACE_WITH_REGS isn't defined, ftrace_caller keeps ftrace_graph_call so that it can be replaced with the calling of prepare_ftrace_return by the enable/disable helper.
As for defining FTRACE_WITH_REGS, ftrace_caller is adjusted to save the necessary regs against the pt_regs layout, so it can reasonably call the graph_ops::func function - ftrace_graph_func. And ftrace_graph_[regs]_call
and its enable/disable helper aren't needed.
Test log:
The tests generated by CONFIG_FTRACE_STARTUP_TEST have passed in the local
qemu-system-riscv64 virt machine. The following is the log during startup.
``` Nov 15 03:07:13 stage4 kernel: Testing tracer function: PASSED Nov 15 03:07:13 stage4 kernel: Testing dynamic ftrace: PASSED Nov 15 03:07:13 stage4 kernel: Testing dynamic ftrace ops #1: Nov 15 03:07:13 stage4 kernel: (1 0 1 0 0) Nov 15 03:07:13 stage4 kernel: (1 1 2 0 0) Nov 15 03:07:13 stage4 kernel: (2 1 3 0 365) Nov 15 03:07:13 stage4 kernel: (2 2 4 0 399) Nov 15 03:07:13 stage4 kernel: (3 2 4 0 146071) Nov 15 03:07:13 stage4 kernel: (3 3 5 0 146105) PASSED Nov 15 03:07:13 stage4 kernel: Testing dynamic ftrace ops #2: Nov 15 03:07:13 stage4 kernel: (1 0 1 589 0) Nov 15 03:07:13 stage4 kernel: (1 1 2 635 0) Nov 15 03:07:13 stage4 kernel: (2 1 3 1 2) Nov 15 03:07:13 stage4 kernel: (2 2 4 125 126) Nov 15 03:07:13 stage4 kernel: (3 2 4 146001 146078) Nov 15 03:07:13 stage4 kernel: (3 3 5 146035 146112) PASSED Nov 15 03:07:13 stage4 kernel: Testing ftrace recursion: PASSED Nov 15 03:07:13 stage4 kernel: Testing ftrace recursion safe: PASSED Nov 15 03:07:13 stage4 kernel: Testing ftrace regs: PASSED Nov 15 03:07:13 stage4 kernel: Testing tracer nop: PASSED Nov 15 03:07:13 stage4 kernel: Testing tracer irqsoff: PASSED Nov 15 03:07:13 stage4 kernel: Testing tracer wakeup: Nov 15 03:07:13 stage4 kernel: sched: DL replenish lagged too much Nov 15 03:07:13 stage4 kernel: PASSED Nov 15 03:07:13 stage4 kernel: Testing tracer wakeup_rt: PASSED Nov 15 03:07:13 stage4 kernel: Testing tracer wakeup_dl: PASSED Nov 15 03:07:13 stage4 kernel: Testing tracer function_graph: PASSED ```
Add WITH_DIRECT_CALLS support [3]
=================================
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:
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.
[1] https://lpc.events/event/16/contributions/1171/
[2] https://lore.kernel.org/lkml/[email protected]/
[3] https://lore.kernel.org/linux-riscv/[email protected]/
Changes in v5:
- Sort Kconfig entries in alphabetical order.
Changes in v4:
https://lore.kernel.org/linux-riscv/[email protected]/
- Include [3] for maintenance. [Song Shuai]
Changes in V3:
https://lore.kernel.org/linux-riscv/[email protected]/
- Include [2] for maintenance. [Song Shuai]
Changes in V2:
https://lore.kernel.org/linux-riscv/[email protected]/
- Add Signed-off for preemption fixup.
Changes in V1:
https://lore.kernel.org/linux-riscv/[email protected]/
Andy Chiu (1):
riscv: ftrace: Fixup panic by disabling preemption
Guo Ren (2):
riscv: ftrace: Remove wasted nops for !RISCV_ISA_C
riscv: ftrace: Reduce the detour code size to half
Song Shuai (4):
riscv: ftrace: Add ftrace_graph_func
riscv: ftrace: Make ftrace_caller call ftrace_graph_func
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/Makefile | 6 +- arch/riscv/include/asm/ftrace.h | 69 ++++++-- arch/riscv/kernel/ftrace.c | 91 ++++------ arch/riscv/kernel/mcount-dyn.S | 181 +++++++++++++------- samples/ftrace/ftrace-direct-modify.c | 33 ++++ 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 +++ 10 files changed, 353 insertions(+), 137 deletions(-)
Signature · 1 line
2.36.1
PATCH v5 1/7 riscv: ftrace: Fixup panic by disabling preemption
From: Andy Chiu <[email protected]>
In RISCV, we must use an AUIPC + JALR pair to encode an immediate, forming a jump that jumps to an address over 4K. This may cause errors if we want to enable kernel preemption and remove dependency from patching code with stop_machine(). For example, if a task was switched out on auipc. And, if we changed the ftrace function before it was switched back, then it would jump to an address that has updated 11:0 bits mixing with previous XLEN:12 part.
p: patched area performed by dynamic ftraceftrace_prologue:
p| REG_S ra, -SZREG(sp) p| auipc ra, 0x? ------------> preempted ... change ftrace function ... p| jalr -?(ra) <------------- switched back p| REG_L ra, -SZREG(sp)
func:
xxx
ret
---
@Andy, could you give the patch a Signed-off-by? I just copy your most important comment, so the first author should be you. First, let's fix the problem caused by my previous patch, and you can continue your ftrace preemption work.
--- arch/riscv/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/Kconfig +1 -1
@@ -136,7 +136,7 @@ config RISCV select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER - select HAVE_FUNCTION_TRACER if !XIP_KERNEL + select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION config ARCH_MMAP_RND_BITS_MIN default 18 if 64BIT
Signature · 1 line
2.36.1
PATCH v5 2/7 riscv: ftrace: Remove wasted nops for !RISCV_ISA_C
In reply to [email protected]From: Guo Ren <[email protected]>
When CONFIG_RISCV_ISA_C=n, -fpatchable-function-entry=8 would generate more nops than we expect. Because it treat nop opcode as 0x00000013 instead of 0x0001.
Dump of assembler code for function dw_pcie_free_msi:
0xffffffff806fce94 <+0>: sd ra,-8(sp) 0xffffffff806fce98 <+4>: auipc ra,0xff90f 0xffffffff806fce9c <+8>: jalr -684(ra) # 0xffffffff8000bbec <ftrace_caller> 0xffffffff806fcea0 <+12>: ld ra,-8(sp) 0xffffffff806fcea4 <+16>: nop /* wasted */ 0xffffffff806fcea8 <+20>: nop /* wasted */ 0xffffffff806fceac <+24>: nop /* wasted */ 0xffffffff806fceb0 <+28>: nop /* wasted */ 0xffffffff806fceb4 <+0>: addi sp,sp,-48 0xffffffff806fceb8 <+4>: sd s0,32(sp) 0xffffffff806fcebc <+8>: sd s1,24(sp) 0xffffffff806fcec0 <+12>: sd s2,16(sp) 0xffffffff806fcec4 <+16>: sd s3,8(sp) 0xffffffff806fcec8 <+20>: sd ra,40(sp) 0xffffffff806fcecc <+24>: addi s0,sp,48
--- arch/riscv/Makefile | 4 ++++ 1 file changed, 4 insertions(+)
arch/riscv/Makefile +4 -0
@@ -11,7 +11,11 @@ LDFLAGS_vmlinux := ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY +ifeq ($(CONFIG_RISCV_ISA_C),y) CC_FLAGS_FTRACE := -fpatchable-function-entry=8 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +endif endif ifeq ($(CONFIG_CMODEL_MEDLOW),y)
Signature · 1 line
2.36.1
PATCH v5 3/7 riscv: ftrace: Reduce the detour code size to half
In reply to [email protected]From: Guo Ren <[email protected]>
Use a temporary register to reduce the size of detour code from 16 bytes to 8 bytes. The previous implementation is from afc76b8b8011 ("riscv: Using PATCHABLE_FUNCTION_ENTRY instead of MCOUNT").
Before the patch:
<func_prolog>:
0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra) 12: REG_L ra, -SZREG(sp)
(func_boddy)
After the patch:
<func_prolog>:
0: auipc t0, ?
4: jalr t0, ?(t0)
(func_boddy)
---
Changes in v4:
- Add Sob of Song Shuai
Changes in v3:
- Fixup ftrace_modify_call without "caller = rec->ip + 4". [Song Shuai]
- Optimize .macro make_call_ra & make_call_t0
--- arch/riscv/Makefile | 4 +- arch/riscv/include/asm/ftrace.h | 50 +++++++++++++++++++------ arch/riscv/kernel/ftrace.c | 65 ++++++++++----------------------- arch/riscv/kernel/mcount-dyn.S | 43 +++++++++------------- 4 files changed, 76 insertions(+), 86 deletions(-)
arch/riscv/Makefile +2 -2
@@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endif
arch/riscv/include/asm/ftrace.h +38 -12
69 lines not on this page · raw message
arch/riscv/kernel/ftrace.c +19 -46
138 lines not on this page · raw message
arch/riscv/kernel/mcount-dyn.S +17 -26
131 lines not on this page · raw message
Signature · 1 line
2.36.1
Re: [PATCH -next V5 3/7] riscv: ftrace: Reduce the detour code size to half
Hi,
Quoted 12 lines · On 08.12.2022 12:12, [email protected] wrote:
From: Guo Ren <[email protected]>
Use a temporary register to reduce the size of detour code from 16 bytes to 8 bytes. The previous implementation is from
afc76b8b8011("riscv: UsingPATCHABLE_FUNCTION_ENTRYinstead of MCOUNT").Before the patch:
<func_prolog>:0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra)
Quoted 9 lines
12:
REG_Lra, -SZREG(sp)
(func_boddy)After the patch:
<func_prolog>:
0: auipc t0, ?
4: jalr t0, ?(t0)
(func_boddy)
This patch not just reduces the size of detour code, but also fixes an important issue.
An Ftrace callback registered with FTRACE_OPS_FL_IPMODIFY flag can actually change the instruction pointer, e.g. to "replace" the given kernel function with a new one, which is needed for livepatching, etc.
In this case, the trampoline (ftrace_regs_caller) would not return to <func_prolog+12> but would rather jump to the new function. So, "REG_L ra, -SZREG(sp)" would not run and the original return address would not be restored. The kernel is likely to hang or crash as a result.
This can be easily demonstrated if one tries to "replace", say, cmdline_proc_show() with a new function with the same signature using instruction_pointer_set(&fregs->regs, new_func_addr) in the Ftrace callback.
Your patch fixes the issue, and such "function replacement" starts working, which is great.
Quoted 165 lines
Co-developed-by Song Shuai <[email protected]>Signed-off-by Song Shuai <[email protected]>Signed-off-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>---
Changes in v4:
- Add Sob of Song ShuaiChanges in v3:
- Fixupftrace_modify_callwithout "caller =rec->ip+ 4". [Song Shuai]
- Optimize .macromake_call_ra&make_call_t0--- arch/riscv/Makefile | 4 +- arch/riscv/include/asm/ftrace.h | 50 +++++++++++++++++++------ arch/riscv/kernel/ftrace.c | 65 ++++++++++----------------------- arch/riscv/kernel/mcount-dyn.S | 43 +++++++++------------- 4 files changed, 76 insertions(+), 86 deletions(-)arch/riscv/Makefile +2
-2@@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endifarch/riscv/include/asm/ftrace.h +38
-12@@ -42,6 +42,14 @@ struct dyn_arch_ftrace {* 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to * return address (original pc + 4) * +*<ftrace enable>: +* 0: auipc t0/ra, 0x? +* 4: jalr t0/ra, ?(t0/ra) +* +*<ftrace disable>: +* 0: nop +* 4: nop +* * Dynamic ftrace generates probes to call sites, so we must deal with * both auipc and jalr at the same time. */ @@ -52,25 +60,43 @@ struct dyn_arch_ftrace {#define AUIPC_OFFSET_MASK (0xfffff000)#define AUIPC_PAD (0x00001000)#define JALR_SHIFT 20 -#define JALR_BASIC (0x000080e7) -#define AUIPC_BASIC (0x00000097) +#define JALR_RA (0x000080e7) +#define AUIPC_RA (0x00000097) +#define JALR_T0 (0x000282e7) +#define AUIPC_T0 (0x00000297)#define NOP4 (0x00000013) -#define make_call (caller, callee, call) \ +#define to_jalr_t0 (offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0) + +#define to_auipc_t0 (offset) \ + ((offset & JALR_SIGN_MASK) ? \ + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_T0) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_T0)) + +#define make_call_t0 (caller, callee, call) \ do { \ - call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ - call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_t0(offset); \ + call[1] = to_jalr_t0(offset); \ } while (0) -#define to_jalr_insn (offset) \ - (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC) +#define to_jalr_ra (offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_RA) -#define to_auipc_insn (offset) \ +#define to_auipc_ra (offset) \ ((offset & JALR_SIGN_MASK) ? \ - (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \ - ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC)) + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_RA) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_RA)) + +#define make_call_ra (caller, callee, call) \ +do { \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_ra(offset); \ + call[1] = to_jalr_ra(offset); \ +} while (0)/* * Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here. arch/riscv/kernel/ftrace.c +5
-29@@ -55,12 +55,15 @@ static int ftrace_check_current_call(unsigned long hook_pos, }static int __ftrace_modify_call (unsigned long hook_pos,unsigned long target, -bool enable) +bool enable,bool ra) {unsigned int call[2 ];unsigned int nops[2 ] = {NOP4 ,NOP4 }; -make_call (hook_pos, target, call); +if (ra) +make_call_ra (hook_pos, target, call); +else +make_call_t0 (hook_pos, target, call);/* Replace the auipc-jalr pair at once. Return -EPERM on write error. */ if (patch_text_nosync@@ -70,42 +73,13 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,return 0 ; } -/* -* Put 5 instructions with 16 bytes at the front of function within -* patchable function entry nops' area. -* -* 0: REG_S ra, -SZREG(sp) -* 1: auipc ra, 0x? -* 2: jalr -?(ra) -* 3: REG_L ra, -SZREG(sp) -* -* So the opcodes is: -* 0: 0xfe113c23 (sd)/0xfe112e23 (sw) -* 1: 0x???????? -> auipc -* 2: 0x???????? -> jalr -* 3: 0xff813083 (ld)/0xffc12083 (lw) -*/ -#if __riscv_xlen ==64 -#define INSN0 0xfe113c23 -#define INSN3 0xff813083 -#elif __riscv_xlen ==32 -#define INSN0 0xfe112e23 -#define INSN3 0xffc12083 -#endif - -#define FUNC_ENTRY_SIZE 16 -#define FUNC_ENTRY_JMP 4 -int ftrace_make_call (struct dyn_ftrace *rec,unsigned long addr) { -unsigned int call[4 ] = {INSN0 ,0 ,0 ,INSN3 };
arch/riscv/kernel/ftrace.c static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, · line 82
Quoted 10 lines
arch/riscv/kernel/ftrace.c +3
-5-unsigned long target = addr; -unsigned long caller = rec->ip +FUNC_ENTRY_JMP ; +unsigned int call[2 ]; - call[1 ] =to_auipc_insn ((unsigned int )(target - caller)); - call[2 ] =to_jalr_insn ((unsigned int )(target - caller)); +make_call_t0 (rec->ip, addr, call); -if (patch_text_nosync ((void *)rec->ip, call,FUNC_ENTRY_SIZE )) +if (patch_text_nosync ((void *)rec->ip, call,8 ))
A small nit: how about using sizeof(call) or MCOUNT_INSN_SIZE here rather than 8?
Besides, adding BUILD_BUG_ON(sizeof(call) != MCOUNT_INSN_SIZE) would help catch the errors if someone changes the sizes in the future.
Quoted 199 lines
arch/riscv/kernel/ftrace.c +11
-12return -EPERM ;return 0 ;@@ -114,15 +88,14 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)int ftrace_make_nop (struct module *mod,struct dyn_ftrace *rec,unsigned long addr) { -unsigned int nops[4 ] = {NOP4 ,NOP4 ,NOP4 ,NOP4 }; +unsigned int nops[2 ] = {NOP4 ,NOP4 }; -if (patch_text_nosync ((void *)rec->ip, nops,FUNC_ENTRY_SIZE )) +if (patch_text_nosync ((void *)rec->ip, nops,MCOUNT_INSN_SIZE ))return -EPERM ;return 0 ; } -/* * This is called early on, and isn't wrapped by * ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold @@ -144,10 +117,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec)int ftrace_update_ftrace_func (ftrace_func_t func) {int ret =__ftrace_modify_call ((unsigned long )&ftrace_call, - (unsigned long )func, true); + (unsigned long )func, true, true);if (!ret) { ret =__ftrace_modify_call ((unsigned long )&ftrace_regs_call, - (unsigned long )func, true); + (unsigned long )func, true, true); }return ret;@@ -159,16 +132,16 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) {unsigned int call[2 ]; -unsigned long caller = rec->ip +FUNC_ENTRY_JMP ; +unsigned long caller = rec->ip;int ret; -make_call (caller, old_addr, call); +make_call_t0 (caller, old_addr, call); ret =ftrace_check_current_call (caller, call);if (ret)return ret; -return __ftrace_modify_call (caller, addr, true); +return __ftrace_modify_call (caller, addr, true, false); }#endif @@ -203,12 +176,12 @@ int ftrace_enable_ftrace_graph_caller(void)int ret; ret =__ftrace_modify_call ((unsigned long )&ftrace_graph_call, - (unsigned long )&prepare_ftrace_return, true); + (unsigned long )&prepare_ftrace_return, true, true);if (ret)return ret;return __ftrace_modify_call ((unsigned long )&ftrace_graph_regs_call, - (unsigned long )&prepare_ftrace_return, true); + (unsigned long )&prepare_ftrace_return, true, true); }int ftrace_disable_ftrace_graph_caller (void )@@ -216,12 +189,12 @@ int ftrace_disable_ftrace_graph_caller(void)int ret; ret =__ftrace_modify_call ((unsigned long )&ftrace_graph_call, - (unsigned long )&prepare_ftrace_return, false); + (unsigned long )&prepare_ftrace_return, false, true);if (ret)return ret;return __ftrace_modify_call ((unsigned long )&ftrace_graph_regs_call, - (unsigned long )&prepare_ftrace_return, false); + (unsigned long )&prepare_ftrace_return, false, true); }#endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ arch/riscv/kernel/mcount-dyn.S +13
-22@@ -13,8 +13,8 @@ .text -#define FENTRY_RA_OFFSET 12 -#define ABI_SIZE_ON_STACK 72 +#define FENTRY_RA_OFFSET 8 +#define ABI_SIZE_ON_STACK 80 #define ABI_A0 0 #define ABI_A1 8 #define ABI_A2 16@@ -23,10 +23,10 @@ #define ABI_A5 40 #define ABI_A6 48 #define ABI_A7 56 -#define ABI_RA 64 +#define ABI_T0 64 +#define ABI_RA 72 .macro SAVE_ABI - addi sp, sp, -SZREG addi sp, sp, -ABI_SIZE_ON_STACK REG_S a0, ABI_A0(sp)@@ -37,6 +37,7 @@ REG_S a5, ABI_A5(sp) REG_S a6, ABI_A6(sp) REG_S a7, ABI_A7(sp) + REG_S t0, ABI_T0(sp) REG_S ra, ABI_RA(sp) .endm@@ -49,24 +50,18 @@ REG_L a5, ABI_A5(sp) REG_L a6, ABI_A6(sp) REG_L a7, ABI_A7(sp) + REG_L t0, ABI_T0(sp) REG_L ra, ABI_RA(sp) addi sp, sp, ABI_SIZE_ON_STACK - addi sp, sp, SZREG .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS .macro SAVE_ALL - addi sp, sp, -SZREG addi sp, sp, -PT_SIZE_ON_STACK - REG_S x1, PT_EPC(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_L x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK + REG_S t0, PT_EPC(sp) REG_S x1, PT_RA(sp) - REG_L x1, PT_EPC(sp) - REG_S x2, PT_SP(sp) REG_S x3, PT_GP(sp) REG_S x4, PT_TP(sp)@@ -100,11 +95,8 @@ .endm .macro RESTORE_ALL + REG_L t0, PT_EPC(sp) REG_L x1, PT_RA(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_S x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK - REG_L x1, PT_EPC(sp) REG_L x2, PT_SP(sp) REG_L x3, PT_GP(sp) REG_L x4, PT_TP(sp)@@ -137,17 +129,16 @@ REG_L x31, PT_T6(sp) addi sp, sp, PT_SIZE_ON_STACK - addi sp, sp, SZREG .endm #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_caller) SAVE_ABI - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, ABI_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_call:@@ -155,8 +146,8 @@ ftrace_call: call ftrace_stub #ifdef CONFIG_FUNCTION_GRAPH_TRACER - addi a0, sp, ABI_SIZE_ON_STACK - REG_L a1, ABI_RA(sp) + addi a0, sp, ABI_RA + REG_L a1, ABI_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0@@ -166,17 +157,17 @@ ftrace_graph_call: call ftrace_stub #endif RESTORE_ABI - ret + jr t0 ENDPROC(ftrace_caller)
arch/riscv/kernel/mcount-dyn.S ftrace_graph_call: · line 170
Quoted 10 lines
arch/riscv/kernel/mcount-dyn.S +2
-2#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS ENTRY(ftrace_regs_caller) SAVE_ALL - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, PT_SIZE_ON_STACK(sp) + REG_L a1, PT_RA(sp)
ra contains the original return address at this point, so, I think, you can load it into a1 directly, like you do in ftrace_caller:
mv a1, ra
Quoted 21 lines
arch/riscv/kernel/mcount-dyn.S +2
-3mv a3, sp ftrace_regs_call:@@ -185,7 +176,7 @@ ftrace_regs_call: #ifdef CONFIG_FUNCTION_GRAPH_TRACER addi a0, sp, PT_RA - REG_L a1, PT_EPC(sp) + REG_L a1, PT_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0@@ -196,6 +187,6 @@ ftrace_graph_regs_call: #endif RESTORE_ALL - ret + jr t0 ENDPROC(ftrace_regs_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ --
Quoted 1 line
2.36.1
Regards,
Evgenii
Re: [PATCH -next V5 3/7] riscv: ftrace: Reduce the detour code size to half
Quoted 38 lines · Evgenii Shatokhin
Hi,
Quoted 12 lines · On 08.12.2022 12:12, [email protected] wrote:
From: Guo Ren <[email protected]>
Use a temporary register to reduce the size of detour code from 16 bytes to 8 bytes. The previous implementation is from
afc76b8b8011("riscv: UsingPATCHABLE_FUNCTION_ENTRYinstead of MCOUNT").Before the patch:
<func_prolog>:0: REG_S ra, -SZREG(sp) 4: auipc ra, ? 8: jalr ?(ra)Quoted 9 lines
12:
REG_Lra, -SZREG(sp)
(func_boddy)After the patch:
<func_prolog>:
0: auipc t0, ?
4: jalr t0, ?(t0)
(func_boddy)This patch not just reduces the size of detour code, but also fixes an important issue.
An Ftrace callback registered with
FTRACE_OPS_FL_IPMODIFYflag can actually change the instruction pointer, e.g. to "replace" the given kernel function with a new one, which is needed for livepatching, etc.In this case, the trampoline (
ftrace_regs_caller) would not return to <func_prolog+12> but would rather jump to the new function. So, "REG_Lra, -SZREG(sp)" would not run and the original return address would not be restored. The kernel is likely to hang or crash as a result.
Quoted 6 lines
This can be easily demonstrated if one tries to "replace", say,
cmdline_proc_show()with a new function with the same signature usinginstruction_pointer_set(&fregs->regs, new_func_addr)in the Ftrace callback.Your patch fixes the issue, and such "function replacement" starts working, which is great.
Thx for the comment and test.
Quoted 174 lines
Quoted 174 lines
Co-developed-by Song Shuai <[email protected]>Signed-off-by Song Shuai <[email protected]>Signed-off-by Guo Ren <[email protected]>Signed-off-by Guo Ren <[email protected]>---
Changes in v4:
- Add Sob of Song ShuaiChanges in v3:
- Fixupftrace_modify_callwithout "caller =rec->ip+ 4". [Song Shuai]
- Optimize .macromake_call_ra&make_call_t0--- arch/riscv/Makefile | 4 +- arch/riscv/include/asm/ftrace.h | 50 +++++++++++++++++++------ arch/riscv/kernel/ftrace.c | 65 ++++++++++----------------------- arch/riscv/kernel/mcount-dyn.S | 43 +++++++++------------- 4 files changed, 76 insertions(+), 86 deletions(-)arch/riscv/Makefile +2
-2@@ -12,9 +12,9 @@ ifeq ($(CONFIG_DYNAMIC_FTRACE),y) LDFLAGS_vmlinux := --no-relax KBUILD_CPPFLAGS += -DCC_USING_PATCHABLE_FUNCTION_ENTRY ifeq ($(CONFIG_RISCV_ISA_C),y) - CC_FLAGS_FTRACE := -fpatchable-function-entry=8 -else CC_FLAGS_FTRACE := -fpatchable-function-entry=4 +else + CC_FLAGS_FTRACE := -fpatchable-function-entry=2 endif endifarch/riscv/include/asm/ftrace.h +38
-12@@ -42,6 +42,14 @@ struct dyn_arch_ftrace {* 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to * return address (original pc + 4) * +*<ftrace enable>: +* 0: auipc t0/ra, 0x? +* 4: jalr t0/ra, ?(t0/ra) +* +*<ftrace disable>: +* 0: nop +* 4: nop +* * Dynamic ftrace generates probes to call sites, so we must deal with * both auipc and jalr at the same time. */ @@ -52,25 +60,43 @@ struct dyn_arch_ftrace {#define AUIPC_OFFSET_MASK (0xfffff000)#define AUIPC_PAD (0x00001000)#define JALR_SHIFT 20 -#define JALR_BASIC (0x000080e7) -#define AUIPC_BASIC (0x00000097) +#define JALR_RA (0x000080e7) +#define AUIPC_RA (0x00000097) +#define JALR_T0 (0x000282e7) +#define AUIPC_T0 (0x00000297)#define NOP4 (0x00000013) -#define make_call (caller, callee, call) \ +#define to_jalr_t0 (offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0) + +#define to_auipc_t0 (offset) \ + ((offset & JALR_SIGN_MASK) ? \ + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_T0) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_T0)) + +#define make_call_t0 (caller, callee, call) \ do { \ - call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ - call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \ - (unsigned long)caller)); \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_t0(offset); \ + call[1] = to_jalr_t0(offset); \ } while (0) -#define to_jalr_insn (offset) \ - (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC) +#define to_jalr_ra (offset) \ + (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_RA) -#define to_auipc_insn (offset) \ +#define to_auipc_ra (offset) \ ((offset & JALR_SIGN_MASK) ? \ - (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \ - ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC)) + (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_RA) : \ + ((offset & AUIPC_OFFSET_MASK) | AUIPC_RA)) + +#define make_call_ra (caller, callee, call) \ +do { \ + unsigned int offset = \ + (unsigned long) callee - (unsigned long) caller; \ + call[0] = to_auipc_ra(offset); \ + call[1] = to_jalr_ra(offset); \ +} while (0)/* * Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here. arch/riscv/kernel/ftrace.c +7
-33@@ -55,12 +55,15 @@ static int ftrace_check_current_call(unsigned long hook_pos, }static int __ftrace_modify_call (unsigned long hook_pos,unsigned long target, -bool enable) +bool enable,bool ra) {unsigned int call[2 ];unsigned int nops[2 ] = {NOP4 ,NOP4 }; -make_call (hook_pos, target, call); +if (ra) +make_call_ra (hook_pos, target, call); +else +make_call_t0 (hook_pos, target, call);/* Replace the auipc-jalr pair at once. Return -EPERM on write error. */ if (patch_text_nosync@@ -70,42 +73,13 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,return 0 ; } -/* -* Put 5 instructions with 16 bytes at the front of function within -* patchable function entry nops' area. -* -* 0: REG_S ra, -SZREG(sp) -* 1: auipc ra, 0x? -* 2: jalr -?(ra) -* 3: REG_L ra, -SZREG(sp) -* -* So the opcodes is: -* 0: 0xfe113c23 (sd)/0xfe112e23 (sw) -* 1: 0x???????? -> auipc -* 2: 0x???????? -> jalr -* 3: 0xff813083 (ld)/0xffc12083 (lw) -*/ -#if __riscv_xlen ==64 -#define INSN0 0xfe113c23 -#define INSN3 0xff813083 -#elif __riscv_xlen ==32 -#define INSN0 0xfe112e23 -#define INSN3 0xffc12083 -#endif - -#define FUNC_ENTRY_SIZE 16 -#define FUNC_ENTRY_JMP 4 -int ftrace_make_call (struct dyn_ftrace *rec,unsigned long addr) { -unsigned int call[4 ] = {INSN0 ,0 ,0 ,INSN3 }; -unsigned long target = addr; -unsigned long caller = rec->ip +FUNC_ENTRY_JMP ; +unsigned int call[2 ]; - call[1 ] =to_auipc_insn ((unsigned int )(target - caller)); - call[2 ] =to_jalr_insn ((unsigned int )(target - caller)); +make_call_t0 (rec->ip, addr, call);
Quoted 5 lines
arch/riscv/kernel/ftrace.c static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target, · line 82
Quoted 2 lines
arch/riscv/kernel/ftrace.c +1
-1-if (patch_text_nosync ((void *)rec->ip, call,FUNC_ENTRY_SIZE )) +if (patch_text_nosync ((void *)rec->ip, call,8 ))A small nit: how about using sizeof(call) or
MCOUNT_INSN_SIZEhere rather than 8?
Yes, I shouldn't use 8 here.
Quoted 3 lines
Besides, adding
BUILD_BUG_ON(sizeof(call) != MCOUNT_INSN_SIZE)would help catch the errors if someone changes the sizes in the future.
The call[2], nop[2], and replace[2] all need it, and it could be another enhancement patchset for the future.
Quoted 244 lines
Quoted 200 lines
arch/riscv/kernel/ftrace.c +11
-12return -EPERM ;return 0 ;@@ -114,15 +88,14 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)int ftrace_make_nop (struct module *mod,struct dyn_ftrace *rec,unsigned long addr) { -unsigned int nops[4 ] = {NOP4 ,NOP4 ,NOP4 ,NOP4 }; +unsigned int nops[2 ] = {NOP4 ,NOP4 }; -if (patch_text_nosync ((void *)rec->ip, nops,FUNC_ENTRY_SIZE )) +if (patch_text_nosync ((void *)rec->ip, nops,MCOUNT_INSN_SIZE ))return -EPERM ;return 0 ; } -/* * This is called early on, and isn't wrapped by * ftrace_arch_code_modify_{prepare,post_process}() and therefor doesn't hold @@ -144,10 +117,10 @@ int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec)int ftrace_update_ftrace_func (ftrace_func_t func) {int ret =__ftrace_modify_call ((unsigned long )&ftrace_call, - (unsigned long )func, true); + (unsigned long )func, true, true);if (!ret) { ret =__ftrace_modify_call ((unsigned long )&ftrace_regs_call, - (unsigned long )func, true); + (unsigned long )func, true, true); }return ret;@@ -159,16 +132,16 @@ int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr, unsigned long addr) {unsigned int call[2 ]; -unsigned long caller = rec->ip +FUNC_ENTRY_JMP ; +unsigned long caller = rec->ip;int ret; -make_call (caller, old_addr, call); +make_call_t0 (caller, old_addr, call); ret =ftrace_check_current_call (caller, call);if (ret)return ret; -return __ftrace_modify_call (caller, addr, true); +return __ftrace_modify_call (caller, addr, true, false); }#endif @@ -203,12 +176,12 @@ int ftrace_enable_ftrace_graph_caller(void)int ret; ret =__ftrace_modify_call ((unsigned long )&ftrace_graph_call, - (unsigned long )&prepare_ftrace_return, true); + (unsigned long )&prepare_ftrace_return, true, true);if (ret)return ret;return __ftrace_modify_call ((unsigned long )&ftrace_graph_regs_call, - (unsigned long )&prepare_ftrace_return, true); + (unsigned long )&prepare_ftrace_return, true, true); }int ftrace_disable_ftrace_graph_caller (void )@@ -216,12 +189,12 @@ int ftrace_disable_ftrace_graph_caller(void)int ret; ret =__ftrace_modify_call ((unsigned long )&ftrace_graph_call, - (unsigned long )&prepare_ftrace_return, false); + (unsigned long )&prepare_ftrace_return, false, true);if (ret)return ret;return __ftrace_modify_call ((unsigned long )&ftrace_graph_regs_call, - (unsigned long )&prepare_ftrace_return, false); + (unsigned long )&prepare_ftrace_return, false, true); }#endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ arch/riscv/kernel/mcount-dyn.S +13
-22@@ -13,8 +13,8 @@ .text -#define FENTRY_RA_OFFSET 12 -#define ABI_SIZE_ON_STACK 72 +#define FENTRY_RA_OFFSET 8 +#define ABI_SIZE_ON_STACK 80 #define ABI_A0 0 #define ABI_A1 8 #define ABI_A2 16@@ -23,10 +23,10 @@ #define ABI_A5 40 #define ABI_A6 48 #define ABI_A7 56 -#define ABI_RA 64 +#define ABI_T0 64 +#define ABI_RA 72 .macro SAVE_ABI - addi sp, sp, -SZREG addi sp, sp, -ABI_SIZE_ON_STACK REG_S a0, ABI_A0(sp)@@ -37,6 +37,7 @@ REG_S a5, ABI_A5(sp) REG_S a6, ABI_A6(sp) REG_S a7, ABI_A7(sp) + REG_S t0, ABI_T0(sp) REG_S ra, ABI_RA(sp) .endm@@ -49,24 +50,18 @@ REG_L a5, ABI_A5(sp) REG_L a6, ABI_A6(sp) REG_L a7, ABI_A7(sp) + REG_L t0, ABI_T0(sp) REG_L ra, ABI_RA(sp) addi sp, sp, ABI_SIZE_ON_STACK - addi sp, sp, SZREG .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS .macro SAVE_ALL - addi sp, sp, -SZREG addi sp, sp, -PT_SIZE_ON_STACK - REG_S x1, PT_EPC(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_L x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK + REG_S t0, PT_EPC(sp) REG_S x1, PT_RA(sp) - REG_L x1, PT_EPC(sp) - REG_S x2, PT_SP(sp) REG_S x3, PT_GP(sp) REG_S x4, PT_TP(sp)@@ -100,11 +95,8 @@ .endm .macro RESTORE_ALL + REG_L t0, PT_EPC(sp) REG_L x1, PT_RA(sp) - addi sp, sp, PT_SIZE_ON_STACK - REG_S x1, (sp) - addi sp, sp, -PT_SIZE_ON_STACK - REG_L x1, PT_EPC(sp) REG_L x2, PT_SP(sp) REG_L x3, PT_GP(sp) REG_L x4, PT_TP(sp)@@ -137,17 +129,16 @@ REG_L x31, PT_T6(sp) addi sp, sp, PT_SIZE_ON_STACK - addi sp, sp, SZREG .endm #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_caller) SAVE_ABI - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, ABI_SIZE_ON_STACK(sp) + mv a1, ra mv a3, sp ftrace_call:@@ -155,8 +146,8 @@ ftrace_call: call ftrace_stub #ifdef CONFIG_FUNCTION_GRAPH_TRACER - addi a0, sp, ABI_SIZE_ON_STACK - REG_L a1, ABI_RA(sp) + addi a0, sp, ABI_RA + REG_L a1, ABI_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0@@ -166,17 +157,17 @@ ftrace_graph_call: call ftrace_stub #endif RESTORE_ABI - ret + jr t0 ENDPROC(ftrace_caller)arch/riscv/kernel/mcount-dyn.S ftrace_graph_call: · line 170
Quoted 10 lines
arch/riscv/kernel/mcount-dyn.S +2
-2#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS ENTRY(ftrace_regs_caller) SAVE_ALL - addi a0, ra, -FENTRY_RA_OFFSET + addi a0, t0, -FENTRY_RA_OFFSET la a1, function_trace_op REG_L a2, 0(a1) - REG_L a1, PT_SIZE_ON_STACK(sp) + REG_L a1, PT_RA(sp)ra contains the original return address at this point, so, I think, you can load it into a1 directly, like you do in
ftrace_caller:mv a1, ra
Quoted 21 lines
arch/riscv/kernel/mcount-dyn.S +2
-3mv a3, sp ftrace_regs_call:@@ -185,7 +176,7 @@ ftrace_regs_call: #ifdef CONFIG_FUNCTION_GRAPH_TRACER addi a0, sp, PT_RA - REG_L a1, PT_EPC(sp) + REG_L a1, PT_T0(sp) addi a1, a1, -FENTRY_RA_OFFSET #ifdef HAVE_FUNCTION_GRAPH_FP_TEST mv a2, s0@@ -196,6 +187,6 @@ ftrace_graph_regs_call: #endif RESTORE_ALL - ret + jr t0 ENDPROC(ftrace_regs_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ --Quoted 1 line
2.36.1
Regards,
Evgenii
Signature · 2 lines
Best Regards Guo Ren
PATCH v5 4/7 riscv: ftrace: Add ftrace_graph_func
In reply to [email protected]From: Song Shuai <[email protected]>
Here implements ftrace_graph_func as the function graph tracing function with FTRACE_WITH_REGS defined.
function_graph_func gets the point of the parent IP and the frame pointer from fregs and call prepare_ftrace_return for function graph tracing.
If FTRACE_WITH_REGS isn't defined, the enable/disable helpers of ftrace_graph_[regs]_call are revised for serving only ftrace_graph_call in the !FTRACE_WITH_REGS version ftrace_caller.
--- arch/riscv/include/asm/ftrace.h | 13 +++++++++++-- arch/riscv/kernel/ftrace.c | 30 +++++++++++++----------------- 2 files changed, 24 insertions(+), 19 deletions(-)
arch/riscv/include/asm/ftrace.h +11 -2
@@ -107,8 +107,17 @@ do { \struct dyn_ftrace ;int ftrace_init_nop (struct module *mod,struct dyn_ftrace *rec);#define ftrace_init_nop ftrace_init_nop -#endif -#endif +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS +struct ftrace_ops ; +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 +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ + +#endif /* __ASSEMBLY__ */ + +#endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* _ASM_RISCV_FTRACE_H */
arch/riscv/kernel/ftrace.c +13 -17
@@ -169,32 +169,28 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr, }#ifdef CONFIG_DYNAMIC_FTRACE +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS +void ftrace_graph_func (unsigned long ip,unsigned long parent_ip, +struct ftrace_ops *op,struct ftrace_regs *fregs) +{ +struct pt_regs *regs =arch_ftrace_get_regs (fregs); +unsigned long *parent = (unsigned long *)®s->ra; + +prepare_ftrace_return (parent, ip,frame_pointer (regs)); +} +#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ extern void ftrace_graph_call (void ); -extern void ftrace_graph_regs_call (void );int ftrace_enable_ftrace_graph_caller (void ) { -int ret; - - ret =__ftrace_modify_call ((unsigned long )&ftrace_graph_call, - (unsigned long )&prepare_ftrace_return, true, true); -if (ret) -return ret; - -return __ftrace_modify_call ((unsigned long )&ftrace_graph_regs_call, +return __ftrace_modify_call ((unsigned long )&ftrace_graph_call, (unsigned long )&prepare_ftrace_return, true, true); }int ftrace_disable_ftrace_graph_caller (void ) { -int ret; - - ret =__ftrace_modify_call ((unsigned long )&ftrace_graph_call, - (unsigned long )&prepare_ftrace_return, false, true); -if (ret) -return ret; - -return __ftrace_modify_call ((unsigned long )&ftrace_graph_regs_call, +return __ftrace_modify_call ((unsigned long )&ftrace_graph_call, (unsigned long )&prepare_ftrace_return, false, true); } +#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ #endif /* CONFIG_DYNAMIC_FTRACE */ #endif /* CONFIG_FUNCTION_GRAPH_TRACER */
Signature · 1 line
2.36.1
PATCH v5 5/7 riscv: ftrace: Make ftrace_caller call ftrace_graph_func
In reply to [email protected]From: Song Shuai <[email protected]>
In order to make the function graph use ftrace directly, ftrace_caller should be adjusted to save the necessary regs against the pt_regs layout so it can call ftrace_graph_func reasonably.
SAVE_ALL now saves all the regs according to the pt_regs struct. Here supersedes SAVE_ALL by SAVE_ABI_REGS which has an extra option to allow saving only the necessary ABI-related regs for ftrace_caller.
ftrace_caller and ftrace_regs_caller save their regs with the respective option of SAVE_ABI_REGS, then call the tracing function, especially graph_ops's ftrace_graph_func. So the ftrace_graph_[regs]_call labels aren't needed anymore if FTRACE_WITH_REGS is defined.
As the previous patch described, the ftrace_caller remains with its ftrace_graph_call if FTRACE_WITH_REGS isn't defined,
For convenience, the original argument setup for the tracing function in ftrace_[regs]_caller is separated as PREPARE_ARGS.
--- arch/riscv/kernel/mcount-dyn.S | 142 ++++++++++++++++++++++++--------- 1 file changed, 104 insertions(+), 38 deletions(-)
arch/riscv/kernel/mcount-dyn.S +104 -38
@@ -57,19 +57,52 @@ .endm #ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS - .macro SAVE_ALL + +/** +* SAVE_ABI_REGS - save regs against the pt_regs struct +* +* @all: tell if saving all the regs +* +* If all is set, all the regs will be saved, otherwise only ABI +* related regs (a0-a7,epc,ra and optional s0) will be saved. +* +* After the stack is established, +* +* 0(sp) stores the PC of the traced function which can be accessed +* by &(fregs)->regs->epc in tracing function. Note that the real +* function entry address should be computed with -FENTRY_RA_OFFSET. +* +* 8(sp) stores the function return address (i.e. parent IP) that +* can be accessed by &(fregs)->regs->ra in tracing function. +* +* The other regs are saved at the respective localtion and accessed +* by the respective pt_regs member. +* +* Here is the layout of stack for your reference. +* +* PT_SIZE_ON_STACK -> +++++++++ +* + ..... + +* + t3-t6 + +* + s2-s11+ +* + a0-a7 + --++++-> ftrace_caller saved +* + s1 + + +* + s0 + --+ +* + t0-t2 + + +* + tp + + +* + gp + + +* + sp + + +* + ra + --+ // parent IP +* sp -> + epc + --+ // PC +* +++++++++ +**/ + .macro SAVE_ABI_REGS, all=0 addi sp, sp, -PT_SIZE_ON_STACK REG_S t0, PT_EPC(sp) REG_S x1, PT_RA(sp) - REG_S x2, PT_SP(sp) - REG_S x3, PT_GP(sp) - REG_S x4, PT_TP(sp) - REG_S x5, PT_T0(sp) - REG_S x6, PT_T1(sp) - REG_S x7, PT_T2(sp) - REG_S x8, PT_S0(sp) - REG_S x9, PT_S1(sp) + + // always save the ABI regs + REG_S x10, PT_A0(sp) REG_S x11, PT_A1(sp) REG_S x12, PT_A2(sp)
@@ -78,6 +111,18 @@ REG_S x15, PT_A5(sp) REG_S x16, PT_A6(sp) REG_S x17, PT_A7(sp) + + // save the leftover regs + + .if \all == 1 + REG_S x2, PT_SP(sp) + REG_S x3, PT_GP(sp) + REG_S x4, PT_TP(sp) + REG_S x5, PT_T0(sp) + REG_S x6, PT_T1(sp) + REG_S x7, PT_T2(sp) + REG_S x8, PT_S0(sp) + REG_S x9, PT_S1(sp) REG_S x18, PT_S2(sp) REG_S x19, PT_S3(sp) REG_S x20, PT_S4(sp)
@@ -92,19 +137,19 @@ REG_S x29, PT_T4(sp) REG_S x30, PT_T5(sp) REG_S x31, PT_T6(sp) + + // save s0 if FP_TEST defined + + .else +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST + REG_S x8, PT_S0(sp) +#endif + .endif .endm - .macro RESTORE_ALL + .macro RESTORE_ABI_REGS, all=0 REG_L t0, PT_EPC(sp) REG_L x1, PT_RA(sp) - REG_L x2, PT_SP(sp) - REG_L x3, PT_GP(sp) - REG_L x4, PT_TP(sp) - REG_L x5, PT_T0(sp) - REG_L x6, PT_T1(sp) - REG_L x7, PT_T2(sp) - REG_L x8, PT_S0(sp) - REG_L x9, PT_S1(sp) REG_L x10, PT_A0(sp) REG_L x11, PT_A1(sp) REG_L x12, PT_A2(sp)
@@ -113,6 +158,16 @@ REG_L x15, PT_A5(sp) REG_L x16, PT_A6(sp) REG_L x17, PT_A7(sp) + + .if \all == 1 + REG_L x2, PT_SP(sp) + REG_L x3, PT_GP(sp) + REG_L x4, PT_TP(sp) + REG_L x5, PT_T0(sp) + REG_L x6, PT_T1(sp) + REG_L x7, PT_T2(sp) + REG_L x8, PT_S0(sp) + REG_L x9, PT_S1(sp) REG_L x18, PT_S2(sp) REG_L x19, PT_S3(sp) REG_L x20, PT_S4(sp)
@@ -128,10 +183,25 @@ REG_L x30, PT_T5(sp) REG_L x31, PT_T6(sp) + .else +#ifdef HAVE_FUNCTION_GRAPH_FP_TEST + REG_L x8, PT_S0(sp) +#endif + .endif addi sp, sp, PT_SIZE_ON_STACK .endm + + .macro PREPARE_ARGS + addi a0, t0, -FENTRY_RA_OFFSET // ip + la a1, function_trace_op + REG_L a2, 0(a1) // op + REG_L a1, PT_RA(sp) // parent_ip + mv a3, sp // fregs + .endm + #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ +#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS ENTRY(ftrace_caller) SAVE_ABI
@@ -160,33 +230,29 @@ ftrace_graph_call: jr t0 ENDPROC(ftrace_caller) -#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS +#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_regs_caller) - SAVE_ALL - - addi a0, t0, -FENTRY_RA_OFFSET - la a1, function_trace_op - REG_L a2, 0(a1) - REG_L a1, PT_RA(sp) - mv a3, sp + SAVE_ABI_REGS 1 + PREPARE_ARGS ftrace_regs_call: .global ftrace_regs_call call ftrace_stub -#ifdef CONFIG_FUNCTION_GRAPH_TRACER - addi a0, sp, PT_RA - REG_L a1, PT_T0(sp) - addi a1, a1, -FENTRY_RA_OFFSET -#ifdef HAVE_FUNCTION_GRAPH_FP_TEST - mv a2, s0 -#endif -ftrace_graph_regs_call: - .global ftrace_graph_regs_call - call ftrace_stub -#endif - RESTORE_ALL + RESTORE_ABI_REGS 1 jr t0 ENDPROC(ftrace_regs_caller) + +ENTRY(ftrace_caller) + SAVE_ABI_REGS 0 + PREPARE_ARGS + +ftrace_call: + .global ftrace_call + call ftrace_stub + + RESTORE_ABI_REGS 0 + jr t0 +ENDPROC(ftrace_caller) #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
Signature · 1 line
2.36.1
PATCH v5 6/7 riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
In reply to [email protected]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.
--- arch/riscv/Kconfig | 1 + arch/riscv/include/asm/ftrace.h | 6 ++++++ arch/riscv/kernel/mcount-dyn.S | 4 ++++ 3 files changed, 11 insertions(+)
arch/riscv/Kconfig +1 -0
@@ -133,6 +133,7 @@ config RISCV select UACCESS_MEMCPY if !MMU select ZONE_DMA32 if 64BIT select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8) + select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL select HAVE_FUNCTION_GRAPH_TRACER
arch/riscv/include/asm/ftrace.h +6 -0
@@ -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__ */
arch/riscv/kernel/mcount-dyn.S +4 -0
@@ -232,6 +232,7 @@ ENDPROC(ftrace_caller) #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */ ENTRY(ftrace_regs_caller) + move t1, zero SAVE_ABI_REGS 1 PREPARE_ARGS
@@ -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)
Signature · 1 line
2.36.1
PATCH v5 7/7 samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]
In reply to [email protected]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/.
--- samples/ftrace/ftrace-direct-modify.c | 33 ++++++++++++++++++ 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 ++++++++++++ 5 files changed, 140 insertions(+)
samples/ftrace/ftrace-direct-modify.c +33 -0
@@ -22,6 +22,39 @@ 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 #include <asm/ibt.h>
samples/ftrace/ftrace-direct-multi-modify.c +37 -0
@@ -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>
samples/ftrace/ftrace-direct-multi.c +22 -0
@@ -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>
samples/ftrace/ftrace-direct-too.c +26 -0
@@ -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>
samples/ftrace/ftrace-direct.c +22 -0
@@ -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>
Signature · 1 line
2.36.1