[PATCH 00/16] cpus: Improvements around BQL locking, improving docstring

[PATCH 00/16] cpus: Improvements around BQL locking, improving docstring

[Philippe Mathieu-Daudé]

Hi,

While working on the hybrid-accelerator, I had to debug issues
with vCPU scheduling and BQL. This series contains patches I
wrote mostly as note for myself to better understand what was
going on. Please review carefully as I'm way out of my confort
zone...

Thanks,

Phil.

Philippe Mathieu-Daudé (16):
  accel: Label outer / inner vCPU execution loops
  cpus: Improve qemu_cpu_kick_self() docstring
  cpus: Rename cpu_stop_current() -> qemu_cpu_stop_self()
  cpus: Rename qemu_cpu_stop() to qemu_cpu_ack_stop_request()
  cpus: Improve cpu_pause() docstring
  cpus: Improve cpu_exit() docstring
  cpus: Document process_queued_cpu_work*() runs on current vCPU
  cpus: Slightly improve *run_on_cpu() docstrings
  accel/tcg: Use RUN_ON_CPU_NULL for tcg_commit() callbacks
  accel/kvm: Clarify use of @r variable in vcpu thread loop
  accel/kvm: Consistently return CPU halt state from
    process_async_events
  target/arm/hvf: Move flush_cpu_state() into the inner vCPU loop
  target/arm/hvf: Lock BQL outside of the vCPU inner execution loop
  target/i386/hvf: Lock BQL outside of the vCPU inner execution loop
  target/arm/powerctl: Move BQL assertions to async work context
  target/arm/whpx: Fix argument order typo in memory access error
    message

 include/hw/core/cpu.h             | 54 +++++++++++++++++++------------
 include/system/cpus.h             | 19 +++++++++--
 accel/dummy-cpus.c                |  1 +
 accel/hvf/hvf-accel-ops.c         |  1 +
 accel/kvm/kvm-accel-ops.c         | 10 +++---
 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 +
 hw/ppc/spapr_rtas.c               |  2 +-
 system/cpus.c                     |  9 +++---
 system/physmem.c                  |  4 +--
 system/runstate.c                 |  6 ++--
 target/arm/arm-powerctl.c         |  9 +-----
 target/arm/hvf/hvf.c              | 16 +++++----
 target/arm/kvm.c                  |  2 +-
 target/arm/whpx/whpx-all.c        | 10 +++---
 target/i386/hvf/hvf.c             | 20 +++++++-----
 target/i386/nvmm/nvmm-accel-ops.c |  1 +
 target/i386/whpx/whpx-all.c       |  1 +
 target/riscv/kvm/kvm-cpu.c        |  2 +-
 21 files changed, 106 insertions(+), 66 deletions(-)

-- 
2.53.0

[PATCH 01/16] accel: Label outer / inner vCPU execution loops

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 02/16] cpus: Improve qemu_cpu_kick_self() docstring

[Philippe Mathieu-Daudé]

Be a bit more descriptive than "Unblock cpu" :)

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 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

[PATCH 03/16] cpus: Rename cpu_stop_current() -> qemu_cpu_stop_self()

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 04/16] cpus: Rename qemu_cpu_stop() to qemu_cpu_ack_stop_request()

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 05/16] cpus: Improve cpu_pause() docstring

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 06/16] cpus: Improve cpu_exit() docstring

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 07/16] cpus: Document process_queued_cpu_work*() runs on current vCPU

[Philippe Mathieu-Daudé]

Mention process_queued_cpu_work*() functions must run on
a vCPU thread.

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 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

[PATCH 08/16] cpus: Slightly improve *run_on_cpu() docstrings

[Philippe Mathieu-Daudé]

Be a bit more precise mentioning BQL use and blocking calls.

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 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

[PATCH 09/16] accel/tcg: Use RUN_ON_CPU_NULL for tcg_commit() callbacks

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
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

[PATCH 10/16] accel/kvm: Clarify use of @r variable in vcpu thread loop

[Philippe Mathieu-Daudé]

@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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 11/16] accel/kvm: Consistently return CPU halt state from process_async_events

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 12/16] target/arm/hvf: Move flush_cpu_state() into the inner vCPU loop

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 13/16] target/arm/hvf: Lock BQL outside of the vCPU inner execution loop

