[PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support

guoren@kernel.org posted 7 patches 2 years, 9 months ago
[PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
Posted by guoren@kernel.org 2 years, 9 months ago
From: Song Shuai <suagrfillet@gmail.com>

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 | 6 ++++++
 arch/riscv/kernel/mcount-dyn.S  | 4 ++++
 3 files changed, 11 insertions(+)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index 1d0e5838b11b..2828537abfcd 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -278,6 +278,7 @@ config ARCH_RV64I
 	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
 	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
+	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
 	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
 	select HAVE_FUNCTION_GRAPH_TRACER
 	select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 84f856a3286e..4539f10fea56 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -114,6 +114,12 @@ struct ftrace_regs;
 void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
 		       struct ftrace_ops *op, struct ftrace_regs *fregs);
 #define ftrace_graph_func ftrace_graph_func
+
+static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
+{
+		regs->t1 = addr;
+}
+
 #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
 
 #endif /* __ASSEMBLY__ */
diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
index 466c6ef217b1..fef7c460f991 100644
--- a/arch/riscv/kernel/mcount-dyn.S
+++ b/arch/riscv/kernel/mcount-dyn.S
@@ -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)
-- 
2.36.1
Re: [PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
Posted by Conor Dooley 2 years, 9 months ago
On Mon, Nov 28, 2022 at 10:32:29PM -0500, guoren@kernel.org wrote:
> From: Song Shuai <suagrfillet@gmail.com>
> 
> 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 | 6 ++++++
>  arch/riscv/kernel/mcount-dyn.S  | 4 ++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 1d0e5838b11b..2828537abfcd 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -278,6 +278,7 @@ config ARCH_RV64I
>  	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>  	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
>  	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> +	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>  	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
>  	select HAVE_FUNCTION_GRAPH_TRACER
>  	select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION

Please sort new entries here in alphabetical order, so move the new
entry up by one line :)

Thanks,
Conor.
Re: [PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
Posted by Palmer Dabbelt 2 years, 9 months ago
On Thu, 01 Dec 2022 12:02:56 PST (-0800), Conor Dooley wrote:
> On Mon, Nov 28, 2022 at 10:32:29PM -0500, guoren@kernel.org wrote:
>> From: Song Shuai <suagrfillet@gmail.com>
>>
>> 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 | 6 ++++++
>>  arch/riscv/kernel/mcount-dyn.S  | 4 ++++
>>  3 files changed, 11 insertions(+)
>>
>> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
>> index 1d0e5838b11b..2828537abfcd 100644
>> --- a/arch/riscv/Kconfig
>> +++ b/arch/riscv/Kconfig
>> @@ -278,6 +278,7 @@ config ARCH_RV64I
>>  	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>>  	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
>>  	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>> +	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>>  	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
>>  	select HAVE_FUNCTION_GRAPH_TRACER
>>  	select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
>
> Please sort new entries here in alphabetical order, so move the new
> entry up by one line :)

IIRC whomever sorted these entrties orignially posted a script that does 
that.  Maybe that should be integrated into either checkpatch or one of 
the patchwork robots so we don't have to manually remember the alphabet?
Re: [PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
Posted by Conor Dooley 2 years, 9 months ago
On Thu, Dec 01, 2022 at 12:06:39PM -0800, Palmer Dabbelt wrote:
> On Thu, 01 Dec 2022 12:02:56 PST (-0800), Conor Dooley wrote:
> > On Mon, Nov 28, 2022 at 10:32:29PM -0500, guoren@kernel.org wrote:
> > > From: Song Shuai <suagrfillet@gmail.com>
> > > 
> > > 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 | 6 ++++++
> > >  arch/riscv/kernel/mcount-dyn.S  | 4 ++++
> > >  3 files changed, 11 insertions(+)
> > > 
> > > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > > index 1d0e5838b11b..2828537abfcd 100644
> > > --- a/arch/riscv/Kconfig
> > > +++ b/arch/riscv/Kconfig
> > > @@ -278,6 +278,7 @@ config ARCH_RV64I
> > >  	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> > >  	select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> > >  	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > > +	select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> > >  	select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> > >  	select HAVE_FUNCTION_GRAPH_TRACER
> > >  	select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > 
> > Please sort new entries here in alphabetical order, so move the new
> > entry up by one line :)
> 
> IIRC whomever sorted these entrties orignially posted a script that does
> that.  Maybe that should be integrated into either checkpatch or one of the
> patchwork robots so we don't have to manually remember the alphabet?

If it actually does *sorting* I am not sure it's a suitable inclusion
as-is that'd seem better as a pre-commit hook. But maybe it can be
co-opted somehow. I'll try to dig it up off of lore tomorrow.

