All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state
@ 2016-07-06 21:14 Sergey Fedorov
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 01/11] atomic: introduce atomic_dec_fetch Sergey Fedorov
                   ` (11 more replies)
  0 siblings, 12 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov

From: Sergey Fedorov <serge.fdrv@gmail.com>

Hi,

This is a v2 of the RFC series [3] that was a follow-up for a discussion
on the subject [1]. This is sill marked as FRC becuse it laks bsd-user
support.

Basically, this series suggests a way to perform some operations in
quiescent state. The goal is to implement such a mechanism which can be
used for safe translation buffer flush in multi-threaded user-mode
emulation (and later in MTTCG) and merge it into mainline as soon as
possible.

This series is a more elaborated version of the original series since
there was some positive feedback and I'd appreciate some comments from
maintainers this time. 

The first two patches are some useful tweak from Alex's MTTCG trees.

The patches 3 through 8 are arrangements for the patch 9 which adds
support for CPU work in linux-user. This wouldn't make any sense without
the patch 10 which is the subject matter of this series. Although there
is nothing special to do in case of single-threaded round-robin CPU loop
of current system-mode emulation to ensure quiescent state, that is
shown in the patch 10, how it would look like in MTTCG. The last patch
actually employs this new mechanism making translation buffer flush
thread safe.

The considerations on expensiveness of work item dynamic allocation [2]
was not taken into account. I'll just mention here that the desired
effect can be achieved by either using dynamic arrays for CPU work
queues or making queue_work_on_cpu() from the patch 3 a public interface
thus allowing to use preallocated work items.

I would like your comments in order to produce something upstreamable
quickly!

This series is available at a public git repository:

    https://github.com/sergefdrv/qemu.git safe-cpu-work-v2

Summary of changes in v2:
 - atomic_dec_fetch() used to decrement 'safe_work_pending'
 - more work to use/fix passing CPUState to run_on_cpu helpers
 - instead of wrapping conditional variables access, use QemuMutex and QemuCond
   in linux-user and just wrap getting of the relevant mutex.
 - document new public API
 - Rename 'tcg_pending_cpus' to 'tcg_pending_threads'

Kind regards,
Sergey

[1] http://thread.gmane.org/gmane.comp.emulators.qemu/417599
[2] http://thread.gmane.org/gmane.comp.emulators.qemu/407030/focus=407039
[3] http://thread.gmane.org/gmane.comp.emulators.qemu/420363

Alex Bennée (2):
  atomic: introduce atomic_dec_fetch.
  cpus: pass CPUState to run_on_cpu helpers

Sergey Fedorov (9):
  cpus: Move common code out of {async_,}run_on_cpu()
  cpus: Wrap mutex used to protect CPU work
  cpus: Rename flush_queued_work()
  linux-user: Use QemuMutex and QemuCond
  linux-user: Rework exclusive operation mechanism
  linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick()
  linux-user: Support CPU work queue
  cpu-exec-common: Introduce async_safe_run_on_cpu()
  tcg: Make tb_flush() thread safe

 cpu-exec-common.c          | 132 +++++++++++++++++++++++++++++++++++++++++++++
 cpus.c                     | 106 +++++++-----------------------------
 hw/i386/kvm/apic.c         |   3 +-
 hw/i386/kvmvapic.c         |   6 +--
 hw/ppc/ppce500_spin.c      |  31 ++++-------
 hw/ppc/spapr.c             |   6 +--
 hw/ppc/spapr_hcall.c       |  17 +++---
 include/exec/exec-all.h    |  31 +++++++++++
 include/qemu/atomic.h      |   4 ++
 include/qom/cpu.h          |  22 ++++++--
 kvm-all.c                  |  21 +++-----
 linux-user/main.c          |  94 ++++++++++++++++++++------------
 target-i386/helper.c       |  19 +++----
 target-i386/kvm.c          |   6 +--
 target-s390x/cpu.c         |   4 +-
 target-s390x/cpu.h         |   7 +--
 target-s390x/kvm.c         |  98 ++++++++++++++++-----------------
 target-s390x/misc_helper.c |   4 +-
 translate-all.c            |  13 +++--
 19 files changed, 371 insertions(+), 253 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 01/11] atomic: introduce atomic_dec_fetch.
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
@ 2016-07-06 21:14 ` Sergey Fedorov
  2016-07-06 21:14   ` [Qemu-devel] " Sergey Fedorov
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov

From: Alex Bennée <alex.bennee@linaro.org>

Useful for counting down.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 include/qemu/atomic.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 7a590969b59f..8ac9ca7f457a 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -162,6 +162,8 @@
 #define atomic_fetch_and(ptr, n) __atomic_fetch_and(ptr, n, __ATOMIC_SEQ_CST)
 #define atomic_fetch_or(ptr, n)  __atomic_fetch_or(ptr, n, __ATOMIC_SEQ_CST)
 
+#define atomic_dec_fetch(ptr)  __atomic_sub_fetch(ptr, 1, __ATOMIC_SEQ_CST)
+
 /* And even shorter names that return void.  */
 #define atomic_inc(ptr)    ((void) __atomic_fetch_add(ptr, 1, __ATOMIC_SEQ_CST))
 #define atomic_dec(ptr)    ((void) __atomic_fetch_sub(ptr, 1, __ATOMIC_SEQ_CST))
@@ -357,6 +359,8 @@
 #define atomic_fetch_or        __sync_fetch_and_or
 #define atomic_cmpxchg         __sync_val_compare_and_swap
 
+#define atomic_dec_fetch(ptr)  __sync_sub_and_fetch(ptr, 1)
+
 /* And even shorter names that return void.  */
 #define atomic_inc(ptr)        ((void) __sync_fetch_and_add(ptr, 1))
 #define atomic_dec(ptr)        ((void) __sync_fetch_and_add(ptr, -1))
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
@ 2016-07-06 21:14   ` Sergey Fedorov
  2016-07-06 21:14   ` [Qemu-devel] " Sergey Fedorov
                     ` (10 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov,
	Peter Crosthwaite, Eduardo Habkost, Michael S. Tsirkin,
	David Gibson, Alexander Graf, Marcelo Tosatti,
	Christian Borntraeger, Cornelia Huck, qemu-ppc, kvm

From: Alex Bennée <alex.bennee@linaro.org>

CPUState is a fairly common pointer to pass to these helpers. This means
if you need other arguments for the async_run_on_cpu case you end up
having to do a g_malloc to stuff additional data into the routine. For
the current users this isn't a massive deal but for MTTCG this gets
cumbersome when the only other parameter is often an address.

This adds the typedef run_on_cpu_func for helper functions which has an
explicit CPUState * passed as the first parameter. All the users of
run_on_cpu and async_run_on_cpu have had their helpers updated to use
CPUState where available.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
[Sergey Fedorov:
 - eliminate more CPUState in user data;
 - remove unnecessary user data passing;
 - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 - eliminate more CPUState in user data
 - remove unnecessary user data passing
 - fix target-s390x/kvm.c and target-s390x/misc_helper.c

---
 cpus.c                     | 15 ++++---
 hw/i386/kvm/apic.c         |  3 +-
 hw/i386/kvmvapic.c         |  6 +--
 hw/ppc/ppce500_spin.c      | 31 +++++----------
 hw/ppc/spapr.c             |  6 +--
 hw/ppc/spapr_hcall.c       | 17 ++++----
 include/qom/cpu.h          |  8 ++--
 kvm-all.c                  | 21 ++++------
 target-i386/helper.c       | 19 ++++-----
 target-i386/kvm.c          |  6 +--
 target-s390x/cpu.c         |  4 +-
 target-s390x/cpu.h         |  7 +---
 target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
 target-s390x/misc_helper.c |  4 +-
 14 files changed, 108 insertions(+), 137 deletions(-)

diff --git a/cpus.c b/cpus.c
index 84c3520d446f..049c2d04e150 100644
--- a/cpus.c
+++ b/cpus.c
@@ -551,9 +551,8 @@ static const VMStateDescription vmstate_timers = {
     }
 };
 
-static void cpu_throttle_thread(void *opaque)
+static void cpu_throttle_thread(CPUState *cpu, void *opaque)
 {
-    CPUState *cpu = opaque;
     double pct;
     double throttle_ratio;
     long sleeptime_ns;
@@ -583,7 +582,7 @@ static void cpu_throttle_timer_tick(void *opaque)
     }
     CPU_FOREACH(cpu) {
         if (!atomic_xchg(&cpu->throttle_thread_scheduled, 1)) {
-            async_run_on_cpu(cpu, cpu_throttle_thread, cpu);
+            async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
         }
     }
 
@@ -911,12 +910,12 @@ void qemu_init_cpu_loop(void)
     qemu_thread_get_self(&io_thread);
 }
 
-void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
+void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
 {
     struct qemu_work_item wi;
 
     if (qemu_cpu_is_self(cpu)) {
-        func(data);
+        func(cpu, data);
         return;
     }
 
@@ -944,12 +943,12 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
     }
 }
 
-void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
+void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
 {
     struct qemu_work_item *wi;
 
     if (qemu_cpu_is_self(cpu)) {
-        func(data);
+        func(cpu, data);
         return;
     }
 
@@ -1000,7 +999,7 @@ static void flush_queued_work(CPUState *cpu)
             cpu->queued_work_last = NULL;
         }
         qemu_mutex_unlock(&cpu->work_mutex);
-        wi->func(wi->data);
+        wi->func(cpu, wi->data);
         qemu_mutex_lock(&cpu->work_mutex);
         if (wi->free) {
             g_free(wi);
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index c5983c79be47..9b66e741d4b4 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -125,10 +125,9 @@ static void kvm_apic_vapic_base_update(APICCommonState *s)
     }
 }
 
-static void do_inject_external_nmi(void *data)
+static void do_inject_external_nmi(CPUState *cpu, void *data)
 {
     APICCommonState *s = data;
-    CPUState *cpu = CPU(s->cpu);
     uint32_t lvt;
     int ret;
 
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 3bf1ddd97612..1bc02fb2f1a1 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -483,7 +483,7 @@ typedef struct VAPICEnableTPRReporting {
     bool enable;
 } VAPICEnableTPRReporting;
 
-static void vapic_do_enable_tpr_reporting(void *data)
+static void vapic_do_enable_tpr_reporting(CPUState *cpu, void *data)
 {
     VAPICEnableTPRReporting *info = data;
 
@@ -734,10 +734,10 @@ static void vapic_realize(DeviceState *dev, Error **errp)
     nb_option_roms++;
 }
 
-static void do_vapic_enable(void *data)
+static void do_vapic_enable(CPUState *cs, void *data)
 {
     VAPICROMState *s = data;
-    X86CPU *cpu = X86_CPU(first_cpu);
+    X86CPU *cpu = X86_CPU(cs);
 
     static const uint8_t enabled = 1;
     cpu_physical_memory_write(s->vapic_paddr + offsetof(VAPICState, enabled),
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index 22c584eb8dd0..8e16f651ea95 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -54,11 +54,6 @@ typedef struct SpinState {
     SpinInfo spin[MAX_CPUS];
 } SpinState;
 
-typedef struct spin_kick {
-    PowerPCCPU *cpu;
-    SpinInfo *spin;
-} SpinKick;
-
 static void spin_reset(void *opaque)
 {
     SpinState *s = opaque;
@@ -89,16 +84,15 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
     env->tlb_dirty = true;
 }
 
-static void spin_kick(void *data)
+static void spin_kick(CPUState *cs, void *data)
 {
-    SpinKick *kick = data;
-    CPUState *cpu = CPU(kick->cpu);
-    CPUPPCState *env = &kick->cpu->env;
-    SpinInfo *curspin = kick->spin;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    SpinInfo *curspin = data;
     hwaddr map_size = 64 * 1024 * 1024;
     hwaddr map_start;
 
-    cpu_synchronize_state(cpu);
+    cpu_synchronize_state(cs);
     stl_p(&curspin->pir, env->spr[SPR_BOOKE_PIR]);
     env->nip = ldq_p(&curspin->addr) & (map_size - 1);
     env->gpr[3] = ldq_p(&curspin->r3);
@@ -112,10 +106,10 @@ static void spin_kick(void *data)
     map_start = ldq_p(&curspin->addr) & ~(map_size - 1);
     mmubooke_create_initial_mapping(env, 0, map_start, map_size);
 
-    cpu->halted = 0;
-    cpu->exception_index = -1;
-    cpu->stopped = false;
-    qemu_cpu_kick(cpu);
+    cs->halted = 0;
+    cs->exception_index = -1;
+    cs->stopped = false;
+    qemu_cpu_kick(cs);
 }
 
 static void spin_write(void *opaque, hwaddr addr, uint64_t value,
@@ -153,12 +147,7 @@ static void spin_write(void *opaque, hwaddr addr, uint64_t value,
 
     if (!(ldq_p(&curspin->addr) & 1)) {
         /* run CPU */
-        SpinKick kick = {
-            .cpu = POWERPC_CPU(cpu),
-            .spin = curspin,
-        };
-
-        run_on_cpu(cpu, spin_kick, &kick);
+        run_on_cpu(cpu, spin_kick, curspin);
     }
 }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7f33a1b2b57d..882a3c621367 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2170,10 +2170,8 @@ static void spapr_machine_finalizefn(Object *obj)
     g_free(spapr->kvm_type);
 }
 
-static void ppc_cpu_do_nmi_on_cpu(void *arg)
+static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg)
 {
-    CPUState *cs = arg;
-
     cpu_synchronize_state(cs);
     ppc_cpu_do_system_reset(cs);
 }
@@ -2183,7 +2181,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
     CPUState *cs;
 
     CPU_FOREACH(cs) {
-        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
+        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, NULL);
     }
 }
 
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 73af112e1d36..e5eca67f5b71 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -13,19 +13,18 @@
 #include "kvm_ppc.h"
 
 struct SPRSyncState {
-    CPUState *cs;
     int spr;
     target_ulong value;
     target_ulong mask;
 };
 
-static void do_spr_sync(void *arg)
+static void do_spr_sync(CPUState *cs, void *arg)
 {
     struct SPRSyncState *s = arg;
-    PowerPCCPU *cpu = POWERPC_CPU(s->cs);
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
 
-    cpu_synchronize_state(s->cs);
+    cpu_synchronize_state(cs);
     env->spr[s->spr] &= ~s->mask;
     env->spr[s->spr] |= s->value;
 }
