Synchronized p2m lookups v2

diff -r 1756949233a9 -r e29c2634afb1 xen/arch/x86/mm/hap/hap.c
@@ -786,7 +786,14 @@ hap_paging_get_mode(struct vcpu *v)
 static void hap_update_paging_modes(struct vcpu *v)
 {
     struct domain *d = v->domain;
+    unsigned long cr3_gfn = v->arch.hvm_vcpu.guest_cr[3];
+    p2m_type_t t;
 
+    /* We hold onto the cr3 as it may be modified later, and
+     * we need to respect lock ordering. No need for 
+     * checks here as they are performed by vmx_load_pdptrs
+     * (the potential user of the cr3) */
+    (void)get_gfn(d, cr3_gfn, &t);
     paging_lock(d);
 
     v->arch.paging.mode = hap_paging_get_mode(v);

@@ -803,6 +810,7 @@ static void hap_update_paging_modes(stru
     hap_update_cr3(v, 0);
 
     paging_unlock(d);
+    put_gfn(d, cr3_gfn);
 }
 
 #if CONFIG_PAGING_LEVELS == 3

diff -r 1756949233a9 -r e29c2634afb1 xen/arch/x86/mm/mm-locks.h
@@ -144,6 +144,31 @@ static inline void mm_enforce_order_unlo
  *                                                                      *
  ************************************************************************/
 
+declare_mm_lock(nestedp2m)
+#define nestedp2m_lock(d)   mm_lock(nestedp2m, &(d)->arch.nested_p2m_lock)
+#define nestedp2m_unlock(d) mm_unlock(&(d)->arch.nested_p2m_lock)
+
+/* P2M lock (per-p2m-table)
+ * 
+ * This protects all queries and updates to the p2m table. 
+ *
+ * A note about ordering:
+ *   The order established here is enforced on all mutations of a p2m.
+ *   For lookups, the order established here is enforced only for hap
+ *   domains (1. shadow domains present a few nasty inversions; 
+ *            2. shadow domains do not support paging and sharing, 
+ *               the main sources of dynamic p2m mutations)
+ * 
+ * The lock is recursive as it is common for a code path to look up a gfn
+ * and later mutate it.
+ */
+
+declare_mm_lock(p2m)
+#define p2m_lock(p)           mm_lock_recursive(p2m, &(p)->lock)
+#define p2m_lock_recursive(p) mm_lock_recursive(p2m, &(p)->lock)
+#define p2m_unlock(p)         mm_unlock(&(p)->lock)
+#define p2m_locked_by_me(p)   mm_locked_by_me(&(p)->lock)
+
 /* Sharing per page lock
  *
  * This is an external lock, not represented by an mm_lock_t. The memory

@@ -167,21 +192,6 @@ declare_mm_order_constraint(per_page_sha
  * - setting the "cr3" field of any p2m table to a non-CR3_EADDR value. 
  *   (i.e. assigning a p2m table to be the shadow of that cr3 */
 
-declare_mm_lock(nestedp2m)
-#define nestedp2m_lock(d)   mm_lock(nestedp2m, &(d)->arch.nested_p2m_lock)
-#define nestedp2m_unlock(d) mm_unlock(&(d)->arch.nested_p2m_lock)
-
-/* P2M lock (per-p2m-table)
- * 
- * This protects all updates to the p2m table.  Updates are expected to
- * be safe against concurrent reads, which do *not* require the lock. */
-
-declare_mm_lock(p2m)
-#define p2m_lock(p)           mm_lock(p2m, &(p)->lock)
-#define p2m_lock_recursive(p) mm_lock_recursive(p2m, &(p)->lock)
-#define p2m_unlock(p)         mm_unlock(&(p)->lock)
-#define p2m_locked_by_me(p)   mm_locked_by_me(&(p)->lock)
-
 /* Page alloc lock (per-domain)
  *
  * This is an external lock, not represented by an mm_lock_t. However, 

diff -r 1756949233a9 -r e29c2634afb1 xen/arch/x86/mm/p2m.c
@@ -144,9 +144,9 @@ void p2m_change_entry_type_global(struct
     p2m_unlock(p2m);
 }
 
-mfn_t get_gfn_type_access(struct p2m_domain *p2m, unsigned long gfn,
+mfn_t __get_gfn_type_access(struct p2m_domain *p2m, unsigned long gfn,
                     p2m_type_t *t, p2m_access_t *a, p2m_query_t q,
-                    unsigned int *page_order)
+                    unsigned int *page_order, bool_t locked)
 {
     mfn_t mfn;
 

@@ -158,6 +158,11 @@ mfn_t get_gfn_type_access(struct p2m_dom
         return _mfn(gfn);
     }
 
+    /* For now only perform locking on hap domains */
+    if ( locked && (hap_enabled(p2m->domain)) )
+        /* Grab the lock here, don't release until put_gfn */
+        p2m_lock(p2m);
+
     mfn = p2m->get_entry(p2m, gfn, t, a, q, page_order);
 
 #ifdef __x86_64__

@@ -182,6 +187,18 @@ mfn_t get_gfn_type_access(struct p2m_dom
     return mfn;
 }
 
+void __put_gfn(struct p2m_domain *p2m, unsigned long gfn)
+{
+    if ( !p2m || !paging_mode_translate(p2m->domain) 
+              || !hap_enabled(p2m->domain) )
+        /* Nothing to do in this case */
+        return;
+
+    ASSERT(p2m_locked_by_me(p2m));
+
+    p2m_unlock(p2m);
+}
+
 int set_p2m_entry(struct p2m_domain *p2m, unsigned long gfn, mfn_t mfn, 
                   unsigned int page_order, p2m_type_t p2mt, p2m_access_t p2ma)
 {

diff -r 1756949233a9 -r e29c2634afb1 xen/include/asm-x86/mm.h
@@ -350,9 +350,9 @@ void clear_superpage_mark(struct page_in
  * of (gfn,domain) tupples to a list of gfn's that the shared page is currently 
  * backing. Nesting may happen when sharing (and locking) two pages -- deadlock 
  * is avoided by locking pages in increasing order.
- * Memory sharing may take the p2m_lock within a page_lock/unlock
- * critical section. We enforce ordering between page_lock and p2m_lock using an
- * mm-locks.h construct. 
+ * All memory sharing code paths take the p2m lock of the affected gfn before
+ * taking the lock for the underlying page. We enforce ordering between page_lock 
+ * and p2m_lock using an mm-locks.h construct. 
  *
  * These two users (pte serialization and memory sharing) do not collide, since
  * sharing is only supported for hvm guests, which do not perform pv pte updates.

68 lines not on this page · raw message

diff -r e29c2634afb1 -r 63cb81c2448e xen/arch/x86/mm/mem_sharing.c
@@ -877,9 +877,7 @@ int mem_sharing_add_to_physmap(struct do
         goto err_unlock;
     }
 
-    p2m_lock(p2m);
     ret = set_p2m_entry(p2m, cgfn, smfn, PAGE_ORDER_4K, p2m_ram_shared, a);
-    p2m_unlock(p2m);
 
     /* Tempted to turn this into an assert */
     if ( !ret )

diff -r e29c2634afb1 -r 63cb81c2448e xen/arch/x86/mm/mm-locks.h
@@ -165,9 +165,11 @@ declare_mm_lock(nestedp2m)
 
 declare_mm_lock(p2m)
 #define p2m_lock(p)           mm_lock_recursive(p2m, &(p)->lock)
-#define p2m_lock_recursive(p) mm_lock_recursive(p2m, &(p)->lock)
+#define gfn_lock(p,g,o)       mm_lock_recursive(p2m, &(p)->lock)
 #define p2m_unlock(p)         mm_unlock(&(p)->lock)
+#define gfn_unlock(p,g,o)     mm_unlock(&(p)->lock)
 #define p2m_locked_by_me(p)   mm_locked_by_me(&(p)->lock)
+#define gfn_locked_by_me(p,g) mm_locked_by_me(&(p)->lock)
 
 /* Sharing per page lock
  *

diff -r e29c2634afb1 -r 63cb81c2448e xen/arch/x86/mm/p2m-ept.c
@@ -46,31 +46,6 @@ static inline bool_t is_epte_valid(ept_e
     return (e->epte != 0 && e->sa_p2mt != p2m_invalid);
 }
 
-/* Non-ept "lock-and-check" wrapper */
-static int ept_pod_check_and_populate(struct p2m_domain *p2m, unsigned long gfn,
-                                      ept_entry_t *entry, int order,
-                                      p2m_query_t q)
-{
-    int r;
-
-    /* This is called from the p2m lookups, which can happen with or 
-     * without the lock hed. */
-    p2m_lock_recursive(p2m);
-
-    /* Check to make sure this is still PoD */
-    if ( entry->sa_p2mt != p2m_populate_on_demand )
-    {
-        p2m_unlock(p2m);
-        return 0;
-    }
-
-    r = p2m_pod_demand_populate(p2m, gfn, order, q);
-
-    p2m_unlock(p2m);
-
-    return r;
-}
-
 static void ept_p2m_type_to_flags(ept_entry_t *entry, p2m_type_t type, p2m_access_t access)
 {
     /* First apply type permissions */

@@ -551,8 +526,8 @@ static mfn_t ept_get_entry(struct p2m_do
             index = gfn_remainder >> ( i * EPT_TABLE_ORDER);
             ept_entry = table + index;
 
-            if ( !ept_pod_check_and_populate(p2m, gfn,
-                                             ept_entry, 9, q) )
+            if ( !p2m_pod_demand_populate(p2m, gfn, 
+                                            PAGE_ORDER_2M, q) )
                 goto retry;
             else
                 goto out;

@@ -574,8 +549,8 @@ static mfn_t ept_get_entry(struct p2m_do
 
         ASSERT(i == 0);
         
-        if ( ept_pod_check_and_populate(p2m, gfn,
-                                        ept_entry, 0, q) )
+        if ( p2m_pod_demand_populate(p2m, gfn, 
+                                        PAGE_ORDER_4K, q) )
             goto out;
     }
 

diff -r e29c2634afb1 -r 63cb81c2448e xen/arch/x86/mm/p2m-pod.c
@@ -521,7 +521,7 @@ p2m_pod_decrease_reservation(struct doma
     /* Figure out if we need to steal some freed memory for our cache */
     steal_for_cache =  ( p2m->pod.entry_count > p2m->pod.count );
 
-    p2m_lock(p2m);
+    gfn_lock(p2m, gpfn, order);
 
     if ( unlikely(d->is_dying) )
         goto out_unlock;

@@ -615,7 +615,7 @@ out_entry_check:
     }
 
 out_unlock:
-    p2m_unlock(p2m);
+    gfn_unlock(p2m, gpfn, order);
 
 out:
     return ret;

@@ -964,7 +964,7 @@ p2m_pod_demand_populate(struct p2m_domai
     mfn_t mfn;
     int i;
 
-    ASSERT(p2m_locked_by_me(p2m));
+    ASSERT(gfn_locked_by_me(p2m, gfn));
 
     /* This check is done with the p2m lock held.  This will make sure that
      * even if d->is_dying changes under our feet, p2m_pod_empty_cache() 

@@ -972,6 +972,7 @@ p2m_pod_demand_populate(struct p2m_domai
     if ( unlikely(d->is_dying) )
         goto out_fail;
 
+    
     /* Because PoD does not have cache list for 1GB pages, it has to remap
      * 1GB region to 2MB chunks for a retry. */
     if ( order == PAGE_ORDER_1G )

@@ -981,6 +982,9 @@ p2m_pod_demand_populate(struct p2m_domai
          * split 1GB into 512 2MB pages here. But We only do once here because
          * set_p2m_entry() should automatically shatter the 1GB page into 
          * 512 2MB pages. The rest of 511 calls are unnecessary.
+         *
+         * NOTE: In a fine-grained p2m locking scenario this operation
+         * may need to promote its locking from gfn->1g superpage
          */
         set_p2m_entry(p2m, gfn_aligned, _mfn(0), PAGE_ORDER_2M,
                       p2m_populate_on_demand, p2m->default_access);

@@ -1104,7 +1108,7 @@ guest_physmap_mark_populate_on_demand(st
     if ( rc != 0 )
         return rc;
 
-    p2m_lock(p2m);
+    gfn_lock(p2m, gfn, order);
 
     P2M_DEBUG("mark pod gfn=%#lx\n", gfn);
 

@@ -1138,7 +1142,7 @@ guest_physmap_mark_populate_on_demand(st
         BUG_ON(p2m->pod.entry_count < 0);
     }
 
-    p2m_unlock(p2m);
+    gfn_unlock(p2m, gfn, order);
 
 out:
     return rc;

87 lines not on this page · raw message

231 lines not on this page · raw message

diff -r 63cb81c2448e -r 5416a3b371f2 xen/arch/x86/mm/mm-locks.h
@@ -194,6 +194,16 @@ declare_mm_order_constraint(per_page_sha
  * - setting the "cr3" field of any p2m table to a non-CR3_EADDR value. 
  *   (i.e. assigning a p2m table to be the shadow of that cr3 */
 
+/* PoD lock (per-p2m-table)
+ * 
+ * Protects private PoD data structs: entry and cache
+ * counts, page lists, sweep parameters. */
+
+declare_mm_lock(pod)
+#define pod_lock(p)           mm_lock(pod, &(p)->pod.lock)
+#define pod_unlock(p)         mm_unlock(&(p)->pod.lock)
+#define pod_locked_by_me(p)   mm_locked_by_me(&(p)->pod.lock)
+
 /* Page alloc lock (per-domain)
  *
  * This is an external lock, not represented by an mm_lock_t. However, 

372 lines not on this page · raw message

diff -r 63cb81c2448e -r 5416a3b371f2 xen/arch/x86/mm/p2m-pt.c
@@ -954,6 +954,7 @@ long p2m_pt_audit_p2m(struct p2m_domain 
     struct domain *d = p2m->domain;
 
     ASSERT(p2m_locked_by_me(p2m));
+    ASSERT(pod_locked_by_me(p2m));
 
     test_linear = ( (d == current->domain)
                     && !pagetable_is_null(current->arch.monitor_table) );

diff -r 63cb81c2448e -r 5416a3b371f2 xen/arch/x86/mm/p2m.c
@@ -72,6 +72,7 @@ boolean_param("hap_2mb", opt_hap_2mb);
 static void p2m_initialise(struct domain *d, struct p2m_domain *p2m)
 {
     mm_lock_init(&p2m->lock);
+    mm_lock_init(&p2m->pod.lock);
     INIT_LIST_HEAD(&p2m->np2m_list);
     INIT_PAGE_LIST_HEAD(&p2m->pages);
     INIT_PAGE_LIST_HEAD(&p2m->pod.super);

@@ -568,8 +569,10 @@ guest_physmap_add_entry(struct domain *d
             rc = -EINVAL;
         else
         {
-            p2m->pod.entry_count -= pod_count; /* Lock: p2m */
+            pod_lock(p2m);
+            p2m->pod.entry_count -= pod_count;
             BUG_ON(p2m->pod.entry_count < 0);
+            pod_unlock(p2m);
         }
     }
 

@@ -1350,6 +1353,7 @@ p2m_flush_table(struct p2m_domain *p2m)
     /* "Host" p2m tables can have shared entries &c that need a bit more 
      * care when discarding them */
     ASSERT(p2m_is_nestedp2m(p2m));
+    /* Nested p2m's do not do pod, hence the asserts (and no pod lock)*/
     ASSERT(page_list_empty(&p2m->pod.super));
     ASSERT(page_list_empty(&p2m->pod.single));
 

@@ -1507,6 +1511,7 @@ void audit_p2m(struct domain *d,
     P2M_PRINTK("p2m audit starts\n");
 
     p2m_lock(p2m);
+    pod_lock(p2m);
 
     if (p2m->audit_p2m)
         pmbad = p2m->audit_p2m(p2m);

@@ -1567,6 +1572,7 @@ void audit_p2m(struct domain *d,
     }
     spin_unlock(&d->page_alloc_lock);
 
+    pod_unlock(p2m);
     p2m_unlock(p2m);
  
     P2M_PRINTK("p2m audit complete\n");

diff -r 63cb81c2448e -r 5416a3b371f2 xen/include/asm-x86/p2m.h
@@ -261,25 +261,12 @@ struct p2m_domain {
     unsigned long max_mapped_pfn;
 
     /* Populate-on-demand variables
-     * NB on locking.  {super,single,count} are
-     * covered by d->page_alloc_lock, since they're almost always used in
-     * conjunction with that functionality.  {entry_count} is covered by
-     * the domain p2m lock, since it's almost always used in conjunction
-     * with changing the p2m tables.
-     *
-     * At this point, both locks are held in two places.  In both,
-     * the order is [p2m,page_alloc]:
-     * + p2m_pod_decrease_reservation() calls p2m_pod_cache_add(),
-     *   which grabs page_alloc
-     * + p2m_pod_demand_populate() grabs both; the p2m lock to avoid
-     *   double-demand-populating of pages, the page_alloc lock to
-     *   protect moving stuff from the PoD cache to the domain page list.
-     *
-     * We enforce this lock ordering through a construct in mm-locks.h.
-     * This demands, however, that we store the previous lock-ordering
-     * level in effect before grabbing the page_alloc lock. The unlock
-     * level is stored in the arch section of the domain struct.
-     */
+     * All variables are protected with the pod lock. We cannot rely on
+     * the p2m lock if it's turned into a fine-grained lock.
+     * We only use the domain page_alloc lock for additions and 
+     * deletions to the domain's page list. Because we use it nested
+     * within the PoD lock, we enforce it's ordering (by remembering
+     * the unlock level in the arch_domain sub struct). */
     struct {
         struct page_list_head super,   /* List of superpages                */
                          single;       /* Non-super lists                   */

@@ -288,6 +275,8 @@ struct p2m_domain {
         unsigned         reclaim_super; /* Last gpfn of a scan */
         unsigned         reclaim_single; /* Last gpfn of a scan */
         unsigned         max_guest;    /* gpfn of max guest demand-populate */
+        mm_lock_t        lock;         /* Locking of private pod structs,   *
+                                        * not relying on the p2m lock.      */
     } pod;
 };

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/hvm/emulate.c
@@ -67,8 +67,8 @@ static int hvmemul_do_io(
     ram_mfn = get_gfn_unshare(curr->domain, ram_gfn, &p2mt);
     if ( p2m_is_paging(p2mt) )
     {
+        put_gfn(curr->domain, ram_gfn); 
         p2m_mem_paging_populate(curr->domain, ram_gfn);
-        put_gfn(curr->domain, ram_gfn); 
         return X86EMUL_RETRY;
     }
     if ( p2m_is_shared(p2mt) )

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/hvm/hvm.c
@@ -64,6 +64,7 @@
 #include <public/version.h>
 #include <public/memory.h>
 #include <asm/mem_event.h>
+#include <asm/mem_access.h>
 #include <public/mem_event.h>
 
 bool_t __read_mostly hvm_enabled;

@@ -363,8 +364,8 @@ static int hvm_set_ioreq_page(
     }
     if ( p2m_is_paging(p2mt) )
     {
+        put_gfn(d, gmfn);
         p2m_mem_paging_populate(d, gmfn);
-        put_gfn(d, gmfn);
         return -ENOENT;
     }
     if ( p2m_is_shared(p2mt) )

@@ -1195,7 +1196,8 @@ int hvm_hap_nested_page_fault(unsigned l
     mfn_t mfn;
     struct vcpu *v = current;
     struct p2m_domain *p2m;
-    int rc, fall_through = 0;
+    int rc, fall_through = 0, paged = 0;
+    mem_event_request_t *req_ptr = NULL;
 
     /* On Nested Virtualization, walk the guest page table.
      * If this succeeds, all is fine.

@@ -1270,7 +1272,7 @@ int hvm_hap_nested_page_fault(unsigned l
         if ( violation )
         {
             if ( p2m_mem_access_check(gpa, gla_valid, gla, access_r, 
-                                        access_w, access_x) )
+                                        access_w, access_x, &req_ptr) )
             {
                 fall_through = 1;
             } else {

@@ -1297,7 +1299,7 @@ int hvm_hap_nested_page_fault(unsigned l
 #ifdef __x86_64__
     /* Check if the page has been paged out */
     if ( p2m_is_paged(p2mt) || (p2mt == p2m_ram_paging_out) )
-        p2m_mem_paging_populate(v->domain, gfn);
+        paged = 1;
 
     /* Mem sharing: unshare the page and try again */
     if ( access_w && (p2mt == p2m_ram_shared) )

@@ -1343,6 +1345,13 @@ int hvm_hap_nested_page_fault(unsigned l
 
 out_put_gfn:
     put_gfn(p2m->domain, gfn);
+    if ( paged )
+        p2m_mem_paging_populate(v->domain, gfn);
+    if ( req_ptr )
+    {
+        mem_access_send_req(v->domain, req_ptr);
+        xfree(req_ptr);
+    }
     return rc;
 }
 

@@ -1849,8 +1858,8 @@ static void *__hvm_map_guest_frame(unsig
     }
     if ( p2m_is_paging(p2mt) )
     {
+        put_gfn(d, gfn);
         p2m_mem_paging_populate(d, gfn);
-        put_gfn(d, gfn);
         return NULL;
     }
 

@@ -2325,8 +2334,8 @@ static enum hvm_copy_result __hvm_copy(
 
         if ( p2m_is_paging(p2mt) )
         {
+            put_gfn(curr->domain, gfn);
             p2m_mem_paging_populate(curr->domain, gfn);
-            put_gfn(curr->domain, gfn);
             return HVMCOPY_gfn_paged_out;
         }
         if ( p2m_is_shared(p2mt) )

@@ -3923,8 +3932,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
             mfn_t mfn = get_gfn_unshare(d, pfn, &t);
             if ( p2m_is_paging(t) )
             {
+                put_gfn(d, pfn);
                 p2m_mem_paging_populate(d, pfn);
-                put_gfn(d, pfn);
                 rc = -EINVAL;
                 goto param_fail3;
             }

@@ -4040,8 +4049,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
             mfn = get_gfn_unshare(d, pfn, &t);
             if ( p2m_is_paging(t) )
             {
+                put_gfn(d, pfn);
                 p2m_mem_paging_populate(d, pfn);
-                put_gfn(d, pfn);
                 rc = -EINVAL;
                 goto param_fail4;
             }

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/mm.c
@@ -3536,8 +3536,8 @@ int do_mmu_update(
 
             if ( p2m_is_paged(p2mt) )
             {
+                put_gfn(pt_owner, gmfn);
                 p2m_mem_paging_populate(pg_owner, gmfn);
-                put_gfn(pt_owner, gmfn);
                 rc = -ENOENT;
                 break;
             }

@@ -3568,8 +3568,8 @@ int do_mmu_update(
 
                     if ( p2m_is_paged(l1e_p2mt) )
                     {
+                        put_gfn(pg_owner, l1egfn);
                         p2m_mem_paging_populate(pg_owner, l1e_get_pfn(l1e));
-                        put_gfn(pg_owner, l1egfn);
                         rc = -ENOENT;
                         break;
                     }

@@ -3617,8 +3617,8 @@ int do_mmu_update(
 
                     if ( p2m_is_paged(l2e_p2mt) )
                     {
+                        put_gfn(pg_owner, l2egfn);
                         p2m_mem_paging_populate(pg_owner, l2egfn);
-                        put_gfn(pg_owner, l2egfn);
                         rc = -ENOENT;
                         break;
                     }

@@ -3652,8 +3652,8 @@ int do_mmu_update(
 
                     if ( p2m_is_paged(l3e_p2mt) )
                     {
+                        put_gfn(pg_owner, l3egfn);
                         p2m_mem_paging_populate(pg_owner, l3egfn);
-                        put_gfn(pg_owner, l3egfn);
                         rc = -ENOENT;
                         break;
                     }

@@ -3687,8 +3687,8 @@ int do_mmu_update(
 
                     if ( p2m_is_paged(l4e_p2mt) )
                     {
+                        put_gfn(pg_owner, l4egfn);
                         p2m_mem_paging_populate(pg_owner, l4egfn);
-                        put_gfn(pg_owner, l4egfn);
                         rc = -ENOENT;
                         break;
                     }

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/mm/guest_walk.c
@@ -102,8 +102,8 @@ static inline void *map_domain_gfn(struc
     if ( p2m_is_paging(*p2mt) )
     {
         ASSERT(!p2m_is_nestedp2m(p2m));
+        __put_gfn(p2m, gfn_x(gfn));
         p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
-        __put_gfn(p2m, gfn_x(gfn));
         *rc = _PAGE_PAGED;
         return NULL;
     }

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/mm/hap/guest_walk.c
@@ -64,10 +64,9 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
     if ( p2m_is_paging(p2mt) )
     {
         ASSERT(!p2m_is_nestedp2m(p2m));
-        p2m_mem_paging_populate(p2m->domain, cr3 >> PAGE_SHIFT);
-
         pfec[0] = PFEC_page_paged;
         __put_gfn(p2m, top_gfn);
+        p2m_mem_paging_populate(p2m->domain, cr3 >> PAGE_SHIFT);
         return INVALID_GFN;
     }
     if ( p2m_is_shared(p2mt) )

@@ -101,10 +100,9 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA
         if ( p2m_is_paging(p2mt) )
         {
             ASSERT(!p2m_is_nestedp2m(p2m));
-            p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
-
             pfec[0] = PFEC_page_paged;
             __put_gfn(p2m, gfn_x(gfn));
+            p2m_mem_paging_populate(p2m->domain, gfn_x(gfn));
             return INVALID_GFN;
         }
         if ( p2m_is_shared(p2mt) )

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/mm/mem_access.c
@@ -47,6 +47,16 @@ int mem_access_domctl(struct domain *d, 
     return rc;
 }
 
+int mem_access_send_req(struct domain *d, mem_event_request_t *req)
+{
+    int rc = mem_event_claim_slot(d, &d->mem_event->access);
+    if ( rc < 0 )
+        return rc;
+
+    mem_event_put_request(d, &d->mem_event->access, req);
+
+    return 0;
+} 
 
 /*
  * Local variables:

diff -r 5416a3b371f2 -r f2d967332acc xen/arch/x86/mm/mem_event.c
@@ -423,6 +423,11 @@ static int mem_event_wait_slot(struct me
     return rc;
 }
 
+bool_t mem_event_check_ring(struct mem_event_domain *med)
+{
+    return (med->ring_page != NULL);
+}
+
 /*
  * Determines whether or not the current vCPU belongs to the target domain,
  * and calls the appropriate wait function.  If it is a guest vCPU, then we

82 lines not on this page · raw message

diff -r 5416a3b371f2 -r f2d967332acc xen/common/grant_table.c
@@ -164,8 +164,8 @@ static int __get_paged_frame(unsigned lo
         *frame = mfn_x(mfn);
         if ( p2m_is_paging(p2mt) )
         {
+            put_gfn(rd, gfn);
             p2m_mem_paging_populate(rd, gfn);
-            put_gfn(rd, gfn);
             rc = GNTST_eagain;
         }
     } else {

diff -r 5416a3b371f2 -r f2d967332acc xen/common/memory.c
@@ -166,8 +166,8 @@ int guest_remove_page(struct domain *d, 
     if ( unlikely(p2m_is_paging(p2mt)) )
     {
         guest_physmap_remove_page(d, gmfn, mfn, 0);
+        put_gfn(d, gmfn);
         p2m_mem_paging_drop_page(d, gmfn, p2mt);
-        put_gfn(d, gmfn);
         return 1;
     }
 #else

diff -r 5416a3b371f2 -r f2d967332acc xen/include/asm-x86/mem_access.h
@@ -23,6 +23,7 @@
 
 int mem_access_domctl(struct domain *d, xen_domctl_mem_event_op_t *mec,
                       XEN_GUEST_HANDLE(void) u_domctl);
+int mem_access_send_req(struct domain *d, mem_event_request_t *req);
 
 
 /*

diff -r 5416a3b371f2 -r f2d967332acc xen/include/asm-x86/mem_event.h
@@ -24,6 +24,9 @@
 #ifndef __MEM_EVENT_H__
 #define __MEM_EVENT_H__
 
+/* Returns whether a ring has been set up */
+bool_t mem_event_check_ring(struct mem_event_domain *med);
+
 /* Returns 0 on success, -ENOSYS if there is no ring, -EBUSY if there is no
  * available space. For success or -EBUSY, the vCPU may be left blocked
  * temporarily to ensure that the ring does not lose future events.  In

diff -r 5416a3b371f2 -r f2d967332acc xen/include/asm-x86/p2m.h
@@ -494,9 +494,12 @@ static inline void p2m_mem_paging_popula
 #ifdef __x86_64__
 /* Send mem event based on the access (gla is -1ull if not available).  Handles
  * the rw2rx conversion. Boolean return value indicates if access rights have 
- * been promoted with no underlying vcpu pause. */
+ * been promoted with no underlying vcpu pause. If the req_ptr has been populated, 
+ * then the caller must put the event in the ring (once having released get_gfn*
+ * locks -- caller must also xfree the request. */
 bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, unsigned long gla, 
-                          bool_t access_r, bool_t access_w, bool_t access_x);
+                          bool_t access_r, bool_t access_w, bool_t access_x,
+                          mem_event_request_t **req_ptr);
 /* Resumes the running of the VCPU, restarting the last instruction */
 void p2m_mem_access_resume(struct domain *d);
 

@@ -513,7 +516,8 @@ int p2m_get_mem_access(struct domain *d,
 #else
 static inline bool_t p2m_mem_access_check(unsigned long gpa, bool_t gla_valid, 
                                         unsigned long gla, bool_t access_r, 
-                                        bool_t access_w, bool_t access_x)
+                                        bool_t access_w, bool_t access_x,
+                                        mem_event_request_t **req_ptr)
 { return 1; }
 static inline int p2m_set_mem_access(struct domain *d, 
                                      unsigned long start_pfn, 

diff -r f2d967332acc -r 316d227dd526 xen/arch/x86/mm/p2m.c
@@ -361,6 +361,8 @@ void p2m_teardown(struct p2m_domain *p2m
     if (p2m == NULL)
         return;
 
+    p2m_lock(p2m);
+
 #ifdef __x86_64__
     for ( gfn=0; gfn < p2m->max_mapped_pfn; gfn++ )
     {

@@ -374,8 +376,6 @@ void p2m_teardown(struct p2m_domain *p2m
     }
 #endif
 
-    p2m_lock(p2m);
-
     p2m->phys_table = pagetable_null();
 
     while ( (pg = page_list_remove_head(&p2m->pages)) )

diff -r 316d227dd526 -r 7fe1bb9208df xen/arch/x86/hvm/emulate.c
@@ -660,12 +660,13 @@ static int hvmemul_rep_movs(
 {
     struct hvm_emulate_ctxt *hvmemul_ctxt =
         container_of(ctxt, struct hvm_emulate_ctxt, ctxt);
-    unsigned long saddr, daddr, bytes, sgfn, dgfn;
+    unsigned long saddr, daddr, bytes;
     paddr_t sgpa, dgpa;
     uint32_t pfec = PFEC_page_present;
-    p2m_type_t p2mt;
+    p2m_type_t sp2mt, dp2mt;
     int rc, df = !!(ctxt->regs->eflags & X86_EFLAGS_DF);
     char *buf;
+    struct two_gfns tg;
 
     rc = hvmemul_virtual_to_linear(
         src_seg, src_offset, bytes_per_rep, reps, hvm_access_read,

@@ -693,26 +694,23 @@ static int hvmemul_rep_movs(
     if ( rc != X86EMUL_OKAY )
         return rc;
 
-    /* XXX In a fine-grained p2m locking scenario, we need to sort this
-     * get_gfn's, or else we might deadlock */
-    sgfn = sgpa >> PAGE_SHIFT;
-    (void)get_gfn(current->domain, sgfn, &p2mt);
-    if ( !p2m_is_ram(p2mt) && !p2m_is_grant(p2mt) )
+    get_two_gfns(current->domain, sgpa >> PAGE_SHIFT, &sp2mt, NULL, NULL,
+                 current->domain, dgpa >> PAGE_SHIFT, &dp2mt, NULL, NULL,
+                 p2m_guest, &tg);
+
+    if ( !p2m_is_ram(sp2mt) && !p2m_is_grant(sp2mt) )
     {
         rc = hvmemul_do_mmio(
             sgpa, reps, bytes_per_rep, dgpa, IOREQ_READ, df, NULL);
-        put_gfn(current->domain, sgfn);
+        put_two_gfns(&tg);
         return rc;
     }
 
-    dgfn = dgpa >> PAGE_SHIFT;
-    (void)get_gfn(current->domain, dgfn, &p2mt);
-    if ( !p2m_is_ram(p2mt) && !p2m_is_grant(p2mt) )
+    if ( !p2m_is_ram(dp2mt) && !p2m_is_grant(dp2mt) )
     {
         rc = hvmemul_do_mmio(
             dgpa, reps, bytes_per_rep, sgpa, IOREQ_WRITE, df, NULL);
-        put_gfn(current->domain, sgfn);
-        put_gfn(current->domain, dgfn);
+        put_two_gfns(&tg);
         return rc;
     }
 

@@ -730,8 +728,7 @@ static int hvmemul_rep_movs(
      */
     if ( ((dgpa + bytes_per_rep) > sgpa) && (dgpa < (sgpa + bytes)) )
     {
-        put_gfn(current->domain, sgfn);
-        put_gfn(current->domain, dgfn);
+        put_two_gfns(&tg);
         return X86EMUL_UNHANDLEABLE;
     }
 

@@ -743,8 +740,7 @@ static int hvmemul_rep_movs(
     buf = xmalloc_bytes(bytes);
     if ( buf == NULL )
     {
-        put_gfn(current->domain, sgfn);
-        put_gfn(current->domain, dgfn);
+        put_two_gfns(&tg);
         return X86EMUL_UNHANDLEABLE;
     }
 

@@ -757,8 +753,7 @@ static int hvmemul_rep_movs(
         rc = hvm_copy_to_guest_phys(dgpa, buf, bytes);
 
     xfree(buf);
-    put_gfn(current->domain, sgfn);
-    put_gfn(current->domain, dgfn);
+    put_two_gfns(&tg);
 
     if ( rc == HVMCOPY_gfn_paged_out )
         return X86EMUL_RETRY;

diff -r 316d227dd526 -r 7fe1bb9208df xen/arch/x86/mm/mem_sharing.c
@@ -727,11 +727,11 @@ int mem_sharing_share_pages(struct domai
     int ret = -EINVAL;
     mfn_t smfn, cmfn;
     p2m_type_t smfn_type, cmfn_type;
+    struct two_gfns tg;
 
-    /* XXX if sd == cd handle potential deadlock by ordering
-     * the get_ and put_gfn's */
-    smfn = get_gfn(sd, sgfn, &smfn_type);
-    cmfn = get_gfn(cd, cgfn, &cmfn_type);
+    get_two_gfns(sd, sgfn, &smfn_type, NULL, &smfn,
+                 cd, cgfn, &cmfn_type, NULL, &cmfn,
+                 p2m_query, &tg);
 
     /* This tricky business is to avoid two callers deadlocking if 
      * grabbing pages in opposite client/source order */

@@ -828,8 +828,7 @@ int mem_sharing_share_pages(struct domai
     ret = 0;
     
 err_out:
-    put_gfn(cd, cgfn);
-    put_gfn(sd, sgfn);
+    put_two_gfns(&tg);
     return ret;
 }
 

@@ -843,11 +842,11 @@ int mem_sharing_add_to_physmap(struct do
     struct gfn_info *gfn_info;
     struct p2m_domain *p2m = p2m_get_hostp2m(cd);
     p2m_access_t a;
-    
-    /* XXX if sd == cd handle potential deadlock by ordering
-     * the get_ and put_gfn's */
-    smfn = get_gfn_query(sd, sgfn, &smfn_type);
-    cmfn = get_gfn_type_access(p2m, cgfn, &cmfn_type, &a, p2m_query, NULL);
+    struct two_gfns tg;
+
+    get_two_gfns(sd, sgfn, &smfn_type, NULL, &smfn,
+                 cd, cgfn, &cmfn_type, &a, &cmfn,
+                 p2m_query, &tg);
 
     /* Get the source shared page, check and lock */
     ret = XEN_DOMCTL_MEM_SHARING_S_HANDLE_INVALID;

@@ -897,8 +896,7 @@ int mem_sharing_add_to_physmap(struct do
 err_unlock:
     mem_sharing_page_unlock(spage);
 err_out:
-    put_gfn(cd, cgfn);
-    put_gfn(sd, sgfn);
+    put_two_gfns(&tg);
     return ret;
 }
 

65 lines not on this page · raw message

diff -r 7fe1bb9208df -r 667191f054c3 xen/arch/x86/mm/p2m.c
@@ -438,7 +438,7 @@ p2m_remove_page(struct p2m_domain *p2m, 
         for ( i = 0; i < (1UL << page_order); i++ )
         {
             mfn_return = p2m->get_entry(p2m, gfn + i, &t, &a, p2m_query, NULL);
-            if ( !p2m_is_grant(t) )
+            if ( !p2m_is_grant(t) && !p2m_is_shared(t) )
                 set_gpfn_from_mfn(mfn+i, INVALID_M2P_ENTRY);
             ASSERT( !p2m_is_valid(t) || mfn + i == mfn_x(mfn_return) );
         }

@@ -500,6 +500,22 @@ guest_physmap_add_entry(struct domain *d
     for ( i = 0; i < (1UL << page_order); i++ )
     {
         omfn = p2m->get_entry(p2m, gfn + i, &ot, &a, p2m_query, NULL);
+#ifdef __x86_64__
+        if ( p2m_is_shared(ot) )
+        {
+            /* Do an unshare to cleanly take care of all corner 
+             * cases. */
+            int rc;
+            rc = mem_sharing_unshare_page(p2m->domain, gfn + i, 0);
+            if ( rc )
+            {
+                p2m_unlock(p2m);
+                return rc;
+            }
+            omfn = p2m->get_entry(p2m, gfn + i, &ot, &a, p2m_query, NULL);
+            ASSERT(!p2m_is_shared(ot));
+        }
+#endif /* __x86_64__ */
         if ( p2m_is_grant(ot) )
         {
             /* Really shouldn't be unmapping grant maps this way */

@@ -528,6 +544,14 @@ guest_physmap_add_entry(struct domain *d
     /* Then, look for m->p mappings for this range and deal with them */
     for ( i = 0; i < (1UL << page_order); i++ )
     {
+        if ( page_get_owner(mfn_to_page(_mfn(mfn + i))) == dom_cow )
+        {
+            /* This is no way to add a shared page to your physmap! */
+            gdprintk(XENLOG_ERR, "Adding shared mfn %lx directly to dom %hu "
+                        "physmap not allowed.\n", mfn+i, d->domain_id);
+            p2m_unlock(p2m);
+            return -EINVAL;
+        }
         if ( page_get_owner(mfn_to_page(_mfn(mfn + i))) != d )
             continue;
         ogfn = mfn_to_gfn(d, _mfn(mfn+i));