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 */
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();
--
2.45.2On 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
On Fri, 9 Aug 2024 at 04:52, Gavin Shan <gshan@redhat.com> 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. Well, sort of, but they're conceptually different. kvm_arch_get_default_type() is the API for "give me the value I need to pass to kvm_ioctl(s, KVM_CREATE_VM, type)"; it's architecture independent and different architectures do different things. In the future Arm might need to do something other than "just pass in the IPA size". kvm_arm_get_max_vm_ipa_size() does exactly what it says on the tin: it is an Arm specific function that returns the maximum supported IPA size. I would prefer not to conflate the two. thanks -- PMM
On 8/9/24 6:59 PM, Peter Maydell wrote: > On Fri, 9 Aug 2024 at 04:52, Gavin Shan <gshan@redhat.com> 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. > > Well, sort of, but they're conceptually different. > > kvm_arch_get_default_type() is the API for "give me the value > I need to pass to kvm_ioctl(s, KVM_CREATE_VM, type)"; it's > architecture independent and different architectures do > different things. In the future Arm might need to do something > other than "just pass in the IPA size". > > kvm_arm_get_max_vm_ipa_size() does exactly what it says on the > tin: it is an Arm specific function that returns the maximum > supported IPA size. > > I would prefer not to conflate the two. > Agreed. Thanks for the explanation. Lets keep them separate and ignore this series. By the way, mc->kvm_type() can be used if one machine (platform) needs to do more things than kvm_arch_get_default_type(), similar to what we're doing on 'virt' machine currently. Thanks, Gavin
© 2016 - 2024 Red Hat, Inc.