Re: [PATCH 02/18] x86/boot/reloc: create generic alloc and copy functions

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 30/01/15 17:54, Daniel Kiper wrote:
> Create generic alloc and copy functions. We need
> separate tools for memory allocation and copy to
> provide multiboot2 protocol support.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@oracle.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

> ---
>  xen/arch/x86/boot/reloc.c |   52 ++++++++++++++++++++++++++++-----------------
>  1 file changed, 33 insertions(+), 19 deletions(-)
>
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index 63045c0..f964cda 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -33,9 +33,10 @@ asm (
>  typedef unsigned int u32;
>  #include "../../../include/xen/multiboot.h"
>  
> -static void *reloc_mbi_struct(void *old, unsigned int bytes)
> +static u32 alloc_struct(u32 bytes)
>  {
> -    void *new;
> +    u32 s;
> +
>      asm(
>      "    call 1f                      \n"
>      "1:  pop  %%edx                   \n"
> @@ -43,50 +44,63 @@ static void *reloc_mbi_struct(void *old, unsigned int bytes)
>      "    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;
> +       : "=&r" (s) : "r" (bytes) : "edx", "memory");
> +
> +    return s;
> +}
> +
> +static u32 copy_struct(u32 src, u32 bytes)
> +{
> +    u32 dst, dst_asm;
> +
> +    dst = alloc_struct(bytes);
> +    dst_asm = dst;
> +
> +    asm volatile("rep movsb" : "+S" (src), "+D" (dst_asm), "+c" (bytes) : : "memory");
> +
> +    return dst;
>  }
>  
> -static char *reloc_mbi_string(char *old)
> +static u32 copy_string(u32 src)
>  {
>      char *p;
> -    for ( p = old; *p != '\0'; p++ )
> +
> +    if ( src == 0 )
> +        return 0;
> +
> +    for ( p = (char *)src; *p != '\0'; p++ )
>          continue;
> -    return reloc_mbi_struct(old, p - old + 1);
> +
> +    return copy_struct(src, p - (char *)src + 1);
>  }
>  
>  multiboot_info_t *reloc(multiboot_info_t *mbi_old)
>  {
> -    multiboot_info_t *mbi = reloc_mbi_struct(mbi_old, sizeof(*mbi));
> +    multiboot_info_t *mbi = (multiboot_info_t *)copy_struct((u32)mbi_old, sizeof(*mbi));
>      int i;
>  
>      if ( mbi->flags & MBI_CMDLINE )
> -        mbi->cmdline = (u32)reloc_mbi_string((char *)mbi->cmdline);
> +        mbi->cmdline = copy_string(mbi->cmdline);
>  
>      if ( mbi->flags & MBI_MODULES )
>      {
> -        module_t *mods = reloc_mbi_struct(
> -            (module_t *)mbi->mods_addr, mbi->mods_count * sizeof(module_t));
> +        module_t *mods = (module_t *)copy_struct(
> +            mbi->mods_addr, mbi->mods_count * sizeof(module_t));
>  
>          mbi->mods_addr = (u32)mods;
>  
>          for ( i = 0; i < mbi->mods_count; i++ )
>          {
>              if ( mods[i].string )
> -                mods[i].string = (u32)reloc_mbi_string((char *)mods[i].string);
> +                mods[i].string = copy_string(mods[i].string);
>          }
>      }
>  
>      if ( mbi->flags & MBI_MEMMAP )
> -        mbi->mmap_addr = (u32)reloc_mbi_struct(
> -            (memory_map_t *)mbi->mmap_addr, mbi->mmap_length);
> +        mbi->mmap_addr = copy_struct(mbi->mmap_addr, mbi->mmap_length);
>  
>      if ( mbi->flags & MBI_LOADERNAME )
> -        mbi->boot_loader_name = (u32)reloc_mbi_string(
> -            (char *)mbi->boot_loader_name);
> +        mbi->boot_loader_name = copy_string(mbi->boot_loader_name);
>  
>      /* Mask features we don't understand or don't relocate. */
>      mbi->flags &= (MBI_MEMLIMITS |