Re: [PATCH 4/4] x86/xen: drop USERGS_SYSRET64 paravirt call

2020-11-17 Thread Jürgen Groß via Virtualization

On 16.11.20 17:28, Andy Lutomirski wrote:

On Mon, Nov 16, 2020 at 7:23 AM Juergen Gross  wrote:


USERGS_SYSRET64 is used to return from a syscall via sysret, but
a Xen PV guest will nevertheless use the iret hypercall, as there
is no sysret PV hypercall defined.

So instead of testing all the prerequisites for doing a sysret and
then mangling the stack for Xen PV again for doing an iret just use
the iret exit from the beginning.

This can easily be done via an ALTERNATIVE like it is done for the
sysenter compat case already.

While at it remove to stale sysret32 remnants.

Signed-off-by: Juergen Gross 


Acked-by: Andy Lutomirski 

FWIW, you've lost the VGCF_in_syscall optimization.  Let me see if I
can give it back to you better.


Ah, right.

Nevertheless a simple kernel build is about 0.5% faster with this
patch.


Juergen


OpenPGP_0xB0DE9DD628BF132F.asc
Description: application/pgp-keys


OpenPGP_signature
Description: OpenPGP digital signature
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH 4/4] x86/xen: drop USERGS_SYSRET64 paravirt call

2020-11-16 Thread Andy Lutomirski
On Mon, Nov 16, 2020 at 7:23 AM Juergen Gross  wrote:
>
> USERGS_SYSRET64 is used to return from a syscall via sysret, but
> a Xen PV guest will nevertheless use the iret hypercall, as there
> is no sysret PV hypercall defined.
>
> So instead of testing all the prerequisites for doing a sysret and
> then mangling the stack for Xen PV again for doing an iret just use
> the iret exit from the beginning.
>
> This can easily be done via an ALTERNATIVE like it is done for the
> sysenter compat case already.
>
> While at it remove to stale sysret32 remnants.

s/to/the/
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


Re: [PATCH 4/4] x86/xen: drop USERGS_SYSRET64 paravirt call

2020-11-16 Thread Andy Lutomirski
On Mon, Nov 16, 2020 at 7:23 AM Juergen Gross  wrote:
>
> USERGS_SYSRET64 is used to return from a syscall via sysret, but
> a Xen PV guest will nevertheless use the iret hypercall, as there
> is no sysret PV hypercall defined.
>
> So instead of testing all the prerequisites for doing a sysret and
> then mangling the stack for Xen PV again for doing an iret just use
> the iret exit from the beginning.
>
> This can easily be done via an ALTERNATIVE like it is done for the
> sysenter compat case already.
>
> While at it remove to stale sysret32 remnants.
>
> Signed-off-by: Juergen Gross 

Acked-by: Andy Lutomirski 

FWIW, you've lost the VGCF_in_syscall optimization.  Let me see if I
can give it back to you better.

> ---
>  arch/x86/entry/entry_64.S | 22 +-
>  arch/x86/include/asm/irqflags.h   |  6 --
>  arch/x86/include/asm/paravirt.h   |  5 -
>  arch/x86/include/asm/paravirt_types.h |  8 
>  arch/x86/kernel/asm-offsets_64.c  |  2 --
>  arch/x86/kernel/paravirt.c|  5 +
>  arch/x86/kernel/paravirt_patch.c  |  4 
>  arch/x86/xen/enlighten_pv.c   |  1 -
>  arch/x86/xen/xen-asm.S| 20 
>  arch/x86/xen/xen-ops.h|  2 --
>  10 files changed, 10 insertions(+), 65 deletions(-)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index a876204a73e0..df865eebd3d7 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -46,14 +46,6 @@
>  .code64
>  .section .entry.text, "ax"
>
> -#ifdef CONFIG_PARAVIRT_XXL
> -SYM_CODE_START(native_usergs_sysret64)
> -   UNWIND_HINT_EMPTY
> -   swapgs
> -   sysretq
> -SYM_CODE_END(native_usergs_sysret64)
> -#endif /* CONFIG_PARAVIRT_XXL */
> -
>  /*
>   * 64-bit SYSCALL instruction entry. Up to 6 arguments in registers.
>   *
> @@ -123,12 +115,15 @@ SYM_INNER_LABEL(entry_SYSCALL_64_after_hwframe, 
> SYM_L_GLOBAL)
>  * Try to use SYSRET instead of IRET if we're returning to
>  * a completely clean 64-bit userspace context.  If we're not,
>  * go to the slow exit path.
> +* In the Xen PV case we must use iret anyway.
>  */
> -   movqRCX(%rsp), %rcx
> -   movqRIP(%rsp), %r11
>
> -   cmpq%rcx, %r11  /* SYSRET requires RCX == RIP */
> -   jne swapgs_restore_regs_and_return_to_usermode
> +   ALTERNATIVE __stringify( \
> +   movqRCX(%rsp), %rcx; \
> +   movqRIP(%rsp), %r11; \
> +   cmpq%rcx, %r11; /* SYSRET requires RCX == RIP */ \
> +   jne swapgs_restore_regs_and_return_to_usermode), \
> +   "jmpswapgs_restore_regs_and_return_to_usermode", X86_FEATURE_XENPV

I'm not in love with this save-a-few-bytes stringify, but I can live with it.

--Andy
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization


[PATCH 4/4] x86/xen: drop USERGS_SYSRET64 paravirt call

2020-11-16 Thread Juergen Gross via Virtualization
USERGS_SYSRET64 is used to return from a syscall via sysret, but
a Xen PV guest will nevertheless use the iret hypercall, as there
is no sysret PV hypercall defined.

