xen/grant: Switch {create, replace}_grant_p2m_mapping to typesafe MFN
authorJulien Grall <julien.grall@arm.com>
Wed, 21 Feb 2018 13:46:26 +0000 (13:46 +0000)
committerJulien Grall <julien.grall@arm.com>
Fri, 6 Apr 2018 08:43:29 +0000 (09:43 +0100)
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>
xen/arch/arm/mm.c
xen/arch/x86/hvm/grant_table.c
xen/arch/x86/pv/grant_table.c
xen/common/grant_table.c
xen/include/asm-arm/grant_table.h
xen/include/asm-x86/grant_table.h
xen/include/asm-x86/hvm/grant_table.h
xen/include/asm-x86/pv/grant_table.h

index 7af6baa3d6763a7ef7d371aeee4dc24b2eba8e34..49080ca0ac2cb0ca3cb30a2b3ad17e08f6567992 100644 (file)
@@ -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;
 }
index 9ca9fe042508a72872870be6f47eb998f2d6766c..ecd7d078ab057e014d3eb76b7e80828fdfc90ba4 100644 (file)
@@ -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;
index 4dbc550366000191c98373b847129a02a03d8f77..458085e1b61e45eb136e36a0f73e908590855931 100644 (file)
@@ -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;
     }
 
index 18201912e4356cfbceecd360e24acbd9e140a305..f9e3d1bb95278432395540ab640ed7328cfb4974 100644 (file)
@@ -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;
 
index d2027d26b23d9ffd493ab99bb85da7aa9c6a6c82..24644084a10c52d59e49db85d53fad077762d0b6 100644 (file)
@@ -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
index 4ac0b9b4c7c5dbf86cf350a7a5c51b1a95d1a6b0..fc07291ff20f1c284191017ba036b7cc923dbfb8 100644 (file)
@@ -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)
 {
index 711ce9b56024027fe2ab243cee35438079def23f..a5612585b35e520a87f0566a1f49d4574a34681f 100644 (file)
 
 #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;
index 556e68f0eb4382ff10ff48d7bff3246fdb72889d..85442b60749f6b17195eb9925887f9c98ae6a28b 100644 (file)
 
 #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;