Re: [Qemu-devel] [PATCH 4/4] x86: Support feature=force on the command-line

[Igor Mammedov <imammedo@redhat.com>]

On Tue,  2 May 2017 17:31:15 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:

> Introduce a new CPUFeatureSetting QAPI data type, and use it to support
> feature=force on -cpu.
commit message lacks rationale why it's needed.
I suspect that it's to enable forced mwait.
It would be nice to put here answer to what
motivated to write this patch and reference commit ids
of kernel patches if there are/needed any to make it work.

> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
[...]
> @@ -3717,25 +3727,46 @@ static void x86_cpu_set_bit_prop(Object *obj, Visitor *v, const char *name,
>      X86CPU *cpu = X86_CPU(obj);
>      BitProperty *fp = opaque;
>      Error *local_err = NULL;
> -    bool value;
> +    CPUFeatureSetting *value = NULL;
>  
>      if (dev->realized) {
>          qdev_prop_set_after_realize(dev, name, errp);
>          return;
>      }
>  
> -    visit_type_bool(v, name, &value, &local_err);
> +    visit_type_CPUFeatureSetting(v, name, &value, &local_err);
>      if (local_err) {
>          error_propagate(errp, local_err);
>          return;
>      }
>  
> -    if (value) {
> -        cpu->env.features[fp->w] |= fp->mask;
> -    } else {
> -        cpu->env.features[fp->w] &= ~fp->mask;
> +    switch (value->type) {
> +    case QTYPE_QBOOL:
> +        if (value->u.q_bool) {
> +            cpu->env.features[fp->w] |= fp->mask;
> +        } else {
> +            cpu->env.features[fp->w] &= ~fp->mask;
> +        }
> +        cpu->env.forced_features[fp->w] &= ~fp->mask;
> +        cpu->env.user_features[fp->w] |= fp->mask;
> +    break;
> +    case QTYPE_QSTRING:
> +        switch (value->u.q_enum) {
> +        case CPU_FEATURE_SETTING_ENUM_FORCE:
> +            cpu->env.features[fp->w] |= fp->mask;
> +            cpu->env.forced_features[fp->w] |= fp->mask;
> +        break;
> +        default:
> +            error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
> +                       "CPUFeatureSetting");
> +        }
> +    break;
> +    default:
> +        error_setg(errp, QERR_INVALID_PARAMETER_TYPE, name,
> +                   "CPUFeatureSetting");
>      }
> -    cpu->env.user_features[fp->w] |= fp->mask;
that's the only place it was set and then used by x86_cpu_expand_features()
Is it correct to remove setter?


> +
> +    qapi_free_CPUFeatureSetting(value);
>  }
>  
>  static void x86_cpu_release_bit_prop(Object *obj, const char *name,