[Xen-devel,for-4.11,v7,13/16] xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN

Message ID 20180403153251.19595-14-julien.grall@arm.com
State Accepted
Commit a7982ad7b03b77d96c6d9cd3eea4b5ba7f77f377
Headers show
Series
  • xen: Convert page_to_mfn and mfn_to_page to use typesafe MFN
Related show

Commit Message

Julien Grall April 3, 2018, 3:32 p.m.
The current prototype is slightly confusing because it takes a guest
physical address and a machine physical frame (not address!). Switching to
MFN will improve safety and reduce the chance to mistakenly invert the
2 parameters.

Signed-off-by: Julien grall <julien.grall@arm.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>

---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

    Changes in v5:
        - Add Wei's and Jan's reviewed-by

    Changes in v4:
        - Patch added
---
 xen/arch/arm/mm.c                     | 10 +++++-----
 xen/arch/x86/hvm/grant_table.c        | 14 +++++++-------
 xen/arch/x86/pv/grant_table.c         | 10 +++++-----
 xen/common/grant_table.c              |  8 ++++----
 xen/include/asm-arm/grant_table.h     |  9 ++++-----
 xen/include/asm-x86/grant_table.h     |  4 ++--
 xen/include/asm-x86/hvm/grant_table.h |  8 ++++----
 xen/include/asm-x86/pv/grant_table.h  |  8 ++++----
 8 files changed, 35 insertions(+), 36 deletions(-)

Comments

