Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1727778 > unrolled thread

[RFC 00/17] Pile o' entry stack changes

Started byAndy Lutomirski <luto@kernel.org>
First post2017-09-06 23:50 +0200
Last post2017-09-07 09:10 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel


Contents

  [RFC 00/17] Pile o' entry stack changes Andy Lutomirski <luto@kernel.org> - 2017-09-06 23:50 +0200
    [RFC 04/17] x86/asm/64: Simplify reg restore code in the standard IRET paths Andy Lutomirski <luto@kernel.org> - 2017-09-06 23:50 +0200
    [RFC 03/17] x86/asm/64: Move SWAPGS into the common iret-to-usermode path Andy Lutomirski <luto@kernel.org> - 2017-09-06 23:50 +0200
    [RFC 02/17] x86/asm/64: Split the iret-to-user and iret-to-kernel paths Andy Lutomirski <luto@kernel.org> - 2017-09-06 23:50 +0200
    Re: [RFC 00/17] Pile o' entry stack changes Andi Kleen <andi@firstfloor.org> - 2017-09-07 00:20 +0200
      Re: [RFC 00/17] Pile o' entry stack changes Andy Lutomirski <luto@kernel.org> - 2017-09-07 02:10 +0200
        Re: [RFC 00/17] Pile o' entry stack changes Ingo Molnar <mingo@kernel.org> - 2017-09-07 09:10 +0200

#1727778 — [RFC 00/17] Pile o' entry stack changes

FromAndy Lutomirski <luto@kernel.org>
Date2017-09-06 23:50 +0200
Subject[RFC 00/17] Pile o' entry stack changes
Message-ID<umQdc-7qi-3@gated-at.bofh.it>
Hi all-

Here's a pile of entry changes.  In brief summary:

 - Lots of people (Linus included) have asked to convert the entry
   code to pop registers on exit instead of movqing them off the
   stack.  This makes a bunch of progress in that direction.

 - Linux's sp0 handling has annoyed me for a while.  We have
   thread_struct::sp0, which never made much sense to me.  This
   series removes it on x86_64 and removes most references on
   x86_32.

 - Xen PV's cpuinit code did incomprehensible things with stack
   pointers.  This makes it comprehensible.

Juergen, this needs a bit of help on Xen -- see the NMI patch for details.

Reviews would be appreciated :)

Andy Lutomirski (17):
  x86/asm/64: Remove the restore_c_regs_and_iret label
  x86/asm/64: Split the iret-to-user and iret-to-kernel paths
  x86/asm/64: Move SWAPGS into the common iret-to-usermode path
  x86/asm/64: Simplify reg restore code in the standard IRET paths
  x86/asm/64: Shrink paranoid_exit_restore and make labels local
  x86/asm/64: Use pop instead of movq in syscall_return_via_sysret
  x86/asm/64: Merge the fast and slow SYSRET paths
  x86/asm/64: De-Xen-ify our NMI code
  x86/asm/32: Pull MSR_IA32_SYSENTER_CS update code out of
    native_load_sp0()
  x86/asm/64: Pass sp0 directly to load_sp0()
  x86/asm: Add task_top_of_stack() to find the top of a task's stack
  x86/xen/64: Clean up SP code in cpu_initialize_context()
  x86/boot/64: Stop initializing TSS.sp0 at boot
  x86/asm/64: Remove all remaining direct thread_struct::sp0 reads
  x86/boot/32: Fix cpu_current_top_of_stack initialization at boot
  x86/asm/64: Remove thread_struct::sp0
  x86/traps: Use a new on_thread_stack() helper to clean up an assertion

 arch/x86/entry/calling.h              |   9 +++
 arch/x86/entry/entry_64.S             | 133 ++++++++++++++++++----------------
 arch/x86/entry/entry_64_compat.S      |   3 +-
 arch/x86/include/asm/compat.h         |   1 +
 arch/x86/include/asm/paravirt.h       |   5 +-
 arch/x86/include/asm/paravirt_types.h |   2 +-
 arch/x86/include/asm/processor.h      |  68 +++++++++--------
 arch/x86/include/asm/switch_to.h      |  23 ++++++
 arch/x86/include/asm/thread_info.h    |  11 ---
 arch/x86/kernel/cpu/common.c          |  12 ++-
 arch/x86/kernel/head_64.S             |   2 +-
 arch/x86/kernel/process.c             |   3 +-
 arch/x86/kernel/process_32.c          |   3 +-
 arch/x86/kernel/process_64.c          |   5 +-
 arch/x86/kernel/smpboot.c             |   3 +-
 arch/x86/kernel/traps.c               |   3 +-
 arch/x86/kernel/vm86_32.c             |  14 ++--
 arch/x86/lguest/boot.c                |   7 +-
 arch/x86/xen/enlighten_pv.c           |   7 +-
 arch/x86/xen/smp_pv.c                 |  17 ++++-
 20 files changed, 192 insertions(+), 139 deletions(-)

