On Fri, Nov 20, 2020 at 10:08:46AM +0100, Claudio Fontana wrote:
> On 11/19/20 8:23 PM, Eduardo Habkost wrote:
> > On Thu, Nov 19, 2020 at 09:53:09AM +0100, Claudio Fontana wrote:
> >> Hi,
> >>
> >> On 11/18/20 7:28 PM, Eduardo Habkost wrote:
> >>> On Wed, Nov 18, 2020 at 11:29:36AM +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 accel interface of the X86CPUClass is set at MODULE_INIT_ACCEL_CPU
> >>>> time, when the accelerator is known.
> >>>>
> >>>> Signed-off-by: Claudio Fontana <cfont...@suse.de>
> >>>> ---
> >>> [...]
> >>>> +/**
> >>>> + * X86CPUAccel:
> >>>> + * @name: string name of the X86 CPU Accelerator
> >>>> + *
> >>>> + * @common_class_init: initializer for the common cpu
> >>>
> >>> So this will be called for every single CPU class.
> >>
> >> Not really, it's called for every TYPE_X86_CPU cpu class (if an accel
> >> interface is registered).
> >
> > This means every single non-abstract CPU class in
> > qemu-system-x86_64, correct?
> >
> >>
> >> This function extends the existing x86_cpu_common_class_init
> >> (target/i386/cpu.c),
> >> where some methods of the base class CPUClass are set.
> >>
> >>>
> >>>> + * @instance_init: cpu instance initialization
> >>>> + * @realizefn: realize function, called first in x86 cpu realize
> >>>> + *
> >>>> + * X86 CPU accelerator-specific CPU initializations
> >>>> + */
> >>>> +
> >>>> +struct X86CPUAccel {
> >>>> + const char *name;
> >>>> +
> >>>> + void (*common_class_init)(X86CPUClass *xcc);
> >>>> + void (*instance_init)(X86CPU *cpu);
> >>>> + void (*realizefn)(X86CPU *cpu, Error **errp);
> >>>> };
> >>>>
> >>>> +void x86_cpu_accel_init(const X86CPUAccel *accel);
> >>> [...]
> >>>> +static void x86_cpu_accel_init_aux(ObjectClass *klass, void *opaque)
> >>>> +{
> >>>> + X86CPUClass *xcc = X86_CPU_CLASS(klass);
> >>>> + const X86CPUAccel **accel = opaque;
> >>>> +
> >>>> + xcc->accel = *accel;
> >>>> + xcc->accel->common_class_init(xcc);
> >>>> +}
> >>>> +
> >>>> +void x86_cpu_accel_init(const X86CPUAccel *accel)
> >>>> +{
> >>>> + object_class_foreach(x86_cpu_accel_init_aux, TYPE_X86_CPU, false,
> >>>> &accel);
> >>>> +}
> >>>
> >>> This matches the documented behavior.
> >>>
> >>> [...]
> >>>> +void host_cpu_class_init(X86CPUClass *xcc)
> >>>> +{
> >>>> + xcc->host_cpuid_required = true;
> >>>> + xcc->ordering = 8;
> >>>> + xcc->model_description =
> >>>> + g_strdup_printf("%s processor with all supported host features
> >>>> ",
> >>>> + xcc->accel->name);
> >>>> +}
> >>> [...]
> >>>> +static void hvf_cpu_common_class_init(X86CPUClass *xcc)
> >>>> +{
> >>>> + host_cpu_class_init(xcc);
> >>>
> >>> Why are you calling host_cpu_class_init() for all CPU types?
> >>
> >> I am not..
> >
> > I don't get it. You are calling host_cpu_class_init() for every
> > single non-abstract TYPE_X86_CPU subclass (which includes all CPU
> > models in qemu-system-x86_64), and I don't understand why, or if
> > this is really intentional.
>
> It is really intentional what is done here,
>
> when HVF accelerator is enabled, and only when the HVF accelerator is enabled,
>
> all X86 CPU classes and subclasses (cpu models, which have been
> implemented as subclasses of TYPE_X86_CPU), are updated with a
> link to the accelerator-specific HVF interface.
I'm confused. That (updating the class with a link) is not what
host_cpu_class_init() does.
This is what host_cpu_class_init() does:
void host_cpu_class_init(X86CPUClass *xcc)
{
xcc->host_cpuid_required = true;
xcc->ordering = 8;
xcc->model_description =
g_strdup_printf("%s processor with all supported host features ",
xcc->accel->name);
}
Why do you want to call host_cpu_class_init() for every non-abstract
TYPE_X86_CPU subclass?
--
Eduardo
