On Wed, Aug 12, 2020 at 12:16:43PM +0200, Auger Eric wrote: > Hi Drew, > > On 8/5/20 11:16 AM, Andrew Jones wrote: > > When we compile without KVM support !defined(CONFIG_KVM) we generate > > stubs for functions that the linker will still encounter. Sometimes > > these stubs can be executed safely and are placed in paths where they > > get executed with or without KVM. Other functions should never be > > called without KVM. Those functions should be guarded by kvm_enabled(), > > but should also be robust to refactoring mistakes. Putting a > > g_assert_not_reached() in the function should help. Additionally, > > the g_assert_not_reached() calls may actually help the linker remove > > some code. > > > > We remove the stubs for kvm_arm_get/put_virtual_time(), as they aren't > > necessary at all - the only caller is in kvm.c > > > > Signed-off-by: Andrew Jones <drjo...@redhat.com> > > --- > > target/arm/kvm_arm.h | 44 +++++++++++++++++++++++++++----------------- > > 1 file changed, 27 insertions(+), 17 deletions(-) > > > > diff --git a/target/arm/kvm_arm.h b/target/arm/kvm_arm.h > > index adb38514bf20..0da00eb6b20c 100644 > > --- a/target/arm/kvm_arm.h > > +++ b/target/arm/kvm_arm.h > > @@ -344,16 +344,10 @@ int kvm_arm_set_irq(int cpu, int irqtype, int irq, > > int level); > > > > #else > > > > -static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) > > -{ > > - /* > > - * This should never actually be called in the "not KVM" case, > > - * but set up the fields to indicate an error anyway. > > - */ > > - cpu->kvm_target = QEMU_KVM_ARM_TARGET_NONE; > > - cpu->host_cpu_probe_failed = true; > > -} > > - > > +/* > > + * It's safe to call these functions without KVM support. > > + * They should either do nothing or return "not supported". > > + */ > > static inline void kvm_arm_add_vcpu_properties(Object *obj) {} > this one also is guarded by kvm_enabled() in target/arm/cpu.c > don't you want to add g_assert_not_reached()?
Yes, that would make sense. I'll do that for v3. > > > > static inline bool kvm_arm_aarch32_supported(void) > > @@ -371,23 +365,39 @@ static inline bool kvm_arm_sve_supported(void) > > return false; > > } > > > > +/* > > + * These functions should never actually be called without KVM support. > > + */ > > +static inline void kvm_arm_set_cpu_features_from_host(ARMCPU *cpu) > > +{ > > + g_assert_not_reached(); > > +} > > + > > static inline int kvm_arm_get_max_vm_ipa_size(MachineState *ms) > > { > > - return -ENOENT; > > + g_assert_not_reached(); > > } > > > > static inline int kvm_arm_vgic_probe(void) > > { > > - return 0; > > + g_assert_not_reached(); > > } > > > > -static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) {} > > -static inline void kvm_arm_pmu_init(CPUState *cs) {} > > +static inline void kvm_arm_pmu_set_irq(CPUState *cs, int irq) > > +{ > > + g_assert_not_reached(); > > +} > > > > -static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) {} > > +static inline void kvm_arm_pmu_init(CPUState *cs) > > +{ > > + g_assert_not_reached(); > > +} > > + > > +static inline void kvm_arm_sve_get_vls(CPUState *cs, unsigned long *map) > > +{ > > + g_assert_not_reached(); > > +} > > > > -static inline void kvm_arm_get_virtual_time(CPUState *cs) {} > > -static inline void kvm_arm_put_virtual_time(CPUState *cs) {} > > #endif > > > > static inline const char *gic_class_name(void) > > > > Besides, > Reviewed-by: Eric Auger <eric.au...@redhat.com> Thanks, drew