-- 
2.13.5

[toc] | [next] | [standalone]


#1727779 — [RFC 04/17] x86/asm/64: Simplify reg restore code in the standard IRET paths

FromAndy Lutomirski <luto@kernel.org>
Date2017-09-06 23:50 +0200
Subject[RFC 04/17] x86/asm/64: Simplify reg restore code in the standard IRET paths
Message-ID<umQmS-7tQ-21@gated-at.bofh.it>
In reply to#1727778
The old code restored all the registers with movq instead of pop.
In theory, this was done because some CPUs have higher movq
throughput, but any gain there would be tiny and is almost certainly
outweighed by the higher text size.

This saves 96 bytes of text.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/calling.h  |  9 +++++++++
 arch/x86/entry/entry_64.S | 28 ++++++++++++++++++++++------
 2 files changed, 31 insertions(+), 6 deletions(-)

diff --git a/arch/x86/entry/calling.h b/arch/x86/entry/calling.h
index 05ed3d393da7..0a2c73fe2cfc 100644
--- a/arch/x86/entry/calling.h
+++ b/arch/x86/entry/calling.h
@@ -147,6 +147,15 @@ For 32-bit we have the following conventions - kernel is built with
 	movq 5*8+\offset(%rsp), %rbx
 	.endm
 
+	.macro POP_EXTRA_REGS
+	popq %r15
+	popq %r14
+	popq %r13
+	popq %r12
+	popq %rbp
+	popq %rbx
+	.endm
+
 	.macro RESTORE_C_REGS_HELPER rstor_rax=1, rstor_rcx=1, rstor_r11=1, rstor_r8910=1, rstor_rdx=1
 	.if \rstor_r11
 	movq 6*8(%rsp), %r11
diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 2cd01ed9cd59..7f1a83b17b4a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -521,9 +521,17 @@ GLOBAL(retint_user)
 
 GLOBAL(swapgs_restore_regs_and_return_to_usermode)
 	SWAPGS
-	RESTORE_EXTRA_REGS
-	RESTORE_C_REGS
-	REMOVE_PT_GPREGS_FROM_STACK 8
+	POP_EXTRA_REGS
+	popq	%r11
+	popq	%r10
+	popq	%r9
+	popq	%r8
+	popq	%rax
+	popq	%rcx
+	popq	%rdx
+	popq	%rsi
+	popq	%rdi
+	addq	$8, %rsp
 	INTERRUPT_RETURN
 
 
@@ -546,9 +554,17 @@ retint_kernel:
 	TRACE_IRQS_IRETQ
 
 GLOBAL(restore_regs_and_return_to_kernel)
