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

[Paolo Bonzini <pbonzini@redhat.com>]

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