Re: [RFC PATCH 6/7] accel/hvf: Use CPU_FOREACH_HVF()

[Daniel Henrique Barboza <dbarboza@ventanamicro.com>]

On 1/6/25 5:02 PM, Philippe Mathieu-Daudé wrote:
> Only iterate over HVF vCPUs when running HVF specific code.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   include/system/hvf_int.h  | 4 ++++
>   accel/hvf/hvf-accel-ops.c | 9 +++++----
>   target/arm/hvf/hvf.c      | 4 ++--
>   3 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/include/system/hvf_int.h b/include/system/hvf_int.h
> index 42ae18433f0..3cf64faabd1 100644
> --- a/include/system/hvf_int.h
> +++ b/include/system/hvf_int.h
> @@ -11,6 +11,8 @@
>   #ifndef HVF_INT_H
>   #define HVF_INT_H
>   
> +#include "system/hw_accel.h"
> +
>   #ifdef __aarch64__
>   #include <Hypervisor/Hypervisor.h>
>   typedef hv_vcpu_t hvf_vcpuid;
> @@ -74,4 +76,6 @@ int hvf_put_registers(CPUState *);
>   int hvf_get_registers(CPUState *);
>   void hvf_kick_vcpu_thread(CPUState *cpu);
>   
> +#define CPU_FOREACH_HVF(cpu) CPU_FOREACH_HWACCEL(cpu)


Cosmetic comment: given that this is HVF specific code and we only support one hw
accelerator at a time, I'd skip this alias and use CPU_FOREACH_HWACCEL(cpu) directly.
It would make it easier when grepping to see where and how the macro is being used.
Same thing in the next patch with CPU_FOREACH_KVM().


LGTM otherwise. Thanks,

Daniel


> +
>   #endif
> diff --git a/accel/hvf/hvf-accel-ops.c b/accel/hvf/hvf-accel-ops.c
> index 945ba720513..bbbe2f8d45b 100644
> --- a/accel/hvf/hvf-accel-ops.c
> +++ b/accel/hvf/hvf-accel-ops.c
> @@ -504,7 +504,7 @@ static int hvf_insert_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
>           }
>       }
>   
> -    CPU_FOREACH(cpu) {
> +    CPU_FOREACH_HVF(cpu) {
>           err = hvf_update_guest_debug(cpu);
>           if (err) {
>               return err;
> @@ -543,7 +543,7 @@ static int hvf_remove_breakpoint(CPUState *cpu, int type, vaddr addr, vaddr len)
>           }
>       }
>   
> -    CPU_FOREACH(cpu) {
> +    CPU_FOREACH_HVF(cpu) {
>           err = hvf_update_guest_debug(cpu);
>           if (err) {
>               return err;
> @@ -560,7 +560,7 @@ static void hvf_remove_all_breakpoints(CPUState *cpu)
>       QTAILQ_FOREACH_SAFE(bp, &hvf_state->hvf_sw_breakpoints, entry, next) {
>           if (hvf_arch_remove_sw_breakpoint(cpu, bp) != 0) {
>               /* Try harder to find a CPU that currently sees the breakpoint. */
> -            CPU_FOREACH(tmpcpu)
> +            CPU_FOREACH_HVF(tmpcpu)
>               {
>                   if (hvf_arch_remove_sw_breakpoint(tmpcpu, bp) == 0) {
>                       break;
> @@ -572,7 +572,7 @@ static void hvf_remove_all_breakpoints(CPUState *cpu)
>       }
>       hvf_arch_remove_all_hw_breakpoints();
>   
> -    CPU_FOREACH(cpu) {
> +    CPU_FOREACH_HVF(cpu) {
>           hvf_update_guest_debug(cpu);
>       }
>   }
> @@ -581,6 +581,7 @@ static void hvf_accel_ops_class_init(ObjectClass *oc, void *data)
>   {
>       AccelOpsClass *ops = ACCEL_OPS_CLASS(oc);
>   
> +    ops->get_cpus_queue = hw_accel_get_cpus_queue;
>       ops->create_vcpu_thread = hvf_start_vcpu_thread;
>       ops->kick_vcpu_thread = hvf_kick_vcpu_thread;
>   
> diff --git a/target/arm/hvf/hvf.c b/target/arm/hvf/hvf.c
> index 0afd96018e0..13400ff0d5f 100644
> --- a/target/arm/hvf/hvf.c
> +++ b/target/arm/hvf/hvf.c
> @@ -2269,10 +2269,10 @@ static void hvf_arch_set_traps(void)
>   
>       /* Check whether guest debugging is enabled for at least one vCPU; if it
>        * is, enable exiting the guest on all vCPUs */
> -    CPU_FOREACH(cpu) {
> +    CPU_FOREACH_HVF(cpu) {
>           should_enable_traps |= cpu->accel->guest_debug_enabled;
>       }
> -    CPU_FOREACH(cpu) {
> +    CPU_FOREACH_HVF(cpu) {
>           /* Set whether debug exceptions exit the guest */
>           r = hv_vcpu_set_trap_debug_exceptions(cpu->accel->fd,
>                                                 should_enable_traps);