* [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state
@ 2016-09-12 11:12 Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers Paolo Bonzini
` (16 more replies)
0 siblings, 17 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
In addition to fixing some of the issues found by Alex, safe work items
need not run anymore with a mutex taken. Of course, cpu_exec_start/end
and start_exclusive/end_exclusive are essentially the read and write
side of a specialized rwlock, so there is still a lock in disguise looming
to cause deadlocks; however, it does removes worries about recursive
locking from CPU list manipulations.
The new patches are 8, 12 and 13. Patch 12 of v6 has been split
across patch 10 and patch 16.
Paolo
v6->v7: Do not separate qemu_work_item and SafeWorkItem
More cleanups/optimizations of exclusive section logic
Alex Bennée (1):
cpus: pass CPUState to run_on_cpu helpers
Paolo Bonzini (9):
cpus-common: move CPU list management to common code
cpus-common: fix uninitialized variable use in run_on_cpu
cpus-common: move exclusive work infrastructure from linux-user
docs: include formal model for TCG exclusive sections
cpus-common: always defer async_run_on_cpu work items
cpus-common: remove redundant call to exclusive_idle()
cpus-common: simplify locking for start_exclusive/end_exclusive
cpus-common: Introduce async_safe_run_on_cpu()
cpus-common: lock-free fast path for cpu_exec_start/end
Sergey Fedorov (6):
cpus: Move common code out of {async_, }run_on_cpu()
cpus: Rename flush_queued_work()
linux-user: Use QemuMutex and QemuCond
linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick()
cpus-common: move CPU work item management to common code
tcg: Make tb_flush() thread safe
Makefile.target | 2 +-
bsd-user/main.c | 33 ++---
cpu-exec.c | 12 +-
cpus-common.c | 335 +++++++++++++++++++++++++++++++++++++++++++++
cpus.c | 99 +-------------
docs/tcg-exclusive.promela | 224 ++++++++++++++++++++++++++++++
exec.c | 37 +----
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/cpu-all.h | 4 +
include/exec/cpu-common.h | 2 +
include/exec/exec-all.h | 11 --
include/exec/tb-context.h | 2 +-
include/qom/cpu.h | 99 ++++++++++++--
kvm-all.c | 21 +--
linux-user/main.c | 130 ++++++------------
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 | 38 +++--
vl.c | 1 +
27 files changed, 836 insertions(+), 415 deletions(-)
create mode 100644 cpus-common.c
create mode 100644 docs/tcg-exclusive.promela
--
2.7.4
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-15 15:30 ` Alex Bennée
2016-09-12 11:12 ` [Qemu-devel] [PATCH 02/16] cpus: Move common code out of {async_, }run_on_cpu() Paolo Bonzini
` (15 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
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>
Acked-by: David Gibson <david@gibson.dropbear.id.au> (ppc parts)
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> (s390 parts)
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-3-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
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 0308431..2508cbf 100644
--- a/cpus.c
+++ b/cpus.c
@@ -556,9 +556,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;
@@ -588,7 +587,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);
}
}
@@ -916,12 +915,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;
}
@@ -949,12 +948,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;
}
@@ -1005,7 +1004,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 2bd0de8..295b675 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 3bf1ddd..1bc02fb 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 22c584e..8e16f65 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 ca77bb0..c202427 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -2136,10 +2136,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);
}
@@ -2149,7 +2147,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 73af112..e5eca67 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 ce0c406..4aa9e61 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -232,9 +232,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;
@@ -623,7 +625,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:
@@ -633,7 +635,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 ebf35b0..d945c2b 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1847,10 +1847,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;
@@ -1860,34 +1858,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)
@@ -2229,7 +2223,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;
@@ -2247,7 +2241,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 1c250b8..9bc961b 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 = ¶ms->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, ¶ms);
}
}
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index d1a25c5..791c8b4 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -156,10 +156,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);
}
@@ -169,7 +167,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 2f3c8e2..35ae2ce 100644
--- a/target-s390x/cpu.c
+++ b/target-s390x/cpu.c
@@ -164,7 +164,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
@@ -220,7 +220,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 ac75360..f4dacdf 100644
--- a/target-s390x/cpu.h
+++ b/target-s390x/cpu.h
@@ -502,17 +502,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 dfaf1ca..d745bb0 100644
--- a/target-s390x/kvm.c
+++ b/target-s390x/kvm.c
@@ -1385,7 +1385,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;
@@ -1398,38 +1397,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;
}
@@ -1496,56 +1497,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 (!s390_has_feat(S390_FEAT_VECTOR)) {
@@ -1554,7 +1557,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;
}
@@ -1565,31 +1568,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;
@@ -1597,20 +1601,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);
@@ -1618,11 +1620,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);
@@ -1630,12 +1632,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)) {
@@ -1644,13 +1647,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;
}
@@ -1658,7 +1661,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 86da194..4df2ec6 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();
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 02/16] cpus: Move common code out of {async_, }run_on_cpu()
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 03/16] cpus: Rename flush_queued_work() Paolo Bonzini
` (14 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee, Sergey Fedorov
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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-4-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus.c | 42 ++++++++++++++++++------------------------
1 file changed, 18 insertions(+), 24 deletions(-)
diff --git a/cpus.c b/cpus.c
index 2508cbf..6113eb7 100644
--- a/cpus.c
+++ b/cpus.c
@@ -915,6 +915,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;
@@ -928,18 +944,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;
@@ -962,18 +967,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)
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 03/16] cpus: Rename flush_queued_work()
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 02/16] cpus: Move common code out of {async_, }run_on_cpu() Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 04/16] linux-user: Use QemuMutex and QemuCond Paolo Bonzini
` (13 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee, Sergey Fedorov
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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-6-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus.c b/cpus.c
index 6113eb7..ab15214 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;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 04/16] linux-user: Use QemuMutex and QemuCond
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (2 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 03/16] cpus: Rename flush_queued_work() Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 05/16] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Paolo Bonzini
` (12 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee, Sergey Fedorov
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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-7-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
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 6004ece..a3af553 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -111,17 +111,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();
}
@@ -138,14 +146,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);
}
}
@@ -155,7 +163,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);
}
}
@@ -165,7 +173,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;
@@ -177,7 +185,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);
}
}
@@ -185,42 +193,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);
}
@@ -4211,6 +4219,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) {
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 05/16] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick()
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (3 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 04/16] linux-user: Use QemuMutex and QemuCond Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code Paolo Bonzini
` (11 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee, Sergey Fedorov
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>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-9-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
linux-user/main.c | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/linux-user/main.c b/linux-user/main.c
index a3af553..0f24d57 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -3777,6 +3777,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) {
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (4 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 05/16] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-22 15:24 ` Alex Bennée
2016-09-12 11:12 ` [Qemu-devel] [PATCH 07/16] cpus-common: move CPU work item " Paolo Bonzini
` (10 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
Add a mutex for the CPU list to system emulation, as it will be used to
manage safe work. Abstract manipulation of the CPU list in new functions
cpu_list_add and cpu_list_remove.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
Makefile.target | 2 +-
bsd-user/main.c | 9 +----
cpus-common.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++
exec.c | 37 ++-------------------
include/exec/cpu-all.h | 4 +++
include/exec/cpu-common.h | 2 ++
include/exec/exec-all.h | 11 -------
include/qom/cpu.h | 12 +++++++
linux-user/main.c | 17 +++-------
vl.c | 1 +
10 files changed, 110 insertions(+), 68 deletions(-)
create mode 100644 cpus-common.c
diff --git a/Makefile.target b/Makefile.target
index 8c7a072..5f2cf85 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -86,7 +86,7 @@ all: $(PROGS) stap
# cpu emulator library
obj-y = exec.o translate-all.o cpu-exec.o
obj-y += translate-common.o
-obj-y += cpu-exec-common.o
+obj-y += cpu-exec-common.o cpus-common.o
obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
obj-$(CONFIG_TCG_INTERPRETER) += tci.o
obj-y += tcg/tcg-common.o
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 0fb08e4..591c424 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -95,14 +95,6 @@ void fork_end(int child)
}
}
-void cpu_list_lock(void)
-{
-}
-
-void cpu_list_unlock(void)
-{
-}
-
#ifdef TARGET_I386
/***********************************************************/
/* CPUX86 core interface */
@@ -748,6 +740,7 @@ int main(int argc, char **argv)
if (argc <= 1)
usage();
+ qemu_init_cpu_list();
module_call_init(MODULE_INIT_QOM);
if ((envlist = envlist_create()) == NULL) {
diff --git a/cpus-common.c b/cpus-common.c
new file mode 100644
index 0000000..52198d8
--- /dev/null
+++ b/cpus-common.c
@@ -0,0 +1,83 @@
+/*
+ * CPU thread main loop - common bits for user and system mode emulation
+ *
+ * Copyright (c) 2003-2005 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "sysemu/cpus.h"
+#include "exec/memory-internal.h"
+
+static QemuMutex qemu_cpu_list_mutex;
+
+void qemu_init_cpu_list(void)
+{
+ qemu_mutex_init(&qemu_cpu_list_mutex);
+}
+
+void cpu_list_lock(void)
+{
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+}
+
+void cpu_list_unlock(void)
+{
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+}
+
+static bool cpu_index_auto_assigned;
+
+static int cpu_get_free_index(void)
+{
+ CPUState *some_cpu;
+ int cpu_index = 0;
+
+ cpu_index_auto_assigned = true;
+ CPU_FOREACH(some_cpu) {
+ cpu_index++;
+ }
+ return cpu_index;
+}
+
+void cpu_list_add(CPUState *cpu)
+{
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
+ cpu->cpu_index = cpu_get_free_index();
+ assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
+ } else {
+ assert(!cpu_index_auto_assigned);
+ }
+ QTAILQ_INSERT_TAIL(&cpus, cpu, node);
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+}
+
+void cpu_list_remove(CPUState *cpu)
+{
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ if (!QTAILQ_IN_USE(cpu, node)) {
+ /* there is nothing to undo since cpu_exec_init() hasn't been called */
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+ return;
+ }
+
+ assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
+
+ QTAILQ_REMOVE(&cpus, cpu, node);
+ cpu->cpu_index = UNASSIGNED_CPU_INDEX;
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+}
diff --git a/exec.c b/exec.c
index ce3fb9e..784990c 100644
--- a/exec.c
+++ b/exec.c
@@ -598,36 +598,11 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
}
#endif
-static bool cpu_index_auto_assigned;
-
-static int cpu_get_free_index(void)
-{
- CPUState *some_cpu;
- int cpu_index = 0;
-
- cpu_index_auto_assigned = true;
- CPU_FOREACH(some_cpu) {
- cpu_index++;
- }
- return cpu_index;
-}
-
void cpu_exec_exit(CPUState *cpu)
{
CPUClass *cc = CPU_GET_CLASS(cpu);
- cpu_list_lock();
- if (!QTAILQ_IN_USE(cpu, node)) {
- /* there is nothing to undo since cpu_exec_init() hasn't been called */
- cpu_list_unlock();
- return;
- }
-
- assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
-
- QTAILQ_REMOVE(&cpus, cpu, node);
- cpu->cpu_index = UNASSIGNED_CPU_INDEX;
- cpu_list_unlock();
+ cpu_list_remove(cpu);
if (cc->vmsd != NULL) {
vmstate_unregister(NULL, cc->vmsd, cpu);
@@ -663,15 +638,7 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
object_ref(OBJECT(cpu->memory));
#endif
- cpu_list_lock();
- if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
- cpu->cpu_index = cpu_get_free_index();
- assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
- } else {
- assert(!cpu_index_auto_assigned);
- }
- QTAILQ_INSERT_TAIL(&cpus, cpu, node);
- cpu_list_unlock();
+ cpu_list_add(cpu);
#ifndef CONFIG_USER_ONLY
if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index b6a7059..c694d16 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -187,6 +187,10 @@ void address_space_stq(AddressSpace *as, hwaddr addr, uint64_t val,
MemTxAttrs attrs, MemTxResult *result);
#endif
+/* The CPU list lock nests outside tb_lock/tb_unlock. */
+void cpu_list_lock(void);
+void cpu_list_unlock(void);
+
/* page related stuff */
#define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
index 952bcfe..1d41452 100644
--- a/include/exec/cpu-common.h
+++ b/include/exec/cpu-common.h
@@ -23,6 +23,8 @@ typedef struct CPUListState {
FILE *file;
} CPUListState;
+void qemu_init_cpu_list(void);
+
#if !defined(CONFIG_USER_ONLY)
enum device_endian {
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index a0e87be..36ab8b6 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -56,17 +56,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
target_ulong pc, target_ulong cs_base,
uint32_t flags,
int cflags);
-#if defined(CONFIG_USER_ONLY)
-void cpu_list_lock(void);
-void cpu_list_unlock(void);
-#else
-static inline void cpu_list_unlock(void)
-{
-}
-static inline void cpu_list_lock(void)
-{
-}
-#endif
void cpu_exec_init(CPUState *cpu, Error **errp);
void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 4aa9e61..ea3233f 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -545,6 +545,18 @@ static inline int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
#endif
/**
+ * cpu_list_add:
+ * @cpu: The CPU to be added to the list of CPUs.
+ */
+void cpu_list_add(CPUState *cpu);
+
+/**
+ * cpu_list_remove:
+ * @cpu: The CPU to be removed from the list of CPUs.
+ */
+void cpu_list_remove(CPUState *cpu);
+
+/**
* cpu_reset:
* @cpu: The CPU whose state is to be reset.
*/
diff --git a/linux-user/main.c b/linux-user/main.c
index 0f24d57..c3fefb6 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -111,7 +111,6 @@ 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 QemuMutex cpu_list_mutex;
static QemuMutex exclusive_lock;
static QemuCond exclusive_cond;
static QemuCond exclusive_resume;
@@ -119,7 +118,6 @@ 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);
@@ -128,6 +126,7 @@ void qemu_init_cpu_loop(void)
/* Make sure everything is in a consistent state for calling fork(). */
void fork_start(void)
{
+ cpu_list_lock();
qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
qemu_mutex_lock(&exclusive_lock);
mmap_fork_start();
@@ -147,14 +146,15 @@ void fork_end(int child)
}
pending_cpus = 0;
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);
+ qemu_init_cpu_list();
gdbserver_fork(thread_cpu);
} else {
qemu_mutex_unlock(&exclusive_lock);
qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
+ cpu_list_unlock();
}
}
@@ -221,16 +221,6 @@ static inline void cpu_exec_end(CPUState *cpu)
qemu_mutex_unlock(&exclusive_lock);
}
-void cpu_list_lock(void)
-{
- qemu_mutex_lock(&cpu_list_mutex);
-}
-
-void cpu_list_unlock(void)
-{
- qemu_mutex_unlock(&cpu_list_mutex);
-}
-
#ifdef TARGET_I386
/***********************************************************/
@@ -4229,6 +4219,7 @@ int main(int argc, char **argv, char **envp)
int ret;
int execfd;
+ qemu_init_cpu_list();
qemu_init_cpu_loop();
module_call_init(MODULE_INIT_QOM);
diff --git a/vl.c b/vl.c
index ee557a1..7b1b04a 100644
--- a/vl.c
+++ b/vl.c
@@ -2978,6 +2978,7 @@ int main(int argc, char **argv, char **envp)
Error *err = NULL;
bool list_data_dirs = false;
+ qemu_init_cpu_list();
qemu_init_cpu_loop();
qemu_mutex_lock_iothread();
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 07/16] cpus-common: move CPU work item management to common code
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (5 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
` (9 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee, Sergey Fedorov
From: Sergey Fedorov <serge.fdrv@gmail.com>
Make CPU work core functions common between system and user-mode
emulation. User-mode does not use run_on_cpu, so do not implement it.
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>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-10-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
bsd-user/main.c | 11 +++++--
cpus-common.c | 94 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
cpus.c | 82 +-----------------------------------------------
include/qom/cpu.h | 27 +++++++++++-----
linux-user/main.c | 25 +++++++++++++++
5 files changed, 148 insertions(+), 91 deletions(-)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 591c424..6dfa912 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -68,11 +68,11 @@ int cpu_get_pic_interrupt(CPUX86State *env)
#endif
/* These are no-ops because we are not threadsafe. */
-static inline void cpu_exec_start(CPUArchState *env)
+static inline void cpu_exec_start(CPUState *cpu)
{
}
-static inline void cpu_exec_end(CPUArchState *env)
+static inline void cpu_exec_end(CPUState *cpu)
{
}
@@ -164,7 +164,11 @@ void cpu_loop(CPUX86State *env)
//target_siginfo_t info;
for(;;) {
+ cpu_exec_start(cs);
trapnr = cpu_exec(cs);
+ cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch(trapnr) {
case 0x80:
/* syscall from int $0x80 */
@@ -505,7 +509,10 @@ void cpu_loop(CPUSPARCState *env)
//target_siginfo_t info;
while (1) {
+ cpu_exec_start(cs);
trapnr = cpu_exec(cs);
+ cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
switch (trapnr) {
#ifndef TARGET_SPARC64
diff --git a/cpus-common.c b/cpus-common.c
index 52198d8..b98f548 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -23,10 +23,12 @@
#include "exec/memory-internal.h"
static QemuMutex qemu_cpu_list_mutex;
+static QemuCond qemu_work_cond;
void qemu_init_cpu_list(void)
{
qemu_mutex_init(&qemu_cpu_list_mutex);
+ qemu_cond_init(&qemu_work_cond);
}
void cpu_list_lock(void)
@@ -81,3 +83,95 @@ void cpu_list_remove(CPUState *cpu)
cpu->cpu_index = UNASSIGNED_CPU_INDEX;
qemu_mutex_unlock(&qemu_cpu_list_mutex);
}
+
+struct qemu_work_item {
+ struct qemu_work_item *next;
+ run_on_cpu_func func;
+ void *data;
+ int done;
+ bool free;
+};
+
+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 do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
+ QemuMutex *mutex)
+{
+ 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, 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 ab15214..e1bdc16 100644
--- a/cpus.c
+++ b/cpus.c
@@ -901,73 +901,21 @@ 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)
{
qemu_init_sigbus();
qemu_cond_init(&qemu_cpu_cond);
qemu_cond_init(&qemu_pause_cond);
- qemu_cond_init(&qemu_work_cond);
qemu_cond_init(&qemu_io_proceeded_cond);
qemu_mutex_init(&qemu_global_mutex);
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;
-
- 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_global_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);
+ do_run_on_cpu(cpu, func, data, &qemu_global_mutex);
}
static void qemu_kvm_destroy_vcpu(CPUState *cpu)
@@ -982,34 +930,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/qom/cpu.h b/include/qom/cpu.h
index ea3233f..c04e510 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -233,14 +233,7 @@ struct kvm_run;
/* work queue */
typedef void (*run_on_cpu_func)(CPUState *cpu, void *data);
-
-struct qemu_work_item {
- struct qemu_work_item *next;
- run_on_cpu_func func;
- void *data;
- int done;
- bool free;
-};
+struct qemu_work_item;
/**
* CPUState:
@@ -630,6 +623,18 @@ void qemu_cpu_kick(CPUState *cpu);
bool cpu_is_stopped(CPUState *cpu);
/**
+ * do_run_on_cpu:
+ * @cpu: The vCPU to run on.
+ * @func: The function to be executed.
+ * @data: Data to pass to the function.
+ * @mutex: Mutex to release while waiting for @func to run.
+ *
+ * Used internally in the implementation of run_on_cpu.
+ */
+void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
+ QemuMutex *mutex);
+
+/**
* run_on_cpu:
* @cpu: The vCPU to run on.
* @func: The function to be executed.
@@ -808,6 +813,12 @@ void cpu_remove(CPUState *cpu);
void cpu_remove_sync(CPUState *cpu);
/**
+ * 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);
+
+/**
* qemu_init_vcpu:
* @cpu: The vCPU to initialize.
*
diff --git a/linux-user/main.c b/linux-user/main.c
index c3fefb6..e86093d 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -294,6 +294,8 @@ void cpu_loop(CPUX86State *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch(trapnr) {
case 0x80:
/* linux syscall from int $0x80 */
@@ -735,6 +737,8 @@ void cpu_loop(CPUARMState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch(trapnr) {
case EXCP_UDEF:
{
@@ -1071,6 +1075,7 @@ void cpu_loop(CPUARMState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
switch (trapnr) {
case EXCP_SWI:
@@ -1159,6 +1164,8 @@ void cpu_loop(CPUUniCore32State *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch (trapnr) {
case UC32_EXCP_PRIV:
{
@@ -1364,6 +1371,7 @@ void cpu_loop (CPUSPARCState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
/* Compute PSR before exposing state. */
if (env->cc_op != CC_OP_FLAGS) {
@@ -1636,6 +1644,8 @@ void cpu_loop(CPUPPCState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch(trapnr) {
case POWERPC_EXCP_NONE:
/* Just go on */
@@ -2482,6 +2492,8 @@ void cpu_loop(CPUMIPSState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch(trapnr) {
case EXCP_SYSCALL:
env->active_tc.PC += 4;
@@ -2722,6 +2734,7 @@ void cpu_loop(CPUOpenRISCState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
gdbsig = 0;
switch (trapnr) {
@@ -2816,6 +2829,7 @@ void cpu_loop(CPUSH4State *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
switch (trapnr) {
case 0x160:
@@ -2882,6 +2896,8 @@ void cpu_loop(CPUCRISState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch (trapnr) {
case 0xaa:
{
@@ -2947,6 +2963,8 @@ void cpu_loop(CPUMBState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch (trapnr) {
case 0xaa:
{
@@ -3064,6 +3082,8 @@ void cpu_loop(CPUM68KState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch(trapnr) {
case EXCP_ILLEGAL:
{
@@ -3207,6 +3227,7 @@ void cpu_loop(CPUAlphaState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
/* All of the traps imply a transition through PALcode, which
implies an REI instruction has been executed. Which means
@@ -3399,6 +3420,8 @@ void cpu_loop(CPUS390XState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch (trapnr) {
case EXCP_INTERRUPT:
/* Just indicate that signals should be handled asap. */
@@ -3708,6 +3731,8 @@ void cpu_loop(CPUTLGState *env)
cpu_exec_start(cs);
trapnr = cpu_exec(cs);
cpu_exec_end(cs);
+ process_queued_cpu_work(cs);
+
switch (trapnr) {
case TILEGX_EXCP_SYSCALL:
{
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (6 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 07/16] cpus-common: move CPU work item " Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-22 15:37 ` Alex Bennée
2016-09-12 11:12 ` [Qemu-devel] [PATCH 09/16] cpus-common: move exclusive work infrastructure from linux-user Paolo Bonzini
` (8 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index b98f548..cdfdb14 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -88,8 +88,7 @@ struct qemu_work_item {
struct qemu_work_item *next;
run_on_cpu_func func;
void *data;
- int done;
- bool free;
+ bool free, done;
};
static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
@@ -120,6 +119,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
wi.func = func;
wi.data = data;
+ wi.done = false;
wi.free = false;
queue_work_on_cpu(cpu, &wi);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 09/16] cpus-common: move exclusive work infrastructure from linux-user
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (7 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 10/16] docs: include formal model for TCG exclusive sections Paolo Bonzini
` (7 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
This will serve as the base for async_safe_run_on_cpu.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
bsd-user/main.c | 17 -----------
cpus-common.c | 82 +++++++++++++++++++++++++++++++++++++++++++++++++++
cpus.c | 2 ++
include/qom/cpu.h | 44 +++++++++++++++++++++++++++-
linux-user/main.c | 87 -------------------------------------------------------
5 files changed, 127 insertions(+), 105 deletions(-)
diff --git a/bsd-user/main.c b/bsd-user/main.c
index 6dfa912..35125b7 100644
--- a/bsd-user/main.c
+++ b/bsd-user/main.c
@@ -67,23 +67,6 @@ int cpu_get_pic_interrupt(CPUX86State *env)
}
#endif
-/* These are no-ops because we are not threadsafe. */
-static inline void cpu_exec_start(CPUState *cpu)
-{
-}
-
-static inline void cpu_exec_end(CPUState *cpu)
-{
-}
-
-static inline void start_exclusive(void)
-{
-}
-
-static inline void end_exclusive(void)
-{
-}
-
void fork_start(void)
{
}
diff --git a/cpus-common.c b/cpus-common.c
index cdfdb14..1a32522 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -23,11 +23,21 @@
#include "exec/memory-internal.h"
static QemuMutex qemu_cpu_list_mutex;
+static QemuCond exclusive_cond;
+static QemuCond exclusive_resume;
static QemuCond qemu_work_cond;
+static int pending_cpus;
+
void qemu_init_cpu_list(void)
{
+ /* This is needed because qemu_init_cpu_list is also called by the
+ * child process in a fork. */
+ pending_cpus = 0;
+
qemu_mutex_init(&qemu_cpu_list_mutex);
+ qemu_cond_init(&exclusive_cond);
+ qemu_cond_init(&exclusive_resume);
qemu_cond_init(&qemu_work_cond);
}
@@ -55,6 +65,12 @@ static int cpu_get_free_index(void)
return cpu_index;
}
+static void finish_safe_work(CPUState *cpu)
+{
+ cpu_exec_start(cpu);
+ cpu_exec_end(cpu);
+}
+
void cpu_list_add(CPUState *cpu)
{
qemu_mutex_lock(&qemu_cpu_list_mutex);
@@ -66,6 +82,8 @@ void cpu_list_add(CPUState *cpu)
}
QTAILQ_INSERT_TAIL(&cpus, cpu, node);
qemu_mutex_unlock(&qemu_cpu_list_mutex);
+
+ finish_safe_work(cpu);
}
void cpu_list_remove(CPUState *cpu)
@@ -148,6 +166,70 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
queue_work_on_cpu(cpu, wi);
}
+/* Wait for pending exclusive operations to complete. The exclusive lock
+ must be held. */
+static inline void exclusive_idle(void)
+{
+ while (pending_cpus) {
+ qemu_cond_wait(&exclusive_resume, &qemu_cpu_list_mutex);
+ }
+}
+
+/* Start an exclusive operation.
+ Must only be called from outside cpu_exec, takes
+ qemu_cpu_list_mutex. */
+void start_exclusive(void)
+{
+ CPUState *other_cpu;
+
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ exclusive_idle();
+
+ /* Make all other cpus stop executing. */
+ pending_cpus = 1;
+ CPU_FOREACH(other_cpu) {
+ if (other_cpu->running) {
+ pending_cpus++;
+ qemu_cpu_kick(other_cpu);
+ }
+ }
+ if (pending_cpus > 1) {
+ qemu_cond_wait(&exclusive_cond, &qemu_cpu_list_mutex);
+ }
+}
+
+/* Finish an exclusive operation. Releases qemu_cpu_list_mutex. */
+void end_exclusive(void)
+{
+ pending_cpus = 0;
+ qemu_cond_broadcast(&exclusive_resume);
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+}
+
+/* Wait for exclusive ops to finish, and begin cpu execution. */
+void cpu_exec_start(CPUState *cpu)
+{
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ exclusive_idle();
+ cpu->running = true;
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+}
+
+/* Mark cpu as not executing, and release pending exclusive ops. */
+void cpu_exec_end(CPUState *cpu)
+{
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ cpu->running = false;
+ if (pending_cpus > 1) {
+ pending_cpus--;
+ if (pending_cpus == 1) {
+ qemu_cond_signal(&exclusive_cond);
+ }
+ }
+ exclusive_idle();
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+}
+
void process_queued_cpu_work(CPUState *cpu)
{
struct qemu_work_item *wi;
diff --git a/cpus.c b/cpus.c
index e1bdc16..b024604 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1456,7 +1456,9 @@ static int tcg_cpu_exec(CPUState *cpu)
cpu->icount_decr.u16.low = decr;
cpu->icount_extra = count;
}
+ cpu_exec_start(cpu);
ret = cpu_exec(cpu);
+ cpu_exec_end(cpu);
#ifdef CONFIG_PROFILER
tcg_time += profile_getclock() - ti;
#endif
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index c04e510..f872614 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -242,7 +242,8 @@ struct qemu_work_item;
* @nr_threads: Number of threads within this CPU.
* @numa_node: NUMA node this CPU is belonging to.
* @host_tid: Host thread ID.
- * @running: #true if CPU is currently running (usermode).
+ * @running: #true if CPU is currently running;
+ * valid under cpu_list_lock.
* @created: Indicates whether the CPU thread has been successfully created.
* @interrupt_request: Indicates a pending interrupt request.
* @halted: Nonzero if the CPU is in suspended state.
@@ -819,6 +820,47 @@ void cpu_remove_sync(CPUState *cpu);
void process_queued_cpu_work(CPUState *cpu);
/**
+ * cpu_exec_start:
+ * @cpu: The CPU for the current thread.
+ *
+ * Record that a CPU has started execution and can be interrupted with
+ * cpu_exit.
+ */
+void cpu_exec_start(CPUState *cpu);
+
+/**
+ * cpu_exec_end:
+ * @cpu: The CPU for the current thread.
+ *
+ * Record that a CPU has stopped execution and exclusive sections
+ * can be executed without interrupting it.
+ */
+void cpu_exec_end(CPUState *cpu);
+
+/**
+ * start_exclusive:
+ *
+ * Wait for a concurrent exclusive section to end, and then start
+ * a section of work that is run while other CPUs are not running
+ * between cpu_exec_start and cpu_exec_end. CPUs that are running
+ * cpu_exec are exited immediately. CPUs that call cpu_exec_start
+ * during the exclusive section go to sleep until this CPU calls
+ * end_exclusive.
+ *
+ * Returns with the CPU list lock taken (which nests outside all
+ * other locks except the BQL).
+ */
+void start_exclusive(void);
+
+/**
+ * end_exclusive:
+ *
+ * Concludes an exclusive execution section started by start_exclusive.
+ * Releases the CPU list lock.
+ */
+void end_exclusive(void);
+
+/**
* qemu_init_vcpu:
* @cpu: The vCPU to initialize.
*
diff --git a/linux-user/main.c b/linux-user/main.c
index e86093d..11c5094 100644
--- a/linux-user/main.c
+++ b/linux-user/main.c
@@ -107,28 +107,11 @@ int cpu_get_pic_interrupt(CPUX86State *env)
/***********************************************************/
/* Helper routines for implementing atomic operations. */
-/* To implement exclusive operations we force all cpus to syncronise.
- 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 QemuMutex exclusive_lock;
-static QemuCond exclusive_cond;
-static QemuCond exclusive_resume;
-static int pending_cpus;
-
-void qemu_init_cpu_loop(void)
-{
- 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)
{
cpu_list_lock();
qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
- qemu_mutex_lock(&exclusive_lock);
mmap_fork_start();
}
@@ -144,84 +127,15 @@ void fork_end(int child)
QTAILQ_REMOVE(&cpus, cpu, node);
}
}
- pending_cpus = 0;
- qemu_mutex_init(&exclusive_lock);
- qemu_cond_init(&exclusive_cond);
- qemu_cond_init(&exclusive_resume);
qemu_mutex_init(&tcg_ctx.tb_ctx.tb_lock);
qemu_init_cpu_list();
gdbserver_fork(thread_cpu);
} else {
- qemu_mutex_unlock(&exclusive_lock);
qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
cpu_list_unlock();
}
}
-/* Wait for pending exclusive operations to complete. The exclusive lock
- must be held. */
-static inline void exclusive_idle(void)
-{
- while (pending_cpus) {
- qemu_cond_wait(&exclusive_resume, &exclusive_lock);
- }
-}
-
-/* Start an exclusive operation.
- Must only be called from outside cpu_exec. */
-static inline void start_exclusive(void)
-{
- CPUState *other_cpu;
-
- qemu_mutex_lock(&exclusive_lock);
- exclusive_idle();
-
- pending_cpus = 1;
- /* Make all other cpus stop executing. */
- CPU_FOREACH(other_cpu) {
- if (other_cpu->running) {
- pending_cpus++;
- cpu_exit(other_cpu);
- }
- }
- if (pending_cpus > 1) {
- qemu_cond_wait(&exclusive_cond, &exclusive_lock);
- }
-}
-
-/* Finish an exclusive operation. */
-static inline void __attribute__((unused)) end_exclusive(void)
-{
- pending_cpus = 0;
- 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)
-{
- qemu_mutex_lock(&exclusive_lock);
- exclusive_idle();
- cpu->running = true;
- qemu_mutex_unlock(&exclusive_lock);
-}
-
-/* Mark cpu as not executing, and release pending exclusive ops. */
-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);
- }
- }
- exclusive_idle();
- qemu_mutex_unlock(&exclusive_lock);
-}
-
-
#ifdef TARGET_I386
/***********************************************************/
/* CPUX86 core interface */
@@ -4245,7 +4159,6 @@ int main(int argc, char **argv, char **envp)
int execfd;
qemu_init_cpu_list();
- qemu_init_cpu_loop();
module_call_init(MODULE_INIT_QOM);
if ((envlist = envlist_create()) == NULL) {
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 10/16] docs: include formal model for TCG exclusive sections
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (8 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 09/16] cpus-common: move exclusive work infrastructure from linux-user Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 11/16] cpus-common: always defer async_run_on_cpu work items Paolo Bonzini
` (6 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
docs/tcg-exclusive.promela | 176 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 176 insertions(+)
create mode 100644 docs/tcg-exclusive.promela
diff --git a/docs/tcg-exclusive.promela b/docs/tcg-exclusive.promela
new file mode 100644
index 0000000..4c57398
--- /dev/null
+++ b/docs/tcg-exclusive.promela
@@ -0,0 +1,176 @@
+/*
+ * This model describes the implementation of exclusive sections in
+ * cpus-common.c (start_exclusive, end_exclusive, cpu_exec_start,
+ * cpu_exec_end).
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This file is in the public domain. If you really want a license,
+ * the WTFPL will do.
+ *
+ * To verify it:
+ * spin -a docs/event.promela
+ * ./a.out -a
+ *
+ * Tunable processor macros: N_CPUS, N_EXCLUSIVE, N_CYCLES, TEST_EXPENSIVE.
+ */
+
+// Define the missing parameters for the model
+#ifndef N_CPUS
+#define N_CPUS 2
+#warning defaulting to 2 CPU processes
+#endif
+
+// the expensive test is not so expensive for <= 3 CPUs
+#if N_CPUS <= 3
+#define TEST_EXPENSIVE
+#endif
+
+#ifndef N_EXCLUSIVE
+# if !defined N_CYCLES || N_CYCLES <= 1 || defined TEST_EXPENSIVE
+# define N_EXCLUSIVE 2
+# warning defaulting to 2 concurrent exclusive sections
+# else
+# define N_EXCLUSIVE 1
+# warning defaulting to 1 concurrent exclusive sections
+# endif
+#endif
+#ifndef N_CYCLES
+# if N_EXCLUSIVE <= 1 || defined TEST_EXPENSIVE
+# define N_CYCLES 2
+# warning defaulting to 2 CPU cycles
+# else
+# define N_CYCLES 1
+# warning defaulting to 1 CPU cycles
+# endif
+#endif
+
+
+// synchronization primitives. condition variables require a
+// process-local "cond_t saved;" variable.
+
+#define mutex_t byte
+#define MUTEX_LOCK(m) atomic { m == 0 -> m = 1 }
+#define MUTEX_UNLOCK(m) m = 0
+
+#define cond_t int
+#define COND_WAIT(c, m) { \
+ saved = c; \
+ MUTEX_UNLOCK(m); \
+ c != saved -> MUTEX_LOCK(m); \
+ }
+#define COND_BROADCAST(c) c++
+
+// this is the logic from cpus-common.c
+
+mutex_t mutex;
+cond_t exclusive_cond;
+cond_t exclusive_resume;
+byte pending_cpus;
+
+byte running[N_CPUS];
+byte has_waiter[N_CPUS];
+
+#define exclusive_idle() \
+ do \
+ :: pending_cpus -> COND_WAIT(exclusive_resume, mutex); \
+ :: else -> break; \
+ od
+
+#define start_exclusive() \
+ MUTEX_LOCK(mutex); \
+ exclusive_idle(); \
+ pending_cpus = 1; \
+ \
+ i = 0; \
+ do \
+ :: i < N_CPUS -> { \
+ if \
+ :: running[i] -> has_waiter[i] = 1; pending_cpus++; \
+ :: else -> skip; \
+ fi; \
+ i++; \
+ } \
+ :: else -> break; \
+ od; \
+ \
+ do \
+ :: pending_cpus > 1 -> COND_WAIT(exclusive_cond, mutex); \
+ :: else -> break; \
+ od
+
+#define end_exclusive() \
+ pending_cpus = 0; \
+ COND_BROADCAST(exclusive_resume); \
+ MUTEX_UNLOCK(mutex);
+
+#define cpu_exec_start(id) \
+ MUTEX_LOCK(mutex); \
+ exclusive_idle(); \
+ running[id] = 1; \
+ MUTEX_UNLOCK(mutex);
+
+#define cpu_exec_end(id) \
+ MUTEX_LOCK(mutex); \
+ running[id] = 0; \
+ if \
+ :: pending_cpus -> { \
+ pending_cpus--; \
+ if \
+ :: pending_cpus == 1 -> COND_BROADCAST(exclusive_cond); \
+ :: else -> skip; \
+ fi; \
+ } \
+ :: else -> skip; \
+ fi; \
+ exclusive_idle(); \
+ MUTEX_UNLOCK(mutex);
+
+// Promela processes
+
+byte done_cpu;
+byte in_cpu;
+active[N_CPUS] proctype cpu()
+{
+ byte id = _pid % N_CPUS;
+ byte cycles = 0;
+ cond_t saved;
+
+ do
+ :: cycles == N_CYCLES -> break;
+ :: else -> {
+ cycles++;
+ cpu_exec_start(id)
+ in_cpu++;
+ done_cpu++;
+ in_cpu--;
+ cpu_exec_end(id)
+ }
+ od;
+}
+
+byte done_exclusive;
+byte in_exclusive;
+active[N_EXCLUSIVE] proctype exclusive()
+{
+ cond_t saved;
+ byte i;
+
+ start_exclusive();
+ in_exclusive = 1;
+ done_exclusive++;
+ in_exclusive = 0;
+ end_exclusive();
+}
+
+#define LIVENESS (done_cpu == N_CPUS * N_CYCLES && done_exclusive == N_EXCLUSIVE)
+#define SAFETY !(in_exclusive && in_cpu)
+
+never { /* ! ([] SAFETY && <> [] LIVENESS) */
+ do
+ // once the liveness property is satisfied, this is not executable
+ // and the never clause is not accepted
+ :: ! LIVENESS -> accept_liveness: skip
+ :: 1 -> assert(SAFETY)
+ od;
+}
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 11/16] cpus-common: always defer async_run_on_cpu work items
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (9 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 10/16] docs: include formal model for TCG exclusive sections Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 12/16] cpus-common: remove redundant call to exclusive_idle() Paolo Bonzini
` (5 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
async_run_on_cpu is only called from the I/O thread, not from CPU threads,
so it doesn't make any difference. It will make a difference however
for async_safe_run_on_cpu.
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 1a32522..09b19f8 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -153,11 +153,6 @@ 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;
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 12/16] cpus-common: remove redundant call to exclusive_idle()
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (10 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 11/16] cpus-common: always defer async_run_on_cpu work items Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 13/16] cpus-common: simplify locking for start_exclusive/end_exclusive Paolo Bonzini
` (4 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
No need to call exclusive_idle() from cpu_exec_end since it is done
immediately afterwards in cpu_exec_start. Any exclusive section could
run as soon as cpu_exec_end leaves, because cpu->running is false and the
mutex is not taken, so the call does not add any protection either.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 1 -
docs/tcg-exclusive.promela | 1 -
2 files changed, 2 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 09b19f8..cabcb9f 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -221,7 +221,6 @@ void cpu_exec_end(CPUState *cpu)
qemu_cond_signal(&exclusive_cond);
}
}
- exclusive_idle();
qemu_mutex_unlock(&qemu_cpu_list_mutex);
}
diff --git a/docs/tcg-exclusive.promela b/docs/tcg-exclusive.promela
index 4c57398..7793e31 100644
--- a/docs/tcg-exclusive.promela
+++ b/docs/tcg-exclusive.promela
@@ -123,7 +123,6 @@ byte has_waiter[N_CPUS];
} \
:: else -> skip; \
fi; \
- exclusive_idle(); \
MUTEX_UNLOCK(mutex);
// Promela processes
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 13/16] cpus-common: simplify locking for start_exclusive/end_exclusive
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (11 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 12/16] cpus-common: remove redundant call to exclusive_idle() Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu() Paolo Bonzini
` (3 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
It is not necessary to hold qemu_cpu_list_mutex throughout the
exclusive section, because no other exclusive section can run
while pending_cpus != 0.
exclusive_idle() is called in cpu_exec_start(), and that prevents
any CPUs created after start_exclusive() from entering cpu_exec()
during an exclusive section.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 6 ++++++
docs/tcg-exclusive.promela | 4 +++-
include/qom/cpu.h | 4 ----
3 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index cabcb9f..12c8e69 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -191,11 +191,17 @@ void start_exclusive(void)
if (pending_cpus > 1) {
qemu_cond_wait(&exclusive_cond, &qemu_cpu_list_mutex);
}
+
+ /* Can release mutex, no one will enter another exclusive
+ * section until end_exclusive resets pending_cpus to 0.
+ */
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
}
/* Finish an exclusive operation. Releases qemu_cpu_list_mutex. */
void end_exclusive(void)
{
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
pending_cpus = 0;
qemu_cond_broadcast(&exclusive_resume);
qemu_mutex_unlock(&qemu_cpu_list_mutex);
diff --git a/docs/tcg-exclusive.promela b/docs/tcg-exclusive.promela
index 7793e31..3ef0f34 100644
--- a/docs/tcg-exclusive.promela
+++ b/docs/tcg-exclusive.promela
@@ -97,9 +97,11 @@ byte has_waiter[N_CPUS];
do \
:: pending_cpus > 1 -> COND_WAIT(exclusive_cond, mutex); \
:: else -> break; \
- od
+ od; \
+ MUTEX_UNLOCK(mutex);
#define end_exclusive() \
+ MUTEX_LOCK(mutex); \
pending_cpus = 0; \
COND_BROADCAST(exclusive_resume); \
MUTEX_UNLOCK(mutex);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index f872614..934c07a 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -846,9 +846,6 @@ void cpu_exec_end(CPUState *cpu);
* cpu_exec are exited immediately. CPUs that call cpu_exec_start
* during the exclusive section go to sleep until this CPU calls
* end_exclusive.
- *
- * Returns with the CPU list lock taken (which nests outside all
- * other locks except the BQL).
*/
void start_exclusive(void);
@@ -856,7 +853,6 @@ void start_exclusive(void);
* end_exclusive:
*
* Concludes an exclusive execution section started by start_exclusive.
- * Releases the CPU list lock.
*/
void end_exclusive(void);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu()
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (12 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 13/16] cpus-common: simplify locking for start_exclusive/end_exclusive Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 15/16] tcg: Make tb_flush() thread safe Paolo Bonzini
` (2 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 25 +++++++++++++++++++++++--
include/qom/cpu.h | 11 +++++++++++
2 files changed, 34 insertions(+), 2 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 12c8e69..50a92dd 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -106,7 +106,7 @@ struct qemu_work_item {
struct qemu_work_item *next;
run_on_cpu_func func;
void *data;
- bool free, done;
+ bool free, exclusive, done;
};
static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
@@ -139,6 +139,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
wi.data = data;
wi.done = false;
wi.free = false;
+ wi.exclusive = false;
queue_work_on_cpu(cpu, &wi);
while (!atomic_mb_read(&wi.done)) {
@@ -157,6 +158,7 @@ void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
wi->func = func;
wi->data = data;
wi->free = true;
+ wi->exclusive = false;
queue_work_on_cpu(cpu, wi);
}
@@ -230,6 +232,19 @@ void cpu_exec_end(CPUState *cpu)
qemu_mutex_unlock(&qemu_cpu_list_mutex);
}
+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->exclusive = true;
+
+ queue_work_on_cpu(cpu, wi);
+}
+
void process_queued_cpu_work(CPUState *cpu)
{
struct qemu_work_item *wi;
@@ -246,7 +261,13 @@ void process_queued_cpu_work(CPUState *cpu)
cpu->queued_work_last = NULL;
}
qemu_mutex_unlock(&cpu->work_mutex);
- wi->func(cpu, wi->data);
+ if (wi->exclusive) {
+ start_exclusive();
+ wi->func(cpu, wi->data);
+ end_exclusive();
+ } else {
+ wi->func(cpu, wi->data);
+ }
qemu_mutex_lock(&cpu->work_mutex);
if (wi->free) {
g_free(wi);
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 934c07a..d1ca31c 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -656,6 +656,17 @@ 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,
+ * while all other vCPUs are sleeping.
+ */
+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.
*
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 15/16] tcg: Make tb_flush() thread safe
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (13 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu() Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 16/16] cpus-common: lock-free fast path for cpu_exec_start/end Paolo Bonzini
2016-09-14 17:16 ` [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Richard Henderson
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee, Sergey Fedorov
From: Sergey Fedorov <serge.fdrv@gmail.com>
Use async_safe_run_on_cpu() to make tb_flush() thread safe. This is
possible now that code generation does not happen in the middle of
execution.
It can happen that multiple threads schedule a safe work to flush the
translation buffer. To keep statistics and debugging output sane, always
check if the translation buffer has already been flushed.
Signed-off-by: Sergey Fedorov <serge.fdrv@gmail.com>
Signed-off-by: Sergey Fedorov <sergey.fedorov@linaro.org>
[AJB: minor re-base fixes]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <1470158864-17651-13-git-send-email-alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpu-exec.c | 12 ++----------
include/exec/tb-context.h | 2 +-
include/qom/cpu.h | 2 --
translate-all.c | 38 ++++++++++++++++++++++++++++----------
4 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/cpu-exec.c b/cpu-exec.c
index b240b9f..a8ff2a1 100644
--- a/cpu-exec.c
+++ b/cpu-exec.c
@@ -203,20 +203,16 @@ static void cpu_exec_nocache(CPUState *cpu, int max_cycles,
TranslationBlock *orig_tb, bool ignore_icount)
{
TranslationBlock *tb;
- bool old_tb_flushed;
/* Should never happen.
We only end up here when an existing TB is too long. */
if (max_cycles > CF_COUNT_MASK)
max_cycles = CF_COUNT_MASK;
- old_tb_flushed = cpu->tb_flushed;
- cpu->tb_flushed = false;
tb = tb_gen_code(cpu, orig_tb->pc, orig_tb->cs_base, orig_tb->flags,
max_cycles | CF_NOCACHE
| (ignore_icount ? CF_IGNORE_ICOUNT : 0));
- tb->orig_tb = cpu->tb_flushed ? NULL : orig_tb;
- cpu->tb_flushed |= old_tb_flushed;
+ tb->orig_tb = orig_tb;
/* execute the generated code */
trace_exec_tb_nocache(tb, tb->pc);
cpu_tb_exec(cpu, tb);
@@ -337,10 +333,7 @@ static inline TranslationBlock *tb_find(CPUState *cpu,
tb_lock();
have_tb_lock = true;
}
- /* Check if translation buffer has been flushed */
- if (cpu->tb_flushed) {
- cpu->tb_flushed = false;
- } else if (!tb->invalid) {
+ if (!tb->invalid) {
tb_add_jump(last_tb, tb_exit, tb);
}
}
@@ -605,7 +598,6 @@ int cpu_exec(CPUState *cpu)
break;
}
- atomic_mb_set(&cpu->tb_flushed, false); /* reset before first TB lookup */
for(;;) {
cpu_handle_interrupt(cpu, &last_tb);
tb = tb_find(cpu, last_tb, tb_exit);
diff --git a/include/exec/tb-context.h b/include/exec/tb-context.h
index dce95d9..c7f17f2 100644
--- a/include/exec/tb-context.h
+++ b/include/exec/tb-context.h
@@ -38,7 +38,7 @@ struct TBContext {
QemuMutex tb_lock;
/* statistics */
- int tb_flush_count;
+ unsigned tb_flush_count;
int tb_phys_invalidate_count;
};
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index d1ca31c..3eb595c 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -253,7 +253,6 @@ struct qemu_work_item;
* @crash_occurred: Indicates the OS reported a crash (panic) for this CPU
* @tcg_exit_req: Set to force TCG to stop executing linked TBs for this
* CPU and return to its top level loop.
- * @tb_flushed: Indicates the translation buffer has been flushed.
* @singlestep_enabled: Flags for single-stepping.
* @icount_extra: Instructions until next timer event.
* @icount_decr: Number of cycles left, with interrupt flag in high bit.
@@ -306,7 +305,6 @@ struct CPUState {
bool unplug;
bool crash_occurred;
bool exit_request;
- bool tb_flushed;
uint32_t interrupt_request;
int singlestep_enabled;
int64_t icount_extra;
diff --git a/translate-all.c b/translate-all.c
index b6663dc..ab657e7 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -832,12 +832,19 @@ 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 (!tcg_enabled()) {
- return;
+ unsigned tb_flush_req = (unsigned) (uintptr_t) data;
+
+ tb_lock();
+
+ /* If it's already been done on request of another CPU,
+ * just retry.
+ */
+ if (atomic_read(&tcg_ctx.tb_ctx.tb_flush_count) != tb_flush_req) {
+ goto done;
}
+
#if defined(DEBUG_FLUSH)
printf("qemu: flush code_size=%ld nb_tbs=%d avg_tb_size=%ld\n",
(unsigned long)(tcg_ctx.code_gen_ptr - tcg_ctx.code_gen_buffer),
@@ -856,7 +863,6 @@ void tb_flush(CPUState *cpu)
for (i = 0; i < TB_JMP_CACHE_SIZE; ++i) {
atomic_set(&cpu->tb_jmp_cache[i], NULL);
}
- atomic_mb_set(&cpu->tb_flushed, true);
}
tcg_ctx.tb_ctx.nb_tbs = 0;
@@ -866,7 +872,19 @@ void tb_flush(CPUState *cpu)
tcg_ctx.code_gen_ptr = tcg_ctx.code_gen_buffer;
/* XXX: flush processor icache at this point if cache flush is
expensive */
- tcg_ctx.tb_ctx.tb_flush_count++;
+ atomic_inc(&tcg_ctx.tb_ctx.tb_flush_count);
+
+done:
+ tb_unlock();
+}
+
+void tb_flush(CPUState *cpu)
+{
+ if (tcg_enabled()) {
+ uintptr_t tb_flush_req = (uintptr_t)
+ atomic_read(&tcg_ctx.tb_ctx.tb_flush_count);
+ async_safe_run_on_cpu(cpu, do_tb_flush, (void *) tb_flush_req);
+ }
}
#ifdef DEBUG_TB_CHECK
@@ -1173,9 +1191,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;
@@ -1773,7 +1790,8 @@ void dump_exec_info(FILE *f, fprintf_function cpu_fprintf)
qht_statistics_destroy(&hst);
cpu_fprintf(f, "\nStatistics:\n");
- cpu_fprintf(f, "TB flush count %d\n", tcg_ctx.tb_ctx.tb_flush_count);
+ cpu_fprintf(f, "TB flush count %d\n",
+ atomic_read(&tcg_ctx.tb_ctx.tb_flush_count));
cpu_fprintf(f, "TB invalidate count %d\n",
tcg_ctx.tb_ctx.tb_phys_invalidate_count);
cpu_fprintf(f, "TLB flush count %d\n", tlb_flush_count);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 16/16] cpus-common: lock-free fast path for cpu_exec_start/end
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (14 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 15/16] tcg: Make tb_flush() thread safe Paolo Bonzini
@ 2016-09-12 11:12 ` Paolo Bonzini
2016-09-14 17:16 ` [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Richard Henderson
16 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-12 11:12 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, alex.bennee
Set cpu->running without taking the cpu_list lock, only look at it if
there is a concurrent exclusive section. This requires adding a new
field to CPUState, which records whether a running CPU is being counted
in pending_cpus. When an exclusive section is started concurrently with
cpu_exec_start, cpu_exec_start can use the new field to wait for the end
of the exclusive section.
This a separate patch for easier bisection of issues.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 73 ++++++++++++++++++++++++++++++++++++++++------
docs/tcg-exclusive.promela | 53 +++++++++++++++++++++++++++++++--
include/qom/cpu.h | 5 ++--
3 files changed, 117 insertions(+), 14 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 50a92dd..67b42c6 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -184,8 +184,12 @@ void start_exclusive(void)
/* Make all other cpus stop executing. */
pending_cpus = 1;
+
+ /* Write pending_cpus before reading other_cpu->running. */
+ smp_mb();
CPU_FOREACH(other_cpu) {
if (other_cpu->running) {
+ other_cpu->has_waiter = true;
pending_cpus++;
qemu_cpu_kick(other_cpu);
}
@@ -212,24 +216,75 @@ void end_exclusive(void)
/* Wait for exclusive ops to finish, and begin cpu execution. */
void cpu_exec_start(CPUState *cpu)
{
- qemu_mutex_lock(&qemu_cpu_list_mutex);
- exclusive_idle();
cpu->running = true;
- qemu_mutex_unlock(&qemu_cpu_list_mutex);
+
+ /* Write cpu->running before reading pending_cpus. */
+ smp_mb();
+
+ /* 1. start_exclusive saw cpu->running == true and pending_cpus >= 1.
+ * After taking the lock we'll see cpu->has_waiter == true and run---not
+ * for long because start_exclusive kicked us. cpu_exec_end will
+ * decrement pending_cpus and signal the waiter.
+ *
+ * 2. start_exclusive saw cpu->running == false but pending_cpus >= 1.
+ * This includes the case when an exclusive item is running now.
+ * Then we'll see cpu->has_waiter == false and wait for the item to
+ * complete.
+ *
+ * 3. pending_cpus == 0. Then start_exclusive is definitely going to
+ * see cpu->running == true, and it will kick the CPU.
+ */
+ if (pending_cpus) {
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ if (!cpu->has_waiter) {
+ /* Not counted in pending_cpus, let the exclusive item
+ * run. Since we have the lock, set cpu->running to true
+ * while holding it instead of retrying.
+ */
+ cpu->running = false;
+ exclusive_idle();
+ /* Now pending_cpus is zero. */
+ cpu->running = true;
+ } else {
+ /* Counted in pending_cpus, go ahead. */
+ }
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
+ }
}
/* Mark cpu as not executing, and release pending exclusive ops. */
void cpu_exec_end(CPUState *cpu)
{
- qemu_mutex_lock(&qemu_cpu_list_mutex);
cpu->running = false;
- if (pending_cpus > 1) {
- pending_cpus--;
- if (pending_cpus == 1) {
- qemu_cond_signal(&exclusive_cond);
+
+ /* Write cpu->running before reading pending_cpus. */
+ smp_mb();
+
+ /* 1. start_exclusive saw cpu->running == true. Then it will increment
+ * pending_cpus and wait for exclusive_cond. After taking the lock
+ * we'll see cpu->has_waiter == true.
+ *
+ * 2. start_exclusive saw cpu->running == false but here pending_cpus >= 1.
+ * This includes the case when an exclusive item started after setting
+ * cpu->running to false and before we read pending_cpus. Then we'll see
+ * cpu->has_waiter == false and not touch pending_cpus. The next call to
+ * cpu_exec_start will run exclusive_idle if still necessary, thus waiting
+ * for the item to complete.
+ *
+ * 3. pending_cpus == 0. Then start_exclusive is definitely going to
+ * see cpu->running == false, and it can ignore this CPU until the
+ * next cpu_exec_start.
+ */
+ if (pending_cpus) {
+ qemu_mutex_lock(&qemu_cpu_list_mutex);
+ if (cpu->has_waiter) {
+ cpu->has_waiter = false;
+ if (--pending_cpus == 1) {
+ qemu_cond_signal(&exclusive_cond);
+ }
}
+ qemu_mutex_unlock(&qemu_cpu_list_mutex);
}
- qemu_mutex_unlock(&qemu_cpu_list_mutex);
}
void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data)
diff --git a/docs/tcg-exclusive.promela b/docs/tcg-exclusive.promela
index 3ef0f34..f21213f 100644
--- a/docs/tcg-exclusive.promela
+++ b/docs/tcg-exclusive.promela
@@ -12,7 +12,8 @@
* spin -a docs/event.promela
* ./a.out -a
*
- * Tunable processor macros: N_CPUS, N_EXCLUSIVE, N_CYCLES, TEST_EXPENSIVE.
+ * Tunable processor macros: N_CPUS, N_EXCLUSIVE, N_CYCLES, USE_MUTEX,
+ * TEST_EXPENSIVE.
*/
// Define the missing parameters for the model
@@ -21,8 +22,10 @@
#warning defaulting to 2 CPU processes
#endif
-// the expensive test is not so expensive for <= 3 CPUs
-#if N_CPUS <= 3
+// the expensive test is not so expensive for <= 2 CPUs
+// If the mutex is used, it's also cheap (300 MB / 4 seconds) for 3 CPUs
+// For 3 CPUs and the lock-free option it needs 1.5 GB of RAM
+#if N_CPUS <= 2 || (N_CPUS <= 3 && defined USE_MUTEX)
#define TEST_EXPENSIVE
#endif
@@ -106,6 +109,8 @@ byte has_waiter[N_CPUS];
COND_BROADCAST(exclusive_resume); \
MUTEX_UNLOCK(mutex);
+#ifdef USE_MUTEX
+// Simple version using mutexes
#define cpu_exec_start(id) \
MUTEX_LOCK(mutex); \
exclusive_idle(); \
@@ -126,6 +131,48 @@ byte has_waiter[N_CPUS];
:: else -> skip; \
fi; \
MUTEX_UNLOCK(mutex);
+#else
+// Wait-free fast path, only needs mutex when concurrent with
+// an exclusive section
+#define cpu_exec_start(id) \
+ running[id] = 1; \
+ if \
+ :: pending_cpus -> { \
+ MUTEX_LOCK(mutex); \
+ if \
+ :: !has_waiter[id] -> { \
+ running[id] = 0; \
+ exclusive_idle(); \
+ running[id] = 1; \
+ } \
+ :: else -> skip; \
+ fi; \
+ MUTEX_UNLOCK(mutex); \
+ } \
+ :: else -> skip; \
+ fi;
+
+#define cpu_exec_end(id) \
+ running[id] = 0; \
+ if \
+ :: pending_cpus -> { \
+ MUTEX_LOCK(mutex); \
+ if \
+ :: has_waiter[id] -> { \
+ has_waiter[id] = 0; \
+ pending_cpus--; \
+ if \
+ :: pending_cpus == 1 -> COND_BROADCAST(exclusive_cond); \
+ :: else -> skip; \
+ fi; \
+ } \
+ :: else -> skip; \
+ fi; \
+ MUTEX_UNLOCK(mutex); \
+ } \
+ :: else -> skip; \
+ fi
+#endif
// Promela processes
diff --git a/include/qom/cpu.h b/include/qom/cpu.h
index 3eb595c..7589e46 100644
--- a/include/qom/cpu.h
+++ b/include/qom/cpu.h
@@ -242,7 +242,8 @@ struct qemu_work_item;
* @nr_threads: Number of threads within this CPU.
* @numa_node: NUMA node this CPU is belonging to.
* @host_tid: Host thread ID.
- * @running: #true if CPU is currently running;
+ * @running: #true if CPU is currently running (lockless).
+ * @has_waiter: #true if a CPU is currently waiting for the cpu_exec_end;
* valid under cpu_list_lock.
* @created: Indicates whether the CPU thread has been successfully created.
* @interrupt_request: Indicates a pending interrupt request.
@@ -296,7 +297,7 @@ struct CPUState {
#endif
int thread_id;
uint32_t host_tid;
- bool running;
+ bool running, has_waiter;
struct QemuCond *halt_cond;
bool thread_kicked;
bool created;
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
` (15 preceding siblings ...)
2016-09-12 11:12 ` [Qemu-devel] [PATCH 16/16] cpus-common: lock-free fast path for cpu_exec_start/end Paolo Bonzini
@ 2016-09-14 17:16 ` Richard Henderson
2016-09-14 18:40 ` Paolo Bonzini
16 siblings, 1 reply; 27+ messages in thread
From: Richard Henderson @ 2016-09-14 17:16 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: alex.bennee, sergey.fedorov
On 09/12/2016 04:12 AM, Paolo Bonzini wrote:
> In addition to fixing some of the issues found by Alex, safe work items
> need not run anymore with a mutex taken. Of course, cpu_exec_start/end
> and start_exclusive/end_exclusive are essentially the read and write
> side of a specialized rwlock, so there is still a lock in disguise looming
> to cause deadlocks; however, it does removes worries about recursive
> locking from CPU list manipulations.
>
> The new patches are 8, 12 and 13. Patch 12 of v6 has been split
> across patch 10 and patch 16.
>
> Paolo
>
> v6->v7: Do not separate qemu_work_item and SafeWorkItem
> More cleanups/optimizations of exclusive section logic
What tree is this based on?
The patch set doesn't apply cleanly to master or v2.7.0.
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state
2016-09-14 17:16 ` [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Richard Henderson
@ 2016-09-14 18:40 ` Paolo Bonzini
0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-14 18:40 UTC (permalink / raw)
To: Richard Henderson, qemu-devel; +Cc: alex.bennee, sergey.fedorov
On 14/09/2016 19:16, Richard Henderson wrote:
> On 09/12/2016 04:12 AM, Paolo Bonzini wrote:
>> In addition to fixing some of the issues found by Alex, safe work items
>> need not run anymore with a mutex taken. Of course, cpu_exec_start/end
>> and start_exclusive/end_exclusive are essentially the read and write
>> side of a specialized rwlock, so there is still a lock in disguise looming
>> to cause deadlocks; however, it does removes worries about recursive
>> locking from CPU list manipulations.
>>
>> The new patches are 8, 12 and 13. Patch 12 of v6 has been split
>> across patch 10 and patch 16.
>>
>> Paolo
>>
>> v6->v7: Do not separate qemu_work_item and SafeWorkItem
>> More cleanups/optimizations of exclusive section logic
>
> What tree is this based on?
> The patch set doesn't apply cleanly to master or v2.7.0.
It's based on the pull request I sent yesterday.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers
2016-09-12 11:12 ` [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers Paolo Bonzini
@ 2016-09-15 15:30 ` Alex Bennée
0 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2016-09-15 15:30 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, sergey.fedorov
Paolo Bonzini <pbonzini@redhat.com> writes:
> 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>
> Acked-by: David Gibson <david@gibson.dropbear.id.au> (ppc parts)
> Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> (s390 parts)
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <1470158864-17651-3-git-send-email-alex.bennee@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> 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(-)
>
<snip>
>
> diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
> index 3bf1ddd..1bc02fb 100644
> --- a/hw/i386/kvmvapic.c
> +++ b/hw/i386/kvmvapic.c
There is also a new conflict that needs fixing in apic.c on your next
re-base for:
static void kvm_apic_put(CPUState *cs, void *data)
> @@ -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 22c584e..8e16f65 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)
--
Alex Bennée
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code
2016-09-12 11:12 ` [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code Paolo Bonzini
@ 2016-09-22 15:24 ` Alex Bennée
2016-09-22 15:27 ` Paolo Bonzini
0 siblings, 1 reply; 27+ messages in thread
From: Alex Bennée @ 2016-09-22 15:24 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, sergey.fedorov
Paolo Bonzini <pbonzini@redhat.com> writes:
> Add a mutex for the CPU list to system emulation, as it will be used to
> manage safe work. Abstract manipulation of the CPU list in new functions
> cpu_list_add and cpu_list_remove.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> Makefile.target | 2 +-
> bsd-user/main.c | 9 +----
> cpus-common.c | 83 +++++++++++++++++++++++++++++++++++++++++++++++
> exec.c | 37 ++-------------------
> include/exec/cpu-all.h | 4 +++
> include/exec/cpu-common.h | 2 ++
> include/exec/exec-all.h | 11 -------
> include/qom/cpu.h | 12 +++++++
> linux-user/main.c | 17 +++-------
> vl.c | 1 +
> 10 files changed, 110 insertions(+), 68 deletions(-)
> create mode 100644 cpus-common.c
>
> diff --git a/Makefile.target b/Makefile.target
> index 8c7a072..5f2cf85 100644
> --- a/Makefile.target
> +++ b/Makefile.target
> @@ -86,7 +86,7 @@ all: $(PROGS) stap
> # cpu emulator library
> obj-y = exec.o translate-all.o cpu-exec.o
> obj-y += translate-common.o
> -obj-y += cpu-exec-common.o
> +obj-y += cpu-exec-common.o cpus-common.o
> obj-y += tcg/tcg.o tcg/tcg-op.o tcg/optimize.o
> obj-$(CONFIG_TCG_INTERPRETER) += tci.o
> obj-y += tcg/tcg-common.o
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 0fb08e4..591c424 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -95,14 +95,6 @@ void fork_end(int child)
> }
> }
>
> -void cpu_list_lock(void)
> -{
> -}
> -
> -void cpu_list_unlock(void)
> -{
> -}
> -
> #ifdef TARGET_I386
> /***********************************************************/
> /* CPUX86 core interface */
> @@ -748,6 +740,7 @@ int main(int argc, char **argv)
> if (argc <= 1)
> usage();
>
> + qemu_init_cpu_list();
> module_call_init(MODULE_INIT_QOM);
>
> if ((envlist = envlist_create()) == NULL) {
> diff --git a/cpus-common.c b/cpus-common.c
> new file mode 100644
> index 0000000..52198d8
> --- /dev/null
> +++ b/cpus-common.c
> @@ -0,0 +1,83 @@
> +/*
> + * CPU thread main loop - common bits for user and system mode emulation
> + *
> + * Copyright (c) 2003-2005 Fabrice Bellard
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library; if not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include "qemu/osdep.h"
> +#include "cpu.h"
> +#include "sysemu/cpus.h"
> +#include "exec/memory-internal.h"
> +
> +static QemuMutex qemu_cpu_list_mutex;
> +
> +void qemu_init_cpu_list(void)
> +{
> + qemu_mutex_init(&qemu_cpu_list_mutex);
> +}
> +
> +void cpu_list_lock(void)
> +{
> + qemu_mutex_lock(&qemu_cpu_list_mutex);
> +}
> +
> +void cpu_list_unlock(void)
> +{
> + qemu_mutex_unlock(&qemu_cpu_list_mutex);
> +}
> +
> +static bool cpu_index_auto_assigned;
> +
> +static int cpu_get_free_index(void)
> +{
> + CPUState *some_cpu;
> + int cpu_index = 0;
> +
> + cpu_index_auto_assigned = true;
> + CPU_FOREACH(some_cpu) {
> + cpu_index++;
> + }
> + return cpu_index;
> +}
I see this is code moved from elsewhere but I feel a little comment
about this should probably accompany the variable. I think it means once
you have added a CPU without an explicit index all future CPUs will be
auto assigned and you can no longer add CPUs with a specific index?
Does this mean you can't hotplug additional CPUs without having
specified a CPU topology at start-up?
> +
> +void cpu_list_add(CPUState *cpu)
> +{
> + qemu_mutex_lock(&qemu_cpu_list_mutex);
> + if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
> + cpu->cpu_index = cpu_get_free_index();
> + assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
> + } else {
> + assert(!cpu_index_auto_assigned);
> + }
> + QTAILQ_INSERT_TAIL(&cpus, cpu, node);
> + qemu_mutex_unlock(&qemu_cpu_list_mutex);
> +}
> +
> +void cpu_list_remove(CPUState *cpu)
> +{
> + qemu_mutex_lock(&qemu_cpu_list_mutex);
> + if (!QTAILQ_IN_USE(cpu, node)) {
> + /* there is nothing to undo since cpu_exec_init() hasn't been called */
> + qemu_mutex_unlock(&qemu_cpu_list_mutex);
> + return;
> + }
> +
> + assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
> +
> + QTAILQ_REMOVE(&cpus, cpu, node);
> + cpu->cpu_index = UNASSIGNED_CPU_INDEX;
> + qemu_mutex_unlock(&qemu_cpu_list_mutex);
> +}
> diff --git a/exec.c b/exec.c
> index ce3fb9e..784990c 100644
> --- a/exec.c
> +++ b/exec.c
> @@ -598,36 +598,11 @@ AddressSpace *cpu_get_address_space(CPUState *cpu, int asidx)
> }
> #endif
>
> -static bool cpu_index_auto_assigned;
> -
> -static int cpu_get_free_index(void)
> -{
> - CPUState *some_cpu;
> - int cpu_index = 0;
> -
> - cpu_index_auto_assigned = true;
> - CPU_FOREACH(some_cpu) {
> - cpu_index++;
> - }
> - return cpu_index;
> -}
> -
> void cpu_exec_exit(CPUState *cpu)
> {
> CPUClass *cc = CPU_GET_CLASS(cpu);
>
> - cpu_list_lock();
> - if (!QTAILQ_IN_USE(cpu, node)) {
> - /* there is nothing to undo since cpu_exec_init() hasn't been called */
> - cpu_list_unlock();
> - return;
> - }
> -
> - assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus, CPUTailQ)));
> -
> - QTAILQ_REMOVE(&cpus, cpu, node);
> - cpu->cpu_index = UNASSIGNED_CPU_INDEX;
> - cpu_list_unlock();
> + cpu_list_remove(cpu);
>
> if (cc->vmsd != NULL) {
> vmstate_unregister(NULL, cc->vmsd, cpu);
> @@ -663,15 +638,7 @@ void cpu_exec_init(CPUState *cpu, Error **errp)
> object_ref(OBJECT(cpu->memory));
> #endif
>
> - cpu_list_lock();
> - if (cpu->cpu_index == UNASSIGNED_CPU_INDEX) {
> - cpu->cpu_index = cpu_get_free_index();
> - assert(cpu->cpu_index != UNASSIGNED_CPU_INDEX);
> - } else {
> - assert(!cpu_index_auto_assigned);
> - }
> - QTAILQ_INSERT_TAIL(&cpus, cpu, node);
> - cpu_list_unlock();
> + cpu_list_add(cpu);
>
> #ifndef CONFIG_USER_ONLY
> if (qdev_get_vmsd(DEVICE(cpu)) == NULL) {
> diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
> index b6a7059..c694d16 100644
> --- a/include/exec/cpu-all.h
> +++ b/include/exec/cpu-all.h
> @@ -187,6 +187,10 @@ void address_space_stq(AddressSpace *as, hwaddr addr, uint64_t val,
> MemTxAttrs attrs, MemTxResult *result);
> #endif
>
> +/* The CPU list lock nests outside tb_lock/tb_unlock. */
> +void cpu_list_lock(void);
> +void cpu_list_unlock(void);
> +
> /* page related stuff */
>
> #define TARGET_PAGE_SIZE (1 << TARGET_PAGE_BITS)
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 952bcfe..1d41452 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -23,6 +23,8 @@ typedef struct CPUListState {
> FILE *file;
> } CPUListState;
>
> +void qemu_init_cpu_list(void);
> +
> #if !defined(CONFIG_USER_ONLY)
>
> enum device_endian {
> diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
> index a0e87be..36ab8b6 100644
> --- a/include/exec/exec-all.h
> +++ b/include/exec/exec-all.h
> @@ -56,17 +56,6 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> target_ulong pc, target_ulong cs_base,
> uint32_t flags,
> int cflags);
> -#if defined(CONFIG_USER_ONLY)
> -void cpu_list_lock(void);
> -void cpu_list_unlock(void);
> -#else
> -static inline void cpu_list_unlock(void)
> -{
> -}
> -static inline void cpu_list_lock(void)
> -{
> -}
> -#endif
>
> void cpu_exec_init(CPUState *cpu, Error **errp);
> void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
> diff --git a/include/qom/cpu.h b/include/qom/cpu.h
> index 4aa9e61..ea3233f 100644
> --- a/include/qom/cpu.h
> +++ b/include/qom/cpu.h
> @@ -545,6 +545,18 @@ static inline int cpu_asidx_from_attrs(CPUState *cpu, MemTxAttrs attrs)
> #endif
>
> /**
> + * cpu_list_add:
> + * @cpu: The CPU to be added to the list of CPUs.
> + */
> +void cpu_list_add(CPUState *cpu);
> +
> +/**
> + * cpu_list_remove:
> + * @cpu: The CPU to be removed from the list of CPUs.
> + */
> +void cpu_list_remove(CPUState *cpu);
> +
> +/**
> * cpu_reset:
> * @cpu: The CPU whose state is to be reset.
> */
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 0f24d57..c3fefb6 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -111,7 +111,6 @@ 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 QemuMutex cpu_list_mutex;
> static QemuMutex exclusive_lock;
> static QemuCond exclusive_cond;
> static QemuCond exclusive_resume;
> @@ -119,7 +118,6 @@ 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);
> @@ -128,6 +126,7 @@ void qemu_init_cpu_loop(void)
> /* Make sure everything is in a consistent state for calling fork(). */
> void fork_start(void)
> {
> + cpu_list_lock();
> qemu_mutex_lock(&tcg_ctx.tb_ctx.tb_lock);
> qemu_mutex_lock(&exclusive_lock);
> mmap_fork_start();
> @@ -147,14 +146,15 @@ void fork_end(int child)
> }
> pending_cpus = 0;
> 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);
> + qemu_init_cpu_list();
> gdbserver_fork(thread_cpu);
> } else {
> qemu_mutex_unlock(&exclusive_lock);
> qemu_mutex_unlock(&tcg_ctx.tb_ctx.tb_lock);
> + cpu_list_unlock();
> }
> }
>
> @@ -221,16 +221,6 @@ static inline void cpu_exec_end(CPUState *cpu)
> qemu_mutex_unlock(&exclusive_lock);
> }
>
> -void cpu_list_lock(void)
> -{
> - qemu_mutex_lock(&cpu_list_mutex);
> -}
> -
> -void cpu_list_unlock(void)
> -{
> - qemu_mutex_unlock(&cpu_list_mutex);
> -}
> -
>
> #ifdef TARGET_I386
> /***********************************************************/
> @@ -4229,6 +4219,7 @@ int main(int argc, char **argv, char **envp)
> int ret;
> int execfd;
>
> + qemu_init_cpu_list();
> qemu_init_cpu_loop();
> module_call_init(MODULE_INIT_QOM);
>
> diff --git a/vl.c b/vl.c
> index ee557a1..7b1b04a 100644
> --- a/vl.c
> +++ b/vl.c
> @@ -2978,6 +2978,7 @@ int main(int argc, char **argv, char **envp)
> Error *err = NULL;
> bool list_data_dirs = false;
>
> + qemu_init_cpu_list();
> qemu_init_cpu_loop();
> qemu_mutex_lock_iothread();
Otherwise:
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code
2016-09-22 15:24 ` Alex Bennée
@ 2016-09-22 15:27 ` Paolo Bonzini
2016-09-22 15:51 ` Alex Bennée
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-22 15:27 UTC (permalink / raw)
To: Alex Bennée; +Cc: qemu-devel, sergey.fedorov
On 22/09/2016 17:24, Alex Bennée wrote:
> I see this is code moved from elsewhere but I feel a little comment
> about this should probably accompany the variable. I think it means once
> you have added a CPU without an explicit index all future CPUs will be
> auto assigned and you can no longer add CPUs with a specific index?
>
> Does this mean you can't hotplug additional CPUs without having
> specified a CPU topology at start-up?
I honestly have no idea.
Paolo
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu
2016-09-12 11:12 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
@ 2016-09-22 15:37 ` Alex Bennée
0 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2016-09-22 15:37 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, sergey.fedorov
Paolo Bonzini <pbonzini@redhat.com> writes:
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> cpus-common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/cpus-common.c b/cpus-common.c
> index b98f548..cdfdb14 100644
> --- a/cpus-common.c
> +++ b/cpus-common.c
> @@ -88,8 +88,7 @@ struct qemu_work_item {
> struct qemu_work_item *next;
> run_on_cpu_func func;
> void *data;
> - int done;
> - bool free;
> + bool free, done;
> };
>
> static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
> @@ -120,6 +119,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
>
> wi.func = func;
> wi.data = data;
> + wi.done = false;
> wi.free = false;
>
> queue_work_on_cpu(cpu, &wi);
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
--
Alex Bennée
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code
2016-09-22 15:27 ` Paolo Bonzini
@ 2016-09-22 15:51 ` Alex Bennée
0 siblings, 0 replies; 27+ messages in thread
From: Alex Bennée @ 2016-09-22 15:51 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: qemu-devel, sergey.fedorov
Paolo Bonzini <pbonzini@redhat.com> writes:
> On 22/09/2016 17:24, Alex Bennée wrote:
>> I see this is code moved from elsewhere but I feel a little comment
>> about this should probably accompany the variable. I think it means once
>> you have added a CPU without an explicit index all future CPUs will be
>> auto assigned and you can no longer add CPUs with a specific index?
>>
>> Does this mean you can't hotplug additional CPUs without having
>> specified a CPU topology at start-up?
>
> I honestly have no idea.
Well never mind, as the code already exits my r-b still stands.
--
Alex Bennée
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu
2016-09-23 7:31 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
@ 2016-09-23 17:15 ` Richard Henderson
0 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2016-09-23 17:15 UTC (permalink / raw)
To: Paolo Bonzini, qemu-devel; +Cc: serge.fdrv, cota, alex.bennee, sergey.fedorov
On 09/23/2016 12:31 AM, Paolo Bonzini wrote:
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> cpus-common.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Richard Henderson <rth@twiddle.net>
r~
^ permalink raw reply [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu
2016-09-23 7:31 [Qemu-devel] [PATCH v8 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
@ 2016-09-23 7:31 ` Paolo Bonzini
2016-09-23 17:15 ` Richard Henderson
0 siblings, 1 reply; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-23 7:31 UTC (permalink / raw)
To: qemu-devel; +Cc: cota, sergey.fedorov, alex.bennee, serge.fdrv
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index 2005bfe..d6cd426 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -88,8 +88,7 @@ struct qemu_work_item {
struct qemu_work_item *next;
run_on_cpu_func func;
void *data;
- int done;
- bool free;
+ bool free, done;
};
static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
@@ -120,6 +119,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
wi.func = func;
wi.data = data;
+ wi.done = false;
wi.free = false;
queue_work_on_cpu(cpu, &wi);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
* [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu
2016-09-19 12:50 Paolo Bonzini
@ 2016-09-19 12:50 ` Paolo Bonzini
0 siblings, 0 replies; 27+ messages in thread
From: Paolo Bonzini @ 2016-09-19 12:50 UTC (permalink / raw)
To: qemu-devel; +Cc: sergey.fedorov, serge.fdrv, alex.bennee
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
cpus-common.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/cpus-common.c b/cpus-common.c
index a739e66..4232255 100644
--- a/cpus-common.c
+++ b/cpus-common.c
@@ -88,8 +88,7 @@ struct qemu_work_item {
struct qemu_work_item *next;
run_on_cpu_func func;
void *data;
- int done;
- bool free;
+ bool free, done;
};
static void queue_work_on_cpu(CPUState *cpu, struct qemu_work_item *wi)
@@ -120,6 +119,7 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, void *data,
wi.func = func;
wi.data = data;
+ wi.done = false;
wi.free = false;
queue_work_on_cpu(cpu, &wi);
--
2.7.4
^ permalink raw reply related [flat|nested] 27+ messages in thread
end of thread, other threads:[~2016-09-23 17:16 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-12 11:12 [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 01/16] cpus: pass CPUState to run_on_cpu helpers Paolo Bonzini
2016-09-15 15:30 ` Alex Bennée
2016-09-12 11:12 ` [Qemu-devel] [PATCH 02/16] cpus: Move common code out of {async_, }run_on_cpu() Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 03/16] cpus: Rename flush_queued_work() Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 04/16] linux-user: Use QemuMutex and QemuCond Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 05/16] linux-user: Add qemu_cpu_is_self() and qemu_cpu_kick() Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 06/16] cpus-common: move CPU list management to common code Paolo Bonzini
2016-09-22 15:24 ` Alex Bennée
2016-09-22 15:27 ` Paolo Bonzini
2016-09-22 15:51 ` Alex Bennée
2016-09-12 11:12 ` [Qemu-devel] [PATCH 07/16] cpus-common: move CPU work item " Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
2016-09-22 15:37 ` Alex Bennée
2016-09-12 11:12 ` [Qemu-devel] [PATCH 09/16] cpus-common: move exclusive work infrastructure from linux-user Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 10/16] docs: include formal model for TCG exclusive sections Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 11/16] cpus-common: always defer async_run_on_cpu work items Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 12/16] cpus-common: remove redundant call to exclusive_idle() Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 13/16] cpus-common: simplify locking for start_exclusive/end_exclusive Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 14/16] cpus-common: Introduce async_safe_run_on_cpu() Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 15/16] tcg: Make tb_flush() thread safe Paolo Bonzini
2016-09-12 11:12 ` [Qemu-devel] [PATCH 16/16] cpus-common: lock-free fast path for cpu_exec_start/end Paolo Bonzini
2016-09-14 17:16 ` [Qemu-devel] [PATCH v7 00/16] cpu-exec: Safe work in quiescent state Richard Henderson
2016-09-14 18:40 ` Paolo Bonzini
2016-09-19 12:50 Paolo Bonzini
2016-09-19 12:50 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
2016-09-23 7:31 [Qemu-devel] [PATCH v8 00/16] cpu-exec: Safe work in quiescent state Paolo Bonzini
2016-09-23 7:31 ` [Qemu-devel] [PATCH 08/16] cpus-common: fix uninitialized variable use in run_on_cpu Paolo Bonzini
2016-09-23 17:15 ` Richard Henderson
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.