ftrace/x86: Updated mcount cleanup for 3.19
Steven Rostedt
maintainercover ftrace/x86: Updated mcount cleanup for 3.19
git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git
for-next
Head SHA1: 6a06bdbf7f9c669743f58084991ba280f2925586
Steven Rostedt (Red Hat) (9):
ftrace/x86: Have static tracing also use ftrace_caller_setup ftrace/x86: Move MCOUNT_SAVE_FRAME out of header file
ftrace/x86: Rename MCOUNT_SAVE_FRAME and add more detailed comments ftrace/x86: Have save_mcount_regs store RIP in %rdi for first parameter ftrace/x86: Simplify save_mcount_regs on getting RIP
ftrace/x86: Add macro MCOUNT_REG_SIZE for amount of stack used to save mcount regs ftrace/x86: Have save_mcount_regs macro also save stack frames if needed ftrace/x86: Get rid of ftrace_caller_setup
ftrace/fgraph/x86: Have prepare_ftrace_return() take ip as first parameter
----
arch/x86/include/asm/ftrace.h | 33 ------ arch/x86/kernel/ftrace.c | 4 +- arch/x86/kernel/mcount_64.S | 233 ++++++++++++++++++++++++++---------------- 3 files changed, 149 insertions(+), 121 deletions(-)
Steven Rostedt
maintainerPATCH 5/9 ftrace/x86: Simplify save_mcount_regs on getting RIP
From: "Steven Rostedt (Red Hat)" <[email protected]>
Currently save_mcount_regs is passed a "skip" parameter to know how much stack updated the pt_regs, as it tries to keep the saved pt_regs in the same location for all users. This is rather stupid, especially since the part stored on the pt_regs has nothing to do with what is suppose to be in that location.
Instead of doing that, just pass in an "added" parameter that lets that macro know how much stack was added before it was called so that it can get to the RIP. But the difference is that it will now offset the pt_regs by that "added" count. The caller now needs to take care of the offset of the pt_regs.
This will make it easier to simplify the code later.
Reviewed-by Thomas Gleixner <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 37 ++++++++++++++++++------------------- 1 file changed, 18 insertions(+), 19 deletions(-)
arch/x86/kernel/mcount_64.S +18 -19
@@ -37,12 +37,12 @@ * be saved in the locations that pt_regs has them in. */ -/* skip is set if the stack was already partially adjusted */ -.macro save_mcount_regs skip=0 +/* @added: the amount of stack added before calling this */ +.macro save_mcount_regs added=0 /* * We add enough stack to save all regs. */ - subq $(SS+8-\skip), %rsp + subq $(SS+8), %rsp movq %rax, RAX(%rsp) movq %rcx, RCX(%rsp) movq %rdx, RDX(%rsp)
@@ -51,11 +51,11 @@ movq %r8, R8(%rsp) movq %r9, R9(%rsp) /* Move RIP to its proper location */ - movq SS+8(%rsp), %rdi + movq SS+8+\added(%rsp), %rdi movq %rdi, RIP(%rsp) .endm -.macro restore_mcount_regs skip=0 +.macro restore_mcount_regs movq R9(%rsp), %r9 movq R8(%rsp), %r8 movq RDI(%rsp), %rdi
@@ -63,12 +63,12 @@ movq RDX(%rsp), %rdx movq RCX(%rsp), %rcx movq RAX(%rsp), %rax - addq $(SS+8-\skip), %rsp + addq $(SS+8), %rsp .endm /* skip is set if stack has been adjusted */ -.macro ftrace_caller_setup trace_label skip=0 - save_mcount_regs \skip +.macro ftrace_caller_setup trace_label added=0 + save_mcount_regs \added /* Save this location */ GLOBAL(\trace_label)
@@ -79,9 +79,9 @@ GLOBAL(\trace_label) subq $MCOUNT_INSN_SIZE, %rdi /* Load the parent_ip into the second parameter */ #ifdef CC_USING_FENTRY - movq SS+16(%rsp), %rsi + movq SS+16+\added(%rsp), %rsi #else - movq 8(%rbp), %rsi + movq 8+\added(%rbp), %rsi #endif .endm
@@ -156,10 +156,10 @@ GLOBAL(ftrace_stub) END(ftrace_caller) ENTRY(ftrace_regs_caller) - /* Save the current flags before compare (in SS location)*/ + /* Save the current flags before any operations that can change them */ pushfq - /* skip=8 to skip flags saved in SS */ + /* added 8 bytes to save flags */ ftrace_caller_setup ftrace_regs_caller_op_ptr 8 /* Save the rest of pt_regs */
@@ -172,15 +172,15 @@ ENTRY(ftrace_regs_caller) movq %rbp, RBP(%rsp) movq %rbx, RBX(%rsp) /* Copy saved flags */ - movq SS(%rsp), %rcx + movq SS+8(%rsp), %rcx movq %rcx, EFLAGS(%rsp) /* Kernel segments */ movq $__KERNEL_DS, %rcx movq %rcx, SS(%rsp) movq $__KERNEL_CS, %rcx movq %rcx, CS(%rsp) - /* Stack - skipping return address */ - leaq SS+16(%rsp), %rcx + /* Stack - skipping return address and flags */ + leaq SS+8*3(%rsp), %rcx movq %rcx, RSP(%rsp) /* regs go into 4th parameter */
@@ -195,11 +195,11 @@ GLOBAL(ftrace_regs_call) /* Copy flags back to SS, to restore them */ movq EFLAGS(%rsp), %rax - movq %rax, SS(%rsp) + movq %rax, SS+8(%rsp) /* Handlers can change the RIP */ movq RIP(%rsp), %rax - movq %rax, SS+8(%rsp) + movq %rax, SS+8*2(%rsp) /* restore the rest of pt_regs */ movq R15(%rsp), %r15
@@ -210,8 +210,7 @@ GLOBAL(ftrace_regs_call) movq RBP(%rsp), %rbp movq RBX(%rsp), %rbx - /* skip=8 to skip flags saved in SS */ - restore_mcount_regs 8 + restore_mcount_regs /* Restore flags */ popfq
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 4/9 ftrace/x86: Have save_mcount_regs store RIP in %rdi for first parameter
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Instead of having save_mcount_regs store the RIP in %rdx as a temp register to place it in the proper location of the pt_regs on the stack. Use the %rdi register as the temp register. This lets us remove the extra store in the ftrace_caller_setup macro.
Reviewed-by Thomas Gleixner <[email protected]>
Suggested-by Linus Torvalds <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
arch/x86/kernel/mcount_64.S +3 -4
@@ -51,8 +51,8 @@ movq %r8, R8(%rsp) movq %r9, R9(%rsp) /* Move RIP to its proper location */ - movq SS+8(%rsp), %rdx - movq %rdx, RIP(%rsp) + movq SS+8(%rsp), %rdi + movq %rdi, RIP(%rsp) .endm .macro restore_mcount_regs skip=0
@@ -75,8 +75,7 @@ GLOBAL(\trace_label) /* Load the ftrace_ops into the 3rd parameter */ movq function_trace_op(%rip), %rdx - /* Load ip into the first parameter */ - movq RIP(%rsp), %rdi + /* %rdi already has %rip from the save_mcount_regs macro */ subq $MCOUNT_INSN_SIZE, %rdi /* Load the parent_ip into the second parameter */ #ifdef CC_USING_FENTRY
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 8/9 ftrace/x86: Get rid of ftrace_caller_setup
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Move all the work from ftrace_caller_setup into save_mcount_regs. This simplifies the code and makes it easier to understand.
Reviewed-by Thomas Gleixner <[email protected]>
Suggested-by Linus Torvalds <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 71 +++++++++++++++++++++++++++------------------ 1 file changed, 42 insertions(+), 29 deletions(-)
arch/x86/kernel/mcount_64.S +42 -29
@@ -54,7 +54,15 @@ * be saved in the locations that pt_regs has them in. */ -/* @added: the amount of stack added before calling this */ +/* + * @added: the amount of stack added before calling this + * + * After this is called, the following registers contain: + * + * %rdi - holds the address that called the trampoline + * %rsi - holds the parent function (traced function's return address) + * %rdx - holds the original %rbp + */ .macro save_mcount_regs added=0 /* Always save the original rbp */
@@ -101,9 +109,24 @@ movq MCOUNT_REG_SIZE-8(%rsp), %rdx movq %rdx, RBP(%rsp) + /* Copy the parent address into %rsi (second parameter) */ +#ifdef CC_USING_FENTRY + movq MCOUNT_REG_SIZE+8+\added(%rsp), %rsi +#else + /* %rdx contains original %rbp */ + movq 8(%rdx), %rsi +#endif + /* Move RIP to its proper location */ movq MCOUNT_REG_SIZE+\added(%rsp), %rdi movq %rdi, RIP(%rsp) + + /* + * Now %rdi (the first parameter) has the return address of + * where ftrace_call returns. But the callbacks expect the + * the address of the call itself. + */ + subq $MCOUNT_INSN_SIZE, %rdi .endm .macro restore_mcount_regs
@@ -122,28 +145,6 @@ .endm -/* skip is set if stack has been adjusted */ -.macro ftrace_caller_setup trace_label added=0 - save_mcount_regs \added - - /* Save this location */ -GLOBAL(\trace_label) - /* Load the ftrace_ops into the 3rd parameter */ - movq function_trace_op(%rip), %rdx - - /* %rdi already has %rip from the save_mcount_regs macro */ - subq $MCOUNT_INSN_SIZE, %rdi - /* Load the parent_ip into the second parameter */ -#ifdef CC_USING_FENTRY - movq MCOUNT_REG_SIZE+8+\added(%rsp), %rsi -#else - /* Need to grab the original %rbp */ - movq RBP(%rsp), %rsi - /* Now parent address is 8 above original %rbp */ - movq 8(%rsi), %rsi -#endif -.endm - #ifdef CONFIG_DYNAMIC_FTRACE ENTRY(function_hook)
@@ -151,7 +152,13 @@ ENTRY(function_hook) END(function_hook) ENTRY(ftrace_caller) - ftrace_caller_setup ftrace_caller_op_ptr + /* save_mcount_regs fills in first two parameters */ + save_mcount_regs + +GLOBAL(ftrace_caller_op_ptr) + /* Load the ftrace_ops into the 3rd parameter */ + movq function_trace_op(%rip), %rdx + /* regs go into 4th parameter (but make it NULL) */ movq $0, %rcx
@@ -182,7 +189,12 @@ ENTRY(ftrace_regs_caller) pushfq /* added 8 bytes to save flags */ - ftrace_caller_setup ftrace_regs_caller_op_ptr 8 + save_mcount_regs 8 + /* save_mcount_regs fills in first two parameters */ + +GLOBAL(ftrace_regs_caller_op_ptr) + /* Load the ftrace_ops into the 3rd parameter */ + movq function_trace_op(%rip), %rdx /* Save the rest of pt_regs */ movq %r15, R15(%rsp)
@@ -263,7 +275,8 @@ GLOBAL(ftrace_stub) retq trace: - ftrace_caller_setup ftrace_caller_op_ptr + /* save_mcount_regs fills in first two parameters */ + save_mcount_regs call *ftrace_trace_function
@@ -276,16 +289,16 @@ END(function_hook) #ifdef CONFIG_FUNCTION_GRAPH_TRACER ENTRY(ftrace_graph_caller) + /* Saves rbp into %rdx */ save_mcount_regs #ifdef CC_USING_FENTRY leaq MCOUNT_REG_SIZE+8(%rsp), %rdi movq $0, %rdx /* No framepointers needed */ #else - /* Need to grab the original %rbp */ - movq RBP(%rsp), %rdx - /* Now parent address is 8 above original %rbp */ + /* Save address of the return address of traced function */ leaq 8(%rdx), %rdi + /* ftrace does sanity checks against frame pointers */ movq (%rdx), %rdx #endif movq RIP(%rsp), %rsi
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 9/9 ftrace/fgraph/x86: Have prepare_ftrace_return() take ip as first parameter
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
The function graph helper function prepare_ftrace_return() which does the work to hijack the parent pointer has that parent pointer as its first parameter. Instead, if we make it the second parameter and have ip as the first parameter (self_addr), then it can use the %rdi from save_mcount_regs that loads it already.
Reviewed-by Thomas Gleixner <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/ftrace.c | 4 ++-- arch/x86/kernel/mcount_64.S | 11 ++++------- 2 files changed, 6 insertions(+), 9 deletions(-)
arch/x86/kernel/ftrace.c +2 -2
@@ -871,7 +871,7 @@ static void *addr_from_call(void *ptr)return ptr +MCOUNT_INSN_SIZE + calc.offset; } -void prepare_ftrace_return (unsigned long *parent,unsigned long self_addr, +void prepare_ftrace_return (unsigned long self_addr,unsigned long *parent,unsigned long frame_pointer);/*
@@ -964,7 +964,7 @@ int ftrace_disable_ftrace_graph_caller(void)* Hook the return address and push it in the stack of return addrs * in current thread info. */ -void prepare_ftrace_return (unsigned long *parent,unsigned long self_addr, +void prepare_ftrace_return (unsigned long self_addr,unsigned long *parent,unsigned long frame_pointer) {unsigned long old;
arch/x86/kernel/mcount_64.S +4 -7
@@ -124,7 +124,7 @@ /* * Now %rdi (the first parameter) has the return address of * where ftrace_call returns. But the callbacks expect the - * the address of the call itself. + * address of the call itself. */ subq $MCOUNT_INSN_SIZE, %rdi .endm
@@ -289,21 +289,18 @@ END(function_hook) #ifdef CONFIG_FUNCTION_GRAPH_TRACER ENTRY(ftrace_graph_caller) - /* Saves rbp into %rdx */ + /* Saves rbp into %rdx and fills first parameter */ save_mcount_regs #ifdef CC_USING_FENTRY - leaq MCOUNT_REG_SIZE+8(%rsp), %rdi + leaq MCOUNT_REG_SIZE+8(%rsp), %rsi movq $0, %rdx /* No framepointers needed */ #else /* Save address of the return address of traced function */ - leaq 8(%rdx), %rdi + leaq 8(%rdx), %rsi /* ftrace does sanity checks against frame pointers */ movq (%rdx), %rdx #endif - movq RIP(%rsp), %rsi - subq $MCOUNT_INSN_SIZE, %rsi - call prepare_ftrace_return restore_mcount_regs
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 6/9 ftrace/x86: Add macro MCOUNT_REG_SIZE for amount of stack used to save mcount regs
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
The macro save_mcount_regs saves regs onto the stack. But to uncouple the amount of stack used in that macro from the users of the macro, we need to have a define that tells all the users how much stack is used by that macro. This way we can change the amount of stack the macro uses without breaking its users.
Also remove some dead code that was left over from commit fdc841b58cf5 "ftrace: x86: Remove check of obsolete variable function_trace_stop".
Reviewed-by Thomas Gleixner <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-)
arch/x86/kernel/mcount_64.S +12 -12
@@ -21,6 +21,9 @@ # define function_hook mcount #endif +/* Size of stack used to save mcount regs in save_mcount_regs */ +#define MCOUNT_REG_SIZE (SS+8) + /* * gcc -pg option adds a call to 'mcount' in most functions. * When -mfentry is used, the call is to 'fentry' and not 'mcount'
@@ -42,7 +45,7 @@ /* * We add enough stack to save all regs. */ - subq $(SS+8), %rsp + subq $MCOUNT_REG_SIZE, %rsp movq %rax, RAX(%rsp) movq %rcx, RCX(%rsp) movq %rdx, RDX(%rsp)
@@ -51,7 +54,7 @@ movq %r8, R8(%rsp) movq %r9, R9(%rsp) /* Move RIP to its proper location */ - movq SS+8+\added(%rsp), %rdi + movq MCOUNT_REG_SIZE+\added(%rsp), %rdi movq %rdi, RIP(%rsp) .endm
@@ -63,7 +66,7 @@ movq RDX(%rsp), %rdx movq RCX(%rsp), %rcx movq RAX(%rsp), %rax - addq $(SS+8), %rsp + addq $MCOUNT_REG_SIZE, %rsp .endm /* skip is set if stack has been adjusted */
@@ -79,7 +82,7 @@ GLOBAL(\trace_label) subq $MCOUNT_INSN_SIZE, %rdi /* Load the parent_ip into the second parameter */ #ifdef CC_USING_FENTRY - movq SS+16+\added(%rsp), %rsi + movq MCOUNT_REG_SIZE+8+\added(%rsp), %rsi #else movq 8+\added(%rbp), %rsi #endif
@@ -172,7 +175,7 @@ ENTRY(ftrace_regs_caller) movq %rbp, RBP(%rsp) movq %rbx, RBX(%rsp) /* Copy saved flags */ - movq SS+8(%rsp), %rcx + movq MCOUNT_REG_SIZE(%rsp), %rcx movq %rcx, EFLAGS(%rsp) /* Kernel segments */ movq $__KERNEL_DS, %rcx
@@ -180,7 +183,7 @@ ENTRY(ftrace_regs_caller) movq $__KERNEL_CS, %rcx movq %rcx, CS(%rsp) /* Stack - skipping return address and flags */ - leaq SS+8*3(%rsp), %rcx + leaq MCOUNT_REG_SIZE+8*2(%rsp), %rcx movq %rcx, RSP(%rsp) /* regs go into 4th parameter */
@@ -195,11 +198,11 @@ GLOBAL(ftrace_regs_call) /* Copy flags back to SS, to restore them */ movq EFLAGS(%rsp), %rax - movq %rax, SS+8(%rsp) + movq %rax, MCOUNT_REG_SIZE(%rsp) /* Handlers can change the RIP */ movq RIP(%rsp), %rax - movq %rax, SS+8*2(%rsp) + movq %rax, MCOUNT_REG_SIZE+8(%rsp) /* restore the rest of pt_regs */ movq R15(%rsp), %r15
@@ -225,9 +228,6 @@ GLOBAL(ftrace_regs_caller_end) jmp ftrace_return - popfq - jmp ftrace_stub - END(ftrace_regs_caller)
@@ -266,7 +266,7 @@ ENTRY(ftrace_graph_caller) save_mcount_regs #ifdef CC_USING_FENTRY - leaq SS+16(%rsp), %rdi + leaq MCOUNT_REG_SIZE+8(%rsp), %rdi movq $0, %rdx /* No framepointers needed */ #else leaq 8(%rbp), %rdi
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 2/9 ftrace/x86: Move MCOUNT_SAVE_FRAME out of header file
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Linus pointed out that MCOUNT_SAVE_FRAME is used in only a single file and that there's no reason that it should be in a header file. Move the macro to the code that uses it.
Reviewed-by Thomas Gleixner <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/include/asm/ftrace.h | 33 --------------------------------- arch/x86/kernel/mcount_64.S | 29 +++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 33 deletions(-)
arch/x86/include/asm/ftrace.h +0 -33
@@ -1,39 +1,6 @@#ifndef _ASM_X86_FTRACE_H#define _ASM_X86_FTRACE_H -#ifdef __ASSEMBLY__ - -/* skip is set if the stack was already partially adjusted */ - .macro MCOUNT_SAVE_FRAME skip=0 -/* -* We add enough stack to save all regs. - */ - subq $(SS+8-\skip), %rsp - movq %rax, RAX(%rsp) - movq %rcx, RCX(%rsp) - movq %rdx, RDX(%rsp) - movq %rsi, RSI(%rsp) - movq %rdi, RDI(%rsp) - movq %r8, R8(%rsp) - movq %r9, R9(%rsp) - /* Move RIP to its proper location */ - movq SS+8(%rsp), %rdx - movq %rdx, RIP(%rsp) - .endm - - .macro MCOUNT_RESTORE_FRAME skip=0 - movq R9(%rsp), %r9 - movq R8(%rsp), %r8 - movq RDI(%rsp), %rdi - movq RSI(%rsp), %rsi - movq RDX(%rsp), %rdx - movq RCX(%rsp), %rcx - movq RAX(%rsp), %rax - addq $(SS+8-\skip), %rsp - .endm - -#endif -#ifdef CONFIG_FUNCTION_TRACER #ifdef CC_USING_FENTRY # define MCOUNT_ADDR ((long)(__fentry__))
arch/x86/kernel/mcount_64.S +29 -0
@@ -21,6 +21,35 @@ # define function_hook mcount #endif +/* skip is set if the stack was already partially adjusted */ +.macro MCOUNT_SAVE_FRAME skip=0 + /* + * We add enough stack to save all regs. + */ + subq $(SS+8-\skip), %rsp + movq %rax, RAX(%rsp) + movq %rcx, RCX(%rsp) + movq %rdx, RDX(%rsp) + movq %rsi, RSI(%rsp) + movq %rdi, RDI(%rsp) + movq %r8, R8(%rsp) + movq %r9, R9(%rsp) + /* Move RIP to its proper location */ + movq SS+8(%rsp), %rdx + movq %rdx, RIP(%rsp) + .endm + +.macro MCOUNT_RESTORE_FRAME skip=0 + movq R9(%rsp), %r9 + movq R8(%rsp), %r8 + movq RDI(%rsp), %rdi + movq RSI(%rsp), %rsi + movq RDX(%rsp), %rdx + movq RCX(%rsp), %rcx + movq RAX(%rsp), %rax + addq $(SS+8-\skip), %rsp + .endm + /* skip is set if stack has been adjusted */ .macro ftrace_caller_setup trace_label skip=0 MCOUNT_SAVE_FRAME \skip
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 7/9 ftrace/x86: Have save_mcount_regs macro also save stack frames if needed
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
The save_mcount_regs macro saves and restores the required mcount regs that need to be saved before calling C code. It is done for all the function hook utilities (static tracing, dynamic tracing, regs, function graph).
When frame pointers are enabled, the ftrace trampolines need to set up frames and pointers such that a back trace (dump stack) can continue passed them. Currently, a separate macro is used (create_frame) to do this, but it's only done for the ftrace_caller and ftrace_reg_caller functions. It is not done for the static tracer or function graph tracing.
Instead of having a separate macro doing the recording of the frames, have the save_mcount_regs perform this task. This also has all tracers saving the frame pointers when needed.
Reviewed-by Thomas Gleixner <[email protected]>
Suggested-by Linus Torvalds <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 118 +++++++++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 51 deletions(-)
arch/x86/kernel/mcount_64.S +67 -51
@@ -21,8 +21,22 @@ # define function_hook mcount #endif +/* All cases save the original rbp (8 bytes) */ +#ifdef CONFIG_FRAME_POINTER +# ifdef CC_USING_FENTRY +/* Save parent and function stack frames (rip and rbp) */ +# define MCOUNT_FRAME_SIZE (8+16*2) +# else +/* Save just function stack frame (rip and rbp) */ +# define MCOUNT_FRAME_SIZE (8+16) +# endif +#else +/* No need to save a stack frame */ +# define MCOUNT_FRAME_SIZE 8 +#endif /* CONFIG_FRAME_POINTER */ + /* Size of stack used to save mcount regs in save_mcount_regs */ -#define MCOUNT_REG_SIZE (SS+8) +#define MCOUNT_REG_SIZE (SS+8 + MCOUNT_FRAME_SIZE) /* * gcc -pg option adds a call to 'mcount' in most functions.
@@ -42,10 +56,37 @@ /* @added: the amount of stack added before calling this */ .macro save_mcount_regs added=0 - /* - * We add enough stack to save all regs. - */ - subq $MCOUNT_REG_SIZE, %rsp + + /* Always save the original rbp */ + pushq %rbp + +#ifdef CONFIG_FRAME_POINTER + /* + * Stack traces will stop at the ftrace trampoline if the frame pointer + * is not set up properly. If fentry is used, we need to save a frame + * pointer for the parent as well as the function traced, because the + * fentry is called before the stack frame is set up, where as mcount + * is called afterward. + */ +#ifdef CC_USING_FENTRY + /* Save the parent pointer (skip orig rbp and our return address) */ + pushq \added+8*2(%rsp) + pushq %rbp + movq %rsp, %rbp + /* Save the return address (now skip orig rbp, rbp and parent) */ + pushq \added+8*3(%rsp) +#else + /* Can't assume that rip is before this (unless added was zero) */ + pushq \added+8(%rsp) +#endif + pushq %rbp + movq %rsp, %rbp +#endif /* CONFIG_FRAME_POINTER */ + + /* + * We add enough stack to save all regs. + */ + subq $(MCOUNT_REG_SIZE - MCOUNT_FRAME_SIZE), %rsp movq %rax, RAX(%rsp) movq %rcx, RCX(%rsp) movq %rdx, RDX(%rsp)
@@ -53,6 +94,13 @@ movq %rdi, RDI(%rsp) movq %r8, R8(%rsp) movq %r9, R9(%rsp) + /* + * Save the original RBP. Even though the mcount ABI does not + * require this, it helps out callers. + */ + movq MCOUNT_REG_SIZE-8(%rsp), %rdx + movq %rdx, RBP(%rsp) + /* Move RIP to its proper location */ movq MCOUNT_REG_SIZE+\added(%rsp), %rdi movq %rdi, RIP(%rsp)
@@ -66,7 +114,12 @@ movq RDX(%rsp), %rdx movq RCX(%rsp), %rcx movq RAX(%rsp), %rax + + /* ftrace_regs_caller can modify %rbp */ + movq RBP(%rsp), %rbp + addq $MCOUNT_REG_SIZE, %rsp + .endm /* skip is set if stack has been adjusted */
@@ -84,7 +137,10 @@ GLOBAL(\trace_label) #ifdef CC_USING_FENTRY movq MCOUNT_REG_SIZE+8+\added(%rsp), %rsi #else - movq 8+\added(%rbp), %rsi + /* Need to grab the original %rbp */ + movq RBP(%rsp), %rsi + /* Now parent address is 8 above original %rbp */ + movq 8(%rsi), %rsi #endif .endm
@@ -94,51 +150,14 @@ ENTRY(function_hook) retq END(function_hook) -#ifdef CONFIG_FRAME_POINTER -/* - * Stack traces will stop at the ftrace trampoline if the frame pointer - * is not set up properly. If fentry is used, we need to save a frame - * pointer for the parent as well as the function traced, because the - * fentry is called before the stack frame is set up, where as mcount - * is called afterward. - */ -.macro create_frame parent rip -#ifdef CC_USING_FENTRY - pushq \parent - pushq %rbp - movq %rsp, %rbp -#endif - pushq \rip - pushq %rbp - movq %rsp, %rbp -.endm - -.macro restore_frame -#ifdef CC_USING_FENTRY - addq $16, %rsp -#endif - popq %rbp - addq $8, %rsp -.endm -#else -.macro create_frame parent rip -.endm -.macro restore_frame -.endm -#endif /* CONFIG_FRAME_POINTER */ - ENTRY(ftrace_caller) ftrace_caller_setup ftrace_caller_op_ptr /* regs go into 4th parameter (but make it NULL) */ movq $0, %rcx - create_frame %rsi, %rdi - GLOBAL(ftrace_call) call ftrace_stub - restore_frame - restore_mcount_regs /*
@@ -172,7 +191,6 @@ ENTRY(ftrace_regs_caller) movq %r12, R12(%rsp) movq %r11, R11(%rsp) movq %r10, R10(%rsp) - movq %rbp, RBP(%rsp) movq %rbx, RBX(%rsp) /* Copy saved flags */ movq MCOUNT_REG_SIZE(%rsp), %rcx
@@ -189,13 +207,9 @@ ENTRY(ftrace_regs_caller) /* regs go into 4th parameter */ leaq (%rsp), %rcx - create_frame %rsi, %rdi - GLOBAL(ftrace_regs_call) call ftrace_stub - restore_frame - /* Copy flags back to SS, to restore them */ movq EFLAGS(%rsp), %rax movq %rax, MCOUNT_REG_SIZE(%rsp)
@@ -210,7 +224,6 @@ GLOBAL(ftrace_regs_call) movq R13(%rsp), %r13 movq R12(%rsp), %r12 movq R10(%rsp), %r10 - movq RBP(%rsp), %rbp movq RBX(%rsp), %rbx restore_mcount_regs
@@ -269,8 +282,11 @@ ENTRY(ftrace_graph_caller) leaq MCOUNT_REG_SIZE+8(%rsp), %rdi movq $0, %rdx /* No framepointers needed */ #else - leaq 8(%rbp), %rdi - movq (%rbp), %rdx + /* Need to grab the original %rbp */ + movq RBP(%rsp), %rdx + /* Now parent address is 8 above original %rbp */ + leaq 8(%rdx), %rdi + movq (%rdx), %rdx #endif movq RIP(%rsp), %rsi subq $MCOUNT_INSN_SIZE, %rsi
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 3/9 ftrace/x86: Rename MCOUNT_SAVE_FRAME and add more detailed comments
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
The name MCOUNT_SAVE_FRAME is rather confusing as it really isn't a function frame that is saved, but just the required mcount registers that are needed to be saved before C code may be called. The word "frame" confuses it as being a function frame which it is not.
Rename MCOUNT_SAVE_FRAME and MCOUNT_RESTORE_FRAME to save_mcount_regs and restore_mcount_regs respectively. Noticed the lower case, which keeps it from screaming at the reviewers.
Reviewed-by Thomas Gleixner <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 32 ++++++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 8 deletions(-)
arch/x86/kernel/mcount_64.S +24 -8
@@ -21,8 +21,24 @@ # define function_hook mcount #endif +/* + * gcc -pg option adds a call to 'mcount' in most functions. + * When -mfentry is used, the call is to 'fentry' and not 'mcount' + * and is done before the function's stack frame is set up. + * They both require a set of regs to be saved before calling + * any C code and restored before returning back to the function. + * + * On boot up, all these calls are converted into nops. When tracing + * is enabled, the call can jump to either ftrace_caller or + * ftrace_regs_caller. Callbacks (tracing functions) that require + * ftrace_regs_caller (like kprobes) need to have pt_regs passed to + * it. For this reason, the size of the pt_regs structure will be + * allocated on the stack and the required mcount registers will + * be saved in the locations that pt_regs has them in. + */ + /* skip is set if the stack was already partially adjusted */ -.macro MCOUNT_SAVE_FRAME skip=0 +.macro save_mcount_regs skip=0 /* * We add enough stack to save all regs. */
@@ -39,7 +55,7 @@ movq %rdx, RIP(%rsp) .endm -.macro MCOUNT_RESTORE_FRAME skip=0 +.macro restore_mcount_regs skip=0 movq R9(%rsp), %r9 movq R8(%rsp), %r8 movq RDI(%rsp), %rdi
@@ -52,7 +68,7 @@ /* skip is set if stack has been adjusted */ .macro ftrace_caller_setup trace_label skip=0 - MCOUNT_SAVE_FRAME \skip + save_mcount_regs \skip /* Save this location */ GLOBAL(\trace_label)
@@ -121,7 +137,7 @@ GLOBAL(ftrace_call) restore_frame - MCOUNT_RESTORE_FRAME + restore_mcount_regs /* * The copied trampoline must call ftrace_return as it
@@ -196,7 +212,7 @@ GLOBAL(ftrace_regs_call) movq RBX(%rsp), %rbx /* skip=8 to skip flags saved in SS */ - MCOUNT_RESTORE_FRAME 8 + restore_mcount_regs 8 /* Restore flags */ popfq
@@ -240,7 +256,7 @@ trace: call *ftrace_trace_function - MCOUNT_RESTORE_FRAME + restore_mcount_regs jmp fgraph_trace END(function_hook)
@@ -249,7 +265,7 @@ END(function_hook) #ifdef CONFIG_FUNCTION_GRAPH_TRACER ENTRY(ftrace_graph_caller) - MCOUNT_SAVE_FRAME + save_mcount_regs #ifdef CC_USING_FENTRY leaq SS+16(%rsp), %rdi
@@ -263,7 +279,7 @@ ENTRY(ftrace_graph_caller) call prepare_ftrace_return - MCOUNT_RESTORE_FRAME + restore_mcount_regs retq END(ftrace_graph_caller)
Signature · 1 line
2.1.3
Steven Rostedt
maintainerPATCH 1/9 ftrace/x86: Have static tracing also use ftrace_caller_setup
In reply to Steven RostedtFrom: "Steven Rostedt (Red Hat)" <[email protected]>
Linus pointed out that there were locations that did the hard coded update of the parent and rip parameters. One of them was the static tracer which could also use the ftrace_caller_setup to do that work. In fact, because it did not use it, it is prone to bugs, and since the static tracer is hardly ever used (who wants function tracing code always being called?) it doesn't get tested very often. I only run a few "does it still work" tests on it. But I do not run stress tests on that code. Although, since it is never turned off, just having it on should be stressful enough. (especially for the performance folks)
There's no reason that the static tracer can't also use ftrace_caller_setup. Have it do so.
Reviewed-by Thomas Gleixner <[email protected]>
Signed-off-by Steven Rostedt <[email protected]>
--- arch/x86/kernel/mcount_64.S | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-)
arch/x86/kernel/mcount_64.S +7 -15
@@ -21,12 +21,6 @@ # define function_hook mcount #endif -#ifdef CONFIG_DYNAMIC_FTRACE - -ENTRY(function_hook) - retq -END(function_hook) - /* skip is set if stack has been adjusted */ .macro ftrace_caller_setup trace_label skip=0 MCOUNT_SAVE_FRAME \skip
@@ -47,6 +41,12 @@ GLOBAL(\trace_label) #endif .endm +#ifdef CONFIG_DYNAMIC_FTRACE + +ENTRY(function_hook) + retq +END(function_hook) + #ifdef CONFIG_FRAME_POINTER /* * Stack traces will stop at the ftrace trampoline if the frame pointer
@@ -207,15 +207,7 @@ GLOBAL(ftrace_stub) retq trace: - MCOUNT_SAVE_FRAME - - movq RIP(%rsp), %rdi -#ifdef CC_USING_FENTRY - movq SS+16(%rsp), %rsi -#else - movq 8(%rbp), %rsi -#endif - subq $MCOUNT_INSN_SIZE, %rdi + ftrace_caller_setup ftrace_caller_op_ptr call *ftrace_trace_function
Signature · 1 line
2.1.3