linux-kernel - Re: [PATCH 3/6] x86,64: Simplify 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-Id: <4E1185F3020000780004BF21@nat28.tlf.novell.com>
Date:	Mon, 04 Jul 2011 08:20:51 +0100
From:	"Jan Beulich" <JBeulich@...ell.com>
To:	"Frederic Weisbecker" <fweisbec@...il.com>
Cc:	"Peter Zijlstra" <a.p.zijlstra@...llo.nl>,
	"Ingo Molnar" <mingo@...e.hu>,
	"Thomas Gleixner" <tglx@...utronix.de>,
	"Arnaldo Carvalho de Melo" <acme@...hat.com>,
	"LKML" <linux-kernel@...r.kernel.org>,
	"H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH 3/6] x86,64: Simplify save_regs()

>>> On 02.07.11 at 18:29, Frederic Weisbecker <fweisbec@...il.com> wrote:
> The save_regs function that saves the regs on low level
> irq entry is complicated because of the fact it changes
> its stack in the middle and also because it manipulates
> data allocated in the caller frame and accesses there
> are directly calculated from callee rsp value with the
> return address in the middle of the way.
> 
> This complicates the static stack offsets calculation and
> require more dynamic ones. It also needs a save/restore
> of the function's return address.
> 
> To simplify and optimize this, turn save_regs() into a
> macro.

So this got pulled out into a function a couple of releases ago,
and now it's being converted back? Wasn't the original patch's
intention to reduce the amount of duplication of generated
code?

> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Ingo Molnar <mingo@...e.hu>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: H. Peter Anvin <hpa@...or.com>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> Cc: Jan Beulich <JBeulich@...ell.com>
> ---
>  arch/x86/kernel/entry_64.S |   44 +++++++++++++++++---------------------------
>  1 files changed, 17 insertions(+), 27 deletions(-)
> 
> diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
> index 8a445a0..b6b2e85 100644
> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -297,27 +297,22 @@ ENDPROC(native_usergs_sysret64)
>  	.endm
>  
>  /* save partial stack frame */
> -	.pushsection .kprobes.text, "ax"
> -ENTRY(save_args)
> -	XCPT_FRAME
> +	.macro SAVE_ARGS_IRQ
>  	cld
> -	/*
> -	 * start from rbp in pt_regs and jump over
> -	 * return address.
> -	 */
> -	movq_cfi rdi, RDI+8-RBP
> -	movq_cfi rsi, RSI+8-RBP
> -	movq_cfi rdx, RDX+8-RBP
> -	movq_cfi rcx, RCX+8-RBP
> -	movq_cfi rax, RAX+8-RBP
> -	movq_cfi  r8,  R8+8-RBP
> -	movq_cfi  r9,  R9+8-RBP
> -	movq_cfi r10, R10+8-RBP
> -	movq_cfi r11, R11+8-RBP
> -
> -	leaq -RBP+8(%rsp),%rdi	/* arg1 for handler */
> -	movq_cfi rbp, 8		/* push %rbp */
> -	leaq 8(%rsp), %rbp		/* mov %rsp, %ebp */
> +	/* start from rbp in pt_regs and jump over */
> +	movq_cfi rdi, RDI-RBP
> +	movq_cfi rsi, RSI-RBP
> +	movq_cfi rdx, RDX-RBP
> +	movq_cfi rcx, RCX-RBP
> +	movq_cfi rax, RAX-RBP
> +	movq_cfi  r8,  R8-RBP
> +	movq_cfi  r9,  R9-RBP
> +	movq_cfi r10, R10-RBP
> +	movq_cfi r11, R11-RBP
> +
> +	leaq -RBP(%rsp),%rdi	/* arg1 for handler */
> +	movq_cfi rbp, 0		/* push %rbp */
> +	movq %rsp, %rbp
>  	testl $3, CS(%rdi)
>  	je 1f
>  	SWAPGS
> @@ -329,19 +324,14 @@ ENTRY(save_args)
>  	 */
>  1:	incl PER_CPU_VAR(irq_count)
>  	jne 2f
> -	popq_cfi %rax			/* move return address... */
>  	mov PER_CPU_VAR(irq_stack_ptr),%rsp
>  	EMPTY_FRAME 0
>  	pushq_cfi %rbp			/* backlink for unwinder */
> -	pushq_cfi %rax			/* ... to the new stack */
>  	/*
>  	 * We entered an interrupt context - irqs are off:
>  	 */
>  2:	TRACE_IRQS_OFF
> -	ret
> -	CFI_ENDPROC
> -END(save_args)
> -	.popsection
> +	.endm
>  
>  ENTRY(save_rest)
>  	PARTIAL_FRAME 1 REST_SKIP+8
> @@ -791,7 +781,7 @@ END(interrupt)
>  	/* reserve pt_regs for scratch regs and rbp */
>  	subq $ORIG_RAX-RBP, %rsp
>  	CFI_ADJUST_CFA_OFFSET ORIG_RAX-RBP
> -	call save_args
> +	SAVE_ARGS_IRQ
>  	PARTIAL_FRAME 0

With the above CFI adjustments all looking correct, this PARTIAL_FRAME
directive now is at best redundant, and hence should be removed.

Jan

>  	call \func
>  	.endm




--
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/