Messages in this thread | | | Date | Wed, 29 Mar 2023 12:31:13 +0800 | Subject | Re: [PATCH v8 7/8] LoongArch: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL | From | Donglin Peng <> |
| |
On 2023/3/29 12:08, Qing Zhang wrote: > Hi, Donglin > > On 2023/3/28 下午9:43, Donglin Peng wrote: >> The commit d4815c5d1bbd ("function_graph: Support recording and >> printing the return value of function") laid the groundwork for the >> for the funcgraph-retval, and this modification makes it available >> on the LoongArch platform. >> >> We introduce a new structure called fgraph_ret_regs for the LoongArch >> platform to hold return registers and the frame pointer. We then fill >> its content in the return_to_handler and pass its address to the >> function ftrace_return_to_handler to record the return value. >> >> Signed-off-by: Donglin Peng <pengdonglin@sangfor.com.cn> >> --- >> v8: >> - Modify the control range of CONFIG_HAVE_FUNCTION_GRAPH_RETVAL >> --- >> arch/loongarch/Kconfig | 1 + >> arch/loongarch/include/asm/ftrace.h | 18 ++++++++++++++++++ >> arch/loongarch/kernel/mcount.S | 6 ++++-- >> arch/loongarch/kernel/mcount_dyn.S | 7 ++++--- >> 4 files changed, 27 insertions(+), 5 deletions(-) >> >> diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig >> index 7fd51257e0ed..4bf60132869b 100644 >> --- a/arch/loongarch/Kconfig >> +++ b/arch/loongarch/Kconfig >> @@ -99,6 +99,7 @@ config LOONGARCH >> select HAVE_FAST_GUP >> select HAVE_FTRACE_MCOUNT_RECORD >> select HAVE_FUNCTION_ARG_ACCESS_API >> + select HAVE_FUNCTION_GRAPH_RETVAL if HAVE_FUNCTION_GRAPH_TRACER >> select HAVE_FUNCTION_GRAPH_TRACER >> select HAVE_FUNCTION_TRACER >> select HAVE_GENERIC_VDSO >> diff --git a/arch/loongarch/include/asm/ftrace.h >> b/arch/loongarch/include/asm/ftrace.h >> index 3418d32d4fc7..433c6218888b 100644 >> --- a/arch/loongarch/include/asm/ftrace.h >> +++ b/arch/loongarch/include/asm/ftrace.h >> @@ -63,4 +63,22 @@ void ftrace_graph_func(unsigned long ip, unsigned >> long parent_ip, >> #endif /* CONFIG_FUNCTION_TRACER */ >> +#ifndef __ASSEMBLY__ >> +struct fgraph_ret_regs { >> + unsigned long a0; >> + unsigned long a1; >> + unsigned long fp; >> +}; > > This overall looks good, but some places need attention: > > This will need to be padded to 16 bytes, as within the kernel, > loongarch requires the SP to be aligned to 16 bytes at all time. > Please can you add an `__unused/padding` field to ensure.
Thank you for pointing out this issue, and I will fix it.
>> + >> +static inline unsigned long fgraph_ret_regs_return_value(struct >> fgraph_ret_regs *ret_regs) >> +{ >> + return ret_regs->a0; >> +} >> + >> +static inline unsigned long fgraph_ret_regs_frame_pointer(struct >> fgraph_ret_regs *ret_regs) >> +{ >> + return ret_regs->fp; >> +} >> +#endif >> + >> #endif /* _ASM_LOONGARCH_FTRACE_H */ >> diff --git a/arch/loongarch/kernel/mcount.S >> b/arch/loongarch/kernel/mcount.S >> index 8cdc1563cd33..3e405c0212c0 100644 >> --- a/arch/loongarch/kernel/mcount.S >> +++ b/arch/loongarch/kernel/mcount.S >> @@ -79,10 +79,12 @@ SYM_FUNC_START(ftrace_graph_caller) >> SYM_FUNC_END(ftrace_graph_caller) >> SYM_FUNC_START(return_to_handler) >> - PTR_ADDI sp, sp, -2 * SZREG >> + PTR_ADDI sp, sp, -3 * SZREG > As above, this will need to be padded to keep the stack aligned to 16 > bytes. > PTR_ADDI sp, sp, -4 * SZREG
Thanks.
> >> PTR_S a0, sp, 0 >> PTR_S a1, sp, SZREG >> + PTR_S zero, sp, 2 * SZREG >> + move a0, sp >> bl ftrace_return_to_handler >> /* Restore the real parent address: a0 -> ra */ >> @@ -90,7 +92,7 @@ SYM_FUNC_START(return_to_handler) >> PTR_L a0, sp, 0 >> PTR_L a1, sp, SZREG >> - PTR_ADDI sp, sp, 2 * SZREG >> + PTR_ADDI sp, sp, 3 * SZREG >> jr ra >> SYM_FUNC_END(return_to_handler) >> #endif /* CONFIG_FUNCTION_GRAPH_TRACER */ >> diff --git a/arch/loongarch/kernel/mcount_dyn.S >> b/arch/loongarch/kernel/mcount_dyn.S >> index bbabf06244c2..ab85a953c6d3 100644 >> --- a/arch/loongarch/kernel/mcount_dyn.S >> +++ b/arch/loongarch/kernel/mcount_dyn.S >> @@ -131,18 +131,19 @@ SYM_CODE_END(ftrace_graph_caller) >> SYM_CODE_START(return_to_handler) >> /* Save return value regs */ >> - PTR_ADDI sp, sp, -2 * SZREG >> + PTR_ADDI sp, sp, -3 * SZREG > as above.
Thanks.
> > Thanks, > - Qing >> PTR_S a0, sp, 0 >> PTR_S a1, sp, SZREG >> + PTR_S zero, sp, 2 * SZREG >> - move a0, zero >> + move a0, sp >> bl ftrace_return_to_handler >> move ra, a0 >> /* Restore return value regs */ >> PTR_L a0, sp, 0 >> PTR_L a1, sp, SZREG >> - PTR_ADDI sp, sp, 2 * SZREG >> + PTR_ADDI sp, sp, 3 * SZREG >> jr ra >> SYM_CODE_END(return_to_handler) >> >
| |