From: Konrad Rzeszutek Wilk <konrad@kernel.org>
To: Jan Beulich <JBeulich@suse.com>
Cc: xen-devel <xen-devel@lists.xen.org>
Subject: Re: [PATCH 1/3] x86: use MOV instead of PUSH/POP when saving/restoring register state
Date: Tue, 2 Oct 2012 13:57:17 -0400 [thread overview]
Message-ID: <20121002175716.GA30321@phenom.dumpdata.com> (raw)
In-Reply-To: <506B23BF020000780009F29A@nat28.tlf.novell.com>
On Tue, Oct 02, 2012 at 04:26:23PM +0100, Jan Beulich wrote:
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
What's the reasoning behind it? Just that mov's are faster than
pop/push?
>
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -21,8 +21,7 @@ ENTRY(compat_hypercall)
> UNLIKELY_START(ne, msi_check)
> movl $HYPERCALL_VECTOR,%edi
> call check_for_unexpected_msi
> - RESTORE_ALL
> - SAVE_ALL
> + LOAD_C_CLOBBERED
> UNLIKELY_END(msi_check)
>
> GET_CURRENT(%rbx)
> @@ -173,8 +172,7 @@ compat_bad_hypercall:
> /* %rbx: struct vcpu, interrupts disabled */
> compat_restore_all_guest:
> ASSERT_INTERRUPTS_DISABLED
> - RESTORE_ALL
> - addq $8,%rsp
> + RESTORE_ALL adj=8
> .Lft0: iretq
>
> .section .fixup,"ax"
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -47,12 +47,10 @@ restore_all_guest:
> cmpl $1,%ecx
> ja .Lforce_iret
>
> - addq $8,%rsp
> - popq %rcx # RIP
> - popq %r11 # CS
> - cmpw $FLAT_USER_CS32,%r11
> - popq %r11 # RFLAGS
> - popq %rsp # RSP
> + cmpw $FLAT_USER_CS32,16(%rsp)# CS
> + movq 8(%rsp),%rcx # RIP
> + movq 24(%rsp),%r11 # RFLAGS
> + movq 32(%rsp),%rsp # RSP
> je 1f
> sysretq
> 1: sysretl
> @@ -101,8 +99,7 @@ failsafe_callback:
> ALIGN
> /* No special register assumptions. */
> restore_all_xen:
> - RESTORE_ALL
> - addq $8,%rsp
> + RESTORE_ALL adj=8
> iretq
>
> /*
> @@ -311,8 +308,7 @@ ENTRY(int80_direct_trap)
> UNLIKELY_START(ne, msi_check)
> movl $0x80,%edi
> call check_for_unexpected_msi
> - RESTORE_ALL
> - SAVE_ALL
> + LOAD_C_CLOBBERED
> UNLIKELY_END(msi_check)
>
> GET_CURRENT(%rbx)
> --- a/xen/include/asm-x86/x86_64/asm_defns.h
> +++ b/xen/include/asm-x86/x86_64/asm_defns.h
> @@ -5,11 +5,11 @@
>
> #ifdef CONFIG_FRAME_POINTER
> /* Indicate special exception stack frame by inverting the frame pointer. */
> -#define SETUP_EXCEPTION_FRAME_POINTER \
> - movq %rsp,%rbp; \
> +#define SETUP_EXCEPTION_FRAME_POINTER(offs) \
> + leaq offs(%rsp),%rbp; \
> notq %rbp
> #else
> -#define SETUP_EXCEPTION_FRAME_POINTER
> +#define SETUP_EXCEPTION_FRAME_POINTER(off)
> #endif
>
> #ifndef NDEBUG
> @@ -27,40 +27,49 @@
> #define ASSERT_INTERRUPTS_DISABLED ASSERT_INTERRUPT_STATUS(z)
>
> #define SAVE_ALL \
> + addq $-(UREGS_error_code-UREGS_r15), %rsp; \
> cld; \
> - pushq %rdi; \
> - pushq %rsi; \
> - pushq %rdx; \
> - pushq %rcx; \
> - pushq %rax; \
> - pushq %r8; \
> - pushq %r9; \
> - pushq %r10; \
> - pushq %r11; \
> - pushq %rbx; \
> - pushq %rbp; \
> - SETUP_EXCEPTION_FRAME_POINTER; \
> - pushq %r12; \
> - pushq %r13; \
> - pushq %r14; \
> - pushq %r15;
> -
> -#define RESTORE_ALL \
> - popq %r15; \
> - popq %r14; \
> - popq %r13; \
> - popq %r12; \
> - popq %rbp; \
> - popq %rbx; \
> - popq %r11; \
> - popq %r10; \
> - popq %r9; \
> - popq %r8; \
> - popq %rax; \
> - popq %rcx; \
> - popq %rdx; \
> - popq %rsi; \
> - popq %rdi;
> + movq %rdi,UREGS_rdi(%rsp); \
> + movq %rsi,UREGS_rsi(%rsp); \
> + movq %rdx,UREGS_rdx(%rsp); \
> + movq %rcx,UREGS_rcx(%rsp); \
> + movq %rax,UREGS_rax(%rsp); \
> + movq %r8,UREGS_r8(%rsp); \
> + movq %r9,UREGS_r9(%rsp); \
> + movq %r10,UREGS_r10(%rsp); \
> + movq %r11,UREGS_r11(%rsp); \
> + movq %rbx,UREGS_rbx(%rsp); \
> + movq %rbp,UREGS_rbp(%rsp); \
> + SETUP_EXCEPTION_FRAME_POINTER(UREGS_rbp); \
> + movq %r12,UREGS_r12(%rsp); \
> + movq %r13,UREGS_r13(%rsp); \
> + movq %r14,UREGS_r14(%rsp); \
> + movq %r15,UREGS_r15(%rsp); \
> +
> +#ifdef __ASSEMBLY__
> +.macro LOAD_C_CLOBBERED
> + movq UREGS_r11(%rsp),%r11
> + movq UREGS_r10(%rsp),%r10
> + movq UREGS_r9(%rsp),%r9
> + movq UREGS_r8(%rsp),%r8
> + movq UREGS_rax(%rsp),%rax
> + movq UREGS_rcx(%rsp),%rcx
> + movq UREGS_rdx(%rsp),%rdx
> + movq UREGS_rsi(%rsp),%rsi
> + movq UREGS_rdi(%rsp),%rdi
> +.endm
> +
> +.macro RESTORE_ALL adj=0
> + movq UREGS_r15(%rsp),%r15
> + movq UREGS_r14(%rsp),%r14
> + movq UREGS_r13(%rsp),%r13
> + movq UREGS_r12(%rsp),%r12
> + movq UREGS_rbp(%rsp),%rbp
> + movq UREGS_rbx(%rsp),%rbx
> + LOAD_C_CLOBBERED
> + subq $-(UREGS_error_code-UREGS_r15+\adj), %rsp
> +.endm
> +#endif
>
> #ifdef PERF_COUNTERS
> #define PERFC_INCR(_name,_idx,_cur) \
> @@ -94,7 +103,7 @@
> __asm__( \
> "\n" __ALIGN_STR"\n" \
> "common_interrupt:\n\t" \
> - STR(SAVE_ALL) \
> + STR(SAVE_ALL) "\n\t" \
> "movq %rsp,%rdi\n\t" \
> "callq " STR(do_IRQ) "\n\t" \
> "jmp ret_from_intr\n");
>
>
> x86: use MOV instead of PUSH/POP when saving/restoring register state
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>
> --- a/xen/arch/x86/x86_64/compat/entry.S
> +++ b/xen/arch/x86/x86_64/compat/entry.S
> @@ -21,8 +21,7 @@ ENTRY(compat_hypercall)
> UNLIKELY_START(ne, msi_check)
> movl $HYPERCALL_VECTOR,%edi
> call check_for_unexpected_msi
> - RESTORE_ALL
> - SAVE_ALL
> + LOAD_C_CLOBBERED
> UNLIKELY_END(msi_check)
>
> GET_CURRENT(%rbx)
> @@ -173,8 +172,7 @@ compat_bad_hypercall:
> /* %rbx: struct vcpu, interrupts disabled */
> compat_restore_all_guest:
> ASSERT_INTERRUPTS_DISABLED
> - RESTORE_ALL
> - addq $8,%rsp
> + RESTORE_ALL adj=8
> .Lft0: iretq
>
> .section .fixup,"ax"
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -47,12 +47,10 @@ restore_all_guest:
> cmpl $1,%ecx
> ja .Lforce_iret
>
> - addq $8,%rsp
> - popq %rcx # RIP
> - popq %r11 # CS
> - cmpw $FLAT_USER_CS32,%r11
> - popq %r11 # RFLAGS
> - popq %rsp # RSP
> + cmpw $FLAT_USER_CS32,16(%rsp)# CS
> + movq 8(%rsp),%rcx # RIP
> + movq 24(%rsp),%r11 # RFLAGS
> + movq 32(%rsp),%rsp # RSP
> je 1f
> sysretq
> 1: sysretl
> @@ -101,8 +99,7 @@ failsafe_callback:
> ALIGN
> /* No special register assumptions. */
> restore_all_xen:
> - RESTORE_ALL
> - addq $8,%rsp
> + RESTORE_ALL adj=8
> iretq
>
> /*
> @@ -311,8 +308,7 @@ ENTRY(int80_direct_trap)
> UNLIKELY_START(ne, msi_check)
> movl $0x80,%edi
> call check_for_unexpected_msi
> - RESTORE_ALL
> - SAVE_ALL
> + LOAD_C_CLOBBERED
> UNLIKELY_END(msi_check)
>
> GET_CURRENT(%rbx)
> --- a/xen/include/asm-x86/x86_64/asm_defns.h
> +++ b/xen/include/asm-x86/x86_64/asm_defns.h
> @@ -5,11 +5,11 @@
>
> #ifdef CONFIG_FRAME_POINTER
> /* Indicate special exception stack frame by inverting the frame pointer. */
> -#define SETUP_EXCEPTION_FRAME_POINTER \
> - movq %rsp,%rbp; \
> +#define SETUP_EXCEPTION_FRAME_POINTER(offs) \
> + leaq offs(%rsp),%rbp; \
> notq %rbp
> #else
> -#define SETUP_EXCEPTION_FRAME_POINTER
> +#define SETUP_EXCEPTION_FRAME_POINTER(off)
> #endif
>
> #ifndef NDEBUG
> @@ -27,40 +27,49 @@
> #define ASSERT_INTERRUPTS_DISABLED ASSERT_INTERRUPT_STATUS(z)
>
> #define SAVE_ALL \
> + addq $-(UREGS_error_code-UREGS_r15), %rsp; \
> cld; \
> - pushq %rdi; \
> - pushq %rsi; \
> - pushq %rdx; \
> - pushq %rcx; \
> - pushq %rax; \
> - pushq %r8; \
> - pushq %r9; \
> - pushq %r10; \
> - pushq %r11; \
> - pushq %rbx; \
> - pushq %rbp; \
> - SETUP_EXCEPTION_FRAME_POINTER; \
> - pushq %r12; \
> - pushq %r13; \
> - pushq %r14; \
> - pushq %r15;
> -
> -#define RESTORE_ALL \
> - popq %r15; \
> - popq %r14; \
> - popq %r13; \
> - popq %r12; \
> - popq %rbp; \
> - popq %rbx; \
> - popq %r11; \
> - popq %r10; \
> - popq %r9; \
> - popq %r8; \
> - popq %rax; \
> - popq %rcx; \
> - popq %rdx; \
> - popq %rsi; \
> - popq %rdi;
> + movq %rdi,UREGS_rdi(%rsp); \
> + movq %rsi,UREGS_rsi(%rsp); \
> + movq %rdx,UREGS_rdx(%rsp); \
> + movq %rcx,UREGS_rcx(%rsp); \
> + movq %rax,UREGS_rax(%rsp); \
> + movq %r8,UREGS_r8(%rsp); \
> + movq %r9,UREGS_r9(%rsp); \
> + movq %r10,UREGS_r10(%rsp); \
> + movq %r11,UREGS_r11(%rsp); \
> + movq %rbx,UREGS_rbx(%rsp); \
> + movq %rbp,UREGS_rbp(%rsp); \
> + SETUP_EXCEPTION_FRAME_POINTER(UREGS_rbp); \
> + movq %r12,UREGS_r12(%rsp); \
> + movq %r13,UREGS_r13(%rsp); \
> + movq %r14,UREGS_r14(%rsp); \
> + movq %r15,UREGS_r15(%rsp); \
> +
> +#ifdef __ASSEMBLY__
> +.macro LOAD_C_CLOBBERED
> + movq UREGS_r11(%rsp),%r11
> + movq UREGS_r10(%rsp),%r10
> + movq UREGS_r9(%rsp),%r9
> + movq UREGS_r8(%rsp),%r8
> + movq UREGS_rax(%rsp),%rax
> + movq UREGS_rcx(%rsp),%rcx
> + movq UREGS_rdx(%rsp),%rdx
> + movq UREGS_rsi(%rsp),%rsi
> + movq UREGS_rdi(%rsp),%rdi
> +.endm
> +
> +.macro RESTORE_ALL adj=0
> + movq UREGS_r15(%rsp),%r15
> + movq UREGS_r14(%rsp),%r14
> + movq UREGS_r13(%rsp),%r13
> + movq UREGS_r12(%rsp),%r12
> + movq UREGS_rbp(%rsp),%rbp
> + movq UREGS_rbx(%rsp),%rbx
> + LOAD_C_CLOBBERED
> + subq $-(UREGS_error_code-UREGS_r15+\adj), %rsp
> +.endm
> +#endif
>
> #ifdef PERF_COUNTERS
> #define PERFC_INCR(_name,_idx,_cur) \
> @@ -94,7 +103,7 @@
> __asm__( \
> "\n" __ALIGN_STR"\n" \
> "common_interrupt:\n\t" \
> - STR(SAVE_ALL) \
> + STR(SAVE_ALL) "\n\t" \
> "movq %rsp,%rdi\n\t" \
> "callq " STR(do_IRQ) "\n\t" \
> "jmp ret_from_intr\n");
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
next prev parent reply other threads:[~2012-10-02 17:57 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2012-10-02 15:20 [PATCH 0/3] x86: adjust entry frame generation Jan Beulich
2012-10-02 15:26 ` [PATCH 1/3] x86: use MOV instead of PUSH/POP when saving/restoring register state Jan Beulich
2012-10-02 17:01 ` Keir Fraser
2012-10-02 17:57 ` Konrad Rzeszutek Wilk [this message]
2012-10-03 13:01 ` Jan Beulich
2012-10-02 15:26 ` [PATCH 2/3] x86: consolidate frame state manipulation functions Jan Beulich
2012-10-02 17:01 ` Keir Fraser
2012-10-02 15:27 ` [PATCH 3/3] x86: save/restore only partial register state where possible Jan Beulich
2012-10-02 17:02 ` Keir Fraser
2012-10-03 13:05 ` Jan Beulich
2012-10-03 14:35 ` Keir Fraser
2012-10-30 14:26 ` Jan Beulich
2012-10-30 14:20 ` [PATCH 0/2, v2] x86: adjust entry frame generation Jan Beulich
2012-10-30 14:27 ` [PATCH 1/2, v2] x86: use MOV instead of PUSH/POP when saving/restoring register state Jan Beulich
2012-10-30 15:19 ` Mats Petersson
2012-10-30 14:36 ` Keir Fraser
2012-10-30 15:33 ` Jan Beulich
2012-10-30 14:29 ` [PATCH 2/2, v2] x86: save/restore only partial register state where possible Jan Beulich
2012-10-30 14:35 ` [PATCH 0/2, v2] x86: adjust entry frame generation Keir Fraser
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20121002175716.GA30321@phenom.dumpdata.com \
--to=konrad@kernel.org \
--cc=JBeulich@suse.com \
--cc=xen-devel@lists.xen.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).