Re: [PATCH-for-9.0 08/16] target/arm/kvm: Have kvm_arm_pmu_init take a ARMCPU argument

[Gavin Shan <gshan@xxxxxxxxxx>]

Hi Phil,

On 11/24/23 05:35, Philippe Mathieu-Daudé wrote:
> Unify the "kvm_arm.h" API: All functions related to ARM vCPUs
> take a ARMCPU* argument. Use the CPU() QOM cast macro When
> calling the generic vCPU API from "sysemu/kvm.h".
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
> ---
>   target/arm/kvm_arm.h | 4 ++--
>   hw/arm/virt.c        | 2 +-
>   target/arm/kvm.c     | 6 +++---
>   3 files changed, 6 insertions(+), 6 deletions(-)

One nit below, but I guess it doesn't matter.

Reviewed-by: Gavin Shan <gshan@xxxxxxxxxx>

> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index 0e12a008ab..fde1c45609 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -200,8 +200,8 @@ int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa);
>   
>   int kvm_arm_vgic_probe(void);
>   
> +void kvm_arm_pmu_init(ARMCPU *cpu);
>   void kvm_arm_pmu_set_irq(CPUState *cs, int irq);
> -void kvm_arm_pmu_init(CPUState *cs);
>   

Why the order of the declaration is changed? I guess the reason would be
kvm_arm_pmu_init() is called prior to kvm_arm_pmu_set_irq().

>   /**
>    * kvm_arm_pvtime_init:
> @@ -263,7 +263,7 @@ static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
>       g_assert_not_reached();
>   }
>   
> -static inline void kvm_arm_pmu_init(CPUState *cs)
> +static inline void kvm_arm_pmu_init(ARMCPU *cpu)
>   {
>       g_assert_not_reached();
>   }
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index b6efe9da4d..63f3c0b750 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2000,7 +2000,7 @@ static void virt_cpu_post_init(VirtMachineState *vms, MemoryRegion *sysmem)
>                   if (kvm_irqchip_in_kernel()) {
>                       kvm_arm_pmu_set_irq(cpu, VIRTUAL_PMU_IRQ);
>                   }
> -                kvm_arm_pmu_init(cpu);
> +                kvm_arm_pmu_init(ARM_CPU(cpu));
>               }
>               if (steal_time) {
>                   kvm_arm_pvtime_init(ARM_CPU(cpu), pvtime_reg_base
> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 82c5924ab5..e7cbe1ff05 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -1711,17 +1711,17 @@ static bool kvm_arm_set_device_attr(ARMCPU *cpu, struct kvm_device_attr *attr,
>       return true;
>   }
>   
> -void kvm_arm_pmu_init(CPUState *cs)
> +void kvm_arm_pmu_init(ARMCPU *cpu)
>   {
>       struct kvm_device_attr attr = {
>           .group = KVM_ARM_VCPU_PMU_V3_CTRL,
>           .attr = KVM_ARM_VCPU_PMU_V3_INIT,
>       };
>   
> -    if (!ARM_CPU(cs)->has_pmu) {
> +    if (!cpu->has_pmu) {
>           return;
>       }
> -    if (!kvm_arm_set_device_attr(ARM_CPU(cs), &attr, "PMU")) {
> +    if (!kvm_arm_set_device_attr(cpu, &attr, "PMU")) {
>           error_report("failed to init PMU");
>           abort();
>       }

Thanks,
Gavin