Re: [Qemu-devel] [PATCH 15/37] target-i386: set default value of "hypervisor" feature using static property

[Eduardo Habkost]

On 22/10/2012, at 17:03, Igor Mammedov <address@hidden> wrote:

> Signed-off-by: Igor Mammedov <address@hidden>
> ---
> target-i386/cpu.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3131945..dc4fcdf 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -174,7 +174,7 @@ static Property cpu_x86_properties[] = {
>     DEFINE_PROP_BIT("f-xsave", X86CPU, env.cpuid_ext_features, 26, false),
>     DEFINE_PROP_BIT("f-osxsave", X86CPU, env.cpuid_ext_features, 27, false),
>     DEFINE_PROP_BIT("f-avx", X86CPU, env.cpuid_ext_features, 28, false),
> -    DEFINE_PROP_BIT("f-hypervisor", X86CPU, env.cpuid_ext_features, 31, 
> false),
> +    DEFINE_PROP_BIT("f-hypervisor", X86CPU, env.cpuid_ext_features, 31, 
> true),
>     DEFINE_PROP_BIT("f-syscall", X86CPU, env.cpuid_ext2_features, 11, false),
>     DEFINE_PROP_BIT("f-nx", X86CPU, env.cpuid_ext2_features, 20, false),
>     DEFINE_PROP_BIT("f-xd", X86CPU, env.cpuid_ext2_features, 20, false),
> @@ -1307,11 +1307,12 @@ static int cpu_x86_find_by_name(X86CPU *cpu, 
> x86_def_t *x86_cpu_def,
> {
>     unsigned int i;
>     x86_def_t *def;
> +    CPUX86State *env = &cpu->env;
> 
>     char *s = g_strdup(cpu_model);
>     char *featurestr, *name = strtok(s, ",");
>     /* Features to be added*/
> -    uint32_t plus_features = 0, plus_ext_features = 0;
> +    uint32_t plus_features = 0, plus_ext_features = env->cpuid_ext_features;

Moving data back and forth between CPUX86State and x86_def_t makes the 
initialization ordering confusing (today data is moved from x86_def_t to 
X86CPU, and never the other way around).

As this code is removed in the next patches, I don't mind too much, but maybe 
it's simpler to implement this change only after the "use static properties for 
setting cpuid features" patch?

>     uint32_t plus_ext2_features = 0, plus_ext3_features = 0;
>     uint32_t plus_kvm_features = 0, plus_svm_features = 0;
>     uint32_t plus_7_0_ebx_features = 0;
> @@ -1345,10 +1346,6 @@ static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t 
> *x86_cpu_def,
>     plus_kvm_features = 0;
> #endif
> 
> -    add_flagname_to_bitmaps("hypervisor", &plus_features,
> -            &plus_ext_features, &plus_ext2_features, &plus_ext3_features,
> -            &plus_kvm_features, &plus_svm_features,  &plus_7_0_ebx_features);
> -
>     featurestr = strtok(NULL, ",");
> 
>     while (featurestr) {
> -- 
> 1.7.11.7
> 
> 

-- 
Eduardo