Re: [PATCH 4/8] accel/tcg: introduce tcg_kick_vcpu_thread

["Philippe Mathieu-Daudé" <philmd@linaro.org>]

On 8/8/25 20:59, Paolo Bonzini wrote:
> qemu_cpu_kick() does not always have to set cpu->exit_request,
> though it does for now.  Introduce a function tcg_kick_vcpu_thread()
> which can be changed in the future to not set cpu->exit_request,
> and reserve cpu_exit() for when *all accelerators* need to set
> that flag.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>   docs/devel/tcg-icount.rst       |  2 +-
>   accel/tcg/tcg-accel-ops-mttcg.h |  3 ---
>   include/exec/cpu-common.h       |  1 +
>   accel/tcg/cpu-exec.c            | 17 ++++++++++++++++-
>   accel/tcg/tcg-accel-ops-mttcg.c |  5 -----
>   accel/tcg/tcg-accel-ops-rr.c    |  2 +-
>   accel/tcg/tcg-accel-ops.c       |  2 +-
>   bsd-user/main.c                 |  2 +-
>   linux-user/main.c               |  2 +-
>   9 files changed, 22 insertions(+), 14 deletions(-)
> 
> diff --git a/docs/devel/tcg-icount.rst b/docs/devel/tcg-icount.rst
> index 7df883446a7..a1dcd79e0fd 100644
> --- a/docs/devel/tcg-icount.rst
> +++ b/docs/devel/tcg-icount.rst
> @@ -37,7 +37,7 @@ translator starts by allocating a budget of instructions to be
>   executed. The budget of instructions is limited by how long it will be
>   until the next timer will expire. We store this budget as part of a
>   vCPU icount_decr field which shared with the machinery for handling
> -cpu_exit(). The whole field is checked at the start of every
> +qemu_cpu_kick(). The whole field is checked at the start of every
>   translated block and will cause a return to the outer loop to deal
>   with whatever caused the exit.
>   
> diff --git a/accel/tcg/tcg-accel-ops-mttcg.h b/accel/tcg/tcg-accel-ops-mttcg.h
> index 8ffa7a9a9fe..5c145cc8595 100644
> --- a/accel/tcg/tcg-accel-ops-mttcg.h
> +++ b/accel/tcg/tcg-accel-ops-mttcg.h
> @@ -10,9 +10,6 @@
>   #ifndef TCG_ACCEL_OPS_MTTCG_H
>   #define TCG_ACCEL_OPS_MTTCG_H
>   
> -/* kick MTTCG vCPU thread */
> -void mttcg_kick_vcpu_thread(CPUState *cpu);
> -
>   /* start an mttcg vCPU thread */
>   void mttcg_start_vcpu_thread(CPUState *cpu);
>   
> diff --git a/include/exec/cpu-common.h b/include/exec/cpu-common.h
> index 9b658a3f48f..e843b09cc99 100644
> --- a/include/exec/cpu-common.h
> +++ b/include/exec/cpu-common.h
> @@ -226,6 +226,7 @@ G_NORETURN void cpu_loop_exit(CPUState *cpu);
>   G_NORETURN void cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
>   
>   /* accel/tcg/cpu-exec.c */
> +void tcg_kick_vcpu_thread(CPUState *cpu);

"exec/cpu-common.h" is not the place, maybe "accel/tcg/cpu-ops.h"?

(Nitpicking, for clarity I'd rather a preliminary pach renaming
mttcg_kick_vcpu_thread -> tcg_kick_vcpu_thread and using it
instead of cpu_exit, then this patch only modifying
tcg_kick_vcpu_thread).

