Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests

On Wed, Nov 18, 2015 at 12:50 PM, Brian Gerst [off-list ref] wrote:

On Wed, Nov 18, 2015 at 3:21 PM, Andy Lutomirski [off-list ref] wrote:

quoted

On Wed, Nov 18, 2015 at 12:06 PM, Boris Ostrovsky
[off-list ref] wrote:

quoted

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).

Since we end up calling xen_iret from xen_sysexit we don't need to fix
up the stack and instead follow entry_SYSENTER_32's IRET path directly
to xen_iret.

We can do the same thing for compat mode even though stack does not need
to be fixed. This will allow us to drop usergs_sysret32 paravirt op (in
the subsequent patch)

Looks generally quite nice.  Minor comments below:

quoted

--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S

@@ -308,7 +308,8 @@ sysenter_past_esp:

        movl    %esp, %eax
        call    do_fast_syscall_32
-       testl   %eax, %eax
+       /* XEN PV guests always use IRET path */
+       ALTERNATIVE "testl %eax, %eax", "xor %eax, %eax", X86_FEATURE_XENPV
        jz      .Lsyscall_32_done

Could we make this a little less subtle:

ALTERNATIVE "testl %eax, %eax; lz .Lsyscall_32_done", "jmp
.Lsyscasll_32_done", X86_FEATURE_XENPV

Borislav, what do you think?

Ditto for the others.

Can you just add !xen_pv_domain() to the opportunistic SYSRET check
instead?  Bury the alternatives in that macro, ie.
static_cpu_has(X86_FEATURE_XENPV).  That would likely benefit other
code as well.

We could, but that won't help the 64-bit case where we want to keep
the full asm path.

Also, Xen is capable of the equivalent of sysret32 in the compat case.
We might want to enable something like that, and using the existing
opportunistic sysret check may make sense, in which case we wouldn't
want to disable it.

--Andy