So instead of testing all the prerequisites for doing a sysret and
then mangling the stack for Xen PV again for doing an iret just use
the iret exit from the beginning.

This can easily be done via an ALTERNATIVE like it is done for the
sysenter compat case already.

While at it remove to stale sysret32 remnants.

Signed-off-by: Juergen Gross 
---
 arch/x86/entry/entry_64.S | 22 +-
 arch/x86/include/asm/irqflags.h   |  6 --
 arch/x86/include/asm/paravirt.h   |  5 -
 arch/x86/include/asm/paravirt_types.h |  8 
 arch/x86/kernel/asm-offsets_64.c  |  2 --
 arch/x86/kernel/paravirt.c|  5 +
 arch/x86/kernel/paravirt_patch.c  |  4 
 arch/x86/xen/enlighten_pv.c   |  1 -
 arch/x86/xen/xen-asm.S| 20 
 arch/x86/xen/xen-ops.h|  2 --
 10 files changed, 10 insertions(+), 65 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index a876204a73e0..df865eebd3d7 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -46,14 +46,6 @@
 .code64
 .section .entry.text, "ax"
 
-#ifdef CONFIG_PARAVIRT_XXL
-SYM_CODE_START(native_usergs_sysret64)
-   UNWIND_HINT_EMPTY
-   swapgs
-   sysretq
-SYM_CODE_END(native_usergs_sysret64)
-#endif /* CONFIG_PARAVIRT_XXL */
-
 /*
  * 64-bit SYSCALL instruction entry. Up to 6 arguments in registers.
  *
@@ -123,12 +115,15 @@ SYM_INNER_LABEL(entry_SYSCALL_64_after_hwframe, 
SYM_L_GLOBAL)
 * Try to use SYSRET instead of IRET if we're returning to
 * a completely clean 64-bit userspace context.  If we're not,
 * go to the slow exit path.
+* In the Xen PV case we must use iret anyway.
 */
-   movqRCX(%rsp), %rcx
-   movqRIP(%rsp), %r11
 
-   cmpq%rcx, %r11  /* SYSRET requires RCX == RIP */
-   jne swapgs_restore_regs_and_return_to_usermode
+   ALTERNATIVE __stringify( \
+   movqRCX(%rsp), %rcx; \
+   movqRIP(%rsp), %r11; \
+   cmpq%rcx, %r11; /* SYSRET requires RCX == RIP */ \
+   jne swapgs_restore_regs_and_return_to_usermode), \
+   "jmpswapgs_restore_regs_and_return_to_usermode", X86_FEATURE_XENPV
 
/*
 * On Intel CPUs, SYSRET with non-canonical RCX/RIP will #GP
@@ -215,7 +210,8 @@ syscall_return_via_sysret:
 
popq%rdi
popq%rsp
-   USERGS_SYSRET64
+   swapgs
+   sysretq
 SYM_CODE_END(entry_SYSCALL_64)
 
 /*
diff --git a/arch/x86/include/asm/irqflags.h b/arch/x86/include/asm/irqflags.h
index 8c86edefa115..e585a4705b8d 100644
--- a/arch/x86/include/asm/irqflags.h
+++ b/arch/x86/include/asm/irqflags.h
@@ -132,12 +132,6 @@ static __always_inline unsigned long 
arch_local_irq_save(void)
 #endif
 
 #define INTERRUPT_RETURN   jmp native_iret
-#define USERGS_SYSRET64\
-   swapgs; \
-   sysretq;
-#define USERGS_SYSRET32\
-   swapgs; \
-   sysretl
 
 #else
 #define INTERRUPT_RETURN   iret
diff --git a/arch/x86/include/asm/paravirt.h b/arch/x86/include/asm/paravirt.h
index 5647bcdba776..8121cf9b8d81 100644
--- a/arch/x86/include/asm/paravirt.h
+++ b/arch/x86/include/asm/paravirt.h
@@ -776,11 +776,6 @@ extern void default_banner(void);
 
 #ifdef CONFIG_X86_64
 #ifdef CONFIG_PARAVIRT_XXL
-#define USERGS_SYSRET64
\
-   PARA_SITE(PARA_PATCH(PV_CPU_usergs_sysret64),   \
- ANNOTATE_RETPOLINE_SAFE;  \
- jmp PARA_INDIRECT(pv_ops+PV_CPU_usergs_sysret64);)
-
 #ifdef CONFIG_DEBUG_ENTRY
 #define SAVE_FLAGS(clobbers)\
PARA_SITE(PARA_PATCH(PV_IRQ_save_fl),   \
diff --git a/arch/x86/include/asm/paravirt_types.h 
b/arch/x86/include/asm/paravirt_types.h
index 903d71884fa2..55d8b7950e61 100644
--- a/arch/x86/include/asm/paravirt_types.h
+++ b/arch/x86/include/asm/paravirt_types.h
@@ -157,14 +157,6 @@ struct pv_cpu_ops {
 
u64 (*read_pmc)(int counter);
 
-   /*
-* Switch to usermode gs and return to 64-bit usermode using
-* sysret.  Only used in 64-bit kernels to return to 64-bit
-* processes.  Usermode register state, including %rsp, must
-* already be restored.
-*/
-   void (*usergs_sysret64)(void);
-
/* Normal iret.  Jump to this with the standard iret stack
   frame set up. */
void (*iret)(void);
diff --git a/arch/x86/kernel/asm-offsets_64.c b/arch/x86/kernel/asm-offsets_64.c
index 1354bc30614d..b14533af7676 100644
---