properly reference count DOMCTL_{,un}pausedomain hypercalls

For safety reasons, c/s 6ae2df93c27 "mem_access: Add helper API to setup
ring and enable mem_access" has to pause the domain while it performs a set of
operations.

However without properly reference counted hypercalls, xc_mem_event_enable()
now unconditionally unpauses a previously paused domain.

To prevent toolstack software running wild, there is an arbitrary limit of 255
on the toolstack pause count.  This is high enough for several components of
the toolstack to safely use, but prevents over/underflow of d->pause_count.

The previous domain_{,un}pause_by_systemcontroller() functions are updated to
return an error code.  domain_pause_by_systemcontroller() is modified to have
a common stub and take a pause_fn pointer, allowing for both sync and nosync
domain pauses.  domain_pause_for_debugger() has a hand-rolled nosync pause
replaced with the new domain_pause_by_systemcontroller_nosync(), and has its
variables shuffled slightly to avoid rereading current multiple times.

Suggested-by: Don Slutz <dslutz@verizon.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
With a couple of formatting adjustments:
Reviewed-by: Jan Beulich <jbeulich@suse.com>
x86/gdbsx: invert preconditions for XEN_DOMCTL_gdbsx_{,un}pausevcpu hypercalls

c/s 3eb1c708ab "properly reference count DOMCTL_{,un}pausedomain hypercalls"
accidentally inverted the use of d->controller_pause_count.

Revert back to how it was originally, i.e. the XEN_DOMCTL_gdbsx_{,un}pausevcpu
hypercalls are only valid for a domain already paused by the system controller.