[Philippe Mathieu-Daudé]

Reduce lock contention by acquiring / releasing the BQL
outside of the entire vCPU inner loop.

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 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

[PATCH 14/16] target/i386/hvf: Lock BQL outside of the vCPU inner execution loop

[Philippe Mathieu-Daudé]

Reduce lock contention by acquiring / releasing the BQL
outside of the entire vCPU inner loop.

Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
---
 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

[PATCH 15/16] target/arm/powerctl: Move BQL assertions to async work context

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

[PATCH 16/16] target/arm/whpx: Fix argument order typo in memory access error message

[Philippe Mathieu-Daudé]

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é <philmd@oss.qualcomm.com>
---
 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

Re: [PATCH 02/16] cpus: Improve qemu_cpu_kick_self() docstring

[Paolo Bonzini]

On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> Be a bit more descriptive than "Unblock cpu" :)
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>   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.
> + */

Neither is correct.  What it does is force the vCPU not to enter the
accelerator, and immediately exit again to process events.  This is only
needed because KVM doesn't want you to process events because KVM allows
you to do so.

I'd rather have something like:

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 83cbd120a84..e5d068e341f 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -3292,7 +3292,7 @@ static void kvm_cpu_kick_self(void)
      if (kvm_immediate_exit) {
          kvm_cpu_kick(current_cpu);
      } else {
-        qemu_cpu_kick_self();
+        cpus_kick_thread(current_cpu);
      }
  }
  
diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
index 8a1af35ed32..b9b1ee39850 100644
--- a/target/i386/nvmm/nvmm-all.c
+++ b/target/i386/nvmm/nvmm-all.c
@@ -753,7 +753,7 @@ nvmm_vcpu_loop(CPUState *cpu)
  #if NVMM_USER_VERSION >= 2
              nvmm_vcpu_stop(vcpu);
  #else
-            qemu_cpu_kick_self();
+            cpus_kick_thread(current_cpu);
  #endif
          }
  
and get rid of qemu_cpu_kick_self() completely.

Paolo

Re: [PATCH 02/16] cpus: Improve qemu_cpu_kick_self() docstring

[Philippe Mathieu-Daudé]

On 17/8/26 17:28, Paolo Bonzini wrote:
> On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
>> Be a bit more descriptive than "Unblock cpu" :)
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>> ---
>>   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.
>> + */
> 
> Neither is correct.

"Must be called from within the vCPU thread itself." is because of
the assertion:

system/cpus.c:452:void qemu_cpu_kick_self(void)
system/cpus.c-453-{
system/cpus.c-454-    assert(current_cpu);
system/cpus.c-455-    cpus_kick_thread(current_cpu);
system/cpus.c-456-}

>  What it does is force the vCPU not to enter the
> accelerator, and immediately exit again to process events.  This is only
> needed because KVM doesn't want you to process events because KVM allows
> you to do so.

I got confused by this comment in accel/mshv/mshv-all.c and interpreted
too much:

/*
  * The signal handler is triggered when QEMU's main thread receives a 
SIG_IPI
  * (SIGUSR1). This signal causes the current CPU thread to be kicked, 
forcing a
  * VM exit on the CPU. The VM exit generates an exit reason that breaks 
the loop
  * (see mshv_cpu_exec). If the exit is due to a Ctrl+A+x command, the 
system
  * will shut down. For other cases, the system will continue running.
  */
static void sa_ipi_handler(int sig)
{
     /* TODO: call IOCTL to set_immediate_exit, once implemented. */

     qemu_cpu_kick_self();
}
> 
> I'd rather have something like:
> 
> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> index 83cbd120a84..e5d068e341f 100644
> --- a/accel/kvm/kvm-all.c
> +++ b/accel/kvm/kvm-all.c
> @@ -3292,7 +3292,7 @@ static void kvm_cpu_kick_self(void)
>       if (kvm_immediate_exit) {
>           kvm_cpu_kick(current_cpu);
>       } else {
> -        qemu_cpu_kick_self();
> +        cpus_kick_thread(current_cpu);
>       }
>   }
> 
> diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
> index 8a1af35ed32..b9b1ee39850 100644
> --- a/target/i386/nvmm/nvmm-all.c
> +++ b/target/i386/nvmm/nvmm-all.c
> @@ -753,7 +753,7 @@ nvmm_vcpu_loop(CPUState *cpu)
>   #if NVMM_USER_VERSION >= 2
>               nvmm_vcpu_stop(vcpu);
>   #else
> -            qemu_cpu_kick_self();
> +            cpus_kick_thread(current_cpu);
>   #endif
>           }
> 
> and get rid of qemu_cpu_kick_self() completely.

