[PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>

From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>

The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
operation to specify if registers need to saved/restored by the ftrace handler.
This is needed by kgraft and possibly other ftrace-based tools, and the ARM
architecture is currently lacking this feature. It would also be the first step
to support the "Kprobes-on-ftrace" optimization on ARM.

This patch introduces a new ftrace handler that stores the registers on the
stack before calling the next stage. The registers are restored from the stack
before going back to the instrumented function.

A side-effect of this patch is to activate the support for ftrace_modify_call()
as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
 arch/arm/Kconfig               |  2 ++
 arch/arm/include/asm/ftrace.h  |  4 +++
 arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
 4 files changed, 117 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529f..87f1a9f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -50,6 +50,7 @@ config ARM
 	select HAVE_DMA_API_DEBUG
 	select HAVE_DMA_CONTIGUOUS if MMU
 	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
 	select HAVE_EXIT_THREAD
 	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
@@ -90,6 +91,7 @@ config ARM
 	select PERF_USE_VMALLOC
 	select RTC_LIB
 	select SYS_SUPPORTS_APM_EMULATION
+	select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
 	# Above selects are sorted alphabetically; please add new ones
 	# according to that.  Thanks.
 	help
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index bfe2a2f..f434ce9 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -1,6 +1,10 @@
 #ifndef _ASM_ARM_FTRACE
 #define _ASM_ARM_FTRACE
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR		((unsigned long)(__gnu_mcount_nc))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index c73c403..fd75bae 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -92,12 +92,73 @@
 2:	mcount_exit
 .endm
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+.macro __ftrace_regs_caller
+
+	add 	ip, sp, #4	@ move in IP the value of SP as it was
+				@ before the push {lr} of the mcount mechanism
+	stmdb	sp!, {ip,lr,pc}
+	stmdb	sp!, {r0-r11,lr}
+
+	@ stack content at this point:
+	@ 0  4          44    48   52       56   60   64
+	@ RO | R1 | ... | R11 | LR | SP + 4 | LR | PC | previous LR |
+
+	mov r3, sp				@ struct pt_regs*
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+	ldr	r1, [sp, #64]			@ lr of instrumented func
+	mcount_adjust_addr	r0, lr		@ instrumented function
+
+	.globl ftrace_regs_call
+ftrace_regs_call:
+	bl	ftrace_stub
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	.globl ftrace_graph_regs_call
+ftrace_graph_regs_call:
+	mov	r0, r0
+#endif
+	ldr	lr, [sp, #64]		@ get the previous LR value from stack
+	ldmia	sp, {r0-r11, ip, sp}	@ pop the saved registers INCLUDING
+					@ the stack pointer
+	ret	ip
+.endm
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.macro __ftrace_graph_regs_caller
+
+	sub	r0, fp, #4			@ lr of instrumented routine (parent)
+
+	@ called from __ftrace_regs_caller
+	ldr	r1, [sp, #56]			@ instrumented routine (func)
+	mcount_adjust_addr	r1, r1
+
+	mov	r2, fp				@ frame pointer
+	bl	prepare_ftrace_return
+
+	ldr	lr, [fp, #-4]			@ restore lr from the stack
+	ldmia	sp, {r0-r11, ip, sp}		@ restore r0 through sp
+	ret	ip
+.endm
+#endif
+#endif
+
 .macro __ftrace_caller suffix
 	mcount_enter
 
 	mcount_get_lr	r1			@ lr of instrumented func
 	mcount_adjust_addr	r0, lr		@ instrumented function
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+	mov r3, #0				@ regs is NULL
+#endif
+
 	.globl ftrace_call\suffix
 ftrace_call\suffix:
 	bl	ftrace_stub
@@ -212,6 +273,15 @@ UNWIND(.fnstart)
 	__ftrace_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_regs_caller)
+#endif
+
 #endif
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -220,6 +290,14 @@ UNWIND(.fnstart)
 	__ftrace_graph_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_graph_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_graph_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_graph_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_graph_regs_caller)
+#endif
 #endif
 
 .purgem mcount_enter
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 3f17594..d8d4753 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -139,6 +139,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 
 	ret = ftrace_modify_code(pc, 0, new, false);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret) {
+		pc = (unsigned long)&ftrace_regs_call;
+		new = ftrace_call_replace(pc, (unsigned long)func);
+
+		ret = ftrace_modify_code(pc, 0, new, false);
+	}
+#endif
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
@@ -157,6 +166,20 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace(rec);
+
+	new = ftrace_call_replace(ip, adjust_address(rec, addr));
+
+	return ftrace_modify_code(rec->ip, old, new, true);
+}
+
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+				unsigned long addr)
+{
+	unsigned long new, old;
+	unsigned long ip = rec->ip;
+
+	old = ftrace_call_replace(ip, adjust_address(rec, old_addr));
+
 	new = ftrace_call_replace(ip, adjust_address(rec, addr));
 
 	return ftrace_modify_code(rec->ip, old, new, true);
@@ -229,6 +252,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 extern unsigned long ftrace_graph_call;
 extern unsigned long ftrace_graph_call_old;
 extern void ftrace_graph_caller_old(void);
+extern unsigned long ftrace_graph_regs_call;
+extern void ftrace_graph_regs_caller(void);
 
 static int __ftrace_modify_caller(unsigned long *callsite,
 				  void (*func) (void), bool enable)
@@ -251,6 +276,14 @@ static int ftrace_modify_graph_caller(bool enable)
 				     ftrace_graph_caller,
 				     enable);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret)
