qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] cpus: Cleanups around cpu running state changes
@ 2025-09-25  2:55 Philippe Mathieu-Daudé
  2025-09-25  2:55 ` [PATCH 1/4] cpus: Only resume halted CPUs Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25  2:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Philippe Mathieu-Daudé,
	Mads Ynddal, Alexander Graf, Cameron Esfahani, Richard Henderson,
	Paolo Bonzini, Peter Maydell, Roman Bolshakov, Alex Bennée,
	qemu-arm, Manos Pitsidianakis

Few cleanups following Paolo's recent "accel, cpus: clean up
cpu->exit_request" work (now merged), extracted from my
"split-accel" work.

Philippe Mathieu-Daudé (4):
  cpus: Only resume halted CPUs
  cpus: Access CPUState::thread_kicked atomically
  accel/hvf: Make async_safe_run_on_cpu() safe
  accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped

 accel/tcg/tcg-accel-ops-rr.c | 2 +-
 system/cpus.c                | 8 +++++---
 target/arm/hvf/hvf.c         | 2 ++
 target/i386/hvf/hvf.c        | 2 ++
 4 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.51.0



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/4] cpus: Only resume halted CPUs
  2025-09-25  2:55 [PATCH 0/4] cpus: Cleanups around cpu running state changes Philippe Mathieu-Daudé
@ 2025-09-25  2:55 ` Philippe Mathieu-Daudé
  2025-09-25  8:34   ` Philippe Mathieu-Daudé
  2025-09-25  2:55 ` [PATCH 2/4] cpus: Access CPUState::thread_kicked atomically Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25  2:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Philippe Mathieu-Daudé,
	Mads Ynddal, Alexander Graf, Cameron Esfahani, Richard Henderson,
	Paolo Bonzini, Peter Maydell, Roman Bolshakov, Alex Bennée,
	qemu-arm, Manos Pitsidianakis

Avoid kicking running CPUs, trying to resume them.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/cpus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/system/cpus.c b/system/cpus.c
index aa7bfcf56e5..6062226d4ac 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -666,7 +666,9 @@ void resume_all_vcpus(void)
 
     qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
     CPU_FOREACH(cpu) {
-        cpu_resume(cpu);
+        if (cpu->halted) {
+            cpu_resume(cpu);
+        }
     }
 }
 
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/4] cpus: Access CPUState::thread_kicked atomically
  2025-09-25  2:55 [PATCH 0/4] cpus: Cleanups around cpu running state changes Philippe Mathieu-Daudé
  2025-09-25  2:55 ` [PATCH 1/4] cpus: Only resume halted CPUs Philippe Mathieu-Daudé
@ 2025-09-25  2:55 ` Philippe Mathieu-Daudé
  2025-09-29  5:52   ` Manos Pitsidianakis
  2025-09-25  2:55 ` [PATCH 3/4] accel/hvf: Make async_safe_run_on_cpu() safe Philippe Mathieu-Daudé
  2025-09-25  2:55 ` [PATCH 4/4] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped Philippe Mathieu-Daudé
  3 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25  2:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Philippe Mathieu-Daudé,
	Mads Ynddal, Alexander Graf, Cameron Esfahani, Richard Henderson,
	Paolo Bonzini, Peter Maydell, Roman Bolshakov, Alex Bennée,
	qemu-arm, Manos Pitsidianakis

cpus_kick_thread() is called via cpu_exit() -> qemu_cpu_kick(),
and also via gdb_syscall_handling(). Access the CPUState field
using atomic accesses. See commit 8ac2ca02744 ("accel: use atomic
accesses for exit_request") for rationale.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 system/cpus.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/system/cpus.c b/system/cpus.c
index 6062226d4ac..818a8047198 100644
--- a/system/cpus.c
+++ b/system/cpus.c
@@ -480,10 +480,10 @@ void qemu_process_cpu_events(CPUState *cpu)
 
 void cpus_kick_thread(CPUState *cpu)
 {
-    if (cpu->thread_kicked) {
+    if (qatomic_read(&cpu->thread_kicked)) {
         return;
     }
-    cpu->thread_kicked = true;
+    qatomic_set(&cpu->thread_kicked, true);
 
 #ifndef _WIN32
     int err = pthread_kill(cpu->thread->thread, SIG_IPI);
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/4] accel/hvf: Make async_safe_run_on_cpu() safe
  2025-09-25  2:55 [PATCH 0/4] cpus: Cleanups around cpu running state changes Philippe Mathieu-Daudé
  2025-09-25  2:55 ` [PATCH 1/4] cpus: Only resume halted CPUs Philippe Mathieu-Daudé
  2025-09-25  2:55 ` [PATCH 2/4] cpus: Access CPUState::thread_kicked atomically Philippe Mathieu-Daudé
@ 2025-09-25  2:55 ` Philippe Mathieu-Daudé
  2025-09-26  0:13   ` Richard Henderson
  2025-09-25  2:55 ` [PATCH 4/4] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped Philippe Mathieu-Daudé
  3 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25  2:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Philippe Mathieu-Daudé,
	Mads Ynddal, Alexander Graf, Cameron Esfahani, Richard Henderson,
	Paolo Bonzini, Peter Maydell, Roman Bolshakov, Alex Bennée,
	qemu-arm, Manos Pitsidianakis, Peter Collingbourne,
	Roman Bolshakov, Sergio Lopez

