Re: [PATCH 1/4] hw/arm/virt: hide virt_kvm_type() on !CONFIG_KVM

[Gavin Shan <gshan@redhat.com>]

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