[PATCH v2] x86: Return to kernel without IRET

[PATCH v2] x86: Return to kernel without IRET

[Andy Lutomirski]

On my box, this saves about 100ns on each interrupt and trap that
happens while running in kernel space.  This speeds up my kernel_pf
microbenchmark by about 17%.

Signed-off-by: Andy Lutomirski <luto@amacapital.net>
---

Changes from v1:
 - Comment fix *facepalm*

Changes from the RFC:
 - Much better comments
 - Rewritten to use popq_cfi directly instead of RESTORE_ARGS
 - Uses sti to restore IF so we get the interrupt shadow

 arch/x86/kernel/entry_64.S | 49 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kernel/entry_64.S b/arch/x86/kernel/entry_64.S
index 1e96c36..504cec5 100644
--- a/arch/x86/kernel/entry_64.S
+++ b/arch/x86/kernel/entry_64.S
@@ -1033,9 +1033,56 @@ retint_swapgs:		/* return to user-space */
 retint_restore_args:	/* return to kernel space */
 	DISABLE_INTERRUPTS(CLBR_ANY)
 	/*
-	 * The iretq could re-enable interrupts:
+	 * The sti could re-enable interrupts:
 	 */
 	TRACE_IRQS_IRETQ
+
+	/*
+	 * Fast return to kernel.  The stack looks like:
+	 *
+	 * previous frame
+	 * possible 8 byte gap for alignment
+	 * SS RSP EFLAGS CS RIP
+	 * ORIG_RAX RDI ... R11
+	 *
+	 * We rewrite it to:
+	 *
+	 * previous frame
+	 * RIP (EFLAGS & ~IF) ...
+	 * pointer to the EFLAGS slot
+	 * RDI ... R11
+	 */
+	movq RSP-ARGOFFSET(%rsp), %rsi
+	subq $16, %rsi
+	movq EFLAGS-ARGOFFSET(%rsp), %rdi
+	movq RIP-ARGOFFSET(%rsp), %rcx
+	btr $9, %rdi
+	movq %rdi, (%rsi)
+	movq %rcx, 8(%rsi)
+	movq %rsi, ORIG_RAX-ARGOFFSET(%rsp)
+	popq_cfi %r11
+	popq_cfi %r10
+	popq_cfi %r9
+	popq_cfi %r8
+	popq_cfi %rax
+	popq_cfi %rcx
+	popq_cfi %rdx
+	popq_cfi %rsi
+	popq_cfi %rdi
+
+	popq %rsp
+	jc 1f
+	/* Interrupts were not enabled */
+	popfq_cfi
+	retq
+1:
+	CFI_ADJUST_CFA_OFFSET 8
+	/* Interrupts were enabled */
+	popfq_cfi
+	sti
+	/* Interrupts are still off because of the one-insn grace period. */
+	retq
+
 restore_args:
 	RESTORE_ARGS 1,8,1
 
-- 
1.9.0

Re: [PATCH v2] x86: Return to kernel without IRET

[H. Peter Anvin]

On 05/02/2014 04:53 PM, Andy Lutomirski wrote:
> On my box, this saves about 100ns on each interrupt and trap that
> happens while running in kernel space.  This speeds up my kernel_pf
> microbenchmark by about 17%.
> 
> Signed-off-by: Andy Lutomirski <luto@amacapital.net>

I'd really like to see a workload which would genuinely benefit before
adding more complexity.  Now... if we can determine that it doesn't harm
anything and would solve the NMI nesting problem cleaner than the
current solution, that would justify things, too...

	-hpa

Re: [PATCH v2] x86: Return to kernel without IRET

[Linus Torvalds]

On Fri, May 2, 2014 at 4:53 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On my box, this saves about 100ns on each interrupt and trap that
> happens while running in kernel space.  This speeds up my kernel_pf
> microbenchmark by about 17%.

Btw, would you mind _trying_ to do a similar trick for the "return to
user space" case?

At least as a proof-of-concept, having a code sequence in user mode
trampoline that does

   popq %rsi
   popq %r11
   retq $128

and building up a stack in user space at '%rsp-128' that has the
values or rsi/r11/rip should allow us to use 'sysret'. Hmm?

            Linus

Re: [PATCH v2] x86: Return to kernel without IRET

[H. Peter Anvin]

