Re: [Qemu-devel] [PATCH 16/37] target-i386: set kvm CPUID default feature values using static properties

[Igor Mammedov]

On Mon, 22 Oct 2012 19:20:10 -0400
Don Slutz <address@hidden> wrote:

> On 10/22/12 11:03, Igor Mammedov wrote:
> > Replace setting default supported kvm features in cpu_x86_find_by_name()
> > by default values in corresponding static properties.
> >
> > - Compile in kvm CPUID features only if CONFIG_KVM is defined.
> > - Make "f-kvm_steal_tm" and "f-kvmclock_stable" CPUID features visible as 
> > properties.
> Not sure that KVM_FEATURE_STEAL_TIME should translate to kvm_steal_tm.  
> I think that I have seen that new names should use "-".  So I would 
> think the new name should be "f-kvm-steal-time". And 
> KVM_FEATURE_CLOCKSOURCE_STABLE_BIT should be "f-kvm-clock-stable".
That would be inconsistent with current naming scheme, I may be it should be
f-kvm_steal_time and f-kvm_clock_stable for this series and than later all
the features converted in a single batch, its a bit out of scope of this
patch. Alternatively a separate patch could be send to convert current
feature name arrays to '-' scheme. I'll happily rebase series on top of it. 

> >
> > Signed-off-by: Igor Mammedov <address@hidden>
> > ---
> >   target-i386/cpu.c | 30 +++++++++++-------------------
> >   1 file changed, 11 insertions(+), 19 deletions(-)
> >
> > diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> > index dc4fcdf..407c5ce 100644
> > --- a/target-i386/cpu.c
> > +++ b/target-i386/cpu.c
> > @@ -200,12 +200,16 @@ static Property cpu_x86_properties[] = {
> >       DEFINE_PROP_BIT("f-fma4", X86CPU, env.cpuid_ext3_features, 16, false),
> >       DEFINE_PROP_BIT("f-cvt16", X86CPU, env.cpuid_ext3_features, 18, 
> > false),
> >       DEFINE_PROP_BIT("f-nodeid_msr", X86CPU, env.cpuid_ext3_features, 19, 
> > false),
> > -    DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features,  0, 
> > false),
> > -    DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features,  
> > 1, false),
> > -    DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features,  2, 
> > false),
> > -    DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features,  3, 
> > false),
> > -    DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features,  4, 
> > false),
> > -    DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features,  6, 
> > false),
> > +#if defined(CONFIG_KVM)
> > +    DEFINE_PROP_BIT("f-kvmclock", X86CPU, env.cpuid_kvm_features,  0, 
> > true),
> > +    DEFINE_PROP_BIT("f-kvm_nopiodelay", X86CPU, env.cpuid_kvm_features,  
> > 1, true),
> > +    DEFINE_PROP_BIT("f-kvm_mmu", X86CPU, env.cpuid_kvm_features,  2, true),
> > +    DEFINE_PROP_BIT("f-kvmclock2", X86CPU, env.cpuid_kvm_features,  3, 
> > true),
> > +    DEFINE_PROP_BIT("f-kvm_asyncpf", X86CPU, env.cpuid_kvm_features,  4, 
> > true),
> > +    DEFINE_PROP_BIT("f-kvm_steal_tm", X86CPU, env.cpuid_kvm_features,  5, 
> > true),
> > +    DEFINE_PROP_BIT("f-kvm_pv_eoi", X86CPU, env.cpuid_kvm_features,  6, 
> > true),
> > +    DEFINE_PROP_BIT("f-kvmclock_stable", X86CPU, env.cpuid_kvm_features,  
> > 24, true),
> > +#endif
> >       DEFINE_PROP_BIT("f-npt", X86CPU, env.cpuid_svm_features,  0, false),
> >       DEFINE_PROP_BIT("f-lbrv", X86CPU, env.cpuid_svm_features,  1, false),
> >       DEFINE_PROP_BIT("f-svm_lock", X86CPU, env.cpuid_svm_features,  2, 
> > false),
> > @@ -1314,7 +1318,7 @@ static int cpu_x86_find_by_name(X86CPU *cpu, 
> > x86_def_t *x86_cpu_def,
> >       /* Features to be added*/
> >       uint32_t plus_features = 0, plus_ext_features = 
> > env->cpuid_ext_features;
> >       uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
> > -    uint32_t plus_kvm_features = 0, plus_svm_features = 0;
> > +    uint32_t plus_kvm_features = env->cpuid_kvm_features, 
> > plus_svm_features = 0;
> >       uint32_t plus_7_0_ebx_features = 0;
> >       /* Features to be removed */
> >       uint32_t minus_features = 0, minus_ext_features = 0;
> > @@ -1334,18 +1338,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, 
> > x86_def_t *x86_cpu_def,
> >           memcpy(x86_cpu_def, def, sizeof(*def));
> >       }
> >   
> > -#if defined(CONFIG_KVM)
> > -    plus_kvm_features = (1 << KVM_FEATURE_CLOCKSOURCE) |
> > -        (1 << KVM_FEATURE_NOP_IO_DELAY) |
> > -        (1 << KVM_FEATURE_MMU_OP) |
> > -        (1 << KVM_FEATURE_CLOCKSOURCE2) |
> > -        (1 << KVM_FEATURE_ASYNC_PF) |
> > -        (1 << KVM_FEATURE_STEAL_TIME) |
> > -        (1 << KVM_FEATURE_CLOCKSOURCE_STABLE_BIT);
> > -#else
> > -    plus_kvm_features = 0;
> > -#endif
> > -
> >       featurestr = strtok(NULL, ",");
> >   
> >       while (featurestr) {
>    -Don Slutz
> 


-- 
Regards,
  Igor