Wrap hv_vcpu_run() calls with cpu_exec_start/end() in order to
have the main loop perform more exclusive sections while all
vCPUs are quiescent.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
Could be related to the dubious thread_kicked access and pselect() in
hvf_wait_for_ipi() (commit 219c101fa7f "arm/hvf: Add a WFI handler").

Cc: Peter Collingbourne <pcc@google.com>
Cc: Alexander Graf <agraf@csgraf.de>
Cc: Roman Bolshakov <r.bolshakov@yadro.com>
Cc: Sergio Lopez <slp@redhat.com>
---
 target/arm/hvf/hvf.c  | 2 ++
 target/i386/hvf/hvf.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
index b77db99079e..377eb4bbc6b 100644
--- a/target/arm/hvf/hvf.c
+++ b/target/arm/hvf/hvf.c
@@ -1927,7 +1927,9 @@ int hvf_vcpu_exec(CPUState *cpu)
     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:
diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
index 8445cadecec..913ad47f3d8 100644
--- a/target/i386/hvf/hvf.c
+++ b/target/i386/hvf/hvf.c
@@ -749,8 +749,10 @@ int hvf_vcpu_exec(CPUState *cpu)
             return EXCP_HLT;
         }
 
+        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);
 
         /* handle VMEXIT */
         uint64_t exit_reason = rvmcs(cpu->accel->fd, VMCS_EXIT_REASON);
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 4/4] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped
  2025-09-25  2:55 [PATCH 0/4] cpus: Cleanups around cpu running state changes Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2025-09-25  2:55 ` [PATCH 3/4] accel/hvf: Make async_safe_run_on_cpu() safe Philippe Mathieu-Daudé
@ 2025-09-25  2:55 ` Philippe Mathieu-Daudé
  2025-09-26  0:44   ` Richard Henderson
  3 siblings, 1 reply; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25  2:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Philippe Mathieu-Daudé,
	Mads Ynddal, Alexander Graf, Cameron Esfahani, Richard Henderson,
	Paolo Bonzini, Peter Maydell, Roman Bolshakov, Alex Bennée,
	qemu-arm, Manos Pitsidianakis

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 accel/tcg/tcg-accel-ops-rr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
index 2fb46439971..f84342e0449 100644
--- a/accel/tcg/tcg-accel-ops-rr.c
+++ b/accel/tcg/tcg-accel-ops-rr.c
@@ -197,7 +197,7 @@ static void *rr_cpu_thread_fn(void *arg)
     qemu_guest_random_seed_thread_part2(cpu->random_seed);
 
     /* wait for initial kick-off after machine start */
-    while (first_cpu->stopped) {
+    while (cpu_is_stopped(first_cpu)) {
         qemu_cond_wait_bql(first_cpu->halt_cond);
 
         /* process any pending work */
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/4] cpus: Only resume halted CPUs
  2025-09-25  2:55 ` [PATCH 1/4] cpus: Only resume halted CPUs Philippe Mathieu-Daudé
