Re: [PATCH 3/4] hw/arm/virt: Use kvm_arch_get_default_type()

[Gavin Shan <gshan@redhat.com>]

On 8/9/24 1:51 PM, Gavin Shan wrote:
> kvm_arch_get_default_type() and kvm_arm_get_max_vm_ipa_size() are
> interchangeable since the type is equivalent to IPA size (bits)
> with one exception that IPA size (bits) is 40 when the type is zero.
> 
> Replace kvm_arm_get_max_vm_ipa_size() with kvm_arch_get_default_type().
> After this, kvm_arm_get_max_vm_ipa_size() needn't to be a public API
> any more.
> 
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>   hw/arm/virt.c        | 14 ++++++--------
>   target/arm/kvm.c     |  2 +-
>   target/arm/kvm_arm.h | 15 ---------------
>   3 files changed, 7 insertions(+), 24 deletions(-)
> 
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 09b7a158a9..f35857aa95 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2995,10 +2995,12 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>   static int virt_kvm_type(MachineState *ms, const char *type_str)
>   {
>       VirtMachineState *vms = VIRT_MACHINE(ms);
> -    int max_vm_pa_size, requested_pa_size;
> +    int type, max_vm_pa_size, requested_pa_size;
>       bool fixed_ipa;
>   
> -    max_vm_pa_size = kvm_arm_get_max_vm_ipa_size(ms, &fixed_ipa);
> +    /* The IPA size is 40 bits when the type is zero */
> +    type = kvm_arch_get_default_type(ms);
> +    max_vm_pa_size = (type == 0) ? 40 : type;
>   
>       /* we freeze the memory map to compute the highest gpa */
>       virt_set_memmap(vms, max_vm_pa_size);
> @@ -3017,12 +3019,8 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
>                        requested_pa_size, max_vm_pa_size);
>           return -1;
>       }
> -    /*
> -     * We return the requested PA log size, unless KVM only supports
> -     * the implicit legacy 40b IPA setting, in which case the kvm_type
> -     * must be 0.
> -     */
> -    return fixed_ipa ? 0 : requested_pa_size;
> +
> +    return type;
>   }
>   #endif /* CONFIG_KVM */
>   

This actually needs to be something like below. It's incorrect to ignore
@requested_pa_size here.

         return (type == 0) ? 0 : requested_pa_size;

> diff --git a/target/arm/kvm.c b/target/arm/kvm.c
> index 849e2e21b3..65893c9c12 100644
> --- a/target/arm/kvm.c
> +++ b/target/arm/kvm.c
> @@ -526,7 +526,7 @@ bool kvm_arm_pmu_supported(void)
>       return kvm_check_extension(kvm_state, KVM_CAP_ARM_PMU_V3);
>   }
>   
> -int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
> +static int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
>   {
>       KVMState *s = KVM_STATE(ms->accelerator);
>       int ret;
> diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h
> index cfaa0d9bc7..fd919d4738 100644
> --- a/target/arm/kvm_arm.h
> +++ b/target/arm/kvm_arm.h
> @@ -188,16 +188,6 @@ bool kvm_arm_pmu_supported(void);
>    */
>   bool kvm_arm_sve_supported(void);
>   
> -/**
> - * kvm_arm_get_max_vm_ipa_size:
> - * @ms: Machine state handle
> - * @fixed_ipa: True when the IPA limit is fixed at 40. This is the case
> - * for legacy KVM.
> - *
> - * Returns the number of bits in the IPA address space supported by KVM
> - */
> -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);
> @@ -248,11 +238,6 @@ static inline void kvm_arm_add_vcpu_properties(ARMCPU *cpu)
>       g_assert_not_reached();
>   }
>   
> -static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms, bool *fixed_ipa)
> -{
> -    g_assert_not_reached();
> -}
> -
>   static inline int kvm_arm_vgic_probe(void)
>   {
>       g_assert_not_reached();

Thanks,
Gavin