Stefano Stabellini April 5, 2018, 8:10 p.m. | #1
On Tue, 3 Apr 2018, Julien Grall wrote:
> The current prototype is slightly confusing because it takes a guest
> physical address and a machine physical frame (not address!). Switching to
> MFN will improve safety and reduce the chance to mistakenly invert the
> 2 parameters.
> 
> Signed-off-by: Julien grall <julien.grall@arm.com>
> Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: George Dunlap <George.Dunlap@eu.citrix.com>
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Tim Deegan <tim@xen.org>
> Cc: Wei Liu <wei.liu2@citrix.com>
> 
>     Changes in v5:
>         - Add Wei's and Jan's reviewed-by
> 
>     Changes in v4:
>         - Patch added
> ---
>  xen/arch/arm/mm.c                     | 10 +++++-----
>  xen/arch/x86/hvm/grant_table.c        | 14 +++++++-------
>  xen/arch/x86/pv/grant_table.c         | 10 +++++-----
>  xen/common/grant_table.c              |  8 ++++----
>  xen/include/asm-arm/grant_table.h     |  9 ++++-----
>  xen/include/asm-x86/grant_table.h     |  4 ++--
>  xen/include/asm-x86/hvm/grant_table.h |  8 ++++----
>  xen/include/asm-x86/pv/grant_table.h  |  8 ++++----
>  8 files changed, 35 insertions(+), 36 deletions(-)
> 
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 7af6baa3d6..49080ca0ac 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -1418,7 +1418,7 @@ void gnttab_mark_dirty(struct domain *d, unsigned long l)
>      }
>  }
>  
> -int create_grant_host_mapping(unsigned long addr, unsigned long frame,
> +int create_grant_host_mapping(unsigned long addr, mfn_t frame,
>                                unsigned int flags, unsigned int cache_flags)
>  {
>      int rc;
> @@ -1431,7 +1431,7 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame,
>          t = p2m_grant_map_ro;
>  
>      rc = guest_physmap_add_entry(current->domain, gaddr_to_gfn(addr),
> -                                 _mfn(frame), 0, t);
> +                                 frame, 0, t);
>  
>      if ( rc )
>          return GNTST_general_error;
> @@ -1439,8 +1439,8 @@ int create_grant_host_mapping(unsigned long addr, unsigned long frame,
>          return GNTST_okay;
>  }
>  
> -int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
> -        unsigned long new_addr, unsigned int flags)
> +int replace_grant_host_mapping(unsigned long addr, mfn_t mfn,
> +                               unsigned long new_addr, unsigned int flags)
>  {
>      gfn_t gfn = gaddr_to_gfn(addr);
>      struct domain *d = current->domain;
> @@ -1449,7 +1449,7 @@ int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
>      if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
>          return GNTST_general_error;
>  
> -    rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
> +    rc = guest_physmap_remove_page(d, gfn, mfn, 0);
>  
>      return rc ? GNTST_general_error : GNTST_okay;
>  }
> diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c
> index 9ca9fe0425..ecd7d078ab 100644
> --- a/xen/arch/x86/hvm/grant_table.c
> +++ b/xen/arch/x86/hvm/grant_table.c
> @@ -25,7 +25,7 @@
>  
>  #include <asm/p2m.h>
>  
> -int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>                               unsigned int flags,
>                               unsigned int cache_flags)
>  {
> @@ -41,14 +41,14 @@ int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
>          p2mt = p2m_grant_map_rw;
>      rc = guest_physmap_add_entry(current->domain,
>                                   _gfn(addr >> PAGE_SHIFT),
> -                                 _mfn(frame), PAGE_ORDER_4K, p2mt);
> +                                 frame, PAGE_ORDER_4K, p2mt);
>      if ( rc )
>          return GNTST_general_error;
>      else
>          return GNTST_okay;
>  }
>  
> -int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>                                uint64_t new_addr, unsigned int flags)
>  {
>      unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
> @@ -60,15 +60,15 @@ int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
>          return GNTST_general_error;
>  
>      old_mfn = get_gfn(d, gfn, &type);
> -    if ( !p2m_is_grant(type) || mfn_x(old_mfn) != frame )
> +    if ( !p2m_is_grant(type) || !mfn_eq(old_mfn, frame) )
>      {
>          put_gfn(d, gfn);
>          gdprintk(XENLOG_WARNING,
> -                 "old mapping invalid (type %d, mfn %" PRI_mfn ", frame %lx)\n",
> -                 type, mfn_x(old_mfn), frame);
> +                 "old mapping invalid (type %d, mfn %" PRI_mfn ", frame %"PRI_mfn")\n",
> +                 type, mfn_x(old_mfn), mfn_x(frame));
>          return GNTST_general_error;
>      }
> -    if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K) )
> +    if ( guest_physmap_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K) )
>      {
>          put_gfn(d, gfn);
>          return GNTST_general_error;
> diff --git a/xen/arch/x86/pv/grant_table.c b/xen/arch/x86/pv/grant_table.c
> index 4dbc550366..458085e1b6 100644
> --- a/xen/arch/x86/pv/grant_table.c
> +++ b/xen/arch/x86/pv/grant_table.c
> @@ -50,7 +50,7 @@ static unsigned int grant_to_pte_flags(unsigned int grant_flags,
>      return pte_flags;
>  }
>  
> -int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
>                              unsigned int flags, unsigned int cache_flags)
>  {
>      struct vcpu *curr = current;
> @@ -60,7 +60,7 @@ int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
>      mfn_t gl1mfn;
>      int rc = GNTST_general_error;
>  
> -    nl1e = l1e_from_pfn(frame, grant_to_pte_flags(flags, cache_flags));
> +    nl1e = l1e_from_mfn(frame, grant_to_pte_flags(flags, cache_flags));
>      nl1e = adjust_guest_l1e(nl1e, currd);
>  
>      /*
> @@ -192,7 +192,7 @@ static bool steal_linear_address(unsigned long linear, l1_pgentry_t *out)
>   * new_addr has only ever been available via GNTABOP_unmap_and_replace, and
>   * only when !(flags & GNTMAP_contains_pte).
>   */
> -int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
>                               uint64_t new_addr, unsigned int flags)
>  {
>      struct vcpu *curr = current;
> @@ -282,14 +282,14 @@ int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
>       * Check that the address supplied is actually mapped to frame (with
>       * appropriate permissions).
>       */
> -    if ( unlikely(l1e_get_pfn(ol1e) != frame) ||
> +    if ( unlikely(!mfn_eq(l1e_get_mfn(ol1e), frame)) ||
>           unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
>                    (_PAGE_PRESENT | _PAGE_RW)) )
>      {
>          gdprintk(XENLOG_ERR,
>                   "PTE %"PRIpte" for %"PRIx64" doesn't match grant (%"PRIpte")\n",
>                   l1e_get_intpte(ol1e), addr,
> -                 l1e_get_intpte(l1e_from_pfn(frame, grant_pte_flags)));
> +                 l1e_get_intpte(l1e_from_mfn(frame, grant_pte_flags)));
>          goto out_unlock;
>      }
>  
> diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
> index 18201912e4..f9e3d1bb95 100644
> --- a/xen/common/grant_table.c
> +++ b/xen/common/grant_table.c
> @@ -1071,7 +1071,7 @@ map_grant_ref(
>  
>          if ( op->flags & GNTMAP_host_map )
>          {
> -            rc = create_grant_host_mapping(op->host_addr, frame, op->flags,
> +            rc = create_grant_host_mapping(op->host_addr, _mfn(frame), op->flags,
>                                             cache_flags);
>              if ( rc != GNTST_okay )
>                  goto undo_out;
> @@ -1111,7 +1111,7 @@ map_grant_ref(
>                  typecnt++;
>              }
>  
> -            rc = create_grant_host_mapping(op->host_addr, frame, op->flags, 0);
> +            rc = create_grant_host_mapping(op->host_addr, _mfn(frame), op->flags, 0);
>              if ( rc != GNTST_okay )
>                  goto undo_out;
>  
> @@ -1188,7 +1188,7 @@ map_grant_ref(
>   undo_out:
>      if ( host_map_created )
>      {
> -        replace_grant_host_mapping(op->host_addr, frame, 0, op->flags);
> +        replace_grant_host_mapping(op->host_addr, _mfn(frame), 0, op->flags);
>          gnttab_flush_tlb(ld);
>      }
>  
> @@ -1374,7 +1374,7 @@ unmap_common(
>      if ( op->host_addr && (flags & GNTMAP_host_map) )
>      {
>          if ( (rc = replace_grant_host_mapping(op->host_addr,
> -                                              op->frame, op->new_addr,
> +                                              _mfn(op->frame), op->new_addr,
>                                                flags)) < 0 )
>              goto act_release_out;
>  
> diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
> index d2027d26b2..24644084a1 100644
> --- a/xen/include/asm-arm/grant_table.h
> +++ b/xen/include/asm-arm/grant_table.h
> @@ -14,12 +14,11 @@ struct grant_table_arch {
>  };
>  
>  void gnttab_clear_flag(unsigned long nr, uint16_t *addr);
> -int create_grant_host_mapping(unsigned long gpaddr,
> -        unsigned long mfn, unsigned int flags, unsigned int
> -        cache_flags);
> +int create_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
> +                              unsigned int flags, unsigned int cache_flags);
>  #define gnttab_host_mapping_get_page_type(ro, ld, rd) (0)
> -int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
> -        unsigned long new_gpaddr, unsigned int flags);
> +int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
> +                               unsigned long new_gpaddr, unsigned int flags);
>  void gnttab_mark_dirty(struct domain *d, unsigned long l);
>  #define gnttab_create_status_page(d, t, i) do {} while (0)
>  #define gnttab_release_host_mappings(domain) 1
> diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
> index 4ac0b9b4c7..fc07291ff2 100644
> --- a/xen/include/asm-x86/grant_table.h
> +++ b/xen/include/asm-x86/grant_table.h
> @@ -21,7 +21,7 @@ struct grant_table_arch {
>   * Caller must own caller's BIGLOCK, is responsible for flushing the TLB, and
>   * must hold a reference to the page.
>   */
> -static inline int create_grant_host_mapping(uint64_t addr, unsigned long frame,
> +static inline int create_grant_host_mapping(uint64_t addr, mfn_t frame,
>                                              unsigned int flags,
>                                              unsigned int cache_flags)
>  {
> @@ -30,7 +30,7 @@ static inline int create_grant_host_mapping(uint64_t addr, unsigned long frame,
>      return create_grant_pv_mapping(addr, frame, flags, cache_flags);
>  }
>  
> -static inline int replace_grant_host_mapping(uint64_t addr, unsigned long frame,
> +static inline int replace_grant_host_mapping(uint64_t addr, mfn_t frame,
>                                               uint64_t new_addr,
>                                               unsigned int flags)
>  {
> diff --git a/xen/include/asm-x86/hvm/grant_table.h b/xen/include/asm-x86/hvm/grant_table.h
> index 711ce9b560..a5612585b3 100644
> --- a/xen/include/asm-x86/hvm/grant_table.h
> +++ b/xen/include/asm-x86/hvm/grant_table.h
> @@ -23,24 +23,24 @@
>  
>  #ifdef CONFIG_HVM
>  
> -int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>                               unsigned int flags,
>                               unsigned int cache_flags);
> -int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>                                uint64_t new_addr, unsigned int flags);
>  
>  #else
>  
>  #include <public/grant_table.h>
>  
> -static inline int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +static inline int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>                                             unsigned int flags,
>                                             unsigned int cache_flags)
>  {
>      return GNTST_general_error;
>  }
>  
> -static inline int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
> +static inline int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
>                                              uint64_t new_addr, unsigned int flags)
>  {
>      return GNTST_general_error;
> diff --git a/xen/include/asm-x86/pv/grant_table.h b/xen/include/asm-x86/pv/grant_table.h
> index 556e68f0eb..85442b6074 100644
> --- a/xen/include/asm-x86/pv/grant_table.h
> +++ b/xen/include/asm-x86/pv/grant_table.h
> @@ -23,23 +23,23 @@
>  
>  #ifdef CONFIG_PV
>  
> -int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
>                              unsigned int flags, unsigned int cache_flags);
> -int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
>                               uint64_t new_addr, unsigned int flags);
>  
>  #else
>  
>  #include <public/grant_table.h>
>  
> -static inline int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +static inline int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
>                                            unsigned int flags,
>                                            unsigned int cache_flags)
>  {
>      return GNTST_general_error;
>  }
>  
> -static inline int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
> +static inline int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
>                                             uint64_t new_addr, unsigned int flags)
>  {
>      return GNTST_general_error;
> -- 
> 2.11.0
>

Patch

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7af6baa3d6..49080ca0ac 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -1418,7 +1418,7 @@  void gnttab_mark_dirty(struct domain *d, unsigned long l)
     }
 }
 