@ 2025-09-25  8:34   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 9+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-09-25  8:34 UTC (permalink / raw)
  To: qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Mads Ynddal, Alexander Graf,
	Cameron Esfahani, Richard Henderson, Paolo Bonzini, Peter Maydell,
	Roman Bolshakov, Alex Bennée, qemu-arm, Manos Pitsidianakis

On 25/9/25 04:55, Philippe Mathieu-Daudé wrote:
> Avoid kicking running CPUs, trying to resume them.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   system/cpus.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/system/cpus.c b/system/cpus.c
> index aa7bfcf56e5..6062226d4ac 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -666,7 +666,9 @@ void resume_all_vcpus(void)
>   
>       qemu_clock_enable(QEMU_CLOCK_VIRTUAL, true);
>       CPU_FOREACH(cpu) {
> -        cpu_resume(cpu);
> +        if (cpu->halted) {
> +            cpu_resume(cpu);
> +        }
>       }
>   }
>   

Please disregard this patch for now, as other changes (non
included in this series) are required before this change is
correct.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] accel/hvf: Make async_safe_run_on_cpu() safe
  2025-09-25  2:55 ` [PATCH 3/4] accel/hvf: Make async_safe_run_on_cpu() safe Philippe Mathieu-Daudé
@ 2025-09-26  0:13   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2025-09-26  0:13 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel

On 9/24/25 19:55, Philippe Mathieu-Daudé wrote:
> Wrap hv_vcpu_run() calls with cpu_exec_start/end() in order to
> have the main loop perform more exclusive sections while all
> vCPUs are quiescent.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
> Could be related to the dubious thread_kicked access and pselect() in
> hvf_wait_for_ipi() (commit 219c101fa7f "arm/hvf: Add a WFI handler").
> 
> Cc: Peter Collingbourne <pcc@google.com>
> Cc: Alexander Graf <agraf@csgraf.de>
> Cc: Roman Bolshakov <r.bolshakov@yadro.com>
> Cc: Sergio Lopez <slp@redhat.com>
> ---
>   target/arm/hvf/hvf.c  | 2 ++
>   target/i386/hvf/hvf.c | 2 ++
>   2 files changed, 4 insertions(+)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

> 
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index b77db99079e..377eb4bbc6b 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -1927,7 +1927,9 @@ int hvf_vcpu_exec(CPUState *cpu)
>       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:
> diff --git a/target/i386/hvf/hvf.c b/target/i386/hvf/hvf.c
> index 8445cadecec..913ad47f3d8 100644
> --- a/target/i386/hvf/hvf.c
> +++ b/target/i386/hvf/hvf.c
> @@ -749,8 +749,10 @@ int hvf_vcpu_exec(CPUState *cpu)
>               return EXCP_HLT;
>           }
>   
> +        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);
>   
>           /* handle VMEXIT */
>           uint64_t exit_reason = rvmcs(cpu->accel->fd, VMCS_EXIT_REASON);



^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 4/4] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped
  2025-09-25  2:55 ` [PATCH 4/4] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped Philippe Mathieu-Daudé
