x86_64: LRET to userspace

PATCH In review Andy Lutomirski 3 participants 6 messages Jul 23, 2014 – Jul 29, 2014

Subsystems (3)

Stable branch x86 PARAVIRT_OPS interface

lore

Share & tools

lore

Andy Lutomirski

author

2014-07-23 15:34 UTC #0 lore raw

cover x86_64: LRET to userspace

This series is based on tip/perf/urgent. It should apply to any recent kernel, but my test case [1] will OOPS without the fix in tip/perf/urgent.

The first patch or something like it is needed for 3.16, regardless of any lret hackery -- I think it's making its way toward a pull request this week, but I haven't seen it land in -tip yet.

I'm repeating it here because the LRET stuff won't work on paravirt, and patch 1 puts the machinery for that in place.

tl;dr Read patch 2. To test, apply patch 1 and consider basing on tip/perf/urgent.

[1] sigreturn_32 from https://gitorious.org/linux-test-utils/linux-clock-tests/

Andy Lutomirski (2):
x86_64,entry,xen: Do not invoke espfix64 on Xen x86_64,entry: Use lret to return to userspace when possible

 arch/x86/include/asm/irqflags.h     |   3 +-
 arch/x86/include/asm/paravirt.h     |   4 ++
 arch/x86/include/asm/traps.h        |   6 ++
 arch/x86/kernel/cpu/mcheck/mce.c    |   2 +
 arch/x86/kernel/entry_64.S          | 121 ++++++++++++++++++++++++++++++------
 arch/x86/kernel/nmi.c               |  21 +++++++
 arch/x86/kernel/paravirt_patch_64.c |   2 -
 7 files changed, 136 insertions(+), 23 deletions(-)

Signature · 1 line

1.9.3

Andy Lutomirski

author

2014-07-23 15:34 UTC #1 lore raw

PATCH 1/2 x86_64,entry,xen: Do not invoke espfix64 on Xen

This moves the espfix64 logic into native_iret. To make this work, it gets rid of the native patch for INTERRUPT_RETURN:
INTERRUPT_RETURN on native kernels is now 'jmp native_iret'.

This changes the 16-bit SS behavior on Xen from OOPSing to leaking some bits of the Xen hypervisor's RSP (I think).

Signed-off-by Andy Lutomirski <[email protected]>

---
 arch/x86/include/asm/irqflags.h     |  2 +-
 arch/x86/kernel/entry_64.S          | 28 ++++++++++------------------
 arch/x86/kernel/paravirt_patch_64.c |  2 --
 3 files changed, 11 insertions(+), 21 deletions(-)

arch/x86/include/asm/irqflags.h +1 -1

index bba3cf8..0a8b519 100644
@@ -129,7 +129,7 @@ static inline notrace unsigned long arch_local_irq_save(void)
 
 #define PARAVIRT_ADJUST_EXCEPTION_FRAME	/*  */
 
-#define INTERRUPT_RETURN	iretq
+#define INTERRUPT_RETURN	jmp native_iret
 #define USERGS_SYSRET64				\
 	swapgs;					\
 	sysretq;

arch/x86/kernel/entry_64.S +10 ~~-18~~

index b25ca96..c844f08 100644
@@ -830,27 +830,24 @@ restore_args:
 	RESTORE_ARGS 1,8,1
 
 irq_return:
+	INTERRUPT_RETURN
+
+ENTRY(native_iret)
 	/*
 	 * Are we returning to a stack segment from the LDT?  Note: in
 	 * 64-bit mode SS:RSP on the exception stack is always valid.
 	 */
 #ifdef CONFIG_X86_ESPFIX64
 	testb $4,(SS-RIP)(%rsp)
-	jnz irq_return_ldt
+	jnz native_irq_return_ldt
 #endif
 
-irq_return_iret:
-	INTERRUPT_RETURN
-	_ASM_EXTABLE(irq_return_iret, bad_iret)
-
-#ifdef CONFIG_PARAVIRT
-ENTRY(native_iret)
+native_irq_return_iret:
 	iretq
-	_ASM_EXTABLE(native_iret, bad_iret)
-#endif
+	_ASM_EXTABLE(native_irq_return_iret, bad_iret)
 
 #ifdef CONFIG_X86_ESPFIX64
