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

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

On 11/8/25 10:33, Philippe Mathieu-Daudé wrote:
> On 11/8/25 08:10, Paolo Bonzini wrote:
>> On 8/10/25 01:26, Richard Henderson wrote:
>>> On 8/9/25 04:59, Paolo Bonzini wrote:
>>>> +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);
>>>> +}
>>>
>>> So, now both cpu_exit and cpu_kick set exit_request.
>>>
>>> You ifdef this out again for user-only in patch 7, but this does 
>>> suggest that kick and exit are essentially interchangeable.  You 
>>> rearrange things a bit in patch 6, but it's still not clear to me 
>>> what the difference between the two should be.  There's certainly 
>>> nothing at all in include/hw/core/cpu.h to differentiate them.
>>>
>>> Should we instead eliminate one of kick or exit, unifying the paths?
>> In cpu-exec.c terms, qemu_cpu_kick() *should* go out to 
>> cpu_handle_interrupt() whereas cpu_exit() *should* go out to 
>> cpu_handle_exception().  The difference matters for some accelerators 
>> where qemu_cpu_kick() tries not to take the BQL in the vCPU thread.
> 
> While I'm still learning this area, this answer makes sense to me.
> If this is correct (reviewed by others), can we document it in the
> corresponding prototype declarations?

Maybe we can rename as something like cpu_kick_for_interrupt() and
cpu_exit_for_exception()?  Or cpu_kick_maybe_[handle]_interrupt()
and cpu_exit_maybe_[handle]_exception()...