diff mbox series

[v8,7/8] LoongArch: ftrace: Enable HAVE_FUNCTION_GRAPH_RETVAL

Message ID 20230328134319.2185812-8-pengdonglin@sangfor.com.cn (mailing list archive)
State Superseded
Headers show
Series function_graph: Support recording and printing the return value of function | expand

Commit Message

pengdonglin March 28, 2023, 1:43 p.m. UTC 
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(-)

Comments

Qing Zhang March 29, 2023, 4:08 a.m. UTC | #1 
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.
> +
> +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

>   	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,
- 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)
>
pengdonglin March 29, 2023, 4:31 a.m. UTC | #2 
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)
>>
>
diff mbox series

Patch

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;
+};
+
+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
 	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
 	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)