From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH for-xen-4.5 v3 03/16] x86/boot/reloc: Create generic alloc and copy functions Date: Thu, 9 Oct 2014 10:45:08 +0100 Message-ID: <54365924.2060403@citrix.com> References: <1412790736-28915-1-git-send-email-daniel.kiper@oracle.com> <1412790736-28915-4-git-send-email-daniel.kiper@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XcAHb-0007zA-Jb for xen-devel@lists.xenproject.org; Thu, 09 Oct 2014 09:45:15 +0000 In-Reply-To: <1412790736-28915-4-git-send-email-daniel.kiper@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Daniel Kiper , xen-devel@lists.xenproject.org Cc: jgross@suse.com, keir@xen.org, ian.campbell@citrix.com, stefano.stabellini@eu.citrix.com, ross.philipson@citrix.com, roy.franz@linaro.org, ning.sun@intel.com, jbeulich@suse.com, qiaowei.ren@intel.com, richard.l.maliszewski@intel.com, gang.wei@intel.com, fu.wei@linaro.org List-Id: xen-devel@lists.xenproject.org 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 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 |