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

[Eduardo Habkost]

On 22/10/2012, at 17:03, Igor Mammedov <address@hidden> 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.
> 
> 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;

This is where things gets interesting: now the PV EOI feature is enabled by 
default, but only on the pc-1.3 machine-type.

Maybe at this point in the series we are already able to use global properties 
for compatibility, so we can make the changes above, add "f-kvm_pv_eoi=false" 
to pc-1.2, and kill kvm_default_features and enable_kvm_pv_eoi(), all in the 
same patch?


>     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) {
> -- 
> 1.7.11.7
> 
> 

-- 
Eduardo