-	RESTORE_EXTRA_REGS
-	RESTORE_C_REGS
-	REMOVE_PT_GPREGS_FROM_STACK 8
+	POP_EXTRA_REGS
+	popq	%r11
+	popq	%r10
+	popq	%r9
+	popq	%r8
+	popq	%rax
+	popq	%rcx
+	popq	%rdx
+	popq	%rsi
+	popq	%rdi
+	addq	$8, %rsp
 	INTERRUPT_RETURN
 
 ENTRY(native_iret)
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1727782 — [RFC 03/17] x86/asm/64: Move SWAPGS into the common iret-to-usermode path

FromAndy Lutomirski <luto@kernel.org>
Date2017-09-06 23:50 +0200
Subject[RFC 03/17] x86/asm/64: Move SWAPGS into the common iret-to-usermode path
Message-ID<umQmS-7tQ-27@gated-at.bofh.it>
In reply to#1727778
All of the code paths that ended up doing IRET to usermode did
SWAPGS immediately beforehand.  Move the SWAPGS into the common
code.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/entry_64.S        | 26 ++++++++++----------------
 arch/x86/entry/entry_64_compat.S |  3 +--
 2 files changed, 11 insertions(+), 18 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 34156d50c3d6..2cd01ed9cd59 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -249,7 +249,7 @@ return_from_SYSCALL_64:
 	movq	RCX(%rsp), %rcx
 	movq	RIP(%rsp), %r11
 	cmpq	%rcx, %r11			/* RCX == RIP */
-	jne	opportunistic_sysret_failed
+	jne	swapgs_restore_regs_and_return_to_usermode
 
 	/*
 	 * On Intel CPUs, SYSRET with non-canonical RCX/RIP will #GP
@@ -267,14 +267,14 @@ return_from_SYSCALL_64:
 
 	/* If this changed %rcx, it was not canonical */
 	cmpq	%rcx, %r11
-	jne	opportunistic_sysret_failed
+	jne	swapgs_restore_regs_and_return_to_usermode
 
 	cmpq	$__USER_CS, CS(%rsp)		/* CS must match SYSRET */
-	jne	opportunistic_sysret_failed
+	jne	swapgs_restore_regs_and_return_to_usermode
 
 	movq	R11(%rsp), %r11
 	cmpq	%r11, EFLAGS(%rsp)		/* R11 == RFLAGS */
-	jne	opportunistic_sysret_failed
+	jne	swapgs_restore_regs_and_return_to_usermode
 
 	/*
 	 * SYSCALL clears RF when it saves RFLAGS in R11 and SYSRET cannot
@@ -295,12 +295,12 @@ return_from_SYSCALL_64:
 	 * would never get past 'stuck_here'.
 	 */
 	testq	$(X86_EFLAGS_RF|X86_EFLAGS_TF), %r11
-	jnz	opportunistic_sysret_failed
+	jnz	swapgs_restore_regs_and_return_to_usermode
 
 	/* nothing to check for RSP */
 
 	cmpq	$__USER_DS, SS(%rsp)		/* SS must match SYSRET */
-	jne	opportunistic_sysret_failed
+	jne	swapgs_restore_regs_and_return_to_usermode
 
 	/*
 	 * We win! This label is here just for ease of understanding
@@ -312,10 +312,6 @@ syscall_return_via_sysret:
 	RESTORE_C_REGS_EXCEPT_RCX_R11
 	movq	RSP(%rsp), %rsp
 	USERGS_SYSRET64
-
-opportunistic_sysret_failed:
-	SWAPGS
-	jmp	restore_regs_and_return_to_usermode
 END(entry_SYSCALL_64)
 
 ENTRY(stub_ptregs_64)
@@ -411,8 +407,7 @@ ENTRY(ret_from_fork)
 	movq	%rsp, %rdi
 	call	syscall_return_slowpath	/* returns with IRQs disabled */
 	TRACE_IRQS_ON			/* user mode is traced as IRQS on */