-irq_return_ldt:
+native_irq_return_ldt:
 	pushq_cfi %rax
 	pushq_cfi %rdi
 	SWAPGS

@@ -872,7 +869,7 @@ irq_return_ldt:
 	SWAPGS
 	movq %rax,%rsp
 	popq_cfi %rax
-	jmp irq_return_iret
+	jmp native_irq_return_iret
 #endif
 
 	.section .fixup,"ax"

@@ -956,13 +953,8 @@ __do_double_fault:
 	cmpl $__KERNEL_CS,CS(%rdi)
 	jne do_double_fault
 	movq RIP(%rdi),%rax
-	cmpq $irq_return_iret,%rax
-#ifdef CONFIG_PARAVIRT
-	je 1f
-	cmpq $native_iret,%rax
-#endif
+	cmpq $native_irq_return_iret,%rax
 	jne do_double_fault		/* This shouldn't happen... */
-1:
 	movq PER_CPU_VAR(kernel_stack),%rax
 	subq $(6*8-KERNEL_STACK_OFFSET),%rax	/* Reset to original stack */
 	movq %rax,RSP(%rdi)

@@ -1428,7 +1420,7 @@ error_sti:
  */
 error_kernelspace:
 	incl %ebx
-	leaq irq_return_iret(%rip),%rcx
+	leaq native_irq_return_iret(%rip),%rcx
 	cmpq %rcx,RIP+8(%rsp)
 	je error_swapgs
 	movl %ecx,%eax	/* zero extend */

arch/x86/kernel/paravirt_patch_64.c +0 -2

index 3f08f34..a1da673 100644
@@ -6,7 +6,6 @@ DEF_NATIVE(pv_irq_ops, irq_disable, "cli");
 DEF_NATIVE(pv_irq_ops, irq_enable, "sti");
 DEF_NATIVE(pv_irq_ops, restore_fl, "pushq %rdi; popfq");
 DEF_NATIVE(pv_irq_ops, save_fl, "pushfq; popq %rax");
-DEF_NATIVE(pv_cpu_ops, iret, "iretq");
 DEF_NATIVE(pv_mmu_ops, read_cr2, "movq %cr2, %rax");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "movq %cr3, %rax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "movq %rdi, %cr3");

@@ -50,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 		PATCH_SITE(pv_irq_ops, save_fl);
 		PATCH_SITE(pv_irq_ops, irq_enable);
 		PATCH_SITE(pv_irq_ops, irq_disable);
-		PATCH_SITE(pv_cpu_ops, iret);
 		PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
 		PATCH_SITE(pv_cpu_ops, usergs_sysret32);
 		PATCH_SITE(pv_cpu_ops, usergs_sysret64);

Signature · 1 line

1.9.3

tip-bot for Andy Lutomirski

merge bot

2014-07-28 22:33 UTC #2 lore raw

[tip:x86/urgent] x86_64/entry/xen: Do not invoke espfix64 on Xen

x86_64/entry/xen: Do not invoke espfix64 on Xen

Commit-ID:  7209a75d2009dbf7745e2fd354abf25c3deb3ca3
Gitweb:     http://git.kernel.org/tip/7209a75d2009dbf7745e2fd354abf25c3deb3ca3
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Wed, 23 Jul 2014 08:34:11 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 28 Jul 2014 15:25:40 -0700

x86_64/entry/xen: Do not invoke espfix64 on Xen

This moves the espfix64 logic into native_iret. To make this work, it gets rid of the native patch for INTERRUPT_RETURN:
INTERRUPT_RETURN on native kernels is now 'jmp native_iret'.

This changes the 16-bit SS behavior on Xen from OOPSing to leaking some bits of the Xen hypervisor's RSP (I think).

[ hpa: this is a nonzero cost on native, but probably not enough to
measure. Xen needs to fix this in their own code, probably doing something equivalent to espfix64. ]

Signed-off-by Andy Lutomirski <[email protected]>

Link http://lkml.kernel.org/r/7b8f1d8ef6597cb16ae004a43c56980a7de3cf94.1406129132.git.luto@amacapital.net open here

