On 11/20/20 6:44 PM, Eduardo Habkost wrote: > On Fri, Nov 20, 2020 at 03:49:09PM +0100, Claudio Fontana wrote: >> split cpu.c into: >> >> cpu.c cpuid and common x86 cpu functionality >> host-cpu.c host x86 cpu functions and "host" cpu type >> kvm/cpu.c KVM x86 cpu type >> hvf/cpu.c HVF x86 cpu type >> tcg/cpu.c TCG x86 cpu type >> >> The link to the accel class is set in the X86CPUClass classes >> at MODULE_INIT_ACCEL_CPU time, when the accelerator is known. >> >> Signed-off-by: Claudio Fontana <cfont...@suse.de> > [...] >> +static void hvf_cpu_accel_class_init(ObjectClass *oc, void *data) >> +{ >> + X86CPUAccelClass *acc = X86_CPU_ACCEL_CLASS(oc); >> + >> + acc->cpu_realizefn = host_cpu_realizefn; >> + acc->cpu_common_class_init = hvf_cpu_common_class_init; >> + acc->cpu_instance_init = hvf_cpu_instance_init; >> +}; >> +static const TypeInfo hvf_cpu_accel_type_info = { >> + .name = X86_CPU_ACCEL_TYPE_NAME("hvf"), >> + >> + .parent = TYPE_X86_CPU_ACCEL, >> + .class_init = hvf_cpu_accel_class_init, >> +}; >> +static void hvf_cpu_accel_register_types(void) >> +{ >> + type_register_static(&hvf_cpu_accel_type_info); >> +} >> +type_init(hvf_cpu_accel_register_types); >> + >> +static void hvf_cpu_accel_init(void) >> +{ >> + if (hvf_enabled()) { >> + x86_cpu_accel_init(X86_CPU_ACCEL_TYPE_NAME("hvf")); >> + } >> +} >> + >> +accel_cpu_init(hvf_cpu_accel_init); > > The point of my suggestion of using QOM is to not require > separate accel_cpu_init() functions and (hvf|tcg|kvm)_enabled() > checks. > > If we still have separate accel_cpu_init() functions for calling > x86_cpu_accel_init() with the right argument, using a pointer to > static variables like &hvf_cpu_accel (like you did before) was > simpler and required less boilerplate code.
Yes I agree. > > However, the difference is that with the X86_CPU_ACCEL_TYPE_NAME > macro + object_class_by_name(), you don't need the separate > accel_cpu_init() functions for each accelerator. > > All you need is a single: > > x86_cpu_accel_init(X86_CPU_ACCEL_TYPE_NAME(chosen_accel_name)); > > call somewhere in the initialization path. Makes sense. The problem is just determining chosen_accel_name. > > A good place for the x86_cpu_accel_init() call would be > do_configure_accelerator(), but the function is arch-specific. > That's why I suggested a cpu_accel_arch_init() function at > https://lore.kernel.org/qemu-devel/20201118220750.gp1509...@habkost.net > Fine by me. I'd use a specific init step for this, but that also works. Ciao, Clauidio