Re: [Xen-devel] [PATCH] xen/x86: Improve hypercall page writing

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 19.12.16 at 17:55, <andrew.cooper3@xxxxxxxxxx> 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@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel