* [PATCH V9 0/4] riscv: Optimize function trace
@ 2023-05-10 10:18 Song Shuai
2023-05-10 10:18 ` [PATCH V9 1/4] riscv: select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY Song Shuai
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Song Shuai @ 2023-05-10 10:18 UTC (permalink / raw)
To: paul.walmsley, palmer, aou, rostedt, mhiramat, mark.rutland,
guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
Changes in v9:
1. add Acked-by from Björn Töpel in patch 1
2. rebase patch2/patch3 on Linux v6.4-rc1
- patch 2: to make the `SAVE_ABI_REGS` configurable, revert the
modification of mcount-dyn.S from commit (45b32b946a97 "riscv:
entry: Consolidate general regs saving/restoring")
- patch 3: to pass the trace_selftest, add the implement of
`ftrace_stub_direct_tramp` from commit (fee86a4ed536 "ftrace:
selftest: remove broken trace_direct_tramp") ; and fixup the context
conflict in Kconfig
You can directly try it with:
https://github.com/sugarfillet/linux/tree/6.4-rc1-rv-ftrace-v9
This series optimizes function trace. The first 3 independent
patches has been picked in the V7 version of this series, the
subsequent version continues the following 4 patches:
select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY [1] (patch 1)
==========================================================
In RISC-V, -fpatchable-function-entry option is used to support
dynamic ftrace in this commit afc76b8b8011 ("riscv: Using
PATCHABLE_FUNCTION_ENTRY instead of MCOUNT"). So recordmcount
don't have to be called to create the __mcount_loc section before
the vmlinux linking.
Here selects FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY to tell
Makefile not to run recordmcount.
Make function graph use ftrace directly [2] (patch 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.
Add WITH_DIRECT_CALLS support [3] (patch 3, 4)
==============================================
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 series's old changes related these patches
==========================================
Changes in v8:
https://lore.kernel.org/linux-riscv/20230324033342.3177979-1-suagrfillet@gmail.com/
- Fix incorrect address values in the 4nd patch
- Rebased on v6.3-rc2
Changes in v7:
https://lore.kernel.org/linux-riscv/20230112090603.1295340-1-guoren@kernel.org/
- Fixup RESTORE_ABI_REGS by remove PT_T0(sp) overwrite.
- Add FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY [1]
- Fixup kconfig with HAVE_SAMPLE_FTRACE_DIRECT &
HAVE_SAMPLE_FTRACE_DIRECT_MULTI
Changes in v6:
https://lore.kernel.org/linux-riscv/20230107133549.4192639-1-guoren@kernel.org/
- Replace 8 with MCOUNT_INSN_SIZE
- Replace "REG_L a1, PT_RA(sp)" with "mv a1, ra"
- Add Evgenii Shatokhin comment
Changes in v5:
https://lore.kernel.org/linux-riscv/20221208091244.203407-1-guoren@kernel.org/
- Sort Kconfig entries in alphabetical order.
Changes in v4:
https://lore.kernel.org/linux-riscv/20221129033230.255947-1-guoren@kernel.org/
- Include [3] for maintenance. [Song Shuai]
Changes in V3:
https://lore.kernel.org/linux-riscv/20221123153950.2911981-1-guoren@kernel.org/
- Include [2] for maintenance. [Song Shuai]
[1]: https://lore.kernel.org/linux-riscv/CAAYs2=j3Eak9vU6xbAw0zPuoh00rh8v5C2U3fePkokZFibWs2g@mail.gmail.com/T/#t
[2]: https://lore.kernel.org/lkml/20221120084230.910152-1-suagrfillet@gmail.com/
[3]: https://lore.kernel.org/linux-riscv/20221123142025.1504030-1-suagrfillet@gmail.com/
Song Shuai (4):
riscv: select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY
riscv: ftrace: Add 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 | 4 +
arch/riscv/include/asm/ftrace.h | 19 ++-
arch/riscv/kernel/ftrace.c | 30 ++--
arch/riscv/kernel/mcount-dyn.S | 176 +++++++++++++++++---
samples/ftrace/ftrace-direct-modify.c | 33 ++++
samples/ftrace/ftrace-direct-multi-modify.c | 39 +++++
samples/ftrace/ftrace-direct-multi.c | 23 +++
samples/ftrace/ftrace-direct-too.c | 26 +++
samples/ftrace/ftrace-direct.c | 22 +++
9 files changed, 328 insertions(+), 44 deletions(-)
--
2.20.1
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V9 1/4] riscv: select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY
2023-05-10 10:18 [PATCH V9 0/4] riscv: Optimize function trace Song Shuai
@ 2023-05-10 10:18 ` Song Shuai
2023-05-10 10:18 ` [PATCH V9 2/4] riscv: ftrace: Add ftrace_graph_func Song Shuai
` (2 subsequent siblings)
3 siblings, 0 replies; 11+ messages in thread
From: Song Shuai @ 2023-05-10 10:18 UTC (permalink / raw)
To: paul.walmsley, palmer, aou, rostedt, mhiramat, mark.rutland,
guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel, Björn Töpel
In RISC-V, -fpatchable-function-entry option is used to support
dynamic ftrace in this commit afc76b8b8011 ("riscv: Using
PATCHABLE_FUNCTION_ENTRY instead of MCOUNT"). So recordmcount
don't have to be called to create the __mcount_loc section before
the vmlinux linking.
Here selects FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY to tell
Makefile not to run recordmcount.
Link: https://lore.kernel.org/linux-riscv/CAAYs2=j3Eak9vU6xbAw0zPuoh00rh8v5C2U3fePkokZFibWs2g@mail.gmail.com/T/#t
Link: https://lore.kernel.org/linux-riscv/Y4jtfrJt+%2FQ5nMOz@spud/
Signed-off-by: Song Shuai <suagrfillet@gmail.com>
Tested-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Guo Ren <guoren@kernel.org>
Acked-by: Björn Töpel <bjorn@rivosinc.com>
---
arch/riscv/Kconfig | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 348c0fa1fc8c..e0632493482f 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -58,6 +58,7 @@ config RISCV
select COMMON_CLK
select CPU_PM if CPU_IDLE || HIBERNATION
select EDAC_SUPPORT
+ select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY if DYNAMIC_FTRACE
select GENERIC_ARCH_TOPOLOGY
select GENERIC_ATOMIC64 if !64BIT
select GENERIC_CLOCKEVENTS_BROADCAST if SMP
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V9 2/4] riscv: ftrace: Add ftrace_graph_func
2023-05-10 10:18 [PATCH V9 0/4] riscv: Optimize function trace Song Shuai
2023-05-10 10:18 ` [PATCH V9 1/4] riscv: select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY Song Shuai
@ 2023-05-10 10:18 ` Song Shuai
2023-05-11 7:08 ` Björn Töpel
2023-05-10 10:18 ` [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support Song Shuai
2023-05-10 10:18 ` [PATCH V9 4/4] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI] Song Shuai
3 siblings, 1 reply; 11+ messages in thread
From: Song Shuai @ 2023-05-10 10:18 UTC (permalink / raw)
To: paul.walmsley, palmer, aou, rostedt, mhiramat, mark.rutland,
guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
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.
Signed-off-by: Song Shuai <suagrfillet@gmail.com>
Tested-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/include/asm/ftrace.h | 11 ++-
arch/riscv/kernel/ftrace.c | 30 +++---
arch/riscv/kernel/mcount-dyn.S | 166 +++++++++++++++++++++++++++-----
3 files changed, 163 insertions(+), 44 deletions(-)
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index d47d87c2d7e3..84f856a3286e 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -107,7 +107,16 @@ do { \
struct dyn_ftrace;
int ftrace_init_nop(struct module *mod, struct dyn_ftrace *rec);
#define ftrace_init_nop ftrace_init_nop
-#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 */
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 03a6434a8cdd..f5aa24d9e1c1 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -178,32 +178,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 */
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index 669b8697aa38..f26e9f6e2fed 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -57,31 +57,150 @@
.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)
+
+ // always save the ABI regs
+
+ REG_S x10, PT_A0(sp)
+ REG_S x11, PT_A1(sp)
+ REG_S x12, PT_A2(sp)
+ REG_S x13, PT_A3(sp)
+ REG_S x14, PT_A4(sp)
+ 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)
- save_from_x6_to_x31
+ 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)
+ REG_S x21, PT_S5(sp)
+ REG_S x22, PT_S6(sp)
+ REG_S x23, PT_S7(sp)
+ REG_S x24, PT_S8(sp)
+ REG_S x25, PT_S9(sp)
+ REG_S x26, PT_S10(sp)
+ REG_S x27, PT_S11(sp)
+ REG_S x28, PT_T3(sp)
+ 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 x10, PT_A0(sp)
+ REG_L x11, PT_A1(sp)
+ REG_L x12, PT_A2(sp)
+ REG_L x13, PT_A3(sp)
+ REG_L x14, PT_A4(sp)
+ 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)
- /* Restore t0 with PT_EPC */
- REG_L x5, PT_EPC(sp)
- restore_from_x6_to_x31
+ 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)
+ REG_L x21, PT_S5(sp)
+ REG_L x22, PT_S6(sp)
+ REG_L x23, PT_S7(sp)
+ REG_L x24, PT_S8(sp)
+ REG_L x25, PT_S9(sp)
+ REG_L x26, PT_S10(sp)
+ REG_L x27, PT_S11(sp)
+ REG_L x28, PT_T3(sp)
+ REG_L x29, PT_T4(sp)
+ 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
+ mv a1, ra // parent_ip
+ mv a3, sp // fregs
+ .endm
+
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS
ENTRY(ftrace_caller)
SAVE_ABI
@@ -110,33 +229,28 @@ 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)
- mv a1, ra
- 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_EPC(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
+ 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
-#endif
- RESTORE_ALL
+ RESTORE_ABI_REGS 0
jr t0
-ENDPROC(ftrace_regs_caller)
+ENDPROC(ftrace_caller)
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
2023-05-10 10:18 [PATCH V9 0/4] riscv: Optimize function trace Song Shuai
2023-05-10 10:18 ` [PATCH V9 1/4] riscv: select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY Song Shuai
2023-05-10 10:18 ` [PATCH V9 2/4] riscv: ftrace: Add ftrace_graph_func Song Shuai
@ 2023-05-10 10:18 ` Song Shuai
2023-05-11 7:19 ` Björn Töpel
2023-05-10 10:18 ` [PATCH V9 4/4] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI] Song Shuai
3 siblings, 1 reply; 11+ messages in thread
From: Song Shuai @ 2023-05-10 10:18 UTC (permalink / raw)
To: paul.walmsley, palmer, aou, rostedt, mhiramat, mark.rutland,
guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
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 <suagrfillet@gmail.com>
Tested-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/Kconfig | 1 +
arch/riscv/include/asm/ftrace.h | 8 ++++++++
arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++
3 files changed, 19 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e0632493482f..fdf0b219a02c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -144,6 +144,7 @@ config RISCV
select UACCESS_MEMCPY if !MMU
select ZONE_DMA32 if 64BIT
select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
+ 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
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 84f856a3286e..84904c1e4369 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -114,6 +114,14 @@ 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;
+}
+#define arch_ftrace_set_direct_caller(fregs, addr) \
+ __arch_ftrace_set_direct_caller(&(fregs)->regs, 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 f26e9f6e2fed..9d405baedb52 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -231,6 +231,7 @@ ENDPROC(ftrace_caller)
#else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
ENTRY(ftrace_regs_caller)
+ move t1, zero
SAVE_ABI_REGS 1
PREPARE_ARGS
@@ -239,7 +240,10 @@ ftrace_regs_call:
call ftrace_stub
RESTORE_ABI_REGS 1
+ bnez t1,.Ldirect
jr t0
+.Ldirect:
+ jr t1
ENDPROC(ftrace_regs_caller)
ENTRY(ftrace_caller)
@@ -254,3 +258,9 @@ ftrace_call:
jr t0
ENDPROC(ftrace_caller)
#endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
+SYM_CODE_START(ftrace_stub_direct_tramp)
+ jr t0
+SYM_CODE_END(ftrace_stub_direct_tramp)
+#endif /* CONFIG_DYNAMIC_FTRACE_WITH_DIRECT_CALLS */
--
2.20.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH V9 4/4] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]
2023-05-10 10:18 [PATCH V9 0/4] riscv: Optimize function trace Song Shuai
` (2 preceding siblings ...)
2023-05-10 10:18 ` [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support Song Shuai
@ 2023-05-10 10:18 ` Song Shuai
2023-05-11 7:20 ` Björn Töpel
3 siblings, 1 reply; 11+ messages in thread
From: Song Shuai @ 2023-05-10 10:18 UTC (permalink / raw)
To: paul.walmsley, palmer, aou, rostedt, mhiramat, mark.rutland,
guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
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/.
Link: https://lore.kernel.org/linux-riscv/c68bac83-5c88-80b1-bac9-e1fd4ea8f07e@yadro.com/T/#ma13012560331c66b051b580b3ab4a04ba44455ec
Tested-by: Evgenii Shatokhin <e.shatokhin@yadro.com>
Signed-off-by: Song Shuai <suagrfillet@gmail.com>
Tested-by: Guo Ren <guoren@kernel.org>
Signed-off-by: Guo Ren <guoren@kernel.org>
---
arch/riscv/Kconfig | 2 ++
samples/ftrace/ftrace-direct-modify.c | 33 +++++++++++++++++
samples/ftrace/ftrace-direct-multi-modify.c | 39 +++++++++++++++++++++
samples/ftrace/ftrace-direct-multi.c | 23 ++++++++++++
samples/ftrace/ftrace-direct-too.c | 26 ++++++++++++++
samples/ftrace/ftrace-direct.c | 22 ++++++++++++
6 files changed, 145 insertions(+)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index fdf0b219a02c..cb94ef086f0c 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -121,6 +121,8 @@ config RISCV
select HAVE_POSIX_CPU_TIMERS_TASK_WORK
select HAVE_REGS_AND_STACK_ACCESS_API
select HAVE_RSEQ
+ select HAVE_SAMPLE_FTRACE_DIRECT
+ select HAVE_SAMPLE_FTRACE_DIRECT_MULTI
select HAVE_STACKPROTECTOR
select HAVE_SYSCALL_TRACEPOINTS
select IRQ_DOMAIN
diff --git a/samples/ftrace/ftrace-direct-modify.c b/samples/ftrace/ftrace-direct-modify.c
index 06d889149012..b355e6994c5c 100644
--- a/samples/ftrace/ftrace-direct-modify.c
+++ b/samples/ftrace/ftrace-direct-modify.c
@@ -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>
diff --git a/samples/ftrace/ftrace-direct-multi-modify.c b/samples/ftrace/ftrace-direct-multi-modify.c
index 62f6b681999e..6a4ee86d1f70 100644
--- a/samples/ftrace/ftrace-direct-multi-modify.c
+++ b/samples/ftrace/ftrace-direct-multi-modify.c
@@ -20,6 +20,45 @@ 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"
+" mv a0,t0\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"
+" mv a0,t0\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 5482cf616b43..e00a33b7d3c2 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"
+" sd t0,8(sp)\n"
+" sd ra,16(sp)\n"
+" mv a0,t0\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 a05bc2cc2261..af0b90c4d6d5 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 06879bbd3399..47977e262291 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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V9 2/4] riscv: ftrace: Add ftrace_graph_func
2023-05-10 10:18 ` [PATCH V9 2/4] riscv: ftrace: Add ftrace_graph_func Song Shuai
@ 2023-05-11 7:08 ` Björn Töpel
0 siblings, 0 replies; 11+ messages in thread
From: Björn Töpel @ 2023-05-11 7:08 UTC (permalink / raw)
To: Song Shuai, paul.walmsley, palmer, aou, rostedt, mhiramat,
mark.rutland, guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
Song Shuai <suagrfillet@gmail.com> writes:
> 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.
>
> Signed-off-by: Song Shuai <suagrfillet@gmail.com>
> Tested-by: Guo Ren <guoren@kernel.org>
> Signed-off-by: Guo Ren <guoren@kernel.org>
[...]
> +
> + // always save the ABI regs
> +
> + REG_S x10, PT_A0(sp)
> + REG_S x11, PT_A1(sp)
> + REG_S x12, PT_A2(sp)
> + REG_S x13, PT_A3(sp)
> + REG_S x14, PT_A4(sp)
> + REG_S x15, PT_A5(sp)
> + REG_S x16, PT_A6(sp)
> + REG_S x17, PT_A7(sp)
Really a nit/more general comment; the RISC-V assembly files is a bit
all over the place in terms of style; When doing changes, try to
prettify it with proper tabs, and maybe we'll have eventual
consistency. ;-)
No tabs ^^^...
> +
> + // 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)
> - save_from_x6_to_x31
> + 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)
> + REG_S x21, PT_S5(sp)
> + REG_S x22, PT_S6(sp)
> + REG_S x23, PT_S7(sp)
> + REG_S x24, PT_S8(sp)
> + REG_S x25, PT_S9(sp)
> + REG_S x26, PT_S10(sp)
> + REG_S x27, PT_S11(sp)
> + REG_S x28, PT_T3(sp)
> + 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 x10, PT_A0(sp)
> + REG_L x11, PT_A1(sp)
> + REG_L x12, PT_A2(sp)
> + REG_L x13, PT_A3(sp)
> + REG_L x14, PT_A4(sp)
> + 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)
> - /* Restore t0 with PT_EPC */
> - REG_L x5, PT_EPC(sp)
> - restore_from_x6_to_x31
> + 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)
> + REG_L x21, PT_S5(sp)
> + REG_L x22, PT_S6(sp)
> + REG_L x23, PT_S7(sp)
> + REG_L x24, PT_S8(sp)
> + REG_L x25, PT_S9(sp)
> + REG_L x26, PT_S10(sp)
> + REG_L x27, PT_S11(sp)
> + REG_L x28, PT_T3(sp)
> + REG_L x29, PT_T4(sp)
> + 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
> + mv a1, ra // parent_ip
> + mv a3, sp // fregs
> + .endm
...but here...
> +
> #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>
> +#ifndef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> ENTRY(ftrace_caller)
> SAVE_ABI
>
> @@ -110,33 +229,28 @@ 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)
> - mv a1, ra
> - 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_EPC(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
> + RESTORE_ABI_REGS 1
> + jr t0
...and not here.
Not a biggie! Nice cleanup!
Acked-by: Björn Töpel <bjorn@rivosinc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
2023-05-10 10:18 ` [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support Song Shuai
@ 2023-05-11 7:19 ` Björn Töpel
2023-05-11 9:18 ` Song Shuai
2023-05-11 13:30 ` Evgenii Shatokhin
0 siblings, 2 replies; 11+ messages in thread
From: Björn Töpel @ 2023-05-11 7:19 UTC (permalink / raw)
To: Song Shuai, paul.walmsley, palmer, aou, rostedt, mhiramat,
mark.rutland, guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
Song Shuai <suagrfillet@gmail.com> writes:
> 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 <suagrfillet@gmail.com>
> Tested-by: Guo Ren <guoren@kernel.org>
> Signed-off-by: Guo Ren <guoren@kernel.org>
> ---
> arch/riscv/Kconfig | 1 +
> arch/riscv/include/asm/ftrace.h | 8 ++++++++
> arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++
> 3 files changed, 19 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index e0632493482f..fdf0b219a02c 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -144,6 +144,7 @@ config RISCV
> select UACCESS_MEMCPY if !MMU
> select ZONE_DMA32 if 64BIT
> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
> + 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
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 84f856a3286e..84904c1e4369 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -114,6 +114,14 @@ 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;
> +}
> +#define arch_ftrace_set_direct_caller(fregs, addr) \
> + __arch_ftrace_set_direct_caller(&(fregs)->regs, 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 f26e9f6e2fed..9d405baedb52 100644
> --- a/arch/riscv/kernel/mcount-dyn.S
> +++ b/arch/riscv/kernel/mcount-dyn.S
> @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller)
>
> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> ENTRY(ftrace_regs_caller)
> + move t1, zero
Please use "mv", and not "move" [1].
> SAVE_ABI_REGS 1
> PREPARE_ARGS
>
> @@ -239,7 +240,10 @@ ftrace_regs_call:
> call ftrace_stub
>
> RESTORE_ABI_REGS 1
> + bnez t1,.Ldirect
> jr t0
> +.Ldirect:
> + jr t1
Again, while you're doing changes here, please try to align op/operands.
Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support!
This does not take the WITH_CALL_OPS approach Mark suggested in the v7
threads, but given that text patching story on RISC-V is still a bit sad
(inconsistency in the RV tree, no specification, cannot work with
preempt, ...) I'd say this approach is OK for now, and we can change to
WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work.
Thoughts?
Björn
[1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V9 4/4] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI]
2023-05-10 10:18 ` [PATCH V9 4/4] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI] Song Shuai
@ 2023-05-11 7:20 ` Björn Töpel
0 siblings, 0 replies; 11+ messages in thread
From: Björn Töpel @ 2023-05-11 7:20 UTC (permalink / raw)
To: Song Shuai, paul.walmsley, palmer, aou, rostedt, mhiramat,
mark.rutland, guoren, suagrfillet, jszhang, e.shatokhin
Cc: linux-riscv, linux-kernel, linux-trace-kernel
Song Shuai <suagrfillet@gmail.com> writes:
> 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/.
>
> Link: https://lore.kernel.org/linux-riscv/c68bac83-5c88-80b1-bac9-e1fd4ea8f07e@yadro.com/T/#ma13012560331c66b051b580b3ab4a04ba44455ec
> Tested-by: Evgenii Shatokhin <e.shatokhin@yadro.com>
> Signed-off-by: Song Shuai <suagrfillet@gmail.com>
> Tested-by: Guo Ren <guoren@kernel.org>
> Signed-off-by: Guo Ren <guoren@kernel.org>
My usual "please align op/operands" nit. ;-)
Acked-by: Björn Töpel <bjorn@rivosinc.com>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
2023-05-11 7:19 ` Björn Töpel
@ 2023-05-11 9:18 ` Song Shuai
2023-05-11 13:30 ` Evgenii Shatokhin
1 sibling, 0 replies; 11+ messages in thread
From: Song Shuai @ 2023-05-11 9:18 UTC (permalink / raw)
To: Björn Töpel
Cc: paul.walmsley, palmer, aou, rostedt, mhiramat, mark.rutland,
guoren, jszhang, e.shatokhin, linux-riscv, linux-kernel,
linux-trace-kernel
Björn Töpel <bjorn@kernel.org> 于2023年5月11日周四 07:19写道:
>
> Song Shuai <suagrfillet@gmail.com> writes:
>
> > 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 <suagrfillet@gmail.com>
> > Tested-by: Guo Ren <guoren@kernel.org>
> > Signed-off-by: Guo Ren <guoren@kernel.org>
> > ---
> > arch/riscv/Kconfig | 1 +
> > arch/riscv/include/asm/ftrace.h | 8 ++++++++
> > arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++
> > 3 files changed, 19 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index e0632493482f..fdf0b219a02c 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -144,6 +144,7 @@ config RISCV
> > select UACCESS_MEMCPY if !MMU
> > select ZONE_DMA32 if 64BIT
> > select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
> > + 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
> > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > index 84f856a3286e..84904c1e4369 100644
> > --- a/arch/riscv/include/asm/ftrace.h
> > +++ b/arch/riscv/include/asm/ftrace.h
> > @@ -114,6 +114,14 @@ 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;
> > +}
> > +#define arch_ftrace_set_direct_caller(fregs, addr) \
> > + __arch_ftrace_set_direct_caller(&(fregs)->regs, 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 f26e9f6e2fed..9d405baedb52 100644
> > --- a/arch/riscv/kernel/mcount-dyn.S
> > +++ b/arch/riscv/kernel/mcount-dyn.S
> > @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller)
> >
> > #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> > ENTRY(ftrace_regs_caller)
> > + move t1, zero
>
> Please use "mv", and not "move" [1].
Will fix it up in the next version
>
> > SAVE_ABI_REGS 1
> > PREPARE_ARGS
> >
> > @@ -239,7 +240,10 @@ ftrace_regs_call:
> > call ftrace_stub
> >
> > RESTORE_ABI_REGS 1
> > + bnez t1,.Ldirect
> > jr t0
> > +.Ldirect:
> > + jr t1
>
> Again, while you're doing changes here, please try to align op/operands.
>
> Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support!
>
> This does not take the WITH_CALL_OPS approach Mark suggested in the v7
> threads, but given that text patching story on RISC-V is still a bit sad
> (inconsistency in the RV tree, no specification, cannot work with
> preempt, ...) I'd say this approach is OK for now, and we can change to
> WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work.
>
> Thoughts?
>
>
> Björn
>
> [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions
--
Thanks,
Song
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
2023-05-11 7:19 ` Björn Töpel
2023-05-11 9:18 ` Song Shuai
@ 2023-05-11 13:30 ` Evgenii Shatokhin
2023-05-21 15:22 ` Guo Ren
1 sibling, 1 reply; 11+ messages in thread
From: Evgenii Shatokhin @ 2023-05-11 13:30 UTC (permalink / raw)
To: Björn Töpel, Song Shuai
Cc: linux-riscv, linux-kernel, linux-trace-kernel, paul.walmsley,
rostedt, palmer, aou, mhiramat, mark.rutland, guoren, jszhang
Hi,
On 11.05.2023 10:19, Björn Töpel wrote:
> Song Shuai <suagrfillet@gmail.com> writes:
>
>> 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 <suagrfillet@gmail.com>
>> Tested-by: Guo Ren <guoren@kernel.org>
>> Signed-off-by: Guo Ren <guoren@kernel.org>
>> ---
>> arch/riscv/Kconfig | 1 +
>> arch/riscv/include/asm/ftrace.h | 8 ++++++++
>> arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++
>> 3 files changed, 19 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index e0632493482f..fdf0b219a02c 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -144,6 +144,7 @@ config RISCV
>> select UACCESS_MEMCPY if !MMU
>> select ZONE_DMA32 if 64BIT
>> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
>> + 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
>> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
>> index 84f856a3286e..84904c1e4369 100644
>> --- a/arch/riscv/include/asm/ftrace.h
>> +++ b/arch/riscv/include/asm/ftrace.h
>> @@ -114,6 +114,14 @@ 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;
>> +}
>> +#define arch_ftrace_set_direct_caller(fregs, addr) \
>> + __arch_ftrace_set_direct_caller(&(fregs)->regs, 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 f26e9f6e2fed..9d405baedb52 100644
>> --- a/arch/riscv/kernel/mcount-dyn.S
>> +++ b/arch/riscv/kernel/mcount-dyn.S
>> @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller)
>>
>> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>> ENTRY(ftrace_regs_caller)
>> + move t1, zero
>
> Please use "mv", and not "move" [1].
>
>> SAVE_ABI_REGS 1
>> PREPARE_ARGS
>>
>> @@ -239,7 +240,10 @@ ftrace_regs_call:
>> call ftrace_stub
>>
>> RESTORE_ABI_REGS 1
>> + bnez t1,.Ldirect
>> jr t0
>> +.Ldirect:
>> + jr t1
>
> Again, while you're doing changes here, please try to align op/operands.
>
> Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support!
>
> This does not take the WITH_CALL_OPS approach Mark suggested in the v7
> threads, but given that text patching story on RISC-V is still a bit sad
> (inconsistency in the RV tree, no specification, cannot work with
> preempt, ...) I'd say this approach is OK for now, and we can change to
> WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work.
>
> Thoughts?
The WITH_CALL_OPS approach seems to need much more time and effort, so,
yes, I'd also use this implementation of DIRECT_CALLS for now. Other
improvements could wait for the future "big patching rework".
FWIW, the implementation of kprobes for RISC-V has been improving
gradually too, not everything was done in the first very patchset, but
it was usable nonetheless.
I have not tested this particular version of this
DYNAMIC_FTRACE_WITH_DIRECT_CALLS series, only some previous ones - there
were no outstanding problems there. The code looks sane to me.
>
>
> Björn
>
> [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions
>
Regards,
Evgenii
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
2023-05-11 13:30 ` Evgenii Shatokhin
@ 2023-05-21 15:22 ` Guo Ren
0 siblings, 0 replies; 11+ messages in thread
From: Guo Ren @ 2023-05-21 15:22 UTC (permalink / raw)
To: Evgenii Shatokhin
Cc: Björn Töpel, Song Shuai, linux-riscv, linux-kernel,
linux-trace-kernel, paul.walmsley, rostedt, palmer, aou,
mhiramat, mark.rutland, jszhang
On Thu, May 11, 2023 at 9:30 PM Evgenii Shatokhin <e.shatokhin@yadro.com> wrote:
>
> Hi,
>
> On 11.05.2023 10:19, Björn Töpel wrote:
> > Song Shuai <suagrfillet@gmail.com> writes:
> >
> >> 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 <suagrfillet@gmail.com>
> >> Tested-by: Guo Ren <guoren@kernel.org>
> >> Signed-off-by: Guo Ren <guoren@kernel.org>
> >> ---
> >> arch/riscv/Kconfig | 1 +
> >> arch/riscv/include/asm/ftrace.h | 8 ++++++++
> >> arch/riscv/kernel/mcount-dyn.S | 10 ++++++++++
> >> 3 files changed, 19 insertions(+)
> >>
> >> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> >> index e0632493482f..fdf0b219a02c 100644
> >> --- a/arch/riscv/Kconfig
> >> +++ b/arch/riscv/Kconfig
> >> @@ -144,6 +144,7 @@ config RISCV
> >> select UACCESS_MEMCPY if !MMU
> >> select ZONE_DMA32 if 64BIT
> >> select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && (CLANG_SUPPORTS_DYNAMIC_FTRACE || GCC_SUPPORTS_DYNAMIC_FTRACE)
> >> + 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
> >> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> >> index 84f856a3286e..84904c1e4369 100644
> >> --- a/arch/riscv/include/asm/ftrace.h
> >> +++ b/arch/riscv/include/asm/ftrace.h
> >> @@ -114,6 +114,14 @@ 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;
> >> +}
> >> +#define arch_ftrace_set_direct_caller(fregs, addr) \
> >> + __arch_ftrace_set_direct_caller(&(fregs)->regs, 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 f26e9f6e2fed..9d405baedb52 100644
> >> --- a/arch/riscv/kernel/mcount-dyn.S
> >> +++ b/arch/riscv/kernel/mcount-dyn.S
> >> @@ -231,6 +231,7 @@ ENDPROC(ftrace_caller)
> >>
> >> #else /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >> ENTRY(ftrace_regs_caller)
> >> + move t1, zero
> >
> > Please use "mv", and not "move" [1].
> >
> >> SAVE_ABI_REGS 1
> >> PREPARE_ARGS
> >>
> >> @@ -239,7 +240,10 @@ ftrace_regs_call:
> >> call ftrace_stub
> >>
> >> RESTORE_ABI_REGS 1
> >> + bnez t1,.Ldirect
> >> jr t0
> >> +.Ldirect:
> >> + jr t1
> >
> > Again, while you're doing changes here, please try to align op/operands.
> >
> > Wearing my BPF hat, I'm happy to finally get DIRECT_CALLS support!
> >
> > This does not take the WITH_CALL_OPS approach Mark suggested in the v7
> > threads, but given that text patching story on RISC-V is still a bit sad
> > (inconsistency in the RV tree, no specification, cannot work with
> > preempt, ...) I'd say this approach is OK for now, and we can change to
> > WITH_CALL_OPS later in a wider "let's improve RISC-V textpatching" work.
> >
> > Thoughts?
>
> The WITH_CALL_OPS approach seems to need much more time and effort, so,
> yes, I'd also use this implementation of DIRECT_CALLS for now. Other
> improvements could wait for the future "big patching rework".
I agree to make DIRECT_CALLS merged first, WITH_CALL_OPS is another
"big patching rework".
>
> FWIW, the implementation of kprobes for RISC-V has been improving
> gradually too, not everything was done in the first very patchset, but
> it was usable nonetheless.
>
> I have not tested this particular version of this
> DYNAMIC_FTRACE_WITH_DIRECT_CALLS series, only some previous ones - there
> were no outstanding problems there. The code looks sane to me.
>
> >
> >
> > Björn
> >
> > [1] https://github.com/riscv-non-isa/riscv-asm-manual/blob/master/riscv-asm.md#-a-listing-of-standard-risc-v-pseudoinstructions
> >
>
> Regards,
> Evgenii
>
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2023-05-21 15:23 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-10 10:18 [PATCH V9 0/4] riscv: Optimize function trace Song Shuai
2023-05-10 10:18 ` [PATCH V9 1/4] riscv: select FTRACE_MCOUNT_USE_PATCHABLE_FUNCTION_ENTRY Song Shuai
2023-05-10 10:18 ` [PATCH V9 2/4] riscv: ftrace: Add ftrace_graph_func Song Shuai
2023-05-11 7:08 ` Björn Töpel
2023-05-10 10:18 ` [PATCH V9 3/4] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support Song Shuai
2023-05-11 7:19 ` Björn Töpel
2023-05-11 9:18 ` Song Shuai
2023-05-11 13:30 ` Evgenii Shatokhin
2023-05-21 15:22 ` Guo Ren
2023-05-10 10:18 ` [PATCH V9 4/4] samples: ftrace: Add riscv support for SAMPLE_FTRACE_DIRECT[_MULTI] Song Shuai
2023-05-11 7:20 ` Björn Töpel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).