@@ -34,7 +33,6 @@ static void set_spr(CPUState *cs, int spr, target_ulong value,
                     target_ulong mask)
 {
     struct SPRSyncState s = {
-        .cs = cs,
         .spr = spr,
         .value = value,
         .mask = mask
@@ -907,17 +905,17 @@ static target_ulong cas_get_option_vector(int vector, target_ulong table)
 }
 
 typedef struct {
-    PowerPCCPU *cpu;
     uint32_t cpu_version;
     Error *err;
 } SetCompatState;
 
-static void do_set_compat(void *arg)
+static void do_set_compat(CPUState *cs, void *arg)
 {
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
     SetCompatState *s = arg;
 
-    cpu_synchronize_state(CPU(s->cpu));
-    ppc_set_compat(s->cpu, s->cpu_version, &s->err);
+    cpu_synchronize_state(cs);
+    ppc_set_compat(cpu, s->cpu_version, &s->err);
 }
 
 #define get_compat_level(cpuver) ( \
@@ -1013,7 +1011,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
     if (old_cpu_version != cpu_version) {
         CPU_FOREACH(cs) {
             SetCompatState s = {
-                .cpu = POWERPC_CPU(cs),
                 .cpu_version = cpu_version,
                 .err = NULL,
             };
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 32f3af3e1c63..4e688f645b4a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -223,9 +223,11 @@ struct kvm_run;
 #define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
 
 /* work queue */
+typedef void (*run_on_cpu_func)(CPUState *cpu, void *data);
+
 struct qemu_work_item {
     struct qemu_work_item *next;
-    void (*func)(void *data);
+    run_on_cpu_func func;
     void *data;
     int done;
     bool free;
@@ -610,7 +612,7 @@ bool cpu_is_stopped(CPUState *cpu);
  *
  * Schedules the function @func for execution on the vCPU @cpu.
  */
-void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
+void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
 
 /**
  * async_run_on_cpu:
@@ -620,7 +622,7 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
  *
  * Schedules the function @func for execution on the vCPU @cpu asynchronously.
  */
-void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
+void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
 
 /**
  * qemu_get_cpu:
diff --git a/kvm-all.c b/kvm-all.c
index a88f917fda69..a8320574cc8a 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1828,10 +1828,8 @@ void kvm_flush_coalesced_mmio_buffer(void)
     s->coalesced_flush_in_progress = false;
 }
 
-static void do_kvm_cpu_synchronize_state(void *arg)
+static void do_kvm_cpu_synchronize_state(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     if (!cpu->kvm_vcpu_dirty) {
         kvm_arch_get_registers(cpu);
         cpu->kvm_vcpu_dirty = true;
@@ -1841,34 +1839,30 @@ static void do_kvm_cpu_synchronize_state(void *arg)
 void kvm_cpu_synchronize_state(CPUState *cpu)
 {
     if (!cpu->kvm_vcpu_dirty) {
-        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, cpu);
+        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, NULL);
     }
 }
 
-static void do_kvm_cpu_synchronize_post_reset(void *arg)
+static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
     cpu->kvm_vcpu_dirty = false;
 }
 
 void kvm_cpu_synchronize_post_reset(CPUState *cpu)
 {
-    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, cpu);
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, NULL);
 }
 
-static void do_kvm_cpu_synchronize_post_init(void *arg)
+static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
     cpu->kvm_vcpu_dirty = false;
 }
 
 void kvm_cpu_synchronize_post_init(CPUState *cpu)
 {
-    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu);
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, NULL);
 }
 
 int kvm_cpu_exec(CPUState *cpu)
@@ -2210,7 +2204,7 @@ struct kvm_set_guest_debug_data {
     int err;
 };
 
-static void kvm_invoke_set_guest_debug(void *data)
+static void kvm_invoke_set_guest_debug(CPUState *unused_cpu, void *data)
 {
     struct kvm_set_guest_debug_data *dbg_data = data;
 
@@ -2228,7 +2222,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
         data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
     }
     kvm_arch_update_guest_debug(cpu, &data.dbg);
-    data.cpu = cpu;
 
     run_on_cpu(cpu, kvm_invoke_set_guest_debug, &data);
     return data.err;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 1c250b82452d..9bc961bff342 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1113,7 +1113,6 @@ out:
 
 typedef struct MCEInjectionParams {
     Monitor *mon;
-    X86CPU *cpu;
     int bank;
     uint64_t status;
     uint64_t mcg_status;
@@ -1122,14 +1121,14 @@ typedef struct MCEInjectionParams {
     int flags;
 } MCEInjectionParams;
 
-static void do_inject_x86_mce(void *data)
+static void do_inject_x86_mce(CPUState *cs, void *data)
 {
     MCEInjectionParams *params = data;
-    CPUX86State *cenv = &params->cpu->env;
-    CPUState *cpu = CPU(params->cpu);
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *cenv = &cpu->env;
     uint64_t *banks = cenv->mce_banks + 4 * params->bank;
 
-    cpu_synchronize_state(cpu);
+    cpu_synchronize_state(cs);
 
     /*
      * If there is an MCE exception being processed, ignore this SRAO MCE
@@ -1149,7 +1148,7 @@ static void do_inject_x86_mce(void *data)
         if ((cenv->mcg_cap & MCG_CTL_P) && cenv->mcg_ctl != ~(uint64_t)0) {
             monitor_printf(params->mon,
                            "CPU %d: Uncorrected error reporting disabled\n",
-                           cpu->cpu_index);
+                           cs->cpu_index);
             return;
         }
 
@@ -1161,7 +1160,7 @@ static void do_inject_x86_mce(void *data)
             monitor_printf(params->mon,
                            "CPU %d: Uncorrected error reporting disabled for"
                            " bank %d\n",
-                           cpu->cpu_index, params->bank);
+                           cs->cpu_index, params->bank);
             return;
         }
 
@@ -1170,7 +1169,7 @@ static void do_inject_x86_mce(void *data)
             monitor_printf(params->mon,
                            "CPU %d: Previous MCE still in progress, raising"
                            " triple fault\n",
-                           cpu->cpu_index);
+                           cs->cpu_index);
             qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
             qemu_system_reset_request();
             return;
@@ -1182,7 +1181,7 @@ static void do_inject_x86_mce(void *data)
         banks[3] = params->misc;
         cenv->mcg_status = params->mcg_status;
         banks[1] = params->status;
-        cpu_interrupt(cpu, CPU_INTERRUPT_MCE);
+        cpu_interrupt(cs, CPU_INTERRUPT_MCE);
     } else if (!(banks[1] & MCI_STATUS_VAL)
                || !(banks[1] & MCI_STATUS_UC)) {
         if (banks[1] & MCI_STATUS_VAL) {
@@ -1204,7 +1203,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
     CPUX86State *cenv = &cpu->env;
     MCEInjectionParams params = {
         .mon = mon,
-        .cpu = cpu,
         .bank = bank,
         .status = status,
         .mcg_status = mcg_status,
@@ -1245,7 +1243,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
             if (other_cs == cs) {
                 continue;
             }
-            params.cpu = X86_CPU(other_cs);
             run_on_cpu(other_cs, do_inject_x86_mce, &params);
         }
     }
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f3698f19b53b..a1c6e656d13c 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -151,10 +151,8 @@ static int kvm_get_tsc(CPUState *cs)
     return 0;
 }
 
-static inline void do_kvm_synchronize_tsc(void *arg)
+static inline void do_kvm_synchronize_tsc(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     kvm_get_tsc(cpu);
 }
 
@@ -164,7 +162,7 @@ void kvm_synchronize_all_tsc(void)
 
     if (kvm_enabled()) {
         CPU_FOREACH(cpu) {
-            run_on_cpu(cpu, do_kvm_synchronize_tsc, cpu);
+            run_on_cpu(cpu, do_kvm_synchronize_tsc, NULL);
         }
     }
 }
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index e43e2d61550b..4f09c647b7a8 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -188,7 +188,7 @@ static void s390_cpu_machine_reset_cb(void *opaque)
 {
     S390CPU *cpu = opaque;
 
-    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, CPU(cpu));
+    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, NULL);
 }
 #endif
 
@@ -238,7 +238,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
 #if !defined(CONFIG_USER_ONLY)
-    run_on_cpu(cs, s390_do_cpu_full_reset, cs);
+    run_on_cpu(cs, s390_do_cpu_full_reset, NULL);
 #else
     cpu_reset(cs);
 #endif
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 8bcb0f75f399..8998ce21ca13 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -498,17 +498,14 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
 #define decode_basedisp_rs decode_basedisp_s
 
 /* helper functions for run_on_cpu() */
-static inline void s390_do_cpu_reset(void *arg)
+static inline void s390_do_cpu_reset(CPUState *cs, void *arg)
 {
-    CPUState *cs = arg;
     S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
 
     scc->cpu_reset(cs);
 }
-static inline void s390_do_cpu_full_reset(void *arg)
+static inline void s390_do_cpu_full_reset(CPUState *cs, void *arg)
 {
-    CPUState *cs = arg;
-
     cpu_reset(cs);
 }
 
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 45e94ca48abd..b37eb9d3cd8c 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -1354,7 +1354,6 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
 }
 
 typedef struct SigpInfo {
-    S390CPU *cpu;
     uint64_t param;
     int cc;
     uint64_t *status_reg;
@@ -1367,38 +1366,40 @@ static void set_sigp_status(SigpInfo *si, uint64_t status)
     si->cc = SIGP_CC_STATUS_STORED;
 }
 
-static void sigp_start(void *arg)
+static void sigp_start(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
 
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
         return;
     }
 
-    s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
+    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_stop(void *arg)
+static void sigp_stop(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     struct kvm_s390_irq irq = {
         .type = KVM_S390_SIGP_STOP,
     };
 
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_OPERATING) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
         si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
         return;
     }
 
     /* disabled wait - sleeping in user space */
-    if (CPU(si->cpu)->halted) {
-        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
+    if (cs->halted) {
+        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
     } else {
         /* execute the stop function */
-        si->cpu->env.sigp_order = SIGP_STOP;
-        kvm_s390_vcpu_interrupt(si->cpu, &irq);
+        cpu->env.sigp_order = SIGP_STOP;
+        kvm_s390_vcpu_interrupt(cpu, &irq);
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
@@ -1465,56 +1466,58 @@ static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
     return 0;
 }
 
-static void sigp_stop_and_store_status(void *arg)
+static void sigp_stop_and_store_status(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     struct kvm_s390_irq irq = {
         .type = KVM_S390_SIGP_STOP,
     };
 
     /* disabled wait - sleeping in user space */
-    if (s390_cpu_get_state(si->cpu) == CPU_STATE_OPERATING &&
-        CPU(si->cpu)->halted) {
-        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
+    if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
+        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
     }
 
-    switch (s390_cpu_get_state(si->cpu)) {
+    switch (s390_cpu_get_state(cpu)) {
     case CPU_STATE_OPERATING:
-        si->cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
-        kvm_s390_vcpu_interrupt(si->cpu, &irq);
+        cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
+        kvm_s390_vcpu_interrupt(cpu, &irq);
         /* store will be performed when handling the stop intercept */
         break;
     case CPU_STATE_STOPPED:
         /* already stopped, just store the status */
-        cpu_synchronize_state(CPU(si->cpu));
-        kvm_s390_store_status(si->cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
+        cpu_synchronize_state(cs);
+        kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
         break;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_store_status_at_address(void *arg)
+static void sigp_store_status_at_address(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     uint32_t address = si->param & 0x7ffffe00u;
 
     /* cpu has to be stopped */
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
         return;
     }
 
-    cpu_synchronize_state(CPU(si->cpu));
+    cpu_synchronize_state(cs);
 
-    if (kvm_s390_store_status(si->cpu, address, false)) {
+    if (kvm_s390_store_status(cpu, address, false)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_store_adtl_status(void *arg)
+static void sigp_store_adtl_status(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
 
     if (!kvm_check_extension(kvm_state, KVM_CAP_S390_VECTOR_REGISTERS)) {
@@ -1523,7 +1526,7 @@ static void sigp_store_adtl_status(void *arg)
     }
 
     /* cpu has to be stopped */
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
         return;
     }
@@ -1534,31 +1537,32 @@ static void sigp_store_adtl_status(void *arg)
         return;
     }
 
-    cpu_synchronize_state(CPU(si->cpu));
+    cpu_synchronize_state(cs);
 
-    if (kvm_s390_store_adtl_status(si->cpu, si->param)) {
+    if (kvm_s390_store_adtl_status(cpu, si->param)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_restart(void *arg)
+static void sigp_restart(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     struct kvm_s390_irq irq = {
         .type = KVM_S390_RESTART,
     };
 
-    switch (s390_cpu_get_state(si->cpu)) {
+    switch (s390_cpu_get_state(cpu)) {
     case CPU_STATE_STOPPED:
         /* the restart irq has to be delivered prior to any other pending irq */
-        cpu_synchronize_state(CPU(si->cpu));
-        do_restart_interrupt(&si->cpu->env);
-        s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
+        cpu_synchronize_state(cs);
+        do_restart_interrupt(&cpu->env);
+        s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
         break;
     case CPU_STATE_OPERATING:
-        kvm_s390_vcpu_interrupt(si->cpu, &irq);
+        kvm_s390_vcpu_interrupt(cpu, &irq);
         break;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
@@ -1566,20 +1570,18 @@ static void sigp_restart(void *arg)
 
 int kvm_s390_cpu_restart(S390CPU *cpu)
 {
-    SigpInfo si = {
-        .cpu = cpu,
-    };
+    SigpInfo si = {};
 
     run_on_cpu(CPU(cpu), sigp_restart, &si);
     DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
     return 0;
 }
 
-static void sigp_initial_cpu_reset(void *arg)
+static void sigp_initial_cpu_reset(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
     SigpInfo *si = arg;
-    CPUState *cs = CPU(si->cpu);
-    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
 
     cpu_synchronize_state(cs);
     scc->initial_cpu_reset(cs);
@@ -1587,11 +1589,11 @@ static void sigp_initial_cpu_reset(void *arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_cpu_reset(void *arg)
+static void sigp_cpu_reset(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
     SigpInfo *si = arg;
-    CPUState *cs = CPU(si->cpu);
-    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
 
     cpu_synchronize_state(cs);
     scc->cpu_reset(cs);
@@ -1599,12 +1601,13 @@ static void sigp_cpu_reset(void *arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_set_prefix(void *arg)
+static void sigp_set_prefix(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     uint32_t addr = si->param & 0x7fffe000u;
 
-    cpu_synchronize_state(CPU(si->cpu));
+    cpu_synchronize_state(cs);
 
     if (!address_space_access_valid(&address_space_memory, addr,
                                     sizeof(struct LowCore), false)) {
@@ -1613,13 +1616,13 @@ static void sigp_set_prefix(void *arg)
     }
 
     /* cpu has to be stopped */
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
         return;
     }
 
-    si->cpu->env.psa = addr;
-    cpu_synchronize_post_init(CPU(si->cpu));
+    cpu->env.psa = addr;
+    cpu_synchronize_post_init(cs);
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
@@ -1627,7 +1630,6 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
                                   uint64_t param, uint64_t *status_reg)
 {
     SigpInfo si = {
-        .cpu = dst_cpu,
         .param = param,
         .status_reg = status_reg,
     };
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 86da1947b984..4df2ec6c7dab 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -126,7 +126,7 @@ static int modified_clear_reset(S390CPU *cpu)
     pause_all_vcpus();
     cpu_synchronize_all_states();
     CPU_FOREACH(t) {
-        run_on_cpu(t, s390_do_cpu_full_reset, t);
+        run_on_cpu(t, s390_do_cpu_full_reset, NULL);
     }
     s390_cmma_reset();
     subsystem_reset();
@@ -145,7 +145,7 @@ static int load_normal_reset(S390CPU *cpu)
     pause_all_vcpus();
     cpu_synchronize_all_states();
     CPU_FOREACH(t) {
-        run_on_cpu(t, s390_do_cpu_reset, t);
+        run_on_cpu(t, s390_do_cpu_reset, NULL);
     }
     s390_cmma_reset();
     subsystem_reset();
-- 
1.9.1


^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
@ 2016-07-06 21:14   ` Sergey Fedorov
  0 siblings, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov,
	Peter Crosthwaite, Eduardo Habkost, Michael S. Tsirkin,
	David Gibson, Alexander Graf, Marcelo Tosatti,
	Christian Borntraeger, Cornelia Huck, qemu-ppc, kvm

From: Alex Bennée <alex.bennee@linaro.org>

CPUState is a fairly common pointer to pass to these helpers. This means
if you need other arguments for the async_run_on_cpu case you end up
having to do a g_malloc to stuff additional data into the routine. For
the current users this isn't a massive deal but for MTTCG this gets
cumbersome when the only other parameter is often an address.

This adds the typedef run_on_cpu_func for helper functions which has an
explicit CPUState * passed as the first parameter. All the users of
run_on_cpu and async_run_on_cpu have had their helpers updated to use
CPUState where available.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
[Sergey Fedorov:
 - eliminate more CPUState in user data;
 - remove unnecessary user data passing;
 - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 - eliminate more CPUState in user data
 - remove unnecessary user data passing
 - fix target-s390x/kvm.c and target-s390x/misc_helper.c

---
 cpus.c                     | 15 ++++---
 hw/i386/kvm/apic.c         |  3 +-
 hw/i386/kvmvapic.c         |  6 +--
 hw/ppc/ppce500_spin.c      | 31 +++++----------
 hw/ppc/spapr.c             |  6 +--
 hw/ppc/spapr_hcall.c       | 17 ++++----
 include/qom/cpu.h          |  8 ++--
 kvm-all.c                  | 21 ++++------
 target-i386/helper.c       | 19 ++++-----
 target-i386/kvm.c          |  6 +--
 target-s390x/cpu.c         |  4 +-
 target-s390x/cpu.h         |  7 +---
 target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
 target-s390x/misc_helper.c |  4 +-
 14 files changed, 108 insertions(+), 137 deletions(-)

diff --git a/cpus.c b/cpus.c
index 84c3520d446f..049c2d04e150 100644
--- a/cpus.c
+++ b/cpus.c
@@ -551,9 +551,8 @@ static const VMStateDescription vmstate_timers = {
     }
 };
 
-static void cpu_throttle_thread(void *opaque)
+static void cpu_throttle_thread(CPUState *cpu, void *opaque)
 {
-    CPUState *cpu = opaque;
     double pct;
     double throttle_ratio;
     long sleeptime_ns;
@@ -583,7 +582,7 @@ static void cpu_throttle_timer_tick(void *opaque)
     }
     CPU_FOREACH(cpu) {
         if (!atomic_xchg(&cpu->throttle_thread_scheduled, 1)) {
-            async_run_on_cpu(cpu, cpu_throttle_thread, cpu);
+            async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
         }
     }
 
@@ -911,12 +910,12 @@ void qemu_init_cpu_loop(void)
     qemu_thread_get_self(&io_thread);
 }
 
-void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
+void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
 {
     struct qemu_work_item wi;
 
     if (qemu_cpu_is_self(cpu)) {
-        func(data);
+        func(cpu, data);
         return;
     }
 
@@ -944,12 +943,12 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
     }
 }
 
-void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
+void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
 {
     struct qemu_work_item *wi;
 
     if (qemu_cpu_is_self(cpu)) {
-        func(data);
+        func(cpu, data);
         return;
     }
 
@@ -1000,7 +999,7 @@ static void flush_queued_work(CPUState *cpu)
             cpu->queued_work_last = NULL;
         }
         qemu_mutex_unlock(&cpu->work_mutex);
-        wi->func(wi->data);
+        wi->func(cpu, wi->data);
         qemu_mutex_lock(&cpu->work_mutex);
         if (wi->free) {
             g_free(wi);
diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
index c5983c79be47..9b66e741d4b4 100644
--- a/hw/i386/kvm/apic.c
+++ b/hw/i386/kvm/apic.c
@@ -125,10 +125,9 @@ static void kvm_apic_vapic_base_update(APICCommonState *s)
     }
 }
 
-static void do_inject_external_nmi(void *data)
+static void do_inject_external_nmi(CPUState *cpu, void *data)
 {
     APICCommonState *s = data;
-    CPUState *cpu = CPU(s->cpu);
     uint32_t lvt;
     int ret;
 
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index 3bf1ddd97612..1bc02fb2f1a1 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -483,7 +483,7 @@ typedef struct VAPICEnableTPRReporting {
     bool enable;
 } VAPICEnableTPRReporting;
 
-static void vapic_do_enable_tpr_reporting(void *data)
+static void vapic_do_enable_tpr_reporting(CPUState *cpu, void *data)
 {
     VAPICEnableTPRReporting *info = data;
 
@@ -734,10 +734,10 @@ static void vapic_realize(DeviceState *dev, Error **errp)
     nb_option_roms++;
 }
 
-static void do_vapic_enable(void *data)
+static void do_vapic_enable(CPUState *cs, void *data)
 {
     VAPICROMState *s = data;
-    X86CPU *cpu = X86_CPU(first_cpu);
+    X86CPU *cpu = X86_CPU(cs);
 
     static const uint8_t enabled = 1;
     cpu_physical_memory_write(s->vapic_paddr + offsetof(VAPICState, enabled),
diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
index 22c584eb8dd0..8e16f651ea95 100644
--- a/hw/ppc/ppce500_spin.c
+++ b/hw/ppc/ppce500_spin.c
@@ -54,11 +54,6 @@ typedef struct SpinState {
     SpinInfo spin[MAX_CPUS];
 } SpinState;
 
-typedef struct spin_kick {
-    PowerPCCPU *cpu;
-    SpinInfo *spin;
-} SpinKick;
-
 static void spin_reset(void *opaque)
 {
     SpinState *s = opaque;
@@ -89,16 +84,15 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
     env->tlb_dirty = true;
 }
 
-static void spin_kick(void *data)
+static void spin_kick(CPUState *cs, void *data)
 {
-    SpinKick *kick = data;
-    CPUState *cpu = CPU(kick->cpu);
-    CPUPPCState *env = &kick->cpu->env;
-    SpinInfo *curspin = kick->spin;
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    SpinInfo *curspin = data;
     hwaddr map_size = 64 * 1024 * 1024;
     hwaddr map_start;
 
-    cpu_synchronize_state(cpu);
+    cpu_synchronize_state(cs);
     stl_p(&curspin->pir, env->spr[SPR_BOOKE_PIR]);
     env->nip = ldq_p(&curspin->addr) & (map_size - 1);
     env->gpr[3] = ldq_p(&curspin->r3);
@@ -112,10 +106,10 @@ static void spin_kick(void *data)
     map_start = ldq_p(&curspin->addr) & ~(map_size - 1);
     mmubooke_create_initial_mapping(env, 0, map_start, map_size);
 
-    cpu->halted = 0;
-    cpu->exception_index = -1;
-    cpu->stopped = false;
-    qemu_cpu_kick(cpu);
+    cs->halted = 0;
+    cs->exception_index = -1;
+    cs->stopped = false;
+    qemu_cpu_kick(cs);
 }
 
 static void spin_write(void *opaque, hwaddr addr, uint64_t value,
@@ -153,12 +147,7 @@ static void spin_write(void *opaque, hwaddr addr, uint64_t value,
 
     if (!(ldq_p(&curspin->addr) & 1)) {
         /* run CPU */
-        SpinKick kick = {
-            .cpu = POWERPC_CPU(cpu),
-            .spin = curspin,
-        };
-
-        run_on_cpu(cpu, spin_kick, &kick);
+        run_on_cpu(cpu, spin_kick, curspin);
     }
 }
 
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 7f33a1b2b57d..882a3c621367 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2170,10 +2170,8 @@ static void spapr_machine_finalizefn(Object *obj)
     g_free(spapr->kvm_type);
 }
 
-static void ppc_cpu_do_nmi_on_cpu(void *arg)
+static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg)
 {
-    CPUState *cs = arg;
-
     cpu_synchronize_state(cs);
     ppc_cpu_do_system_reset(cs);
 }
@@ -2183,7 +2181,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
     CPUState *cs;
 
     CPU_FOREACH(cs) {
-        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
+        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, NULL);
     }
 }
 
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 73af112e1d36..e5eca67f5b71 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -13,19 +13,18 @@
 #include "kvm_ppc.h"
 
 struct SPRSyncState {
-    CPUState *cs;
     int spr;
     target_ulong value;
     target_ulong mask;
 };
 
-static void do_spr_sync(void *arg)
+static void do_spr_sync(CPUState *cs, void *arg)
 {
     struct SPRSyncState *s = arg;
-    PowerPCCPU *cpu = POWERPC_CPU(s->cs);
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
 
-    cpu_synchronize_state(s->cs);
+    cpu_synchronize_state(cs);
     env->spr[s->spr] &= ~s->mask;
     env->spr[s->spr] |= s->value;
 }
@@ -34,7 +33,6 @@ static void set_spr(CPUState *cs, int spr, target_ulong value,
                     target_ulong mask)
 {
     struct SPRSyncState s = {
-        .cs = cs,
         .spr = spr,
         .value = value,
         .mask = mask
@@ -907,17 +905,17 @@ static target_ulong cas_get_option_vector(int vector, target_ulong table)
 }
 
 typedef struct {
-    PowerPCCPU *cpu;
     uint32_t cpu_version;
     Error *err;
 } SetCompatState;
 
-static void do_set_compat(void *arg)
+static void do_set_compat(CPUState *cs, void *arg)
 {
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
     SetCompatState *s = arg;
 
-    cpu_synchronize_state(CPU(s->cpu));
-    ppc_set_compat(s->cpu, s->cpu_version, &s->err);
+    cpu_synchronize_state(cs);
+    ppc_set_compat(cpu, s->cpu_version, &s->err);
 }
 
 #define get_compat_level(cpuver) ( \
@@ -1013,7 +1011,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
     if (old_cpu_version != cpu_version) {
         CPU_FOREACH(cs) {
             SetCompatState s = {
-                .cpu = POWERPC_CPU(cs),
                 .cpu_version = cpu_version,
                 .err = NULL,
             };
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 32f3af3e1c63..4e688f645b4a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -223,9 +223,11 @@ struct kvm_run;
 #define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
 
 /* work queue */
+typedef void (*run_on_cpu_func)(CPUState *cpu, void *data);
+
 struct qemu_work_item {
     struct qemu_work_item *next;
-    void (*func)(void *data);
+    run_on_cpu_func func;
     void *data;
     int done;
     bool free;
@@ -610,7 +612,7 @@ bool cpu_is_stopped(CPUState *cpu);
  *
  * Schedules the function @func for execution on the vCPU @cpu.
  */
-void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
+void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
 
 /**
  * async_run_on_cpu:
@@ -620,7 +622,7 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
  *
  * Schedules the function @func for execution on the vCPU @cpu asynchronously.
  */
-void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
+void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
 
 /**
  * qemu_get_cpu:
diff --git a/kvm-all.c b/kvm-all.c
index a88f917fda69..a8320574cc8a 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1828,10 +1828,8 @@ void kvm_flush_coalesced_mmio_buffer(void)
     s->coalesced_flush_in_progress = false;
 }
 
-static void do_kvm_cpu_synchronize_state(void *arg)
+static void do_kvm_cpu_synchronize_state(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     if (!cpu->kvm_vcpu_dirty) {
         kvm_arch_get_registers(cpu);
         cpu->kvm_vcpu_dirty = true;
@@ -1841,34 +1839,30 @@ static void do_kvm_cpu_synchronize_state(void *arg)
 void kvm_cpu_synchronize_state(CPUState *cpu)
 {
     if (!cpu->kvm_vcpu_dirty) {
-        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, cpu);
+        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, NULL);
     }
 }
 
-static void do_kvm_cpu_synchronize_post_reset(void *arg)
+static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
     cpu->kvm_vcpu_dirty = false;
 }
 
 void kvm_cpu_synchronize_post_reset(CPUState *cpu)
 {
-    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, cpu);
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, NULL);
 }
 
-static void do_kvm_cpu_synchronize_post_init(void *arg)
+static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
     cpu->kvm_vcpu_dirty = false;
 }
 
 void kvm_cpu_synchronize_post_init(CPUState *cpu)
 {
-    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu);
+    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, NULL);
 }
 
 int kvm_cpu_exec(CPUState *cpu)
@@ -2210,7 +2204,7 @@ struct kvm_set_guest_debug_data {
     int err;
 };
 
-static void kvm_invoke_set_guest_debug(void *data)
+static void kvm_invoke_set_guest_debug(CPUState *unused_cpu, void *data)
 {
     struct kvm_set_guest_debug_data *dbg_data = data;
 
@@ -2228,7 +2222,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
         data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
     }
     kvm_arch_update_guest_debug(cpu, &data.dbg);
-    data.cpu = cpu;
 
     run_on_cpu(cpu, kvm_invoke_set_guest_debug, &data);
     return data.err;
diff --git a/target-i386/helper.c b/target-i386/helper.c
index 1c250b82452d..9bc961bff342 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -1113,7 +1113,6 @@ out:
 
 typedef struct MCEInjectionParams {
     Monitor *mon;
-    X86CPU *cpu;
     int bank;
     uint64_t status;
     uint64_t mcg_status;
@@ -1122,14 +1121,14 @@ typedef struct MCEInjectionParams {
     int flags;
 } MCEInjectionParams;
 
-static void do_inject_x86_mce(void *data)
+static void do_inject_x86_mce(CPUState *cs, void *data)
 {
     MCEInjectionParams *params = data;
-    CPUX86State *cenv = &params->cpu->env;
-    CPUState *cpu = CPU(params->cpu);
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *cenv = &cpu->env;
     uint64_t *banks = cenv->mce_banks + 4 * params->bank;
 
-    cpu_synchronize_state(cpu);
+    cpu_synchronize_state(cs);
 
     /*
      * If there is an MCE exception being processed, ignore this SRAO MCE
@@ -1149,7 +1148,7 @@ static void do_inject_x86_mce(void *data)
         if ((cenv->mcg_cap & MCG_CTL_P) && cenv->mcg_ctl != ~(uint64_t)0) {
             monitor_printf(params->mon,
                            "CPU %d: Uncorrected error reporting disabled\n",
-                           cpu->cpu_index);
+                           cs->cpu_index);
             return;
         }
 
@@ -1161,7 +1160,7 @@ static void do_inject_x86_mce(void *data)
             monitor_printf(params->mon,
                            "CPU %d: Uncorrected error reporting disabled for"
                            " bank %d\n",
-                           cpu->cpu_index, params->bank);
+                           cs->cpu_index, params->bank);
             return;
         }
 
@@ -1170,7 +1169,7 @@ static void do_inject_x86_mce(void *data)
             monitor_printf(params->mon,
                            "CPU %d: Previous MCE still in progress, raising"
                            " triple fault\n",
-                           cpu->cpu_index);
+                           cs->cpu_index);
             qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
             qemu_system_reset_request();
             return;
@@ -1182,7 +1181,7 @@ static void do_inject_x86_mce(void *data)
         banks[3] = params->misc;
         cenv->mcg_status = params->mcg_status;
         banks[1] = params->status;
-        cpu_interrupt(cpu, CPU_INTERRUPT_MCE);
+        cpu_interrupt(cs, CPU_INTERRUPT_MCE);
     } else if (!(banks[1] & MCI_STATUS_VAL)
                || !(banks[1] & MCI_STATUS_UC)) {
         if (banks[1] & MCI_STATUS_VAL) {
@@ -1204,7 +1203,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
     CPUX86State *cenv = &cpu->env;
     MCEInjectionParams params = {
         .mon = mon,
-        .cpu = cpu,
         .bank = bank,
         .status = status,
         .mcg_status = mcg_status,
@@ -1245,7 +1243,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
             if (other_cs == cs) {
                 continue;
             }
-            params.cpu = X86_CPU(other_cs);
             run_on_cpu(other_cs, do_inject_x86_mce, &params);
         }
     }
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f3698f19b53b..a1c6e656d13c 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -151,10 +151,8 @@ static int kvm_get_tsc(CPUState *cs)
     return 0;
 }
 
-static inline void do_kvm_synchronize_tsc(void *arg)
+static inline void do_kvm_synchronize_tsc(CPUState *cpu, void *arg)
 {
-    CPUState *cpu = arg;
-
     kvm_get_tsc(cpu);
 }
 
@@ -164,7 +162,7 @@ void kvm_synchronize_all_tsc(void)
 
     if (kvm_enabled()) {
         CPU_FOREACH(cpu) {
-            run_on_cpu(cpu, do_kvm_synchronize_tsc, cpu);
+            run_on_cpu(cpu, do_kvm_synchronize_tsc, NULL);
         }
     }
 }
diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
index e43e2d61550b..4f09c647b7a8 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -188,7 +188,7 @@ static void s390_cpu_machine_reset_cb(void *opaque)
 {
     S390CPU *cpu = opaque;
 
-    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, CPU(cpu));
+    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, NULL);
 }
 #endif
 
@@ -238,7 +238,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
     s390_cpu_gdb_init(cs);
     qemu_init_vcpu(cs);
 #if !defined(CONFIG_USER_ONLY)
-    run_on_cpu(cs, s390_do_cpu_full_reset, cs);
+    run_on_cpu(cs, s390_do_cpu_full_reset, NULL);
 #else
     cpu_reset(cs);
 #endif
diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
index 8bcb0f75f399..8998ce21ca13 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -498,17 +498,14 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
 #define decode_basedisp_rs decode_basedisp_s
 
 /* helper functions for run_on_cpu() */
-static inline void s390_do_cpu_reset(void *arg)
+static inline void s390_do_cpu_reset(CPUState *cs, void *arg)
 {
-    CPUState *cs = arg;
     S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
 
     scc->cpu_reset(cs);
 }
-static inline void s390_do_cpu_full_reset(void *arg)
+static inline void s390_do_cpu_full_reset(CPUState *cs, void *arg)
 {
-    CPUState *cs = arg;
-
     cpu_reset(cs);
 }
 
diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
index 45e94ca48abd..b37eb9d3cd8c 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -1354,7 +1354,6 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
 }
 
 typedef struct SigpInfo {
-    S390CPU *cpu;
     uint64_t param;
     int cc;
     uint64_t *status_reg;
@@ -1367,38 +1366,40 @@ static void set_sigp_status(SigpInfo *si, uint64_t status)
     si->cc = SIGP_CC_STATUS_STORED;
 }
 
-static void sigp_start(void *arg)
+static void sigp_start(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
 
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
         return;
     }
 
-    s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
+    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_stop(void *arg)
+static void sigp_stop(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     struct kvm_s390_irq irq = {
         .type = KVM_S390_SIGP_STOP,
     };
 
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_OPERATING) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
         si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
         return;
     }
 
     /* disabled wait - sleeping in user space */
-    if (CPU(si->cpu)->halted) {
-        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
+    if (cs->halted) {
+        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
     } else {
         /* execute the stop function */
-        si->cpu->env.sigp_order = SIGP_STOP;
-        kvm_s390_vcpu_interrupt(si->cpu, &irq);
+        cpu->env.sigp_order = SIGP_STOP;
+        kvm_s390_vcpu_interrupt(cpu, &irq);
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
@@ -1465,56 +1466,58 @@ static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
     return 0;
 }
 
-static void sigp_stop_and_store_status(void *arg)
+static void sigp_stop_and_store_status(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     struct kvm_s390_irq irq = {
         .type = KVM_S390_SIGP_STOP,
     };
 
     /* disabled wait - sleeping in user space */
-    if (s390_cpu_get_state(si->cpu) == CPU_STATE_OPERATING &&
-        CPU(si->cpu)->halted) {
-        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
+    if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
+        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
     }
 
-    switch (s390_cpu_get_state(si->cpu)) {
+    switch (s390_cpu_get_state(cpu)) {
     case CPU_STATE_OPERATING:
-        si->cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
-        kvm_s390_vcpu_interrupt(si->cpu, &irq);
+        cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
+        kvm_s390_vcpu_interrupt(cpu, &irq);
         /* store will be performed when handling the stop intercept */
         break;
     case CPU_STATE_STOPPED:
         /* already stopped, just store the status */
-        cpu_synchronize_state(CPU(si->cpu));
-        kvm_s390_store_status(si->cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
+        cpu_synchronize_state(cs);
+        kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
         break;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_store_status_at_address(void *arg)
+static void sigp_store_status_at_address(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     uint32_t address = si->param & 0x7ffffe00u;
 
     /* cpu has to be stopped */
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
         return;
     }
 
-    cpu_synchronize_state(CPU(si->cpu));
+    cpu_synchronize_state(cs);
 
-    if (kvm_s390_store_status(si->cpu, address, false)) {
+    if (kvm_s390_store_status(cpu, address, false)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_store_adtl_status(void *arg)
+static void sigp_store_adtl_status(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
 
     if (!kvm_check_extension(kvm_state, KVM_CAP_S390_VECTOR_REGISTERS)) {
@@ -1523,7 +1526,7 @@ static void sigp_store_adtl_status(void *arg)
     }
 
     /* cpu has to be stopped */
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
         return;
     }
@@ -1534,31 +1537,32 @@ static void sigp_store_adtl_status(void *arg)
         return;
     }
 
-    cpu_synchronize_state(CPU(si->cpu));
+    cpu_synchronize_state(cs);
 
-    if (kvm_s390_store_adtl_status(si->cpu, si->param)) {
+    if (kvm_s390_store_adtl_status(cpu, si->param)) {
         set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
         return;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_restart(void *arg)
+static void sigp_restart(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     struct kvm_s390_irq irq = {
         .type = KVM_S390_RESTART,
     };
 
-    switch (s390_cpu_get_state(si->cpu)) {
+    switch (s390_cpu_get_state(cpu)) {
     case CPU_STATE_STOPPED:
         /* the restart irq has to be delivered prior to any other pending irq */
-        cpu_synchronize_state(CPU(si->cpu));
-        do_restart_interrupt(&si->cpu->env);
-        s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
+        cpu_synchronize_state(cs);
+        do_restart_interrupt(&cpu->env);
+        s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
         break;
     case CPU_STATE_OPERATING:
-        kvm_s390_vcpu_interrupt(si->cpu, &irq);
+        kvm_s390_vcpu_interrupt(cpu, &irq);
         break;
     }
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
@@ -1566,20 +1570,18 @@ static void sigp_restart(void *arg)
 
 int kvm_s390_cpu_restart(S390CPU *cpu)
 {
-    SigpInfo si = {
-        .cpu = cpu,
-    };
+    SigpInfo si = {};
 
     run_on_cpu(CPU(cpu), sigp_restart, &si);
     DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
     return 0;
 }
 
-static void sigp_initial_cpu_reset(void *arg)
+static void sigp_initial_cpu_reset(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
     SigpInfo *si = arg;
-    CPUState *cs = CPU(si->cpu);
-    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
 
     cpu_synchronize_state(cs);
     scc->initial_cpu_reset(cs);
@@ -1587,11 +1589,11 @@ static void sigp_initial_cpu_reset(void *arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_cpu_reset(void *arg)
+static void sigp_cpu_reset(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
+    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
     SigpInfo *si = arg;
-    CPUState *cs = CPU(si->cpu);
-    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
 
     cpu_synchronize_state(cs);
     scc->cpu_reset(cs);
@@ -1599,12 +1601,13 @@ static void sigp_cpu_reset(void *arg)
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
-static void sigp_set_prefix(void *arg)
+static void sigp_set_prefix(CPUState *cs, void *arg)
 {
+    S390CPU *cpu = S390_CPU(cs);
     SigpInfo *si = arg;
     uint32_t addr = si->param & 0x7fffe000u;
 
-    cpu_synchronize_state(CPU(si->cpu));
+    cpu_synchronize_state(cs);
 
     if (!address_space_access_valid(&address_space_memory, addr,
                                     sizeof(struct LowCore), false)) {
@@ -1613,13 +1616,13 @@ static void sigp_set_prefix(void *arg)
     }
 
     /* cpu has to be stopped */
-    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
+    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
         set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
         return;
     }
 
-    si->cpu->env.psa = addr;
-    cpu_synchronize_post_init(CPU(si->cpu));
+    cpu->env.psa = addr;
+    cpu_synchronize_post_init(cs);
     si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
 }
 
@@ -1627,7 +1630,6 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
                                   uint64_t param, uint64_t *status_reg)
 {
     SigpInfo si = {
-        .cpu = dst_cpu,
         .param = param,
         .status_reg = status_reg,
     };
diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
index 86da1947b984..4df2ec6c7dab 100644
--- a/target-s390x/misc_helper.c
+++ b/target-s390x/misc_helper.c
@@ -126,7 +126,7 @@ static int modified_clear_reset(S390CPU *cpu)
     pause_all_vcpus();
     cpu_synchronize_all_states();
     CPU_FOREACH(t) {
-        run_on_cpu(t, s390_do_cpu_full_reset, t);
+        run_on_cpu(t, s390_do_cpu_full_reset, NULL);
     }
     s390_cmma_reset();
     subsystem_reset();
@@ -145,7 +145,7 @@ static int load_normal_reset(S390CPU *cpu)
     pause_all_vcpus();
     cpu_synchronize_all_states();
     CPU_FOREACH(t) {
-        run_on_cpu(t, s390_do_cpu_reset, t);
+        run_on_cpu(t, s390_do_cpu_reset, NULL);
     }
     s390_cmma_reset();
     subsystem_reset();
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 03/11] cpus: Move common code out of {async_, }run_on_cpu()
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 01/11] atomic: introduce atomic_dec_fetch Sergey Fedorov
  2016-07-06 21:14   ` [Qemu-devel] " Sergey Fedorov
@ 2016-07-06 21:14 ` Sergey Fedorov
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 04/11] cpus: Wrap mutex used to protect CPU work Sergey Fedorov
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Peter Crosthwaite

From: Sergey Fedorov <serge.fdrv@gmail.com>

Move the code common between run_on_cpu() and async_run_on_cpu() into a
new function queue_work_on_cpu().

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
---
 cpus.c | 42 ++++++++++++++++++------------------------
 1 file changed, 18 insertions(+), 24 deletions(-)

diff --git a/cpus.c b/cpus.c
index 049c2d04e150..04687c85bcd4 100644
--- a/cpus.c
+++ b/cpus.c
@@ -910,6 +910,22 @@ void qemu_init_cpu_loop(void)
     qemu_thread_get_self(&io_thread);
 }
 
+static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
+{
+    qemu_mutex_lock(&cpu->work_mutex);
+    if (cpu->queued_work_first == NULL) {
+        cpu->queued_work_first = wi;
+    } else {
+        cpu->queued_work_last->next = wi;
+    }
+    cpu->queued_work_last = wi;
+    wi->next = NULL;
+    wi->done = false;
+    qemu_mutex_unlock(&cpu->work_mutex);
+
+    qemu_cpu_kick(cpu);
+}
+
 void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
 {
     struct qemu_work_item wi;
@@ -923,18 +939,7 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
     wi.data = data;
     wi.free = false;
 
-    qemu_mutex_lock(&cpu->work_mutex);
-    if (cpu->queued_work_first == NULL) {
-        cpu->queued_work_first = &wi;
-    } else {
-        cpu->queued_work_last->next = &wi;
-    }
-    cpu->queued_work_last = &wi;
-    wi.next = NULL;
-    wi.done = false;
-    qemu_mutex_unlock(&cpu->work_mutex);
-
-    qemu_cpu_kick(cpu);
+    queue_work_on_cpu(cpu, &wi);
     while (!atomic_mb_read(&wi.done)) {
         CPUState *self_cpu = current_cpu;
 
@@ -957,18 +962,7 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
     wi->data = data;
     wi->free = true;
 
-    qemu_mutex_lock(&cpu->work_mutex);
-    if (cpu->queued_work_first == NULL) {
-        cpu->queued_work_first = wi;
-    } else {
-        cpu->queued_work_last->next = wi;
-    }
-    cpu->queued_work_last = wi;
-    wi->next = NULL;
-    wi->done = false;
-    qemu_mutex_unlock(&cpu->work_mutex);
-
-    qemu_cpu_kick(cpu);
+    queue_work_on_cpu(cpu, wi);
 }
 
 static void qemu_kvm_destroy_vcpu(CPUState *cpu)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 04/11] cpus: Wrap mutex used to protect CPU work
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (2 preceding siblings ...)
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 03/11] cpus: Move common code out of {async_, }run_on_cpu() Sergey Fedorov
@ 2016-07-06 21:14 ` Sergey Fedorov
  2016-07-11 12:06   ` Alex Bennée
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 05/11] cpus: Rename flush_queued_work() Sergey Fedorov
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Peter Crosthwaite

From: Sergey Fedorov <serge.fdrv@gmail.com>

This will be useful to enable CPU work on user mode emulation.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 cpus.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/cpus.c b/cpus.c
index 04687c85bcd4..f80ed2aeefdd 100644
--- a/cpus.c
+++ b/cpus.c
@@ -910,6 +910,11 @@ void qemu_init_cpu_loop(void)
     qemu_thread_get_self(&io_thread);
 }
 
+static QemuMutex *qemu_get_cpu_work_mutex(void)
+{
+    return &qemu_global_mutex;
+}
+
 static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
 {
     qemu_mutex_lock(&cpu->work_mutex);
@@ -943,7 +948,7 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
     while (!atomic_mb_read(&wi.done)) {
         CPUState *self_cpu = current_cpu;
 
-        qemu_cond_wait(&qemu_work_cond, &qemu_global_mutex);
+        qemu_cond_wait(&qemu_work_cond, qemu_get_cpu_work_mutex());
         current_cpu = self_cpu;
     }
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 05/11] cpus: Rename flush_queued_work()
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (3 preceding siblings ...)
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 04/11] cpus: Wrap mutex used to protect CPU work Sergey Fedorov
@ 2016-07-06 21:14 ` Sergey Fedorov
  2016-07-11 12:07   ` Alex Bennée
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 06/11] linux-user: Use QemuMutex and QemuCond Sergey Fedorov
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Peter Crosthwaite

From: Sergey Fedorov <serge.fdrv@gmail.com>

To avoid possible confusion, rename flush_queued_work() to
process_queued_cpu_work().

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 cpus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cpus.c b/cpus.c
index f80ed2aeefdd..51fd8c18b4c8 100644
--- a/cpus.c
+++ b/cpus.c
@@ -982,7 +982,7 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu)
 {
 }
 
-static void flush_queued_work(CPUState *cpu)
+static void process_queued_cpu_work(CPUState *cpu)
 {
     struct qemu_work_item *wi;
 
@@ -1017,7 +1017,7 @@ static void qemu_wait_io_event_common(CPUState *cpu)
         cpu->stopped = true;
         qemu_cond_broadcast(&qemu_pause_cond);
     }
-    flush_queued_work(cpu);
+    process_queued_cpu_work(cpu);
     cpu->thread_kicked = false;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 06/11] linux-user: Use QemuMutex and QemuCond
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (4 preceding siblings ...)
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 05/11] cpus: Rename flush_queued_work() Sergey Fedorov
@ 2016-07-06 21:14 ` Sergey Fedorov
  2016-07-11 12:08   ` Alex Bennée
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 07/11] linux-user: Rework exclusive operation mechanism Sergey Fedorov
                   ` (5 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Riku Voipio

From: Sergey Fedorov <serge.fdrv@gmail.com>

Convert pthread_mutex_t and pthread_cond_t to QemuMutex and QemuCond.
This will allow to make some locks and conditional variables common
between user and system mode emulation.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 linux-user/main.c | 53 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index 617a179f14a4..bdbda693cc5f 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -108,17 +108,25 @@ int cpu_get_pic_interrupt(CPUX86State *env)
    We don't require a full sync, only that no cpus are executing guest code.
    The alternative is to map target atomic ops onto host equivalents,
    which requires quite a lot of per host/target work.  */
-static pthread_mutex_t cpu_list_mutex = PTHREAD_MUTEX_INITIALIZER;
-static pthread_mutex_t exclusive_lock = PTHREAD_MUTEX_INITIALIZER;
-static pthread_cond_t exclusive_cond = PTHREAD_COND_INITIALIZER;
-static pthread_cond_t exclusive_resume = PTHREAD_COND_INITIALIZER;
+static QemuMutex cpu_list_mutex;
+static QemuMutex exclusive_lock;
+static QemuCond exclusive_cond;
+static QemuCond exclusive_resume;
 static int pending_cpus;
 
+void qemu_init_cpu_loop(void)
+{
+    qemu_mutex_init(&cpu_list_mutex);
+    qemu_mutex_init(&exclusive_lock);
+    qemu_cond_init(&exclusive_cond);
+    qemu_cond_init(&exclusive_resume);
+}
+
 /* Make sure everything is in a consistent state for calling fork().  */
 void fork_start(void)
 {
     qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
-    pthread_mutex_lock(&exclusive_lock);
+    qemu_mutex_lock(&exclusive_lock);
     mmap_fork_start();
 }
 
@@ -135,14 +143,14 @@ void fork_end(int child)
             }
         }
         pending_cpus = 0;
-        pthread_mutex_init(&exclusive_lock, NULL);
-        pthread_mutex_init(&cpu_list_mutex, NULL);
-        pthread_cond_init(&exclusive_cond, NULL);
-        pthread_cond_init(&exclusive_resume, NULL);
+        qemu_mutex_init(&exclusive_lock);
+        qemu_mutex_init(&cpu_list_mutex);
+        qemu_cond_init(&exclusive_cond);
+        qemu_cond_init(&exclusive_resume);
         qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
         gdbserver_fork(thread_cpu);
     } else {
-        pthread_mutex_unlock(&exclusive_lock);
+        qemu_mutex_unlock(&exclusive_lock);
         qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
     }
 }
@@ -152,7 +160,7 @@ void fork_end(int child)
 static inline void exclusive_idle(void)
 {
     while (pending_cpus) {
-        pthread_cond_wait(&exclusive_resume, &exclusive_lock);
+        qemu_cond_wait(&exclusive_resume, &exclusive_lock);
     }
 }
 
@@ -162,7 +170,7 @@ static inline void start_exclusive(void)
 {
     CPUState *other_cpu;
 
-    pthread_mutex_lock(&exclusive_lock);
+    qemu_mutex_lock(&exclusive_lock);
     exclusive_idle();
 
     pending_cpus = 1;
@@ -174,7 +182,7 @@ static inline void start_exclusive(void)
         }
     }
     if (pending_cpus > 1) {
-        pthread_cond_wait(&exclusive_cond, &exclusive_lock);
+        qemu_cond_wait(&exclusive_cond, &exclusive_lock);
     }
 }
 
@@ -182,42 +190,42 @@ static inline void start_exclusive(void)
 static inline void __attribute__((unused)) end_exclusive(void)
 {
     pending_cpus = 0;
-    pthread_cond_broadcast(&exclusive_resume);
-    pthread_mutex_unlock(&exclusive_lock);
+    qemu_cond_broadcast(&exclusive_resume);
+    qemu_mutex_unlock(&exclusive_lock);
 }
 
 /* Wait for exclusive ops to finish, and begin cpu execution.  */
 static inline void cpu_exec_start(CPUState *cpu)
 {
-    pthread_mutex_lock(&exclusive_lock);
+    qemu_mutex_lock(&exclusive_lock);
     exclusive_idle();
     cpu->running = true;
-    pthread_mutex_unlock(&exclusive_lock);
+    qemu_mutex_unlock(&exclusive_lock);
 }
 
 /* Mark cpu as not executing, and release pending exclusive ops.  */
 static inline void cpu_exec_end(CPUState *cpu)
 {
-    pthread_mutex_lock(&exclusive_lock);
+    qemu_mutex_lock(&exclusive_lock);
     cpu->running = false;
     if (pending_cpus > 1) {
         pending_cpus--;
         if (pending_cpus == 1) {
-            pthread_cond_signal(&exclusive_cond);
+            qemu_cond_signal(&exclusive_cond);
         }
     }
     exclusive_idle();
-    pthread_mutex_unlock(&exclusive_lock);
+    qemu_mutex_unlock(&exclusive_lock);
 }
 
 void cpu_list_lock(void)
 {
-    pthread_mutex_lock(&cpu_list_mutex);
+    qemu_mutex_lock(&cpu_list_mutex);
 }
 
 void cpu_list_unlock(void)
 {
-    pthread_mutex_unlock(&cpu_list_mutex);
+    qemu_mutex_unlock(&cpu_list_mutex);
 }
 
 
@@ -4210,6 +4218,7 @@ int main(int argc, char **argv, char **envp)
     int ret;
     int execfd;
 
+    qemu_init_cpu_loop();
     module_call_init(MODULE_INIT_QOM);
 
     if ((envlist = envlist_create()) == NULL) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 07/11] linux-user: Rework exclusive operation mechanism
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (5 preceding siblings ...)
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 06/11] linux-user: Use QemuMutex and QemuCond Sergey Fedorov
@ 2016-07-06 21:14 ` Sergey Fedorov
  2016-07-14 15:04   ` Alex Bennée
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 08/11] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Sergey Fedorov
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:14 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Riku Voipio

From: Sergey Fedorov <serge.fdrv@gmail.com>

A single variable 'pending_cpus' was used for both counting currently
running CPUs and for signalling the pending exclusive operation request.

To prepare for supporting operations which requires a quiescent state,
like translation buffer flush, it is useful to keep a counter of
currently running CPUs always up to date.

Use a separate variable 'tcg_pending_threads' to count for currently
running CPUs and a separate variable 'exclusive_pending' to indicate
that there's an exclusive operation pending.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 - Rename 'tcg_pending_cpus' to 'tcg_pending_threads'

---
 linux-user/main.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/linux-user/main.c b/linux-user/main.c
index bdbda693cc5f..5ff0b20bad89 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -112,7 +112,8 @@ static QemuMutex cpu_list_mutex;
 static QemuMutex exclusive_lock;
 static QemuCond exclusive_cond;
 static QemuCond exclusive_resume;
-static int pending_cpus;
+static bool exclusive_pending;
+static int tcg_pending_threads;
 
 void qemu_init_cpu_loop(void)
 {
@@ -142,7 +143,8 @@ void fork_end(int child)
                 QTAILQ_REMOVE(&cpus, cpu, node);
             }
         }
-        pending_cpus = 0;
+        tcg_pending_threads = 0;
+        exclusive_pending = false;
         qemu_mutex_init(&exclusive_lock);
         qemu_mutex_init(&cpu_list_mutex);
         qemu_cond_init(&exclusive_cond);
@@ -159,7 +161,7 @@ void fork_end(int child)
    must be held.  */
 static inline void exclusive_idle(void)
 {
-    while (pending_cpus) {
+    while (exclusive_pending) {
         qemu_cond_wait(&exclusive_resume, &exclusive_lock);
     }
 }
@@ -173,15 +175,14 @@ static inline void start_exclusive(void)
     qemu_mutex_lock(&exclusive_lock);
     exclusive_idle();
 
-    pending_cpus = 1;
+    exclusive_pending = true;
     /* Make all other cpus stop executing.  */
     CPU_FOREACH(other_cpu) {
         if (other_cpu->running) {
-            pending_cpus++;
             cpu_exit(other_cpu);
         }
     }
-    if (pending_cpus > 1) {
+    while (tcg_pending_threads) {
         qemu_cond_wait(&exclusive_cond, &exclusive_lock);
     }
 }
@@ -189,7 +190,7 @@ static inline void start_exclusive(void)
 /* Finish an exclusive operation.  */
 static inline void __attribute__((unused)) end_exclusive(void)
 {
-    pending_cpus = 0;
+    exclusive_pending = false;
     qemu_cond_broadcast(&exclusive_resume);
     qemu_mutex_unlock(&exclusive_lock);
 }
@@ -200,6 +201,7 @@ static inline void cpu_exec_start(CPUState *cpu)
     qemu_mutex_lock(&exclusive_lock);
     exclusive_idle();
     cpu->running = true;
+    tcg_pending_threads++;
     qemu_mutex_unlock(&exclusive_lock);
 }
 
@@ -208,11 +210,9 @@ static inline void cpu_exec_end(CPUState *cpu)
 {
     qemu_mutex_lock(&exclusive_lock);
     cpu->running = false;
-    if (pending_cpus > 1) {
-        pending_cpus--;
-        if (pending_cpus == 1) {
-            qemu_cond_signal(&exclusive_cond);
-        }
+    tcg_pending_threads--;
+    if (!tcg_pending_threads) {
+        qemu_cond_signal(&exclusive_cond);
     }
     exclusive_idle();
     qemu_mutex_unlock(&exclusive_lock);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 08/11] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick()
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (6 preceding siblings ...)
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 07/11] linux-user: Rework exclusive operation mechanism Sergey Fedorov
@ 2016-07-06 21:15 ` Sergey Fedorov
  2016-07-14 15:07   ` Alex Bennée
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 09/11] linux-user: Support CPU work queue Sergey Fedorov
                   ` (3 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Riku Voipio

From: Sergey Fedorov <serge.fdrv@gmail.com>

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 linux-user/main.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/linux-user/main.c b/linux-user/main.c
index 5ff0b20bad89..a8790ac63f68 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3785,6 +3785,16 @@ void cpu_loop(CPUTLGState *env)
 
 THREAD CPUState *thread_cpu;
 
+bool qemu_cpu_is_self(CPUState *cpu)
+{
+    return thread_cpu == cpu;
+}
+
+void qemu_cpu_kick(CPUState *cpu)
+{
+    cpu_exit(cpu);
+}
+
 void task_settid(TaskState *ts)
 {
     if (ts->ts_tid == 0) {
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 09/11] linux-user: Support CPU work queue
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (7 preceding siblings ...)
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 08/11] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Sergey Fedorov
@ 2016-07-06 21:15 ` Sergey Fedorov
  2016-07-14 15:10   ` Alex Bennée
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 10/11] cpu-exec-common: Introduce async_safe_run_on_cpu() Sergey Fedorov
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Peter Crosthwaite, Riku Voipio

From: Sergey Fedorov <serge.fdrv@gmail.com>

Make CPU work core functions common between system and user-mode
emulation. User-mode does not have BQL, so process_queued_cpu_work() is
protected by 'exclusive_lock'.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 - 'qemu_work_cond' definition moved to cpu-exec-common.c
 - documentation commend for new public API added

---
 cpu-exec-common.c       | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
 cpus.c                  | 86 +------------------------------------------------
 include/exec/exec-all.h | 17 ++++++++++
 linux-user/main.c       |  8 +++++
 4 files changed, 111 insertions(+), 85 deletions(-)

diff --git a/cpu-exec-common.c b/cpu-exec-common.c
index 0cb4ae60eff9..a233f0124559 100644
--- a/cpu-exec-common.c
+++ b/cpu-exec-common.c
@@ -77,3 +77,88 @@ void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
     }
     siglongjmp(cpu->jmp_env, 1);
 }
+
+QemuCond qemu_work_cond;
+
+static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
+{
+    qemu_mutex_lock(&cpu->work_mutex);
+    if (cpu->queued_work_first == NULL) {
+        cpu->queued_work_first = wi;
+    } else {
+        cpu->queued_work_last->next = wi;
+    }
+    cpu->queued_work_last = wi;
+    wi->next = NULL;
+    wi->done = false;
+    qemu_mutex_unlock(&cpu->work_mutex);
+
+    qemu_cpu_kick(cpu);
+}
+
+void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
+{
+    struct qemu_work_item wi;
+
+    if (qemu_cpu_is_self(cpu)) {
+        func(cpu, data);
+        return;
+    }
+
+    wi.func = func;
+    wi.data = data;
+    wi.free = false;
+
+    queue_work_on_cpu(cpu, &wi);
+    while (!atomic_mb_read(&wi.done)) {
+        CPUState *self_cpu = current_cpu;
+
+        qemu_cond_wait(&qemu_work_cond, qemu_get_cpu_work_mutex());
+        current_cpu = self_cpu;
+    }
+}
+
+void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
+{
+    struct qemu_work_item *wi;
+
+    if (qemu_cpu_is_self(cpu)) {
+        func(cpu, data);
+        return;
+    }
+
+    wi = g_malloc0(sizeof(struct qemu_work_item));
+    wi->func = func;
+    wi->data = data;
+    wi->free = true;
+
+    queue_work_on_cpu(cpu, wi);
+}
+
+void process_queued_cpu_work(CPUState *cpu)
+{
+    struct qemu_work_item *wi;
+
+    if (cpu->queued_work_first == NULL) {
+        return;
+    }
+
+    qemu_mutex_lock(&cpu->work_mutex);
+    while (cpu->queued_work_first != NULL) {
+        wi = cpu->queued_work_first;
+        cpu->queued_work_first = wi->next;
+        if (!cpu->queued_work_first) {
+            cpu->queued_work_last = NULL;
+        }
+        qemu_mutex_unlock(&cpu->work_mutex);
+        wi->func(cpu, wi->data);
+        qemu_mutex_lock(&cpu->work_mutex);
+        if (wi->free) {
+            g_free(wi);
+        } else {
+            atomic_mb_set(&wi->done, true);
+        }
+    }
+    qemu_mutex_unlock(&cpu->work_mutex);
+    qemu_cond_broadcast(&qemu_work_cond);
+}
diff --git a/cpus.c b/cpus.c
index 51fd8c18b4c8..282d7e399902 100644
--- a/cpus.c
+++ b/cpus.c
@@ -896,7 +896,6 @@ static QemuThread io_thread;
 static QemuCond qemu_cpu_cond;
 /* system init */
 static QemuCond qemu_pause_cond;
-static QemuCond qemu_work_cond;
 
 void qemu_init_cpu_loop(void)
 {
@@ -910,66 +909,11 @@ void qemu_init_cpu_loop(void)
     qemu_thread_get_self(&io_thread);
 }
 
-static QemuMutex *qemu_get_cpu_work_mutex(void)
+QemuMutex *qemu_get_cpu_work_mutex(void)
 {
     return &qemu_global_mutex;
 }
 
-static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
-{
-    qemu_mutex_lock(&cpu->work_mutex);
-    if (cpu->queued_work_first == NULL) {
-        cpu->queued_work_first = wi;
-    } else {
-        cpu->queued_work_last->next = wi;
-    }
-    cpu->queued_work_last = wi;
-    wi->next = NULL;
-    wi->done = false;
-    qemu_mutex_unlock(&cpu->work_mutex);
-
-    qemu_cpu_kick(cpu);
-}
-
-void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
-{
-    struct qemu_work_item wi;
-
-    if (qemu_cpu_is_self(cpu)) {
-        func(cpu, data);
-        return;
-    }
-
-    wi.func = func;
-    wi.data = data;
-    wi.free = false;
-
-    queue_work_on_cpu(cpu, &wi);
-    while (!atomic_mb_read(&wi.done)) {
-        CPUState *self_cpu = current_cpu;
-
-        qemu_cond_wait(&qemu_work_cond, qemu_get_cpu_work_mutex());
-        current_cpu = self_cpu;
-    }
-}
-
-void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
-{
-    struct qemu_work_item *wi;
-
-    if (qemu_cpu_is_self(cpu)) {
-        func(cpu, data);
-        return;
-    }
-
-    wi = g_malloc0(sizeof(struct qemu_work_item));
-    wi->func = func;
-    wi->data = data;
-    wi->free = true;
-
-    queue_work_on_cpu(cpu, wi);
-}
-
 static void qemu_kvm_destroy_vcpu(CPUState *cpu)
 {
     if (kvm_destroy_vcpu(cpu) < 0) {
@@ -982,34 +926,6 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu)
 {
 }
 
-static void process_queued_cpu_work(CPUState *cpu)
-{
-    struct qemu_work_item *wi;
-
-    if (cpu->queued_work_first == NULL) {
-        return;
-    }
-
-    qemu_mutex_lock(&cpu->work_mutex);
-    while (cpu->queued_work_first != NULL) {
-        wi = cpu->queued_work_first;
-        cpu->queued_work_first = wi->next;
-        if (!cpu->queued_work_first) {
-            cpu->queued_work_last = NULL;
-        }
-        qemu_mutex_unlock(&cpu->work_mutex);
-        wi->func(cpu, wi->data);
-        qemu_mutex_lock(&cpu->work_mutex);
-        if (wi->free) {
-            g_free(wi);
-        } else {
-            atomic_mb_set(&wi->done, true);
-        }
-    }
-    qemu_mutex_unlock(&cpu->work_mutex);
-    qemu_cond_broadcast(&qemu_work_cond);
-}
-
 static void qemu_wait_io_event_common(CPUState *cpu)
 {
     if (cpu->stop) {
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index c1f59fa59d2c..bceecbd5222a 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -407,4 +407,21 @@ extern int singlestep;
 extern CPUState *tcg_current_cpu;
 extern bool exit_request;
 
+/**
+ * qemu_work_cond - condition to wait for CPU work items completion
+ */
+extern QemuCond qemu_work_cond;
+
+/**
+ * qemu_get_cpu_work_mutex() - get the mutex which protects CPU work execution
+ *
+ * Return: A pointer to the mutex.
+ */
+QemuMutex *qemu_get_cpu_work_mutex(void);
+/**
+ * process_queued_cpu_work() - process all items on CPU work queue
+ * @cpu: The CPU which work queue to process.
+ */
+void process_queued_cpu_work(CPUState *cpu);
+
 #endif
diff --git a/linux-user/main.c b/linux-user/main.c
index a8790ac63f68..fce61d5a35fc 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -121,6 +121,7 @@ void qemu_init_cpu_loop(void)
     qemu_mutex_init(&exclusive_lock);
     qemu_cond_init(&exclusive_cond);
     qemu_cond_init(&exclusive_resume);
+    qemu_cond_init(&qemu_work_cond);
 }
 
 /* Make sure everything is in a consistent state for calling fork().  */
@@ -149,6 +150,7 @@ void fork_end(int child)
         qemu_mutex_init(&cpu_list_mutex);
         qemu_cond_init(&exclusive_cond);
         qemu_cond_init(&exclusive_resume);
+        qemu_cond_init(&qemu_work_cond);
         qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
         gdbserver_fork(thread_cpu);
     } else {
@@ -157,6 +159,11 @@ void fork_end(int child)
     }
 }
 
+QemuMutex *qemu_get_cpu_work_mutex(void)
+{
+    return &exclusive_lock;
+}
+
 /* Wait for pending exclusive operations to complete.  The exclusive lock
    must be held.  */
 static inline void exclusive_idle(void)
@@ -215,6 +222,7 @@ static inline void cpu_exec_end(CPUState *cpu)
         qemu_cond_signal(&exclusive_cond);
     }
     exclusive_idle();
+    process_queued_cpu_work(cpu);
     qemu_mutex_unlock(&exclusive_lock);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 10/11] cpu-exec-common: Introduce async_safe_run_on_cpu()
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (8 preceding siblings ...)
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 09/11] linux-user: Support CPU work queue Sergey Fedorov
@ 2016-07-06 21:15 ` Sergey Fedorov
  2016-07-14 15:57   ` Alex Bennée
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe Sergey Fedorov
  2016-07-14 16:00 ` [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Alex Bennée
  11 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Peter Crosthwaite, Riku Voipio

From: Sergey Fedorov <serge.fdrv@gmail.com>

This patch is based on the ideas found in work of KONRAD Frederic [1],
Alex Bennée [2], and Alvise Rigo [3].

This mechanism allows to perform an operation safely in a quiescent
state. Quiescent state means: (1) no vCPU is running and (2) BQL in
system-mode or 'exclusive_lock' in user-mode emulation is held while
performing the operation. This functionality is required e.g. for
performing translation buffer flush safely in multi-threaded user-mode
emulation.

The existing CPU work queue is used to schedule such safe operations. A
new 'safe' flag is added into struct qemu_work_item to designate the
special requirements of the safe work. An operation in a quiescent sate
can be scheduled by using async_safe_run_on_cpu() function which is
actually the same as sync_run_on_cpu() except that it marks the queued
work item with the 'safe' flag set to true. Given this flag set
queue_work_on_cpu() atomically increments 'safe_work_pending' global
counter and kicks all the CPUs instead of just the target CPU as in case
of normal CPU work. This allows to force other CPUs to exit their
execution loops and wait in wait_safe_cpu_work() function for the safe
work to finish. When a CPU drains its work queue, if it encounters a
work item marked as safe, it first waits for other CPUs to exit their
execution loops, then called the work item function, and finally
decrements 'safe_work_pending' counter with signalling other CPUs to let
them continue execution as soon as all pending safe work items have been
processed. The 'tcg_pending_threads' protected by 'exclusive_lock' in
user-mode or by 'qemu_global_mutex' in system-mode emulation is used to
determine if there is any CPU run and wait for it to exit the execution
loop. The fairness of all the CPU work queues is ensured by draining all
the pending safe work items before any CPU can run.

[1] http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg01128.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg02531.html
[3] http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg04792.html

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 - some conditional varialbes moved to cpu-exec-common.c
 - documentation commend for new public API added
---
 cpu-exec-common.c       | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
 cpus.c                  | 20 ++++++++++++++++++++
 include/exec/exec-all.h | 14 ++++++++++++++
 include/qom/cpu.h       | 14 ++++++++++++++
 linux-user/main.c       | 13 +++++++------
 5 files changed, 103 insertions(+), 7 deletions(-)

diff --git a/cpu-exec-common.c b/cpu-exec-common.c
index a233f0124559..6f278d6d3b70 100644
--- a/cpu-exec-common.c
+++ b/cpu-exec-common.c
@@ -25,6 +25,7 @@
 
 bool exit_request;
 CPUState *tcg_current_cpu;
+int tcg_pending_threads;
 
 /* exit the current TB, but without causing any exception to be raised */
 void cpu_loop_exit_noexc(CPUState *cpu)
@@ -79,6 +80,17 @@ void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
 }
 
 QemuCond qemu_work_cond;
+QemuCond qemu_safe_work_cond;
+QemuCond qemu_exclusive_cond;
+
+static int safe_work_pending;
+
+void wait_safe_cpu_work(void)
+{
+    while (atomic_mb_read(&safe_work_pending) > 0) {
+        qemu_cond_wait(&qemu_safe_work_cond, qemu_get_cpu_work_mutex());
+    }
+}
 
 static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
 {
@@ -91,9 +103,18 @@ static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
     cpu->queued_work_last = wi;
     wi->next = NULL;
     wi->done = false;
+    if (wi->safe) {
+        atomic_inc(&safe_work_pending);
+    }
     qemu_mutex_unlock(&cpu->work_mutex);
 
-    qemu_cpu_kick(cpu);
+    if (!wi->safe) {
+        qemu_cpu_kick(cpu);
+    } else {
+        CPU_FOREACH(cpu) {
+            qemu_cpu_kick(cpu);
+        }
+    }
 }
 
 void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
@@ -108,6 +129,7 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
     wi.func = func;
     wi.data = data;
     wi.free = false;
+    wi.safe = false;
 
     queue_work_on_cpu(cpu, &wi);
     while (!atomic_mb_read(&wi.done)) {
@@ -131,6 +153,20 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
     wi->func = func;
     wi->data = data;
     wi->free = true;
+    wi->safe = false;
+
+    queue_work_on_cpu(cpu, wi);
+}
+
+void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
+{
+    struct qemu_work_item *wi;
+
+    wi = g_malloc0(sizeof(struct qemu_work_item));
+    wi->func = func;
+    wi->data = data;
+    wi->free = true;
+    wi->safe = true;
 
     queue_work_on_cpu(cpu, wi);
 }
@@ -150,9 +186,20 @@ void process_queued_cpu_work(CPUState *cpu)
         if (!cpu->queued_work_first) {
             cpu->queued_work_last = NULL;
         }
+        if (wi->safe) {
+            while (tcg_pending_threads) {
+                qemu_cond_wait(&qemu_exclusive_cond,
+                               qemu_get_cpu_work_mutex());
+            }
+        }
         qemu_mutex_unlock(&cpu->work_mutex);
         wi->func(cpu, wi->data);
         qemu_mutex_lock(&cpu->work_mutex);
+        if (wi->safe) {
+            if (!atomic_dec_fetch(&safe_work_pending)) {
+                qemu_cond_broadcast(&qemu_safe_work_cond);
+            }
+        }
         if (wi->free) {
             g_free(wi);
         } else {
diff --git a/cpus.c b/cpus.c
index 282d7e399902..b7122043f650 100644
--- a/cpus.c
+++ b/cpus.c
@@ -903,6 +903,8 @@ void qemu_init_cpu_loop(void)
     qemu_cond_init(&qemu_cpu_cond);
     qemu_cond_init(&qemu_pause_cond);
     qemu_cond_init(&qemu_work_cond);
+    qemu_cond_init(&qemu_safe_work_cond);
+    qemu_cond_init(&qemu_exclusive_cond);
     qemu_cond_init(&qemu_io_proceeded_cond);
     qemu_mutex_init(&qemu_global_mutex);
 
@@ -926,6 +928,20 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu)
 {
 }
 
+/* called with qemu_global_mutex held */
+static inline void tcg_cpu_exec_start(CPUState *cpu)
+{
+    tcg_pending_threads++;
+}
+
+/* called with qemu_global_mutex held */
+static inline void tcg_cpu_exec_end(CPUState *cpu)
+{
+    if (--tcg_pending_threads) {
+        qemu_cond_broadcast(&qemu_exclusive_cond);
+    }
+}
+
 static void qemu_wait_io_event_common(CPUState *cpu)
 {
     if (cpu->stop) {
@@ -950,6 +966,8 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
     CPU_FOREACH(cpu) {
         qemu_wait_io_event_common(cpu);
     }
+
+    wait_safe_cpu_work();
 }
 
 static void qemu_kvm_wait_io_event(CPUState *cpu)
@@ -1485,7 +1503,9 @@ static void tcg_exec_all(void)
                           (cpu->singlestep_enabled & SSTEP_NOTIMER) == 0);
 
         if (cpu_can_run(cpu)) {
+            tcg_cpu_exec_start(cpu);
             r = tcg_cpu_exec(cpu);
+            tcg_cpu_exec_end(cpu);
             if (r == EXCP_DEBUG) {
                 cpu_handle_guest_debug(cpu);
                 break;
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index bceecbd5222a..992055ce36bf 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -405,12 +405,22 @@ extern int singlestep;
 
 /* cpu-exec.c, accessed with atomic_mb_read/atomic_mb_set */
 extern CPUState *tcg_current_cpu;
+extern int tcg_pending_threads;
 extern bool exit_request;
 
 /**
  * qemu_work_cond - condition to wait for CPU work items completion
  */
 extern QemuCond qemu_work_cond;
+/**
+ * qemu_safe_work_cond - condition to wait for safe CPU work items completion
+ */
+extern QemuCond qemu_safe_work_cond;
+/**
+ * qemu_exclusive_cond - condition to wait for all TCG threads to be out of
+ *                       guest code execution loop
+ */
+extern QemuCond qemu_exclusive_cond;
 
 /**
  * qemu_get_cpu_work_mutex() - get the mutex which protects CPU work execution
@@ -423,5 +433,9 @@ QemuMutex *qemu_get_cpu_work_mutex(void);
  * @cpu: The CPU which work queue to process.
  */
 void process_queued_cpu_work(CPUState *cpu);
+/**
+ * wait_safe_cpu_work() - wait until all safe CPU work items has processed
+ */
+void wait_safe_cpu_work(void);
 
 #endif
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 4e688f645b4a..5128fcc1745a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -231,6 +231,7 @@ struct qemu_work_item {
     void *data;
     int done;
     bool free;
+    bool safe;
 };
 
 /**
@@ -625,6 +626,19 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
 void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
 
 /**
+ * async_safe_run_on_cpu:
+ * @cpu: The vCPU to run on.
+ * @func: The function to be executed.
+ * @data: Data to pass to the function.
+ *
+ * Schedules the function @func for execution on the vCPU @cpu asynchronously
+ * and in quiescent state. Quiescent state means: (1) all other vCPUs are
+ * halted and (2) #qemu_global_mutex (a.k.a. BQL) in system-mode or
+ * #exclusive_lock in user-mode emulation is held while @func is executing.
+ */
+void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
+
+/**
  * qemu_get_cpu:
  * @index: The CPUState@cpu_index value of the CPU to obtain.
  *
diff --git a/linux-user/main.c b/linux-user/main.c
index fce61d5a35fc..d0ff5f9976e5 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -110,18 +110,17 @@ int cpu_get_pic_interrupt(CPUX86State *env)
    which requires quite a lot of per host/target work.  */
 static QemuMutex cpu_list_mutex;
 static QemuMutex exclusive_lock;
-static QemuCond exclusive_cond;
 static QemuCond exclusive_resume;
 static bool exclusive_pending;
-static int tcg_pending_threads;
 
 void qemu_init_cpu_loop(void)
 {
     qemu_mutex_init(&cpu_list_mutex);
     qemu_mutex_init(&exclusive_lock);
-    qemu_cond_init(&exclusive_cond);
     qemu_cond_init(&exclusive_resume);
     qemu_cond_init(&qemu_work_cond);
+    qemu_cond_init(&qemu_safe_work_cond);
+    qemu_cond_init(&qemu_exclusive_cond);
 }
 
 /* Make sure everything is in a consistent state for calling fork().  */
@@ -148,9 +147,10 @@ void fork_end(int child)
         exclusive_pending = false;
         qemu_mutex_init(&exclusive_lock);
         qemu_mutex_init(&cpu_list_mutex);
-        qemu_cond_init(&exclusive_cond);
         qemu_cond_init(&exclusive_resume);
         qemu_cond_init(&qemu_work_cond);
+        qemu_cond_init(&qemu_safe_work_cond);
+        qemu_cond_init(&qemu_exclusive_cond);
         qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
         gdbserver_fork(thread_cpu);
     } else {
@@ -190,7 +190,7 @@ static inline void start_exclusive(void)
         }
     }
     while (tcg_pending_threads) {
-        qemu_cond_wait(&exclusive_cond, &exclusive_lock);
+        qemu_cond_wait(&qemu_exclusive_cond, &exclusive_lock);
     }
 }
 
@@ -219,10 +219,11 @@ static inline void cpu_exec_end(CPUState *cpu)
     cpu->running = false;
     tcg_pending_threads--;
     if (!tcg_pending_threads) {
-        qemu_cond_signal(&exclusive_cond);
+        qemu_cond_broadcast(&qemu_exclusive_cond);
     }
     exclusive_idle();
     process_queued_cpu_work(cpu);
+    wait_safe_cpu_work();
     qemu_mutex_unlock(&exclusive_lock);
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (9 preceding siblings ...)
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 10/11] cpu-exec-common: Introduce async_safe_run_on_cpu() Sergey Fedorov
@ 2016-07-06 21:15 ` Sergey Fedorov
  2016-07-07 20:11   ` Sergey Fedorov
  2016-07-14  8:41   ` Alex Bennée
  2016-07-14 16:00 ` [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Alex Bennée
  11 siblings, 2 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-06 21:15 UTC (permalink / raw)
  To: qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Sergey Fedorov, Sergey Fedorov,
	Peter Crosthwaite

From: Sergey Fedorov <serge.fdrv@gmail.com>

Use async_safe_run_on_cpu() to make tb_flush() thread safe.

Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---

Changes in v2:
 - stale comment about unsafe tb_flush() removed
---
 translate-all.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index eaa95e4cd7dc..e69b5d4e889e 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -831,8 +831,7 @@ static void page_flush_tb(void)
 }
 
 /* flush all the translation blocks */
-/* XXX: tb_flush is currently not thread safe */
-void tb_flush(CPUState *cpu)
+static void do_tb_flush(CPUState *cpu, void *data)
 {
 #if defined(DEBUG_FLUSH)
     printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
@@ -861,6 +860,11 @@ void tb_flush(CPUState *cpu)
     tcg_ctx.tb_ctx.tb_flush_count++;
 }
 
+void tb_flush(CPUState *cpu)
+{
+    async_safe_run_on_cpu(cpu, do_tb_flush, NULL);
+}
+
 #ifdef DEBUG_TB_CHECK
 
 static void
@@ -1163,9 +1167,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
  buffer_overflow:
         /* flush must be done */
         tb_flush(cpu);
-        /* cannot fail at this point */
-        tb = tb_alloc(pc);
-        assert(tb != NULL);
+        mmap_unlock();
+        cpu_loop_exit(cpu);
     }
 
     gen_code_buf = tcg_ctx.code_gen_ptr;
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 33+ messages in thread

* Re: [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
  2016-07-06 21:14   ` [Qemu-devel] " Sergey Fedorov
@ 2016-07-07  0:30     ` David Gibson
  -1 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-07-07  0:30 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, Alexander Graf,
	Marcelo Tosatti, Christian Borntraeger, Cornelia Huck, qemu-ppc,
	kvm

[-- Attachment #1: Type: text/plain, Size: 30077 bytes --]

On Thu, Jul 07, 2016 at 12:14:54AM +0300, Sergey Fedorov wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> CPUState is a fairly common pointer to pass to these helpers. This means
> if you need other arguments for the async_run_on_cpu case you end up
> having to do a g_malloc to stuff additional data into the routine. For
> the current users this isn't a massive deal but for MTTCG this gets
> cumbersome when the only other parameter is often an address.
> 
> This adds the typedef run_on_cpu_func for helper functions which has an
> explicit CPUState * passed as the first parameter. All the users of
> run_on_cpu and async_run_on_cpu have had their helpers updated to use
> CPUState where available.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> [Sergey Fedorov:
>  - eliminate more CPUState in user data;
>  - remove unnecessary user data passing;
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>

ppc parts:

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
> Changes in v2:
>  - eliminate more CPUState in user data
>  - remove unnecessary user data passing
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
> 
> ---
>  cpus.c                     | 15 ++++---
>  hw/i386/kvm/apic.c         |  3 +-
>  hw/i386/kvmvapic.c         |  6 +--
>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>  hw/ppc/spapr.c             |  6 +--
>  hw/ppc/spapr_hcall.c       | 17 ++++----
>  include/qom/cpu.h          |  8 ++--
>  kvm-all.c                  | 21 ++++------
>  target-i386/helper.c       | 19 ++++-----
>  target-i386/kvm.c          |  6 +--
>  target-s390x/cpu.c         |  4 +-
>  target-s390x/cpu.h         |  7 +---
>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>  target-s390x/misc_helper.c |  4 +-
>  14 files changed, 108 insertions(+), 137 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 84c3520d446f..049c2d04e150 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -551,9 +551,8 @@ static const VMStateDescription vmstate_timers = {
>      }
>  };
>  
> -static void cpu_throttle_thread(void *opaque)
> +static void cpu_throttle_thread(CPUState *cpu, void *opaque)
>  {
> -    CPUState *cpu = opaque;
>      double pct;
>      double throttle_ratio;
>      long sleeptime_ns;
> @@ -583,7 +582,7 @@ static void cpu_throttle_timer_tick(void *opaque)
>      }
>      CPU_FOREACH(cpu) {
>          if (!atomic_xchg(&cpu->throttle_thread_scheduled, 1)) {
> -            async_run_on_cpu(cpu, cpu_throttle_thread, cpu);
> +            async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
>          }
>      }
>  
> @@ -911,12 +910,12 @@ void qemu_init_cpu_loop(void)
>      qemu_thread_get_self(&io_thread);
>  }
>  
> -void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> +void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>  {
>      struct qemu_work_item wi;
>  
>      if (qemu_cpu_is_self(cpu)) {
> -        func(data);
> +        func(cpu, data);
>          return;
>      }
>  
> @@ -944,12 +943,12 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
>      }
>  }
>  
> -void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> +void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>  {
>      struct qemu_work_item *wi;
>  
>      if (qemu_cpu_is_self(cpu)) {
> -        func(data);
> +        func(cpu, data);
>          return;
>      }
>  
> @@ -1000,7 +999,7 @@ static void flush_queued_work(CPUState *cpu)
>              cpu->queued_work_last = NULL;
>          }
>          qemu_mutex_unlock(&cpu->work_mutex);
> -        wi->func(wi->data);
> +        wi->func(cpu, wi->data);
>          qemu_mutex_lock(&cpu->work_mutex);
>          if (wi->free) {
>              g_free(wi);
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index c5983c79be47..9b66e741d4b4 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -125,10 +125,9 @@ static void kvm_apic_vapic_base_update(APICCommonState *s)
>      }
>  }
>  
> -static void do_inject_external_nmi(void *data)
> +static void do_inject_external_nmi(CPUState *cpu, void *data)
>  {
>      APICCommonState *s = data;
> -    CPUState *cpu = CPU(s->cpu);
>      uint32_t lvt;
>      int ret;
>  
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index 3bf1ddd97612..1bc02fb2f1a1 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -483,7 +483,7 @@ typedef struct VAPICEnableTPRReporting {
>      bool enable;
>  } VAPICEnableTPRReporting;
>  
> -static void vapic_do_enable_tpr_reporting(void *data)
> +static void vapic_do_enable_tpr_reporting(CPUState *cpu, void *data)
>  {
>      VAPICEnableTPRReporting *info = data;
>  
> @@ -734,10 +734,10 @@ static void vapic_realize(DeviceState *dev, Error **errp)
>      nb_option_roms++;
>  }
>  
> -static void do_vapic_enable(void *data)
> +static void do_vapic_enable(CPUState *cs, void *data)
>  {
>      VAPICROMState *s = data;
> -    X86CPU *cpu = X86_CPU(first_cpu);
> +    X86CPU *cpu = X86_CPU(cs);
>  
>      static const uint8_t enabled = 1;
>      cpu_physical_memory_write(s->vapic_paddr + offsetof(VAPICState, enabled),
> diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
> index 22c584eb8dd0..8e16f651ea95 100644
> --- a/hw/ppc/ppce500_spin.c
> +++ b/hw/ppc/ppce500_spin.c
> @@ -54,11 +54,6 @@ typedef struct SpinState {
>      SpinInfo spin[MAX_CPUS];
>  } SpinState;
>  
> -typedef struct spin_kick {
> -    PowerPCCPU *cpu;
> -    SpinInfo *spin;
> -} SpinKick;
> -
>  static void spin_reset(void *opaque)
>  {
>      SpinState *s = opaque;
> @@ -89,16 +84,15 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
>      env->tlb_dirty = true;
>  }
>  
> -static void spin_kick(void *data)
> +static void spin_kick(CPUState *cs, void *data)
>  {
> -    SpinKick *kick = data;
> -    CPUState *cpu = CPU(kick->cpu);
> -    CPUPPCState *env = &kick->cpu->env;
> -    SpinInfo *curspin = kick->spin;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    SpinInfo *curspin = data;
>      hwaddr map_size = 64 * 1024 * 1024;
>      hwaddr map_start;
>  
> -    cpu_synchronize_state(cpu);
> +    cpu_synchronize_state(cs);
>      stl_p(&curspin->pir, env->spr[SPR_BOOKE_PIR]);
>      env->nip = ldq_p(&curspin->addr) & (map_size - 1);
>      env->gpr[3] = ldq_p(&curspin->r3);
> @@ -112,10 +106,10 @@ static void spin_kick(void *data)
>      map_start = ldq_p(&curspin->addr) & ~(map_size - 1);
>      mmubooke_create_initial_mapping(env, 0, map_start, map_size);
>  
> -    cpu->halted = 0;
> -    cpu->exception_index = -1;
> -    cpu->stopped = false;
> -    qemu_cpu_kick(cpu);
> +    cs->halted = 0;
> +    cs->exception_index = -1;
> +    cs->stopped = false;
> +    qemu_cpu_kick(cs);
>  }
>  
>  static void spin_write(void *opaque, hwaddr addr, uint64_t value,
> @@ -153,12 +147,7 @@ static void spin_write(void *opaque, hwaddr addr, uint64_t value,
>  
>      if (!(ldq_p(&curspin->addr) & 1)) {
>          /* run CPU */
> -        SpinKick kick = {
> -            .cpu = POWERPC_CPU(cpu),
> -            .spin = curspin,
> -        };
> -
> -        run_on_cpu(cpu, spin_kick, &kick);
> +        run_on_cpu(cpu, spin_kick, curspin);
>      }
>  }
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7f33a1b2b57d..882a3c621367 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2170,10 +2170,8 @@ static void spapr_machine_finalizefn(Object *obj)
>      g_free(spapr->kvm_type);
>  }
>  
> -static void ppc_cpu_do_nmi_on_cpu(void *arg)
> +static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg)
>  {
> -    CPUState *cs = arg;
> -
>      cpu_synchronize_state(cs);
>      ppc_cpu_do_system_reset(cs);
>  }
> @@ -2183,7 +2181,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
>      CPUState *cs;
>  
>      CPU_FOREACH(cs) {
> -        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
> +        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, NULL);
>      }
>  }
>  
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 73af112e1d36..e5eca67f5b71 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -13,19 +13,18 @@
>  #include "kvm_ppc.h"
>  
>  struct SPRSyncState {
> -    CPUState *cs;
>      int spr;
>      target_ulong value;
>      target_ulong mask;
>  };
>  
> -static void do_spr_sync(void *arg)
> +static void do_spr_sync(CPUState *cs, void *arg)
>  {
>      struct SPRSyncState *s = arg;
> -    PowerPCCPU *cpu = POWERPC_CPU(s->cs);
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>      CPUPPCState *env = &cpu->env;
>  
> -    cpu_synchronize_state(s->cs);
> +    cpu_synchronize_state(cs);
>      env->spr[s->spr] &= ~s->mask;
>      env->spr[s->spr] |= s->value;
>  }
> @@ -34,7 +33,6 @@ static void set_spr(CPUState *cs, int spr, target_ulong value,
>                      target_ulong mask)
>  {
>      struct SPRSyncState s = {
> -        .cs = cs,
>          .spr = spr,
>          .value = value,
>          .mask = mask
> @@ -907,17 +905,17 @@ static target_ulong cas_get_option_vector(int vector, target_ulong table)
>  }
>  
>  typedef struct {
> -    PowerPCCPU *cpu;
>      uint32_t cpu_version;
>      Error *err;
>  } SetCompatState;
>  
> -static void do_set_compat(void *arg)
> +static void do_set_compat(CPUState *cs, void *arg)
>  {
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>      SetCompatState *s = arg;
>  
> -    cpu_synchronize_state(CPU(s->cpu));
> -    ppc_set_compat(s->cpu, s->cpu_version, &s->err);
> +    cpu_synchronize_state(cs);
> +    ppc_set_compat(cpu, s->cpu_version, &s->err);
>  }
>  
>  #define get_compat_level(cpuver) ( \
> @@ -1013,7 +1011,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>      if (old_cpu_version != cpu_version) {
>          CPU_FOREACH(cs) {
>              SetCompatState s = {
> -                .cpu = POWERPC_CPU(cs),
>                  .cpu_version = cpu_version,
>                  .err = NULL,
>              };
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 32f3af3e1c63..4e688f645b4a 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -223,9 +223,11 @@ struct kvm_run;
>  #define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
>  
>  /* work queue */
> +typedef void (*run_on_cpu_func)(CPUState *cpu, void *data);
> +
>  struct qemu_work_item {
>      struct qemu_work_item *next;
> -    void (*func)(void *data);
> +    run_on_cpu_func func;
>      void *data;
>      int done;
>      bool free;
> @@ -610,7 +612,7 @@ bool cpu_is_stopped(CPUState *cpu);
>   *
>   * Schedules the function @func for execution on the vCPU @cpu.
>   */
> -void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
> +void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>  
>  /**
>   * async_run_on_cpu:
> @@ -620,7 +622,7 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
>   *
>   * Schedules the function @func for execution on the vCPU @cpu asynchronously.
>   */
> -void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
> +void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>  
>  /**
>   * qemu_get_cpu:
> diff --git a/kvm-all.c b/kvm-all.c
> index a88f917fda69..a8320574cc8a 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1828,10 +1828,8 @@ void kvm_flush_coalesced_mmio_buffer(void)
>      s->coalesced_flush_in_progress = false;
>  }
>  
> -static void do_kvm_cpu_synchronize_state(void *arg)
> +static void do_kvm_cpu_synchronize_state(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      if (!cpu->kvm_vcpu_dirty) {
>          kvm_arch_get_registers(cpu);
>          cpu->kvm_vcpu_dirty = true;
> @@ -1841,34 +1839,30 @@ static void do_kvm_cpu_synchronize_state(void *arg)
>  void kvm_cpu_synchronize_state(CPUState *cpu)
>  {
>      if (!cpu->kvm_vcpu_dirty) {
> -        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, cpu);
> +        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, NULL);
>      }
>  }
>  
> -static void do_kvm_cpu_synchronize_post_reset(void *arg)
> +static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
>      cpu->kvm_vcpu_dirty = false;
>  }
>  
>  void kvm_cpu_synchronize_post_reset(CPUState *cpu)
>  {
> -    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, cpu);
> +    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, NULL);
>  }
>  
> -static void do_kvm_cpu_synchronize_post_init(void *arg)
> +static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
>      cpu->kvm_vcpu_dirty = false;
>  }
>  
>  void kvm_cpu_synchronize_post_init(CPUState *cpu)
>  {
> -    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu);
> +    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, NULL);
>  }
>  
>  int kvm_cpu_exec(CPUState *cpu)
> @@ -2210,7 +2204,7 @@ struct kvm_set_guest_debug_data {
>      int err;
>  };
>  
> -static void kvm_invoke_set_guest_debug(void *data)
> +static void kvm_invoke_set_guest_debug(CPUState *unused_cpu, void *data)
>  {
>      struct kvm_set_guest_debug_data *dbg_data = data;
>  
> @@ -2228,7 +2222,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>          data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
>      }
>      kvm_arch_update_guest_debug(cpu, &data.dbg);
> -    data.cpu = cpu;
>  
>      run_on_cpu(cpu, kvm_invoke_set_guest_debug, &data);
>      return data.err;
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 1c250b82452d..9bc961bff342 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1113,7 +1113,6 @@ out:
>  
>  typedef struct MCEInjectionParams {
>      Monitor *mon;
> -    X86CPU *cpu;
>      int bank;
>      uint64_t status;
>      uint64_t mcg_status;
> @@ -1122,14 +1121,14 @@ typedef struct MCEInjectionParams {
>      int flags;
>  } MCEInjectionParams;
>  
> -static void do_inject_x86_mce(void *data)
> +static void do_inject_x86_mce(CPUState *cs, void *data)
>  {
>      MCEInjectionParams *params = data;
> -    CPUX86State *cenv = &params->cpu->env;
> -    CPUState *cpu = CPU(params->cpu);
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *cenv = &cpu->env;
>      uint64_t *banks = cenv->mce_banks + 4 * params->bank;
>  
> -    cpu_synchronize_state(cpu);
> +    cpu_synchronize_state(cs);
>  
>      /*
>       * If there is an MCE exception being processed, ignore this SRAO MCE
> @@ -1149,7 +1148,7 @@ static void do_inject_x86_mce(void *data)
>          if ((cenv->mcg_cap & MCG_CTL_P) && cenv->mcg_ctl != ~(uint64_t)0) {
>              monitor_printf(params->mon,
>                             "CPU %d: Uncorrected error reporting disabled\n",
> -                           cpu->cpu_index);
> +                           cs->cpu_index);
>              return;
>          }
>  
> @@ -1161,7 +1160,7 @@ static void do_inject_x86_mce(void *data)
>              monitor_printf(params->mon,
>                             "CPU %d: Uncorrected error reporting disabled for"
>                             " bank %d\n",
> -                           cpu->cpu_index, params->bank);
> +                           cs->cpu_index, params->bank);
>              return;
>          }
>  
> @@ -1170,7 +1169,7 @@ static void do_inject_x86_mce(void *data)
>              monitor_printf(params->mon,
>                             "CPU %d: Previous MCE still in progress, raising"
>                             " triple fault\n",
> -                           cpu->cpu_index);
> +                           cs->cpu_index);
>              qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
>              qemu_system_reset_request();
>              return;
> @@ -1182,7 +1181,7 @@ static void do_inject_x86_mce(void *data)
>          banks[3] = params->misc;
>          cenv->mcg_status = params->mcg_status;
>          banks[1] = params->status;
> -        cpu_interrupt(cpu, CPU_INTERRUPT_MCE);
> +        cpu_interrupt(cs, CPU_INTERRUPT_MCE);
>      } else if (!(banks[1] & MCI_STATUS_VAL)
>                 || !(banks[1] & MCI_STATUS_UC)) {
>          if (banks[1] & MCI_STATUS_VAL) {
> @@ -1204,7 +1203,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
>      CPUX86State *cenv = &cpu->env;
>      MCEInjectionParams params = {
>          .mon = mon,
> -        .cpu = cpu,
>          .bank = bank,
>          .status = status,
>          .mcg_status = mcg_status,
> @@ -1245,7 +1243,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
>              if (other_cs == cs) {
>                  continue;
>              }
> -            params.cpu = X86_CPU(other_cs);
>              run_on_cpu(other_cs, do_inject_x86_mce, &params);
>          }
>      }
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index f3698f19b53b..a1c6e656d13c 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -151,10 +151,8 @@ static int kvm_get_tsc(CPUState *cs)
>      return 0;
>  }
>  
> -static inline void do_kvm_synchronize_tsc(void *arg)
> +static inline void do_kvm_synchronize_tsc(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      kvm_get_tsc(cpu);
>  }
>  
> @@ -164,7 +162,7 @@ void kvm_synchronize_all_tsc(void)
>  
>      if (kvm_enabled()) {
>          CPU_FOREACH(cpu) {
> -            run_on_cpu(cpu, do_kvm_synchronize_tsc, cpu);
> +            run_on_cpu(cpu, do_kvm_synchronize_tsc, NULL);
>          }
>      }
>  }
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index e43e2d61550b..4f09c647b7a8 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -188,7 +188,7 @@ static void s390_cpu_machine_reset_cb(void *opaque)
>  {
>      S390CPU *cpu = opaque;
>  
> -    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, CPU(cpu));
> +    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, NULL);
>  }
>  #endif
>  
> @@ -238,7 +238,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>      s390_cpu_gdb_init(cs);
>      qemu_init_vcpu(cs);
>  #if !defined(CONFIG_USER_ONLY)
> -    run_on_cpu(cs, s390_do_cpu_full_reset, cs);
> +    run_on_cpu(cs, s390_do_cpu_full_reset, NULL);
>  #else
>      cpu_reset(cs);
>  #endif
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 8bcb0f75f399..8998ce21ca13 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -498,17 +498,14 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
>  #define decode_basedisp_rs decode_basedisp_s
>  
>  /* helper functions for run_on_cpu() */
> -static inline void s390_do_cpu_reset(void *arg)
> +static inline void s390_do_cpu_reset(CPUState *cs, void *arg)
>  {
> -    CPUState *cs = arg;
>      S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
>  
>      scc->cpu_reset(cs);
>  }
> -static inline void s390_do_cpu_full_reset(void *arg)
> +static inline void s390_do_cpu_full_reset(CPUState *cs, void *arg)
>  {
> -    CPUState *cs = arg;
> -
>      cpu_reset(cs);
>  }
>  
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 45e94ca48abd..b37eb9d3cd8c 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -1354,7 +1354,6 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
>  }
>  
>  typedef struct SigpInfo {
> -    S390CPU *cpu;
>      uint64_t param;
>      int cc;
>      uint64_t *status_reg;
> @@ -1367,38 +1366,40 @@ static void set_sigp_status(SigpInfo *si, uint64_t status)
>      si->cc = SIGP_CC_STATUS_STORED;
>  }
>  
> -static void sigp_start(void *arg)
> +static void sigp_start(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>  
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>          return;
>      }
>  
> -    s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
> +    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_stop(void *arg)
> +static void sigp_stop(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      struct kvm_s390_irq irq = {
>          .type = KVM_S390_SIGP_STOP,
>      };
>  
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_OPERATING) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
>          si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>          return;
>      }
>  
>      /* disabled wait - sleeping in user space */
> -    if (CPU(si->cpu)->halted) {
> -        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
> +    if (cs->halted) {
> +        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>      } else {
>          /* execute the stop function */
> -        si->cpu->env.sigp_order = SIGP_STOP;
> -        kvm_s390_vcpu_interrupt(si->cpu, &irq);
> +        cpu->env.sigp_order = SIGP_STOP;
> +        kvm_s390_vcpu_interrupt(cpu, &irq);
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
> @@ -1465,56 +1466,58 @@ static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
>      return 0;
>  }
>  
> -static void sigp_stop_and_store_status(void *arg)
> +static void sigp_stop_and_store_status(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      struct kvm_s390_irq irq = {
>          .type = KVM_S390_SIGP_STOP,
>      };
>  
>      /* disabled wait - sleeping in user space */
> -    if (s390_cpu_get_state(si->cpu) == CPU_STATE_OPERATING &&
> -        CPU(si->cpu)->halted) {
> -        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
> +    if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
> +        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>      }
>  
> -    switch (s390_cpu_get_state(si->cpu)) {
> +    switch (s390_cpu_get_state(cpu)) {
>      case CPU_STATE_OPERATING:
> -        si->cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
> -        kvm_s390_vcpu_interrupt(si->cpu, &irq);
> +        cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
> +        kvm_s390_vcpu_interrupt(cpu, &irq);
>          /* store will be performed when handling the stop intercept */
>          break;
>      case CPU_STATE_STOPPED:
>          /* already stopped, just store the status */
> -        cpu_synchronize_state(CPU(si->cpu));
> -        kvm_s390_store_status(si->cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
> +        cpu_synchronize_state(cs);
> +        kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
>          break;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_store_status_at_address(void *arg)
> +static void sigp_store_status_at_address(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      uint32_t address = si->param & 0x7ffffe00u;
>  
>      /* cpu has to be stopped */
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
>          return;
>      }
>  
> -    cpu_synchronize_state(CPU(si->cpu));
> +    cpu_synchronize_state(cs);
>  
> -    if (kvm_s390_store_status(si->cpu, address, false)) {
> +    if (kvm_s390_store_status(cpu, address, false)) {
>          set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
>          return;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_store_adtl_status(void *arg)
> +static void sigp_store_adtl_status(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>  
>      if (!kvm_check_extension(kvm_state, KVM_CAP_S390_VECTOR_REGISTERS)) {
> @@ -1523,7 +1526,7 @@ static void sigp_store_adtl_status(void *arg)
>      }
>  
>      /* cpu has to be stopped */
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
>          return;
>      }
> @@ -1534,31 +1537,32 @@ static void sigp_store_adtl_status(void *arg)
>          return;
>      }
>  
> -    cpu_synchronize_state(CPU(si->cpu));
> +    cpu_synchronize_state(cs);
>  
> -    if (kvm_s390_store_adtl_status(si->cpu, si->param)) {
> +    if (kvm_s390_store_adtl_status(cpu, si->param)) {
>          set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
>          return;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_restart(void *arg)
> +static void sigp_restart(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      struct kvm_s390_irq irq = {
>          .type = KVM_S390_RESTART,
>      };
>  
> -    switch (s390_cpu_get_state(si->cpu)) {
> +    switch (s390_cpu_get_state(cpu)) {
>      case CPU_STATE_STOPPED:
>          /* the restart irq has to be delivered prior to any other pending irq */
> -        cpu_synchronize_state(CPU(si->cpu));
> -        do_restart_interrupt(&si->cpu->env);
> -        s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
> +        cpu_synchronize_state(cs);
> +        do_restart_interrupt(&cpu->env);
> +        s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
>          break;
>      case CPU_STATE_OPERATING:
> -        kvm_s390_vcpu_interrupt(si->cpu, &irq);
> +        kvm_s390_vcpu_interrupt(cpu, &irq);
>          break;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
> @@ -1566,20 +1570,18 @@ static void sigp_restart(void *arg)
>  
>  int kvm_s390_cpu_restart(S390CPU *cpu)
>  {
> -    SigpInfo si = {
> -        .cpu = cpu,
> -    };
> +    SigpInfo si = {};
>  
>      run_on_cpu(CPU(cpu), sigp_restart, &si);
>      DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
>      return 0;
>  }
>  
> -static void sigp_initial_cpu_reset(void *arg)
> +static void sigp_initial_cpu_reset(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
> +    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
>      SigpInfo *si = arg;
> -    CPUState *cs = CPU(si->cpu);
> -    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
>  
>      cpu_synchronize_state(cs);
>      scc->initial_cpu_reset(cs);
> @@ -1587,11 +1589,11 @@ static void sigp_initial_cpu_reset(void *arg)
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_cpu_reset(void *arg)
> +static void sigp_cpu_reset(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
> +    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
>      SigpInfo *si = arg;
> -    CPUState *cs = CPU(si->cpu);
> -    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
>  
>      cpu_synchronize_state(cs);
>      scc->cpu_reset(cs);
> @@ -1599,12 +1601,13 @@ static void sigp_cpu_reset(void *arg)
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_set_prefix(void *arg)
> +static void sigp_set_prefix(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      uint32_t addr = si->param & 0x7fffe000u;
>  
> -    cpu_synchronize_state(CPU(si->cpu));
> +    cpu_synchronize_state(cs);
>  
>      if (!address_space_access_valid(&address_space_memory, addr,
>                                      sizeof(struct LowCore), false)) {
> @@ -1613,13 +1616,13 @@ static void sigp_set_prefix(void *arg)
>      }
>  
>      /* cpu has to be stopped */
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
>          return;
>      }
>  
> -    si->cpu->env.psa = addr;
> -    cpu_synchronize_post_init(CPU(si->cpu));
> +    cpu->env.psa = addr;
> +    cpu_synchronize_post_init(cs);
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> @@ -1627,7 +1630,6 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
>                                    uint64_t param, uint64_t *status_reg)
>  {
>      SigpInfo si = {
> -        .cpu = dst_cpu,
>          .param = param,
>          .status_reg = status_reg,
>      };
> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
> index 86da1947b984..4df2ec6c7dab 100644
> --- a/target-s390x/misc_helper.c
> +++ b/target-s390x/misc_helper.c
> @@ -126,7 +126,7 @@ static int modified_clear_reset(S390CPU *cpu)
>      pause_all_vcpus();
>      cpu_synchronize_all_states();
>      CPU_FOREACH(t) {
> -        run_on_cpu(t, s390_do_cpu_full_reset, t);
> +        run_on_cpu(t, s390_do_cpu_full_reset, NULL);
>      }
>      s390_cmma_reset();
>      subsystem_reset();
> @@ -145,7 +145,7 @@ static int load_normal_reset(S390CPU *cpu)
>      pause_all_vcpus();
>      cpu_synchronize_all_states();
>      CPU_FOREACH(t) {
> -        run_on_cpu(t, s390_do_cpu_reset, t);
> +        run_on_cpu(t, s390_do_cpu_reset, NULL);
>      }
>      s390_cmma_reset();
>      subsystem_reset();

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
@ 2016-07-07  0:30     ` David Gibson
  0 siblings, 0 replies; 33+ messages in thread
From: David Gibson @ 2016-07-07  0:30 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, Alexander Graf,
	Marcelo Tosatti, Christian Borntraeger, Cornelia Huck, qemu-ppc,
	kvm

[-- Attachment #1: Type: text/plain, Size: 30077 bytes --]

On Thu, Jul 07, 2016 at 12:14:54AM +0300, Sergey Fedorov wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> CPUState is a fairly common pointer to pass to these helpers. This means
> if you need other arguments for the async_run_on_cpu case you end up
> having to do a g_malloc to stuff additional data into the routine. For
> the current users this isn't a massive deal but for MTTCG this gets
> cumbersome when the only other parameter is often an address.
> 
> This adds the typedef run_on_cpu_func for helper functions which has an
> explicit CPUState * passed as the first parameter. All the users of
> run_on_cpu and async_run_on_cpu have had their helpers updated to use
> CPUState where available.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> [Sergey Fedorov:
>  - eliminate more CPUState in user data;
>  - remove unnecessary user data passing;
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>

ppc parts:

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
> 
> Changes in v2:
>  - eliminate more CPUState in user data
>  - remove unnecessary user data passing
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
> 
> ---
>  cpus.c                     | 15 ++++---
>  hw/i386/kvm/apic.c         |  3 +-
>  hw/i386/kvmvapic.c         |  6 +--
>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>  hw/ppc/spapr.c             |  6 +--
>  hw/ppc/spapr_hcall.c       | 17 ++++----
>  include/qom/cpu.h          |  8 ++--
>  kvm-all.c                  | 21 ++++------
>  target-i386/helper.c       | 19 ++++-----
>  target-i386/kvm.c          |  6 +--
>  target-s390x/cpu.c         |  4 +-
>  target-s390x/cpu.h         |  7 +---
>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>  target-s390x/misc_helper.c |  4 +-
>  14 files changed, 108 insertions(+), 137 deletions(-)
> 
> diff --git a/cpus.c b/cpus.c
> index 84c3520d446f..049c2d04e150 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -551,9 +551,8 @@ static const VMStateDescription vmstate_timers = {
>      }
>  };
>  
> -static void cpu_throttle_thread(void *opaque)
> +static void cpu_throttle_thread(CPUState *cpu, void *opaque)
>  {
> -    CPUState *cpu = opaque;
>      double pct;
>      double throttle_ratio;
>      long sleeptime_ns;
> @@ -583,7 +582,7 @@ static void cpu_throttle_timer_tick(void *opaque)
>      }
>      CPU_FOREACH(cpu) {
>          if (!atomic_xchg(&cpu->throttle_thread_scheduled, 1)) {
> -            async_run_on_cpu(cpu, cpu_throttle_thread, cpu);
> +            async_run_on_cpu(cpu, cpu_throttle_thread, NULL);
>          }
>      }
>  
> @@ -911,12 +910,12 @@ void qemu_init_cpu_loop(void)
>      qemu_thread_get_self(&io_thread);
>  }
>  
> -void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> +void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>  {
>      struct qemu_work_item wi;
>  
>      if (qemu_cpu_is_self(cpu)) {
> -        func(data);
> +        func(cpu, data);
>          return;
>      }
>  
> @@ -944,12 +943,12 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
>      }
>  }
>  
> -void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data)
> +void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>  {
>      struct qemu_work_item *wi;
>  
>      if (qemu_cpu_is_self(cpu)) {
> -        func(data);
> +        func(cpu, data);
>          return;
>      }
>  
> @@ -1000,7 +999,7 @@ static void flush_queued_work(CPUState *cpu)
>              cpu->queued_work_last = NULL;
>          }
>          qemu_mutex_unlock(&cpu->work_mutex);
> -        wi->func(wi->data);
> +        wi->func(cpu, wi->data);
>          qemu_mutex_lock(&cpu->work_mutex);
>          if (wi->free) {
>              g_free(wi);
> diff --git a/hw/i386/kvm/apic.c b/hw/i386/kvm/apic.c
> index c5983c79be47..9b66e741d4b4 100644
> --- a/hw/i386/kvm/apic.c
> +++ b/hw/i386/kvm/apic.c
> @@ -125,10 +125,9 @@ static void kvm_apic_vapic_base_update(APICCommonState *s)
>      }
>  }
>  
> -static void do_inject_external_nmi(void *data)
> +static void do_inject_external_nmi(CPUState *cpu, void *data)
>  {
>      APICCommonState *s = data;
> -    CPUState *cpu = CPU(s->cpu);
>      uint32_t lvt;
>      int ret;
>  
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index 3bf1ddd97612..1bc02fb2f1a1 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
> @@ -483,7 +483,7 @@ typedef struct VAPICEnableTPRReporting {
>      bool enable;
>  } VAPICEnableTPRReporting;
>  
> -static void vapic_do_enable_tpr_reporting(void *data)
> +static void vapic_do_enable_tpr_reporting(CPUState *cpu, void *data)
>  {
>      VAPICEnableTPRReporting *info = data;
>  
> @@ -734,10 +734,10 @@ static void vapic_realize(DeviceState *dev, Error **errp)
>      nb_option_roms++;
>  }
>  
> -static void do_vapic_enable(void *data)
> +static void do_vapic_enable(CPUState *cs, void *data)
>  {
>      VAPICROMState *s = data;
> -    X86CPU *cpu = X86_CPU(first_cpu);
> +    X86CPU *cpu = X86_CPU(cs);
>  
>      static const uint8_t enabled = 1;
>      cpu_physical_memory_write(s->vapic_paddr + offsetof(VAPICState, enabled),
> diff --git a/hw/ppc/ppce500_spin.c b/hw/ppc/ppce500_spin.c
> index 22c584eb8dd0..8e16f651ea95 100644
> --- a/hw/ppc/ppce500_spin.c
> +++ b/hw/ppc/ppce500_spin.c
> @@ -54,11 +54,6 @@ typedef struct SpinState {
>      SpinInfo spin[MAX_CPUS];
>  } SpinState;
>  
> -typedef struct spin_kick {
> -    PowerPCCPU *cpu;
> -    SpinInfo *spin;
> -} SpinKick;
> -
>  static void spin_reset(void *opaque)
>  {
>      SpinState *s = opaque;
> @@ -89,16 +84,15 @@ static void mmubooke_create_initial_mapping(CPUPPCState *env,
>      env->tlb_dirty = true;
>  }
>  
> -static void spin_kick(void *data)
> +static void spin_kick(CPUState *cs, void *data)
>  {
> -    SpinKick *kick = data;
> -    CPUState *cpu = CPU(kick->cpu);
> -    CPUPPCState *env = &kick->cpu->env;
> -    SpinInfo *curspin = kick->spin;
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    SpinInfo *curspin = data;
>      hwaddr map_size = 64 * 1024 * 1024;
>      hwaddr map_start;
>  
> -    cpu_synchronize_state(cpu);
> +    cpu_synchronize_state(cs);
>      stl_p(&curspin->pir, env->spr[SPR_BOOKE_PIR]);
>      env->nip = ldq_p(&curspin->addr) & (map_size - 1);
>      env->gpr[3] = ldq_p(&curspin->r3);
> @@ -112,10 +106,10 @@ static void spin_kick(void *data)
>      map_start = ldq_p(&curspin->addr) & ~(map_size - 1);
>      mmubooke_create_initial_mapping(env, 0, map_start, map_size);
>  
> -    cpu->halted = 0;
> -    cpu->exception_index = -1;
> -    cpu->stopped = false;
> -    qemu_cpu_kick(cpu);
> +    cs->halted = 0;
> +    cs->exception_index = -1;
> +    cs->stopped = false;
> +    qemu_cpu_kick(cs);
>  }
>  
>  static void spin_write(void *opaque, hwaddr addr, uint64_t value,
> @@ -153,12 +147,7 @@ static void spin_write(void *opaque, hwaddr addr, uint64_t value,
>  
>      if (!(ldq_p(&curspin->addr) & 1)) {
>          /* run CPU */
> -        SpinKick kick = {
> -            .cpu = POWERPC_CPU(cpu),
> -            .spin = curspin,
> -        };
> -
> -        run_on_cpu(cpu, spin_kick, &kick);
> +        run_on_cpu(cpu, spin_kick, curspin);
>      }
>  }
>  
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7f33a1b2b57d..882a3c621367 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2170,10 +2170,8 @@ static void spapr_machine_finalizefn(Object *obj)
>      g_free(spapr->kvm_type);
>  }
>  
> -static void ppc_cpu_do_nmi_on_cpu(void *arg)
> +static void ppc_cpu_do_nmi_on_cpu(CPUState *cs, void *arg)
>  {
> -    CPUState *cs = arg;
> -
>      cpu_synchronize_state(cs);
>      ppc_cpu_do_system_reset(cs);
>  }
> @@ -2183,7 +2181,7 @@ static void spapr_nmi(NMIState *n, int cpu_index, Error **errp)
>      CPUState *cs;
>  
>      CPU_FOREACH(cs) {
> -        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, cs);
> +        async_run_on_cpu(cs, ppc_cpu_do_nmi_on_cpu, NULL);
>      }
>  }
>  
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 73af112e1d36..e5eca67f5b71 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -13,19 +13,18 @@
>  #include "kvm_ppc.h"
>  
>  struct SPRSyncState {
> -    CPUState *cs;
>      int spr;
>      target_ulong value;
>      target_ulong mask;
>  };
>  
> -static void do_spr_sync(void *arg)
> +static void do_spr_sync(CPUState *cs, void *arg)
>  {
>      struct SPRSyncState *s = arg;
> -    PowerPCCPU *cpu = POWERPC_CPU(s->cs);
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>      CPUPPCState *env = &cpu->env;
>  
> -    cpu_synchronize_state(s->cs);
> +    cpu_synchronize_state(cs);
>      env->spr[s->spr] &= ~s->mask;
>      env->spr[s->spr] |= s->value;
>  }
> @@ -34,7 +33,6 @@ static void set_spr(CPUState *cs, int spr, target_ulong value,
>                      target_ulong mask)
>  {
>      struct SPRSyncState s = {
> -        .cs = cs,
>          .spr = spr,
>          .value = value,
>          .mask = mask
> @@ -907,17 +905,17 @@ static target_ulong cas_get_option_vector(int vector, target_ulong table)
>  }
>  
>  typedef struct {
> -    PowerPCCPU *cpu;
>      uint32_t cpu_version;
>      Error *err;
>  } SetCompatState;
>  
> -static void do_set_compat(void *arg)
> +static void do_set_compat(CPUState *cs, void *arg)
>  {
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
>      SetCompatState *s = arg;
>  
> -    cpu_synchronize_state(CPU(s->cpu));
> -    ppc_set_compat(s->cpu, s->cpu_version, &s->err);
> +    cpu_synchronize_state(cs);
> +    ppc_set_compat(cpu, s->cpu_version, &s->err);
>  }
>  
>  #define get_compat_level(cpuver) ( \
> @@ -1013,7 +1011,6 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu_,
>      if (old_cpu_version != cpu_version) {
>          CPU_FOREACH(cs) {
>              SetCompatState s = {
> -                .cpu = POWERPC_CPU(cs),
>                  .cpu_version = cpu_version,
>                  .err = NULL,
>              };
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 32f3af3e1c63..4e688f645b4a 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -223,9 +223,11 @@ struct kvm_run;
>  #define TB_JMP_CACHE_SIZE (1 << TB_JMP_CACHE_BITS)
>  
>  /* work queue */
> +typedef void (*run_on_cpu_func)(CPUState *cpu, void *data);
> +
>  struct qemu_work_item {
>      struct qemu_work_item *next;
> -    void (*func)(void *data);
> +    run_on_cpu_func func;
>      void *data;
>      int done;
>      bool free;
> @@ -610,7 +612,7 @@ bool cpu_is_stopped(CPUState *cpu);
>   *
>   * Schedules the function @func for execution on the vCPU @cpu.
>   */
> -void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
> +void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>  
>  /**
>   * async_run_on_cpu:
> @@ -620,7 +622,7 @@ void run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
>   *
>   * Schedules the function @func for execution on the vCPU @cpu asynchronously.
>   */
> -void async_run_on_cpu(CPUState *cpu, void (*func)(void *data), void *data);
> +void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>  
>  /**
>   * qemu_get_cpu:
> diff --git a/kvm-all.c b/kvm-all.c
> index a88f917fda69..a8320574cc8a 100644
> --- a/kvm-all.c
> +++ b/kvm-all.c
> @@ -1828,10 +1828,8 @@ void kvm_flush_coalesced_mmio_buffer(void)
>      s->coalesced_flush_in_progress = false;
>  }
>  
> -static void do_kvm_cpu_synchronize_state(void *arg)
> +static void do_kvm_cpu_synchronize_state(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      if (!cpu->kvm_vcpu_dirty) {
>          kvm_arch_get_registers(cpu);
>          cpu->kvm_vcpu_dirty = true;
> @@ -1841,34 +1839,30 @@ static void do_kvm_cpu_synchronize_state(void *arg)
>  void kvm_cpu_synchronize_state(CPUState *cpu)
>  {
>      if (!cpu->kvm_vcpu_dirty) {
> -        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, cpu);
> +        run_on_cpu(cpu, do_kvm_cpu_synchronize_state, NULL);
>      }
>  }
>  
> -static void do_kvm_cpu_synchronize_post_reset(void *arg)
> +static void do_kvm_cpu_synchronize_post_reset(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      kvm_arch_put_registers(cpu, KVM_PUT_RESET_STATE);
>      cpu->kvm_vcpu_dirty = false;
>  }
>  
>  void kvm_cpu_synchronize_post_reset(CPUState *cpu)
>  {
> -    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, cpu);
> +    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_reset, NULL);
>  }
>  
> -static void do_kvm_cpu_synchronize_post_init(void *arg)
> +static void do_kvm_cpu_synchronize_post_init(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      kvm_arch_put_registers(cpu, KVM_PUT_FULL_STATE);
>      cpu->kvm_vcpu_dirty = false;
>  }
>  
>  void kvm_cpu_synchronize_post_init(CPUState *cpu)
>  {
> -    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, cpu);
> +    run_on_cpu(cpu, do_kvm_cpu_synchronize_post_init, NULL);
>  }
>  
>  int kvm_cpu_exec(CPUState *cpu)
> @@ -2210,7 +2204,7 @@ struct kvm_set_guest_debug_data {
>      int err;
>  };
>  
> -static void kvm_invoke_set_guest_debug(void *data)
> +static void kvm_invoke_set_guest_debug(CPUState *unused_cpu, void *data)
>  {
>      struct kvm_set_guest_debug_data *dbg_data = data;
>  
> @@ -2228,7 +2222,6 @@ int kvm_update_guest_debug(CPUState *cpu, unsigned long reinject_trap)
>          data.dbg.control |= KVM_GUESTDBG_ENABLE | KVM_GUESTDBG_SINGLESTEP;
>      }
>      kvm_arch_update_guest_debug(cpu, &data.dbg);
> -    data.cpu = cpu;
>  
>      run_on_cpu(cpu, kvm_invoke_set_guest_debug, &data);
>      return data.err;
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index 1c250b82452d..9bc961bff342 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -1113,7 +1113,6 @@ out:
>  
>  typedef struct MCEInjectionParams {
>      Monitor *mon;
> -    X86CPU *cpu;
>      int bank;
>      uint64_t status;
>      uint64_t mcg_status;
> @@ -1122,14 +1121,14 @@ typedef struct MCEInjectionParams {
>      int flags;
>  } MCEInjectionParams;
>  
> -static void do_inject_x86_mce(void *data)
> +static void do_inject_x86_mce(CPUState *cs, void *data)
>  {
>      MCEInjectionParams *params = data;
> -    CPUX86State *cenv = &params->cpu->env;
> -    CPUState *cpu = CPU(params->cpu);
> +    X86CPU *cpu = X86_CPU(cs);
> +    CPUX86State *cenv = &cpu->env;
>      uint64_t *banks = cenv->mce_banks + 4 * params->bank;
>  
> -    cpu_synchronize_state(cpu);
> +    cpu_synchronize_state(cs);
>  
>      /*
>       * If there is an MCE exception being processed, ignore this SRAO MCE
> @@ -1149,7 +1148,7 @@ static void do_inject_x86_mce(void *data)
>          if ((cenv->mcg_cap & MCG_CTL_P) && cenv->mcg_ctl != ~(uint64_t)0) {
>              monitor_printf(params->mon,
>                             "CPU %d: Uncorrected error reporting disabled\n",
> -                           cpu->cpu_index);
> +                           cs->cpu_index);
>              return;
>          }
>  
> @@ -1161,7 +1160,7 @@ static void do_inject_x86_mce(void *data)
>              monitor_printf(params->mon,
>                             "CPU %d: Uncorrected error reporting disabled for"
>                             " bank %d\n",
> -                           cpu->cpu_index, params->bank);
> +                           cs->cpu_index, params->bank);
>              return;
>          }
>  
> @@ -1170,7 +1169,7 @@ static void do_inject_x86_mce(void *data)
>              monitor_printf(params->mon,
>                             "CPU %d: Previous MCE still in progress, raising"
>                             " triple fault\n",
> -                           cpu->cpu_index);
> +                           cs->cpu_index);
>              qemu_log_mask(CPU_LOG_RESET, "Triple fault\n");
>              qemu_system_reset_request();
>              return;
> @@ -1182,7 +1181,7 @@ static void do_inject_x86_mce(void *data)
>          banks[3] = params->misc;
>          cenv->mcg_status = params->mcg_status;
>          banks[1] = params->status;
> -        cpu_interrupt(cpu, CPU_INTERRUPT_MCE);
> +        cpu_interrupt(cs, CPU_INTERRUPT_MCE);
>      } else if (!(banks[1] & MCI_STATUS_VAL)
>                 || !(banks[1] & MCI_STATUS_UC)) {
>          if (banks[1] & MCI_STATUS_VAL) {
> @@ -1204,7 +1203,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
>      CPUX86State *cenv = &cpu->env;
>      MCEInjectionParams params = {
>          .mon = mon,
> -        .cpu = cpu,
>          .bank = bank,
>          .status = status,
>          .mcg_status = mcg_status,
> @@ -1245,7 +1243,6 @@ void cpu_x86_inject_mce(Monitor *mon, X86CPU *cpu, int bank,
>              if (other_cs == cs) {
>                  continue;
>              }
> -            params.cpu = X86_CPU(other_cs);
>              run_on_cpu(other_cs, do_inject_x86_mce, &params);
>          }
>      }
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index f3698f19b53b..a1c6e656d13c 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -151,10 +151,8 @@ static int kvm_get_tsc(CPUState *cs)
>      return 0;
>  }
>  
> -static inline void do_kvm_synchronize_tsc(void *arg)
> +static inline void do_kvm_synchronize_tsc(CPUState *cpu, void *arg)
>  {
> -    CPUState *cpu = arg;
> -
>      kvm_get_tsc(cpu);
>  }
>  
> @@ -164,7 +162,7 @@ void kvm_synchronize_all_tsc(void)
>  
>      if (kvm_enabled()) {
>          CPU_FOREACH(cpu) {
> -            run_on_cpu(cpu, do_kvm_synchronize_tsc, cpu);
> +            run_on_cpu(cpu, do_kvm_synchronize_tsc, NULL);
>          }
>      }
>  }
> diff --git a/target-s390x/cpu.c b/target-s390x/cpu.c
> index e43e2d61550b..4f09c647b7a8 100644
> --- a/target-s390x/cpu.c
> +++ b/target-s390x/cpu.c
> @@ -188,7 +188,7 @@ static void s390_cpu_machine_reset_cb(void *opaque)
>  {
>      S390CPU *cpu = opaque;
>  
> -    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, CPU(cpu));
> +    run_on_cpu(CPU(cpu), s390_do_cpu_full_reset, NULL);
>  }
>  #endif
>  
> @@ -238,7 +238,7 @@ static void s390_cpu_realizefn(DeviceState *dev, Error **errp)
>      s390_cpu_gdb_init(cs);
>      qemu_init_vcpu(cs);
>  #if !defined(CONFIG_USER_ONLY)
> -    run_on_cpu(cs, s390_do_cpu_full_reset, cs);
> +    run_on_cpu(cs, s390_do_cpu_full_reset, NULL);
>  #else
>      cpu_reset(cs);
>  #endif
> diff --git a/target-s390x/cpu.h b/target-s390x/cpu.h
> index 8bcb0f75f399..8998ce21ca13 100644
> --- a/target-s390x/cpu.h
> +++ b/target-s390x/cpu.h
> @@ -498,17 +498,14 @@ static inline hwaddr decode_basedisp_s(CPUS390XState *env, uint32_t ipb,
>  #define decode_basedisp_rs decode_basedisp_s
>  
>  /* helper functions for run_on_cpu() */
> -static inline void s390_do_cpu_reset(void *arg)
> +static inline void s390_do_cpu_reset(CPUState *cs, void *arg)
>  {
> -    CPUState *cs = arg;
>      S390CPUClass *scc = S390_CPU_GET_CLASS(cs);
>  
>      scc->cpu_reset(cs);
>  }
> -static inline void s390_do_cpu_full_reset(void *arg)
> +static inline void s390_do_cpu_full_reset(CPUState *cs, void *arg)
>  {
> -    CPUState *cs = arg;
> -
>      cpu_reset(cs);
>  }
>  
> diff --git a/target-s390x/kvm.c b/target-s390x/kvm.c
> index 45e94ca48abd..b37eb9d3cd8c 100644
> --- a/target-s390x/kvm.c
> +++ b/target-s390x/kvm.c
> @@ -1354,7 +1354,6 @@ static int handle_diag(S390CPU *cpu, struct kvm_run *run, uint32_t ipb)
>  }
>  
>  typedef struct SigpInfo {
> -    S390CPU *cpu;
>      uint64_t param;
>      int cc;
>      uint64_t *status_reg;
> @@ -1367,38 +1366,40 @@ static void set_sigp_status(SigpInfo *si, uint64_t status)
>      si->cc = SIGP_CC_STATUS_STORED;
>  }
>  
> -static void sigp_start(void *arg)
> +static void sigp_start(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>  
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>          return;
>      }
>  
> -    s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
> +    s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_stop(void *arg)
> +static void sigp_stop(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      struct kvm_s390_irq irq = {
>          .type = KVM_S390_SIGP_STOP,
>      };
>  
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_OPERATING) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_OPERATING) {
>          si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>          return;
>      }
>  
>      /* disabled wait - sleeping in user space */
> -    if (CPU(si->cpu)->halted) {
> -        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
> +    if (cs->halted) {
> +        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>      } else {
>          /* execute the stop function */
> -        si->cpu->env.sigp_order = SIGP_STOP;
> -        kvm_s390_vcpu_interrupt(si->cpu, &irq);
> +        cpu->env.sigp_order = SIGP_STOP;
> +        kvm_s390_vcpu_interrupt(cpu, &irq);
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
> @@ -1465,56 +1466,58 @@ static int kvm_s390_store_status(S390CPU *cpu, hwaddr addr, bool store_arch)
>      return 0;
>  }
>  
> -static void sigp_stop_and_store_status(void *arg)
> +static void sigp_stop_and_store_status(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      struct kvm_s390_irq irq = {
>          .type = KVM_S390_SIGP_STOP,
>      };
>  
>      /* disabled wait - sleeping in user space */
> -    if (s390_cpu_get_state(si->cpu) == CPU_STATE_OPERATING &&
> -        CPU(si->cpu)->halted) {
> -        s390_cpu_set_state(CPU_STATE_STOPPED, si->cpu);
> +    if (s390_cpu_get_state(cpu) == CPU_STATE_OPERATING && cs->halted) {
> +        s390_cpu_set_state(CPU_STATE_STOPPED, cpu);
>      }
>  
> -    switch (s390_cpu_get_state(si->cpu)) {
> +    switch (s390_cpu_get_state(cpu)) {
>      case CPU_STATE_OPERATING:
> -        si->cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
> -        kvm_s390_vcpu_interrupt(si->cpu, &irq);
> +        cpu->env.sigp_order = SIGP_STOP_STORE_STATUS;
> +        kvm_s390_vcpu_interrupt(cpu, &irq);
>          /* store will be performed when handling the stop intercept */
>          break;
>      case CPU_STATE_STOPPED:
>          /* already stopped, just store the status */
> -        cpu_synchronize_state(CPU(si->cpu));
> -        kvm_s390_store_status(si->cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
> +        cpu_synchronize_state(cs);
> +        kvm_s390_store_status(cpu, KVM_S390_STORE_STATUS_DEF_ADDR, true);
>          break;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_store_status_at_address(void *arg)
> +static void sigp_store_status_at_address(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      uint32_t address = si->param & 0x7ffffe00u;
>  
>      /* cpu has to be stopped */
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
>          return;
>      }
>  
> -    cpu_synchronize_state(CPU(si->cpu));
> +    cpu_synchronize_state(cs);
>  
> -    if (kvm_s390_store_status(si->cpu, address, false)) {
> +    if (kvm_s390_store_status(cpu, address, false)) {
>          set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
>          return;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_store_adtl_status(void *arg)
> +static void sigp_store_adtl_status(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>  
>      if (!kvm_check_extension(kvm_state, KVM_CAP_S390_VECTOR_REGISTERS)) {
> @@ -1523,7 +1526,7 @@ static void sigp_store_adtl_status(void *arg)
>      }
>  
>      /* cpu has to be stopped */
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
>          return;
>      }
> @@ -1534,31 +1537,32 @@ static void sigp_store_adtl_status(void *arg)
>          return;
>      }
>  
> -    cpu_synchronize_state(CPU(si->cpu));
> +    cpu_synchronize_state(cs);
>  
> -    if (kvm_s390_store_adtl_status(si->cpu, si->param)) {
> +    if (kvm_s390_store_adtl_status(cpu, si->param)) {
>          set_sigp_status(si, SIGP_STAT_INVALID_PARAMETER);
>          return;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_restart(void *arg)
> +static void sigp_restart(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      struct kvm_s390_irq irq = {
>          .type = KVM_S390_RESTART,
>      };
>  
> -    switch (s390_cpu_get_state(si->cpu)) {
> +    switch (s390_cpu_get_state(cpu)) {
>      case CPU_STATE_STOPPED:
>          /* the restart irq has to be delivered prior to any other pending irq */
> -        cpu_synchronize_state(CPU(si->cpu));
> -        do_restart_interrupt(&si->cpu->env);
> -        s390_cpu_set_state(CPU_STATE_OPERATING, si->cpu);
> +        cpu_synchronize_state(cs);
> +        do_restart_interrupt(&cpu->env);
> +        s390_cpu_set_state(CPU_STATE_OPERATING, cpu);
>          break;
>      case CPU_STATE_OPERATING:
> -        kvm_s390_vcpu_interrupt(si->cpu, &irq);
> +        kvm_s390_vcpu_interrupt(cpu, &irq);
>          break;
>      }
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
> @@ -1566,20 +1570,18 @@ static void sigp_restart(void *arg)
>  
>  int kvm_s390_cpu_restart(S390CPU *cpu)
>  {
> -    SigpInfo si = {
> -        .cpu = cpu,
> -    };
> +    SigpInfo si = {};
>  
>      run_on_cpu(CPU(cpu), sigp_restart, &si);
>      DPRINTF("DONE: KVM cpu restart: %p\n", &cpu->env);
>      return 0;
>  }
>  
> -static void sigp_initial_cpu_reset(void *arg)
> +static void sigp_initial_cpu_reset(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
> +    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
>      SigpInfo *si = arg;
> -    CPUState *cs = CPU(si->cpu);
> -    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
>  
>      cpu_synchronize_state(cs);
>      scc->initial_cpu_reset(cs);
> @@ -1587,11 +1589,11 @@ static void sigp_initial_cpu_reset(void *arg)
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_cpu_reset(void *arg)
> +static void sigp_cpu_reset(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
> +    S390CPUClass *scc = S390_CPU_GET_CLASS(cpu);
>      SigpInfo *si = arg;
> -    CPUState *cs = CPU(si->cpu);
> -    S390CPUClass *scc = S390_CPU_GET_CLASS(si->cpu);
>  
>      cpu_synchronize_state(cs);
>      scc->cpu_reset(cs);
> @@ -1599,12 +1601,13 @@ static void sigp_cpu_reset(void *arg)
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> -static void sigp_set_prefix(void *arg)
> +static void sigp_set_prefix(CPUState *cs, void *arg)
>  {
> +    S390CPU *cpu = S390_CPU(cs);
>      SigpInfo *si = arg;
>      uint32_t addr = si->param & 0x7fffe000u;
>  
> -    cpu_synchronize_state(CPU(si->cpu));
> +    cpu_synchronize_state(cs);
>  
>      if (!address_space_access_valid(&address_space_memory, addr,
>                                      sizeof(struct LowCore), false)) {
> @@ -1613,13 +1616,13 @@ static void sigp_set_prefix(void *arg)
>      }
>  
>      /* cpu has to be stopped */
> -    if (s390_cpu_get_state(si->cpu) != CPU_STATE_STOPPED) {
> +    if (s390_cpu_get_state(cpu) != CPU_STATE_STOPPED) {
>          set_sigp_status(si, SIGP_STAT_INCORRECT_STATE);
>          return;
>      }
>  
> -    si->cpu->env.psa = addr;
> -    cpu_synchronize_post_init(CPU(si->cpu));
> +    cpu->env.psa = addr;
> +    cpu_synchronize_post_init(cs);
>      si->cc = SIGP_CC_ORDER_CODE_ACCEPTED;
>  }
>  
> @@ -1627,7 +1630,6 @@ static int handle_sigp_single_dst(S390CPU *dst_cpu, uint8_t order,
>                                    uint64_t param, uint64_t *status_reg)
>  {
>      SigpInfo si = {
> -        .cpu = dst_cpu,
>          .param = param,
>          .status_reg = status_reg,
>      };
> diff --git a/target-s390x/misc_helper.c b/target-s390x/misc_helper.c
> index 86da1947b984..4df2ec6c7dab 100644
> --- a/target-s390x/misc_helper.c
> +++ b/target-s390x/misc_helper.c
> @@ -126,7 +126,7 @@ static int modified_clear_reset(S390CPU *cpu)
>      pause_all_vcpus();
>      cpu_synchronize_all_states();
>      CPU_FOREACH(t) {
> -        run_on_cpu(t, s390_do_cpu_full_reset, t);
> +        run_on_cpu(t, s390_do_cpu_full_reset, NULL);
>      }
>      s390_cmma_reset();
>      subsystem_reset();
> @@ -145,7 +145,7 @@ static int load_normal_reset(S390CPU *cpu)
>      pause_all_vcpus();
>      cpu_synchronize_all_states();
>      CPU_FOREACH(t) {
> -        run_on_cpu(t, s390_do_cpu_reset, t);
> +        run_on_cpu(t, s390_do_cpu_reset, NULL);
>      }
>      s390_cmma_reset();
>      subsystem_reset();

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe Sergey Fedorov
@ 2016-07-07 20:11   ` Sergey Fedorov
  2016-07-14  8:41   ` Alex Bennée
  1 sibling, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-07 20:11 UTC (permalink / raw)
  To: Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite

On 07/07/16 00:15, Sergey Fedorov wrote:
> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Use async_safe_run_on_cpu() to make tb_flush() thread safe.

I've just realized that this allows to remove CPUState::tb_flushed as well.

Regards,
Sergey

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 04/11] cpus: Wrap mutex used to protect CPU work
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 04/11] cpus: Wrap mutex used to protect CPU work Sergey Fedorov
@ 2016-07-11 12:06   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-11 12:06 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Peter Crosthwaite


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> This will be useful to enable CPU work on user mode emulation.
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  cpus.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/cpus.c b/cpus.c
> index 04687c85bcd4..f80ed2aeefdd 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -910,6 +910,11 @@ void qemu_init_cpu_loop(void)
>      qemu_thread_get_self(&io_thread);
>  }
>
> +static QemuMutex *qemu_get_cpu_work_mutex(void)
> +{
> +    return &qemu_global_mutex;
> +}
> +
>  static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
>  {
>      qemu_mutex_lock(&cpu->work_mutex);
> @@ -943,7 +948,7 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>      while (!atomic_mb_read(&wi.done)) {
>          CPUState *self_cpu = current_cpu;
>
> -        qemu_cond_wait(&qemu_work_cond, &qemu_global_mutex);
> +        qemu_cond_wait(&qemu_work_cond, qemu_get_cpu_work_mutex());
>          current_cpu = self_cpu;
>      }
>  }


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 05/11] cpus: Rename flush_queued_work()
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 05/11] cpus: Rename flush_queued_work() Sergey Fedorov
@ 2016-07-11 12:07   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-11 12:07 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Peter Crosthwaite


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> To avoid possible confusion, rename flush_queued_work() to
> process_queued_cpu_work().
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  cpus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/cpus.c b/cpus.c
> index f80ed2aeefdd..51fd8c18b4c8 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -982,7 +982,7 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu)
>  {
>  }
>
> -static void flush_queued_work(CPUState *cpu)
> +static void process_queued_cpu_work(CPUState *cpu)
>  {
>      struct qemu_work_item *wi;
>
> @@ -1017,7 +1017,7 @@ static void qemu_wait_io_event_common(CPUState *cpu)
>          cpu->stopped = true;
>          qemu_cond_broadcast(&qemu_pause_cond);
>      }
> -    flush_queued_work(cpu);
> +    process_queued_cpu_work(cpu);
>      cpu->thread_kicked = false;
>  }


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 06/11] linux-user: Use QemuMutex and QemuCond
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 06/11] linux-user: Use QemuMutex and QemuCond Sergey Fedorov
@ 2016-07-11 12:08   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-11 12:08 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Riku Voipio


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Convert pthread_mutex_t and pthread_cond_t to QemuMutex and QemuCond.
> This will allow to make some locks and conditional variables common
> between user and system mode emulation.
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  linux-user/main.c | 53 +++++++++++++++++++++++++++++++----------------------
>  1 file changed, 31 insertions(+), 22 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 617a179f14a4..bdbda693cc5f 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -108,17 +108,25 @@ int cpu_get_pic_interrupt(CPUX86State *env)
>     We don't require a full sync, only that no cpus are executing guest code.
>     The alternative is to map target atomic ops onto host equivalents,
>     which requires quite a lot of per host/target work.  */
> -static pthread_mutex_t cpu_list_mutex = PTHREAD_MUTEX_INITIALIZER;
> -static pthread_mutex_t exclusive_lock = PTHREAD_MUTEX_INITIALIZER;
> -static pthread_cond_t exclusive_cond = PTHREAD_COND_INITIALIZER;
> -static pthread_cond_t exclusive_resume = PTHREAD_COND_INITIALIZER;
> +static QemuMutex cpu_list_mutex;
> +static QemuMutex exclusive_lock;
> +static QemuCond exclusive_cond;
> +static QemuCond exclusive_resume;
>  static int pending_cpus;
>
> +void qemu_init_cpu_loop(void)
> +{
> +    qemu_mutex_init(&cpu_list_mutex);
> +    qemu_mutex_init(&exclusive_lock);
> +    qemu_cond_init(&exclusive_cond);
> +    qemu_cond_init(&exclusive_resume);
> +}
> +
>  /* Make sure everything is in a consistent state for calling fork().  */
>  void fork_start(void)
>  {
>      qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
> -    pthread_mutex_lock(&exclusive_lock);
> +    qemu_mutex_lock(&exclusive_lock);
>      mmap_fork_start();
>  }
>
> @@ -135,14 +143,14 @@ void fork_end(int child)
>              }
>          }
>          pending_cpus = 0;
> -        pthread_mutex_init(&exclusive_lock, NULL);
> -        pthread_mutex_init(&cpu_list_mutex, NULL);
> -        pthread_cond_init(&exclusive_cond, NULL);
> -        pthread_cond_init(&exclusive_resume, NULL);
> +        qemu_mutex_init(&exclusive_lock);
> +        qemu_mutex_init(&cpu_list_mutex);
> +        qemu_cond_init(&exclusive_cond);
> +        qemu_cond_init(&exclusive_resume);
>          qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
>          gdbserver_fork(thread_cpu);
>      } else {
> -        pthread_mutex_unlock(&exclusive_lock);
> +        qemu_mutex_unlock(&exclusive_lock);
>          qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
>      }
>  }
> @@ -152,7 +160,7 @@ void fork_end(int child)
>  static inline void exclusive_idle(void)
>  {
>      while (pending_cpus) {
> -        pthread_cond_wait(&exclusive_resume, &exclusive_lock);
> +        qemu_cond_wait(&exclusive_resume, &exclusive_lock);
>      }
>  }
>
> @@ -162,7 +170,7 @@ static inline void start_exclusive(void)
>  {
>      CPUState *other_cpu;
>
> -    pthread_mutex_lock(&exclusive_lock);
> +    qemu_mutex_lock(&exclusive_lock);
>      exclusive_idle();
>
>      pending_cpus = 1;
> @@ -174,7 +182,7 @@ static inline void start_exclusive(void)
>          }
>      }
>      if (pending_cpus > 1) {
> -        pthread_cond_wait(&exclusive_cond, &exclusive_lock);
> +        qemu_cond_wait(&exclusive_cond, &exclusive_lock);
>      }
>  }
>
> @@ -182,42 +190,42 @@ static inline void start_exclusive(void)
>  static inline void __attribute__((unused)) end_exclusive(void)
>  {
>      pending_cpus = 0;
> -    pthread_cond_broadcast(&exclusive_resume);
> -    pthread_mutex_unlock(&exclusive_lock);
> +    qemu_cond_broadcast(&exclusive_resume);
> +    qemu_mutex_unlock(&exclusive_lock);
>  }
>
>  /* Wait for exclusive ops to finish, and begin cpu execution.  */
>  static inline void cpu_exec_start(CPUState *cpu)
>  {
> -    pthread_mutex_lock(&exclusive_lock);
> +    qemu_mutex_lock(&exclusive_lock);
>      exclusive_idle();
>      cpu->running = true;
> -    pthread_mutex_unlock(&exclusive_lock);
> +    qemu_mutex_unlock(&exclusive_lock);
>  }
>
>  /* Mark cpu as not executing, and release pending exclusive ops.  */
>  static inline void cpu_exec_end(CPUState *cpu)
>  {
> -    pthread_mutex_lock(&exclusive_lock);
> +    qemu_mutex_lock(&exclusive_lock);
>      cpu->running = false;
>      if (pending_cpus > 1) {
>          pending_cpus--;
>          if (pending_cpus == 1) {
> -            pthread_cond_signal(&exclusive_cond);
> +            qemu_cond_signal(&exclusive_cond);
>          }
>      }
>      exclusive_idle();
> -    pthread_mutex_unlock(&exclusive_lock);
> +    qemu_mutex_unlock(&exclusive_lock);
>  }
>
>  void cpu_list_lock(void)
>  {
> -    pthread_mutex_lock(&cpu_list_mutex);
> +    qemu_mutex_lock(&cpu_list_mutex);
>  }
>
>  void cpu_list_unlock(void)
>  {
> -    pthread_mutex_unlock(&cpu_list_mutex);
> +    qemu_mutex_unlock(&cpu_list_mutex);
>  }
>
>
> @@ -4210,6 +4218,7 @@ int main(int argc, char **argv, char **envp)
>      int ret;
>      int execfd;
>
> +    qemu_init_cpu_loop();
>      module_call_init(MODULE_INIT_QOM);
>
>      if ((envlist = envlist_create()) == NULL) {


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
  2016-07-06 21:14   ` [Qemu-devel] " Sergey Fedorov
@ 2016-07-11 12:36     ` Christian Borntraeger
  -1 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2016-07-11 12:36 UTC (permalink / raw)
  To: Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, David Gibson,
	Alexander Graf, Marcelo Tosatti, Cornelia Huck, qemu-ppc, kvm

On 07/06/2016 11:14 PM, Sergey Fedorov wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> CPUState is a fairly common pointer to pass to these helpers. This means
> if you need other arguments for the async_run_on_cpu case you end up
> having to do a g_malloc to stuff additional data into the routine. For
> the current users this isn't a massive deal but for MTTCG this gets
> cumbersome when the only other parameter is often an address.
> 
> This adds the typedef run_on_cpu_func for helper functions which has an
> explicit CPUState * passed as the first parameter. All the users of
> run_on_cpu and async_run_on_cpu have had their helpers updated to use
> CPUState where available.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> [Sergey Fedorov:
>  - eliminate more CPUState in user data;
>  - remove unnecessary user data passing;
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
> 
> Changes in v2:
>  - eliminate more CPUState in user data
>  - remove unnecessary user data passing
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
> 
> ---
>  cpus.c                     | 15 ++++---
>  hw/i386/kvm/apic.c         |  3 +-
>  hw/i386/kvmvapic.c         |  6 +--
>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>  hw/ppc/spapr.c             |  6 +--
>  hw/ppc/spapr_hcall.c       | 17 ++++----
>  include/qom/cpu.h          |  8 ++--
>  kvm-all.c                  | 21 ++++------
>  target-i386/helper.c       | 19 ++++-----
>  target-i386/kvm.c          |  6 +--
>  target-s390x/cpu.c         |  4 +-
>  target-s390x/cpu.h         |  7 +---
>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>  target-s390x/misc_helper.c |  4 +-
>  14 files changed, 108 insertions(+), 137 deletions(-)

s390 parts look ok.



^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
@ 2016-07-11 12:36     ` Christian Borntraeger
  0 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2016-07-11 12:36 UTC (permalink / raw)
  To: Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, David Gibson,
	Alexander Graf, Marcelo Tosatti, Cornelia Huck, qemu-ppc, kvm

On 07/06/2016 11:14 PM, Sergey Fedorov wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> CPUState is a fairly common pointer to pass to these helpers. This means
> if you need other arguments for the async_run_on_cpu case you end up
> having to do a g_malloc to stuff additional data into the routine. For
> the current users this isn't a massive deal but for MTTCG this gets
> cumbersome when the only other parameter is often an address.
> 
> This adds the typedef run_on_cpu_func for helper functions which has an
> explicit CPUState * passed as the first parameter. All the users of
> run_on_cpu and async_run_on_cpu have had their helpers updated to use
> CPUState where available.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> [Sergey Fedorov:
>  - eliminate more CPUState in user data;
>  - remove unnecessary user data passing;
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
> 
> Changes in v2:
>  - eliminate more CPUState in user data
>  - remove unnecessary user data passing
>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
> 
> ---
>  cpus.c                     | 15 ++++---
>  hw/i386/kvm/apic.c         |  3 +-
>  hw/i386/kvmvapic.c         |  6 +--
>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>  hw/ppc/spapr.c             |  6 +--
>  hw/ppc/spapr_hcall.c       | 17 ++++----
>  include/qom/cpu.h          |  8 ++--
>  kvm-all.c                  | 21 ++++------
>  target-i386/helper.c       | 19 ++++-----
>  target-i386/kvm.c          |  6 +--
>  target-s390x/cpu.c         |  4 +-
>  target-s390x/cpu.h         |  7 +---
>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>  target-s390x/misc_helper.c |  4 +-
>  14 files changed, 108 insertions(+), 137 deletions(-)

s390 parts look ok.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
  2016-07-11 12:36     ` [Qemu-devel] " Christian Borntraeger
@ 2016-07-11 12:38       ` Sergey Fedorov
  -1 siblings, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-11 12:38 UTC (permalink / raw)
  To: Christian Borntraeger, Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, David Gibson,
	Alexander Graf, Marcelo Tosatti, Cornelia Huck, qemu-ppc, kvm

On 11/07/16 15:36, Christian Borntraeger wrote:
> On 07/06/2016 11:14 PM, Sergey Fedorov wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>>
>> CPUState is a fairly common pointer to pass to these helpers. This means
>> if you need other arguments for the async_run_on_cpu case you end up
>> having to do a g_malloc to stuff additional data into the routine. For
>> the current users this isn't a massive deal but for MTTCG this gets
>> cumbersome when the only other parameter is often an address.
>>
>> This adds the typedef run_on_cpu_func for helper functions which has an
>> explicit CPUState * passed as the first parameter. All the users of
>> run_on_cpu and async_run_on_cpu have had their helpers updated to use
>> CPUState where available.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> [Sergey Fedorov:
>>  - eliminate more CPUState in user data;
>>  - remove unnecessary user data passing;
>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>> ---
>>
>> Changes in v2:
>>  - eliminate more CPUState in user data
>>  - remove unnecessary user data passing
>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
>>
>> ---
>>  cpus.c                     | 15 ++++---
>>  hw/i386/kvm/apic.c         |  3 +-
>>  hw/i386/kvmvapic.c         |  6 +--
>>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>>  hw/ppc/spapr.c             |  6 +--
>>  hw/ppc/spapr_hcall.c       | 17 ++++----
>>  include/qom/cpu.h          |  8 ++--
>>  kvm-all.c                  | 21 ++++------
>>  target-i386/helper.c       | 19 ++++-----
>>  target-i386/kvm.c          |  6 +--
>>  target-s390x/cpu.c         |  4 +-
>>  target-s390x/cpu.h         |  7 +---
>>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>>  target-s390x/misc_helper.c |  4 +-
>>  14 files changed, 108 insertions(+), 137 deletions(-)
> s390 parts look ok.

Can this be considered as "Acked-by" or not exactly?

Thanks,
Sergey

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
@ 2016-07-11 12:38       ` Sergey Fedorov
  0 siblings, 0 replies; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-11 12:38 UTC (permalink / raw)
  To: Christian Borntraeger, Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, David Gibson,
	Alexander Graf, Marcelo Tosatti, Cornelia Huck, qemu-ppc, kvm

On 11/07/16 15:36, Christian Borntraeger wrote:
> On 07/06/2016 11:14 PM, Sergey Fedorov wrote:
>> From: Alex Bennée <alex.bennee@linaro.org>
>>
>> CPUState is a fairly common pointer to pass to these helpers. This means
>> if you need other arguments for the async_run_on_cpu case you end up
>> having to do a g_malloc to stuff additional data into the routine. For
>> the current users this isn't a massive deal but for MTTCG this gets
>> cumbersome when the only other parameter is often an address.
>>
>> This adds the typedef run_on_cpu_func for helper functions which has an
>> explicit CPUState * passed as the first parameter. All the users of
>> run_on_cpu and async_run_on_cpu have had their helpers updated to use
>> CPUState where available.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> [Sergey Fedorov:
>>  - eliminate more CPUState in user data;
>>  - remove unnecessary user data passing;
>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>> ---
>>
>> Changes in v2:
>>  - eliminate more CPUState in user data
>>  - remove unnecessary user data passing
>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
>>
>> ---
>>  cpus.c                     | 15 ++++---
>>  hw/i386/kvm/apic.c         |  3 +-
>>  hw/i386/kvmvapic.c         |  6 +--
>>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>>  hw/ppc/spapr.c             |  6 +--
>>  hw/ppc/spapr_hcall.c       | 17 ++++----
>>  include/qom/cpu.h          |  8 ++--
>>  kvm-all.c                  | 21 ++++------
>>  target-i386/helper.c       | 19 ++++-----
>>  target-i386/kvm.c          |  6 +--
>>  target-s390x/cpu.c         |  4 +-
>>  target-s390x/cpu.h         |  7 +---
>>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>>  target-s390x/misc_helper.c |  4 +-
>>  14 files changed, 108 insertions(+), 137 deletions(-)
> s390 parts look ok.

Can this be considered as "Acked-by" or not exactly?

Thanks,
Sergey

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
  2016-07-11 12:38       ` [Qemu-devel] " Sergey Fedorov
@ 2016-07-11 12:55         ` Christian Borntraeger
  -1 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2016-07-11 12:55 UTC (permalink / raw)
  To: Sergey Fedorov, Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, David Gibson,
	Alexander Graf, Marcelo Tosatti, Cornelia Huck, qemu-ppc, kvm

On 07/11/2016 02:38 PM, Sergey Fedorov wrote:
> On 11/07/16 15:36, Christian Borntraeger wrote:
>> On 07/06/2016 11:14 PM, Sergey Fedorov wrote:
>>> From: Alex Bennée <alex.bennee@linaro.org>
>>>
>>> CPUState is a fairly common pointer to pass to these helpers. This means
>>> if you need other arguments for the async_run_on_cpu case you end up
>>> having to do a g_malloc to stuff additional data into the routine. For
>>> the current users this isn't a massive deal but for MTTCG this gets
>>> cumbersome when the only other parameter is often an address.
>>>
>>> This adds the typedef run_on_cpu_func for helper functions which has an
>>> explicit CPUState * passed as the first parameter. All the users of
>>> run_on_cpu and async_run_on_cpu have had their helpers updated to use
>>> CPUState where available.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> [Sergey Fedorov:
>>>  - eliminate more CPUState in user data;
>>>  - remove unnecessary user data passing;
>>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
>>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>>> ---
>>>
>>> Changes in v2:
>>>  - eliminate more CPUState in user data
>>>  - remove unnecessary user data passing
>>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
>>>
>>> ---
>>>  cpus.c                     | 15 ++++---
>>>  hw/i386/kvm/apic.c         |  3 +-
>>>  hw/i386/kvmvapic.c         |  6 +--
>>>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>>>  hw/ppc/spapr.c             |  6 +--
>>>  hw/ppc/spapr_hcall.c       | 17 ++++----
>>>  include/qom/cpu.h          |  8 ++--
>>>  kvm-all.c                  | 21 ++++------
>>>  target-i386/helper.c       | 19 ++++-----
>>>  target-i386/kvm.c          |  6 +--
>>>  target-s390x/cpu.c         |  4 +-
>>>  target-s390x/cpu.h         |  7 +---
>>>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>>>  target-s390x/misc_helper.c |  4 +-
>>>  14 files changed, 108 insertions(+), 137 deletions(-)
>> s390 parts look ok.
> 
> Can this be considered as "Acked-by" or not exactly?

I looked through all changes, so maybe an Reviewed-by (s390 parts) is ok.


^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers
@ 2016-07-11 12:55         ` Christian Borntraeger
  0 siblings, 0 replies; 33+ messages in thread
From: Christian Borntraeger @ 2016-07-11 12:55 UTC (permalink / raw)
  To: Sergey Fedorov, Sergey Fedorov, qemu-devel
  Cc: MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Alex Bennée, Paolo Bonzini,
	Richard Henderson, Peter Maydell, Peter Crosthwaite,
	Eduardo Habkost, Michael S. Tsirkin, David Gibson,
	Alexander Graf, Marcelo Tosatti, Cornelia Huck, qemu-ppc, kvm

On 07/11/2016 02:38 PM, Sergey Fedorov wrote:
> On 11/07/16 15:36, Christian Borntraeger wrote:
>> On 07/06/2016 11:14 PM, Sergey Fedorov wrote:
>>> From: Alex Bennée <alex.bennee@linaro.org>
>>>
>>> CPUState is a fairly common pointer to pass to these helpers. This means
>>> if you need other arguments for the async_run_on_cpu case you end up
>>> having to do a g_malloc to stuff additional data into the routine. For
>>> the current users this isn't a massive deal but for MTTCG this gets
>>> cumbersome when the only other parameter is often an address.
>>>
>>> This adds the typedef run_on_cpu_func for helper functions which has an
>>> explicit CPUState * passed as the first parameter. All the users of
>>> run_on_cpu and async_run_on_cpu have had their helpers updated to use
>>> CPUState where available.
>>>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> [Sergey Fedorov:
>>>  - eliminate more CPUState in user data;
>>>  - remove unnecessary user data passing;
>>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c]
>>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>>> ---
>>>
>>> Changes in v2:
>>>  - eliminate more CPUState in user data
>>>  - remove unnecessary user data passing
>>>  - fix target-s390x/kvm.c and target-s390x/misc_helper.c
>>>
>>> ---
>>>  cpus.c                     | 15 ++++---
>>>  hw/i386/kvm/apic.c         |  3 +-
>>>  hw/i386/kvmvapic.c         |  6 +--
>>>  hw/ppc/ppce500_spin.c      | 31 +++++----------
>>>  hw/ppc/spapr.c             |  6 +--
>>>  hw/ppc/spapr_hcall.c       | 17 ++++----
>>>  include/qom/cpu.h          |  8 ++--
>>>  kvm-all.c                  | 21 ++++------
>>>  target-i386/helper.c       | 19 ++++-----
>>>  target-i386/kvm.c          |  6 +--
>>>  target-s390x/cpu.c         |  4 +-
>>>  target-s390x/cpu.h         |  7 +---
>>>  target-s390x/kvm.c         | 98 +++++++++++++++++++++++-----------------------
>>>  target-s390x/misc_helper.c |  4 +-
>>>  14 files changed, 108 insertions(+), 137 deletions(-)
>> s390 parts look ok.
> 
> Can this be considered as "Acked-by" or not exactly?

I looked through all changes, so maybe an Reviewed-by (s390 parts) is ok.

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe Sergey Fedorov
  2016-07-07 20:11   ` Sergey Fedorov
@ 2016-07-14  8:41   ` Alex Bennée
  2016-07-14  8:54     ` Sergey Fedorov
  1 sibling, 1 reply; 33+ messages in thread
From: Alex Bennée @ 2016-07-14  8:41 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Peter Crosthwaite


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Use async_safe_run_on_cpu() to make tb_flush() thread safe.
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
>
> Changes in v2:
>  - stale comment about unsafe tb_flush() removed
> ---
>  translate-all.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/translate-all.c b/translate-all.c
> index eaa95e4cd7dc..e69b5d4e889e 100644
> --- a/translate-all.c
> +++ b/translate-all.c
> @@ -831,8 +831,7 @@ static void page_flush_tb(void)
>  }
>
>  /* flush all the translation blocks */
> -/* XXX: tb_flush is currently not thread safe */
> -void tb_flush(CPUState *cpu)
> +static void do_tb_flush(CPUState *cpu, void *data)
>  {
>  #if defined(DEBUG_FLUSH)
>      printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
> @@ -861,6 +860,11 @@ void tb_flush(CPUState *cpu)
>      tcg_ctx.tb_ctx.tb_flush_count++;
>  }
>
> +void tb_flush(CPUState *cpu)
> +{
> +    async_safe_run_on_cpu(cpu, do_tb_flush, NULL);
> +}
> +
>  #ifdef DEBUG_TB_CHECK
>
>  static void
> @@ -1163,9 +1167,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>   buffer_overflow:
>          /* flush must be done */
>          tb_flush(cpu);
> -        /* cannot fail at this point */
> -        tb = tb_alloc(pc);
> -        assert(tb != NULL);
> +        mmap_unlock();
> +        cpu_loop_exit(cpu);

Given our other discussions about lock resetting I wonder if this is
another case where mmap_reset() could be called on cpu_loop_exit?

>      }
>
>      gen_code_buf = tcg_ctx.code_gen_ptr;

Otherwise so far the testing is looking pretty positive in linux-user:

Tested-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe
  2016-07-14  8:41   ` Alex Bennée
@ 2016-07-14  8:54     ` Sergey Fedorov
  2016-07-14  9:49       ` Alex Bennée
  0 siblings, 1 reply; 33+ messages in thread
From: Sergey Fedorov @ 2016-07-14  8:54 UTC (permalink / raw)
  To: Alex Bennée, Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Peter Crosthwaite

On 14/07/16 11:41, Alex Bennée wrote:
> Sergey Fedorov <sergey.fedorov@linaro.org> writes:
>
>> From: Sergey Fedorov <serge.fdrv@gmail.com>
>>
>> Use async_safe_run_on_cpu() to make tb_flush() thread safe.
>>
>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>> ---
>>
>> Changes in v2:
>>  - stale comment about unsafe tb_flush() removed
>> ---
>>  translate-all.c | 13 ++++++++-----
>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/translate-all.c b/translate-all.c
>> index eaa95e4cd7dc..e69b5d4e889e 100644
>> --- a/translate-all.c
>> +++ b/translate-all.c
>> @@ -831,8 +831,7 @@ static void page_flush_tb(void)
>>  }
>>
>>  /* flush all the translation blocks */
>> -/* XXX: tb_flush is currently not thread safe */
>> -void tb_flush(CPUState *cpu)
>> +static void do_tb_flush(CPUState *cpu, void *data)
>>  {
>>  #if defined(DEBUG_FLUSH)
>>      printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
>> @@ -861,6 +860,11 @@ void tb_flush(CPUState *cpu)
>>      tcg_ctx.tb_ctx.tb_flush_count++;
>>  }
>>
>> +void tb_flush(CPUState *cpu)
>> +{
>> +    async_safe_run_on_cpu(cpu, do_tb_flush, NULL);
>> +}
>> +
>>  #ifdef DEBUG_TB_CHECK
>>
>>  static void
>> @@ -1163,9 +1167,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>   buffer_overflow:
>>          /* flush must be done */
>>          tb_flush(cpu);
>> -        /* cannot fail at this point */
>> -        tb = tb_alloc(pc);
>> -        assert(tb != NULL);
>> +        mmap_unlock();
>> +        cpu_loop_exit(cpu);
> Given our other discussions about lock resetting I wonder if this is
> another case where mmap_reset() could be called on cpu_loop_exit?

As I can see, this is the only place mmap_unlock() have to be called
right before cpu_loop_exit(). As I remember, all the other cased in
user-mode emulation were restructured by Peter M. in his syscall/signal
handling series. However, I like the idea to ensure that 'mmap_lock' is
released on any cpu_loop_exit(). What do maintainers think?

Kind regards,
Sergey

>
>>      }
>>
>>      gen_code_buf = tcg_ctx.code_gen_ptr;
> Otherwise so far the testing is looking pretty positive in linux-user:
>
> Tested-by: Alex Bennée <alex.bennee@linaro.org>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>
>
> --
> Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe
  2016-07-14  8:54     ` Sergey Fedorov
@ 2016-07-14  9:49       ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-14  9:49 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: Sergey Fedorov, qemu-devel, MTTCG Devel,
	KONRAD Frédéric, Alvise Rigo, Emilio G. Cota,
	Paolo Bonzini, Richard Henderson, Peter Maydell,
	Peter Crosthwaite


Sergey Fedorov <serge.fdrv@gmail.com> writes:

> On 14/07/16 11:41, Alex Bennée wrote:
>> Sergey Fedorov <sergey.fedorov@linaro.org> writes:
>>
>>> From: Sergey Fedorov <serge.fdrv@gmail.com>
>>>
>>> Use async_safe_run_on_cpu() to make tb_flush() thread safe.
>>>
>>> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
>>> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
>>> ---
>>>
>>> Changes in v2:
>>>  - stale comment about unsafe tb_flush() removed
>>> ---
>>>  translate-all.c | 13 ++++++++-----
>>>  1 file changed, 8 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/translate-all.c b/translate-all.c
>>> index eaa95e4cd7dc..e69b5d4e889e 100644
>>> --- a/translate-all.c
>>> +++ b/translate-all.c
>>> @@ -831,8 +831,7 @@ static void page_flush_tb(void)
>>>  }
>>>
>>>  /* flush all the translation blocks */
>>> -/* XXX: tb_flush is currently not thread safe */
>>> -void tb_flush(CPUState *cpu)
>>> +static void do_tb_flush(CPUState *cpu, void *data)
>>>  {
>>>  #if defined(DEBUG_FLUSH)
>>>      printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
>>> @@ -861,6 +860,11 @@ void tb_flush(CPUState *cpu)
>>>      tcg_ctx.tb_ctx.tb_flush_count++;
>>>  }
>>>
>>> +void tb_flush(CPUState *cpu)
>>> +{
>>> +    async_safe_run_on_cpu(cpu, do_tb_flush, NULL);
>>> +}
>>> +
>>>  #ifdef DEBUG_TB_CHECK
>>>
>>>  static void
>>> @@ -1163,9 +1167,8 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
>>>   buffer_overflow:
>>>          /* flush must be done */
>>>          tb_flush(cpu);
>>> -        /* cannot fail at this point */
>>> -        tb = tb_alloc(pc);
>>> -        assert(tb != NULL);
>>> +        mmap_unlock();
>>> +        cpu_loop_exit(cpu);
>> Given our other discussions about lock resetting I wonder if this is
>> another case where mmap_reset() could be called on cpu_loop_exit?
>
> As I can see, this is the only place mmap_unlock() have to be called
> right before cpu_loop_exit(). As I remember, all the other cased in
> user-mode emulation were restructured by Peter M. in his syscall/signal
> handling series. However, I like the idea to ensure that 'mmap_lock' is
> released on any cpu_loop_exit(). What do maintainers think?
>
> Kind regards,
> Sergey
>
>>
>>>      }
>>>
>>>      gen_code_buf = tcg_ctx.code_gen_ptr;
>> Otherwise so far the testing is looking pretty positive in linux-user:
>>
>> Tested-by: Alex Bennée <alex.bennee@linaro.org>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

I should add for the testing to fail without this series I had to apply
the hot-path fixes otherwise lock contention has a serialising affect on
the flushes anyway.

>>
>>
>> --
>> Alex Bennée


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 07/11] linux-user: Rework exclusive operation mechanism
  2016-07-06 21:14 ` [Qemu-devel] [RFC v2 07/11] linux-user: Rework exclusive operation mechanism Sergey Fedorov
@ 2016-07-14 15:04   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-14 15:04 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Riku Voipio


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> A single variable 'pending_cpus' was used for both counting currently
> running CPUs and for signalling the pending exclusive operation request.
>
> To prepare for supporting operations which requires a quiescent state,
> like translation buffer flush, it is useful to keep a counter of
> currently running CPUs always up to date.
>
> Use a separate variable 'tcg_pending_threads' to count for currently
> running CPUs and a separate variable 'exclusive_pending' to indicate
> that there's an exclusive operation pending.
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
>
> Changes in v2:
>  - Rename 'tcg_pending_cpus' to 'tcg_pending_threads'


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> ---
>  linux-user/main.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index bdbda693cc5f..5ff0b20bad89 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -112,7 +112,8 @@ static QemuMutex cpu_list_mutex;
>  static QemuMutex exclusive_lock;
>  static QemuCond exclusive_cond;
>  static QemuCond exclusive_resume;
> -static int pending_cpus;
> +static bool exclusive_pending;
> +static int tcg_pending_threads;
>
>  void qemu_init_cpu_loop(void)
>  {
> @@ -142,7 +143,8 @@ void fork_end(int child)
>                  QTAILQ_REMOVE(&cpus, cpu, node);
>              }
>          }
> -        pending_cpus = 0;
> +        tcg_pending_threads = 0;
> +        exclusive_pending = false;
>          qemu_mutex_init(&exclusive_lock);
>          qemu_mutex_init(&cpu_list_mutex);
>          qemu_cond_init(&exclusive_cond);
> @@ -159,7 +161,7 @@ void fork_end(int child)
>     must be held.  */
>  static inline void exclusive_idle(void)
>  {
> -    while (pending_cpus) {
> +    while (exclusive_pending) {
>          qemu_cond_wait(&exclusive_resume, &exclusive_lock);
>      }
>  }
> @@ -173,15 +175,14 @@ static inline void start_exclusive(void)
>      qemu_mutex_lock(&exclusive_lock);
>      exclusive_idle();
>
> -    pending_cpus = 1;
> +    exclusive_pending = true;
>      /* Make all other cpus stop executing.  */
>      CPU_FOREACH(other_cpu) {
>          if (other_cpu->running) {
> -            pending_cpus++;
>              cpu_exit(other_cpu);
>          }
>      }
> -    if (pending_cpus > 1) {
> +    while (tcg_pending_threads) {
>          qemu_cond_wait(&exclusive_cond, &exclusive_lock);
>      }
>  }
> @@ -189,7 +190,7 @@ static inline void start_exclusive(void)
>  /* Finish an exclusive operation.  */
>  static inline void __attribute__((unused)) end_exclusive(void)
>  {
> -    pending_cpus = 0;
> +    exclusive_pending = false;
>      qemu_cond_broadcast(&exclusive_resume);
>      qemu_mutex_unlock(&exclusive_lock);
>  }
> @@ -200,6 +201,7 @@ static inline void cpu_exec_start(CPUState *cpu)
>      qemu_mutex_lock(&exclusive_lock);
>      exclusive_idle();
>      cpu->running = true;
> +    tcg_pending_threads++;
>      qemu_mutex_unlock(&exclusive_lock);
>  }
>
> @@ -208,11 +210,9 @@ static inline void cpu_exec_end(CPUState *cpu)
>  {
>      qemu_mutex_lock(&exclusive_lock);
>      cpu->running = false;
> -    if (pending_cpus > 1) {
> -        pending_cpus--;
> -        if (pending_cpus == 1) {
> -            qemu_cond_signal(&exclusive_cond);
> -        }
> +    tcg_pending_threads--;
> +    if (!tcg_pending_threads) {
> +        qemu_cond_signal(&exclusive_cond);
>      }
>      exclusive_idle();
>      qemu_mutex_unlock(&exclusive_lock);


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 08/11] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick()
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 08/11] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Sergey Fedorov
@ 2016-07-14 15:07   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-14 15:07 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Riku Voipio


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
>  linux-user/main.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)


Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 5ff0b20bad89..a8790ac63f68 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -3785,6 +3785,16 @@ void cpu_loop(CPUTLGState *env)
>
>  THREAD CPUState *thread_cpu;
>
> +bool qemu_cpu_is_self(CPUState *cpu)
> +{
> +    return thread_cpu == cpu;
> +}
> +
> +void qemu_cpu_kick(CPUState *cpu)
> +{
> +    cpu_exit(cpu);
> +}
> +
>  void task_settid(TaskState *ts)
>  {
>      if (ts->ts_tid == 0) {


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 09/11] linux-user: Support CPU work queue
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 09/11] linux-user: Support CPU work queue Sergey Fedorov
@ 2016-07-14 15:10   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-14 15:10 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Peter Crosthwaite, Riku Voipio


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Make CPU work core functions common between system and user-mode
> emulation. User-mode does not have BQL, so process_queued_cpu_work() is
> protected by 'exclusive_lock'.
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
>
> Changes in v2:
>  - 'qemu_work_cond' definition moved to cpu-exec-common.c
>  - documentation commend for new public API added

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

>
> ---
>  cpu-exec-common.c       | 85 ++++++++++++++++++++++++++++++++++++++++++++++++
>  cpus.c                  | 86 +------------------------------------------------
>  include/exec/exec-all.h | 17 ++++++++++
>  linux-user/main.c       |  8 +++++
>  4 files changed, 111 insertions(+), 85 deletions(-)
>
> diff --git a/cpu-exec-common.c b/cpu-exec-common.c
> index 0cb4ae60eff9..a233f0124559 100644
> --- a/cpu-exec-common.c
> +++ b/cpu-exec-common.c
> @@ -77,3 +77,88 @@ void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
>      }
>      siglongjmp(cpu->jmp_env, 1);
>  }
> +
> +QemuCond qemu_work_cond;
> +
> +static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
> +{
> +    qemu_mutex_lock(&cpu->work_mutex);
> +    if (cpu->queued_work_first == NULL) {
> +        cpu->queued_work_first = wi;
> +    } else {
> +        cpu->queued_work_last->next = wi;
> +    }
> +    cpu->queued_work_last = wi;
> +    wi->next = NULL;
> +    wi->done = false;
> +    qemu_mutex_unlock(&cpu->work_mutex);
> +
> +    qemu_cpu_kick(cpu);
> +}
> +
> +void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
> +{
> +    struct qemu_work_item wi;
> +
> +    if (qemu_cpu_is_self(cpu)) {
> +        func(cpu, data);
> +        return;
> +    }
> +
> +    wi.func = func;
> +    wi.data = data;
> +    wi.free = false;
> +
> +    queue_work_on_cpu(cpu, &wi);
> +    while (!atomic_mb_read(&wi.done)) {
> +        CPUState *self_cpu = current_cpu;
> +
> +        qemu_cond_wait(&qemu_work_cond, qemu_get_cpu_work_mutex());
> +        current_cpu = self_cpu;
> +    }
> +}
> +
> +void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
> +{
> +    struct qemu_work_item *wi;
> +
> +    if (qemu_cpu_is_self(cpu)) {
> +        func(cpu, data);
> +        return;
> +    }
> +
> +    wi = g_malloc0(sizeof(struct qemu_work_item));
> +    wi->func = func;
> +    wi->data = data;
> +    wi->free = true;
> +
> +    queue_work_on_cpu(cpu, wi);
> +}
> +
> +void process_queued_cpu_work(CPUState *cpu)
> +{
> +    struct qemu_work_item *wi;
> +
> +    if (cpu->queued_work_first == NULL) {
> +        return;
> +    }
> +
> +    qemu_mutex_lock(&cpu->work_mutex);
> +    while (cpu->queued_work_first != NULL) {
> +        wi = cpu->queued_work_first;
> +        cpu->queued_work_first = wi->next;
> +        if (!cpu->queued_work_first) {
> +            cpu->queued_work_last = NULL;
> +        }
> +        qemu_mutex_unlock(&cpu->work_mutex);
> +        wi->func(cpu, wi->data);
> +        qemu_mutex_lock(&cpu->work_mutex);
> +        if (wi->free) {
> +            g_free(wi);
> +        } else {
> +            atomic_mb_set(&wi->done, true);
> +        }
> +    }
> +    qemu_mutex_unlock(&cpu->work_mutex);
> +    qemu_cond_broadcast(&qemu_work_cond);
> +}
> diff --git a/cpus.c b/cpus.c
> index 51fd8c18b4c8..282d7e399902 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -896,7 +896,6 @@ static QemuThread io_thread;
>  static QemuCond qemu_cpu_cond;
>  /* system init */
>  static QemuCond qemu_pause_cond;
> -static QemuCond qemu_work_cond;
>
>  void qemu_init_cpu_loop(void)
>  {
> @@ -910,66 +909,11 @@ void qemu_init_cpu_loop(void)
>      qemu_thread_get_self(&io_thread);
>  }
>
> -static QemuMutex *qemu_get_cpu_work_mutex(void)
> +QemuMutex *qemu_get_cpu_work_mutex(void)
>  {
>      return &qemu_global_mutex;
>  }
>
> -static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
> -{
> -    qemu_mutex_lock(&cpu->work_mutex);
> -    if (cpu->queued_work_first == NULL) {
> -        cpu->queued_work_first = wi;
> -    } else {
> -        cpu->queued_work_last->next = wi;
> -    }
> -    cpu->queued_work_last = wi;
> -    wi->next = NULL;
> -    wi->done = false;
> -    qemu_mutex_unlock(&cpu->work_mutex);
> -
> -    qemu_cpu_kick(cpu);
> -}
> -
> -void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
> -{
> -    struct qemu_work_item wi;
> -
> -    if (qemu_cpu_is_self(cpu)) {
> -        func(cpu, data);
> -        return;
> -    }
> -
> -    wi.func = func;
> -    wi.data = data;
> -    wi.free = false;
> -
> -    queue_work_on_cpu(cpu, &wi);
> -    while (!atomic_mb_read(&wi.done)) {
> -        CPUState *self_cpu = current_cpu;
> -
> -        qemu_cond_wait(&qemu_work_cond, qemu_get_cpu_work_mutex());
> -        current_cpu = self_cpu;
> -    }
> -}
> -
> -void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
> -{
> -    struct qemu_work_item *wi;
> -
> -    if (qemu_cpu_is_self(cpu)) {
> -        func(cpu, data);
> -        return;
> -    }
> -
> -    wi = g_malloc0(sizeof(struct qemu_work_item));
> -    wi->func = func;
> -    wi->data = data;
> -    wi->free = true;
> -
> -    queue_work_on_cpu(cpu, wi);
> -}
> -
>  static void qemu_kvm_destroy_vcpu(CPUState *cpu)
>  {
>      if (kvm_destroy_vcpu(cpu) < 0) {
> @@ -982,34 +926,6 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu)
>  {
>  }
>
> -static void process_queued_cpu_work(CPUState *cpu)
> -{
> -    struct qemu_work_item *wi;
> -
> -    if (cpu->queued_work_first == NULL) {
> -        return;
> -    }
> -
> -    qemu_mutex_lock(&cpu->work_mutex);
> -    while (cpu->queued_work_first != NULL) {
> -        wi = cpu->queued_work_first;
> -        cpu->queued_work_first = wi->next;
> -        if (!cpu->queued_work_first) {
> -            cpu->queued_work_last = NULL;
> -        }
> -        qemu_mutex_unlock(&cpu->work_mutex);
> -        wi->func(cpu, wi->data);
> -        qemu_mutex_lock(&cpu->work_mutex);
> -        if (wi->free) {
> -            g_free(wi);
> -        } else {
> -            atomic_mb_set(&wi->done, true);
> -        }
> -    }
> -    qemu_mutex_unlock(&cpu->work_mutex);
> -    qemu_cond_broadcast(&qemu_work_cond);
> -}
> -
>  static void qemu_wait_io_event_common(CPUState *cpu)
>  {
>      if (cpu->stop) {
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index c1f59fa59d2c..bceecbd5222a 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -407,4 +407,21 @@ extern int singlestep;
>  extern CPUState *tcg_current_cpu;
>  extern bool exit_request;
>
> +/**
> + * qemu_work_cond - condition to wait for CPU work items completion
> + */
> +extern QemuCond qemu_work_cond;
> +
> +/**
> + * qemu_get_cpu_work_mutex() - get the mutex which protects CPU work execution
> + *
> + * Return: A pointer to the mutex.
> + */
> +QemuMutex *qemu_get_cpu_work_mutex(void);
> +/**
> + * process_queued_cpu_work() - process all items on CPU work queue
> + * @cpu: The CPU which work queue to process.
> + */
> +void process_queued_cpu_work(CPUState *cpu);
> +
>  #endif
> diff --git a/linux-user/main.c b/linux-user/main.c
> index a8790ac63f68..fce61d5a35fc 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -121,6 +121,7 @@ void qemu_init_cpu_loop(void)
>      qemu_mutex_init(&exclusive_lock);
>      qemu_cond_init(&exclusive_cond);
>      qemu_cond_init(&exclusive_resume);
> +    qemu_cond_init(&qemu_work_cond);
>  }
>
>  /* Make sure everything is in a consistent state for calling fork().  */
> @@ -149,6 +150,7 @@ void fork_end(int child)
>          qemu_mutex_init(&cpu_list_mutex);
>          qemu_cond_init(&exclusive_cond);
>          qemu_cond_init(&exclusive_resume);
> +        qemu_cond_init(&qemu_work_cond);
>          qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
>          gdbserver_fork(thread_cpu);
>      } else {
> @@ -157,6 +159,11 @@ void fork_end(int child)
>      }
>  }
>
> +QemuMutex *qemu_get_cpu_work_mutex(void)
> +{
> +    return &exclusive_lock;
> +}
> +
>  /* Wait for pending exclusive operations to complete.  The exclusive lock
>     must be held.  */
>  static inline void exclusive_idle(void)
> @@ -215,6 +222,7 @@ static inline void cpu_exec_end(CPUState *cpu)
>          qemu_cond_signal(&exclusive_cond);
>      }
>      exclusive_idle();
> +    process_queued_cpu_work(cpu);
>      qemu_mutex_unlock(&exclusive_lock);
>  }


--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 10/11] cpu-exec-common: Introduce async_safe_run_on_cpu()
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 10/11] cpu-exec-common: Introduce async_safe_run_on_cpu() Sergey Fedorov
@ 2016-07-14 15:57   ` Alex Bennée
  0 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-14 15:57 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov, Peter Crosthwaite, Riku Voipio


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> This patch is based on the ideas found in work of KONRAD Frederic [1],
> Alex Bennée [2], and Alvise Rigo [3].
>
> This mechanism allows to perform an operation safely in a quiescent
> state. Quiescent state means: (1) no vCPU is running and (2) BQL in
> system-mode or 'exclusive_lock' in user-mode emulation is held while
> performing the operation. This functionality is required e.g. for
> performing translation buffer flush safely in multi-threaded user-mode
> emulation.
>
> The existing CPU work queue is used to schedule such safe operations. A
> new 'safe' flag is added into struct qemu_work_item to designate the
> special requirements of the safe work. An operation in a quiescent sate
> can be scheduled by using async_safe_run_on_cpu() function which is
> actually the same as sync_run_on_cpu() except that it marks the queued
> work item with the 'safe' flag set to true. Given this flag set
> queue_work_on_cpu() atomically increments 'safe_work_pending' global
> counter and kicks all the CPUs instead of just the target CPU as in case
> of normal CPU work. This allows to force other CPUs to exit their
> execution loops and wait in wait_safe_cpu_work() function for the safe
> work to finish. When a CPU drains its work queue, if it encounters a
> work item marked as safe, it first waits for other CPUs to exit their
> execution loops, then called the work item function, and finally
> decrements 'safe_work_pending' counter with signalling other CPUs to let
> them continue execution as soon as all pending safe work items have been
> processed. The 'tcg_pending_threads' protected by 'exclusive_lock' in
> user-mode or by 'qemu_global_mutex' in system-mode emulation is used to
> determine if there is any CPU run and wait for it to exit the execution
> loop. The fairness of all the CPU work queues is ensured by draining all
> the pending safe work items before any CPU can run.
>
> [1] http://lists.nongnu.org/archive/html/qemu-devel/2015-08/msg01128.html
> [2] http://lists.nongnu.org/archive/html/qemu-devel/2016-04/msg02531.html
> [3] http://lists.nongnu.org/archive/html/qemu-devel/2016-05/msg04792.html
>
> Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
> Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
> ---
>
> Changes in v2:
>  - some conditional varialbes moved to cpu-exec-common.c
>  - documentation commend for new public API added
> ---
>  cpu-exec-common.c       | 49 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  cpus.c                  | 20 ++++++++++++++++++++
>  include/exec/exec-all.h | 14 ++++++++++++++
>  include/qom/cpu.h       | 14 ++++++++++++++
>  linux-user/main.c       | 13 +++++++------
>  5 files changed, 103 insertions(+), 7 deletions(-)
>
> diff --git a/cpu-exec-common.c b/cpu-exec-common.c
> index a233f0124559..6f278d6d3b70 100644
> --- a/cpu-exec-common.c
> +++ b/cpu-exec-common.c
> @@ -25,6 +25,7 @@
>
>  bool exit_request;
>  CPUState *tcg_current_cpu;
> +int tcg_pending_threads;
>
>  /* exit the current TB, but without causing any exception to be raised */
>  void cpu_loop_exit_noexc(CPUState *cpu)
> @@ -79,6 +80,17 @@ void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc)
>  }
>
>  QemuCond qemu_work_cond;
> +QemuCond qemu_safe_work_cond;
> +QemuCond qemu_exclusive_cond;
> +
> +static int safe_work_pending;
> +
> +void wait_safe_cpu_work(void)
> +{
> +    while (atomic_mb_read(&safe_work_pending) > 0) {
> +        qemu_cond_wait(&qemu_safe_work_cond, qemu_get_cpu_work_mutex());
> +    }
> +}
>
>  static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
>  {
> @@ -91,9 +103,18 @@ static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
>      cpu->queued_work_last = wi;
>      wi->next = NULL;
>      wi->done = false;
> +    if (wi->safe) {
> +        atomic_inc(&safe_work_pending);
> +    }
>      qemu_mutex_unlock(&cpu->work_mutex);
>
> -    qemu_cpu_kick(cpu);
> +    if (!wi->safe) {
> +        qemu_cpu_kick(cpu);
> +    } else {
> +        CPU_FOREACH(cpu) {
> +            qemu_cpu_kick(cpu);
> +        }
> +    }
>  }
>
>  void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
> @@ -108,6 +129,7 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>      wi.func = func;
>      wi.data = data;
>      wi.free = false;
> +    wi.safe = false;
>
>      queue_work_on_cpu(cpu, &wi);
>      while (!atomic_mb_read(&wi.done)) {
> @@ -131,6 +153,20 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
>      wi->func = func;
>      wi->data = data;
>      wi->free = true;
> +    wi->safe = false;
> +
> +    queue_work_on_cpu(cpu, wi);
> +}
> +
> +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
> +{
> +    struct qemu_work_item *wi;
> +
> +    wi = g_malloc0(sizeof(struct qemu_work_item));
> +    wi->func = func;
> +    wi->data = data;
> +    wi->free = true;
> +    wi->safe = true;
>
>      queue_work_on_cpu(cpu, wi);
>  }
> @@ -150,9 +186,20 @@ void process_queued_cpu_work(CPUState *cpu)
>          if (!cpu->queued_work_first) {
>              cpu->queued_work_last = NULL;
>          }
> +        if (wi->safe) {
> +            while (tcg_pending_threads) {
> +                qemu_cond_wait(&qemu_exclusive_cond,
> +                               qemu_get_cpu_work_mutex());
> +            }
> +        }
>          qemu_mutex_unlock(&cpu->work_mutex);
>          wi->func(cpu, wi->data);
>          qemu_mutex_lock(&cpu->work_mutex);
> +        if (wi->safe) {
> +            if (!atomic_dec_fetch(&safe_work_pending)) {
> +                qemu_cond_broadcast(&qemu_safe_work_cond);
> +            }
> +        }
>          if (wi->free) {
>              g_free(wi);
>          } else {
> diff --git a/cpus.c b/cpus.c
> index 282d7e399902..b7122043f650 100644
> --- a/cpus.c
> +++ b/cpus.c
> @@ -903,6 +903,8 @@ void qemu_init_cpu_loop(void)
>      qemu_cond_init(&qemu_cpu_cond);
>      qemu_cond_init(&qemu_pause_cond);
>      qemu_cond_init(&qemu_work_cond);
> +    qemu_cond_init(&qemu_safe_work_cond);
> +    qemu_cond_init(&qemu_exclusive_cond);
>      qemu_cond_init(&qemu_io_proceeded_cond);
>      qemu_mutex_init(&qemu_global_mutex);
>
> @@ -926,6 +928,20 @@ static void qemu_tcg_destroy_vcpu(CPUState *cpu)
>  {
>  }
>
> +/* called with qemu_global_mutex held */
> +static inline void tcg_cpu_exec_start(CPUState *cpu)
> +{
> +    tcg_pending_threads++;
> +}
> +
> +/* called with qemu_global_mutex held */
> +static inline void tcg_cpu_exec_end(CPUState *cpu)
> +{
> +    if (--tcg_pending_threads) {
> +        qemu_cond_broadcast(&qemu_exclusive_cond);
> +    }
> +}
> +
>  static void qemu_wait_io_event_common(CPUState *cpu)
>  {
>      if (cpu->stop) {
> @@ -950,6 +966,8 @@ static void qemu_tcg_wait_io_event(CPUState *cpu)
>      CPU_FOREACH(cpu) {
>          qemu_wait_io_event_common(cpu);
>      }
> +
> +    wait_safe_cpu_work();
>  }
>
>  static void qemu_kvm_wait_io_event(CPUState *cpu)
> @@ -1485,7 +1503,9 @@ static void tcg_exec_all(void)
>                            (cpu->singlestep_enabled & SSTEP_NOTIMER) == 0);
>
>          if (cpu_can_run(cpu)) {
> +            tcg_cpu_exec_start(cpu);
>              r = tcg_cpu_exec(cpu);
> +            tcg_cpu_exec_end(cpu);
>              if (r == EXCP_DEBUG) {
>                  cpu_handle_guest_debug(cpu);
>                  break;
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index bceecbd5222a..992055ce36bf 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -405,12 +405,22 @@ extern int singlestep;
>
>  /* cpu-exec.c, accessed with atomic_mb_read/atomic_mb_set */
>  extern CPUState *tcg_current_cpu;
> +extern int tcg_pending_threads;
>  extern bool exit_request;
>
>  /**
>   * qemu_work_cond - condition to wait for CPU work items completion
>   */
>  extern QemuCond qemu_work_cond;
> +/**
> + * qemu_safe_work_cond - condition to wait for safe CPU work items completion
> + */
> +extern QemuCond qemu_safe_work_cond;
> +/**
> + * qemu_exclusive_cond - condition to wait for all TCG threads to be out of
> + *                       guest code execution loop
> + */
> +extern QemuCond qemu_exclusive_cond;
>
>  /**
>   * qemu_get_cpu_work_mutex() - get the mutex which protects CPU work execution
> @@ -423,5 +433,9 @@ QemuMutex *qemu_get_cpu_work_mutex(void);
>   * @cpu: The CPU which work queue to process.
>   */
>  void process_queued_cpu_work(CPUState *cpu);
> +/**
> + * wait_safe_cpu_work() - wait until all safe CPU work items has processed
> + */
> +void wait_safe_cpu_work(void);
>
>  #endif
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 4e688f645b4a..5128fcc1745a 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -231,6 +231,7 @@ struct qemu_work_item {
>      void *data;
>      int done;
>      bool free;
> +    bool safe;
>  };
>
>  /**
> @@ -625,6 +626,19 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>  void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
>
>  /**
> + * async_safe_run_on_cpu:
> + * @cpu: The vCPU to run on.
> + * @func: The function to be executed.
> + * @data: Data to pass to the function.
> + *
> + * Schedules the function @func for execution on the vCPU @cpu asynchronously
> + * and in quiescent state. Quiescent state means: (1) all other vCPUs are
> + * halted and (2) #qemu_global_mutex (a.k.a. BQL) in system-mode or
> + * #exclusive_lock in user-mode emulation is held while @func is executing.
> + */
> +void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data);
> +
> +/**
>   * qemu_get_cpu:
>   * @index: The CPUState@cpu_index value of the CPU to obtain.
>   *
> diff --git a/linux-user/main.c b/linux-user/main.c
> index fce61d5a35fc..d0ff5f9976e5 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -110,18 +110,17 @@ int cpu_get_pic_interrupt(CPUX86State *env)
>     which requires quite a lot of per host/target work.  */
>  static QemuMutex cpu_list_mutex;
>  static QemuMutex exclusive_lock;
> -static QemuCond exclusive_cond;
>  static QemuCond exclusive_resume;
>  static bool exclusive_pending;
> -static int tcg_pending_threads;
>
>  void qemu_init_cpu_loop(void)
>  {
>      qemu_mutex_init(&cpu_list_mutex);
>      qemu_mutex_init(&exclusive_lock);
> -    qemu_cond_init(&exclusive_cond);
>      qemu_cond_init(&exclusive_resume);
>      qemu_cond_init(&qemu_work_cond);
> +    qemu_cond_init(&qemu_safe_work_cond);
> +    qemu_cond_init(&qemu_exclusive_cond);
>  }
>
>  /* Make sure everything is in a consistent state for calling fork().  */
> @@ -148,9 +147,10 @@ void fork_end(int child)
>          exclusive_pending = false;
>          qemu_mutex_init(&exclusive_lock);
>          qemu_mutex_init(&cpu_list_mutex);
> -        qemu_cond_init(&exclusive_cond);
>          qemu_cond_init(&exclusive_resume);
>          qemu_cond_init(&qemu_work_cond);
> +        qemu_cond_init(&qemu_safe_work_cond);
> +        qemu_cond_init(&qemu_exclusive_cond);
>          qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
>          gdbserver_fork(thread_cpu);
>      } else {
> @@ -190,7 +190,7 @@ static inline void start_exclusive(void)
>          }
>      }
>      while (tcg_pending_threads) {
> -        qemu_cond_wait(&exclusive_cond, &exclusive_lock);
> +        qemu_cond_wait(&qemu_exclusive_cond, &exclusive_lock);
>      }
>  }
>
> @@ -219,10 +219,11 @@ static inline void cpu_exec_end(CPUState *cpu)
>      cpu->running = false;
>      tcg_pending_threads--;
>      if (!tcg_pending_threads) {
> -        qemu_cond_signal(&exclusive_cond);
> +        qemu_cond_broadcast(&qemu_exclusive_cond);
>      }
>      exclusive_idle();
>      process_queued_cpu_work(cpu);
> +    wait_safe_cpu_work();
>      qemu_mutex_unlock(&exclusive_lock);
>  }

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

* Re: [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state
  2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
                   ` (10 preceding siblings ...)
  2016-07-06 21:15 ` [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe Sergey Fedorov
@ 2016-07-14 16:00 ` Alex Bennée
  11 siblings, 0 replies; 33+ messages in thread
From: Alex Bennée @ 2016-07-14 16:00 UTC (permalink / raw)
  To: Sergey Fedorov
  Cc: qemu-devel, MTTCG Devel, KONRAD Frédéric, Alvise Rigo,
	Emilio G. Cota, Paolo Bonzini, Richard Henderson, Peter Maydell,
	Sergey Fedorov


Sergey Fedorov <sergey.fedorov@linaro.org> writes:

> From: Sergey Fedorov <serge.fdrv@gmail.com>
>
> Hi,
>
> This is a v2 of the RFC series [3] that was a follow-up for a discussion
> on the subject [1]. This is sill marked as FRC becuse it laks bsd-user
> support.

Given the maintenance state of BSD I'm not sure how important this is. I
assume the exclusive handling will be pretty much the same though.

>
> Basically, this series suggests a way to perform some operations in
> quiescent state. The goal is to implement such a mechanism which can be
> used for safe translation buffer flush in multi-threaded user-mode
> emulation (and later in MTTCG) and merge it into mainline as soon as
> possible.
>
> This series is a more elaborated version of the original series since
> there was some positive feedback and I'd appreciate some comments from
> maintainers this time.
>
> The first two patches are some useful tweak from Alex's MTTCG trees.
>
> The patches 3 through 8 are arrangements for the patch 9 which adds
> support for CPU work in linux-user. This wouldn't make any sense without
> the patch 10 which is the subject matter of this series. Although there
> is nothing special to do in case of single-threaded round-robin CPU loop
> of current system-mode emulation to ensure quiescent state, that is
> shown in the patch 10, how it would look like in MTTCG. The last patch
> actually employs this new mechanism making translation buffer flush
> thread safe.
>
> The considerations on expensiveness of work item dynamic allocation [2]
> was not taken into account. I'll just mention here that the desired
> effect can be achieved by either using dynamic arrays for CPU work
> queues or making queue_work_on_cpu() from the patch 3 a public interface
> thus allowing to use preallocated work items.
>
> I would like your comments in order to produce something upstreamable
> quickly!
>
> This series is available at a public git repository:
>
>     https://github.com/sergefdrv/qemu.git safe-cpu-work-v2
>

I've finished by review pass now. If Sergey doesn't get a chance to
re-spin before he moves onto other things I'll pick the series up.

--
Alex Bennée

^ permalink raw reply	[flat|nested] 33+ messages in thread

end of thread, other threads:[~2016-07-14 16:00 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-06 21:14 [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Sergey Fedorov
2016-07-06 21:14 ` [Qemu-devel] [RFC v2 01/11] atomic: introduce atomic_dec_fetch Sergey Fedorov
2016-07-06 21:14 ` [RFC v2 02/11] cpus: pass CPUState to run_on_cpu helpers Sergey Fedorov
2016-07-06 21:14   ` [Qemu-devel] " Sergey Fedorov
2016-07-07  0:30   ` David Gibson
2016-07-07  0:30     ` [Qemu-devel] " David Gibson
2016-07-11 12:36   ` Christian Borntraeger
2016-07-11 12:36     ` [Qemu-devel] " Christian Borntraeger
2016-07-11 12:38     ` Sergey Fedorov
2016-07-11 12:38       ` [Qemu-devel] " Sergey Fedorov
2016-07-11 12:55       ` Christian Borntraeger
2016-07-11 12:55         ` [Qemu-devel] " Christian Borntraeger
2016-07-06 21:14 ` [Qemu-devel] [RFC v2 03/11] cpus: Move common code out of {async_, }run_on_cpu() Sergey Fedorov
2016-07-06 21:14 ` [Qemu-devel] [RFC v2 04/11] cpus: Wrap mutex used to protect CPU work Sergey Fedorov
2016-07-11 12:06   ` Alex Bennée
2016-07-06 21:14 ` [Qemu-devel] [RFC v2 05/11] cpus: Rename flush_queued_work() Sergey Fedorov
2016-07-11 12:07   ` Alex Bennée
2016-07-06 21:14 ` [Qemu-devel] [RFC v2 06/11] linux-user: Use QemuMutex and QemuCond Sergey Fedorov
2016-07-11 12:08   ` Alex Bennée
2016-07-06 21:14 ` [Qemu-devel] [RFC v2 07/11] linux-user: Rework exclusive operation mechanism Sergey Fedorov
2016-07-14 15:04   ` Alex Bennée
2016-07-06 21:15 ` [Qemu-devel] [RFC v2 08/11] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Sergey Fedorov
2016-07-14 15:07   ` Alex Bennée
2016-07-06 21:15 ` [Qemu-devel] [RFC v2 09/11] linux-user: Support CPU work queue Sergey Fedorov
2016-07-14 15:10   ` Alex Bennée
2016-07-06 21:15 ` [Qemu-devel] [RFC v2 10/11] cpu-exec-common: Introduce async_safe_run_on_cpu() Sergey Fedorov
2016-07-14 15:57   ` Alex Bennée
2016-07-06 21:15 ` [Qemu-devel] [RFC v2 11/11] tcg: Make tb_flush() thread safe Sergey Fedorov
2016-07-07 20:11   ` Sergey Fedorov
2016-07-14  8:41   ` Alex Bennée
2016-07-14  8:54     ` Sergey Fedorov
2016-07-14  9:49       ` Alex Bennée
2016-07-14 16:00 ` [Qemu-devel] [RFC v2 00/11] cpu-exec: Safe work in quiescent state Alex Bennée

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.