OK I'll take that route.

Re: [PATCH 06/16] cpus: Improve cpu_exit() docstring

[Paolo Bonzini]

On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> 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é <philmd@oss.qualcomm.com>
> ---
>   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.

More precise: to sleep or process asynchronous work items (see the 
run_on_cpu() family of functions).

Paolo

Re: [PATCH 08/16] cpus: Slightly improve *run_on_cpu() docstrings

[Paolo Bonzini]

On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> Be a bit more precise mentioning BQL use and blocking calls.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>   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.

Might as well mention that @func is run without the BQL taken and in 
fact must not take it (see comment in process_queued_cpu_work()).

Paolo

>    */
>   void async_safe_run_on_cpu(CPUState *cpu, run_on_cpu_func func, run_on_cpu_data data);
>   

Re: [PATCH 11/16] accel/kvm: Consistently return CPU halt state from process_async_events

[Paolo Bonzini]

On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> 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 {
>    ...

Your patch is correct because, even if cpu_thread_is_idle() sees 
cpu->halted == false, the flag could become true during 
process_queued_cpu_work() because it releases the BQL.

However, I think the better fix is to remove the return code, and check 
cpu->halted in kvm_cpu_exec().

Thanks,

Paolo

> Change ARM and RISC-V to return cs->halted for consistent behavior.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>   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)

Re: [PATCH 08/16] cpus: Slightly improve *run_on_cpu() docstrings

[Peter Maydell]

On Mon, 17 Aug 2026 at 16:50, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> > Be a bit more precise mentioning BQL use and blocking calls.
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> > ---
> >   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_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.
>
> Might as well mention that @func is run without the BQL taken and in
> fact must not take it (see comment in process_queued_cpu_work()).

We should also mention the massive bear-trap in the run_on_cpu()
function, which is that (a) you must call it with the BQL and
(b) it will *drop the BQL* during the course of execution, so the
caller must be prepared for other threads etc to be able to
reenter it, update state, etc between when it calls run_on_cpu()
and when it returns.

(cf https://gitlab.com/qemu-project/qemu/-/work_items/1109 where
we can segfault in the raspi display device, because
memory_region_snapshot_and_clear_dirty() drops the BQL and allows
the device code to be re-entered, much to its surprise.)

thanks
-- PMM

Re: [PATCH 13/16] target/arm/hvf: Lock BQL outside of the vCPU inner execution loop

[Paolo Bonzini]

On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> Reduce lock contention by acquiring / releasing the BQL
> outside of the entire vCPU inner loop.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>   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)) {

It always return zero, so you can make it void and change to

           if (!(cpu->singlestep_flags & SSTEP_NOIRQ)) {
               hvf_inject_interrupts(cpu);
	  }

However, a bigger question: who calls cpu_reset_interrupt() here:

     if (cpu_test_interrupt(cpu, CPU_INTERRUPT_FIQ)) {
         trace_hvf_inject_fiq();
         hv_vcpu_set_pending_interrupt(cpu->accel->fd, HV_INTERRUPT_TYPE_FIQ,
                                       true);
     }

     if (cpu_test_interrupt(cpu, CPU_INTERRUPT_HARD)) {
         trace_hvf_inject_irq();
         hv_vcpu_set_pending_interrupt(cpu->accel->fd, HV_INTERRUPT_TYPE_IRQ,
                                       true);
     }

and can this be changed to do something like arm_cpu_kvm_set_irq() instead?
Having the function called only with level == true is a bit unsettling...

Paolo

> -            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;
>   }
>   

Re: [PATCH 14/16] target/i386/hvf: Lock BQL outside of the vCPU inner execution loop

[Paolo Bonzini]

