On Wed, Jan 02, 2013 at 08:00:26PM +0100, Igor Mammedov wrote:
> On Fri, 28 Dec 2012 18:34:04 -0200
> Eduardo Habkost <ehabk...@redhat.com> wrote:
>
> > Note that we are initializing the CPU features inside instance_init (and
> > not storing any CPU feature information inside the class struct) because
> > kvm_cpu_fill_host() needs KVM to be initialized, and we can't guarantee
> > that KVM will be initialized when class_init is called.
> initializing defaults in initfn will be broken after we convert features into
> static properties due to all initfn()s are called before static properties
> defaults are set.
You are right because of the plan to use global properties. I didn't
care about property defaults because the -cpu host class wouldn't have
introspectable defaults, but global properties need to work properly so
"-cpu host,+foobar,enforce" keep working.
>
> Is it possible to initialize kvm first before calling class_init().
Summarizing what was discussed on IRC:
- class_init is too early, because it may be called before we even know
what's the accel configuration requested by the user (and I believe
this is by design, as we want to allow a client to list all available
classes and their available properties before configuring anything).
So we can't call kvm_arch_get_supported_cpuid() on class_init.
- CPU instance_init is too late because global properties are set
inside device_initfn(), before the CPU class instance_init function
is called.
So we need something to be called after kvm_init() is called, but before
the CPU objects are created. We already have that: kvm_init() itself. :-)
So I will try to send a new RFC that has a function like:
void kvm_finish_cpu_host_class_init(KVMState *s, X86CPUClass *cc);
and kvm_init() would then call:
kvm_finish_cpu_host_class_init(kvm_state,
X86_CPU_CLASS(object_class_by_name(CPU_CLASS_NAME("host"))));
We considered other more complex approaches (like making the CPU feature
properties tristate [on/off/host] instead of boolean), but that would be
simply a further improvement, maybe to be proposed as part of the x86
CPU properties work.
>
> >
> > Signed-off-by: Eduardo Habkost <ehabk...@redhat.com>
> > ---
> > target-i386/cpu.c | 45 ++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 42 insertions(+), 3 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index c824c08..2b6cc3b 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -330,6 +330,14 @@ typedef struct x86_def_t {
> > #define TCG_SVM_FEATURES 0
> > #define TCG_7_0_EBX_FEATURES (CPUID_7_0_EBX_SMEP | CPUID_7_0_EBX_SMAP)
> >
> > +
> > +/* CPU class name definitions: */
> > +
> > +#define CPU_CLASS_NAME(name) (name "-" TYPE_X86_CPU)
> > +
> > +#define TYPE_X86_HOST_CPU CPU_CLASS_NAME("host")
> > +
> > +
> > /* maintains list of cpu model definitions
> > */
> > static x86_def_t *x86_defs = {NULL};
> > @@ -1221,9 +1229,7 @@ static X86CPU *x86_cpu_create_from_name(const char
> > *name, Error **errp)
> >
> > if (kvm_enabled() && name && strcmp(name, "host") == 0) {
> > #ifdef CONFIG_KVM
> > - cpu = X86_CPU(object_new(TYPE_X86_CPU));
> > - kvm_cpu_fill_host(x86_cpu_def);
> > - cpudef_2_x86_cpu(cpu, x86_cpu_def, &error);
> > + cpu = X86_CPU(object_new(TYPE_X86_HOST_CPU));
> > #endif
> > } else {
> > x86_def_t *def;
> > @@ -2168,9 +2174,42 @@ static const TypeInfo x86_cpu_type_info = {
> > .class_init = x86_cpu_common_class_init,
> > };
> >
> > +#ifdef CONFIG_KVM
> > +
> > +static void x86_host_cpu_initfn(Object *obj)
> > +{
> > + X86CPU *cpu = X86_CPU(obj);
> > + Error *err = NULL;
> > + x86_def_t cpudef;
> > +
> > + memset(&cpudef, 0, sizeof(cpudef));
> > + kvm_cpu_fill_host(&cpudef);
> > + cpudef_2_x86_cpu(cpu, &cpudef, &err);
> > +
> > + if (err) {
> > + error_report("unexpected cpu init error: %s",
> > error_get_pretty(err));
> > + exit(1);
> > + }
> > +}
> > +
> > +static const TypeInfo x86_host_cpu_type_info = {
> > + .name = TYPE_X86_HOST_CPU,
> > + .parent = TYPE_X86_CPU,
> > + .instance_size = sizeof(X86CPU),
> > + .instance_init = x86_host_cpu_initfn,
> > + .abstract = false,
> > + .class_size = sizeof(X86CPUClass),
> > +};
> > +
> > +#endif /* CONFIG_KVM */
> > +
> > +
> > static void x86_cpu_register_types(void)
> > {
> > type_register_static(&x86_cpu_type_info);
> > +#ifdef CONFIG_KVM
> > + type_register_static(&x86_host_cpu_type_info);
> > +#endif
> > }
> >
> > type_init(x86_cpu_register_types)
> > --
> > 1.7.11.7
> >
>
>
> --
> Regards,
> Igor
--
Eduardo
