virt_kvm_type() and mc->kvm_type() are only needed when CONFIG_KVM
is enabled. It's reasonable to hide them when CONFIG_KVM is disabled.

Signed-off-by: Gavin Shan <gshan@redhat.com>
---
 hw/arm/virt.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/hw/arm/virt.c b/hw/arm/virt.c
index 719e83e6a1..83be57db37 100644
--- a/hw/arm/virt.c
+++ b/hw/arm/virt.c
@@ -2991,6 +2991,7 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
  * for arm64 kvm_type [7-0] encodes the requested number of bits
  * in the IPA address space
  */
+#ifdef CONFIG_KVM
 static int virt_kvm_type(MachineState *ms, const char *type_str)
 {
     VirtMachineState *vms = VIRT_MACHINE(ms);
@@ -3025,6 +3026,7 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
      */
     return fixed_ipa ? 0 : requested_pa_size;
 }
+#endif /* CONFIG_KVM */
 
 static void virt_machine_class_init(ObjectClass *oc, void *data)
 {
@@ -3084,7 +3086,9 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
 #endif
     mc->valid_cpu_types = valid_cpu_types;
     mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
+#ifdef CONFIG_KVM
     mc->kvm_type = virt_kvm_type;
+#endif
     assert(!mc->get_hotplug_handler);
     mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
     hc->pre_plug = virt_machine_device_pre_plug_cb;
-- 
2.45.2

On Fri, 9 Aug 2024 at 04:52, Gavin Shan <gshan@redhat.com> wrote:
>
> virt_kvm_type() and mc->kvm_type() are only needed when CONFIG_KVM
> is enabled. It's reasonable to hide them when CONFIG_KVM is disabled.
>
> Signed-off-by: Gavin Shan <gshan@redhat.com>
> ---
>  hw/arm/virt.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 719e83e6a1..83be57db37 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -2991,6 +2991,7 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>   * for arm64 kvm_type [7-0] encodes the requested number of bits
>   * in the IPA address space
>   */
> +#ifdef CONFIG_KVM
>  static int virt_kvm_type(MachineState *ms, const char *type_str)
>  {
>      VirtMachineState *vms = VIRT_MACHINE(ms);
> @@ -3025,6 +3026,7 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
>       */
>      return fixed_ipa ? 0 : requested_pa_size;
>  }
> +#endif /* CONFIG_KVM */
>
>  static void virt_machine_class_init(ObjectClass *oc, void *data)
>  {
> @@ -3084,7 +3086,9 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>  #endif
>      mc->valid_cpu_types = valid_cpu_types;
>      mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
> +#ifdef CONFIG_KVM
>      mc->kvm_type = virt_kvm_type;
> +#endif
>      assert(!mc->get_hotplug_handler);
>      mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
>      hc->pre_plug = virt_machine_device_pre_plug_cb;

This adds extra ifdefs to the source code -- is there any
reason why we need them? If nothing goes wrong, I'd
prefer not to clutter the source with thme.

thanks
-- PMM

On 8/9/24 7:00 PM, Peter Maydell wrote:
> On Fri, 9 Aug 2024 at 04:52, Gavin Shan <gshan@redhat.com> wrote:
>>
>> virt_kvm_type() and mc->kvm_type() are only needed when CONFIG_KVM
>> is enabled. It's reasonable to hide them when CONFIG_KVM is disabled.
>>
>> Signed-off-by: Gavin Shan <gshan@redhat.com>
>> ---
>>   hw/arm/virt.c | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
>> index 719e83e6a1..83be57db37 100644
>> --- a/hw/arm/virt.c
>> +++ b/hw/arm/virt.c
>> @@ -2991,6 +2991,7 @@ static HotplugHandler *virt_machine_get_hotplug_handler(MachineState *machine,
>>    * for arm64 kvm_type [7-0] encodes the requested number of bits
>>    * in the IPA address space
>>    */
>> +#ifdef CONFIG_KVM
>>   static int virt_kvm_type(MachineState *ms, const char *type_str)
>>   {
>>       VirtMachineState *vms = VIRT_MACHINE(ms);
>> @@ -3025,6 +3026,7 @@ static int virt_kvm_type(MachineState *ms, const char *type_str)
>>        */
>>       return fixed_ipa ? 0 : requested_pa_size;
>>   }
>> +#endif /* CONFIG_KVM */
>>
>>   static void virt_machine_class_init(ObjectClass *oc, void *data)
>>   {
>> @@ -3084,7 +3086,9 @@ static void virt_machine_class_init(ObjectClass *oc, void *data)
>>   #endif
>>       mc->valid_cpu_types = valid_cpu_types;
>>       mc->get_default_cpu_node_id = virt_get_default_cpu_node_id;
>> +#ifdef CONFIG_KVM
>>       mc->kvm_type = virt_kvm_type;
>> +#endif
>>       assert(!mc->get_hotplug_handler);
>>       mc->get_hotplug_handler = virt_machine_get_hotplug_handler;
>>       hc->pre_plug = virt_machine_device_pre_plug_cb;
> 
> This adds extra ifdefs to the source code -- is there any
> reason why we need them? If nothing goes wrong, I'd
> prefer not to clutter the source with thme.
> 

There is nothing wrong in current code. Actually, it's preparatory work
to replace kvm_arm_get_max_vm_ipa_size() with kvm_arch_get_default_type()
in mc->kvm_type() in PATCH[3/4]. kvm_arch_get_default_type() is visible
only when CONFIG_KVM is enabled.

Thanks,
Gavin