[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 04/19] x86/boot/reloc: reduce assembly usage as much as possible



>>> On 06.08.16 at 01:04, <daniel.kiper@xxxxxxxxxx> wrote:
> Next patch will leave just required jmp instruction
> in xen/x86/boot/reloc.c.

I can't make sense of this now, and it'll get even more problematic
for archaeologists if the two patches don't get committed one right
after the other. Please instead describe what _this_ patch does
and why.

> --- a/xen/arch/x86/boot/build32.lds
> +++ b/xen/arch/x86/boot/build32.lds
> @@ -24,6 +24,7 @@ SECTIONS
>          *(.text)
>          *(.text.*)
>          *(.rodata)
> +        *(.bss)

The suggested change to the earlier patch would make this
unnecessary, but here you get to see even more clearly why
picking just a few sections is bogus.

>  static void *reloc_mbi_struct(void *old, unsigned int bytes)
>  {
>      void *new;
> -    asm(
> -    "    call 1f                      \n"
> -    "1:  pop  %%edx                   \n"
> -    "    mov  alloc-1b(%%edx),%0      \n"
> -    "    sub  %1,%0                   \n"
> -    "    and  $~15,%0                 \n"
> -    "    mov  %0,alloc-1b(%%edx)      \n"
> -    "    mov  %0,%%edi                \n"
> -    "    rep  movsb                   \n"
> -       : "=&r" (new), "+c" (bytes), "+S" (old)
> -     : : "edx", "edi", "memory");
> -    return new;
> +
> +    alloc -= ALIGN_UP(bytes, 16);
> +    new = (void *)alloc;
> +
> +    while ( bytes-- )
> +        *(char *)new++ = *(char *)old++;
> +
> +    return (void *)alloc;
>  }

To further cut down the number of casts, what about making new
have type char * and doing

    while ( bytes-- )
        new[bytes] = ((char *)old)[bytes];

    return new;

One might even argue old could also be of type char * (and actually
be const), but that would only move the cast into the caller. Yet
perhaps that's still better readable than the expression above.

And then, maybe the code could even mostly stay as it is: Is there
anything keeping alloc from being of type void *?

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.