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

Groups > linux.kernel > #1272526 > unrolled thread

[PATCH 0/3] Fix and cleanup for 32-bit PV sysexit

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/3] Fix and cleanup for 32-bit PV sysexit Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2015-11-18 21:10 +0100
    [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2015-11-18 21:10 +0100
      Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Andy Lutomirski <luto@amacapital.net> - 2015-11-18 21:30 +0100
        Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Borislav Petkov <bp@suse.de> - 2015-11-18 21:50 +0100
        Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Brian Gerst <brgerst@gmail.com> - 2015-11-18 22:00 +0100
          Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Andy Lutomirski <luto@amacapital.net> - 2015-11-18 22:00 +0100
            Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Boris Ostrovsky <boris.ostrovsky@oracle.com> - 2015-11-18 23:10 +0100
        Re: [PATCH 1/3] x86/xen: Avoid fast syscall path for Xen PV guests Borislav Petkov <bp@suse.de> - 2015-11-19 13:10 +0100
    Re: [PATCH 0/3] Fix and cleanup for 32-bit PV sysexit Borislav Petkov <bp@suse.de> - 2015-11-19 13:20 +0100

#1272526 — [PATCH 0/3] Fix and cleanup for 32-bit PV sysexit

The first patch fixes Xen PV regression introduced by 32-bit rewrite. Unlike the
earlier version it uses ALTERNATIVE instruction and avoids using xen_sysexit
(and sysret32 in compat mode) pv ops, as suggested by Andy. (I ended up patching
TEST with XOR to avoid extra NOPs, even though I said yesterday it would be
wrong. It's not wrong)

As result of this patch irq_enable_sysexit and usergs_sysret32 pv ops are not
used anymore by anyone and so can be removed.

Boris Ostrovsky (3):
  x86/xen: Avoid fast syscall path for Xen PV guests
  x86: irq_enable_sysexit pv op is no longer needed
  x86: usergs_sysret32 pv op is no longer needed

 arch/x86/entry/entry_32.S             | 11 ++++-------
 arch/x86/entry/entry_64_compat.S      | 16 ++++++----------
 arch/x86/include/asm/cpufeature.h     |  1 +
 arch/x86/include/asm/paravirt.h       | 12 ------------
 arch/x86/include/asm/paravirt_types.h | 17 -----------------
 arch/x86/kernel/asm-offsets.c         |  3 ---
 arch/x86/kernel/asm-offsets_64.c      |  1 -
 arch/x86/kernel/paravirt.c            | 12 ------------
 arch/x86/kernel/paravirt_patch_32.c   |  2 --
 arch/x86/kernel/paravirt_patch_64.c   |  3 ---
 arch/x86/xen/enlighten.c              |  7 +++----
 arch/x86/xen/xen-asm_32.S             | 14 --------------
 arch/x86/xen/xen-asm_64.S             | 19 -------------------
 arch/x86/xen/xen-ops.h                |  3 ---
 14 files changed, 14 insertions(+), 107 deletions(-)

-- 
1.8.1.4

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

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

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)

Signed-off-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Suggested-by: Andy Lutomirski <luto@amacapital.net>
---
 arch/x86/entry/entry_32.S         | 3 ++-
 arch/x86/entry/entry_64_compat.S  | 6 ++++--
 arch/x86/include/asm/cpufeature.h | 1 +
 arch/x86/xen/enlighten.c          | 4 +++-
 4 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index 3eb572e..901f186 100644
--- 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
 
 /* Opportunistic SYSEXIT */
diff --git a/arch/x86/entry/entry_64_compat.S b/arch/x86/entry/entry_64_compat.S
index c320183..98893d9 100644
--- a/arch/x86/entry/entry_64_compat.S
+++ b/arch/x86/entry/entry_64_compat.S
@@ -121,7 +121,8 @@ sysenter_flags_fixed:
 
 	movq	%rsp, %rdi
 	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
 	jmp	sysret32_from_system_call
 
@@ -200,7 +201,8 @@ ENTRY(entry_SYSCALL_compat)
 
 	movq	%rsp, %rdi
 	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
 
 	/* Opportunistic SYSRET */
diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
index e4f8010..0e4fe5b 100644
--- a/arch/x86/include/asm/cpufeature.h
+++ b/arch/x86/include/asm/cpufeature.h
@@ -216,6 +216,7 @@
 #define X86_FEATURE_PAUSEFILTER ( 8*32+13) /* AMD filtered pause intercept */
 #define X86_FEATURE_PFTHRESHOLD ( 8*32+14) /* AMD pause filter threshold */
 #define X86_FEATURE_VMMCALL     ( 8*32+15) /* Prefer vmmcall to vmcall */
+#define X86_FEATURE_XENPV       ( 8*32+16) /* Xen paravirtual guest */
 
 
 /* Intel-defined CPU features, CPUID level 0x00000007:0 (ebx), word 9 */
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 5774800..d315151 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -1886,8 +1886,10 @@ EXPORT_SYMBOL_GPL(xen_hvm_need_lapic);
 
 static void xen_set_cpu_features(struct cpuinfo_x86 *c)
 {
-	if (xen_pv_domain())
+	if (xen_pv_domain()) {
 		clear_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
+		set_cpu_cap(c, X86_FEATURE_XENPV);
+	}
 }
 
 const struct hypervisor_x86 x86_hyper_xen = {
-- 
1.8.1.4

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

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

On Wed, Nov 18, 2015 at 12:06 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).
>
> 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:

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

> diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> index e4f8010..0e4fe5b 100644
> --- a/arch/x86/include/asm/cpufeature.h
> +++ b/arch/x86/include/asm/cpufeature.h
> @@ -216,6 +216,7 @@
>  #define X86_FEATURE_PAUSEFILTER ( 8*32+13) /* AMD filtered pause intercept */
>  #define X86_FEATURE_PFTHRESHOLD ( 8*32+14) /* AMD pause filter threshold */
>  #define X86_FEATURE_VMMCALL     ( 8*32+15) /* Prefer vmmcall to vmcall */
> +#define X86_FEATURE_XENPV       ( 8*32+16) /* Xen paravirtual guest */
>

This bit is highly magical and I think we need Borislav's ack.

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

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

On Wed, Nov 18, 2015 at 12:21:56PM -0800, Andy Lutomirski wrote:
> > diff --git a/arch/x86/include/asm/cpufeature.h b/arch/x86/include/asm/cpufeature.h
> > index e4f8010..0e4fe5b 100644
> > --- a/arch/x86/include/asm/cpufeature.h
> > +++ b/arch/x86/include/asm/cpufeature.h
> > @@ -216,6 +216,7 @@
> >  #define X86_FEATURE_PAUSEFILTER ( 8*32+13) /* AMD filtered pause intercept */
> >  #define X86_FEATURE_PFTHRESHOLD ( 8*32+14) /* AMD pause filter threshold */
> >  #define X86_FEATURE_VMMCALL     ( 8*32+15) /* Prefer vmmcall to vmcall */
> > +#define X86_FEATURE_XENPV       ( 8*32+16) /* Xen paravirtual guest */
> >
> 
> This bit is highly magical and I think we need Borislav's ack.

Yeah, that should be

#define X86_FEATURE_XENPV       ( 8*32+16) /* "" Xen paravirtual guest */
					      ^^

note the empty "". This prevents it from appearing in /proc/cpuinfo.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
--
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]

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

