There is no reason for some accelerators to use qemu_wait_io_event_common (which is specifically separated for round robin). They can also check on the first pass through the loop as well directly, without setting cpu->exit_request for no particular reason.
There is also no need to use qatomic_set_mb() because the ordering of the communication between I/O and vCPU threads is always the same. In the I/O thread: (a) store other memory locations that will be checked if cpu->exit_request or cpu->interrupt_request is 1 (for example cpu->stop or cpu->work_list for cpu->exit_request) (b) cpu_exit(): store-release cpu->exit_request, or (b) cpu_interrupt(): store-release cpu->interrupt_request >>> at this point, cpu->halt_cond is broadcast and the BQL released (c) do the accelerator-specific kick (e.g. write icount_decr for TCG, pthread_kill for KVM, etc.) In the vCPU thread instead the opposite order is respected: (c) the accelerator's execution loop exits thanks to the kick (b) then the inner execution loop checks cpu->interrupt_request and cpu->exit_request. If needed cpu->interrupt_request is converted into cpu->exit_request when work is needed outside the execution loop. (a) then the other memory locations are checked. Some may need to be read under the BQL, and the vCPU thread may also take for the vCPU thread can sleep on cpu->halt_cond; but in principle this step is correct even without the BQL. Signed-off-by: Paolo Bonzini <pbonz...@redhat.com> --- accel/dummy-cpus.c | 2 +- accel/hvf/hvf-accel-ops.c | 2 +- accel/kvm/kvm-accel-ops.c | 3 ++- accel/kvm/kvm-all.c | 2 -- accel/tcg/cpu-exec.c | 1 - accel/tcg/tcg-accel-ops-mttcg.c | 7 ++---- accel/tcg/tcg-accel-ops-rr.c | 38 ++++++++++++++++--------------- accel/tcg/tcg-accel-ops.c | 2 -- system/cpus.c | 1 + target/i386/nvmm/nvmm-accel-ops.c | 6 ++--- target/i386/nvmm/nvmm-all.c | 2 -- target/i386/whpx/whpx-accel-ops.c | 6 ++--- target/i386/whpx/whpx-all.c | 2 -- 13 files changed, 31 insertions(+), 43 deletions(-) diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c index 03cfc0fa01e..1f74c727c42 100644 --- a/accel/dummy-cpus.c +++ b/accel/dummy-cpus.c @@ -43,6 +43,7 @@ static void *dummy_cpu_thread_fn(void *arg) qemu_guest_random_seed_thread_part2(cpu->random_seed); do { + qemu_wait_io_event(cpu); bql_unlock(); #ifndef _WIN32 do { @@ -57,7 +58,6 @@ static void *dummy_cpu_thread_fn(void *arg) qemu_sem_wait(&cpu->sem); #endif bql_lock(); - qemu_wait_io_event(cpu); } while (!cpu->unplug); bql_unlock(); diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c index d488d6afbac..4ba3e40831f 100644 --- a/accel/hvf/hvf-accel-ops.c +++ b/accel/hvf/hvf-accel-ops.c @@ -192,13 +192,13 @@ static void *hvf_cpu_thread_fn(void *arg) qemu_guest_random_seed_thread_part2(cpu->random_seed); do { + qemu_wait_io_event(cpu); if (cpu_can_run(cpu)) { r = hvf_vcpu_exec(cpu); if (r == EXCP_DEBUG) { cpu_handle_guest_debug(cpu); } } - qemu_wait_io_event(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); hvf_vcpu_destroy(cpu); diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c index b709187c7d7..80f0141a8a6 100644 --- a/accel/kvm/kvm-accel-ops.c +++ b/accel/kvm/kvm-accel-ops.c @@ -47,13 +47,14 @@ static void *kvm_vcpu_thread_fn(void *arg) qemu_guest_random_seed_thread_part2(cpu->random_seed); do { + qemu_wait_io_event(cpu); + if (cpu_can_run(cpu)) { r = kvm_cpu_exec(cpu); if (r == EXCP_DEBUG) { cpu_handle_guest_debug(cpu); } } - qemu_wait_io_event(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); kvm_destroy_vcpu(cpu); diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 57e35960125..db95e06e768 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -3155,7 +3155,6 @@ int kvm_cpu_exec(CPUState *cpu) trace_kvm_cpu_exec(); if (kvm_arch_process_async_events(cpu)) { - qatomic_set(&cpu->exit_request, 0); return EXCP_HLT; } @@ -3345,7 +3344,6 @@ int kvm_cpu_exec(CPUState *cpu) vm_stop(RUN_STATE_INTERNAL_ERROR); } - qatomic_set(&cpu->exit_request, 0); return ret; } diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index b9da2e3770e..f474ccb37f5 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -871,7 +871,6 @@ static inline bool cpu_handle_interrupt(CPUState *cpu, * The corresponding store-release is in cpu_exit. */ if (unlikely(qatomic_load_acquire(&cpu->exit_request)) || icount_exit_request(cpu)) { - qatomic_set(&cpu->exit_request, 0); if (cpu->exception_index == -1) { cpu->exception_index = EXCP_INTERRUPT; } diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c index 5757324a8c2..04012900a30 100644 --- a/accel/tcg/tcg-accel-ops-mttcg.c +++ b/accel/tcg/tcg-accel-ops-mttcg.c @@ -84,10 +84,9 @@ static void *mttcg_cpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); - /* process any pending work */ - qatomic_set(&cpu->exit_request, 1); - do { + qemu_wait_io_event(cpu); + if (cpu_can_run(cpu)) { int r; bql_unlock(); @@ -112,8 +111,6 @@ static void *mttcg_cpu_thread_fn(void *arg) break; } } - - qemu_wait_io_event(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); tcg_cpu_destroy(cpu); diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c index 87b49377c78..205f379b6f8 100644 --- a/accel/tcg/tcg-accel-ops-rr.c +++ b/accel/tcg/tcg-accel-ops-rr.c @@ -211,13 +211,30 @@ static void *rr_cpu_thread_fn(void *arg) cpu = first_cpu; - /* process any pending work */ - qatomic_set(&cpu->exit_request, 1); - while (1) { /* Only used for icount_enabled() */ int64_t cpu_budget = 0; + if (cpu) { + /* + * This could even reset exit_request for all CPUs, but in practice + * races between CPU exits and changes to "cpu" are so rare that + * there's no advantage in doing so. + */ + qatomic_set(&cpu->exit_request, 0); + } + + if (icount_enabled() && all_cpu_threads_idle()) { + /* + * When all cpus are sleeping (e.g in WFI), to avoid a deadlock + * in the main_loop, wake it up in order to start the warp timer. + */ + qemu_notify_event(); + } + + rr_wait_io_event(); + rr_deal_with_unplugged_cpus(); + bql_unlock(); replay_mutex_lock(); bql_lock(); @@ -285,21 +302,6 @@ static void *rr_cpu_thread_fn(void *arg) /* Does not need a memory barrier because a spurious wakeup is okay. */ qatomic_set(&rr_current_cpu, NULL); - - if (cpu && qatomic_read(&cpu->exit_request)) { - qatomic_set_mb(&cpu->exit_request, 0); - } - - if (icount_enabled() && all_cpu_threads_idle()) { - /* - * When all cpus are sleeping (e.g in WFI), to avoid a deadlock - * in the main_loop, wake it up in order to start the warp timer. - */ - qemu_notify_event(); - } - - rr_wait_io_event(); - rr_deal_with_unplugged_cpus(); } g_assert_not_reached(); diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c index f4d5372866a..ad3f29107e1 100644 --- a/accel/tcg/tcg-accel-ops.c +++ b/accel/tcg/tcg-accel-ops.c @@ -82,8 +82,6 @@ int tcg_cpu_exec(CPUState *cpu) ret = cpu_exec(cpu); cpu_exec_end(cpu); - qatomic_set_mb(&cpu->exit_request, 0); - return ret; } diff --git a/system/cpus.c b/system/cpus.c index d2cfa2a9c4e..0cc14eae6a0 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -458,6 +458,7 @@ void qemu_wait_io_event(CPUState *cpu) { bool slept = false; + qatomic_set(&cpu->exit_request, false); while (cpu_thread_is_idle(cpu)) { if (!slept) { slept = true; diff --git a/target/i386/nvmm/nvmm-accel-ops.c b/target/i386/nvmm/nvmm-accel-ops.c index 3658a583bc8..6d2e2542d80 100644 --- a/target/i386/nvmm/nvmm-accel-ops.c +++ b/target/i386/nvmm/nvmm-accel-ops.c @@ -42,16 +42,14 @@ static void *qemu_nvmm_cpu_thread_fn(void *arg) qemu_guest_random_seed_thread_part2(cpu->random_seed); do { + qemu_wait_io_event(cpu); + if (cpu_can_run(cpu)) { r = nvmm_vcpu_exec(cpu); if (r == EXCP_DEBUG) { cpu_handle_guest_debug(cpu); } } - while (cpu_thread_is_idle(cpu)) { - qemu_cond_wait_bql(cpu->halt_cond); - } - qemu_wait_io_event_common(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); nvmm_destroy_vcpu(cpu); diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c index d2d90f38976..09839d45b92 100644 --- a/target/i386/nvmm/nvmm-all.c +++ b/target/i386/nvmm/nvmm-all.c @@ -817,8 +817,6 @@ nvmm_vcpu_loop(CPUState *cpu) cpu_exec_end(cpu); bql_lock(); - qatomic_set(&cpu->exit_request, false); - return ret < 0; } diff --git a/target/i386/whpx/whpx-accel-ops.c b/target/i386/whpx/whpx-accel-ops.c index da58805b1a6..611eeedeef7 100644 --- a/target/i386/whpx/whpx-accel-ops.c +++ b/target/i386/whpx/whpx-accel-ops.c @@ -42,16 +42,14 @@ static void *whpx_cpu_thread_fn(void *arg) qemu_guest_random_seed_thread_part2(cpu->random_seed); do { + qemu_wait_io_event(cpu); + if (cpu_can_run(cpu)) { r = whpx_vcpu_exec(cpu); if (r == EXCP_DEBUG) { cpu_handle_guest_debug(cpu); } } - while (cpu_thread_is_idle(cpu)) { - qemu_cond_wait_bql(cpu->halt_cond); - } - qemu_wait_io_event_common(cpu); } while (!cpu->unplug || cpu_can_run(cpu)); whpx_destroy_vcpu(cpu); diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c index 9b07716121a..2e248a0a6d5 100644 --- a/target/i386/whpx/whpx-all.c +++ b/target/i386/whpx/whpx-all.c @@ -2050,8 +2050,6 @@ static int whpx_vcpu_run(CPUState *cpu) whpx_last_vcpu_stopping(cpu); } - qatomic_set(&cpu->exit_request, false); - return ret < 0; } -- 2.50.1