+		ret = __ftrace_modify_caller(&ftrace_graph_regs_call,
+				     ftrace_graph_regs_caller,
+				     enable);
+#endif
+
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret)
 		ret = __ftrace_modify_caller(&ftrace_graph_call_old,
-- 
2.7.4

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
> From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> 
> From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>

>From statement twice in the commit message. Will resend.
> 
> The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
> operation to specify if registers need to saved/restored by the ftrace handler.
> This is needed by kgraft and possibly other ftrace-based tools, and the ARM
> architecture is currently lacking this feature. It would also be the first step
> to support the "Kprobes-on-ftrace" optimization on ARM.
> 
> This patch introduces a new ftrace handler that stores the registers on the
> stack before calling the next stage. The registers are restored from the stack
> before going back to the instrumented function.
> 
> A side-effect of this patch is to activate the support for ftrace_modify_call()
> as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
> 
> Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> Signed-off-by: Abel Vesa <abelvesa@linux.com>
> ---
>  arch/arm/Kconfig               |  2 ++
>  arch/arm/include/asm/ftrace.h  |  4 +++
>  arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
>  arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
>  4 files changed, 117 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index b5d529f..87f1a9f 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -50,6 +50,7 @@ config ARM
>  	select HAVE_DMA_API_DEBUG
>  	select HAVE_DMA_CONTIGUOUS if MMU
>  	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
> +	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>  	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
>  	select HAVE_EXIT_THREAD
>  	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> @@ -90,6 +91,7 @@ config ARM
>  	select PERF_USE_VMALLOC
>  	select RTC_LIB
>  	select SYS_SUPPORTS_APM_EMULATION
> +	select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
>  	# Above selects are sorted alphabetically; please add new ones
>  	# according to that.  Thanks.
>  	help
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index bfe2a2f..f434ce9 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -1,6 +1,10 @@
>  #ifndef _ASM_ARM_FTRACE
>  #define _ASM_ARM_FTRACE
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +#define ARCH_SUPPORTS_FTRACE_OPS 1
> +#endif
> +
>  #ifdef CONFIG_FUNCTION_TRACER
>  #define MCOUNT_ADDR		((unsigned long)(__gnu_mcount_nc))
>  #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index c73c403..fd75bae 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -92,12 +92,73 @@
>  2:	mcount_exit
>  .endm
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +
> +.macro __ftrace_regs_caller
> +
> +	add 	ip, sp, #4	@ move in IP the value of SP as it was
> +				@ before the push {lr} of the mcount mechanism
> +	stmdb	sp!, {ip,lr,pc}
> +	stmdb	sp!, {r0-r11,lr}
> +
> +	@ stack content at this point:
> +	@ 0  4          44    48   52       56   60   64
> +	@ RO | R1 | ... | R11 | LR | SP + 4 | LR | PC | previous LR |
> +
> +	mov r3, sp				@ struct pt_regs*
> +	ldr r2, =function_trace_op
> +	ldr r2, [r2]				@ pointer to the current
> +						@ function tracing op
> +	ldr	r1, [sp, #64]			@ lr of instrumented func
> +	mcount_adjust_addr	r0, lr		@ instrumented function
> +
> +	.globl ftrace_regs_call
> +ftrace_regs_call:
> +	bl	ftrace_stub
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +	.globl ftrace_graph_regs_call
> +ftrace_graph_regs_call:
> +	mov	r0, r0
> +#endif
> +	ldr	lr, [sp, #64]		@ get the previous LR value from stack
> +	ldmia	sp, {r0-r11, ip, sp}	@ pop the saved registers INCLUDING
> +					@ the stack pointer
> +	ret	ip
> +.endm
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +.macro __ftrace_graph_regs_caller
> +
> +	sub	r0, fp, #4			@ lr of instrumented routine (parent)
> +
> +	@ called from __ftrace_regs_caller
> +	ldr	r1, [sp, #56]			@ instrumented routine (func)
> +	mcount_adjust_addr	r1, r1
> +
> +	mov	r2, fp				@ frame pointer
> +	bl	prepare_ftrace_return
> +
> +	ldr	lr, [fp, #-4]			@ restore lr from the stack
> +	ldmia	sp, {r0-r11, ip, sp}		@ restore r0 through sp
> +	ret	ip
> +.endm
> +#endif
> +#endif
> +
>  .macro __ftrace_caller suffix
>  	mcount_enter
>  
>  	mcount_get_lr	r1			@ lr of instrumented func
>  	mcount_adjust_addr	r0, lr		@ instrumented function
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +	ldr r2, =function_trace_op
> +	ldr r2, [r2]				@ pointer to the current
> +						@ function tracing op
> +	mov r3, #0				@ regs is NULL
> +#endif
> +
>  	.globl ftrace_call\suffix
>  ftrace_call\suffix:
>  	bl	ftrace_stub
> @@ -212,6 +273,15 @@ UNWIND(.fnstart)
>  	__ftrace_caller
>  UNWIND(.fnend)
>  ENDPROC(ftrace_caller)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +ENTRY(ftrace_regs_caller)
> +UNWIND(.fnstart)
> +	__ftrace_regs_caller
> +UNWIND(.fnend)
> +ENDPROC(ftrace_regs_caller)
> +#endif
> +
>  #endif
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> @@ -220,6 +290,14 @@ UNWIND(.fnstart)
>  	__ftrace_graph_caller
>  UNWIND(.fnend)
>  ENDPROC(ftrace_graph_caller)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +ENTRY(ftrace_graph_regs_caller)
> +UNWIND(.fnstart)
> +	__ftrace_graph_regs_caller
> +UNWIND(.fnend)
> +ENDPROC(ftrace_graph_regs_caller)
> +#endif
>  #endif
>  
>  .purgem mcount_enter
> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> index 3f17594..d8d4753 100644
> --- a/arch/arm/kernel/ftrace.c
> +++ b/arch/arm/kernel/ftrace.c
> @@ -139,6 +139,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
>  
>  	ret = ftrace_modify_code(pc, 0, new, false);
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +	if (!ret) {
> +		pc = (unsigned long)&ftrace_regs_call;
> +		new = ftrace_call_replace(pc, (unsigned long)func);
> +
> +		ret = ftrace_modify_code(pc, 0, new, false);
> +	}
> +#endif
> +
>  #ifdef CONFIG_OLD_MCOUNT
>  	if (!ret) {
>  		pc = (unsigned long)&ftrace_call_old;
> @@ -157,6 +166,20 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  	unsigned long ip = rec->ip;
>  
>  	old = ftrace_nop_replace(rec);
> +
> +	new = ftrace_call_replace(ip, adjust_address(rec, addr));
> +
> +	return ftrace_modify_code(rec->ip, old, new, true);
> +}
> +
> +int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
> +				unsigned long addr)
> +{
> +	unsigned long new, old;
> +	unsigned long ip = rec->ip;
> +
> +	old = ftrace_call_replace(ip, adjust_address(rec, old_addr));
> +
>  	new = ftrace_call_replace(ip, adjust_address(rec, addr));
>  
>  	return ftrace_modify_code(rec->ip, old, new, true);
> @@ -229,6 +252,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
>  extern unsigned long ftrace_graph_call;
>  extern unsigned long ftrace_graph_call_old;
>  extern void ftrace_graph_caller_old(void);
> +extern unsigned long ftrace_graph_regs_call;
> +extern void ftrace_graph_regs_caller(void);
>  
>  static int __ftrace_modify_caller(unsigned long *callsite,
>  				  void (*func) (void), bool enable)
> @@ -251,6 +276,14 @@ static int ftrace_modify_graph_caller(bool enable)
>  				     ftrace_graph_caller,
>  				     enable);
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +	if (!ret)
> +		ret = __ftrace_modify_caller(&ftrace_graph_regs_call,
> +				     ftrace_graph_regs_caller,
> +				     enable);
> +#endif
> +
> +
>  #ifdef CONFIG_OLD_MCOUNT
>  	if (!ret)
>  		ret = __ftrace_modify_caller(&ftrace_graph_call_old,
> -- 
> 2.7.4
> 

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Petr Mladek]

On Thu 2016-12-08 22:46:55, Abel Vesa wrote:
> On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
> > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > 
> > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> 
> >From statement twice in the commit message. Will resend.
> > 
> > The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
> > operation to specify if registers need to saved/restored by the ftrace handler.
> > This is needed by kgraft and possibly other ftrace-based tools, and the ARM
> > architecture is currently lacking this feature. It would also be the first step
> > to support the "Kprobes-on-ftrace" optimization on ARM.
> > 
> > This patch introduces a new ftrace handler that stores the registers on the
> > stack before calling the next stage. The registers are restored from the stack
> > before going back to the instrumented function.
> > 
> > A side-effect of this patch is to activate the support for ftrace_modify_call()
> > as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
> > 
> > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > Signed-off-by: Abel Vesa <abelvesa@linux.com>
> > ---
> >  arch/arm/Kconfig               |  2 ++
> >  arch/arm/include/asm/ftrace.h  |  4 +++
> >  arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
> >  arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
> >  4 files changed, 117 insertions(+)
> > 
> > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > index b5d529f..87f1a9f 100644
> > --- a/arch/arm/Kconfig
> > +++ b/arch/arm/Kconfig
> > @@ -50,6 +50,7 @@ config ARM
> >  	select HAVE_DMA_API_DEBUG
> >  	select HAVE_DMA_CONTIGUOUS if MMU
> >  	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
> > +	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> >  	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
> >  	select HAVE_EXIT_THREAD
> >  	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> > @@ -90,6 +91,7 @@ config ARM
> >  	select PERF_USE_VMALLOC
> >  	select RTC_LIB
> >  	select SYS_SUPPORTS_APM_EMULATION
> > +	select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER

FRAME_POINTER is not for free. It takes space on the stack. Also there
is a performance penalty. Do we really need to depend on it? If so,
it might be worth a note in the commit message.

I made only a quick look at the patch. It looks reasonable. But I do
not have enough knowledge about the arm architecture, assembly, and
ftrace-specifics. Also I cannot test it easily. So issues might
be hidden to my eyes.

Best Regards,
Petr

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

On Tue, Jan 10, 2017 at 04:51:12PM +0100, Petr Mladek wrote:
> On Thu 2016-12-08 22:46:55, Abel Vesa wrote:
> > On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > > 
> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > 
> > >From statement twice in the commit message. Will resend.
> > > 
> > > The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
> > > operation to specify if registers need to saved/restored by the ftrace handler.
> > > This is needed by kgraft and possibly other ftrace-based tools, and the ARM
> > > architecture is currently lacking this feature. It would also be the first step
> > > to support the "Kprobes-on-ftrace" optimization on ARM.
> > > 
> > > This patch introduces a new ftrace handler that stores the registers on the
> > > stack before calling the next stage. The registers are restored from the stack
> > > before going back to the instrumented function.
> > > 
> > > A side-effect of this patch is to activate the support for ftrace_modify_call()
> > > as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
> > > 
> > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
> > > Signed-off-by: Abel Vesa <abelvesa@linux.com>
> > > ---
> > >  arch/arm/Kconfig               |  2 ++
> > >  arch/arm/include/asm/ftrace.h  |  4 +++
> > >  arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
> > >  arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
> > >  4 files changed, 117 insertions(+)
> > > 
> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> > > index b5d529f..87f1a9f 100644
> > > --- a/arch/arm/Kconfig
> > > +++ b/arch/arm/Kconfig
> > > @@ -50,6 +50,7 @@ config ARM
> > >  	select HAVE_DMA_API_DEBUG
> > >  	select HAVE_DMA_CONTIGUOUS if MMU
> > >  	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
> > > +	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
> > >  	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
> > >  	select HAVE_EXIT_THREAD
> > >  	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> > > @@ -90,6 +91,7 @@ config ARM
> > >  	select PERF_USE_VMALLOC
> > >  	select RTC_LIB
> > >  	select SYS_SUPPORTS_APM_EMULATION
> > > +	select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
Hi Petr,
> 
> FRAME_POINTER is not for free. It takes space on the stack. Also there
> is a performance penalty. Do we really need to depend on it? If so,
> it might be worth a note in the commit message.

I was trying to create my own patch when I found this work done by 
Jean-Jacques, so I haven't looked specifically for the FRAME_POINTER 
part. I looked now at it and you seem to be right, FRAME_POINTER is 
not needed. 

I will get rid of the FRAME_POINTER part, change the authorship and
send it again in the following days.
>
> I made only a quick look at the patch. It looks reasonable. But I do
> not have enough knowledge about the arm architecture, assembly, and
> ftrace-specifics. Also I cannot test it easily. So issues might
> be hidden to my eyes.
> 
> Best Regards,
> Petr
Thanks,
Abel

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Jean-Jacques Hiblot]

2017-01-12 1:19 GMT+01:00 Abel Vesa <abelvesa@gmail.com>:
> On Tue, Jan 10, 2017 at 04:51:12PM +0100, Petr Mladek wrote:
>> On Thu 2016-12-08 22:46:55, Abel Vesa wrote:
>> > On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
>> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> > >
>> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> >
>> > >From statement twice in the commit message. Will resend.
>> > >
>> > > The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
>> > > operation to specify if registers need to saved/restored by the ftrace handler.
>> > > This is needed by kgraft and possibly other ftrace-based tools, and the ARM
>> > > architecture is currently lacking this feature. It would also be the first step
>> > > to support the "Kprobes-on-ftrace" optimization on ARM.
>> > >
>> > > This patch introduces a new ftrace handler that stores the registers on the
>> > > stack before calling the next stage. The registers are restored from the stack
>> > > before going back to the instrumented function.
>> > >
>> > > A side-effect of this patch is to activate the support for ftrace_modify_call()
>> > > as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
>> > >
>> > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>> > > Signed-off-by: Abel Vesa <abelvesa@linux.com>
>> > > ---
>> > >  arch/arm/Kconfig               |  2 ++
>> > >  arch/arm/include/asm/ftrace.h  |  4 +++
>> > >  arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
>> > >  arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
>> > >  4 files changed, 117 insertions(+)
>> > >
>> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>> > > index b5d529f..87f1a9f 100644
>> > > --- a/arch/arm/Kconfig
>> > > +++ b/arch/arm/Kconfig
>> > > @@ -50,6 +50,7 @@ config ARM
>> > >   select HAVE_DMA_API_DEBUG
>> > >   select HAVE_DMA_CONTIGUOUS if MMU
>> > >   select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
>> > > + select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>> > >   select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
>> > >   select HAVE_EXIT_THREAD
>> > >   select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>> > > @@ -90,6 +91,7 @@ config ARM
>> > >   select PERF_USE_VMALLOC
>> > >   select RTC_LIB
>> > >   select SYS_SUPPORTS_APM_EMULATION
>> > > + select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
> Hi Petr,
>>
>> FRAME_POINTER is not for free. It takes space on the stack. Also there
>> is a performance penalty. Do we really need to depend on it? If so,
>> it might be worth a note in the commit message.
>

FRAME_POINTER is not needed. the dependency is wrong and should be removed.
The code must be modified to not use fp register:

--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -130,7 +130,8 @@ ftrace_graph_regs_call:
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
 .macro __ftrace_graph_regs_caller

-       sub     r0, fp, #4                      @ lr of instrumented
routine (parent)
+       add     r0, sp, #64             @ r0 is now a pointer to lr of
+                                       @ instrumented routine

        @ called from __ftrace_regs_caller
        ldr     r1, [sp, #56]                   @ instrumented routine (func)
@@ -139,8 +140,9 @@ ftrace_graph_regs_call:
        mov     r2, fp                          @ frame pointer
        bl      prepare_ftrace_return

-       ldr     lr, [fp, #-4]                   @ restore lr from the stack
-       ldmia   sp, {r0-r11, ip, sp}            @ restore r0 through sp
+       ldr     lr, [sp, #64]           @ get the previous LR value from stack
+       ldmia   sp, {r0-r11, ip, sp}    @ pop the saved registers INCLUDING
+                                       @ the stack pointer
        ret     ip
 .endm
 #endif


Jean-Jacques


> I was trying to create my own patch when I found this work done by
> Jean-Jacques, so I haven't looked specifically for the FRAME_POINTER
> part. I looked now at it and you seem to be right, FRAME_POINTER is
> not needed.
>
> I will get rid of the FRAME_POINTER part, change the authorship and
> send it again in the following days.
>>
>> I made only a quick look at the patch. It looks reasonable. But I do
>> not have enough knowledge about the arm architecture, assembly, and
>> ftrace-specifics. Also I cannot test it easily. So issues might
>> be hidden to my eyes.
>>
>> Best Regards,
>> Petr
> Thanks,
> Abel

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Jean-Jacques Hiblot]

2017-01-12 15:30 GMT+01:00 Jean-Jacques Hiblot <jjhiblot@traphandler.com>:
> 2017-01-12 1:19 GMT+01:00 Abel Vesa <abelvesa@gmail.com>:
>> On Tue, Jan 10, 2017 at 04:51:12PM +0100, Petr Mladek wrote:
>>> On Thu 2016-12-08 22:46:55, Abel Vesa wrote:
>>> > On Thu, Dec 08, 2016 at 09:46:35PM +0000, Abel Vesa wrote:
>>> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> > >
>>> > > From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> >
>>> > >From statement twice in the commit message. Will resend.
>>> > >
>>> > > The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
>>> > > operation to specify if registers need to saved/restored by the ftrace handler.
>>> > > This is needed by kgraft and possibly other ftrace-based tools, and the ARM
>>> > > architecture is currently lacking this feature. It would also be the first step
>>> > > to support the "Kprobes-on-ftrace" optimization on ARM.
>>> > >
>>> > > This patch introduces a new ftrace handler that stores the registers on the
>>> > > stack before calling the next stage. The registers are restored from the stack
>>> > > before going back to the instrumented function.
>>> > >
>>> > > A side-effect of this patch is to activate the support for ftrace_modify_call()
>>> > > as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture
>>> > >
>>> > > Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
>>> > > Signed-off-by: Abel Vesa <abelvesa@linux.com>
>>> > > ---
>>> > >  arch/arm/Kconfig               |  2 ++
>>> > >  arch/arm/include/asm/ftrace.h  |  4 +++
>>> > >  arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
>>> > >  arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
>>> > >  4 files changed, 117 insertions(+)
>>> > >
>>> > > diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
>>> > > index b5d529f..87f1a9f 100644
>>> > > --- a/arch/arm/Kconfig
>>> > > +++ b/arch/arm/Kconfig
>>> > > @@ -50,6 +50,7 @@ config ARM
>>> > >   select HAVE_DMA_API_DEBUG
>>> > >   select HAVE_DMA_CONTIGUOUS if MMU
>>> > >   select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
>>> > > + select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>>> > >   select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
>>> > >   select HAVE_EXIT_THREAD
>>> > >   select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
>>> > > @@ -90,6 +91,7 @@ config ARM
>>> > >   select PERF_USE_VMALLOC
>>> > >   select RTC_LIB
>>> > >   select SYS_SUPPORTS_APM_EMULATION
>>> > > + select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
>> Hi Petr,
>>>
>>> FRAME_POINTER is not for free. It takes space on the stack. Also there
>>> is a performance penalty. Do we really need to depend on it? If so,
>>> it might be worth a note in the commit message.
>>
>
> FRAME_POINTER is not needed. the dependency is wrong and should be removed.
> The code must be modified to not use fp register:
>
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -130,7 +130,8 @@ ftrace_graph_regs_call:
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  .macro __ftrace_graph_regs_caller
>
> -       sub     r0, fp, #4                      @ lr of instrumented
> routine (parent)
> +       add     r0, sp, #64             @ r0 is now a pointer to lr of
> +                                       @ instrumented routine

I made some tests after sending this email. And it turns out that it
doesn't work if we change  "sub     r0, fp, #4" to "add     r0, sp,
#64 " here.
So it looks like there is a dependency on FRAME_POINTER after all.
Note that the same is true for  __ftrace_graph_caller. I don't know if
the 'graph' feature of ftrace requires intrinsically FRAME_POINTER but
it looks like it currently does on ARM (with or without register
saving)
I'll try to spend some time on the subject next week.

>
>         @ called from __ftrace_regs_caller
>         ldr     r1, [sp, #56]                   @ instrumented routine (func)
> @@ -139,8 +140,9 @@ ftrace_graph_regs_call:
>         mov     r2, fp                          @ frame pointer
>         bl      prepare_ftrace_return
>
> -       ldr     lr, [fp, #-4]                   @ restore lr from the stack
> -       ldmia   sp, {r0-r11, ip, sp}            @ restore r0 through sp
> +       ldr     lr, [sp, #64]           @ get the previous LR value from stack
> +       ldmia   sp, {r0-r11, ip, sp}    @ pop the saved registers INCLUDING
> +                                       @ the stack pointer
>         ret     ip
>  .endm
>  #endif
>
>
> Jean-Jacques
>
>
>> I was trying to create my own patch when I found this work done by
>> Jean-Jacques, so I haven't looked specifically for the FRAME_POINTER
>> part. I looked now at it and you seem to be right, FRAME_POINTER is
>> not needed.
>>
>> I will get rid of the FRAME_POINTER part, change the authorship and
>> send it again in the following days.
>>>
>>> I made only a quick look at the patch. It looks reasonable. But I do
>>> not have enough knowledge about the arm architecture, assembly, and
>>> ftrace-specifics. Also I cannot test it easily. So issues might
>>> be hidden to my eyes.
>>>
>>> Best Regards,
>>> Petr
>> Thanks,
>> Abel

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

Hi Jean-Jacques,

Here is the implementation I've made for ftrace_graph_regs_caller:

.macro __ftrace_graph_regs_caller

       sub     r0, fp, #4                          @ lr of
instrumented routine (parent)

       @ called from __ftrace_regs_caller
       ldr     r1, [sp, #56]                       @ instrumented routine (func)
       mcount_adjust_addr      r1, r1

       sub     r2, r0, #4                          @ frame pointer
       bl      prepare_ftrace_return

       ldr     lr, [sp, #64]                        @ restore lr from the stack
       ldmia   sp, {r0-r11, ip, sp}            @ restore r0 through sp
       ret     ip

.endm

AFAIK, you can still use fp (see the implementation without regs) since
it is an alternative name for r11. The FRAME_POINTER config options does
something else entirely and has nothing to do with what we need here.

I tested it but unfortunately, my current setup is with qemu and I don't have
a real hardware here close to test it properly. Could you please tell me if this
works on your setup?

Also the way I tested it was to comment out the code that uses the default
ftrace_graph_caller in ftrace_modify_graph_caller and enforced the usage of
the one with regs.

I will create a proper patch later today (I need to cleanup some stuff
first) and
send it.

Thanks,
Abel

[PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

From: Jean-Jacques Hiblot <jjhiblot@traphandler.com>

The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
operation to specify if registers need to saved/restored by the ftrace handler.
This is needed by kgraft and possibly other ftrace-based tools, and the ARM
architecture is currently lacking this feature. It would also be the first step
to support the "Kprobes-on-ftrace" optimization on ARM.

This patch introduces a new ftrace handler that stores the registers on the
stack before calling the next stage. The registers are restored from the stack
before going back to the instrumented function.

A side-effect of this patch is to activate the support for ftrace_modify_call()
as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
Signed-off-by: Abel Vesa <abelvesa@linux.com>
---
 arch/arm/Kconfig               |  2 ++
 arch/arm/include/asm/ftrace.h  |  4 +++
 arch/arm/kernel/entry-ftrace.S | 78 ++++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/ftrace.c       | 33 ++++++++++++++++++
 4 files changed, 117 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index b5d529f..87f1a9f 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -50,6 +50,7 @@ config ARM
 	select HAVE_DMA_API_DEBUG
 	select HAVE_DMA_CONTIGUOUS if MMU
 	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
 	select HAVE_EXIT_THREAD
 	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
@@ -90,6 +91,7 @@ config ARM
 	select PERF_USE_VMALLOC
 	select RTC_LIB
 	select SYS_SUPPORTS_APM_EMULATION
+	select FRAME_POINTER if DYNAMIC_FTRACE_WITH_REGS && FUNCTION_GRAPH_TRACER
 	# Above selects are sorted alphabetically; please add new ones
 	# according to that.  Thanks.
 	help
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index bfe2a2f..f434ce9 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -1,6 +1,10 @@
 #ifndef _ASM_ARM_FTRACE
 #define _ASM_ARM_FTRACE
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR		((unsigned long)(__gnu_mcount_nc))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index c73c403..fd75bae 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -92,12 +92,73 @@
 2:	mcount_exit
 .endm
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+.macro __ftrace_regs_caller
+
+	add 	ip, sp, #4	@ move in IP the value of SP as it was
+				@ before the push {lr} of the mcount mechanism
+	stmdb	sp!, {ip,lr,pc}
+	stmdb	sp!, {r0-r11,lr}
+
+	@ stack content at this point:
+	@ 0  4          44    48   52       56   60   64
+	@ RO | R1 | ... | R11 | LR | SP + 4 | LR | PC | previous LR |
+
+	mov r3, sp				@ struct pt_regs*
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+	ldr	r1, [sp, #64]			@ lr of instrumented func
+	mcount_adjust_addr	r0, lr		@ instrumented function
+
+	.globl ftrace_regs_call
+ftrace_regs_call:
+	bl	ftrace_stub
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	.globl ftrace_graph_regs_call
+ftrace_graph_regs_call:
+	mov	r0, r0
+#endif
+	ldr	lr, [sp, #64]		@ get the previous LR value from stack
+	ldmia	sp, {r0-r11, ip, sp}	@ pop the saved registers INCLUDING
+					@ the stack pointer
+	ret	ip
+.endm
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.macro __ftrace_graph_regs_caller
+
+	sub	r0, fp, #4			@ lr of instrumented routine (parent)
+
+	@ called from __ftrace_regs_caller
+	ldr	r1, [sp, #56]			@ instrumented routine (func)
+	mcount_adjust_addr	r1, r1
+
+	mov	r2, fp				@ frame pointer
+	bl	prepare_ftrace_return
+
+	ldr	lr, [fp, #-4]			@ restore lr from the stack
+	ldmia	sp, {r0-r11, ip, sp}		@ restore r0 through sp
+	ret	ip
+.endm
+#endif
+#endif
+
 .macro __ftrace_caller suffix
 	mcount_enter
 
 	mcount_get_lr	r1			@ lr of instrumented func
 	mcount_adjust_addr	r0, lr		@ instrumented function
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+	mov r3, #0				@ regs is NULL
+#endif
+
 	.globl ftrace_call\suffix
 ftrace_call\suffix:
 	bl	ftrace_stub
@@ -212,6 +273,15 @@ UNWIND(.fnstart)
 	__ftrace_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_regs_caller)
+#endif
+
 #endif
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -220,6 +290,14 @@ UNWIND(.fnstart)
 	__ftrace_graph_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_graph_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_graph_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_graph_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_graph_regs_caller)
+#endif
 #endif
 
 .purgem mcount_enter
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 3f17594..d8d4753 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -139,6 +139,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 
 	ret = ftrace_modify_code(pc, 0, new, false);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret) {
+		pc = (unsigned long)&ftrace_regs_call;
+		new = ftrace_call_replace(pc, (unsigned long)func);
+
+		ret = ftrace_modify_code(pc, 0, new, false);
+	}
+#endif
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
@@ -157,6 +166,20 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace(rec);
+
+	new = ftrace_call_replace(ip, adjust_address(rec, addr));
+
+	return ftrace_modify_code(rec->ip, old, new, true);
+}
+
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+				unsigned long addr)
+{
+	unsigned long new, old;
+	unsigned long ip = rec->ip;
+
+	old = ftrace_call_replace(ip, adjust_address(rec, old_addr));
+
 	new = ftrace_call_replace(ip, adjust_address(rec, addr));
 
 	return ftrace_modify_code(rec->ip, old, new, true);
@@ -229,6 +252,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 extern unsigned long ftrace_graph_call;
 extern unsigned long ftrace_graph_call_old;
 extern void ftrace_graph_caller_old(void);
+extern unsigned long ftrace_graph_regs_call;
+extern void ftrace_graph_regs_caller(void);
 
 static int __ftrace_modify_caller(unsigned long *callsite,
 				  void (*func) (void), bool enable)
@@ -251,6 +276,14 @@ static int ftrace_modify_graph_caller(bool enable)
 				     ftrace_graph_caller,
 				     enable);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret)
+		ret = __ftrace_modify_caller(&ftrace_graph_regs_call,
+				     ftrace_graph_regs_caller,
+				     enable);
+#endif
+
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret)
 		ret = __ftrace_modify_caller(&ftrace_graph_call_old,
-- 
2.7.4

[PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[abelvesa]

From: Abel Vesa <abelvesa@gmail.com>

The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
operation to specify if registers need to saved/restored by the ftrace handler.
This is needed by kgraft and possibly other ftrace-based tools, and the ARM
architecture is currently lacking this feature. It would also be the first step
to support the "Kprobes-on-ftrace" optimization on ARM.

This patch introduces a new ftrace handler that stores the registers on the
stack before calling the next stage. The registers are restored from the stack
before going back to the instrumented function.

A side-effect of this patch is to activate the support for ftrace_modify_call()
as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture.

Signed-off-by: Abel Vesa <abelvesa@gmail.com>
---
 arch/arm/Kconfig               |   1 +
 arch/arm/include/asm/ftrace.h  |   4 ++
 arch/arm/kernel/entry-ftrace.S | 100 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/ftrace.c       |  37 +++++++++++++++
 4 files changed, 142 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 4c1a35f..730d456 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -56,6 +56,7 @@ config ARM
 	select HAVE_DMA_API_DEBUG
 	select HAVE_DMA_CONTIGUOUS if MMU
 	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
 	select HAVE_EXIT_THREAD
 	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index 22b7311..f379881 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -1,6 +1,10 @@
 #ifndef _ASM_ARM_FTRACE
 #define _ASM_ARM_FTRACE
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR		((unsigned long)(__gnu_mcount_nc))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index c73c403..efcd9f2 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -92,12 +92,95 @@
 2:	mcount_exit
 .endm
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+.macro __ftrace_regs_caller
+
+	sub	sp, sp, #8	@ space for PC and CPSR OLD_R0,
+				@ OLD_R0 will overwrite previous LR
+
+	add 	ip, sp, #12	@ move in IP the value of SP as it was
+				@ before the push {lr} of the mcount mechanism
+
+	str     lr, [sp, #0]    @ store LR instead of PC
+
+	ldr     lr, [sp, #8]    @ get previous LR
+
+	str	r0, [sp, #8]	@ write r0 as OLD_R0 over previous LR
+
+	stmdb   sp!, {ip, lr}
+	stmdb   sp!, {r0-r11, lr}
+
+	@ stack content at this point:
+	@ 0  4          48   52       56            60   64    68       72
+	@ R0 | R1 | ... | LR | SP + 4 | previous LR | LR | PSR | OLD_R0 |
+
+	mov r3, sp				@ struct pt_regs*
+
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+
+	ldr	r1, [sp, #S_LR]			@ lr of instrumented func
+
+	ldr	lr, [sp, #S_PC]			@ get LR
+
+	mcount_adjust_addr	r0, lr		@ instrumented function
+
+	.globl ftrace_regs_call
+ftrace_regs_call:
+	bl	ftrace_stub
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	.globl ftrace_graph_regs_call
+ftrace_graph_regs_call:
+	mov	r0, r0
+#endif
+
+	@ pop saved regs
+	ldmia   sp!, {r0-r12}			@ restore r0 through r12
+	ldr	ip, [sp, #8]			@ restore PC
+	ldr	lr, [sp, #4]			@ restore LR
+	ldr	sp, [sp, #0]			@ restore SP
+	mov	pc, ip				@ return
+.endm
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.macro __ftrace_graph_regs_caller
+
+	sub     r0, fp, #4              @ lr of instrumented routine (parent)
+
+	@ called from __ftrace_regs_caller
+	ldr     r1, [sp, #S_PC]		@ instrumented routine (func)
+	mcount_adjust_addr	r1, r1
+
+	mov	r2, fp			@ frame pointer
+	bl	prepare_ftrace_return
+
+	@ pop registers saved in ftrace_regs_caller
+	ldmia   sp!, {r0-r12}			@ restore r0 through r12
+	ldr	ip, [sp, #8]			@ restore PC
+	ldr	lr, [sp, #4]			@ restore LR
+	ldr	sp, [sp, #0]			@ restore SP
+	mov	pc, ip				@ return
+
+.endm
+#endif
+#endif
+
 .macro __ftrace_caller suffix
 	mcount_enter
 
 	mcount_get_lr	r1			@ lr of instrumented func
 	mcount_adjust_addr	r0, lr		@ instrumented function
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+	mov r3, #0				@ regs is NULL
+#endif
+
 	.globl ftrace_call\suffix
 ftrace_call\suffix:
 	bl	ftrace_stub
@@ -212,6 +295,15 @@ UNWIND(.fnstart)
 	__ftrace_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_regs_caller)
+#endif
+
 #endif
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -220,6 +312,14 @@ UNWIND(.fnstart)
 	__ftrace_graph_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_graph_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_graph_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_graph_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_graph_regs_caller)
+#endif
 #endif
 
 .purgem mcount_enter
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 833c991..5617932 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -141,6 +141,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 
 	ret = ftrace_modify_code(pc, 0, new, false);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret) {
+		pc = (unsigned long)&ftrace_regs_call;
+		new = ftrace_call_replace(pc, (unsigned long)func);
+
+		ret = ftrace_modify_code(pc, 0, new, false);
+	}
+#endif
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
@@ -159,11 +168,29 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace(rec);
+
+	new = ftrace_call_replace(ip, adjust_address(rec, addr));
+
+	return ftrace_modify_code(rec->ip, old, new, true);
+}
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+				unsigned long addr)
+{
+	unsigned long new, old;
+	unsigned long ip = rec->ip;
+
+	old = ftrace_call_replace(ip, adjust_address(rec, old_addr));
+
 	new = ftrace_call_replace(ip, adjust_address(rec, addr));
 
 	return ftrace_modify_code(rec->ip, old, new, true);
 }
 
+#endif
+
 int ftrace_make_nop(struct module *mod,
 		    struct dyn_ftrace *rec, unsigned long addr)
 {
@@ -231,6 +258,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 extern unsigned long ftrace_graph_call;
 extern unsigned long ftrace_graph_call_old;
 extern void ftrace_graph_caller_old(void);
+extern unsigned long ftrace_graph_regs_call;
+extern void ftrace_graph_regs_caller(void);
 
 static int __ftrace_modify_caller(unsigned long *callsite,
 				  void (*func) (void), bool enable)
@@ -253,6 +282,14 @@ static int ftrace_modify_graph_caller(bool enable)
 				     ftrace_graph_caller,
 				     enable);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret)
+		ret = __ftrace_modify_caller(&ftrace_graph_regs_call,
+				     ftrace_graph_regs_caller,
+				     enable);
+#endif
+
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret)
 		ret = __ftrace_modify_caller(&ftrace_graph_call_old,
-- 
2.7.4

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

On Fri, May 12, 2017 at 09:38:37PM +0100, abelvesa@gmail.com wrote:
> From: Abel Vesa <abelvesa@gmail.com>
To be ignored, wrong git config. Will send another one without the From line.
> 
> The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
> operation to specify if registers need to saved/restored by the ftrace handler.
> This is needed by kgraft and possibly other ftrace-based tools, and the ARM
> architecture is currently lacking this feature. It would also be the first step
> to support the "Kprobes-on-ftrace" optimization on ARM.
> 
> This patch introduces a new ftrace handler that stores the registers on the
> stack before calling the next stage. The registers are restored from the stack
> before going back to the instrumented function.
> 
> A side-effect of this patch is to activate the support for ftrace_modify_call()
> as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture.
> 
> Signed-off-by: Abel Vesa <abelvesa@gmail.com>
> ---
>  arch/arm/Kconfig               |   1 +
>  arch/arm/include/asm/ftrace.h  |   4 ++
>  arch/arm/kernel/entry-ftrace.S | 100 +++++++++++++++++++++++++++++++++++++++++
>  arch/arm/kernel/ftrace.c       |  37 +++++++++++++++
>  4 files changed, 142 insertions(+)
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 4c1a35f..730d456 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -56,6 +56,7 @@ config ARM
>  	select HAVE_DMA_API_DEBUG
>  	select HAVE_DMA_CONTIGUOUS if MMU
>  	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
> +	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
>  	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
>  	select HAVE_EXIT_THREAD
>  	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
> diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
> index 22b7311..f379881 100644
> --- a/arch/arm/include/asm/ftrace.h
> +++ b/arch/arm/include/asm/ftrace.h
> @@ -1,6 +1,10 @@
>  #ifndef _ASM_ARM_FTRACE
>  #define _ASM_ARM_FTRACE
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +#define ARCH_SUPPORTS_FTRACE_OPS 1
> +#endif
> +
>  #ifdef CONFIG_FUNCTION_TRACER
>  #define MCOUNT_ADDR		((unsigned long)(__gnu_mcount_nc))
>  #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
> diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
> index c73c403..efcd9f2 100644
> --- a/arch/arm/kernel/entry-ftrace.S
> +++ b/arch/arm/kernel/entry-ftrace.S
> @@ -92,12 +92,95 @@
>  2:	mcount_exit
>  .endm
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +
> +.macro __ftrace_regs_caller
> +
> +	sub	sp, sp, #8	@ space for PC and CPSR OLD_R0,
> +				@ OLD_R0 will overwrite previous LR
> +
> +	add 	ip, sp, #12	@ move in IP the value of SP as it was
> +				@ before the push {lr} of the mcount mechanism
> +
> +	str     lr, [sp, #0]    @ store LR instead of PC
> +
> +	ldr     lr, [sp, #8]    @ get previous LR
> +
> +	str	r0, [sp, #8]	@ write r0 as OLD_R0 over previous LR
> +
> +	stmdb   sp!, {ip, lr}
> +	stmdb   sp!, {r0-r11, lr}
> +
> +	@ stack content at this point:
> +	@ 0  4          48   52       56            60   64    68       72
> +	@ R0 | R1 | ... | LR | SP + 4 | previous LR | LR | PSR | OLD_R0 |
> +
> +	mov r3, sp				@ struct pt_regs*
> +
> +	ldr r2, =function_trace_op
> +	ldr r2, [r2]				@ pointer to the current
> +						@ function tracing op
> +
> +	ldr	r1, [sp, #S_LR]			@ lr of instrumented func
> +
> +	ldr	lr, [sp, #S_PC]			@ get LR
> +
> +	mcount_adjust_addr	r0, lr		@ instrumented function
> +
> +	.globl ftrace_regs_call
> +ftrace_regs_call:
> +	bl	ftrace_stub
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +	.globl ftrace_graph_regs_call
> +ftrace_graph_regs_call:
> +	mov	r0, r0
> +#endif
> +
> +	@ pop saved regs
> +	ldmia   sp!, {r0-r12}			@ restore r0 through r12
> +	ldr	ip, [sp, #8]			@ restore PC
> +	ldr	lr, [sp, #4]			@ restore LR
> +	ldr	sp, [sp, #0]			@ restore SP
> +	mov	pc, ip				@ return
> +.endm
> +
> +#ifdef CONFIG_FUNCTION_GRAPH_TRACER
> +.macro __ftrace_graph_regs_caller
> +
> +	sub     r0, fp, #4              @ lr of instrumented routine (parent)
> +
> +	@ called from __ftrace_regs_caller
> +	ldr     r1, [sp, #S_PC]		@ instrumented routine (func)
> +	mcount_adjust_addr	r1, r1
> +
> +	mov	r2, fp			@ frame pointer
> +	bl	prepare_ftrace_return
> +
> +	@ pop registers saved in ftrace_regs_caller
> +	ldmia   sp!, {r0-r12}			@ restore r0 through r12
> +	ldr	ip, [sp, #8]			@ restore PC
> +	ldr	lr, [sp, #4]			@ restore LR
> +	ldr	sp, [sp, #0]			@ restore SP
> +	mov	pc, ip				@ return
> +
> +.endm
> +#endif
> +#endif
> +
>  .macro __ftrace_caller suffix
>  	mcount_enter
>  
>  	mcount_get_lr	r1			@ lr of instrumented func
>  	mcount_adjust_addr	r0, lr		@ instrumented function
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +	ldr r2, =function_trace_op
> +	ldr r2, [r2]				@ pointer to the current
> +						@ function tracing op
> +	mov r3, #0				@ regs is NULL
> +#endif
> +
>  	.globl ftrace_call\suffix
>  ftrace_call\suffix:
>  	bl	ftrace_stub
> @@ -212,6 +295,15 @@ UNWIND(.fnstart)
>  	__ftrace_caller
>  UNWIND(.fnend)
>  ENDPROC(ftrace_caller)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +ENTRY(ftrace_regs_caller)
> +UNWIND(.fnstart)
> +	__ftrace_regs_caller
> +UNWIND(.fnend)
> +ENDPROC(ftrace_regs_caller)
> +#endif
> +
>  #endif
>  
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
> @@ -220,6 +312,14 @@ UNWIND(.fnstart)
>  	__ftrace_graph_caller
>  UNWIND(.fnend)
>  ENDPROC(ftrace_graph_caller)
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +ENTRY(ftrace_graph_regs_caller)
> +UNWIND(.fnstart)
> +	__ftrace_graph_regs_caller
> +UNWIND(.fnend)
> +ENDPROC(ftrace_graph_regs_caller)
> +#endif
>  #endif
>  
>  .purgem mcount_enter
> diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
> index 833c991..5617932 100644
> --- a/arch/arm/kernel/ftrace.c
> +++ b/arch/arm/kernel/ftrace.c
> @@ -141,6 +141,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
>  
>  	ret = ftrace_modify_code(pc, 0, new, false);
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +	if (!ret) {
> +		pc = (unsigned long)&ftrace_regs_call;
> +		new = ftrace_call_replace(pc, (unsigned long)func);
> +
> +		ret = ftrace_modify_code(pc, 0, new, false);
> +	}
> +#endif
> +
>  #ifdef CONFIG_OLD_MCOUNT
>  	if (!ret) {
>  		pc = (unsigned long)&ftrace_call_old;
> @@ -159,11 +168,29 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
>  	unsigned long ip = rec->ip;
>  
>  	old = ftrace_nop_replace(rec);
> +
> +	new = ftrace_call_replace(ip, adjust_address(rec, addr));
> +
> +	return ftrace_modify_code(rec->ip, old, new, true);
> +}
> +
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +
> +int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
> +				unsigned long addr)
> +{
> +	unsigned long new, old;
> +	unsigned long ip = rec->ip;
> +
> +	old = ftrace_call_replace(ip, adjust_address(rec, old_addr));
> +
>  	new = ftrace_call_replace(ip, adjust_address(rec, addr));
>  
>  	return ftrace_modify_code(rec->ip, old, new, true);
>  }
>  
> +#endif
> +
>  int ftrace_make_nop(struct module *mod,
>  		    struct dyn_ftrace *rec, unsigned long addr)
>  {
> @@ -231,6 +258,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
>  extern unsigned long ftrace_graph_call;
>  extern unsigned long ftrace_graph_call_old;
>  extern void ftrace_graph_caller_old(void);
> +extern unsigned long ftrace_graph_regs_call;
> +extern void ftrace_graph_regs_caller(void);
>  
>  static int __ftrace_modify_caller(unsigned long *callsite,
>  				  void (*func) (void), bool enable)
> @@ -253,6 +282,14 @@ static int ftrace_modify_graph_caller(bool enable)
>  				     ftrace_graph_caller,
>  				     enable);
>  
> +#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
> +	if (!ret)
> +		ret = __ftrace_modify_caller(&ftrace_graph_regs_call,
> +				     ftrace_graph_regs_caller,
> +				     enable);
> +#endif
> +
> +
>  #ifdef CONFIG_OLD_MCOUNT
>  	if (!ret)
>  		ret = __ftrace_modify_caller(&ftrace_graph_call_old,
> -- 
> 2.7.4
> 

[PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Abel Vesa]

The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
operation to specify if registers need to saved/restored by the ftrace handler.
This is needed by kgraft and possibly other ftrace-based tools, and the ARM
architecture is currently lacking this feature. It would also be the first step
to support the "Kprobes-on-ftrace" optimization on ARM.

This patch introduces a new ftrace handler that stores the registers on the
stack before calling the next stage. The registers are restored from the stack
before going back to the instrumented function.

A side-effect of this patch is to activate the support for ftrace_modify_call()
as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture.

Signed-off-by: Abel Vesa <abelvesa@gmail.com>
---
 arch/arm/Kconfig               |   1 +
 arch/arm/include/asm/ftrace.h  |   4 ++
 arch/arm/kernel/entry-ftrace.S | 100 +++++++++++++++++++++++++++++++++++++++++
 arch/arm/kernel/ftrace.c       |  37 +++++++++++++++
 4 files changed, 142 insertions(+)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 4c1a35f..730d456 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -56,6 +56,7 @@ config ARM
 	select HAVE_DMA_API_DEBUG
 	select HAVE_DMA_CONTIGUOUS if MMU
 	select HAVE_DYNAMIC_FTRACE if (!XIP_KERNEL) && !CPU_ENDIAN_BE32 && MMU
+	select HAVE_DYNAMIC_FTRACE_WITH_REGS if HAVE_DYNAMIC_FTRACE
 	select HAVE_EFFICIENT_UNALIGNED_ACCESS if (CPU_V6 || CPU_V6K || CPU_V7) && MMU
 	select HAVE_EXIT_THREAD
 	select HAVE_FTRACE_MCOUNT_RECORD if (!XIP_KERNEL)
diff --git a/arch/arm/include/asm/ftrace.h b/arch/arm/include/asm/ftrace.h
index 22b7311..f379881 100644
--- a/arch/arm/include/asm/ftrace.h
+++ b/arch/arm/include/asm/ftrace.h
@@ -1,6 +1,10 @@
 #ifndef _ASM_ARM_FTRACE
 #define _ASM_ARM_FTRACE
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+#define ARCH_SUPPORTS_FTRACE_OPS 1
+#endif
+
 #ifdef CONFIG_FUNCTION_TRACER
 #define MCOUNT_ADDR		((unsigned long)(__gnu_mcount_nc))
 #define MCOUNT_INSN_SIZE	4 /* sizeof mcount call */
diff --git a/arch/arm/kernel/entry-ftrace.S b/arch/arm/kernel/entry-ftrace.S
index c73c403..efcd9f2 100644
--- a/arch/arm/kernel/entry-ftrace.S
+++ b/arch/arm/kernel/entry-ftrace.S
@@ -92,12 +92,95 @@
 2:	mcount_exit
 .endm
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+.macro __ftrace_regs_caller
+
+	sub	sp, sp, #8	@ space for PC and CPSR OLD_R0,
+				@ OLD_R0 will overwrite previous LR
+
+	add 	ip, sp, #12	@ move in IP the value of SP as it was
+				@ before the push {lr} of the mcount mechanism
+
+	str     lr, [sp, #0]    @ store LR instead of PC
+
+	ldr     lr, [sp, #8]    @ get previous LR
+
+	str	r0, [sp, #8]	@ write r0 as OLD_R0 over previous LR
+
+	stmdb   sp!, {ip, lr}
+	stmdb   sp!, {r0-r11, lr}
+
+	@ stack content at this point:
+	@ 0  4          48   52       56            60   64    68       72
+	@ R0 | R1 | ... | LR | SP + 4 | previous LR | LR | PSR | OLD_R0 |
+
+	mov r3, sp				@ struct pt_regs*
+
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+
+	ldr	r1, [sp, #S_LR]			@ lr of instrumented func
+
+	ldr	lr, [sp, #S_PC]			@ get LR
+
+	mcount_adjust_addr	r0, lr		@ instrumented function
+
+	.globl ftrace_regs_call
+ftrace_regs_call:
+	bl	ftrace_stub
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+	.globl ftrace_graph_regs_call
+ftrace_graph_regs_call:
+	mov	r0, r0
+#endif
+
+	@ pop saved regs
+	ldmia   sp!, {r0-r12}			@ restore r0 through r12
+	ldr	ip, [sp, #8]			@ restore PC
+	ldr	lr, [sp, #4]			@ restore LR
+	ldr	sp, [sp, #0]			@ restore SP
+	mov	pc, ip				@ return
+.endm
+
+#ifdef CONFIG_FUNCTION_GRAPH_TRACER
+.macro __ftrace_graph_regs_caller
+
+	sub     r0, fp, #4              @ lr of instrumented routine (parent)
+
+	@ called from __ftrace_regs_caller
+	ldr     r1, [sp, #S_PC]		@ instrumented routine (func)
+	mcount_adjust_addr	r1, r1
+
+	mov	r2, fp			@ frame pointer
+	bl	prepare_ftrace_return
+
+	@ pop registers saved in ftrace_regs_caller
+	ldmia   sp!, {r0-r12}			@ restore r0 through r12
+	ldr	ip, [sp, #8]			@ restore PC
+	ldr	lr, [sp, #4]			@ restore LR
+	ldr	sp, [sp, #0]			@ restore SP
+	mov	pc, ip				@ return
+
+.endm
+#endif
+#endif
+
 .macro __ftrace_caller suffix
 	mcount_enter
 
 	mcount_get_lr	r1			@ lr of instrumented func
 	mcount_adjust_addr	r0, lr		@ instrumented function
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	ldr r2, =function_trace_op
+	ldr r2, [r2]				@ pointer to the current
+						@ function tracing op
+	mov r3, #0				@ regs is NULL
+#endif
+
 	.globl ftrace_call\suffix
 ftrace_call\suffix:
 	bl	ftrace_stub
@@ -212,6 +295,15 @@ UNWIND(.fnstart)
 	__ftrace_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_regs_caller)
+#endif
+
 #endif
 
 #ifdef CONFIG_FUNCTION_GRAPH_TRACER
@@ -220,6 +312,14 @@ UNWIND(.fnstart)
 	__ftrace_graph_caller
 UNWIND(.fnend)
 ENDPROC(ftrace_graph_caller)
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+ENTRY(ftrace_graph_regs_caller)
+UNWIND(.fnstart)
+	__ftrace_graph_regs_caller
+UNWIND(.fnend)
+ENDPROC(ftrace_graph_regs_caller)
+#endif
 #endif
 
 .purgem mcount_enter
diff --git a/arch/arm/kernel/ftrace.c b/arch/arm/kernel/ftrace.c
index 833c991..5617932 100644
--- a/arch/arm/kernel/ftrace.c
+++ b/arch/arm/kernel/ftrace.c
@@ -141,6 +141,15 @@ int ftrace_update_ftrace_func(ftrace_func_t func)
 
 	ret = ftrace_modify_code(pc, 0, new, false);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret) {
+		pc = (unsigned long)&ftrace_regs_call;
+		new = ftrace_call_replace(pc, (unsigned long)func);
+
+		ret = ftrace_modify_code(pc, 0, new, false);
+	}
+#endif
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret) {
 		pc = (unsigned long)&ftrace_call_old;
@@ -159,11 +168,29 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
 	unsigned long ip = rec->ip;
 
 	old = ftrace_nop_replace(rec);
+
+	new = ftrace_call_replace(ip, adjust_address(rec, addr));
+
+	return ftrace_modify_code(rec->ip, old, new, true);
+}
+
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+
+int ftrace_modify_call(struct dyn_ftrace *rec, unsigned long old_addr,
+				unsigned long addr)
+{
+	unsigned long new, old;
+	unsigned long ip = rec->ip;
+
+	old = ftrace_call_replace(ip, adjust_address(rec, old_addr));
+
 	new = ftrace_call_replace(ip, adjust_address(rec, addr));
 
 	return ftrace_modify_code(rec->ip, old, new, true);
 }
 
+#endif
+
 int ftrace_make_nop(struct module *mod,
 		    struct dyn_ftrace *rec, unsigned long addr)
 {
@@ -231,6 +258,8 @@ void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 extern unsigned long ftrace_graph_call;
 extern unsigned long ftrace_graph_call_old;
 extern void ftrace_graph_caller_old(void);
+extern unsigned long ftrace_graph_regs_call;
+extern void ftrace_graph_regs_caller(void);
 
 static int __ftrace_modify_caller(unsigned long *callsite,
 				  void (*func) (void), bool enable)
@@ -253,6 +282,14 @@ static int ftrace_modify_graph_caller(bool enable)
 				     ftrace_graph_caller,
 				     enable);
 
+#ifdef CONFIG_DYNAMIC_FTRACE_WITH_REGS
+	if (!ret)
+		ret = __ftrace_modify_caller(&ftrace_graph_regs_call,
+				     ftrace_graph_regs_caller,
+				     enable);
+#endif
+
+
 #ifdef CONFIG_OLD_MCOUNT
 	if (!ret)
 		ret = __ftrace_modify_caller(&ftrace_graph_call_old,
-- 
2.7.4

Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS

[Russell King - ARM Linux]

On Fri, May 12, 2017 at 10:06:47PM +0100, Abel Vesa wrote:
> The DYNAMIC_FTRACE_WITH_REGS configuration makes it possible for a ftrace
> operation to specify if registers need to saved/restored by the ftrace handler.
> This is needed by kgraft and possibly other ftrace-based tools, and the ARM
> architecture is currently lacking this feature. It would also be the first step
> to support the "Kprobes-on-ftrace" optimization on ARM.
> 
> This patch introduces a new ftrace handler that stores the registers on the
> stack before calling the next stage. The registers are restored from the stack
> before going back to the instrumented function.
> 
> A side-effect of this patch is to activate the support for ftrace_modify_call()
> as it defines ARCH_SUPPORTS_FTRACE_OPS for the ARM architecture.

With the exception of the long lines in the commit message (please wrap
before column 72 so that 'git log' is readable on 80 column displays) 
I think this is fine.  Please amend the commit message and sent to the
patch system.  Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.