-int create_grant_host_mapping(unsigned long addr, unsigned long frame,
+int create_grant_host_mapping(unsigned long addr, mfn_t frame,
                               unsigned int flags, unsigned int cache_flags)
 {
     int rc;
@@ -1431,7 +1431,7 @@  int create_grant_host_mapping(unsigned long addr, unsigned long frame,
         t = p2m_grant_map_ro;
 
     rc = guest_physmap_add_entry(current->domain, gaddr_to_gfn(addr),
-                                 _mfn(frame), 0, t);
+                                 frame, 0, t);
 
     if ( rc )
         return GNTST_general_error;
@@ -1439,8 +1439,8 @@  int create_grant_host_mapping(unsigned long addr, unsigned long frame,
         return GNTST_okay;
 }
 
-int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
-        unsigned long new_addr, unsigned int flags)
+int replace_grant_host_mapping(unsigned long addr, mfn_t mfn,
+                               unsigned long new_addr, unsigned int flags)
 {
     gfn_t gfn = gaddr_to_gfn(addr);
     struct domain *d = current->domain;
@@ -1449,7 +1449,7 @@  int replace_grant_host_mapping(unsigned long addr, unsigned long mfn,
     if ( new_addr != 0 || (flags & GNTMAP_contains_pte) )
         return GNTST_general_error;
 
-    rc = guest_physmap_remove_page(d, gfn, _mfn(mfn), 0);
+    rc = guest_physmap_remove_page(d, gfn, mfn, 0);
 
     return rc ? GNTST_general_error : GNTST_okay;
 }
diff --git a/xen/arch/x86/hvm/grant_table.c b/xen/arch/x86/hvm/grant_table.c
index 9ca9fe0425..ecd7d078ab 100644
--- a/xen/arch/x86/hvm/grant_table.c
+++ b/xen/arch/x86/hvm/grant_table.c
@@ -25,7 +25,7 @@ 
 
 #include <asm/p2m.h>
 
-int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                              unsigned int flags,
                              unsigned int cache_flags)
 {
@@ -41,14 +41,14 @@  int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
         p2mt = p2m_grant_map_rw;
     rc = guest_physmap_add_entry(current->domain,
                                  _gfn(addr >> PAGE_SHIFT),
-                                 _mfn(frame), PAGE_ORDER_4K, p2mt);
+                                 frame, PAGE_ORDER_4K, p2mt);
     if ( rc )
         return GNTST_general_error;
     else
         return GNTST_okay;
 }
 
