[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[Xen-changelog] [xen master] properly reference count DOMCTL_{, un}pausedomain hypercalls



commit 3eb1c708ab0fe1067a436498a684907afa14dacf
Author:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
AuthorDate: Thu Jul 3 16:51:13 2014 +0200
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Thu Jul 3 16:51:13 2014 +0200

    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@xxxxxxxxxxx>
    Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    With a couple of formatting adjustments:
    Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
---
 xen/arch/x86/domctl.c   |    6 ++--
 xen/common/domain.c     |   61 +++++++++++++++++++++++++++++++++-------------
 xen/common/domctl.c     |   18 ++++----------
 xen/include/xen/sched.h |   15 +++++++++--
 4 files changed, 64 insertions(+), 36 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index a4effc3..d62c715 100644
--- a/xen/arch/x86/domctl.c
+++ b/xen/arch/x86/domctl.c
@@ -1027,7 +1027,7 @@ long arch_do_domctl(
         struct vcpu *v;
 
         ret = -EBUSY;
-        if ( !d->is_paused_by_controller )
+        if ( d->controller_pause_count > 0 )
             break;
         ret = -EINVAL;
         if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
@@ -1043,7 +1043,7 @@ long arch_do_domctl(
         struct vcpu *v;
 
         ret = -EBUSY;
-        if ( !d->is_paused_by_controller )
+        if ( d->controller_pause_count > 0 )
             break;
         ret = -EINVAL;
         if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
@@ -1061,7 +1061,7 @@ long arch_do_domctl(
         struct vcpu *v;
 
         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 c3a576e..cd64aea 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -317,7 +317,7 @@ struct domain *domain_create(
         if ( (err = xsm_domain_create(XSM_HOOK, d, ssidref)) != 0 )
             goto fail;
 
-        d->is_paused_by_controller = 1;
+        d->controller_pause_count = 1;
         atomic_inc(&d->pause_count);
 
         if ( !is_hardware_domain(d) )
@@ -755,18 +755,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;
+    struct vcpu *curr = current;
+    struct domain *d = curr->domain;
 
-    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) */
-
-    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
@@ -914,17 +909,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 000993f..c326aba 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -181,7 +181,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;
 
@@ -394,22 +394,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) 
u_domctl)
     break;
 
     case XEN_DOMCTL_pausedomain:
-    {
         ret = -EINVAL;
         if ( d != current->domain )
-        {
-            domain_pause_by_systemcontroller(d);
-            ret = 0;
-        }
-    }
-    break;
+            ret = domain_pause_by_systemcontroller(d);
+        break;
 
     case XEN_DOMCTL_unpausedomain:
-    {
-        domain_unpause_by_systemcontroller(d);
-        ret = 0;
-    }
-    break;
+        ret = domain_unpause_by_systemcontroller(d);
+        break;
 
     case XEN_DOMCTL_resumedomain:
     {
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index f920e1a..d5bc461 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -366,7 +366,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;
 
@@ -769,8 +769,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;
--
generated by git-patchbot for /home/xen/git/xen.git#master

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxx
http://lists.xensource.com/xen-changelog


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.