Re: [Qemu-devel] [PATCH 02/10] target-i386: cpu: move features logic that requires CPUState to realize time

[Eduardo Habkost]

On Mon, Jun 06, 2016 at 05:16:44PM +0200, Igor Mammedov wrote:
> Making x86_cpu_parse_featurestr() a pure convertor
> of legacy feature string into global properties, needs
> it to be called before a CPU instance is created so
> parser shouldn't modify CPUState directly or access
> it at all. Hence move current hack that directly pokes
> into CPUState, to set/unset +-feats, from parser to
> CPU's realize method.
> 
> Signed-off-by: Igor Mammedov <address@hidden>

Reviewed-by: Eduardo Habkost <address@hidden>

I will just edit a comment below, when applying, for clarity:

[...]
> +    /* TODO: convert plus_features & minus_features static vars
> +     * to global properties, once broken host_features is fixed
> +     */

I will rewrite this to:

    /*TODO: cpu->host_features inclurrectly overwrites features
     * set using "feat=on|off". Once we fix this, we can convert
     * plus_features & minus_features to global properties
     * inside x86_cpu_parse_featurestr() too.
     */

> +    if (cpu->host_features) {
> +        for (w = 0; w < FEATURE_WORDS; w++) {
> +            env->features[w] =
> +                x86_cpu_get_supported_feature_word(w, cpu->migratable);
> +        }
> +    }
> +
> +    for (w = 0; w < FEATURE_WORDS; w++) {
> +        cpu->env.features[w] |= plus_features[w];
> +        cpu->env.features[w] &= ~minus_features[w];
> +    }
> +
>      if (env->features[FEAT_7_0_EBX] && env->cpuid_level < 7) {
>          env->cpuid_level = 7;
>      }
> -- 
> 1.8.3.1
> 

-- 
Eduardo