Signed-off-by H. Peter Anvin <[email protected]>

Cc <[email protected]>

---
 arch/x86/include/asm/irqflags.h     |  2 +-
 arch/x86/kernel/entry_64.S          | 28 ++++++++++------------------
 arch/x86/kernel/paravirt_patch_64.c |  2 --
 3 files changed, 11 insertions(+), 21 deletions(-)

arch/x86/include/asm/irqflags.h +1 -1

index bba3cf8..0a8b519 100644
@@ -129,7 +129,7 @@ static inline notrace unsigned long arch_local_irq_save(void)
 
 #define PARAVIRT_ADJUST_EXCEPTION_FRAME	/*  */
 
-#define INTERRUPT_RETURN	iretq
+#define INTERRUPT_RETURN	jmp native_iret
 #define USERGS_SYSRET64				\
 	swapgs;					\
 	sysretq;

arch/x86/kernel/entry_64.S +10 ~~-18~~

index b25ca96..c844f08 100644
@@ -830,27 +830,24 @@ restore_args:
 	RESTORE_ARGS 1,8,1
 
 irq_return:
+	INTERRUPT_RETURN
+
+ENTRY(native_iret)
 	/*
 	 * Are we returning to a stack segment from the LDT?  Note: in
 	 * 64-bit mode SS:RSP on the exception stack is always valid.
 	 */
 #ifdef CONFIG_X86_ESPFIX64
 	testb $4,(SS-RIP)(%rsp)
-	jnz irq_return_ldt
+	jnz native_irq_return_ldt
 #endif
 
-irq_return_iret:
-	INTERRUPT_RETURN
-	_ASM_EXTABLE(irq_return_iret, bad_iret)
-
-#ifdef CONFIG_PARAVIRT
-ENTRY(native_iret)
+native_irq_return_iret:
 	iretq
-	_ASM_EXTABLE(native_iret, bad_iret)
-#endif
+	_ASM_EXTABLE(native_irq_return_iret, bad_iret)
 
 #ifdef CONFIG_X86_ESPFIX64
-irq_return_ldt:
+native_irq_return_ldt:
 	pushq_cfi %rax
 	pushq_cfi %rdi
 	SWAPGS

@@ -872,7 +869,7 @@ irq_return_ldt:
 	SWAPGS
 	movq %rax,%rsp
 	popq_cfi %rax
-	jmp irq_return_iret
+	jmp native_irq_return_iret
 #endif
 
 	.section .fixup,"ax"

@@ -956,13 +953,8 @@ __do_double_fault:
 	cmpl $__KERNEL_CS,CS(%rdi)
 	jne do_double_fault
 	movq RIP(%rdi),%rax
-	cmpq $irq_return_iret,%rax
-#ifdef CONFIG_PARAVIRT
-	je 1f
-	cmpq $native_iret,%rax
-#endif
+	cmpq $native_irq_return_iret,%rax
 	jne do_double_fault		/* This shouldn't happen... */
-1:
 	movq PER_CPU_VAR(kernel_stack),%rax
 	subq $(6*8-KERNEL_STACK_OFFSET),%rax	/* Reset to original stack */
 	movq %rax,RSP(%rdi)

@@ -1428,7 +1420,7 @@ error_sti:
  */
 error_kernelspace:
 	incl %ebx
-	leaq irq_return_iret(%rip),%rcx
+	leaq native_irq_return_iret(%rip),%rcx
 	cmpq %rcx,RIP+8(%rsp)
 	je error_swapgs
 	movl %ecx,%eax	/* zero extend */

arch/x86/kernel/paravirt_patch_64.c +0 -2

index 3f08f34..a1da673 100644
@@ -6,7 +6,6 @@ DEF_NATIVE(pv_irq_ops, irq_disable, "cli");
 DEF_NATIVE(pv_irq_ops, irq_enable, "sti");
 DEF_NATIVE(pv_irq_ops, restore_fl, "pushq %rdi; popfq");
 DEF_NATIVE(pv_irq_ops, save_fl, "pushfq; popq %rax");
