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

Groups > linux.kernel > #1269357 > unrolled thread

[PATCH] xen/x86: Adjust stack pointer in xen_sysexit

Back to article view | Back to linux.kernel

Contents

  [PATCH] xen/x86: Adjust stack pointer in xen_sysexit Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2015-11-14 00:20 +0100
    Re: [PATCH] xen/x86: Adjust stack pointer in xen_sysexit Andy Lutomirski <luto@amacapital.net> - 2015-11-14 00:30 +0100
      Re: [PATCH] xen/x86: Adjust stack pointer in xen_sysexit Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2015-11-14 02:30 +0100
        Re: [PATCH] xen/x86: Adjust stack pointer in xen_sysexit Andy Lutomirski <luto@amacapital.net> - 2015-11-15 19:10 +0100

#1269357 — [PATCH] xen/x86: Adjust stack pointer in xen_sysexit

After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
it's not pt_regs).

We need to adjust it so that subsequent xen_iret can use it.

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---

Alternatively, we could return 0 from do_fast_syscall_32() if paravirt_enabled()
is true since Xen PV guests will end up using xen_iret one way or the other. And
then we won't need xen_sysexit at all.

 arch/x86/xen/xen-asm_32.S |   23 ++++++++++++++++-------
 1 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/arch/x86/xen/xen-asm_32.S b/arch/x86/xen/xen-asm_32.S
index fd92a64..c70ec37 100644
--- a/arch/x86/xen/xen-asm_32.S
+++ b/arch/x86/xen/xen-asm_32.S
@@ -36,15 +36,24 @@ check_events:
 
 /*
  * We can't use sysexit directly, because we're not running in ring0.
- * But we can easily fake it up using iret.  Assuming xen_sysexit is
- * jumped to with a standard stack frame, we can just strip it back to
- * a standard iret frame and use iret.
+ * But we can easily fake it up using iret.
+ * We came here from the opportunistic SYSEXIT path in entry_SYSENTER_32
+ * which left the stack looking like this:
+ * 	$__USER_DS
+ * 	%ecx
+ * 	eflags
+ *	$__USER_CS
+ *	%eip
+ *	%eax
+ *	%gs
+ *	%fs
+ *	%es
+ *	%ds	<-- %esp
+ *
+ * so we need to adjust it to look like a standard iret frame
  */
 ENTRY(xen_sysexit)
-	movl PT_EAX(%esp), %eax			/* Shouldn't be necessary? */
-	orl $X86_EFLAGS_IF, PT_EFLAGS(%esp)
-	lea PT_EIP(%esp), %esp
-
+	add $5*4, %esp
 	jmp xen_iret
 ENDPROC(xen_sysexit)
 
-- 
1.7.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1269358

On Fri, Nov 13, 2015 at 3:18 PM, Boris Ostrovsky
<boris.ostrovsky@oracle.com> wrote:
> After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
> ("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
> frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
> it's not pt_regs).
>
> We need to adjust it so that subsequent xen_iret can use it.

I'm wondering if this should be more straightforward:

        movq    %rsp, %rdi
        call    do_fast_syscall_32
        testl   %eax, %eax
        jz      .Lsyscall_32_done

        /* Opportunistic SYSRET */
sysret32_from_system_call:
        XEN_DO_SYSRET32

where XEN_DO_SYSRET32 is a simple pv op that, on Xen, jumps to a
variant of Xen's iret path that knows that the fast path is okay.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1269387

On 11/13/2015 06:26 PM, Andy Lutomirski wrote:
> On Fri, Nov 13, 2015 at 3:18 PM, Boris Ostrovsky
> <boris.ostrovsky@oracle.com> wrote:
>> After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
>> ("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
>> frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
>> it's not pt_regs).
>>
>> We need to adjust it so that subsequent xen_iret can use it.
> I'm wondering if this should be more straightforward:
>
>          movq    %rsp, %rdi
>          call    do_fast_syscall_32
>          testl   %eax, %eax
>          jz      .Lsyscall_32_done
>
>          /* Opportunistic SYSRET */
> sysret32_from_system_call:
>          XEN_DO_SYSRET32
>
> where XEN_DO_SYSRET32 is a simple pv op that, on Xen, jumps to a
> variant of Xen's iret path that knows that the fast path is okay.


This patch is for 32-bit kernel. I actually haven't looked at compat 
code (probably because our tests don't try that), I need to do that too.

As for XEN_DO_SYSRET32 --- we'd presumably need to have a nop for 
baremetal otherwise current paravirt op will use native_usergs_sysret32 
(for compat code). Which means a new pv_op, I think.

-boris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1269727

On Nov 13, 2015 5:23 PM, "Boris Ostrovsky" <boris.ostrovsky@oracle.com> wrote:
>
>
>
> On 11/13/2015 06:26 PM, Andy Lutomirski wrote:
>>
>> On Fri, Nov 13, 2015 at 3:18 PM, Boris Ostrovsky
>> <boris.ostrovsky@oracle.com> wrote:
>>>
>>> After 32-bit syscall rewrite, and specifically after commit 5f310f739b4c
>>> ("x86/entry/32: Re-implement SYSENTER using the new C path"), the stack
>>> frame that is passed to xen_sysexit is no longer a "standard" one (i.e.
>>> it's not pt_regs).
>>>
>>> We need to adjust it so that subsequent xen_iret can use it.
>>
>> I'm wondering if this should be more straightforward:
>>
>>          movq    %rsp, %rdi
>>          call    do_fast_syscall_32
>>          testl   %eax, %eax
>>          jz      .Lsyscall_32_done
>>
>>          /* Opportunistic SYSRET */
>> sysret32_from_system_call:
>>          XEN_DO_SYSRET32
>>
>> where XEN_DO_SYSRET32 is a simple pv op that, on Xen, jumps to a
>> variant of Xen's iret path that knows that the fast path is okay.
>
>
>
> This patch is for 32-bit kernel. I actually haven't looked at compat code (probably because our tests don't try that), I need to do that too.

In 4.4, it's almost identical (which was part of the point of this
whole series).  We use sysret32 instead of sysexit, but the underlying
structure is the same: munge the stack frame and register state
appropriately to use the fast return instruction in question and then
execute it.  In both cases, the only real difference from the IRET
path is that we're willing to lose the values of some subset of cx,
dx, and (on 64-bit kernels) r11.

>
> As for XEN_DO_SYSRET32 --- we'd presumably need to have a nop for baremetal otherwise current paravirt op will use native_usergs_sysret32 (for compat code). Which means a new pv_op, I think.

Agreed, unless...

Does Xen have a cpufeature?  Using ALTERNATIVE instead of a pvop could
be easier to follow and be less code at the same time.  Frankly,
following the control flow from asm through the pre-paravirt-patching
and post-paravirt-patching variants and into the final targets is
getting a little bit old, and ALTERNATIVE is crystal clear in
comparison (and has all the interesting info inline with the rest of
the asm).  Of course, it doesn't work early in boot, but that's fine
for anything involving user/kernel switches.


--Andy

>
> -boris
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web