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

[Gavin Shan]

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