-DEF_NATIVE(pv_cpu_ops, iret, "iretq");
 DEF_NATIVE(pv_mmu_ops, read_cr2, "movq %cr2, %rax");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "movq %cr3, %rax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "movq %rdi, %cr3");

@@ -50,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
 		PATCH_SITE(pv_irq_ops, save_fl);
 		PATCH_SITE(pv_irq_ops, irq_enable);
 		PATCH_SITE(pv_irq_ops, irq_disable);
-		PATCH_SITE(pv_cpu_ops, iret);
 		PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
 		PATCH_SITE(pv_cpu_ops, usergs_sysret32);
 		PATCH_SITE(pv_cpu_ops, usergs_sysret64);

Andy Lutomirski

author

2014-07-29 03:39 UTC #3 lore raw

Re: [tip:x86/urgent] x86_64/entry/xen: Do not invoke espfix64 on Xen

Quoted 13 lines · tip-bot for Andy Lutomirski

Commit-ID:  7209a75d2009dbf7745e2fd354abf25c3deb3ca3
Gitweb:     http://git.kernel.org/tip/7209a75d2009dbf7745e2fd354abf25c3deb3ca3
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Wed, 23 Jul 2014 08:34:11 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 28 Jul 2014 15:25:40 -0700
x86_64/entry/xen: Do not invoke espfix64 on Xen
This moves the espfix64 logic into native_iret. To make this work, it gets rid of the native patch for INTERRUPT_RETURN:
INTERRUPT_RETURN on native kernels is now 'jmp native_iret'.

Quoted 6 lines

This changes the 16-bit SS behavior on Xen from OOPSing to leaking some bits of the Xen hypervisor's RSP (I think).
[ hpa: this is a nonzero cost on native, but probably not enough to
measure. Xen needs to fix this in their own code, probably doing something equivalent to espfix64. ]

Any plan to either fix or remove the buggy bsp-only initialization of espfix64 on paravirt? It's harmless now, but IMO it's still ugly. Or should this wait for 3.17?

--Andy

Quoted 116 lines

Signed-off-by Andy Lutomirski <[email protected]>

Link http://lkml.kernel.org/r/7b8f1d8ef6597cb16ae004a43c56980a7de3cf94.1406129132.git.luto@amacapital.net open here

Signed-off-by H. Peter Anvin <[email protected]>

Cc <[email protected]>

---
 arch/x86/include/asm/irqflags.h     |  2 +-
 arch/x86/kernel/entry_64.S          | 28 ++++++++++------------------
 arch/x86/kernel/paravirt_patch_64.c |  2 --
 3 files changed, 11 insertions(+), 21 deletions(-)

arch/x86/include/asm/irqflags.h +1 -1

index bba3cf8..0a8b519 100644
@@ -129,7 +129,7 @@ static inline notrace unsigned long arch_local_irq_save(void)
 
 #define PARAVIRT_ADJUST_EXCEPTION_FRAME        /*  */
 
-#define INTERRUPT_RETURN       iretq
+#define INTERRUPT_RETURN       jmp native_iret
 #define USERGS_SYSRET64                                \
        swapgs;                                 \
        sysretq;

arch/x86/kernel/entry_64.S +10 ~~-18~~

index b25ca96..c844f08 100644
@@ -830,27 +830,24 @@ restore_args:
        RESTORE_ARGS 1,8,1
 
 irq_return:
+       INTERRUPT_RETURN
+
+ENTRY(native_iret)
        /*
         * Are we returning to a stack segment from the LDT?  Note: in
         * 64-bit mode SS:RSP on the exception stack is always valid.
         */
 #ifdef CONFIG_X86_ESPFIX64
        testb $4,(SS-RIP)(%rsp)
-       jnz irq_return_ldt
+       jnz native_irq_return_ldt
 #endif
 
-irq_return_iret:
-       INTERRUPT_RETURN
-       _ASM_EXTABLE(irq_return_iret, bad_iret)
-
-#ifdef CONFIG_PARAVIRT
-ENTRY(native_iret)
+native_irq_return_iret:
        iretq
-       _ASM_EXTABLE(native_iret, bad_iret)
-#endif
+       _ASM_EXTABLE(native_irq_return_iret, bad_iret)
 
 #ifdef CONFIG_X86_ESPFIX64