-	SWAPGS
-	jmp	restore_regs_and_return_to_usermode
+	jmp	swapgs_restore_regs_and_return_to_usermode
 
 1:
 	/* kernel thread */
@@ -523,9 +518,9 @@ GLOBAL(retint_user)
 	mov	%rsp,%rdi
 	call	prepare_exit_to_usermode
 	TRACE_IRQS_IRETQ
-	SWAPGS
 
-GLOBAL(restore_regs_and_return_to_usermode)
+GLOBAL(swapgs_restore_regs_and_return_to_usermode)
+	SWAPGS
 	RESTORE_EXTRA_REGS
 	RESTORE_C_REGS
 	REMOVE_PT_GPREGS_FROM_STACK 8
@@ -1265,8 +1260,7 @@ ENTRY(nmi)
 	 * Return back to user mode.  We must *not* do the normal exit
 	 * work, because we don't want to enable interrupts.
 	 */
-	SWAPGS
-	jmp	restore_regs_and_return_to_usermode
+	jmp	swapgs_restore_regs_and_return_to_usermode
 
 .Lnmi_from_kernel:
 	/*
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index e4ab8ffa94b1..72e143497ece 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -337,8 +337,7 @@ ENTRY(entry_INT80_compat)
 
 	/* Go back to user mode. */
 	TRACE_IRQS_ON
-	SWAPGS
-	jmp	restore_regs_and_return_to_usermode
+	jmp	swapgs_restore_regs_and_return_to_usermode
 END(entry_INT80_compat)
 
 	ALIGN
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1727783 — [RFC 02/17] x86/asm/64: Split the iret-to-user and iret-to-kernel paths

FromAndy Lutomirski <luto@kernel.org>
Date2017-09-06 23:50 +0200
Subject[RFC 02/17] x86/asm/64: Split the iret-to-user and iret-to-kernel paths
Message-ID<umQmS-7tQ-25@gated-at.bofh.it>
In reply to#1727778
These code paths will diverge soon.

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 arch/x86/entry/entry_64.S        | 20 +++++++++++---------
 arch/x86/entry/entry_64_compat.S |  2 +-
 arch/x86/kernel/head_64.S        |  2 +-
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index a0ebeeb9d12f..34156d50c3d6 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -315,7 +315,7 @@ syscall_return_via_sysret:
 
 opportunistic_sysret_failed:
 	SWAPGS
-	jmp	restore_regs_and_iret
+	jmp	restore_regs_and_return_to_usermode
 END(entry_SYSCALL_64)
 
 ENTRY(stub_ptregs_64)
@@ -412,7 +412,7 @@ ENTRY(ret_from_fork)
 	call	syscall_return_slowpath	/* returns with IRQs disabled */
 	TRACE_IRQS_ON			/* user mode is traced as IRQS on */
 	SWAPGS
-	jmp	restore_regs_and_iret
+	jmp	restore_regs_and_return_to_usermode
 
 1:
 	/* kernel thread */
@@ -524,7 +524,13 @@ GLOBAL(retint_user)
 	call	prepare_exit_to_usermode
 	TRACE_IRQS_IRETQ
 	SWAPGS
-	jmp	restore_regs_and_iret
+
+GLOBAL(restore_regs_and_return_to_usermode)
+	RESTORE_EXTRA_REGS
+	RESTORE_C_REGS
+	REMOVE_PT_GPREGS_FROM_STACK 8
+	INTERRUPT_RETURN
+
 
 /* Returning to kernel space */
 retint_kernel:
@@ -544,11 +550,7 @@ retint_kernel:
 	 */
 	TRACE_IRQS_IRETQ
 
-/*
- * At this label, code paths which return to kernel and to user,
- * which come from interrupts/exception and from syscalls, merge.
- */
-GLOBAL(restore_regs_and_iret)
+GLOBAL(restore_regs_and_return_to_kernel)
 	RESTORE_EXTRA_REGS
 	RESTORE_C_REGS
 	REMOVE_PT_GPREGS_FROM_STACK 8