On 05/02/2014 09:32 PM, Linus Torvalds wrote:
> On Fri, May 2, 2014 at 4:53 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On my box, this saves about 100ns on each interrupt and trap that
>> happens while running in kernel space.  This speeds up my kernel_pf
>> microbenchmark by about 17%.
> 
> Btw, would you mind _trying_ to do a similar trick for the "return to
> user space" case?
> 
> At least as a proof-of-concept, having a code sequence in user mode
> trampoline that does
> 
>    popq %rsi
>    popq %r11
>    retq $128
> 
> and building up a stack in user space at '%rsp-128' that has the
> values or rsi/r11/rip should allow us to use 'sysret'. Hmm?
> 

That would be a security hole if another userspace thread could muck
with the stack.

	-hpa

Re: [PATCH v2] x86: Return to kernel without IRET

[Steven Rostedt]

On Fri, 02 May 2014 21:03:10 -0700
"H. Peter Anvin" <hpa@zytor.com> wrote:

> 
> I'd really like to see a workload which would genuinely benefit before
> adding more complexity.  Now... if we can determine that it doesn't harm
> anything and would solve the NMI nesting problem cleaner than the
> current solution, that would justify things, too...
> 

As I stated before. It doesn't solve the NMI nesting problem. It only
handles page faults. We would have to implement this for breakpoint
return paths too. Is that a plan as well?

-- Steve

Re: [PATCH v2] x86: Return to kernel without IRET

[Linus Torvalds]

On Fri, May 2, 2014 at 11:12 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/02/2014 09:32 PM, Linus Torvalds wrote:
>>
>> At least as a proof-of-concept, having a code sequence in user mode
>> trampoline that does
>>
>>    popq %rsi
>>    popq %r11
>>    retq $128
>>
>> and building up a stack in user space at '%rsp-128' that has the
>> values or rsi/r11/rip should allow us to use 'sysret'. Hmm?
>
> That would be a security hole if another userspace thread could muck
> with the stack.

No, all of the above is in user space, and the pre-restore register
values for rsi/r11/rip/rsp are all user space values (just not the
right ones for the "real" return point). So no security issue.

Now, replacing "iret" with "sysret + user-space trampoline" doesn't
work in general (it gets RF wrong, for example, so it's useless for
single-stepping and breakpoint handling), but I was more thinking that
it would be an interesting way to see what the performance impact of a
faster iret would be.

              Linus

Re: [PATCH v2] x86: Return to kernel without IRET

[H. Peter Anvin]

On 05/03/2014 06:54 AM, Linus Torvalds wrote:
> On Fri, May 2, 2014 at 11:12 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 05/02/2014 09:32 PM, Linus Torvalds wrote:
>>>
>>> At least as a proof-of-concept, having a code sequence in user mode
>>> trampoline that does
>>>
>>>    popq %rsi
>>>    popq %r11
>>>    retq $128
>>>
>>> and building up a stack in user space at '%rsp-128' that has the
>>> values or rsi/r11/rip should allow us to use 'sysret'. Hmm?
>>
>> That would be a security hole if another userspace thread could muck
>> with the stack.
> 
> No, all of the above is in user space, and the pre-restore register
> values for rsi/r11/rip/rsp are all user space values (just not the
> right ones for the "real" return point). So no security issue.
> 
> Now, replacing "iret" with "sysret + user-space trampoline" doesn't
> work in general (it gets RF wrong, for example, so it's useless for
> single-stepping and breakpoint handling), but I was more thinking that
> it would be an interesting way to see what the performance impact of a
> faster iret would be.
> 

Right, brain failure on my part.  I somehow got it in my head you'd run
the above off the user stack while in CPL 0, which would be obviously crazy.

I think this would be rather interesting experiment.

	-hpa

Re: [PATCH v2] x86: Return to kernel without IRET

[H. Peter Anvin]

On 05/03/2014 04:24 AM, Steven Rostedt wrote:
> On Fri, 02 May 2014 21:03:10 -0700
> "H. Peter Anvin" <hpa@zytor.com> wrote:
> 
>>
>> I'd really like to see a workload which would genuinely benefit before
>> adding more complexity.  Now... if we can determine that it doesn't harm
>> anything and would solve the NMI nesting problem cleaner than the
>> current solution, that would justify things, too...
>>
> 
> As I stated before. It doesn't solve the NMI nesting problem. It only
> handles page faults. We would have to implement this for breakpoint
> return paths too. Is that a plan as well?
> 