-irq_return_ldt:
+native_irq_return_ldt:
        pushq_cfi %rax
        pushq_cfi %rdi
        SWAPGS

@@ -872,7 +869,7 @@ irq_return_ldt:
        SWAPGS
        movq %rax,%rsp
        popq_cfi %rax
-       jmp irq_return_iret
+       jmp native_irq_return_iret
 #endif
 
        .section .fixup,"ax"

@@ -956,13 +953,8 @@ __do_double_fault:
        cmpl $__KERNEL_CS,CS(%rdi)
        jne do_double_fault
        movq RIP(%rdi),%rax
-       cmpq $irq_return_iret,%rax
-#ifdef CONFIG_PARAVIRT
-       je 1f
-       cmpq $native_iret,%rax
-#endif
+       cmpq $native_irq_return_iret,%rax
        jne do_double_fault             /* This shouldn't happen... */
-1:
        movq PER_CPU_VAR(kernel_stack),%rax
        subq $(6*8-KERNEL_STACK_OFFSET),%rax    /* Reset to original stack */
        movq %rax,RSP(%rdi)

@@ -1428,7 +1420,7 @@ error_sti:
  */
 error_kernelspace:
        incl %ebx
-       leaq irq_return_iret(%rip),%rcx
+       leaq native_irq_return_iret(%rip),%rcx
        cmpq %rcx,RIP+8(%rsp)
        je error_swapgs
        movl %ecx,%eax  /* zero extend */

arch/x86/kernel/paravirt_patch_64.c +0 -2

index 3f08f34..a1da673 100644
@@ -6,7 +6,6 @@ DEF_NATIVE(pv_irq_ops, irq_disable, "cli");
 DEF_NATIVE(pv_irq_ops, irq_enable, "sti");
 DEF_NATIVE(pv_irq_ops, restore_fl, "pushq %rdi; popfq");
 DEF_NATIVE(pv_irq_ops, save_fl, "pushfq; popq %rax");
-DEF_NATIVE(pv_cpu_ops, iret, "iretq");
 DEF_NATIVE(pv_mmu_ops, read_cr2, "movq %cr2, %rax");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "movq %cr3, %rax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "movq %rdi, %cr3");

@@ -50,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void *ibuf,
                PATCH_SITE(pv_irq_ops, save_fl);
                PATCH_SITE(pv_irq_ops, irq_enable);
                PATCH_SITE(pv_irq_ops, irq_disable);
-               PATCH_SITE(pv_cpu_ops, iret);
                PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
                PATCH_SITE(pv_cpu_ops, usergs_sysret32);
                PATCH_SITE(pv_cpu_ops, usergs_sysret64);

Signature · 2 lines

Andy Lutomirski
AMA Capital Management, LLC

H. Peter Anvin

reviewer

2014-07-29 03:47 UTC #4 lore raw

Re: [tip:x86/urgent] x86_64/entry/xen: Do not invoke espfix64 on Xen

Would like to absolutely minimize changes at this point.

Quoted 154 lines · Andy Lutomirski

Quoted 2 lines · tip-bot for Andy Lutomirski
Commit-ID: 7209a75d2009dbf7745e2fd354abf25c3deb3ca3 Gitweb:
http://git.kernel.org/tip/7209a75d2009dbf7745e2fd354abf25c3deb3ca3
Quoted 11 lines
Author:     Andy Lutomirski <[email protected]>
AuthorDate: Wed, 23 Jul 2014 08:34:11 -0700
Committer:  H. Peter Anvin <[email protected]>
CommitDate: Mon, 28 Jul 2014 15:25:40 -0700
x86_64/entry/xen: Do not invoke espfix64 on Xen
This moves the espfix64 logic into native_iret. To make this work, it gets rid of the native patch for INTERRUPT_RETURN:
INTERRUPT_RETURN on native kernels is now 'jmp native_iret'.
Quoted 6 lines
This changes the 16-bit SS behavior on Xen from OOPSing to leaking some bits of the Xen hypervisor's RSP (I think).
[ hpa: this is a nonzero cost on native, but probably not enough to
measure. Xen needs to fix this in their own code, probably doing something equivalent to espfix64. ]
Any plan to either fix or remove the buggy bsp-only initialization of espfix64 on paravirt? It's harmless now, but IMO it's still ugly. Or should this wait for 3.17?
--Andy
Quoted 3 lines
Signed-off-by Andy Lutomirski <[email protected]>
Link:
http://lkml.kernel.org/r/7b8f1d8ef6597cb16ae004a43c56980a7de3cf94.1406129132.git.luto@amacapital.net
Quoted 5 lines
Signed-off-by H. Peter Anvin <[email protected]>
Cc <[email protected]>
---
 arch/x86/include/asm/irqflags.h     |  2 +-
 arch/x86/kernel/entry_64.S          | 28