@@ -1264,7 +1266,7 @@ ENTRY(nmi)
 	 * work, because we don't want to enable interrupts.
 	 */
 	SWAPGS
-	jmp	restore_regs_and_iret
+	jmp	restore_regs_and_return_to_usermode
 
 .Lnmi_from_kernel:
 	/*
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index 5314d7b8e5ad..e4ab8ffa94b1 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -338,7 +338,7 @@ ENTRY(entry_INT80_compat)
 	/* Go back to user mode. */
 	TRACE_IRQS_ON
 	SWAPGS
-	jmp	restore_regs_and_iret
+	jmp	restore_regs_and_return_to_usermode
 END(entry_INT80_compat)
 
 	ALIGN
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 6225550883df..63fa56ead56e 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -310,7 +310,7 @@ early_idt_handler_common:
 
 20:
 	decl early_recursion_flag(%rip)
-	jmp restore_regs_and_iret
+	jmp restore_regs_and_return_to_kernel
 ENDPROC(early_idt_handler_common)
 
 	__INITDATA
-- 
2.13.5

[toc] | [prev] | [next] | [standalone]


#1727787

FromAndi Kleen <andi@firstfloor.org>
Date2017-09-07 00:20 +0200
Message-ID<umQPT-7Vj-1@gated-at.bofh.it>
In reply to#1727778
Andy Lutomirski <luto@kernel.org> writes:
>
>  - Lots of people (Linus included) have asked to convert the entry
>    code to pop registers on exit instead of movqing them off the
>    stack.  This makes a bunch of progress in that direction.

You should benchmark it on Atoms. Likely it's a regression there
because they don't have the special PUSH/POP acceleration.

-Andi

[toc] | [prev] | [next] | [standalone]


#1727831

FromAndy Lutomirski <luto@kernel.org>
Date2017-09-07 02:10 +0200
Message-ID<umSyl-Bv-5@gated-at.bofh.it>
In reply to#1727787
On Wed, Sep 6, 2017 at 3:16 PM, Andi Kleen <andi@firstfloor.org> wrote:
> Andy Lutomirski <luto@kernel.org> writes:
>>
>>  - Lots of people (Linus included) have asked to convert the entry
>>    code to pop registers on exit instead of movqing them off the
>>    stack.  This makes a bunch of progress in that direction.
>
> You should benchmark it on Atoms. Likely it's a regression there
> because they don't have the special PUSH/POP acceleration.

I'm not entirely sure this is a worthwhile reason.  Atom will lose a
few cycles due to POP throughput, but there's a lot less decode
bandwidth needed and we save a cache line or two.

--Andy

[toc] | [prev] | [next] | [standalone]


#1727958

FromIngo Molnar <mingo@kernel.org>
Date2017-09-07 09:10 +0200
Message-ID<umZ6O-56x-13@gated-at.bofh.it>
In reply to#1727831
* Andy Lutomirski <luto@kernel.org> wrote:

> On Wed, Sep 6, 2017 at 3:16 PM, Andi Kleen <andi@firstfloor.org> wrote:
> > Andy Lutomirski <luto@kernel.org> writes:
> >>
> >>  - Lots of people (Linus included) have asked to convert the entry
> >>    code to pop registers on exit instead of movqing them off the
> >>    stack.  This makes a bunch of progress in that direction.
> >
> > You should benchmark it on Atoms. Likely it's a regression there
> > because they don't have the special PUSH/POP acceleration.
> 
> I'm not entirely sure this is a worthwhile reason.  Atom will lose a
> few cycles due to POP throughput, but there's a lot less decode
> bandwidth needed and we save a cache line or two.

I think we can also safely assume that Atom will eventually either join the
21st century or die out - mild Atom micro-costs are not a good reason to
complicate the entry code...

Thanks,

	Ingo

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web