@ 2025-09-26  0:44   ` Richard Henderson
  0 siblings, 0 replies; 9+ messages in thread
From: Richard Henderson @ 2025-09-26  0:44 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Phil Dennis-Jordan, Pierrick Bouvier, Mads Ynddal, Alexander Graf,
	Cameron Esfahani, Paolo Bonzini, Peter Maydell, Roman Bolshakov,
	Alex Bennée, qemu-arm, Manos Pitsidianakis

On 9/24/25 19:55, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   accel/tcg/tcg-accel-ops-rr.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
> index 2fb46439971..f84342e0449 100644
> --- a/accel/tcg/tcg-accel-ops-rr.c
> +++ b/accel/tcg/tcg-accel-ops-rr.c
> @@ -197,7 +197,7 @@ static void *rr_cpu_thread_fn(void *arg)
>       qemu_guest_random_seed_thread_part2(cpu->random_seed);
>   
>       /* wait for initial kick-off after machine start */
> -    while (first_cpu->stopped) {
> +    while (cpu_is_stopped(first_cpu)) {
>           qemu_cond_wait_bql(first_cpu->halt_cond);
>   
>           /* process any pending work */


Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/4] cpus: Access CPUState::thread_kicked atomically
  2025-09-25  2:55 ` [PATCH 2/4] cpus: Access CPUState::thread_kicked atomically Philippe Mathieu-Daudé
@ 2025-09-29  5:52   ` Manos Pitsidianakis
  0 siblings, 0 replies; 9+ messages in thread
From: Manos Pitsidianakis @ 2025-09-29  5:52 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: qemu-devel, Phil Dennis-Jordan, Pierrick Bouvier, Mads Ynddal,
	Alexander Graf, Cameron Esfahani, Richard Henderson,
	Paolo Bonzini, Peter Maydell, Roman Bolshakov, Alex Bennée,
	qemu-arm

On Thu, Sep 25, 2025 at 5:55 AM Philippe Mathieu-Daudé
<philmd@linaro.org> wrote:
>
> cpus_kick_thread() is called via cpu_exit() -> qemu_cpu_kick(),
> and also via gdb_syscall_handling(). Access the CPUState field
> using atomic accesses. See commit 8ac2ca02744 ("accel: use atomic
> accesses for exit_request") for rationale.
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>  system/cpus.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/system/cpus.c b/system/cpus.c
> index 6062226d4ac..818a8047198 100644
> --- a/system/cpus.c
> +++ b/system/cpus.c
> @@ -480,10 +480,10 @@ void qemu_process_cpu_events(CPUState *cpu)
>
>  void cpus_kick_thread(CPUState *cpu)
>  {
> -    if (cpu->thread_kicked) {
> +    if (qatomic_read(&cpu->thread_kicked)) {
>          return;
>      }
> -    cpu->thread_kicked = true;
> +    qatomic_set(&cpu->thread_kicked, true);
>
>  #ifndef _WIN32
>      int err = pthread_kill(cpu->thread->thread, SIG_IPI);
> --
> 2.51.0
>

Reviewed-by: Manos Pitsidianakis <manos.pitsidianakis@linaro.org>


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2025-09-29  5:54 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-25  2:55 [PATCH 0/4] cpus: Cleanups around cpu running state changes Philippe Mathieu-Daudé
2025-09-25  2:55 ` [PATCH 1/4] cpus: Only resume halted CPUs Philippe Mathieu-Daudé
2025-09-25  8:34   ` Philippe Mathieu-Daudé
2025-09-25  2:55 ` [PATCH 2/4] cpus: Access CPUState::thread_kicked atomically Philippe Mathieu-Daudé
2025-09-29  5:52   ` Manos Pitsidianakis
2025-09-25  2:55 ` [PATCH 3/4] accel/hvf: Make async_safe_run_on_cpu() safe Philippe Mathieu-Daudé
2025-09-26  0:13   ` Richard Henderson
2025-09-25  2:55 ` [PATCH 4/4] accel/tcg: Use cpu_is_stopped() helper to access CPUState::stopped Philippe Mathieu-Daudé
2025-09-26  0:44   ` Richard Henderson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).