++++++++++------------------
Review
Quoted 4 lines
 arch/x86/kernel/paravirt_patch_64.c |  2 --
 3 files changed, 11 insertions(+), 21 deletions(-)
(quoted diff) +0 -0
b/arch/x86/include/asm/irqflags.h
arch/x86/include/asm/irqflags.h static inline notrace unsigned long
Quoted 4 lines
index bba3cf8..0a8b519 100644
arch/x86/include/asm/irqflags.h +0 -0
@@ -129,7 +129,7 @@ static inline notrace unsigned long
arch_local_irq_save(void)
Quoted 13 lines
arch/x86/include/asm/irqflags.h +1 -1
 #define PARAVIRT_ADJUST_EXCEPTION_FRAME        /*  */
 
-#define INTERRUPT_RETURN       iretq
+#define INTERRUPT_RETURN       jmp native_iret
 #define USERGS_SYSRET64                                \
        swapgs;                                 \
        sysretq;
arch/x86/kernel/entry_64.S +0 -0
index b25ca96..c844f08 100644
arch/x86/kernel/entry_64.S restore_args: · line 837
Quoted 8 lines
arch/x86/kernel/entry_64.S +3 -0
@@ -830,27 +830,24 @@ restore_args:
        RESTORE_ARGS 1,8,1
 
 irq_return:
+       INTERRUPT_RETURN
+
+ENTRY(native_iret)
        /*
         * Are we returning to a stack segment from the LDT?  Note:
in
Quoted 36 lines
arch/x86/kernel/entry_64.S +5 ~~-11~~
         * 64-bit mode SS:RSP on the exception stack is always valid.
         */
 #ifdef CONFIG_X86_ESPFIX64
        testb $4,(SS-RIP)(%rsp)
-       jnz irq_return_ldt
+       jnz native_irq_return_ldt
 #endif
 
-irq_return_iret:
-       INTERRUPT_RETURN
-       _ASM_EXTABLE(irq_return_iret, bad_iret)
-
-#ifdef CONFIG_PARAVIRT
-ENTRY(native_iret)
+native_irq_return_iret:
        iretq
-       _ASM_EXTABLE(native_iret, bad_iret)
-#endif
+       _ASM_EXTABLE(native_irq_return_iret, bad_iret)
 
 #ifdef CONFIG_X86_ESPFIX64
-irq_return_ldt:
+native_irq_return_ldt:
        pushq_cfi %rax
        pushq_cfi %rdi
        SWAPGS
@@ -872,7 +869,7 @@ irq_return_ldt:
        SWAPGS
        movq %rax,%rsp
        popq_cfi %rax
-       jmp irq_return_iret
+       jmp native_irq_return_iret
 #endif
 
        .section .fixup,"ax"
arch/x86/kernel/entry_64.S __do_double_fault: · line 957
Quoted 10 lines
arch/x86/kernel/entry_64.S +1 -5
@@ -956,13 +953,8 @@ __do_double_fault:
        cmpl $__KERNEL_CS,CS(%rdi)
        jne do_double_fault
        movq RIP(%rdi),%rax
