From 27e76d010104646c997d20ca0996fb5a046587b0 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 29 Aug 2025 11:26:05 +0200 Subject: cpu-common: use atomic access for interrupt_request MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writes to interrupt_request used non-atomic accesses, but there are a few cases where the access was not protected by the BQL. Now that there is a full set of helpers, it's easier to guarantee that interrupt_request accesses are fully atomic, so just drop the requirement instead of fixing them. Reviewed-by: Igor Mammedov Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Signed-off-by: Paolo Bonzini --- hw/core/cpu-common.c | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) (limited to 'hw/core/cpu-common.c') diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 259cf2a3c3..152abc9024 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -67,19 +67,9 @@ CPUState *cpu_create(const char *typename) return cpu; } -/* Resetting the IRQ comes from across the code base so we take the - * BQL here if we need to. cpu_interrupt assumes it is held.*/ void cpu_reset_interrupt(CPUState *cpu, int mask) { - bool need_lock = !bql_locked(); - - if (need_lock) { - bql_lock(); - } - cpu->interrupt_request &= ~mask; - if (need_lock) { - bql_unlock(); - } + qatomic_and(&cpu->interrupt_request, ~mask); } void cpu_exit(CPUState *cpu) -- cgit 1.4.1 From ac6c8a390b451913995ee784ef7261b8928e5ace Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 1 Aug 2025 14:57:51 +0200 Subject: accel: use store_release/load_acquire for cross-thread exit_request Reads and writes cpu->exit_request do not use a load-acquire/store-release pair right now, but this means that cpu_exit() may not write cpu->exit_request after any flags that are read by the vCPU thread. Probably everything is protected one way or the other by the BQL, because cpu->exit_request leads to the slow path, where the CPU thread often takes the BQL (for example, to go to sleep by waiting on the BQL-protected cpu->halt_cond); but it's not clear, so use load-acquire/store-release consistently. Reviewed-by: Richard Henderson Reviewed-by: Peter Xu Reviewed-by: Igor Mammedov Signed-off-by: Paolo Bonzini --- accel/kvm/kvm-all.c | 19 +++++++++---------- accel/tcg/cpu-exec.c | 7 +++++-- accel/tcg/tcg-accel-ops-rr.c | 11 +++++++++-- hw/core/cpu-common.c | 3 ++- target/i386/nvmm/nvmm-all.c | 5 ++--- target/i386/whpx/whpx-all.c | 3 ++- 6 files changed, 29 insertions(+), 19 deletions(-) (limited to 'hw/core/cpu-common.c') diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index f36dfe3349..bd9e5e3886 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -3029,10 +3029,6 @@ static void kvm_eat_signals(CPUState *cpu) if (kvm_immediate_exit) { qatomic_set(&cpu->kvm_run->immediate_exit, 0); - /* Write kvm_run->immediate_exit before the cpu->exit_request - * write in kvm_cpu_exec. - */ - smp_wmb(); return; } @@ -3187,7 +3183,8 @@ int kvm_cpu_exec(CPUState *cpu) } kvm_arch_pre_run(cpu, run); - if (qatomic_read(&cpu->exit_request)) { + /* Corresponding store-release is in cpu_exit. */ + if (qatomic_load_acquire(&cpu->exit_request)) { trace_kvm_interrupt_exit_request(); /* * KVM requires us to reenter the kernel after IO exits to complete @@ -3197,13 +3194,15 @@ int kvm_cpu_exec(CPUState *cpu) kvm_cpu_kick_self(); } - /* Read cpu->exit_request before KVM_RUN reads run->immediate_exit. - * Matching barrier in kvm_eat_signals. - */ - smp_rmb(); - run_ret = kvm_vcpu_ioctl(cpu, KVM_RUN, 0); + /* + * After writing cpu->exit_request, cpu_exit() sends a signal that writes + * kvm->run->immediate_exit. The signal is already happening after the + * write to cpu->exit_request so, if KVM read kvm->run->immediate_exit + * as true, cpu->exit_request will always read as true. + */ + attrs = kvm_arch_post_run(cpu, run); #ifdef KVM_HAVE_MCE_INJECTION diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index 508d2d2d9e..f838535d11 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -851,8 +851,11 @@ static inline bool cpu_handle_interrupt(CPUState *cpu, } #endif /* !CONFIG_USER_ONLY */ - /* Finally, check if we need to exit to the main loop. */ - if (unlikely(qatomic_read(&cpu->exit_request)) || icount_exit_request(cpu)) { + /* + * Finally, check if we need to exit to the main loop. + * 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-rr.c b/accel/tcg/tcg-accel-ops-rr.c index 6eec5c9eee..e8b0e370a8 100644 --- a/accel/tcg/tcg-accel-ops-rr.c +++ b/accel/tcg/tcg-accel-ops-rr.c @@ -242,10 +242,17 @@ static void *rr_cpu_thread_fn(void *arg) cpu = first_cpu; } - while (cpu && cpu_work_list_empty(cpu) && !cpu->exit_request) { - /* Store rr_current_cpu before evaluating cpu_can_run(). */ + while (cpu && cpu_work_list_empty(cpu)) { + /* + * Store rr_current_cpu before evaluating cpu->exit_request. + * Pairs with rr_kick_next_cpu(). + */ qatomic_set_mb(&rr_current_cpu, cpu); + /* Pairs with store-release in cpu_exit. */ + if (qatomic_load_acquire(&cpu->exit_request)) { + break; + } current_cpu = cpu; qemu_clock_enable(QEMU_CLOCK_VIRTUAL, diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 152abc9024..42463e6258 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -74,7 +74,8 @@ void cpu_reset_interrupt(CPUState *cpu, int mask) void cpu_exit(CPUState *cpu) { - qatomic_set(&cpu->exit_request, 1); + /* Ensure cpu_exec will see the reason why the exit request was set. */ + qatomic_store_release(&cpu->exit_request, true); /* Ensure cpu_exec will see the exit request after TCG has exited. */ smp_wmb(); qatomic_set(&cpu->neg.icount_decr.u16.high, -1); diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c index e1151b04c6..10bd51d9b5 100644 --- a/target/i386/nvmm/nvmm-all.c +++ b/target/i386/nvmm/nvmm-all.c @@ -743,7 +743,8 @@ nvmm_vcpu_loop(CPUState *cpu) nvmm_vcpu_pre_run(cpu); - if (qatomic_read(&cpu->exit_request)) { + /* Corresponding store-release is in cpu_exit. */ + if (qatomic_load_acquire(&cpu->exit_request)) { #if NVMM_USER_VERSION >= 2 nvmm_vcpu_stop(vcpu); #else @@ -751,8 +752,6 @@ nvmm_vcpu_loop(CPUState *cpu) #endif } - /* Read exit_request before the kernel reads the immediate exit flag */ - smp_rmb(); ret = nvmm_vcpu_run(mach, vcpu); if (ret == -1) { error_report("NVMM: Failed to exec a virtual processor," diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c index c09a0a64f2..2106c29c3a 100644 --- a/target/i386/whpx/whpx-all.c +++ b/target/i386/whpx/whpx-all.c @@ -1714,7 +1714,8 @@ static int whpx_vcpu_run(CPUState *cpu) if (exclusive_step_mode == WHPX_STEP_NONE) { whpx_vcpu_pre_run(cpu); - if (qatomic_read(&cpu->exit_request)) { + /* Corresponding store-release is in cpu_exit. */ + if (qatomic_load_acquire(&cpu->exit_request)) { whpx_vcpu_kick(cpu); } } -- cgit 1.4.1 From dcb46ecb2e90d532fcdc04702c92e732a0ef77e8 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 1 Aug 2025 13:24:48 +0200 Subject: cpus: remove TCG-ism from cpu_exit() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that TCG has its own kick function, make cpu_exit() do the right kick for all accelerators. Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Reviewed-by: Igor Mammedov Signed-off-by: Paolo Bonzini --- hw/core/cpu-common.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'hw/core/cpu-common.c') diff --git a/hw/core/cpu-common.c b/hw/core/cpu-common.c index 42463e6258..41a339903c 100644 --- a/hw/core/cpu-common.c +++ b/hw/core/cpu-common.c @@ -76,9 +76,7 @@ void cpu_exit(CPUState *cpu) { /* Ensure cpu_exec will see the reason why the exit request was set. */ qatomic_store_release(&cpu->exit_request, true); - /* Ensure cpu_exec will see the exit request after TCG has exited. */ - smp_wmb(); - qatomic_set(&cpu->neg.icount_decr.u16.high, -1); + qemu_cpu_kick(cpu); } static int cpu_common_gdb_read_register(CPUState *cpu, GByteArray *buf, int reg) -- cgit 1.4.1