Avoid deadlocks on domctl_lock when pausing domains/vcpus.

Signed-off-by: Keir Fraser <keir.fraser@citrix.com>

4 files changed, 43 insertions, 9 deletions

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 9a6d6cd5a1..bc2046fb78 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2489,20 +2489,23 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg)
                 if ( !paging_mode_hap(d) )
                     break;
 
-                domain_pause(d);
-
                 /*
                  * Update GUEST_CR3 in each VMCS to point at identity map.
                  * All foreign updates to guest state must synchronise on
                  * the domctl_lock.
                  */
-                spin_lock(&domctl_lock);
+                rc = -EAGAIN;
+                if ( !domctl_lock_acquire() )
+                    break;
+
+                rc = 0;
+                domain_pause(d);
                 d->arch.hvm_domain.params[a.index] = a.value;
                 for_each_vcpu ( d, v )
                     paging_update_cr3(v);
-                spin_unlock(&domctl_lock);
-
                 domain_unpause(d);
+
+                domctl_lock_release();
                 break;
             case HVM_PARAM_DM_DOMAIN:
                 /* Privileged domains only, as we must domain_pause(d). */
diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index 23c2f4e529..4a8df90101 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -25,7 +25,7 @@
 #include <public/domctl.h>
 #include <xsm/xsm.h>
 
-DEFINE_SPINLOCK(domctl_lock);
+static DEFINE_SPINLOCK(domctl_lock);
 
 extern long arch_do_domctl(
     struct xen_domctl *op, XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);
@@ -188,6 +188,33 @@ static unsigned int default_vcpu0_location(void)
     return cpu;
 }
 
+bool_t domctl_lock_acquire(void)
+{
+    /*
+     * Caller may try to pause its own VCPUs. We must prevent deadlock
+     * against other non-domctl routines which try to do the same.
+     */
+    if ( !spin_trylock(&current->domain->hypercall_deadlock_mutex) )
+        return 0;
+
+    /*
+     * Trylock here is paranoia if we have multiple privileged domains. Then
+     * we could have one domain trying to pause another which is spinning
+     * on domctl_lock -- results in deadlock.
+     */
+    if ( spin_trylock(&domctl_lock) )
+        return 1;
+
+    spin_unlock(&current->domain->hypercall_deadlock_mutex);
+    return 0;
+}
+
+void domctl_lock_release(void)
+{
+    spin_unlock(&domctl_lock);
+    spin_unlock(&current->domain->hypercall_deadlock_mutex);
+}
+
 long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
 {
     long ret = 0;
@@ -202,7 +229,9 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
     if ( op->interface_version != XEN_DOMCTL_INTERFACE_VERSION )
         return -EACCES;
 
-    spin_lock(&domctl_lock);
+    if ( !domctl_lock_acquire() )
+        return hypercall_create_continuation(
+            __HYPERVISOR_domctl, "h", u_domctl);
 
     switch ( op->cmd )
     {
@@ -866,7 +895,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
         break;
     }
 
-    spin_unlock(&domctl_lock);
+    domctl_lock_release();
 
     return ret;
 }
diff --git a/xen/include/xen/domain.h b/xen/include/xen/domain.h
index 65df554421..282e5665d0 100644
--- a/xen/include/xen/domain.h
+++ b/xen/include/xen/domain.h
@@ -58,6 +58,9 @@ void arch_dump_domain_info(struct domain *d);
 
 void arch_vcpu_reset(struct vcpu *v);
 
+bool_t domctl_lock_acquire(void);
+void domctl_lock_release(void);
+
 extern unsigned int xen_processor_pmbits;
 
 #endif /* __XEN_DOMAIN_H__ */
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 99d2e0008a..43758b9de8 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -30,7 +30,6 @@ do_sched_op(
     int cmd,
     XEN_GUEST_HANDLE(void) arg);
 
-extern spinlock_t domctl_lock;
 extern long
 do_domctl(
     XEN_GUEST_HANDLE(xen_domctl_t) u_domctl);