* [PATCH] xen/x86: Improve hypercall page writing
@ 2016-12-19 16:55 Andrew Cooper
2016-12-19 19:14 ` Boris Ostrovsky
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Andrew Cooper @ 2016-12-19 16:55 UTC (permalink / raw)
To: Xen-devel
Cc: Kevin Tian, Jan Beulich, Andrew Cooper, Jun Nakajima,
Boris Ostrovsky, Suravee Suthikulpanit
Use memcpy() rather than individual writes with explicit casts. The
__builtin_memcpy() wrapper does a better job at combining adjacent writes into
a larger word size.
This results in better generated assembly. No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Jun Nakajima <jun.nakajima@intel.com>
CC: Kevin Tian <kevin.tian@intel.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com>
---
xen/arch/x86/hvm/svm/svm.c | 10 +++++-----
xen/arch/x86/hvm/vmx/vmx.c | 10 +++++-----
xen/arch/x86/x86_64/compat/traps.c | 15 +++++++++------
xen/arch/x86/x86_64/traps.c | 31 ++++++++++++++++++-------------
4 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/svm.c b/xen/arch/x86/hvm/svm/svm.c
index 811ea4e..962667a 100644
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -888,12 +888,12 @@ static void svm_init_hypercall_page(struct domain *d, void *hypercall_page)
continue;
p = (char *)(hypercall_page + (i * 32));
- *(u8 *)(p + 0) = 0xb8; /* mov imm32, %eax */
+ *(u8 *)(p + 0) = 0xb8; /* mov $<i>, %eax */
*(u32 *)(p + 1) = i;
- *(u8 *)(p + 5) = 0x0f; /* vmmcall */
- *(u8 *)(p + 6) = 0x01;
- *(u8 *)(p + 7) = 0xd9;
- *(u8 *)(p + 8) = 0xc3; /* ret */
+ memcpy(p + 5,
+ "\x0f\x01\xd9" /* vmmcall */
+ "\xc3", /* ret */
+ 4);
}
/* Don't support HYPERVISOR_iret at the moment */
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index d50d49e..c0e62a2 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1295,12 +1295,12 @@ static void vmx_init_hypercall_page(struct domain *d, void *hypercall_page)
continue;
p = (char *)(hypercall_page + (i * 32));
- *(u8 *)(p + 0) = 0xb8; /* mov imm32, %eax */
+ *(u8 *)(p + 0) = 0xb8; /* mov $<i>, %eax */
*(u32 *)(p + 1) = i;
- *(u8 *)(p + 5) = 0x0f; /* vmcall */
- *(u8 *)(p + 6) = 0x01;
- *(u8 *)(p + 7) = 0xc1;
- *(u8 *)(p + 8) = 0xc3; /* ret */
+ memcpy(p + 5,
+ "\x0f\x01\xc1" /* vmmcall */
+ "\xc3", /* ret */
+ 4);
}
/* Don't support HYPERVISOR_iret at the moment */
diff --git a/xen/arch/x86/x86_64/compat/traps.c b/xen/arch/x86/x86_64/compat/traps.c
index 95c5cb3..8b03291 100644
--- a/xen/arch/x86/x86_64/compat/traps.c
+++ b/xen/arch/x86/x86_64/compat/traps.c
@@ -388,8 +388,10 @@ static void hypercall_page_initialise_ring1_kernel(void *hypercall_page)
p = (char *)(hypercall_page + (i * 32));
*(u8 *)(p+ 0) = 0xb8; /* mov $<i>,%eax */
*(u32 *)(p+ 1) = i;
- *(u16 *)(p+ 5) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
- *(u8 *)(p+ 7) = 0xc3; /* ret */
+ memcpy(p + 5,
+ "\xcd\x82" /* int $HYPERCALL_VECTOR */
+ "\xc3", /* ret */
+ 3);
}
/*
@@ -398,10 +400,11 @@ static void hypercall_page_initialise_ring1_kernel(void *hypercall_page)
* calling it.
*/
p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
- *(u8 *)(p+ 0) = 0x50; /* push %eax */
- *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */
- *(u32 *)(p+ 2) = __HYPERVISOR_iret;
- *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
+ memcpy(p,
+ "\x50" /* push %eax */
+ "\xb8\x17\x00\x00\x00" /* mov $__HYPERVISOR_iret, %eax */
+ "\xcd\x82", /* int $HYPERCALL_VECTOR */
+ 8);
}
/*
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index fc8cde6..7a586cd 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -591,14 +591,18 @@ static void hypercall_page_initialise_ring3_kernel(void *hypercall_page)
continue;
p = (char *)(hypercall_page + (i * 32));
- *(u8 *)(p+ 0) = 0x51; /* push %rcx */
- *(u16 *)(p+ 1) = 0x5341; /* push %r11 */
- *(u8 *)(p+ 3) = 0xb8; /* mov $<i>,%eax */
+ memcpy(p,
+ "\x51" /* push %rcx */
+ "\x41\x53" /* push %r11 */
+ "\xb8", /* mov $<i>,%eax */
+ 4);
*(u32 *)(p+ 4) = i;
- *(u16 *)(p+ 8) = 0x050f; /* syscall */
- *(u16 *)(p+10) = 0x5b41; /* pop %r11 */
- *(u8 *)(p+12) = 0x59; /* pop %rcx */
- *(u8 *)(p+13) = 0xc3; /* ret */
+ memcpy(p + 8,
+ "\x0f\x05" /* syscall */
+ "\x41\x5b" /* pop %r11 */
+ "\x59" /* pop %rcx */
+ "\xc3", /* ret */
+ 6);
}
/*
@@ -607,12 +611,13 @@ static void hypercall_page_initialise_ring3_kernel(void *hypercall_page)
* calling it.
*/
p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
- *(u8 *)(p+ 0) = 0x51; /* push %rcx */
- *(u16 *)(p+ 1) = 0x5341; /* push %r11 */
- *(u8 *)(p+ 3) = 0x50; /* push %rax */
- *(u8 *)(p+ 4) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */
- *(u32 *)(p+ 5) = __HYPERVISOR_iret;
- *(u16 *)(p+ 9) = 0x050f; /* syscall */
+ memcpy(p,
+ "\x51" /* push %rcx */
+ "\x41\x53" /* push %r11 */
+ "\x50" /* push %rax */
+ "\xb8\x17\x00\x00\x00" /* mov $__HYPERVISOR_iret,%eax */
+ "\x0f\x05", /* syscall */
+ 11);
}
#include "compat/traps.c"
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: Improve hypercall page writing
2016-12-19 16:55 [PATCH] xen/x86: Improve hypercall page writing Andrew Cooper
@ 2016-12-19 19:14 ` Boris Ostrovsky
2016-12-20 5:11 ` Tian, Kevin
2016-12-20 7:48 ` Jan Beulich
2 siblings, 0 replies; 6+ messages in thread
From: Boris Ostrovsky @ 2016-12-19 19:14 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Suravee Suthikulpanit, Kevin Tian, Jun Nakajima, Jan Beulich
On 12/19/2016 11:55 AM, Andrew Cooper wrote:
> Use memcpy() rather than individual writes with explicit casts. The
> __builtin_memcpy() wrapper does a better job at combining adjacent writes into
> a larger word size.
>
> This results in better generated assembly. No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: Improve hypercall page writing
2016-12-19 16:55 [PATCH] xen/x86: Improve hypercall page writing Andrew Cooper
2016-12-19 19:14 ` Boris Ostrovsky
@ 2016-12-20 5:11 ` Tian, Kevin
2016-12-20 7:48 ` Jan Beulich
2 siblings, 0 replies; 6+ messages in thread
From: Tian, Kevin @ 2016-12-20 5:11 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Suravee Suthikulpanit, Boris Ostrovsky, Nakajima, Jun, Jan Beulich
> From: Andrew Cooper [mailto:andrew.cooper3@citrix.com]
> Sent: Tuesday, December 20, 2016 12:55 AM
>
> Use memcpy() rather than individual writes with explicit casts. The
> __builtin_memcpy() wrapper does a better job at combining adjacent writes into
> a larger word size.
>
> This results in better generated assembly. No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Acked-by: Kevin Tian <kevin.tian@intel.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: Improve hypercall page writing
2016-12-19 16:55 [PATCH] xen/x86: Improve hypercall page writing Andrew Cooper
2016-12-19 19:14 ` Boris Ostrovsky
2016-12-20 5:11 ` Tian, Kevin
@ 2016-12-20 7:48 ` Jan Beulich
2016-12-20 12:21 ` Andrew Cooper
2 siblings, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2016-12-20 7:48 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Kevin Tian, Jun Nakajima, SuraveeSuthikulpanit,
Xen-devel
>>> On 19.12.16 at 17:55, <andrew.cooper3@citrix.com> wrote:
> Use memcpy() rather than individual writes with explicit casts. The
> __builtin_memcpy() wrapper does a better job at combining adjacent writes into
> a larger word size.
>
> This results in better generated assembly. No functional change.
I don't think generated code matters much for these functions, so
I'd don't view open coding constants like ...
> --- a/xen/arch/x86/x86_64/compat/traps.c
> +++ b/xen/arch/x86/x86_64/compat/traps.c
> @@ -388,8 +388,10 @@ static void hypercall_page_initialise_ring1_kernel(void *hypercall_page)
> p = (char *)(hypercall_page + (i * 32));
> *(u8 *)(p+ 0) = 0xb8; /* mov $<i>,%eax */
> *(u32 *)(p+ 1) = i;
> - *(u16 *)(p+ 5) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
> - *(u8 *)(p+ 7) = 0xc3; /* ret */
> + memcpy(p + 5,
> + "\xcd\x82" /* int $HYPERCALL_VECTOR */
... here or ...
> @@ -398,10 +400,11 @@ static void hypercall_page_initialise_ring1_kernel(void *hypercall_page)
> * calling it.
> */
> p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
> - *(u8 *)(p+ 0) = 0x50; /* push %eax */
> - *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */
> - *(u32 *)(p+ 2) = __HYPERVISOR_iret;
> - *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
> + memcpy(p,
> + "\x50" /* push %eax */
> + "\xb8\x17\x00\x00\x00" /* mov $__HYPERVISOR_iret, %eax */
> + "\xcd\x82", /* int $HYPERCALL_VECTOR */
here as a good idea. If you used a static const uint8_t[] instead of
a string literal (which even includes a pointless nul terminator), all of
this could be avoided afaict.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: Improve hypercall page writing
2016-12-20 7:48 ` Jan Beulich
@ 2016-12-20 12:21 ` Andrew Cooper
2016-12-20 12:57 ` Jan Beulich
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Cooper @ 2016-12-20 12:21 UTC (permalink / raw)
To: Jan Beulich
Cc: Boris Ostrovsky, Kevin Tian, Jun Nakajima, SuraveeSuthikulpanit,
Xen-devel
On 20/12/2016 07:48, Jan Beulich wrote:
>
>> @@ -398,10 +400,11 @@ static void hypercall_page_initialise_ring1_kernel(void *hypercall_page)
>> * calling it.
>> */
>> p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
>> - *(u8 *)(p+ 0) = 0x50; /* push %eax */
>> - *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */
>> - *(u32 *)(p+ 2) = __HYPERVISOR_iret;
>> - *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
>> + memcpy(p,
>> + "\x50" /* push %eax */
>> + "\xb8\x17\x00\x00\x00" /* mov $__HYPERVISOR_iret, %eax */
>> + "\xcd\x82", /* int $HYPERCALL_VECTOR */
> here as a good idea.
Well, they are fixed in the ABI. It is not as if we could ever change them.
> If you used a static const uint8_t[] instead of
> a string literal (which even includes a pointless nul terminator), all of
> this could be avoided afaict.
I can see how that would work for the `int $0x82` case, but how do you
propose fitting 4 bytes of __HYPERVISOR_iret into an initialiser for a
uint8_t array?
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] xen/x86: Improve hypercall page writing
2016-12-20 12:21 ` Andrew Cooper
@ 2016-12-20 12:57 ` Jan Beulich
0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2016-12-20 12:57 UTC (permalink / raw)
To: Andrew Cooper
Cc: Boris Ostrovsky, Kevin Tian, Jun Nakajima, SuraveeSuthikulpanit,
Xen-devel
>>> On 20.12.16 at 13:21, <andrew.cooper3@citrix.com> wrote:
> On 20/12/2016 07:48, Jan Beulich wrote:
>>
>>> @@ -398,10 +400,11 @@ static void hypercall_page_initialise_ring1_kernel(void
> *hypercall_page)
>>> * calling it.
>>> */
>>> p = (char *)(hypercall_page + (__HYPERVISOR_iret * 32));
>>> - *(u8 *)(p+ 0) = 0x50; /* push %eax */
>>> - *(u8 *)(p+ 1) = 0xb8; /* mov $__HYPERVISOR_iret,%eax */
>>> - *(u32 *)(p+ 2) = __HYPERVISOR_iret;
>>> - *(u16 *)(p+ 6) = (HYPERCALL_VECTOR << 8) | 0xcd; /* int $xx */
>>> + memcpy(p,
>>> + "\x50" /* push %eax */
>>> + "\xb8\x17\x00\x00\x00" /* mov $__HYPERVISOR_iret, %eax */
>>> + "\xcd\x82", /* int $HYPERCALL_VECTOR */
>> here as a good idea.
>
> Well, they are fixed in the ABI. It is not as if we could ever change them.
>
>> If you used a static const uint8_t[] instead of
>> a string literal (which even includes a pointless nul terminator), all of
>> this could be avoided afaict.
>
> I can see how that would work for the `int $0x82` case, but how do you
> propose fitting 4 bytes of __HYPERVISOR_iret into an initialiser for a
> uint8_t array?
One byte __HYPERVISOR_iret, the other three zeros. perhaps
accompanied by a BUILD_BUG_ON(). Another alternative would
be a static const struct.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-12-20 12:57 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-12-19 16:55 [PATCH] xen/x86: Improve hypercall page writing Andrew Cooper
2016-12-19 19:14 ` Boris Ostrovsky
2016-12-20 5:11 ` Tian, Kevin
2016-12-20 7:48 ` Jan Beulich
2016-12-20 12:21 ` Andrew Cooper
2016-12-20 12:57 ` Jan Beulich
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.