On Wed, Nov 18, 2015 at 3:21 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Wed, Nov 18, 2015 at 12:06 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).
>>
>> 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:
>
>> --- 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.

--
Brian Gerst
--
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]

#1272569 — 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 <brgerst@gmail.com> wrote:
> On Wed, Nov 18, 2015 at 3:21 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> On Wed, Nov 18, 2015 at 12:06 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).
>>>
>>> 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:
>>
>>> --- 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
--
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]

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

On 11/18/2015 03:58 PM, Andy Lutomirski wrote:
> On Wed, Nov 18, 2015 at 12:50 PM, Brian Gerst <brgerst@gmail.com> wrote:
>>
>> 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.

I don't think I understand what you mean here. Which full asm path?

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

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

On Wed, Nov 18, 2015 at 12:21:56PM -0800, Andy Lutomirski wrote:
> 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?

I don't mind either.

I would've said your version doesn't touch %eax so the result in there
might be useful for callers but all paths do overwrite it, AFAICT.

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
--
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]

#1273082

On Wed, Nov 18, 2015 at 03:06:16PM -0500, Boris Ostrovsky wrote:
> The first patch fixes Xen PV regression introduced by 32-bit rewrite. Unlike the
> earlier version it uses ALTERNATIVE instruction and avoids using xen_sysexit
> (and sysret32 in compat mode) pv ops, as suggested by Andy. (I ended up patching
> TEST with XOR to avoid extra NOPs, even though I said yesterday it would be
> wrong. It's not wrong)
> 
> As result of this patch irq_enable_sysexit and usergs_sysret32 pv ops are not
> used anymore by anyone and so can be removed.
> 
> Boris Ostrovsky (3):
>   x86/xen: Avoid fast syscall path for Xen PV guests
>   x86: irq_enable_sysexit pv op is no longer needed
>   x86: usergs_sysret32 pv op is no longer needed
> 
>  arch/x86/entry/entry_32.S             | 11 ++++-------
>  arch/x86/entry/entry_64_compat.S      | 16 ++++++----------
>  arch/x86/include/asm/cpufeature.h     |  1 +
>  arch/x86/include/asm/paravirt.h       | 12 ------------
>  arch/x86/include/asm/paravirt_types.h | 17 -----------------
>  arch/x86/kernel/asm-offsets.c         |  3 ---
>  arch/x86/kernel/asm-offsets_64.c      |  1 -
>  arch/x86/kernel/paravirt.c            | 12 ------------
>  arch/x86/kernel/paravirt_patch_32.c   |  2 --
>  arch/x86/kernel/paravirt_patch_64.c   |  3 ---
>  arch/x86/xen/enlighten.c              |  7 +++----
>  arch/x86/xen/xen-asm_32.S             | 14 --------------
>  arch/x86/xen/xen-asm_64.S             | 19 -------------------
>  arch/x86/xen/xen-ops.h                |  3 ---
>  14 files changed, 14 insertions(+), 107 deletions(-)

Can't argue with that diffstat!

:-)

-- 
Regards/Gruss,
    Boris.

SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg)
-- 
--
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