On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
> Reduce lock contention by acquiring / releasing the BQL
> outside of the entire vCPU inner loop.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
> ---
>   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)) {

Nope, hvf_inject_interrupts() calls cpu_get_pic_interrupt() which needs 
the BQL.

Paolo

> -            return EXCP_INTERRUPT;
> +            ret = EXCP_INTERRUPT;
> +            break;
>           }
> +
>           vmx_update_tpr(cpu);
>   
> -        bql_unlock();
>           if (!cpu_is_bsp(X86_CPU(cpu)) && cpu->halted) {

This is also incorrect by the way - chceking cpu->halted needs the BQL. 
So bql_unlock() must be moved right before cpu_exec_start().  Can you 
fix it?

Thanks,

Paolo

> -            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;
>   }
>   

Re: [PATCH 13/16] target/arm/hvf: Lock BQL outside of the vCPU inner execution loop

[Philippe Mathieu-Daudé]

On 17/8/26 18:11, Paolo Bonzini wrote:
> On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
>> Reduce lock contention by acquiring / releasing the BQL
>> outside of the entire vCPU inner loop.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
>> ---
>>   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)) {
> 
> It always return zero, so you can make it void and change to
> 
>            if (!(cpu->singlestep_flags & SSTEP_NOIRQ)) {
>                hvf_inject_interrupts(cpu);
>        }
> 
> However, a bigger question: who calls cpu_reset_interrupt() here:
> 
>      if (cpu_test_interrupt(cpu, CPU_INTERRUPT_FIQ)) {
>          trace_hvf_inject_fiq();
>          hv_vcpu_set_pending_interrupt(cpu->accel->fd, 
> HV_INTERRUPT_TYPE_FIQ,
>                                        true);
>      }
> 
>      if (cpu_test_interrupt(cpu, CPU_INTERRUPT_HARD)) {
>          trace_hvf_inject_irq();
>          hv_vcpu_set_pending_interrupt(cpu->accel->fd, 
> HV_INTERRUPT_TYPE_IRQ,
>                                        true);
>      }
> 
> and can this be changed to do something like arm_cpu_kvm_set_irq() instead?
> Having the function called only with level == true is a bit unsettling...

Digging further this other change from a86024eb2df56f looks dubious:

  int hvf_arch_vcpu_exec(CPUState *cpu)
  {
      int ret;
      hv_return_t r;

      if (cpu->halted) {
-        return EXCP_HLT;
+        if (!cpu_has_work(cpu)) {
+            return EXCP_HLT;
+        }
+        cpu->halted = 0;
+        if (!hvf_irqchip_in_kernel()) {
+            timer_del(cpu->accel->wfi_timer);
+        }
      }

> 
> Paolo
> 
>> -            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;
>>   }
> 
> 

Re: [PATCH 11/16] accel/kvm: Consistently return CPU halt state from process_async_events

[Philippe Mathieu-Daudé]

On 17/8/26 17:59, Paolo Bonzini wrote:
> On 8/13/26 20:16, Philippe Mathieu-Daudé wrote:
>> 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 {
>>    ...
> 
> Your patch is correct because, even if cpu_thread_is_idle() sees cpu- 
>  >halted == false, the flag could become true during 
> process_queued_cpu_work() because it releases the BQL.
> 
> However, I think the better fix is to remove the return code, and check 
> cpu->halted in kvm_cpu_exec().

Looking at it, even better would be to move *process_async_events()
from inner loop to outer loop for all accelerators. For HW accels it
seems quite trivial; for TCG I need to figure if it is safe to call
icount_prepare_for_run() with the BQL taken, due to the replay_mutex
uses -- here the win is cpu_handle_halt() ends restricted to system
and we can remove the odd EXCP_HLT handling in user emulation.

> 
> Thanks,
> 
> Paolo

Re: [PATCH 02/16] cpus: Improve qemu_cpu_kick_self() docstring

[Paolo Bonzini]

On Mon, Aug 17, 2026 at 5:46 PM Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
> >> -/* 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.
> >> + */
> >
> > Neither is correct.

Right, the first of the two in "neither" referred to the existing
"unblock CPU" comment (it's clearly unblocked if it's calling
qemu_cpu_kick_self!).  The requirement to call from the vCPU thread is
correct, sorry about the imprecise reference.

> I got confused by this comment in accel/mshv/mshv-all.c and interpreted
> too much:

Yes, that code is broken and there is a patch queued to make it go away.

Paolo

> /*
>   * The signal handler is triggered when QEMU's main thread receives a
> SIG_IPI
>   * (SIGUSR1). This signal causes the current CPU thread to be kicked,
> forcing a
>   * VM exit on the CPU. The VM exit generates an exit reason that breaks
> the loop
>   * (see mshv_cpu_exec). If the exit is due to a Ctrl+A+x command, the
> system
>   * will shut down. For other cases, the system will continue running.
>   */
> static void sa_ipi_handler(int sig)
> {
>      /* TODO: call IOCTL to set_immediate_exit, once implemented. */
>
>      qemu_cpu_kick_self();
> }
> >
> > I'd rather have something like:
> >
> > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
> > index 83cbd120a84..e5d068e341f 100644
> > --- a/accel/kvm/kvm-all.c
> > +++ b/accel/kvm/kvm-all.c
> > @@ -3292,7 +3292,7 @@ static void kvm_cpu_kick_self(void)
> >       if (kvm_immediate_exit) {
> >           kvm_cpu_kick(current_cpu);
> >       } else {
> > -        qemu_cpu_kick_self();
> > +        cpus_kick_thread(current_cpu);
> >       }
> >   }
> >
> > diff --git a/target/i386/nvmm/nvmm-all.c b/target/i386/nvmm/nvmm-all.c
> > index 8a1af35ed32..b9b1ee39850 100644
> > --- a/target/i386/nvmm/nvmm-all.c
> > +++ b/target/i386/nvmm/nvmm-all.c
> > @@ -753,7 +753,7 @@ nvmm_vcpu_loop(CPUState *cpu)
> >   #if NVMM_USER_VERSION >= 2
> >               nvmm_vcpu_stop(vcpu);
> >   #else
> > -            qemu_cpu_kick_self();
> > +            cpus_kick_thread(current_cpu);
> >   #endif
> >           }
> >
> > and get rid of qemu_cpu_kick_self() completely.
>
> OK I'll take that route.
>