>   int cpu_exec(CPUState *cpu);
>   
>   /**
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index 4bd9ee01c19..1a973596d87 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -748,6 +748,20 @@ static inline bool cpu_handle_exception(CPUState *cpu, int *ret)
>       return false;
>   }
>   
> +void tcg_kick_vcpu_thread(CPUState *cpu)
> +{
> +    /*
> +     * Ensure cpu_exec will see the reason why the exit request was set.
> +     * FIXME: this is not always needed.  Other accelerators instead
> +     * read interrupt_request and set exit_request on demand from the
> +     * CPU thread; see kvm_arch_pre_run() for example.
> +     */
> +    qatomic_store_release(&cpu->exit_request, 1);
> +
> +    /* Ensure cpu_exec will see the exit request after TCG has exited.  */
> +    qatomic_store_release(&cpu->neg.icount_decr.u16.high, -1);
> +}
> +
>   static inline bool icount_exit_request(CPUState *cpu)
>   {
>       if (!icount_enabled()) {
> @@ -774,7 +788,8 @@ static inline bool cpu_handle_interrupt(CPUState *cpu,
>       /* Clear the interrupt flag now since we're processing
>        * cpu->interrupt_request and cpu->exit_request.
>        * Ensure zeroing happens before reading cpu->exit_request or
> -     * cpu->interrupt_request (see also smp_wmb in cpu_exit())
> +     * cpu->interrupt_request (see also store-release in
> +     * tcg_kick_vcpu_thread())
>        */
>       qatomic_set_mb(&cpu->neg.icount_decr.u16.high, 0);
>   
> diff --git a/accel/tcg/tcg-accel-ops-mttcg.c b/accel/tcg/tcg-accel-ops-mttcg.c
> index 39237421757..5757324a8c2 100644
> --- a/accel/tcg/tcg-accel-ops-mttcg.c
> +++ b/accel/tcg/tcg-accel-ops-mttcg.c
> @@ -123,11 +123,6 @@ static void *mttcg_cpu_thread_fn(void *arg)
>       return NULL;
>   }
>   
> -void mttcg_kick_vcpu_thread(CPUState *cpu)
> -{
> -    cpu_exit(cpu);
> -}
> -
>   void mttcg_start_vcpu_thread(CPUState *cpu)
>   {
>       char thread_name[VCPU_THREAD_NAME_SIZE];
> diff --git a/accel/tcg/tcg-accel-ops-rr.c b/accel/tcg/tcg-accel-ops-rr.c
> index c06f3beef2e..87b49377c78 100644
> --- a/accel/tcg/tcg-accel-ops-rr.c
> +++ b/accel/tcg/tcg-accel-ops-rr.c
> @@ -43,7 +43,7 @@ void rr_kick_vcpu_thread(CPUState *unused)
>       CPUState *cpu;
>   
>       CPU_FOREACH(cpu) {
> -        cpu_exit(cpu);
> +        tcg_kick_vcpu_thread(cpu);
>       };
>   }
>   
> diff --git a/accel/tcg/tcg-accel-ops.c b/accel/tcg/tcg-accel-ops.c
> index 02c7600bb7d..f4d5372866a 100644
> --- a/accel/tcg/tcg-accel-ops.c
> +++ b/accel/tcg/tcg-accel-ops.c
> @@ -207,7 +207,7 @@ static void tcg_accel_ops_init(AccelClass *ac)
>   
>       if (qemu_tcg_mttcg_enabled()) {
>           ops->create_vcpu_thread = mttcg_start_vcpu_thread;
> -        ops->kick_vcpu_thread = mttcg_kick_vcpu_thread;
> +        ops->kick_vcpu_thread = tcg_kick_vcpu_thread;
>           ops->handle_interrupt = tcg_handle_interrupt;
>       } else {
>           ops->create_vcpu_thread = rr_start_vcpu_thread;
> diff --git a/bsd-user/main.c b/bsd-user/main.c
> index 7e5d4bbce09..20159207040 100644
> --- a/bsd-user/main.c
> +++ b/bsd-user/main.c
> @@ -216,7 +216,7 @@ bool qemu_cpu_is_self(CPUState *cpu)
>   
>   void qemu_cpu_kick(CPUState *cpu)
>   {
> -    cpu_exit(cpu);
> +    tcg_kick_vcpu_thread(cpu);
>   }
>   
>   /* Assumes contents are already zeroed.  */
> diff --git a/linux-user/main.c b/linux-user/main.c
> index 68972f00a15..dc68fd448b7 100644
> --- a/linux-user/main.c
> +++ b/linux-user/main.c
> @@ -192,7 +192,7 @@ bool qemu_cpu_is_self(CPUState *cpu)
>   
>   void qemu_cpu_kick(CPUState *cpu)
>   {
> -    cpu_exit(cpu);
> +    tcg_kick_vcpu_thread(cpu);
>   }
>   
>   void task_settid(TaskState *ts)