x86/mm: Fix memory-sharing code's locking discipline.

memshr_audit is sometimes called with the shr_lock held.  Make it so for
every call.

Move the unsharing loop in p2m_teardown out of the p2m_lock to avoid
deadlocks.

Signed-off-by: Tim Deegan <Tim.Deegan@citrix.com>

diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 28018f254a7b9782aaffe8aa0145ab7f4e074f5b..d575008b15a189765ca2bd41de286c1f57b46805 100644 (file)
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -225,7 +225,7 @@ static void mem_sharing_audit(void)
     int bucket;
     struct page_info *pg;
 
-    shr_lock();
+    ASSERT(shr_locked_by_me());
 
     for(bucket=0; bucket < SHR_HASH_LENGTH; bucket++)
     {
@@ -285,8 +285,6 @@ static void mem_sharing_audit(void)
             e = e->next;
         }
     }
-
-    shr_unlock();
 }
 #endif
 
@@ -632,10 +630,10 @@ int mem_sharing_unshare_page(struct domain *d,
     shr_handle_t handle;
     struct list_head *le;
 
-    mem_sharing_audit();
-    /* Remove the gfn_info from the list */
     shr_lock();
+    mem_sharing_audit();
     
+    /* Remove the gfn_info from the list */
     mfn = gfn_to_mfn(d, gfn, &p2mt);
     
     /* Has someone already unshared it? */
@@ -739,7 +737,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
         case XEN_DOMCTL_MEM_SHARING_OP_CONTROL:
         {
             d->arch.hvm_domain.mem_sharing_enabled = mec->u.enable;
-            mem_sharing_audit();
             rc = 0;
         }
         break;
@@ -752,7 +749,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
                 return -EINVAL;
             rc = mem_sharing_nominate_page(d, gfn, 0, &handle);
             mec->u.nominate.handle = handle;
-            mem_sharing_audit();
         }
         break;
 
@@ -768,7 +764,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
                 return -EINVAL;
             rc = mem_sharing_nominate_page(d, gfn, 3, &handle);
             mec->u.nominate.handle = handle;
-            mem_sharing_audit();
         }
         break;
 
@@ -777,7 +772,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
             shr_handle_t sh = mec->u.share.source_handle;
             shr_handle_t ch = mec->u.share.client_handle;
             rc = mem_sharing_share_pages(sh, ch); 
-            mem_sharing_audit();
         }
         break;
 
@@ -785,7 +779,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
         {
             if(!mem_sharing_enabled(d))
                 return -EINVAL;
-            mem_sharing_audit();
             rc = mem_sharing_sharing_resume(d);
         }
         break;
@@ -794,7 +787,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
         {
             unsigned long gfn = mec->u.debug.u.gfn;
             rc = mem_sharing_debug_gfn(d, gfn);
-            mem_sharing_audit();
         }
         break;
 
@@ -802,7 +794,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
         {
             unsigned long mfn = mec->u.debug.u.mfn;
             rc = mem_sharing_debug_mfn(mfn);
-            mem_sharing_audit();
         }
         break;
 
@@ -810,7 +801,6 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
         {
             grant_ref_t gref = mec->u.debug.u.gref;
             rc = mem_sharing_debug_gref(d, gref);
-            mem_sharing_audit();
         }
         break;
 
@@ -819,6 +809,10 @@ int mem_sharing_domctl(struct domain *d, xen_domctl_mem_sharing_op_t *mec)
             break;
     }
 
+    shr_lock();
+    mem_sharing_audit();
+    shr_unlock();
+
     return rc;
 }
 

diff --git a/xen/arch/x86/mm/p2m.c b/xen/arch/x86/mm/p2m.c
index c4a531dca314db9a0858f9360f35d8b085b6253c..e555343b518568e5f789614badbd0fd08860b146 100644 (file)
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -296,12 +296,10 @@ 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++ )
     {
-        mfn = p2m->get_entry(p2m, gfn, &t, &a, p2m_query);
+        mfn = gfn_to_mfn_type_p2m(p2m, gfn, &t, &a, p2m_query);
         if ( mfn_valid(mfn) && (t == p2m_ram_shared) )
         {
             ASSERT(!p2m_is_nestedp2m(p2m));
@@ -311,6 +309,8 @@ void p2m_teardown(struct p2m_domain *p2m)
     }
 #endif
 
+    p2m_lock(p2m);
+
     p2m->phys_table = pagetable_null();
 
     while ( (pg = page_list_remove_head(&p2m->pages)) )