Reported-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
master commit: 3eb1c708ab0fe1067a436498a684907afa14dacf
master date: 2014-07-03 16:51:13 +0200
master commit: 680d79f10bb70691a9ae3b4a6a8b669e0f2837f6
master date: 2014-07-25 11:53:31 +0200

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index bb80b234a63a698dc2bcd407a8ec9a50d3f65cf1..48de4bfc86cf7f280987061ed128668dc2bd0eac 100644 (file)
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1306,7 +1306,7 @@ long arch_do_domctl(
             break;
 
         ret = -EBUSY;
-        if ( !d->is_paused_by_controller )
+        if ( !d->controller_pause_count )
         {
             rcu_unlock_domain(d);
             break;
@@ -1334,7 +1334,7 @@ long arch_do_domctl(
             break;
 
         ret = -EBUSY;
-        if ( !d->is_paused_by_controller )
+        if ( !d->controller_pause_count )
         {
             rcu_unlock_domain(d);
             break;
@@ -1364,7 +1364,7 @@ long arch_do_domctl(
             break;
 
         domctl->u.gdbsx_domstatus.vcpu_id = -1;
-        domctl->u.gdbsx_domstatus.paused = d->is_paused_by_controller;
+        domctl->u.gdbsx_domstatus.paused = d->controller_pause_count > 0;
         if ( domctl->u.gdbsx_domstatus.paused )
         {
             for_each_vcpu ( d, v )

diff --git a/xen/common/domain.c b/xen/common/domain.c
index 1fde2610693424a1a2878a7f54ef45b93104e99f..73842f9312977e41d88b12a4ba791e8183b9f471 100644 (file)
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -257,7 +257,7 @@ struct domain *domain_create(
         if ( (err = xsm_domain_create(d, ssidref)) != 0 )
             goto fail;
 
-        d->is_paused_by_controller = 1;
+        d->controller_pause_count = 1;
         atomic_inc(&d->pause_count);
 
         if ( domid )
@@ -621,18 +621,13 @@ void vcpu_end_shutdown_deferral(struct vcpu *v)
 #ifdef HAS_GDBSX
 void domain_pause_for_debugger(void)
 {
-    struct domain *d = current->domain;
-    struct vcpu *v;
-
-    atomic_inc(&d->pause_count);
-    if ( test_and_set_bool(d->is_paused_by_controller) )
-        domain_unpause(d); /* race-free atomic_dec(&d->pause_count) */
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
 
-    for_each_vcpu ( d, v )
-        vcpu_sleep_nosync(v);
+    domain_pause_by_systemcontroller_nosync(d);
 
     /* if gdbsx active, we just need to pause the domain */
-    if (current->arch.gdbsx_vcpu_event == 0)
+    if ( curr->arch.gdbsx_vcpu_event == 0 )
         send_global_virq(VIRQ_DEBUGGER);
 }
 #endif
@@ -779,17 +774,49 @@ void domain_unpause(struct domain *d)
             vcpu_wake(v);
 }
 
-void domain_pause_by_systemcontroller(struct domain *d)
+int __domain_pause_by_systemcontroller(struct domain *d,
+                                       void (*pause_fn)(struct domain *d))
 {
-    domain_pause(d);
-    if ( test_and_set_bool(d->is_paused_by_controller) )
-        domain_unpause(d);
+    int old, new, prev = d->controller_pause_count;
+
+    do
+    {
+        old = prev;
+        new = old + 1;
+
+        /*
+         * Limit the toolstack pause count to an arbitrary 255 to prevent the
+         * toolstack overflowing d->pause_count with many repeated hypercalls.
+         */
+        if ( new > 255 )
+            return -EUSERS;
+
+        prev = cmpxchg(&d->controller_pause_count, old, new);
+    } while ( prev != old );
+
+    pause_fn(d);
+
+    return 0;
 }
 
-void domain_unpause_by_systemcontroller(struct domain *d)
+int domain_unpause_by_systemcontroller(struct domain *d)
 {
-    if ( test_and_clear_bool(d->is_paused_by_controller) )
-        domain_unpause(d);
+    int old, new, prev = d->controller_pause_count;
+
+    do
+    {
+        old = prev;
+        new = old - 1;
+
+        if ( new < 0 )
+            return -EINVAL;
+
+        prev = cmpxchg(&d->controller_pause_count, old, new);
+    } while ( prev != old );
+
+    domain_unpause(d);
+
+    return 0;
 }
 
 int vcpu_reset(struct vcpu *v)

diff --git a/xen/common/domctl.c b/xen/common/domctl.c
index b3bfb388ced321f6f807f05f48fe887824dc87c3..0ad3f7535a155c33c273ad6975f6563f47f805a3 100644 (file)
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -144,7 +144,7 @@ void getdomaininfo(struct domain *d, struct xen_domctl_getdomaininfo *info)
     info->flags = (info->nr_online_vcpus ? flags : 0) |
         ((d->is_dying == DOMDYING_dead) ? XEN_DOMINF_dying    : 0) |
         (d->is_shut_down                ? XEN_DOMINF_shutdown : 0) |
-        (d->is_paused_by_controller     ? XEN_DOMINF_paused   : 0) |
+        (d->controller_pause_count > 0  ? XEN_DOMINF_paused   : 0) |
         (d->debugger_attached           ? XEN_DOMINF_debugged : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;
 
@@ -363,10 +363,7 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
 
             ret = -EINVAL;
             if ( d != current->domain )
-            {
-                domain_pause_by_systemcontroller(d);
-                ret = 0;
-            }
+                ret = domain_pause_by_systemcontroller(d);
         pausedomain_out:
             rcu_unlock_domain(d);
         }
@@ -388,9 +385,8 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
             break;
         }
 
-        domain_unpause_by_systemcontroller(d);
+        ret = domain_unpause_by_systemcontroller(d);
         rcu_unlock_domain(d);
-        ret = 0;
     }
     break;
 

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 0ab0fb41cf308fbf0333af5efaa69250a11ef1db..d04e96b9617a5d6163f9b9abf944b0d608bb7a72 100644 (file)
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -283,7 +283,7 @@ struct domain
     /* Is this guest dying (i.e., a zombie)? */
     enum { DOMDYING_alive, DOMDYING_dying, DOMDYING_dead } is_dying;
     /* Domain is paused by controller software? */
-    bool_t           is_paused_by_controller;
+    int              controller_pause_count;
     /* Domain's VCPUs are pinned 1:1 to physical CPUs? */
     bool_t           is_pinned;
 
@@ -662,8 +662,17 @@ void domain_pause(struct domain *d);
 void domain_pause_nosync(struct domain *d);
 void vcpu_unpause(struct vcpu *v);
 void domain_unpause(struct domain *d);
-void domain_pause_by_systemcontroller(struct domain *d);
-void domain_unpause_by_systemcontroller(struct domain *d);
+int domain_unpause_by_systemcontroller(struct domain *d);
+int __domain_pause_by_systemcontroller(struct domain *d,
+                                       void (*pause_fn)(struct domain *d));
+static inline int domain_pause_by_systemcontroller(struct domain *d)
+{
+    return __domain_pause_by_systemcontroller(d, domain_pause);
+}
+static inline int domain_pause_by_systemcontroller_nosync(struct domain *d)
+{
+    return __domain_pause_by_systemcontroller(d, domain_pause_nosync);
+}
 void cpu_init(void);
 
 struct scheduler;