I would assume we would do it for *ALL* the IRETs.  There are only three
IRETs in the kernel last I checked...

	-hpa

Re: [PATCH v2] x86: Return to kernel without IRET

[Andy Lutomirski]

On Sat, May 3, 2014 at 3:19 PM, H. Peter Anvin <hpa@zytor.com> wrote:
> On 05/03/2014 04:24 AM, Steven Rostedt wrote:
>> On Fri, 02 May 2014 21:03:10 -0700
>> "H. Peter Anvin" <hpa@zytor.com> wrote:
>>
>>>
>>> I'd really like to see a workload which would genuinely benefit before
>>> adding more complexity.  Now... if we can determine that it doesn't harm
>>> anything and would solve the NMI nesting problem cleaner than the
>>> current solution, that would justify things, too...
>>>
>>
>> As I stated before. It doesn't solve the NMI nesting problem. It only
>> handles page faults. We would have to implement this for breakpoint
>> return paths too. Is that a plan as well?
>>
>
> I would assume we would do it for *ALL* the IRETs.  There are only three
> IRETs in the kernel last I checked...

I think we should carefully avoid doing it for returns from NMI, though :)

If you want a realistic benchmark that will speed up, packet
forwarding might be a good place to look.

--Andy

Re: [PATCH v2] x86: Return to kernel without IRET

[Andy Lutomirski]

On Sat, May 3, 2014 at 4:51 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Sat, May 3, 2014 at 3:19 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 05/03/2014 04:24 AM, Steven Rostedt wrote:
>>> On Fri, 02 May 2014 21:03:10 -0700
>>> "H. Peter Anvin" <hpa@zytor.com> wrote:
>>>
>>>>
>>>> I'd really like to see a workload which would genuinely benefit before
>>>> adding more complexity.  Now... if we can determine that it doesn't harm
>>>> anything and would solve the NMI nesting problem cleaner than the
>>>> current solution, that would justify things, too...
>>>>
>>>
>>> As I stated before. It doesn't solve the NMI nesting problem. It only
>>> handles page faults. We would have to implement this for breakpoint
>>> return paths too. Is that a plan as well?
>>>
>>
>> I would assume we would do it for *ALL* the IRETs.  There are only three
>> IRETs in the kernel last I checked...
>
> I think we should carefully avoid doing it for returns from NMI, though :)
>
> If you want a realistic benchmark that will speed up, packet
> forwarding might be a good place to look.

Hmm.  I think my patch will blow up with EFI mixed mode if any EFI
functions are called with interrupts enabled.  It may also blow up
with when suspending or doing other BIOS things like that.  It should
probably check the actual value of CS as opposed to just the CPL.

I'm not sure what's happening with the alternate GDT in the EFI stuff.

--Andy

Re: [PATCH v2] x86: Return to kernel without IRET

[H. Peter Anvin]

Not for return from NMIs themselves, to be sure

On May 3, 2014 4:51:37 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Sat, May 3, 2014 at 3:19 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>> On 05/03/2014 04:24 AM, Steven Rostedt wrote:
>>> On Fri, 02 May 2014 21:03:10 -0700
>>> "H. Peter Anvin" <hpa@zytor.com> wrote:
>>>
>>>>
>>>> I'd really like to see a workload which would genuinely benefit
>before
>>>> adding more complexity.  Now... if we can determine that it doesn't
>harm
>>>> anything and would solve the NMI nesting problem cleaner than the
>>>> current solution, that would justify things, too...
>>>>
>>>
>>> As I stated before. It doesn't solve the NMI nesting problem. It
>only
>>> handles page faults. We would have to implement this for breakpoint
>>> return paths too. Is that a plan as well?
>>>
>>
>> I would assume we would do it for *ALL* the IRETs.  There are only
>three
>> IRETs in the kernel last I checked...
>
>I think we should carefully avoid doing it for returns from NMI, though
>:)
>
>If you want a realistic benchmark that will speed up, packet
>forwarding might be a good place to look.
>
>--Andy

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

Re: [PATCH v2] x86: Return to kernel without IRET

[H. Peter Anvin]

We have to do that anyway to deal with 16- and 32-bit userspace return.