-int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                               uint64_t new_addr, unsigned int flags)
 {
     unsigned long gfn = (unsigned long)(addr >> PAGE_SHIFT);
@@ -60,15 +60,15 @@  int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
         return GNTST_general_error;
 
     old_mfn = get_gfn(d, gfn, &type);
-    if ( !p2m_is_grant(type) || mfn_x(old_mfn) != frame )
+    if ( !p2m_is_grant(type) || !mfn_eq(old_mfn, frame) )
     {
         put_gfn(d, gfn);
         gdprintk(XENLOG_WARNING,
-                 "old mapping invalid (type %d, mfn %" PRI_mfn ", frame %lx)\n",
-                 type, mfn_x(old_mfn), frame);
+                 "old mapping invalid (type %d, mfn %" PRI_mfn ", frame %"PRI_mfn")\n",
+                 type, mfn_x(old_mfn), mfn_x(frame));
         return GNTST_general_error;
     }
-    if ( guest_physmap_remove_page(d, _gfn(gfn), _mfn(frame), PAGE_ORDER_4K) )
+    if ( guest_physmap_remove_page(d, _gfn(gfn), frame, PAGE_ORDER_4K) )
     {
         put_gfn(d, gfn);
         return GNTST_general_error;
diff --git a/xen/arch/x86/pv/grant_table.c b/xen/arch/x86/pv/grant_table.c
index 4dbc550366..458085e1b6 100644
--- a/xen/arch/x86/pv/grant_table.c
+++ b/xen/arch/x86/pv/grant_table.c
@@ -50,7 +50,7 @@  static unsigned int grant_to_pte_flags(unsigned int grant_flags,
     return pte_flags;
 }
 
-int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
+int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
                             unsigned int flags, unsigned int cache_flags)
 {
     struct vcpu *curr = current;
@@ -60,7 +60,7 @@  int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
     mfn_t gl1mfn;
     int rc = GNTST_general_error;
 
-    nl1e = l1e_from_pfn(frame, grant_to_pte_flags(flags, cache_flags));
+    nl1e = l1e_from_mfn(frame, grant_to_pte_flags(flags, cache_flags));
     nl1e = adjust_guest_l1e(nl1e, currd);
 
     /*
@@ -192,7 +192,7 @@  static bool steal_linear_address(unsigned long linear, l1_pgentry_t *out)
  * new_addr has only ever been available via GNTABOP_unmap_and_replace, and
  * only when !(flags & GNTMAP_contains_pte).
  */
-int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
+int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
                              uint64_t new_addr, unsigned int flags)
 {
     struct vcpu *curr = current;
@@ -282,14 +282,14 @@  int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
      * Check that the address supplied is actually mapped to frame (with
      * appropriate permissions).
      */
-    if ( unlikely(l1e_get_pfn(ol1e) != frame) ||
+    if ( unlikely(!mfn_eq(l1e_get_mfn(ol1e), frame)) ||
          unlikely((l1e_get_flags(ol1e) ^ grant_pte_flags) &
                   (_PAGE_PRESENT | _PAGE_RW)) )
     {
         gdprintk(XENLOG_ERR,
                  "PTE %"PRIpte" for %"PRIx64" doesn't match grant (%"PRIpte")\n",
                  l1e_get_intpte(ol1e), addr,
-                 l1e_get_intpte(l1e_from_pfn(frame, grant_pte_flags)));
+                 l1e_get_intpte(l1e_from_mfn(frame, grant_pte_flags)));
         goto out_unlock;
     }
 
diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c
index 18201912e4..f9e3d1bb95 100644
--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -1071,7 +1071,7 @@  map_grant_ref(
 
         if ( op->flags & GNTMAP_host_map )
         {
-            rc = create_grant_host_mapping(op->host_addr, frame, op->flags,
+            rc = create_grant_host_mapping(op->host_addr, _mfn(frame), op->flags,
                                            cache_flags);
             if ( rc != GNTST_okay )
                 goto undo_out;
@@ -1111,7 +1111,7 @@  map_grant_ref(
                 typecnt++;
             }
 
-            rc = create_grant_host_mapping(op->host_addr, frame, op->flags, 0);
+            rc = create_grant_host_mapping(op->host_addr, _mfn(frame), op->flags, 0);
             if ( rc != GNTST_okay )
                 goto undo_out;
 
@@ -1188,7 +1188,7 @@  map_grant_ref(
  undo_out:
     if ( host_map_created )
     {
-        replace_grant_host_mapping(op->host_addr, frame, 0, op->flags);
+        replace_grant_host_mapping(op->host_addr, _mfn(frame), 0, op->flags);
         gnttab_flush_tlb(ld);
     }
 
@@ -1374,7 +1374,7 @@  unmap_common(
     if ( op->host_addr && (flags & GNTMAP_host_map) )
     {
         if ( (rc = replace_grant_host_mapping(op->host_addr,
-                                              op->frame, op->new_addr,
+                                              _mfn(op->frame), op->new_addr,
                                               flags)) < 0 )
             goto act_release_out;
 
diff --git a/xen/include/asm-arm/grant_table.h b/xen/include/asm-arm/grant_table.h
index d2027d26b2..24644084a1 100644
--- a/xen/include/asm-arm/grant_table.h
+++ b/xen/include/asm-arm/grant_table.h
@@ -14,12 +14,11 @@  struct grant_table_arch {
 };
 
 void gnttab_clear_flag(unsigned long nr, uint16_t *addr);
-int create_grant_host_mapping(unsigned long gpaddr,
-        unsigned long mfn, unsigned int flags, unsigned int
-        cache_flags);
+int create_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
+                              unsigned int flags, unsigned int cache_flags);
 #define gnttab_host_mapping_get_page_type(ro, ld, rd) (0)
-int replace_grant_host_mapping(unsigned long gpaddr, unsigned long mfn,
-        unsigned long new_gpaddr, unsigned int flags);
+int replace_grant_host_mapping(unsigned long gpaddr, mfn_t mfn,
+                               unsigned long new_gpaddr, unsigned int flags);
 void gnttab_mark_dirty(struct domain *d, unsigned long l);
 #define gnttab_create_status_page(d, t, i) do {} while (0)
 #define gnttab_release_host_mappings(domain) 1
diff --git a/xen/include/asm-x86/grant_table.h b/xen/include/asm-x86/grant_table.h
index 4ac0b9b4c7..fc07291ff2 100644
--- a/xen/include/asm-x86/grant_table.h
+++ b/xen/include/asm-x86/grant_table.h
@@ -21,7 +21,7 @@  struct grant_table_arch {
  * Caller must own caller's BIGLOCK, is responsible for flushing the TLB, and
  * must hold a reference to the page.
  */
-static inline int create_grant_host_mapping(uint64_t addr, unsigned long frame,
+static inline int create_grant_host_mapping(uint64_t addr, mfn_t frame,
                                             unsigned int flags,
                                             unsigned int cache_flags)
 {
@@ -30,7 +30,7 @@  static inline int create_grant_host_mapping(uint64_t addr, unsigned long frame,
     return create_grant_pv_mapping(addr, frame, flags, cache_flags);
 }
 
-static inline int replace_grant_host_mapping(uint64_t addr, unsigned long frame,
+static inline int replace_grant_host_mapping(uint64_t addr, mfn_t frame,
                                              uint64_t new_addr,
                                              unsigned int flags)
 {
diff --git a/xen/include/asm-x86/hvm/grant_table.h b/xen/include/asm-x86/hvm/grant_table.h
index 711ce9b560..a5612585b3 100644
--- a/xen/include/asm-x86/hvm/grant_table.h
+++ b/xen/include/asm-x86/hvm/grant_table.h
@@ -23,24 +23,24 @@ 
 
 #ifdef CONFIG_HVM
 
-int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                              unsigned int flags,
                              unsigned int cache_flags);
-int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                               uint64_t new_addr, unsigned int flags);
 
 #else
 
 #include <public/grant_table.h>
 
-static inline int create_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+static inline int create_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                                            unsigned int flags,
                                            unsigned int cache_flags)
 {
     return GNTST_general_error;
 }
 
-static inline int replace_grant_p2m_mapping(uint64_t addr, unsigned long frame,
+static inline int replace_grant_p2m_mapping(uint64_t addr, mfn_t frame,
                                             uint64_t new_addr, unsigned int flags)
 {
     return GNTST_general_error;
diff --git a/xen/include/asm-x86/pv/grant_table.h b/xen/include/asm-x86/pv/grant_table.h
index 556e68f0eb..85442b6074 100644
--- a/xen/include/asm-x86/pv/grant_table.h
+++ b/xen/include/asm-x86/pv/grant_table.h
@@ -23,23 +23,23 @@ 
 
 #ifdef CONFIG_PV
 
-int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
+int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
                             unsigned int flags, unsigned int cache_flags);
-int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
+int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
                              uint64_t new_addr, unsigned int flags);
 
 #else
 
 #include <public/grant_table.h>
 
-static inline int create_grant_pv_mapping(uint64_t addr, unsigned long frame,
+static inline int create_grant_pv_mapping(uint64_t addr, mfn_t frame,
                                           unsigned int flags,
                                           unsigned int cache_flags)
 {
     return GNTST_general_error;
 }
 
-static inline int replace_grant_pv_mapping(uint64_t addr, unsigned long frame,
+static inline int replace_grant_pv_mapping(uint64_t addr, mfn_t frame,
                                            uint64_t new_addr, unsigned int flags)
 {
     return GNTST_general_error;