linux-kernel - Re: [RFC PATCH -tip 2/9] ftrace/x86-64: support SAVE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 29 May 2012 19:05:27 -0400
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
Cc:	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Ananth N Mavinakayanahalli <ananth@...ibm.com>,
	"Frank Ch. Eigler" <fche@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Frederic Weisbecker <fweisbec@...il.com>,
	yrl.pp-manager.tt@...achi.com
Subject: Re: [RFC PATCH -tip 2/9] ftrace/x86-64: support SAVE_REGS feature
 on x86-64

On Tue, 2012-05-29 at 21:49 +0900, Masami Hiramatsu wrote:
> Add register saving/restoring sequence in ftrace_caller
> on x86-64 for enabling SAVE_REGS feature.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Steven Rostedt <rostedt@...dmis.org>
> ---
> 
>  arch/x86/include/asm/ftrace.h |    4 ++++
>  arch/x86/kernel/entry_64.S    |   38 +++++++++++++++++++++++++++++++++-----
>  2 files changed, 37 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/include/asm/ftrace.h b/arch/x86/include/asm/ftrace.h
> index 18d9005..ffb9564 100644
> --- a/arch/x86/include/asm/ftrace.h
> +++ b/arch/x86/include/asm/ftrace.h
> @@ -32,6 +32,10 @@
>  #define MCOUNT_ADDR		((long)(mcount))
>  #define MCOUNT_INSN_SIZE	5 /* sizeof mcount call */
>  
> +#if defined(CONFIG_DYNAMIC_FTRACE) && defined(CONFIG_X86_64)
> +#define ARCH_SUPPORTS_FTRACE_SAVE_REGS 1
> +#endif
> +
>  #ifndef __ASSEMBLY__
>  extern void mcount(void);
>  extern int modifying_ftrace_code;
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 320852d..6d0545b 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -73,21 +73,44 @@ ENTRY(mcount)
>  	retq
>  END(mcount)
>  
> +	.macro MCOUNT_SAVE_ALL
> +	SAVE_ARGS (RSP-ORIG_RAX)
> +	SAVE_REST
> +	/* Adjust eflags, rsp and rip */
> +	movq RSP(%rsp), %rdx
> +	movq %rdx, EFLAGS(%rsp)
> +	leaq SS+8(%rsp), %rdx
> +	movq %rdx, RSP(%rsp)
> +	movq SS(%rsp), %rdi
> +	subq $MCOUNT_INSN_SIZE, %rdi
> +	movq %rdi, RIP(%rsp)
> +	.endm
> +
> +	.macro MCOUNT_RESTORE_ALL
> +	/* store eflags into regs->rsp */
> +	movq EFLAGS(%rsp), %rax
> +	movq %rax, RSP(%rsp)
> +	RESTORE_REST
> +	RESTORE_ARGS 1, (RSP-ORIG_RAX)
> +	.endm
> +
>  ENTRY(ftrace_caller)
> +	CFI_STARTPROC	simple
> +	pushfq_cfi
>  	cmpl $0, function_trace_stop
> -	jne  ftrace_stub
> +	jne  ftrace_exit
>  
> -	MCOUNT_SAVE_FRAME
> +	MCOUNT_SAVE_ALL

No! Please do not make ftrace save all regs for everyone. This is
critical. This will slow down function tracing even more. We do not need
to save all regs for every function call, unless it is asked for.

It should only save all regs if the ftrace_ops that is registered asked
for it. I had code that actually does that. I can merge your patches
with them if you would like.

Thanks,

-- Steve

>  
> -	movq 0x38(%rsp), %rdi
>  	movq 8(%rbp), %rsi
> -	subq $MCOUNT_INSN_SIZE, %rdi
> +	movq %rsp, %rdx
>  
>  GLOBAL(ftrace_call)
>  	call ftrace_stub
>  
> -	MCOUNT_RESTORE_FRAME
> +	MCOUNT_RESTORE_ALL
>  
> +	popfq_cfi
>  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
>  GLOBAL(ftrace_graph_call)
>  	jmp ftrace_stub
> @@ -95,6 +118,11 @@ GLOBAL(ftrace_graph_call)
>  
>  GLOBAL(ftrace_stub)
>  	retq
> +
> +ftrace_exit:
> +	popfq_cfi
> +	retq
> +	CFI_ENDPROC
>  END(ftrace_caller)
>  
>  #else /* ! CONFIG_DYNAMIC_FTRACE */


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/