On May 3, 2014 5:31:41 PM PDT, Andy Lutomirski <luto@amacapital.net> wrote:
>On Sat, May 3, 2014 at 4:51 PM, Andy Lutomirski <luto@amacapital.net>
>wrote:
>> On Sat, May 3, 2014 at 3:19 PM, H. Peter Anvin <hpa@zytor.com> wrote:
>>> On 05/03/2014 04:24 AM, Steven Rostedt wrote:
>>>> On Fri, 02 May 2014 21:03:10 -0700
>>>> "H. Peter Anvin" <hpa@zytor.com> wrote:
>>>>
>>>>>
>>>>> I'd really like to see a workload which would genuinely benefit
>before
>>>>> adding more complexity.  Now... if we can determine that it
>doesn't harm
>>>>> anything and would solve the NMI nesting problem cleaner than the
>>>>> current solution, that would justify things, too...
>>>>>
>>>>
>>>> As I stated before. It doesn't solve the NMI nesting problem. It
>only
>>>> handles page faults. We would have to implement this for breakpoint
>>>> return paths too. Is that a plan as well?
>>>>
>>>
>>> I would assume we would do it for *ALL* the IRETs.  There are only
>three
>>> IRETs in the kernel last I checked...
>>
>> I think we should carefully avoid doing it for returns from NMI,
>though :)
>>
>> If you want a realistic benchmark that will speed up, packet
>> forwarding might be a good place to look.
>
>Hmm.  I think my patch will blow up with EFI mixed mode if any EFI
>functions are called with interrupts enabled.  It may also blow up
>with when suspending or doing other BIOS things like that.  It should
>probably check the actual value of CS as opposed to just the CPL.
>
>I'm not sure what's happening with the alternate GDT in the EFI stuff.
>
>--Andy

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

Re: [PATCH v2] x86: Return to kernel without IRET

[Andy Lutomirski]

On May 3, 2014 3:19 PM, "H. Peter Anvin" <hpa@zytor.com> wrote:
>
> On 05/03/2014 04:24 AM, Steven Rostedt wrote:
> > On Fri, 02 May 2014 21:03:10 -0700
> > "H. Peter Anvin" <hpa@zytor.com> wrote:
> >
> >>
> >> I'd really like to see a workload which would genuinely benefit before
> >> adding more complexity.  Now... if we can determine that it doesn't harm
> >> anything and would solve the NMI nesting problem cleaner than the
> >> current solution, that would justify things, too...
> >>
> >
> > As I stated before. It doesn't solve the NMI nesting problem. It only
> > handles page faults. We would have to implement this for breakpoint
> > return paths too. Is that a plan as well?
> >
>
> I would assume we would do it for *ALL* the IRETs.  There are only three
> IRETs in the kernel last I checked...
>

I think that doing this for all the non-NMI IRETs may be an enormous
mess because of syscall.  syscall immediates followed by #MC or #DB
will explode using a ret trampoline, since the return RSP value will
be bogus.

This isn't a problem for non-IST IRETs, since they only happen when
the return stack is valid.

We could maybe do an iretless return only when we're on usergs, but
this may still not fix the problem, and it doesn't fix NMI nesting:
#NM followed by #MC or #DB before swapgs will still do IRET.  Also,
Andi's FSGSBASE patches are about to remove the ability to distinguish
user vs kernel gs during IST interrupt processing.

We could check the return RIP and do a nasty fixup (i.e. emulate the
stack switch and possible swapgs prior to return), but this will be
really messy, and Andi's patches will just make it worse.  I don't
really want to do this.

So this might be non-IST only unless anyone has a better idea.

This may mean that the iretless return path should only happen when CS
is the normal kernel value (sorry, Xen) and the saved IF is 1.  That
gets rid of the annoying branch to deal with IF.

Grr.  I want a way to do this without a trampoline on the stack.  The
new instruction I want is:

FASTRET - fast return to kernel or user space

FASTRET pops RIP, CS, EFLAGS, RSP, and SS.  It does not unmask NMI.
It, like SYSCALL and SYSRET, completely ignores the GDT; it restores
the selector values for CS and SS but fills the rest of the processor
state with default 64-bit values.  It does, however, set CPL to match
whatever was on the stack.

Then the kernel code is straightforward: if (!NMI && (CS == kernelcs64
&& SS == kernelss) || (CS == usercs64 && SS == userss) FASTRET else
IRET.

BTW, what, if anything, prevents #MC from nesting?  I suspect that we
are completely screwed if #MC nests.  Maybe the answer is that a
machine-check-worthy error that happens during #MC handling is more or
less fatal anyway.

--Andy

This requires an IRET-style s

>         -hpa
>
>