Re: [PATCH 11/16] accel/kvm: Consistently return CPU halt state from process_async_events

[Paolo Bonzini]

On Tue, Aug 18, 2026 at 11:24 AM Philippe Mathieu-Daudé
<philmd@oss.qualcomm.com> wrote:
> > Your patch is correct because, even if cpu_thread_is_idle() sees cpu-
> >  >halted == false, the flag could become true during
> > process_queued_cpu_work() because it releases the BQL.
> >
> > However, I think the better fix is to remove the return code, and check
> > cpu->halted in kvm_cpu_exec().
>
> Looking at it, even better would be to move *process_async_events()
> from inner loop to outer loop for all accelerators. For HW accels it
> seems quite trivial; for TCG I need to figure if it is safe to call
> icount_prepare_for_run() with the BQL taken, due to the replay_mutex
> uses -- here the win is cpu_handle_halt() ends restricted to system
> and we can remove the odd EXCP_HLT handling in user emulation.

Makes sense, but probably it would be a series of its own. :)

Paolo

Re: [PATCH 11/16] accel/kvm: Consistently return CPU halt state from process_async_events

[Philippe Mathieu-Daudé]

On 18/8/26 12:30, Paolo Bonzini wrote:
> On Tue, Aug 18, 2026 at 11:24 AM Philippe Mathieu-Daudé
> <philmd@oss.qualcomm.com> wrote:
>>> Your patch is correct because, even if cpu_thread_is_idle() sees cpu-
>>>   >halted == false, the flag could become true during
>>> process_queued_cpu_work() because it releases the BQL.
>>>
>>> However, I think the better fix is to remove the return code, and check
>>> cpu->halted in kvm_cpu_exec().
>>
>> Looking at it, even better would be to move *process_async_events()
>> from inner loop to outer loop for all accelerators. For HW accels it
>> seems quite trivial; for TCG I need to figure if it is safe to call
>> icount_prepare_for_run() with the BQL taken, due to the replay_mutex
>> uses -- here the win is cpu_handle_halt() ends restricted to system
>> and we can remove the odd EXCP_HLT handling in user emulation.
> 
> Makes sense, but probably it would be a series of its own. :)

Good, right.

(FYI the motivation behind these cleanups and documentation is to
get hybrid-acceleration, so a consistent vcpu_thread_routine between
accelerators:
https://lore.kernel.org/qemu-devel/20250620172751.94231-10-philmd@linaro.org/)