| Message ID | 20240809035134.699830-2-gshan@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/arm/virt: Improve virt_kvm_type() | expand |
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
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;
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(+)