Re: [Xen-devel] [PATCH for-xen-4.5 v3 03/16] x86/boot/reloc: Create generic alloc and copy functions

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 08/10/14 18:52, Daniel Kiper wrote:
> Create generic alloc and copy functions. We need them to
> introduce MBD struct and multiboot2 protocol. Please
> check later patches for more details.
>
> Signed-off-by: Daniel Kiper <daniel.kiper@xxxxxxxxxx>

copy_string() is fine, as all its uses are in terms of u32s, but why do
the other two change from pointers to u32s now?

You could drop almost all of the casts with the reintroduction of the
void pointers.  This code, after all, is only ever going to be 32bit.

~Andrew

> ---
>  xen/arch/x86/boot/reloc.c |   52 
> +++++++++++++++++++++++++++++----------------
>  1 file changed, 34 insertions(+), 18 deletions(-)
>
> diff --git a/xen/arch/x86/boot/reloc.c b/xen/arch/x86/boot/reloc.c
> index 0c03291..b678f67 100644
> --- a/xen/arch/x86/boot/reloc.c
> +++ b/xen/arch/x86/boot/reloc.c
> @@ -34,9 +34,10 @@ asm (
>      "    .long 0                       \n"
>      );
>  
> -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"
> @@ -44,50 +45,65 @@ 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"
> +       : "=&r" (s) : "r" (bytes) : "edx");
> +
> +    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                   \n"
> -       : "=&r" (new), "+c" (bytes), "+S" (old)
> -     : : "edx", "edi");
> -    return new;
> +       : "+S" (src), "+D" (dst_asm), "+c" (bytes));
> +
> +    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 |


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