Thanks,
Conor.
Re: [PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
Posted by Song Shuai 2 years, 9 months ago
<guoren@kernel.org> 于2022年11月29日周二 03:33写道:
>
> From: Song Shuai <suagrfillet@gmail.com>
>
> 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 | 6 ++++++
>  arch/riscv/kernel/mcount-dyn.S  | 4 ++++
>  3 files changed, 11 insertions(+)
>
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 1d0e5838b11b..2828537abfcd 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -278,6 +278,7 @@ config ARCH_RV64I
>         select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
>         select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
I noticed this cc-option uses the '-fpatchable-function-entry=8' to
judge if the compiler supports this option.
Should we keep up with the CC_FLAGS_FTRACE modified in ("riscv:
ftrace: Reduce the detour code size to half"),
or follow the parisc architecture to set the value as '1,1' in the
case of the CC_FLAGS_FTRACE is not constant.
```
./arch/parisc/Makefile:75:CC_FLAGS_FTRACE :=
-fpatchable-function-entry=$(NOP_COUNT),$(shell echo
$$(($(NOP_COUNT)-1)))
./arch/parisc/Kconfig:70:       select HAVE_DYNAMIC_FTRACE if
$(cc-option,-fpatchable-function-entry=1,1)
```
>         select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> +       select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
>         select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
>         select HAVE_FUNCTION_GRAPH_TRACER
>         select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> index 84f856a3286e..4539f10fea56 100644
> --- a/arch/riscv/include/asm/ftrace.h
> +++ b/arch/riscv/include/asm/ftrace.h
> @@ -114,6 +114,12 @@ struct ftrace_regs;
>  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
>                        struct ftrace_ops *op, struct ftrace_regs *fregs);
>  #define ftrace_graph_func ftrace_graph_func
> +
> +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> +{
> +               regs->t1 = addr;
> +}
> +
>  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
>
>  #endif /* __ASSEMBLY__ */
> diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> index 466c6ef217b1..fef7c460f991 100644
> --- a/arch/riscv/kernel/mcount-dyn.S
> +++ b/arch/riscv/kernel/mcount-dyn.S
> @@ -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)
> --
> 2.36.1
>


-- 
Thanks,
Song
Re: [PATCH V4 6/7] riscv: ftrace: Add DYNAMIC_FTRACE_WITH_DIRECT_CALLS support
Posted by Guo Ren 2 years, 9 months ago
On Tue, Nov 29, 2022 at 8:03 PM Song Shuai <suagrfillet@gmail.com> wrote:
>
> <guoren@kernel.org> 于2022年11月29日周二 03:33写道:
> >
> > From: Song Shuai <suagrfillet@gmail.com>
> >
> > 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 | 6 ++++++
> >  arch/riscv/kernel/mcount-dyn.S  | 4 ++++
> >  3 files changed, 11 insertions(+)
> >
> > diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> > index 1d0e5838b11b..2828537abfcd 100644
> > --- a/arch/riscv/Kconfig
> > +++ b/arch/riscv/Kconfig
> > @@ -278,6 +278,7 @@ config ARCH_RV64I
> >         select ARCH_SUPPORTS_INT128 if CC_HAS_INT128
> >         select HAVE_DYNAMIC_FTRACE if !XIP_KERNEL && MMU && $(cc-option,-fpatchable-function-entry=8)
> I noticed this cc-option uses the '-fpatchable-function-entry=8' to
> judge if the compiler supports this option.
> Should we keep up with the CC_FLAGS_FTRACE modified in ("riscv:
> ftrace: Reduce the detour code size to half"),
> or follow the parisc architecture to set the value as '1,1' in the
> case of the CC_FLAGS_FTRACE is not constant.
> ```
> ./arch/parisc/Makefile:75:CC_FLAGS_FTRACE :=
> -fpatchable-function-entry=$(NOP_COUNT),$(shell echo
> $$(($(NOP_COUNT)-1)))
> ./arch/parisc/Kconfig:70:       select HAVE_DYNAMIC_FTRACE if
> $(cc-option,-fpatchable-function-entry=1,1)
The -fpatchable-function-entry=8 is still okay for riscv because it's
big enough. Your proposal could be another optimization patch, not a
fixup (No bug happens :).

> ```
> >         select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > +       select HAVE_DYNAMIC_FTRACE_WITH_DIRECT_CALLS
> >         select HAVE_FTRACE_MCOUNT_RECORD if !XIP_KERNEL
> >         select HAVE_FUNCTION_GRAPH_TRACER
> >         select HAVE_FUNCTION_TRACER if !XIP_KERNEL && !PREEMPTION
> > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > index 84f856a3286e..4539f10fea56 100644
> > --- a/arch/riscv/include/asm/ftrace.h
> > +++ b/arch/riscv/include/asm/ftrace.h
> > @@ -114,6 +114,12 @@ struct ftrace_regs;
> >  void ftrace_graph_func(unsigned long ip, unsigned long parent_ip,
> >                        struct ftrace_ops *op, struct ftrace_regs *fregs);
> >  #define ftrace_graph_func ftrace_graph_func
> > +
> > +static inline void arch_ftrace_set_direct_caller(struct pt_regs *regs, unsigned long addr)
> > +{
> > +               regs->t1 = addr;
> > +}
> > +
> >  #endif /* CONFIG_DYNAMIC_FTRACE_WITH_REGS */
> >
> >  #endif /* __ASSEMBLY__ */
> > diff --git a/arch/riscv/kernel/mcount-dyn.S b/arch/riscv/kernel/mcount-dyn.S
> > index 466c6ef217b1..fef7c460f991 100644
> > --- a/arch/riscv/kernel/mcount-dyn.S
> > +++ b/arch/riscv/kernel/mcount-dyn.S
> > @@ -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)
> > --
> > 2.36.1
> >
>
>
> --
> Thanks,
> Song



-- 
Best Regards
 Guo Ren