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

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