lkml.org 
[lkml]   [2015]   [Nov]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] xen/x86: Adjust stack pointer in xen_sysexit
From
Date
On 11/15/2015 01:02 PM, Andy Lutomirski wrote:
> 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.


So it turned out that for compat mode we don't need to do anything since
xen_sysret32 doesn't assume any stack format (or, rather, it assumes
that it can't be used) and builds the IRET frame itself.


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


We don't currently have a Xen-specific CPU feature. We could, in
principle, add it but we can't replace all of current paravirt patching
with a single feature since PVH guests use a subset of existing pv ops
(and in the future it may become even more fine-grained).

And I don't think we should go ALTERNATIVE route for one set of features
and keep pv ops for the rest --- it should be either one or the other.


-boris



\
 
 \ /
  Last update: 2015-11-16 17:41    [W:0.097 / U:0.600 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site