Re: [Qemu-devel] [PATCH qom-cpu 4/9] target-i386: Replace cpuid_*features fields with a feature word array

["Andreas Färber" <afaerber@suse.de>]

Am 22.04.2013 21:00, schrieb Eduardo Habkost:
> This replaces the feature-bit fields on both X86CPU and x86_def_t
> structs with an array.
> 
> With this, we will be able to simplify code that simply does the same
> operation on all feature words (e.g. kvm_check_features_against_host(),
> filter_features_for_kvm(), add_flagname_to_bitmaps(), CPU feature-bit
> property lookup/registration, and the proposed "feature-words" property)
> 
> The following field replacements were made on X86CPU and x86_def_t:
> 
>   (cpuid_)features         -> features[FEAT_1_EDX]
>   (cpuid_)ext_features     -> features[FEAT_1_ECX]
>   (cpuid_)ext2_features    -> features[FEAT_8000_0001_EDX]
>   (cpuid_)ext3_features    -> features[FEAT_8000_0001_ECX]
>   (cpuid_)ext4_features    -> features[FEAT_C000_0001_EDX]
>   (cpuid_)kvm_features     -> features[FEAT_KVM]
>   (cpuid_)svm_features     -> features[FEAT_SVM]
>   (cpuid_)7_0_ebx_features -> features[FEAT_7_0_EBX]
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> Reviewed-By: Igor Mammedov <imammedo@redhat.com>
[...]
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 73ae2ef..110ef98 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
[...]
> @@ -1490,22 +1485,22 @@ static void cpu_x86_parse_featurestr(X86CPU *cpu, char *features, Error **errp)
>          }
>          featurestr = strtok(NULL, ",");
>      }
> -    env->cpuid_features |= plus_features[FEAT_1_EDX];
> -    env->cpuid_ext_features |= plus_features[FEAT_1_ECX];
> -    env->cpuid_ext2_features |= plus_features[FEAT_8000_0001_EDX];
> -    env->cpuid_ext3_features |= plus_features[FEAT_8000_0001_ECX];
> -    env->cpuid_ext4_features |= plus_features[FEAT_C000_0001_EDX];
> -    env->cpuid_kvm_features |= plus_features[FEAT_KVM];
> -    env->cpuid_svm_features |= plus_features[FEAT_SVM];
> -    env->cpuid_7_0_ebx_features |= plus_features[FEAT_7_0_EBX];
> -    env->cpuid_features &= ~minus_features[FEAT_1_EDX];
> -    env->cpuid_ext_features &= ~minus_features[FEAT_1_ECX];
> -    env->cpuid_ext2_features &= ~minus_features[FEAT_8000_0001_EDX];
> -    env->cpuid_ext3_features &= ~minus_features[FEAT_8000_0001_ECX];
> -    env->cpuid_ext4_features &= ~minus_features[FEAT_C000_0001_EDX];
> -    env->cpuid_kvm_features &= ~minus_features[FEAT_KVM];
> -    env->cpuid_svm_features &= ~minus_features[FEAT_SVM];
> -    env->cpuid_7_0_ebx_features &= ~minus_features[FEAT_7_0_EBX];
> +    env->features[FEAT_1_EDX] |= plus_features[FEAT_1_EDX];
> +    env->features[FEAT_1_ECX] |= plus_features[FEAT_1_ECX];
> +    env->features[FEAT_8000_0001_EDX] |= plus_features[FEAT_8000_0001_EDX];
> +    env->features[FEAT_8000_0001_ECX] |= plus_features[FEAT_8000_0001_ECX];
> +    env->features[FEAT_C000_0001_EDX] |= plus_features[FEAT_C000_0001_EDX];
> +    env->features[FEAT_KVM] |= plus_features[FEAT_KVM];
> +    env->features[FEAT_SVM] |= plus_features[FEAT_SVM];
> +    env->features[FEAT_7_0_EBX] |= plus_features[FEAT_7_0_EBX];
> +    env->features[FEAT_1_EDX] &= ~minus_features[FEAT_1_EDX];
> +    env->features[FEAT_1_ECX] &= ~minus_features[FEAT_1_ECX];
> +    env->features[FEAT_8000_0001_EDX] &= ~minus_features[FEAT_8000_0001_EDX];
> +    env->features[FEAT_8000_0001_ECX] &= ~minus_features[FEAT_8000_0001_ECX];
> +    env->features[FEAT_C000_0001_EDX] &= ~minus_features[FEAT_C000_0001_EDX];
> +    env->features[FEAT_KVM] &= ~minus_features[FEAT_KVM];
> +    env->features[FEAT_SVM] &= ~minus_features[FEAT_SVM];
> +    env->features[FEAT_7_0_EBX] &= ~minus_features[FEAT_7_0_EBX];
>  
>  out:
>      return;

Can this be done in a loop as a follow-up?

[...]
> @@ -1630,24 +1625,24 @@ static void cpu_x86_register(X86CPU *cpu, const char *name, Error **errp)
>      }
>  
>      if (kvm_enabled()) {
> -        def->kvm_features |= kvm_default_features;
> +        def->features[FEAT_KVM] |= kvm_default_features;
>      }
> -    def->ext_features |= CPUID_EXT_HYPERVISOR;
> +    def->features[FEAT_1_ECX] |= CPUID_EXT_HYPERVISOR;
>  
>      object_property_set_str(OBJECT(cpu), def->vendor, "vendor", errp);
>      object_property_set_int(OBJECT(cpu), def->level, "level", errp);
>      object_property_set_int(OBJECT(cpu), def->family, "family", errp);
>      object_property_set_int(OBJECT(cpu), def->model, "model", errp);
>      object_property_set_int(OBJECT(cpu), def->stepping, "stepping", errp);
> -    env->cpuid_features = def->features;
> -    env->cpuid_ext_features = def->ext_features;
> -    env->cpuid_ext2_features = def->ext2_features;
> -    env->cpuid_ext3_features = def->ext3_features;
> +    env->features[FEAT_1_EDX] = def->features[FEAT_1_EDX];
> +    env->features[FEAT_1_ECX] = def->features[FEAT_1_ECX];
> +    env->features[FEAT_8000_0001_EDX] = def->features[FEAT_8000_0001_EDX];
> +    env->features[FEAT_8000_0001_ECX] = def->features[FEAT_8000_0001_ECX];
>      object_property_set_int(OBJECT(cpu), def->xlevel, "xlevel", errp);
> -    env->cpuid_kvm_features = def->kvm_features;
> -    env->cpuid_svm_features = def->svm_features;
> -    env->cpuid_ext4_features = def->ext4_features;
> -    env->cpuid_7_0_ebx_features = def->cpuid_7_0_ebx_features;
> +    env->features[FEAT_KVM] = def->features[FEAT_KVM];
> +    env->features[FEAT_SVM] = def->features[FEAT_SVM];
> +    env->features[FEAT_C000_0001_EDX] = def->features[FEAT_C000_0001_EDX];
> +    env->features[FEAT_7_0_EBX] = def->features[FEAT_7_0_EBX];
>      env->cpuid_xlevel2 = def->xlevel2;
>  
>      object_property_set_str(OBJECT(cpu), def->model_id, "model-id", errp);
[snip]

Dito?

Andreas

-- 
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg, Germany
GF: Jeff Hawn, Jennifer Guild, Felix Imendörffer; HRB 16746 AG Nürnberg