Re: [RFC v4 9/9] i386: split cpu accelerators from cpu.c

[Eduardo Habkost]

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 <cfontana@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.

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.

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
20201118220750.GP1509407@habkost.net">https://lore.kernel.org/qemu-devel/20201118220750.GP1509407@habkost.net

-- 
Eduardo