Distinguish between the 2 levels of vCPU execution loops: - "Outer vCPU loop" handles CPU events in vCPU thread - "Inner vCPU loop" handles actual vCPU execution This helps following the same pattern between all our accelerators. Signed-off-by: Philippe Mathieu-Daudé --- accel/dummy-cpus.c | 1 + accel/hvf/hvf-accel-ops.c | 1 + accel/kvm/kvm-accel-ops.c | 1 + accel/kvm/kvm-all.c | 1 + accel/mshv/mshv-all.c | 2 ++ accel/tcg/tcg-accel-ops-mttcg.c | 1 + accel/whpx/whpx-accel-ops.c | 1 + target/arm/hvf/hvf.c | 1 + target/arm/whpx/whpx-all.c | 6 +++--- target/i386/hvf/hvf.c | 1 + target/i386/nvmm/nvmm-accel-ops.c | 1 + target/i386/whpx/whpx-all.c | 1 + 12 files changed, 15 insertions(+), 3 deletions(-) diff --git a/accel/dummy-cpus.c b/accel/dummy-cpus.c index 5752f6302c8..f9a8d5fff07 100644 --- a/accel/dummy-cpus.c +++ b/accel/dummy-cpus.c @@ -42,6 +42,7 @@ static void *dummy_cpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); bql_unlock(); diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c index d2276d8513e..d8fc7be707f 100644 --- a/accel/hvf/hvf-accel-ops.c +++ b/accel/hvf/hvf-accel-ops.c @@ -172,6 +172,7 @@ static void *hvf_cpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); if (cpu_can_run(cpu)) { diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c index c8e7aa38709..73ff93aab67 100644 --- a/accel/kvm/kvm-accel-ops.c +++ b/accel/kvm/kvm-accel-ops.c @@ -47,6 +47,7 @@ static void *kvm_vcpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 83cbd120a84..b74c7f96866 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -3438,6 +3438,7 @@ int kvm_cpu_exec(CPUState *cpu) bql_unlock(); cpu_exec_start(cpu); + /* Inner vCPU loop */ do { MemTxAttrs attrs; diff --git a/accel/mshv/mshv-all.c b/accel/mshv/mshv-all.c index 72721d0f0de..af172a8ee34 100644 --- a/accel/mshv/mshv-all.c +++ b/accel/mshv/mshv-all.c @@ -616,6 +616,7 @@ static int mshv_cpu_exec(CPUState *cpu) bql_unlock(); cpu_exec_start(cpu); + /* Inner vCPU loop */ do { if (cpu->vcpu_dirty) { ret = mshv_arch_store_vcpu_state(cpu); @@ -705,6 +706,7 @@ static void *mshv_vcpu_thread(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); if (cpu_can_run(cpu)) { diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c index 69560fdb9d8..e33e35dab3d 100644 --- a/accel/tcg/tcg-accel-ops-mttcg.c +++ b/accel/tcg/tcg-accel-ops-mttcg.c @@ -86,6 +86,7 @@ static void *mttcg_cpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); diff --git a/accel/whpx/whpx-accel-ops.c b/accel/whpx/whpx-accel-ops.c index ca5a119521a..535667061b3 100644 --- a/accel/whpx/whpx-accel-ops.c +++ b/accel/whpx/whpx-accel-ops.c @@ -42,6 +42,7 @@ static void *whpx_cpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index da3ec521fc7..3eeb9f70de4 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -2600,6 +2600,7 @@ int hvf_arch_vcpu_exec(CPUState *cpu) flush_cpu_state(cpu); + /* Inner vCPU loop */ do { if (!(cpu->singlestep_flags & SSTEP_NOIRQ) && hvf_inject_interrupts(cpu)) { diff --git a/target/arm/whpx/whpx-all.c b/target/arm/whpx/whpx-all.c index 00a5de8cdc1..5e4c25bb7d5 100644 --- a/target/arm/whpx/whpx-all.c +++ b/target/arm/whpx/whpx-all.c @@ -407,9 +407,9 @@ int whpx_vcpu_run(CPUState *cpu) } bql_unlock(); - - cpu_exec_start(cpu); + + /* Inner vCPU loop */ do { bool advance_pc = false; if (cpu->vcpu_dirty) { @@ -495,8 +495,8 @@ int whpx_vcpu_run(CPUState *cpu) } while (!ret); cpu_exec_end(cpu); - bql_lock(); + current_cpu = cpu; if (--whpx->running_cpus == 0) { diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c index 150598418e2..0ffa3658dc5 100644 --- a/target/i386/hvf/hvf.c +++ b/target/i386/hvf/hvf.c @@ -1009,6 +1009,7 @@ int hvf_arch_vcpu_exec(CPUState *cpu) return EXCP_HLT; } + /* Inner vCPU loop */ do { if (cpu->vcpu_dirty) { hvf_arch_put_registers(cpu); diff --git a/target/i386/nvmm/nvmm-accel-ops.c b/target/i386/nvmm/nvmm-accel-ops.c index dd5d5428b1c..da28fbe3de8 100644 --- a/target/i386/nvmm/nvmm-accel-ops.c +++ b/target/i386/nvmm/nvmm-accel-ops.c @@ -41,6 +41,7 @@ static void *qemu_nvmm_cpu_thread_fn(void *arg) cpu_thread_signal_created(cpu); qemu_guest_random_seed_thread_part2(cpu->random_seed); + /* Outer vCPU loop */ do { qemu_process_cpu_events(cpu); diff --git a/target/i386/whpx/whpx-all.c b/target/i386/whpx/whpx-all.c index 634d5428219..b4f499905a7 100644 --- a/target/i386/whpx/whpx-all.c +++ b/target/i386/whpx/whpx-all.c @@ -2251,6 +2251,7 @@ int whpx_vcpu_run(CPUState *cpu) cpu_exec_start(cpu); } + /* Inner vCPU loop */ do { if (cpu->vcpu_dirty) { whpx_set_registers(cpu, WHPX_LEVEL_RUNTIME_STATE); -- 2.53.0 Be a bit more descriptive than "Unblock cpu" :) Signed-off-by: Philippe Mathieu-Daudé --- include/system/cpus.h | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/include/system/cpus.h b/include/system/cpus.h index 508444ccf1c..ade13e068eb 100644 --- a/include/system/cpus.h +++ b/include/system/cpus.h @@ -30,7 +30,15 @@ void resume_all_vcpus(void); void pause_all_vcpus(void); void cpu_stop_current(void); -/* Unblock cpu */ +/** + * qemu_cpu_kick_self - Force vCPU to re-enter to its inner main loop + * + * Signal the current vCPU thread to exit any blocking operations and + * re-enter its inner execution loop to process pending requests, + * possibly returning to its outer execution loop. + * + * Must be called from within the vCPU thread itself. + */ void qemu_cpu_kick_self(void); bool cpus_are_resettable(void); -- 2.53.0 There is a single method with ending with _current() suffix. Align with qemu_cpu_kick_self() and pthread conventions by renaming cpu_stop_current() -> qemu_cpu_stop_self() for consistency. Signed-off-by: Philippe Mathieu-Daudé --- include/system/cpus.h | 8 +++++++- hw/ppc/spapr_rtas.c | 2 +- system/cpus.c | 2 +- system/runstate.c | 6 +++--- 4 files changed, 12 insertions(+), 6 deletions(-) diff --git a/include/system/cpus.h b/include/system/cpus.h index ade13e068eb..2365c7c4597 100644 --- a/include/system/cpus.h +++ b/include/system/cpus.h @@ -28,7 +28,13 @@ bool qemu_in_vcpu_thread(void); void qemu_init_cpu_loop(void); void resume_all_vcpus(void); void pause_all_vcpus(void); -void cpu_stop_current(void); + +/** + * qemu_cpu_stop_self - Stop execution of the current vCPU + * + * Note: No effect if not executing within a vCPU thread. + */ +void qemu_cpu_stop_self(void); /** * qemu_cpu_kick_self - Force vCPU to re-enter to its inner main loop diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c index 328fc27c400..80cd4246af8 100644 --- a/hw/ppc/spapr_rtas.c +++ b/hw/ppc/spapr_rtas.c @@ -93,7 +93,7 @@ static void rtas_power_off(PowerPCCPU *cpu, SpaprMachineState *spapr, return; } qemu_system_shutdown_request(SHUTDOWN_CAUSE_GUEST_SHUTDOWN); - cpu_stop_current(); + qemu_cpu_stop_self(); rtas_st(rets, 0, RTAS_OUT_SUCCESS); } diff --git a/system/cpus.c b/system/cpus.c index f37e215c586..1a6fd286a81 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -674,7 +674,7 @@ void qemu_init_vcpu(CPUState *cpu) } } -void cpu_stop_current(void) +void qemu_cpu_stop_self(void) { if (current_cpu) { current_cpu->stop = true; diff --git a/system/runstate.c b/system/runstate.c index d3e64d2b625..1302fea9ccd 100644 --- a/system/runstate.c +++ b/system/runstate.c @@ -472,7 +472,7 @@ int vm_stop(RunState state) * FIXME: should not return to device code in case * vm_stop() has been requested. */ - cpu_stop_current(); + qemu_cpu_stop_self(); return 0; } @@ -900,7 +900,7 @@ void qemu_system_reset_request(ShutdownCause reason) } else { reset_requested = reason; } - cpu_stop_current(); + qemu_cpu_stop_self(); qemu_notify_event(); } @@ -918,7 +918,7 @@ void qemu_system_suspend_request(void) return; } suspend_requested = 1; - cpu_stop_current(); + qemu_cpu_stop_self(); qemu_notify_event(); } -- 2.53.0 Stopping a vCPU is not a simple state transition: an asynchronous request (CPUState::stop flag) is acknowledged (CPUState::stopped flag). Use a better name to clarify. Signed-off-by: Philippe Mathieu-Daudé --- system/cpus.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/system/cpus.c b/system/cpus.c index 1a6fd286a81..f69558c760c 100644 --- a/system/cpus.c +++ b/system/cpus.c @@ -382,7 +382,8 @@ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data) do_run_on_cpu(cpu, func, data, &bql); } -static void qemu_cpu_stop(CPUState *cpu, bool exit) +/* Must be called by the owning thread */ +static void qemu_cpu_ack_stop_request(CPUState *cpu, bool exit) { g_assert(qemu_cpu_is_self(cpu)); cpu->stop = false; @@ -397,7 +398,7 @@ void qemu_process_cpu_events_common(CPUState *cpu) { qatomic_set_mb(&cpu->thread_kicked, false); if (cpu->stop) { - qemu_cpu_stop(cpu, false); + qemu_cpu_ack_stop_request(cpu, false); } process_queued_cpu_work(cpu); } @@ -555,7 +556,7 @@ void cpu_thread_signal_destroyed(CPUState *cpu) void cpu_pause(CPUState *cpu) { if (qemu_cpu_is_self(cpu)) { - qemu_cpu_stop(cpu, true); + qemu_cpu_ack_stop_request(cpu, true); } else { cpu->stop = true; cpu_exit(cpu); -- 2.53.0 Mention cpu_pause() might not be immediately fullfilled: when the requested vCPU is not the current one, the request is asynchroneous. Mention the method goes in pair with cpu_resume(). Signed-off-by: Philippe Mathieu-Daudé --- include/hw/core/cpu.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 81af7b9ee1a..f1ac492714e 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1045,10 +1045,13 @@ void cpu_reset_interrupt(CPUState *cpu, int mask); void cpu_exit(CPUState *cpu); /** - * cpu_pause: + * cpu_pause - Request the CPU to pause execution * @cpu: The CPU to pause. * - * Pauses CPU, i.e. puts CPU into stopped state. + * If @cpu is the current thread, immediately pause its execution; + * otherwise asynchronously request it to pause. + * + * Note: Use cpu_resume() to restart execution. */ void cpu_pause(CPUState *cpu); -- 2.53.0 Mention that what the vCPU exits is the execution loop :) Mention some of the reasons for calling cpu_exit(). Signed-off-by: Philippe Mathieu-Daudé --- include/hw/core/cpu.h | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index f1ac492714e..7da81f3ee85 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1037,10 +1037,11 @@ static inline void cpu_set_pc(CPUState *cpu, vaddr addr) void cpu_reset_interrupt(CPUState *cpu, int mask); /** - * cpu_exit: - * @cpu: The CPU to exit. + * cpu_exit - Request the CPU to exit its execution loop + * @cpu: The CPU to interrupt. * - * Requests the CPU @cpu to exit execution. + * Request an immediate exit from the CPU execution loop to process + * pending work such as interrupts, timers, or other async requests. */ void cpu_exit(CPUState *cpu); -- 2.53.0 Mention process_queued_cpu_work*() functions must run on a vCPU thread. Signed-off-by: Philippe Mathieu-Daudé --- include/hw/core/cpu.h | 4 ++++ include/system/cpus.h | 1 + 2 files changed, 5 insertions(+) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 7da81f3ee85..183ef9f4ef2 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1081,6 +1081,8 @@ void free_queued_cpu_work(CPUState *cpu); /** * process_queued_cpu_work() - process all items on CPU work queue * @cpu: The CPU which work queue to process. + * + * Must be called from within the vCPU thread itself. */ void process_queued_cpu_work(CPUState *cpu); @@ -1179,6 +1181,8 @@ G_NORETURN void cpu_abort(CPUState *cpu, const char *fmt, ...) * * Perform accelerator-independent work after the CPU has left * the inner execution loop. + * + * Must be called from within the vCPU thread itself. */ void qemu_process_cpu_events(CPUState *cpu); diff --git a/include/system/cpus.h b/include/system/cpus.h index 2365c7c4597..0bdb231b996 100644 --- a/include/system/cpus.h +++ b/include/system/cpus.h @@ -17,6 +17,7 @@ bool cpu_work_list_empty(CPUState *cpu); bool cpu_thread_is_idle(CPUState *cpu); bool all_cpu_threads_idle(void); bool cpu_can_run(CPUState *cpu); +/* Must be called from within the vCPU thread itself */ void qemu_process_cpu_events_common(CPUState *cpu); void cpu_thread_signal_created(CPUState *cpu); void cpu_thread_signal_destroyed(CPUState *cpu); -- 2.53.0 Be a bit more precise mentioning BQL use and blocking calls. Signed-off-by: Philippe Mathieu-Daudé --- include/hw/core/cpu.h | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 183ef9f4ef2..d610f87abd3 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -908,36 +908,40 @@ void do_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data, QemuMutex *mutex); /** - * run_on_cpu: + * run_on_cpu - Execute a function on a vCPU and wait for completion * @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. + * Schedules the function @func for execution on the vCPU @cpu and + * block until it completes. + * + * Note: The caller must hold the BQL. */ void run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data); /** - * async_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. - */ -void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data); - -/** - * async_safe_run_on_cpu: + * async_run_on_cpu - Queue a function for execution on a vCPU * @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. + * then return immediately without waiting. * - * Unlike run_on_cpu and async_run_on_cpu, the function is run outside the - * BQL. + * Note: The vCPU will hold the BQL when executing @func. + */ +void async_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data); + +/** + * async_safe_run_on_cpu - Queue a function to run with other vCPUs paused + * @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. + * Unlike async_run_on_cpu(), all other vCPUs are paused while @func executes, + * ensuring @cpu has exclusive access to shared state. */ void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data); -- 2.53.0 tcg_commit_cpu() does not consume the thread-local parameter. Replace RUN_ON_CPU_HOST_PTR(cpuas) with RUN_ON_CPU_NULL when scheduling the function to avoid any confusion. Signed-off-by: Philippe Mathieu-Daudé --- Maybe this AS parameter should be used? --- system/physmem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/system/physmem.c b/system/physmem.c index 362a00f76c0..b97016b1303 100644 --- a/system/physmem.c +++ b/system/physmem.c @@ -3088,9 +3088,9 @@ static void tcg_commit(MemoryListener *listener) * all of the tcg machinery for run-on is initialized: thus halt_cond. */ if (cpu->halt_cond) { - async_run_on_cpu(cpu, tcg_commit_cpu, RUN_ON_CPU_HOST_PTR(cpuas)); + async_run_on_cpu(cpu, tcg_commit_cpu, RUN_ON_CPU_NULL); } else { - tcg_commit_cpu(cpu, RUN_ON_CPU_HOST_PTR(cpuas)); + tcg_commit_cpu(cpu, RUN_ON_CPU_NULL); } } -- 2.53.0 @r is used for 2 distinct concepts: - kvm_init_vcpu() returns an errno. However we don't consume it, besides it is called with &error_fatal. Drop the unused returned value. - kvm_cpu_exec() returns an exception code. Reduce the variable scope and rename @r as @excp to clarify. Signed-off-by: Philippe Mathieu-Daudé --- accel/kvm/kvm-accel-ops.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/accel/kvm/kvm-accel-ops.c b/accel/kvm/kvm-accel-ops.c index 73ff93aab67..06f56773324 100644 --- a/accel/kvm/kvm-accel-ops.c +++ b/accel/kvm/kvm-accel-ops.c @@ -31,7 +31,6 @@ static void *kvm_vcpu_thread_fn(void *arg) { CPUState *cpu = arg; - int r; rcu_register_thread(); @@ -40,7 +39,7 @@ static void *kvm_vcpu_thread_fn(void *arg) cpu->thread_id = qemu_get_thread_id(); current_cpu = cpu; - r = kvm_init_vcpu(cpu, &error_fatal); + kvm_init_vcpu(cpu, &error_fatal); kvm_init_cpu_signals(cpu); /* signal CPU creation */ @@ -52,8 +51,10 @@ static void *kvm_vcpu_thread_fn(void *arg) qemu_process_cpu_events(cpu); if (cpu_can_run(cpu)) { - r = kvm_cpu_exec(cpu); - if (r == EXCP_DEBUG) { + int excp; + + excp = kvm_cpu_exec(cpu); + if (excp == EXCP_DEBUG) { cpu_handle_guest_debug(cpu); } } -- 2.53.0 When comparing the KVM implementations, RISCV and ARM differ from S390x / PPC and LoongArch. The caller expects a non-zero value to return pausing execution with the EXCP_HLT exception: 3427 int kvm_cpu_exec(CPUState *cpu) 3428 { ... 3434 if (kvm_arch_process_async_events(cpu)) { 3435 return EXCP_HLT; 3436 } 3437 3438 bql_unlock(); 3439 cpu_exec_start(cpu); 3440 3441 /* Inner vCPU loop */ 3442 do { ... Change ARM and RISC-V to return cs->halted for consistent behavior. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/kvm.c | 2 +- target/riscv/kvm/kvm-cpu.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/kvm.c b/target/arm/kvm.c index d40a6a98591..5caabfab63f 100644 --- a/target/arm/kvm.c +++ b/target/arm/kvm.c @@ -1575,7 +1575,7 @@ bool kvm_arch_stop_on_emulation_error(CPUState *cs) int kvm_arch_process_async_events(CPUState *cs) { - return 0; + return cs->halted; } /** diff --git a/target/riscv/kvm/kvm-cpu.c b/target/riscv/kvm/kvm-cpu.c index 97069bf597a..1a61cd5ef37 100644 --- a/target/riscv/kvm/kvm-cpu.c +++ b/target/riscv/kvm/kvm-cpu.c @@ -1568,7 +1568,7 @@ int kvm_arch_irqchip_create(KVMState *s) int kvm_arch_process_async_events(CPUState *cs) { - return 0; + return cs->halted; } void kvm_arch_pre_run(CPUState *cs, struct kvm_run *run) -- 2.53.0 To ensure the vCPU state is flushed on each inner loop iteration, move the flush_cpu_state() inside it, right after the interrupt injection check (catching any interrupt changes from previous loop). Since flush_cpu_state() uses CPUState::vcpu_dirty field as a lazy synchronization mechanism, there is no performance penalty if the cache is clean. This got missed while refactoring the inner loop in commit 93ac76507. Fixes: 93ac765076a ("target/arm/hvf: Keep calling hv_vcpu_run() in loop") Signed-off-by: Philippe Mathieu-Daudé --- target/arm/hvf/hvf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index 3eeb9f70de4..c14ea54a67f 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -2598,8 +2598,6 @@ int hvf_arch_vcpu_exec(CPUState *cpu) } } - flush_cpu_state(cpu); - /* Inner vCPU loop */ do { if (!(cpu->singlestep_flags & SSTEP_NOIRQ) && @@ -2607,6 +2605,8 @@ int hvf_arch_vcpu_exec(CPUState *cpu) return EXCP_INTERRUPT; } + flush_cpu_state(cpu); + bql_unlock(); cpu_exec_start(cpu); r = hv_vcpu_run(cpu->accel->fd); -- 2.53.0 Reduce lock contention by acquiring / releasing the BQL outside of the entire vCPU inner loop. Signed-off-by: Philippe Mathieu-Daudé --- target/arm/hvf/hvf.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c index c14ea54a67f..9c6408d2e25 100644 --- a/target/arm/hvf/hvf.c +++ b/target/arm/hvf/hvf.c @@ -2598,20 +2598,20 @@ int hvf_arch_vcpu_exec(CPUState *cpu) } } + bql_unlock(); + cpu_exec_start(cpu); + /* Inner vCPU loop */ do { if (!(cpu->singlestep_flags & SSTEP_NOIRQ) && hvf_inject_interrupts(cpu)) { - return EXCP_INTERRUPT; + ret = EXCP_INTERRUPT; + break; } flush_cpu_state(cpu); - bql_unlock(); - cpu_exec_start(cpu); r = hv_vcpu_run(cpu->accel->fd); - cpu_exec_end(cpu); - bql_lock(); switch (r) { case HV_SUCCESS: ret = hvf_handle_vmexit(cpu, cpu->accel->exit); @@ -2624,6 +2624,9 @@ int hvf_arch_vcpu_exec(CPUState *cpu) } } while (ret == 0); + cpu_exec_end(cpu); + bql_lock(); + return ret; } -- 2.53.0 Reduce lock contention by acquiring / releasing the BQL outside of the entire vCPU inner loop. Signed-off-by: Philippe Mathieu-Daudé --- target/i386/hvf/hvf.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c index 0ffa3658dc5..f8bc01d38d3 100644 --- a/target/i386/hvf/hvf.c +++ b/target/i386/hvf/hvf.c @@ -1009,6 +1009,9 @@ int hvf_arch_vcpu_exec(CPUState *cpu) return EXCP_HLT; } + bql_unlock(); + cpu_exec_start(cpu); + /* Inner vCPU loop */ do { if (cpu->vcpu_dirty) { @@ -1017,26 +1020,26 @@ int hvf_arch_vcpu_exec(CPUState *cpu) } if (hvf_inject_interrupts(cpu)) { - return EXCP_INTERRUPT; + ret = EXCP_INTERRUPT; + break; } + vmx_update_tpr(cpu); - bql_unlock(); if (!cpu_is_bsp(X86_CPU(cpu)) && cpu->halted) { - bql_lock(); - return EXCP_HLT; + ret = EXCP_HLT; + break; } - cpu_exec_start(cpu); - hv_return_t r = hv_vcpu_run_until(cpu->accel->fd, HV_DEADLINE_FOREVER); assert_hvf_ok(r); - cpu_exec_end(cpu); - ret = hvf_handle_vmexit(cpu); } while (ret == 0); + cpu_exec_end(cpu); + bql_lock(); + return ret; } -- 2.53.0 We can not guarantees the callers of the following public API functions hold the BQL: - arm_set_cpu_on() - arm_set_cpu_on_and_reset() - arm_set_cpu_off() Move the assertion to arm_reset_cpu_async_work() where the async context guarantees the BQL is held. Fixes: 062ba099e01 ("target-arm/powerctl: defer cpu reset work to CPU context") Signed-off-by: Philippe Mathieu-Daudé --- target/arm/arm-powerctl.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/target/arm/arm-powerctl.c b/target/arm/arm-powerctl.c index a06be5cc997..a938b30943c 100644 --- a/target/arm/arm-powerctl.c +++ b/target/arm/arm-powerctl.c @@ -88,8 +88,6 @@ int arm_set_cpu_on(uint64_t cpuid, uint64_t entry, uint64_t context_id, ARMCPU *target_cpu; struct CpuOnInfo *info; - assert(bql_locked()); - trace_arm_powerctl_set_cpu_on(cpuid, target_el, target_aa64 ? "aarch64" : "aarch32", entry, context_id); @@ -194,8 +192,6 @@ int arm_set_cpu_on_and_reset(uint64_t cpuid) CPUState *target_cpu_state; ARMCPU *target_cpu; - assert(bql_locked()); - trace_arm_powerctl_set_cpu_on_and_reset(cpuid); /* Retrieve the cpu we are powering up */ @@ -249,8 +245,6 @@ int arm_set_cpu_off(uint64_t cpuid) CPUState *target_cpu_state; ARMCPU *target_cpu; - assert(bql_locked()); - trace_arm_powerctl_set_cpu_off(cpuid); /* change to the cpu we are powering up */ @@ -276,6 +270,7 @@ int arm_set_cpu_off(uint64_t cpuid) static void arm_reset_cpu_async_work(CPUState *target_cpu_state, run_on_cpu_data data) { + assert(bql_locked()); /* Reset the cpu */ cpu_reset(target_cpu_state); } @@ -285,8 +280,6 @@ int arm_reset_cpu(uint64_t cpuid) CPUState *target_cpu_state; ARMCPU *target_cpu; - assert(bql_locked()); - trace_arm_powerctl_set_cpu_off(cpuid); /* change to the cpu we are resetting */ -- 2.53.0 The format string expects "Gva = ... Gpa = ..." but we provide these addresses swapped, causing confusing misaligned debug output. Re-order by swapping back. Fixes: 59432082cef ("whpx: add arm64 support") Signed-off-by: Philippe Mathieu-Daudé --- target/arm/whpx/whpx-all.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/target/arm/whpx/whpx-all.c b/target/arm/whpx/whpx-all.c index 5e4c25bb7d5..e59580d54ee 100644 --- a/target/arm/whpx/whpx-all.c +++ b/target/arm/whpx/whpx-all.c @@ -442,8 +442,8 @@ int whpx_vcpu_run(CPUState *cpu) error_report("WHPX: cached access to unmapped memory" "Pc = 0x%llx Gva = 0x%llx Gpa = 0x%llx", vcpu->exit_ctx.MemoryAccess.Header.Pc, - vcpu->exit_ctx.MemoryAccess.Gpa, - vcpu->exit_ctx.MemoryAccess.Gva); + vcpu->exit_ctx.MemoryAccess.Gva, + vcpu->exit_ctx.MemoryAccess.Gpa); break; } -- 2.53.0