-       cmpq $irq_return_iret,%rax
-#ifdef CONFIG_PARAVIRT
-       je 1f
-       cmpq $native_iret,%rax
-#endif
+       cmpq $native_irq_return_iret,%rax
        jne do_double_fault             /* This shouldn't happen...
*/
arch/x86/kernel/entry_64.S __do_double_fault:
Quoted 3 lines
arch/x86/kernel/entry_64.S +0 -1
-1:
        movq PER_CPU_VAR(kernel_stack),%rax
        subq $(6*8-KERNEL_STACK_OFFSET),%rax    /* Reset to original
stack */
Review
Quoted 11 lines
arch/x86/kernel/entry_64.S +1 -1
        movq %rax,RSP(%rdi)
@@ -1428,7 +1420,7 @@ error_sti:
  */
 error_kernelspace:
        incl %ebx
-       leaq irq_return_iret(%rip),%rcx
+       leaq native_irq_return_iret(%rip),%rcx
        cmpq %rcx,RIP+8(%rsp)
        je error_swapgs
        movl %ecx,%eax  /* zero extend */
(quoted diff) +0 -0
b/arch/x86/kernel/paravirt_patch_64.c
arch/x86/kernel/paravirt_patch_64.c unsigned native_patch(u8 type, u16 clobbers, void
Quoted 12 lines
index 3f08f34..a1da673 100644
arch/x86/kernel/paravirt_patch_64.c +0 -1
@@ -6,7 +6,6 @@ DEF_NATIVE(pv_irq_ops, irq_disable, "cli");
 DEF_NATIVE(pv_irq_ops, irq_enable, "sti");
 DEF_NATIVE(pv_irq_ops, restore_fl, "pushq %rdi; popfq");
 DEF_NATIVE(pv_irq_ops, save_fl, "pushfq; popq %rax");
-DEF_NATIVE(pv_cpu_ops, iret, "iretq");
 DEF_NATIVE(pv_mmu_ops, read_cr2, "movq %cr2, %rax");
 DEF_NATIVE(pv_mmu_ops, read_cr3, "movq %cr3, %rax");
 DEF_NATIVE(pv_mmu_ops, write_cr3, "movq %rdi, %cr3");
@@ -50,7 +49,6 @@ unsigned native_patch(u8 type, u16 clobbers, void
*ibuf,
Quoted 7 lines
arch/x86/kernel/paravirt_patch_64.c +0 -1
                PATCH_SITE(pv_irq_ops, save_fl);
                PATCH_SITE(pv_irq_ops, irq_enable);
                PATCH_SITE(pv_irq_ops, irq_disable);
-               PATCH_SITE(pv_cpu_ops, iret);
                PATCH_SITE(pv_cpu_ops, irq_enable_sysexit);
                PATCH_SITE(pv_cpu_ops, usergs_sysret32);
                PATCH_SITE(pv_cpu_ops, usergs_sysret64);

Signature · 1 line

Sent from my mobile phone.  Please pardon brevity and lack of formatting.

Andy Lutomirski

author

2014-07-23 15:34 UTC #5 lore raw

PATCH 2/2 x86_64,entry: Use lret to return to userspace when possible

In reply to Andy Lutomirski

IRET serializes, but LRET does not. Try to use LRET to return to userspace when possible. It's possible if the saved RF and TF are clear, IF is set, and espfix isn't needed.

This cuts about 23ns off of the IRET-to-userspace path on my machine. (YMMV -- this was in a tight loop, and I can imagine the performance hit from serialization to be somewhat higher in real code.)

Signed-off-by Andy Lutomirski <[email protected]>

---

I've tested normal code, iret faults, lret faults, and returns to funny SS values.

I haven't explicitly exercised this code under heavy NMI load, nor have I tested it on Xen (although, unless I screwed up, it shouldn't do anything on Xen).

Benchmark away. I know that Linus had a test for this stuff. I doubt that this will be as spectacular as my old sysret trampoline hack, but it could still be a nice speedup.

 arch/x86/include/asm/irqflags.h  |  3 +-
 arch/x86/include/asm/paravirt.h  |  4 ++
 arch/x86/include/asm/traps.h     |  6 +++
 arch/x86/kernel/cpu/mcheck/mce.c |  2 +
 arch/x86/kernel/entry_64.S       | 93 +++++++++++++++++++++++++++++++++++++++-
 arch/x86/kernel/nmi.c            | 21 +++++++++
 6 files changed, 126 insertions(+), 3 deletions(-)

arch/x86/include/asm/irqflags.h +2 -1

index 0a8b519..04c45bb 100644
@@ -129,7 +129,8 @@ static inline notrace unsigned long arch_local_irq_save(void)
 
 #define PARAVIRT_ADJUST_EXCEPTION_FRAME	/*  */
 
-#define INTERRUPT_RETURN	jmp native_iret
+#define INTERRUPT_RETURN		jmp native_iret
+#define INTERRUPT_RETURN_UNBLOCK_NMI	jmp native_irq_return_need_iret
 #define USERGS_SYSRET64				\
 	swapgs;					\
 	sysretq;

arch/x86/include/asm/paravirt.h +4 -0

index cd6e161..3716b3d 100644
@@ -913,6 +913,10 @@ extern void default_banner(void);
 	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_iret), CLBR_NONE,	\
 		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret))
 
