* [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
@ 2016-12-08 21:46 Abel Vesa
2016-12-08 22:46 ` Abel Vesa
0 siblings, 1 reply; 12+ messages in thread
From: Abel Vesa @ 2016-12-08 21:46 UTC (permalink / raw)
To: linux, rostedt, mingo, abelvesa, viro, jjhiblot
Cc: linux-arm-kernel, linux-kernel, pmladek, robin.murphy, zhouchengming1
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2016-12-08 21:46 [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS Abel Vesa
@ 2016-12-08 22:46 ` Abel Vesa
2017-01-10 15:51 ` Petr Mladek
0 siblings, 1 reply; 12+ messages in thread
From: Abel Vesa @ 2016-12-08 22:46 UTC (permalink / raw)
To: Abel Vesa
Cc: linux, rostedt, mingo, viro, jjhiblot, linux-arm-kernel,
linux-kernel, pmladek, robin.murphy, zhouchengming1
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2016-12-08 22:46 ` Abel Vesa
@ 2017-01-10 15:51 ` Petr Mladek
2017-01-12 0:19 ` Abel Vesa
0 siblings, 1 reply; 12+ messages in thread
From: Petr Mladek @ 2017-01-10 15:51 UTC (permalink / raw)
To: Abel Vesa
Cc: Abel Vesa, linux, rostedt, mingo, viro, jjhiblot,
linux-arm-kernel, linux-kernel, robin.murphy, zhouchengming1
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2017-01-10 15:51 ` Petr Mladek
@ 2017-01-12 0:19 ` Abel Vesa
2017-01-12 14:30 ` Jean-Jacques Hiblot
0 siblings, 1 reply; 12+ messages in thread
From: Abel Vesa @ 2017-01-12 0:19 UTC (permalink / raw)
To: Petr Mladek
Cc: Abel Vesa, linux, rostedt, mingo, viro, jjhiblot,
linux-arm-kernel, linux-kernel, robin.murphy, zhouchengming1
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2017-01-12 0:19 ` Abel Vesa
@ 2017-01-12 14:30 ` Jean-Jacques Hiblot
2017-01-13 8:30 ` Jean-Jacques Hiblot
0 siblings, 1 reply; 12+ messages in thread
From: Jean-Jacques Hiblot @ 2017-01-12 14:30 UTC (permalink / raw)
To: Abel Vesa
Cc: Petr Mladek, Abel Vesa, Russell King - ARM Linux, Steven Rostedt,
mingo, viro, Jean-Jacques Hiblot, linux-arm-kernel,
Linux Kernel Mailing List, robin.murphy, zhouchengming1
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2017-01-12 14:30 ` Jean-Jacques Hiblot
@ 2017-01-13 8:30 ` Jean-Jacques Hiblot
2017-01-24 0:43 ` Abel Vesa
0 siblings, 1 reply; 12+ messages in thread
From: Jean-Jacques Hiblot @ 2017-01-13 8:30 UTC (permalink / raw)
To: Jean-Jacques Hiblot
Cc: Abel Vesa, Petr Mladek, Abel Vesa, Russell King - ARM Linux,
Steven Rostedt, mingo, viro, linux-arm-kernel,
Linux Kernel Mailing List, robin.murphy, zhouchengming1
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2017-01-13 8:30 ` Jean-Jacques Hiblot
@ 2017-01-24 0:43 ` Abel Vesa
0 siblings, 0 replies; 12+ messages in thread
From: Abel Vesa @ 2017-01-24 0:43 UTC (permalink / raw)
To: Jean-Jacques Hiblot
Cc: Petr Mladek, Russell King - ARM Linux, Steven Rostedt, mingo,
viro, linux-arm-kernel, Linux Kernel Mailing List, Robin Murphy,
zhouchengming
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
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
@ 2016-12-08 22:49 Abel Vesa
0 siblings, 0 replies; 12+ messages in thread
From: Abel Vesa @ 2016-12-08 22:49 UTC (permalink / raw)
To: linux, rostedt, mingo, abelvesa, viro, jjhiblot
Cc: linux-arm-kernel, linux-kernel, pmladek, robin.murphy, zhouchengming1
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
@ 2017-05-12 20:38 abelvesa
2017-05-12 21:05 ` Abel Vesa
0 siblings, 1 reply; 12+ messages in thread
From: abelvesa @ 2017-05-12 20:38 UTC (permalink / raw)
To: robin.murphy, jjhiblot, nicstange, Russell King, Steven Rostedt,
Ingo Molnar
Cc: pmladek, mhiramat, linux-arm-kernel, linux-kernel, Abel Vesa
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2017-05-12 20:38 abelvesa
@ 2017-05-12 21:05 ` Abel Vesa
0 siblings, 0 replies; 12+ messages in thread
From: Abel Vesa @ 2017-05-12 21:05 UTC (permalink / raw)
To: robin.murphy, jjhiblot, nicstange, Russell King, Steven Rostedt,
Ingo Molnar
Cc: pmladek, mhiramat, linux-arm-kernel, linux-kernel
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
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
@ 2017-05-12 21:06 Abel Vesa
2017-05-23 19:53 ` Russell King - ARM Linux
0 siblings, 1 reply; 12+ messages in thread
From: Abel Vesa @ 2017-05-12 21:06 UTC (permalink / raw)
To: robin.murphy, jjhiblot, nicstange, Russell King, Steven Rostedt,
Ingo Molnar
Cc: pmladek, mhiramat, linux-arm-kernel, linux-kernel
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
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS
2017-05-12 21:06 Abel Vesa
@ 2017-05-23 19:53 ` Russell King - ARM Linux
0 siblings, 0 replies; 12+ messages in thread
From: Russell King - ARM Linux @ 2017-05-23 19:53 UTC (permalink / raw)
To: Abel Vesa
Cc: robin.murphy, jjhiblot, nicstange, Steven Rostedt, Ingo Molnar,
pmladek, mhiramat, linux-arm-kernel, linux-kernel
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.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2017-05-23 19:53 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-08 21:46 [PATCH] arm: ftrace: Adds support for CONFIG_DYNAMIC_FTRACE_WITH_REGS Abel Vesa
2016-12-08 22:46 ` Abel Vesa
2017-01-10 15:51 ` Petr Mladek
2017-01-12 0:19 ` Abel Vesa
2017-01-12 14:30 ` Jean-Jacques Hiblot
2017-01-13 8:30 ` Jean-Jacques Hiblot
2017-01-24 0:43 ` Abel Vesa
2016-12-08 22:49 Abel Vesa
2017-05-12 20:38 abelvesa
2017-05-12 21:05 ` Abel Vesa
2017-05-12 21:06 Abel Vesa
2017-05-23 19:53 ` Russell King - ARM Linux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).