Re: [PATCH qom-cpu-next v3 3/4] target-i386: Slim conversion to X86CPU subclasses

[Igor Mammedov <imammedo@redhat.com>]

On Mon, 4 Feb 2013 17:05:01 +0100
Igor Mammedov <imammedo@redhat.com> wrote:

> On Mon, 04 Feb 2013 13:52:32 +0100
> Andreas Färber <afaerber@suse.de> wrote:
> 
> > Am 04.02.2013 12:08, schrieb Igor Mammedov:
> > > On Sat,  2 Feb 2013 01:37:07 +0100
> > > Andreas Färber <afaerber@suse.de> wrote:
> > > 
[...]
> 
> > >> @@ -2198,6 +2018,8 @@ static void x86_cpu_initfn(Object *obj)
> > >>      CPUState *cs = CPU(obj);
> > >>      X86CPU *cpu = X86_CPU(obj);
> > >>      CPUX86State *env = &cpu->env;
> > >> +    X86CPUClass *xcc = X86_CPU_GET_CLASS(obj);
> > >> +    const x86_def_t *def = &xcc->info;
> > >>      static int inited;
> > >>  
> > >>      cpu_exec_init(env);
> > >> @@ -2227,6 +2049,41 @@ static void x86_cpu_initfn(Object *obj)
> > >>                          x86_cpuid_get_tsc_freq,
> > >>                          x86_cpuid_set_tsc_freq, NULL, NULL, NULL);
> > >>  
> > >> +    /* sysenter isn't supported in compatibility mode on AMD,
> > >> +     * syscall isn't supported in compatibility mode on Intel.
> > >> +     * Normally we advertise the actual CPU vendor, but you can
> > >> +     * override this using the 'vendor' property if you want to use
> > >> +     * KVM's sysenter/syscall emulation in compatibility mode and
> > >> +     * when doing cross vendor migration
> > >> +     */
> > >> +    if (kvm_enabled()) {
> > >> +        host_cpuid(0, 0, NULL, &env->cpuid_vendor1, &env->cpuid_vendor2,
> > >> +                   &env->cpuid_vendor3);
> > > This is not per instance specific, this override would be better done to
> > > sub-classes in kvm_arch_init().
> > 
> > Hmm... I think the issue I addresses this way was that kvm.c didn't have
> > access to your static vendor_words2str() helper. Writing the words into
> > the word fields is more direct than converting them to a string and
> > writing them back into the words.
> you have these correct vendor words ready after you called
> kvm_host_cpu_class_init() in kvm_arch_init(), so just direct copy from host
> class would do.
I was talking nonsense here.
Why not to make vendor_words2str() helper not static and use it in kvm.c
instead of doing partial init of host class and then reuse the value you got
to override vendor in other classes?

[...]

-- 
Regards,
  Igor