Re: [PATCH 2/3] xen/domain_page: Convert copy/clear_domain_page() to using mfn_t

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

On 01/07/15 14:41, Ben Catterall wrote:
> From: Andrew Cooper <andrew.cooper3@citrix.com>
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> [Convert grant_table.c to pass mfn_t types and fix ARM compiling]
>
> Signed-off-by: Ben Catterall <Ben.Catterall@citrix.com>

Reviwed-by: Andrew Cooper <andrew.cooper3@citrix.com> for the additions
beyond my half of the patch.

Also CC'ing George and David as maintainers of areas touched.

~Andrew

> ---
>  xen/arch/x86/mm.c             |  7 ++++---
>  xen/common/grant_table.c      |  2 +-
>  xen/common/kimage.c           | 12 ++++++------
>  xen/common/memory.c           | 12 +++++-------
>  xen/include/xen/domain_page.h | 15 ++++++---------
>  5 files changed, 22 insertions(+), 26 deletions(-)
>
> diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
> index 9e08c9b..8a7524f 100644
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -3294,7 +3294,7 @@ long do_mmuext_op(
>              /* A page is dirtied when it's being cleared. */
>              paging_mark_dirty(pg_owner, page_to_mfn(page));
>  
> -            clear_domain_page(page_to_mfn(page));
> +            clear_domain_page(_mfn(page_to_mfn(page)));
>  
>              put_page_and_type(page);
>              break;
> @@ -3328,7 +3328,8 @@ long do_mmuext_op(
>              /* A page is dirtied when it's being copied to. */
>              paging_mark_dirty(pg_owner, page_to_mfn(dst_page));
>  
> -            copy_domain_page(page_to_mfn(dst_page), page_to_mfn(src_page));
> +            copy_domain_page(_mfn(page_to_mfn(dst_page)),
> +                             _mfn(page_to_mfn(src_page)));
>  
>              put_page_and_type(dst_page);
>              put_page(src_page);
> @@ -6005,7 +6006,7 @@ int create_perdomain_mapping(struct domain *d, unsigned long va,
>              pg = alloc_domheap_page(d, MEMF_no_owner);
>              if ( pg )
>              {
> -                clear_domain_page(page_to_mfn(pg));
> +                clear_domain_page(_mfn(page_to_mfn(pg)));
>                  if ( !IS_NIL(ppg) )
>                      *ppg++ = pg;
>                  l1tab[l1_table_offset(va)] =
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index a011276..9786ecd 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1732,7 +1732,7 @@ gnttab_transfer(
>                  goto unlock_and_copyback;
>              }
>  
> -            copy_domain_page(page_to_mfn(new_page), mfn);
> +            copy_domain_page(_mfn(page_to_mfn(new_page)), _mfn(mfn));
>  
>              page->count_info &= ~(PGC_count_mask|PGC_allocated);
>              free_domheap_page(page);
> diff --git a/xen/common/kimage.c b/xen/common/kimage.c
> index 8c4854d..742e4e8 100644
> --- a/xen/common/kimage.c
> +++ b/xen/common/kimage.c
> @@ -77,7 +77,7 @@ static struct page_info *kimage_alloc_zeroed_page(unsigned memflags)
>      if ( !page )
>          return NULL;
>  
> -    clear_domain_page(page_to_mfn(page));
> +    clear_domain_page(_mfn(page_to_mfn(page)));
>  
>      return page;
>  }
> @@ -409,7 +409,7 @@ static struct page_info *kimage_alloc_crash_control_page(struct kexec_image *ima
>      if ( page )
>      {
>          image->next_crash_page = hole_end;
> -        clear_domain_page(page_to_mfn(page));
> +        clear_domain_page(_mfn(page_to_mfn(page)));
>      }
>  
>      return page;
> @@ -637,15 +637,15 @@ static struct page_info *kimage_alloc_page(struct kexec_image *image,
>          if ( old )
>          {
>              /* If so move it. */
> -            unsigned long old_mfn = *old >> PAGE_SHIFT;
> -            unsigned long mfn = addr >> PAGE_SHIFT;
> +            mfn_t old_mfn = _mfn(*old >> PAGE_SHIFT);
> +            mfn_t mfn = _mfn(addr >> PAGE_SHIFT);
>  
>              copy_domain_page(mfn, old_mfn);
>              clear_domain_page(old_mfn);
>              *old = (addr & ~PAGE_MASK) | IND_SOURCE;
>              unmap_domain_page(old);
>  
> -            page = mfn_to_page(old_mfn);
> +            page = mfn_to_page(mfn_x(old_mfn));
>              break;
>          }
>          else
> @@ -917,7 +917,7 @@ int kimage_build_ind(struct kexec_image *image, unsigned long ind_mfn,
>                  goto done;
>              }
>  
> -            copy_domain_page(page_to_mfn(xen_page), mfn);
> +            copy_domain_page(_mfn(page_to_mfn(xen_page)), _mfn(mfn));
>              put_page(guest_page);
>  
>              ret = kimage_add_page(image, page_to_maddr(xen_page));
> diff --git a/xen/common/memory.c b/xen/common/memory.c
> index c84fcdd..ae4c32e 100644
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -1170,25 +1170,23 @@ long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>      return rc;
>  }
>  
> -#ifdef CONFIG_DOMAIN_PAGE
> -void clear_domain_page(unsigned long mfn)
> +void clear_domain_page(mfn_t mfn)
>  {
> -    void *ptr = map_domain_page(mfn);
> +    void *ptr = map_domain_page(mfn_x(mfn));
>  
>      clear_page(ptr);
>      unmap_domain_page(ptr);
>  }
>  
> -void copy_domain_page(unsigned long dmfn, unsigned long smfn)
> +void copy_domain_page(mfn_t dest, mfn_t source)
>  {
> -    const void *src = map_domain_page(smfn);
> -    void *dst = map_domain_page(dmfn);
> +    const void *src = map_domain_page(mfn_x(source));
> +    void *dst = map_domain_page(mfn_x(dest));
>  
>      copy_page(dst, src);
>      unmap_domain_page(dst);
>      unmap_domain_page(src);
>  }
> -#endif
>  
>  void destroy_ring_for_helper(
>      void **_va, struct page_info *page)
> diff --git a/xen/include/xen/domain_page.h b/xen/include/xen/domain_page.h
> index 1aac0eb..41f365c 100644
> --- a/xen/include/xen/domain_page.h
> +++ b/xen/include/xen/domain_page.h
> @@ -11,6 +11,12 @@
>  
>  #include <xen/mm.h>
>  
> +/*
> + * Clear a given page frame, or copy between two of them.
> + */
> +void clear_domain_page(mfn_t mfn);
> +void copy_domain_page(mfn_t dst, const mfn_t src);
> +
>  #ifdef CONFIG_DOMAIN_PAGE
>  
>  /*
> @@ -25,12 +31,6 @@ void *map_domain_page(unsigned long mfn);
>   */
>  void unmap_domain_page(const void *va);
>  
> -/*
> - * Clear a given page frame, or copy between two of them.
> - */
> -void clear_domain_page(unsigned long mfn);
> -void copy_domain_page(unsigned long dmfn, unsigned long smfn);
> -
>  /* 
>   * Given a VA from map_domain_page(), return its underlying MFN.
>   */
> @@ -116,9 +116,6 @@ domain_mmap_cache_destroy(struct domain_mmap_cache *cache)
>  #define map_domain_page(mfn)                mfn_to_virt(mfn)
>  #define __map_domain_page(pg)               page_to_virt(pg)
>  #define unmap_domain_page(va)               ((void)(va))
> -#define clear_domain_page(mfn)              clear_page(mfn_to_virt(mfn))
> -#define copy_domain_page(dmfn, smfn)        copy_page(mfn_to_virt(dmfn), \
> -                                                      mfn_to_virt(smfn))
>  #define domain_page_map_to_mfn(va)          virt_to_mfn((unsigned long)(va))
>  
>  static inline void *map_domain_page_global(mfn_t mfn)