[PATCH v2 0/3] Improvements to allow refcoutning of DOMCTL_{, un}pausedomain

[PATCH v2 0/3] Improvements to allow refcoutning of DOMCTL_{, un}pausedomain

[Andrew Cooper]

The first patch has no functional change, but introduces a common atomic.h

The second patch introduces new common atomic_t manipulation function

The third patch uses these new common functions as a basis for fixing the
DOMCTL_{,un}pausedomain reference counting issues.

This has been functionally tested on x86, and compile tested on both flavours
of ARM.

Andrew Cooper (3):
  xen/atomic: Introduce common atomic header and update includes
  xen/atomic: Implement atomic_{inc,dec}_bounded()
  xen/common: Properly reference count DOMCTL_{,un}pausedomain
    hypercalls

 xen/arch/x86/apic.c                 |    2 +-
 xen/arch/x86/cpu/mcheck/barrier.h   |    2 +-
 xen/arch/x86/cpu/mcheck/mce.h       |    2 +-
 xen/arch/x86/crash.c                |    2 +-
 xen/arch/x86/domctl.c               |    7 +++---
 xen/arch/x86/i8259.c                |    2 +-
 xen/arch/x86/mm/mem_sharing.c       |    2 +-
 xen/arch/x86/mm/shadow/private.h    |    2 +-
 xen/arch/x86/traps.c                |    2 +-
 xen/common/Makefile                 |    1 +
 xen/common/atomic.c                 |   35 ++++++++++++++++++++++++++++++
 xen/common/domain.c                 |   41 ++++++++++++++++++++---------------
 xen/common/domctl.c                 |    9 ++++----
 xen/common/kexec.c                  |    2 +-
 xen/common/rcupdate.c               |    2 +-
 xen/common/sched_credit.c           |    2 +-
 xen/common/spinlock.c               |    2 +-
 xen/common/timer.c                  |    2 +-
 xen/common/trace.c                  |    2 +-
 xen/drivers/passthrough/arm/smmu.c  |    2 +-
 xen/include/acpi/platform/aclinux.h |    2 +-
 xen/include/asm-x86/atomic.h        |    5 +++++
 xen/include/asm-x86/irq.h           |    2 +-
 xen/include/asm-x86/spinlock.h      |    2 +-
 xen/include/xen/atomic.h            |   26 ++++++++++++++++++++++
 xen/include/xen/gdbstub.h           |    2 +-
 xen/include/xen/sched.h             |   17 +++++++++++----
 xen/xsm/flask/avc.c                 |    2 +-
 28 files changed, 132 insertions(+), 49 deletions(-)
 create mode 100644 xen/common/atomic.c
 create mode 100644 xen/include/xen/atomic.h

-- 
1.7.10.4

[PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes

[Andrew Cooper]

This patch does reorder the #includes in asm-arm/atomic.h to pull in
definitions in the correct order, which would otherwise cause a build failure
in patch #2 because of struct vcpu_guest_core_regs being referenced before
definition.

No functional change.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>
---
 xen/arch/x86/apic.c                 |    2 +-
 xen/arch/x86/cpu/mcheck/barrier.h   |    2 +-
 xen/arch/x86/cpu/mcheck/mce.h       |    2 +-
 xen/arch/x86/crash.c                |    2 +-
 xen/arch/x86/i8259.c                |    2 +-
 xen/arch/x86/mm/mem_sharing.c       |    2 +-
 xen/arch/x86/mm/shadow/private.h    |    2 +-
 xen/arch/x86/traps.c                |    2 +-
 xen/common/kexec.c                  |    2 +-
 xen/common/rcupdate.c               |    2 +-
 xen/common/sched_credit.c           |    2 +-
 xen/common/spinlock.c               |    2 +-
 xen/common/timer.c                  |    2 +-
 xen/common/trace.c                  |    2 +-
 xen/drivers/passthrough/arm/smmu.c  |    2 +-
 xen/include/acpi/platform/aclinux.h |    2 +-
 xen/include/asm-arm/atomic.h        |    2 +-
 xen/include/asm-x86/irq.h           |    2 +-
 xen/include/asm-x86/spinlock.h      |    2 +-
 xen/include/xen/atomic.h            |    6 ++++++
 xen/include/xen/gdbstub.h           |    2 +-
 xen/include/xen/sched.h             |    2 +-
 xen/xsm/flask/avc.c                 |    2 +-
 23 files changed, 28 insertions(+), 22 deletions(-)
 create mode 100644 xen/include/xen/atomic.h

diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
index 0e5e302..47f8ef9 100644
--- a/xen/arch/x86/apic.c
+++ b/xen/arch/x86/apic.c
@@ -28,7 +28,7 @@
 #include <xen/softirq.h>
 #include <asm/mc146818rtc.h>
 #include <asm/msr.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/mpspec.h>
 #include <asm/flushtlb.h>
 #include <asm/hardirq.h>
diff --git a/xen/arch/x86/cpu/mcheck/barrier.h b/xen/arch/x86/cpu/mcheck/barrier.h
index 87f7550..ca33f5d 100644
--- a/xen/arch/x86/cpu/mcheck/barrier.h
+++ b/xen/arch/x86/cpu/mcheck/barrier.h
@@ -1,7 +1,7 @@
 #ifndef _MCHECK_BARRIER_H
 #define _MCHECK_BARRIER_H
 
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 
 /* MCE handling */
 struct mce_softirq_barrier {
diff --git a/xen/arch/x86/cpu/mcheck/mce.h b/xen/arch/x86/cpu/mcheck/mce.h
index e83d431..2891469 100644
--- a/xen/arch/x86/cpu/mcheck/mce.h
+++ b/xen/arch/x86/cpu/mcheck/mce.h
@@ -7,7 +7,7 @@
 #include <xen/smp.h>
 #include <asm/types.h>
 #include <asm/traps.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/percpu.h>
 
 #include "x86_mca.h"
diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index c0b83df..974fd0f 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -8,7 +8,7 @@
  * - Magnus Damm <magnus@valinux.co.jp>
  */
 
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/elf.h>
 #include <asm/percpu.h>
 #include <xen/types.h>
diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index c2c9005..2a8cf76 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -12,7 +12,7 @@
 #include <xen/errno.h>
 #include <xen/sched.h>
 #include <xen/irq.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/system.h>
 #include <asm/io.h>
 #include <asm/desc.h>
diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c
index 7293f31..92a68eb 100644
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -31,7 +31,7 @@
 #include <asm/string.h>
 #include <asm/p2m.h>
 #include <asm/mem_event.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <xen/rcupdate.h>
 #include <asm/event.h>
 #include <xsm/xsm.h>
diff --git a/xen/arch/x86/mm/shadow/private.h b/xen/arch/x86/mm/shadow/private.h
index b778fcf..4958500 100644
--- a/xen/arch/x86/mm/shadow/private.h
+++ b/xen/arch/x86/mm/shadow/private.h
@@ -30,7 +30,7 @@
 #include <xen/domain_page.h>
 #include <asm/x86_emulate.h>
 #include <asm/hvm/support.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 
 #include "../mm-locks.h"
 
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 677074b..45f9ed6 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -52,7 +52,7 @@
 #include <xen/watchdog.h>
 #include <asm/system.h>
 #include <asm/io.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <xen/bitops.h>
 #include <asm/desc.h>
 #include <asm/debugreg.h>
diff --git a/xen/common/kexec.c b/xen/common/kexec.c
index 2239ee8..5055461 100644
--- a/xen/common/kexec.c
+++ b/xen/common/kexec.c
@@ -20,7 +20,7 @@
 #include <xen/keyhandler.h>
 #include <public/kexec.h>
 #include <xen/cpumask.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <xen/spinlock.h>
 #include <xen/version.h>
 #include <xen/console.h>
diff --git a/xen/common/rcupdate.c b/xen/common/rcupdate.c
index e9979cd..5d7a31b 100644
--- a/xen/common/rcupdate.c
+++ b/xen/common/rcupdate.c
@@ -39,7 +39,7 @@
 #include <xen/smp.h>
 #include <xen/rcupdate.h>
 #include <xen/sched.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <xen/bitops.h>
 #include <xen/percpu.h>
 #include <xen/softirq.h>
diff --git a/xen/common/sched_credit.c b/xen/common/sched_credit.c
index 8b02b7b..68caa8f 100644
--- a/xen/common/sched_credit.c
+++ b/xen/common/sched_credit.c
@@ -18,7 +18,7 @@
 #include <xen/time.h>
 #include <xen/sched-if.h>
 #include <xen/softirq.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/div64.h>
 #include <xen/errno.h>
 #include <xen/keyhandler.h>
diff --git a/xen/common/spinlock.c b/xen/common/spinlock.c
index 575cc6d..11a63c7 100644
--- a/xen/common/spinlock.c
+++ b/xen/common/spinlock.c
@@ -8,7 +8,7 @@
 #include <xen/preempt.h>
 #include <public/sysctl.h>
 #include <asm/processor.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 
 #ifndef NDEBUG
 
diff --git a/xen/common/timer.c b/xen/common/timer.c
index f36aebc..96573ac 100644
--- a/xen/common/timer.c
+++ b/xen/common/timer.c
@@ -23,7 +23,7 @@
 #include <xen/symbols.h>
 #include <asm/system.h>
 #include <asm/desc.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 
 /* We program the time hardware this far behind the closest deadline. */
 static unsigned int timer_slop __read_mostly = 50000; /* 50 us */
diff --git a/xen/common/trace.c b/xen/common/trace.c
index f651cf3..0a7b2c3 100644
--- a/xen/common/trace.c
+++ b/xen/common/trace.c
@@ -31,7 +31,7 @@
 #include <xen/percpu.h>
 #include <xen/pfn.h>
 #include <xen/cpu.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <public/sysctl.h>
 
 #ifdef CONFIG_COMPAT
diff --git a/xen/drivers/passthrough/arm/smmu.c b/xen/drivers/passthrough/arm/smmu.c
index f4eb2a2..5189d46 100644
--- a/xen/drivers/passthrough/arm/smmu.c
+++ b/xen/drivers/passthrough/arm/smmu.c
@@ -43,7 +43,7 @@
 #include <xen/vmap.h>
 #include <xen/rbtree.h>
 #include <xen/sched.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/device.h>
 #include <asm/io.h>
 #include <asm/platform.h>
diff --git a/xen/include/acpi/platform/aclinux.h b/xen/include/acpi/platform/aclinux.h
index 239ced2..4f304c3 100644
--- a/xen/include/acpi/platform/aclinux.h
+++ b/xen/include/acpi/platform/aclinux.h
@@ -54,7 +54,7 @@
 #include <xen/ctype.h>
 #include <xen/spinlock.h>
 #include <asm/system.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/div64.h>
 #include <asm/acpi.h>
 #include <asm/current.h>
diff --git a/xen/include/asm-arm/atomic.h b/xen/include/asm-arm/atomic.h
index 7d15fb0..ee29d4e 100644
--- a/xen/include/asm-arm/atomic.h
+++ b/xen/include/asm-arm/atomic.h
@@ -2,8 +2,8 @@
 #define __ARCH_ARM_ATOMIC__
 
 #include <xen/config.h>
-#include <xen/prefetch.h>
 #include <asm/system.h>
+#include <xen/prefetch.h>
 
 #define build_atomic_read(name, size, width, type, reg)\
 static inline type name(const volatile type *addr) \
diff --git a/xen/include/asm-x86/irq.h b/xen/include/asm-x86/irq.h
index 9066d38..9906035 100644
--- a/xen/include/asm-x86/irq.h
+++ b/xen/include/asm-x86/irq.h
@@ -4,7 +4,7 @@
 /* (C) 1992, 1993 Linus Torvalds, (C) 1997 Ingo Molnar */
 
 #include <xen/config.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <xen/cpumask.h>
 #include <xen/smp.h>
 #include <xen/hvm/irq.h>
diff --git a/xen/include/asm-x86/spinlock.h b/xen/include/asm-x86/spinlock.h
index 6bc044c..a315e88 100644
--- a/xen/include/asm-x86/spinlock.h
+++ b/xen/include/asm-x86/spinlock.h
@@ -3,7 +3,7 @@
 
 #include <xen/config.h>
 #include <xen/lib.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 
 typedef struct {
     volatile s16 lock;
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
new file mode 100644
index 0000000..141f9e9
--- /dev/null
+++ b/xen/include/xen/atomic.h
@@ -0,0 +1,6 @@
+#ifndef __XEN_ATOMIC_H__
+#define __XEN_ATOMIC_H__
+
+#include <asm/atomic.h>
+
+#endif /* __XEN_ATOMIC_H__ */
diff --git a/xen/include/xen/gdbstub.h b/xen/include/xen/gdbstub.h
index 67d7410..6b11425 100644
--- a/xen/include/xen/gdbstub.h
+++ b/xen/include/xen/gdbstub.h
@@ -21,7 +21,7 @@
 #ifndef __XEN_GDBSTUB_H__
 #define __XEN_GDBSTUB_H__
 
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/page.h>
 
 #ifdef CRASH_DEBUG
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index f920e1a..a953469 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -17,7 +17,7 @@
 #include <xen/mm.h>
 #include <xen/smp.h>
 #include <xen/perfc.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <xen/wait.h>
 #include <public/xen.h>
 #include <public/domctl.h>
diff --git a/xen/xsm/flask/avc.c b/xen/xsm/flask/avc.c
index fc6580e..234aaba 100644
--- a/xen/xsm/flask/avc.c
+++ b/xen/xsm/flask/avc.c
@@ -26,7 +26,7 @@
 #include <xen/sched.h>
 #include <xen/init.h>
 #include <xen/rcupdate.h>
-#include <asm/atomic.h>
+#include <xen/atomic.h>
 #include <asm/current.h>
 #include <public/xsm/flask_op.h>
 
-- 
1.7.10.4

[PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded()

[Andrew Cooper]

These will increment/decremented an atomic_t, while ensuring that the atomic_t
doesn't hit a specific bound.

This involves introducing atomic_cmpxchg() for x86, which previously only
existed on ARM.

Suggested-by: Don Slutz <dslutz@verizon.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/common/Makefile          |    1 +
 xen/common/atomic.c          |   35 +++++++++++++++++++++++++++++++++++
 xen/include/asm-x86/atomic.h |    5 +++++
 xen/include/xen/atomic.h     |   20 ++++++++++++++++++++
 4 files changed, 61 insertions(+)
 create mode 100644 xen/common/atomic.c

diff --git a/xen/common/Makefile b/xen/common/Makefile
index 3683ae3..7c1ee1d 100644
--- a/xen/common/Makefile
+++ b/xen/common/Makefile
@@ -1,3 +1,4 @@
+obj-y += atomic.o
 obj-y += bitmap.o
 obj-y += core_parking.o
 obj-y += cpu.o
diff --git a/xen/common/atomic.c b/xen/common/atomic.c
new file mode 100644
index 0000000..64408e3
--- /dev/null
+++ b/xen/common/atomic.c
@@ -0,0 +1,35 @@
+#include <xen/atomic.h>
+
+bool_t atomic_inc_bounded(atomic_t *v, int bound)
+{
+    int old, new, prev = atomic_read(v);
+
+    do
+    {
+        old = prev;
+        new = old + 1;
+        if ( new >= bound )
+            return 0;
+
+        prev = atomic_cmpxchg(v, old, new);
+    } while ( prev != old );
+
+    return 1;
+}
+
+bool_t atomic_dec_bounded(atomic_t *v, int bound)
+{
+    int old, new, prev = atomic_read(v);
+
+    do
+    {
+        old = prev;
+        new = old - 1;
+        if ( new <= bound )
+            return 0;
+
+        prev = atomic_cmpxchg(v, old, new);
+    } while ( prev != old );
+
+    return 1;
+}
diff --git a/xen/include/asm-x86/atomic.h b/xen/include/asm-x86/atomic.h
index 8972463..43a250b 100644
--- a/xen/include/asm-x86/atomic.h
+++ b/xen/include/asm-x86/atomic.h
@@ -255,4 +255,9 @@ static inline atomic_t atomic_compareandswap(
     return rc;
 }
 
+static inline int atomic_cmpxchg(atomic_t *ptr, int old, int new)
+{
+    return __cmpxchg(&ptr->counter, old, new, sizeof(ptr->counter));
+}
+
 #endif /* __ARCH_X86_ATOMIC__ */
diff --git a/xen/include/xen/atomic.h b/xen/include/xen/atomic.h
index 141f9e9..d1a6b57 100644
--- a/xen/include/xen/atomic.h
+++ b/xen/include/xen/atomic.h
@@ -3,4 +3,24 @@
 
 #include <asm/atomic.h>
 
+/**
+ * atomic_inc_bounded - increment @v if it wouldn't exceed @bound
+ * @v:     pointer of type atomic_t
+ * @bound: boundary to compare against
+ *
+ * returns true if @v was incremented, or false if incrementing @v would hit
+ * @bound.
+ */
+bool_t atomic_inc_bounded(atomic_t *v, int bound);
+
+/**
+ * atomic_dec_bounded - decrement @v if it wouldn't exceed @bound
+ * @v:     pointer of type atomic_t
+ * @bound: boundary to compare against
+ *
+ * returns true if @v was decremented, or false if decrementing @v would hit
+ * @bound.
+ */
+bool_t atomic_dec_bounded(atomic_t *v, int bound);
+
 #endif /* __XEN_ATOMIC_H__ */
-- 
1.7.10.4

[PATCH v2 3/3] xen/common: Properly reference count DOMCTL_{, un}pausedomain hypercalls

[Andrew Cooper]

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.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Ian Campbell <ian.campbell@citrix.com>
CC: Stefano Stabellini <stefano.stabellini@citrix.com>
CC: Tim Deegan <tim@xen.org>

---

This has been functionally tested on x86, and compile tested on both flavours
of ARM.

v2:
 * Use atomic_{inc,dec}_bounded() instead of having a spinlock.
---
 xen/arch/x86/domctl.c   |    7 ++++---
 xen/common/domain.c     |   41 ++++++++++++++++++++++++-----------------
 xen/common/domctl.c     |    9 ++++-----
 xen/include/xen/sched.h |   15 ++++++++++++---
 4 files changed, 44 insertions(+), 28 deletions(-)

diff --git a/xen/arch/x86/domctl.c b/xen/arch/x86/domctl.c
index a4effc3..efa8746 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 ( atomic_read(&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 ( atomic_read(&d->controller_pause_count) > 0 )
             break;
         ret = -EINVAL;
         if ( domctl->u.gdbsx_pauseunp_vcpu.vcpu >= MAX_VIRT_CPUS ||
@@ -1061,7 +1061,8 @@ 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 =
+            atomic_read(&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..f81e989 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;
+        _atomic_set(&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 *v = current;
+    struct domain *d = v->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 (v->arch.gdbsx_vcpu_event == 0)
         send_global_virq(VIRQ_DEBUGGER);
 }
 #endif
@@ -914,17 +909,29 @@ 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);
+    /*
+     * Limit the toolstack pause count to an arbitrary 255 to prevent the
+     * toolstack overflowing d->pause_count with many repeated hypercalls.
+     */
+    if ( !atomic_inc_bounded(&d->controller_pause_count, 256) )
+        return -EUSERS;
+
+    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);
+    if ( !atomic_dec_bounded(&d->controller_pause_count, -1) )
+        return -EINVAL;
+
+    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..7af1605 100644
--- a/xen/common/domctl.c
+++ b/xen/common/domctl.c
@@ -181,7 +181,8 @@ 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) |
+        (atomic_read(&d->controller_pause_count) > 0
+                                        ? XEN_DOMINF_paused   : 0) |
         (d->debugger_attached           ? XEN_DOMINF_debugged : 0) |
         d->shutdown_code << XEN_DOMINF_shutdownshift;
 
@@ -398,16 +399,14 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl)
         ret = -EINVAL;
         if ( d != current->domain )
         {
-            domain_pause_by_systemcontroller(d);
-            ret = 0;
+            ret = domain_pause_by_systemcontroller(d);
         }
     }
     break;
 
     case XEN_DOMCTL_unpausedomain:
     {
-        domain_unpause_by_systemcontroller(d);
-        ret = 0;
+        ret = domain_unpause_by_systemcontroller(d);
     }
     break;
 
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index a953469..41179a0 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;
+    atomic_t         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;
-- 
1.7.10.4

Re: [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes

[Jan Beulich]

>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/apic.c
> +++ b/xen/arch/x86/apic.c
> @@ -28,7 +28,7 @@
>  #include <xen/softirq.h>
>  #include <asm/mc146818rtc.h>
>  #include <asm/msr.h>
> -#include <asm/atomic.h>
> +#include <xen/atomic.h>

I appreciate you having taken the time to replace all these, but - do
we really need to do this replacement at this point? Especially arch
code should be fine using the asm/ header instad of the xen/ one.

Jan

Re: [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded()

[Jan Beulich]

>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
> These will increment/decremented an atomic_t, while ensuring that the 
> atomic_t
> doesn't hit a specific bound.
> 
> This involves introducing atomic_cmpxchg() for x86, which previously only
> existed on ARM.
> 
> Suggested-by: Don Slutz <dslutz@verizon.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Keir Fraser <keir@xen.org>
> CC: Jan Beulich <JBeulich@suse.com>

You should be getting used to Cc all "REST" maintainers on respective
changes.

> --- /dev/null
> +++ b/xen/common/atomic.c
> @@ -0,0 +1,35 @@
> +#include <xen/atomic.h>
> +
> +bool_t atomic_inc_bounded(atomic_t *v, int bound)
> +{
> +    int old, new, prev = atomic_read(v);
> +
> +    do
> +    {
> +        old = prev;
> +        new = old + 1;
> +        if ( new >= bound )
> +            return 0;
> +
> +        prev = atomic_cmpxchg(v, old, new);
> +    } while ( prev != old );
> +
> +    return 1;
> +}
> +
> +bool_t atomic_dec_bounded(atomic_t *v, int bound)
> +{
> +    int old, new, prev = atomic_read(v);
> +
> +    do
> +    {
> +        old = prev;
> +        new = old - 1;
> +        if ( new <= bound )
> +            return 0;
> +
> +        prev = atomic_cmpxchg(v, old, new);
> +    } while ( prev != old );
> +
> +    return 1;
> +}

Same question here: Do we really need these? There are various uses
of cmpxchg() in common code already, and in patch 3 you don't really
need the cmpxchg to happen on an atomic_t, so plain cmpxchg() with
the little bit of extra logic open coded would seem fine to me.

Jan

Re: [PATCH v2 1/3] xen/atomic: Introduce common atomic header and update includes

[Andrew Cooper]

On 02/07/14 15:01, Jan Beulich wrote:
>>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/apic.c
>> +++ b/xen/arch/x86/apic.c
>> @@ -28,7 +28,7 @@
>>  #include <xen/softirq.h>
>>  #include <asm/mc146818rtc.h>
>>  #include <asm/msr.h>
>> -#include <asm/atomic.h>
>> +#include <xen/atomic.h>
> I appreciate you having taken the time to replace all these, but - do
> we really need to do this replacement at this point? Especially arch
> code should be fine using the asm/ header instad of the xen/ one.
>
> Jan
>

Not strictly, but I figured it was cleaner to update everything at once
than to have a mix of common and arch specific includes.

(Also it didn't take long at all.  `git grep ... | xargs sed` is a
remakably quick refactoring tool, and gcc will quickly tell you if it
went wrong)

~Andrew

Re: [PATCH v2 2/3] xen/atomic: Implement atomic_{inc, dec}_bounded()

[Andrew Cooper]

On 02/07/14 15:05, Jan Beulich wrote:
>>>> On 02.07.14 at 15:47, <andrew.cooper3@citrix.com> wrote:
>> These will increment/decremented an atomic_t, while ensuring that the 
>> atomic_t
>> doesn't hit a specific bound.
>>
>> This involves introducing atomic_cmpxchg() for x86, which previously only
>> existed on ARM.
>>
>> Suggested-by: Don Slutz <dslutz@verizon.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> CC: Keir Fraser <keir@xen.org>
>> CC: Jan Beulich <JBeulich@suse.com>
> You should be getting used to Cc all "REST" maintainers on respective
> changes.

Oops yes - I will see about updating my auto-cc list hooks.

>
>> --- /dev/null
>> +++ b/xen/common/atomic.c
>> @@ -0,0 +1,35 @@
>> +#include <xen/atomic.h>
>> +
>> +bool_t atomic_inc_bounded(atomic_t *v, int bound)
>> +{
>> +    int old, new, prev = atomic_read(v);
>> +
>> +    do
>> +    {
>> +        old = prev;
>> +        new = old + 1;
>> +        if ( new >= bound )
>> +            return 0;
>> +
>> +        prev = atomic_cmpxchg(v, old, new);
>> +    } while ( prev != old );
>> +
>> +    return 1;
>> +}
>> +
>> +bool_t atomic_dec_bounded(atomic_t *v, int bound)
>> +{
>> +    int old, new, prev = atomic_read(v);
>> +
>> +    do
>> +    {
>> +        old = prev;
>> +        new = old - 1;
>> +        if ( new <= bound )
>> +            return 0;
>> +
>> +        prev = atomic_cmpxchg(v, old, new);
>> +    } while ( prev != old );
>> +
>> +    return 1;
>> +}
> Same question here: Do we really need these? There are various uses
> of cmpxchg() in common code already, and in patch 3 you don't really
> need the cmpxchg to happen on an atomic_t, so plain cmpxchg() with
> the little bit of extra logic open coded would seem fine to me.
>
> Jan
>

These as in atomic_{inc,dec}_bounded() ?

They logically fit alongside atomic_{inc,dec}_and_test(), and I came to
the conclusion that they would likely be useful elsewhere.

I realise that these are not fantastic reasons alone...

~Andrew