All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ingo Molnar <mingo@kernel.org>
To: Denys Vlasenko <dvlasenk@redhat.com>
Cc: Andy Lutomirski <luto@amacapital.net>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Steven Rostedt <rostedt@goodmis.org>,
	Borislav Petkov <bp@alien8.de>, "H. Peter Anvin" <hpa@zytor.com>,
	Oleg Nesterov <oleg@redhat.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Alexei Starovoitov <ast@plumgrid.com>,
	Will Drewry <wad@chromium.org>, Kees Cook <keescook@chromium.org>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/4] x86: save user rsp in pt_regs->sp on SYSCALL64 fastpath
Date: Tue, 10 Mar 2015 13:51:51 +0100	[thread overview]
Message-ID: <20150310125151.GB21686@gmail.com> (raw)
In-Reply-To: <1425926364-9526-4-git-send-email-dvlasenk@redhat.com>


* Denys Vlasenko <dvlasenk@redhat.com> wrote:

> PER_CPU(old_rsp) usage is simplified - now it is used only
> as temp storage, and userspace stack pointer is immediately stored
> in pt_regs->sp on syscall entry, instead of being used later,
> on syscall exit.
> 
> Instead of PER_CPU(old_rsp) and task->thread.usersp, C code
> uses pt_regs->sp now.
> 
> FIXUP/RESTORE_TOP_OF_STACK are simplified.

Just trying to judge the performance impact:

> --- a/arch/x86/kernel/entry_64.S
> +++ b/arch/x86/kernel/entry_64.S
> @@ -128,8 +128,6 @@ ENDPROC(native_usergs_sysret64)
>   * manipulation.
>   */
>  	.macro FIXUP_TOP_OF_STACK tmp offset=0
> -	movq PER_CPU_VAR(old_rsp),\tmp
> -	movq \tmp,RSP+\offset(%rsp)
>  	movq $__USER_DS,SS+\offset(%rsp)
>  	movq $__USER_CS,CS+\offset(%rsp)
>  	movq RIP+\offset(%rsp),\tmp  /* get rip */
> @@ -139,8 +137,7 @@ ENDPROC(native_usergs_sysret64)
>  	.endm
>  
>  	.macro RESTORE_TOP_OF_STACK tmp offset=0
> -	movq RSP+\offset(%rsp),\tmp
> -	movq \tmp,PER_CPU_VAR(old_rsp)
> +	/* nothing to do */
>  	.endm
>  
>  /*
> @@ -253,11 +247,13 @@ GLOBAL(system_call_after_swapgs)
>  	 */
>  	ENABLE_INTERRUPTS(CLBR_NONE)
>  	ALLOC_PT_GPREGS_ON_STACK 8		/* +8: space for orig_ax */
> +	movq	%rcx,RIP(%rsp)
> +	movq	PER_CPU_VAR(old_rsp),%rcx
> +	movq	%r11,EFLAGS(%rsp)
> +	movq	%rcx,RSP(%rsp)
> +	movq_cfi rax,ORIG_RAX
>  	SAVE_C_REGS_EXCEPT_RAX_RCX_R11
>  	movq	$-ENOSYS,RAX(%rsp)
> -	movq_cfi rax,ORIG_RAX
> -	movq	%r11,EFLAGS(%rsp)
> -	movq	%rcx,RIP(%rsp)
>  	CFI_REL_OFFSET rip,RIP
>  	testl $_TIF_WORK_SYSCALL_ENTRY,TI_flags+THREAD_INFO(%rsp,RIP)
>  	jnz tracesys

So there are now +2 instructions (5 instead of 3) in the system_call 
path, but there are -2 instructions in the SYSRETQ path, so combined 
it's a wash performance-wise, right?

Thanks,

	Ingo

  parent reply	other threads:[~2015-03-10 12:52 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-03-09 18:39 [PATCH 0/4 v2] x86: entry_64.S: steps towards simpler iret frame handling Denys Vlasenko
2015-03-09 18:39 ` [PATCH 1/4] x86: save r11 into pt_regs->flags on SYSCALL64 fastpath Denys Vlasenko
2015-03-09 20:02   ` Andy Lutomirski
2015-03-16 12:04   ` [tip:x86/asm] x86/asm/entry/64: Save R11 into pt_regs-> flags " tip-bot for Denys Vlasenko
2015-03-09 18:39 ` [PATCH 3/4] x86: save user rsp in pt_regs->sp " Denys Vlasenko
2015-03-09 20:11   ` Andy Lutomirski
2015-03-09 20:32     ` Denys Vlasenko
2015-03-09 20:43       ` Andy Lutomirski
2015-03-10 12:51   ` Ingo Molnar [this message]
2015-03-10 13:10     ` Andy Lutomirski
2015-03-10 13:18     ` Denys Vlasenko
2015-03-10 13:20       ` Andy Lutomirski
2015-03-10 13:26         ` Ingo Molnar
2015-03-10 13:21       ` Ingo Molnar
2015-03-10 13:26         ` Andy Lutomirski
2015-03-10 14:00           ` Denys Vlasenko
2015-03-10 14:02             ` Andy Lutomirski
2015-03-10 14:09               ` Denys Vlasenko
2015-03-10 13:28         ` Ingo Molnar
2015-03-10 13:50         ` Denys Vlasenko
2015-03-16  9:44           ` Ingo Molnar
2015-03-16 12:05   ` [tip:x86/asm] x86/asm/entry/64: Save user RSP in pt_regs-> sp " tip-bot for Denys Vlasenko
2015-03-10  6:00 ` [PATCH 0/4 v2] x86: entry_64.S: steps towards simpler iret frame handling Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150310125151.GB21686@gmail.com \
    --to=mingo@kernel.org \
    --cc=ast@plumgrid.com \
    --cc=bp@alien8.de \
    --cc=dvlasenk@redhat.com \
    --cc=fweisbec@gmail.com \
    --cc=hpa@zytor.com \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@amacapital.net \
    --cc=oleg@redhat.com \
    --cc=rostedt@goodmis.org \
    --cc=torvalds@linux-foundation.org \
    --cc=wad@chromium.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.