Re: [Xen-devel] [RFC 02/14] xen/arm: Remove the parameter "attrindx" in copy_paddr

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Wed, 2014-03-12 at 16:15 +0000, Julien Grall wrote:
> copy_addr

s/copy_p?addr/copy_from_paddr/ throughout the subject and changelog.

>  is only used with BUFFERABLE, there is some place where DEV_SHARED
> was used by mistake.

A hangover from flash support or something else?

> The parameter "attrindx" can be safely remove and let copy_paddr to map
> every page with BUFFERABLE attribute.

What about if e.g. the firmware provides a dtb which is in flash or
where the kernel/initrd which it references are in flash. There's
nothing inherently wrong with that is there? Maybe BUFFERABLE is still
appropriate there for r/o use.

> 
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c |    2 +-
>  xen/arch/arm/kernel.c       |   15 +++++++--------
>  xen/arch/arm/setup.c        |    2 +-
>  xen/include/asm-arm/setup.h |    2 +-
>  4 files changed, 10 insertions(+), 11 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 5ca2f15..7bb2c28 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -976,7 +976,7 @@ static void initrd_load(struct kernel_info *kinfo)
>  
>          dst = map_domain_page(ma>>PAGE_SHIFT);
>  
> -        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
> +        copy_from_paddr(dst + s, paddr + offs, l);
>  
>          unmap_domain_page(dst);
>          offs += l;
> diff --git a/xen/arch/arm/kernel.c b/xen/arch/arm/kernel.c
> index 492ce6d..0bc7eb1 100644
> --- a/xen/arch/arm/kernel.c
> +++ b/xen/arch/arm/kernel.c
> @@ -40,7 +40,7 @@ struct minimal_dtb_header {
>   * @paddr: source physical address
>   * @len: length to copy
>   */
> -void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int 
> attrindx)
> +void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len)
>  {
>      void *src = (void *)FIXMAP_ADDR(FIXMAP_MISC);
>  
> @@ -52,7 +52,7 @@ void copy_from_paddr(void *dst, paddr_t paddr, unsigned 
> long len, int attrindx)
>          s = paddr & (PAGE_SIZE-1);
>          l = min(PAGE_SIZE - s, len);
>  
> -        set_fixmap(FIXMAP_MISC, p, attrindx);
> +        set_fixmap(FIXMAP_MISC, p, BUFFERABLE);
>          memcpy(dst, src + s, l);
>          clean_xen_dcache_va_range(dst, l);
>  
> @@ -145,7 +145,7 @@ static void kernel_zimage_load(struct kernel_info *info)
>  
>          dst = map_domain_page(ma>>PAGE_SHIFT);
>  
> -        copy_from_paddr(dst + s, paddr + offs, l, BUFFERABLE);
> +        copy_from_paddr(dst + s, paddr + offs, l);
>  
>          unmap_domain_page(dst);
>          offs += l;
> @@ -178,7 +178,7 @@ static int kernel_try_zimage64_prepare(struct kernel_info 
> *info,
>      if ( size < sizeof(zimage) )
>          return -EINVAL;
>  
> -    copy_from_paddr(&zimage, addr, sizeof(zimage), DEV_SHARED);
> +    copy_from_paddr(&zimage, addr, sizeof(zimage));
>  
>      if ( zimage.magic0 != ZIMAGE64_MAGIC_V0 &&
>           zimage.magic1 != ZIMAGE64_MAGIC_V1 )
> @@ -223,7 +223,7 @@ static int kernel_try_zimage32_prepare(struct kernel_info 
> *info,
>      if ( size < ZIMAGE32_HEADER_LEN )
>          return -EINVAL;
>  
> -    copy_from_paddr(zimage, addr, sizeof(zimage), DEV_SHARED);
> +    copy_from_paddr(zimage, addr, sizeof(zimage));
>  
>      if (zimage[ZIMAGE32_MAGIC_OFFSET/4] != ZIMAGE32_MAGIC)
>          return -EINVAL;
> @@ -239,8 +239,7 @@ static int kernel_try_zimage32_prepare(struct kernel_info 
> *info,
>       */
>      if ( addr + end - start + sizeof(dtb_hdr) <= size )
>      {
> -        copy_from_paddr(&dtb_hdr, addr + end - start,
> -                        sizeof(dtb_hdr), DEV_SHARED);
> +        copy_from_paddr(&dtb_hdr, addr + end - start, sizeof(dtb_hdr));
>          if (be32_to_cpu(dtb_hdr.magic) == DTB_MAGIC) {
>              end += be32_to_cpu(dtb_hdr.total_size);
>  
> @@ -311,7 +310,7 @@ static int kernel_try_elf_prepare(struct kernel_info 
> *info,
>      if ( info->kernel_img == NULL )
>          panic("Cannot allocate temporary buffer for kernel");
>  
> -    copy_from_paddr(info->kernel_img, addr, size, BUFFERABLE);
> +    copy_from_paddr(info->kernel_img, addr, size);
>  
>      if ( (rc = elf_init(&info->elf.elf, info->kernel_img, size )) != 0 )
>          goto err;
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 9480f42..959744e 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -488,7 +488,7 @@ static void __init setup_mm(unsigned long dtb_paddr, 
> size_t dtb_size)
>  
>      /* Copy the DTB. */
>      fdt = mfn_to_virt(alloc_boot_pages(dtb_pages, 1));
> -    copy_from_paddr(fdt, dtb_paddr, dtb_size, BUFFERABLE);
> +    copy_from_paddr(fdt, dtb_paddr, dtb_size);
>      device_tree_flattened = fdt;
>  
>      /* Add non-xenheap memory */
> diff --git a/xen/include/asm-arm/setup.h b/xen/include/asm-arm/setup.h
> index 44a3b4d..b09f688 100644
> --- a/xen/include/asm-arm/setup.h
> +++ b/xen/include/asm-arm/setup.h
> @@ -5,7 +5,7 @@
>  
>  void arch_init_memory(void);
>  
> -void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len, int 
> attrindx);
> +void copy_from_paddr(void *dst, paddr_t paddr, unsigned long len);
>  
>  void arch_get_xen_caps(xen_capabilities_info_t *info);
>  



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