+#define INTERRUPT_RETURN_UNBLOCK_NMI					\
+	PARA_SITE(PARA_PATCH(pv_cpu_ops, PV_CPU_iret), CLBR_NONE,	\
+		  jmp PARA_INDIRECT(pv_cpu_ops+PV_CPU_iret))
+
 #define DISABLE_INTERRUPTS(clobbers)					\
 	PARA_SITE(PARA_PATCH(pv_irq_ops, PV_IRQ_irq_disable), clobbers, \
 		  PV_SAVE_REGS(clobbers | CLBR_CALLEE_SAVE);		\

arch/x86/include/asm/traps.h +6 -0

index bc8352e..2e3dfe8 100644
@@ -134,4 +134,10 @@ enum {
 	X86_TRAP_IRET = 32,	/* 32, IRET Exception */
 };
 
+#ifdef CONFIG_X86_64
+extern void fixup_lret_nmi(struct pt_regs *regs);
+#else
+static inline void fixup_lret_nmi(struct pt_regs *regs) {}
+#endif
+
 #endif /* _ASM_X86_TRAPS_H */

arch/x86/kernel/cpu/mcheck/mce.c +2 -0

index bb92f38..0bb9b9b 100644
@@ -43,6 +43,7 @@
 #include <linux/export.h>
 
 #include <asm/processor.h>
+#include <asm/traps.h>
 #include <asm/mce.h>
 #include <asm/msr.h>

@@ -1168,6 +1169,7 @@ void do_machine_check(struct pt_regs *regs, long error_code)
 	mce_wrmsrl(MSR_IA32_MCG_STATUS, 0);
 out:
 	sync_core();
+	fixup_lret_nmi(regs);
 }
 EXPORT_SYMBOL_GPL(do_machine_check);

arch/x86/kernel/entry_64.S +91 -2

129 lines not on this page · raw message

arch/x86/kernel/nmi.c +21 -0

index c3e985d..ca8be8e 100644
@@ -306,12 +306,33 @@ NOKPROBE_SYMBOL(unknown_nmi_error);
 static DEFINE_PER_CPU(bool, swallow_nmi);
 static DEFINE_PER_CPU(unsigned long, last_nmi_rip);
 
+#ifdef CONFIG_X86_64
+void fixup_lret_nmi(struct pt_regs *regs)
+{
+	/*
+	 * There is no architectural guarantee that an NMI or MCE can't
+	 * happen between sti and lret.  To avoid returning to the lret
+	 * instruction with interrupts on, we back up one instruction.
+	 */
+	extern const char native_lret_to_userspace[];
+	extern const char native_sti_before_lret_to_userspace[];
+
+	if (!user_mode_vm(regs) &&
+	    regs->ip == (unsigned long)native_lret_to_userspace) {
+		regs->ip = (unsigned long)native_sti_before_lret_to_userspace;
+		regs->flags &= ~X86_EFLAGS_IF;
+	}
+}
+#endif
+
 static void default_do_nmi(struct pt_regs *regs)
 {
 	unsigned char reason = 0;
 	int handled;
 	bool b2b = false;
 
+	fixup_lret_nmi(regs);
+
 	/*
 	 * CPU-specific NMI must be processed before non-CPU-specific
 	 * NMI, otherwise we may lose it, because the CPU-specific

Signature · 1 line

1.9.3