Re: [Qemu-devel] [PATCH v2 09/15] target-i386: Add property getter for CPU model

[Michael Roth]

On Tue, Apr 24, 2012 at 11:33:35AM +0200, Andreas Färber wrote:
> Signed-off-by: Andreas Färber <address@hidden>
> ---
>  target-i386/cpu.c |   14 +++++++++++++-
>  1 files changed, 13 insertions(+), 1 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 9479717..643289f 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -640,6 +640,18 @@ static void x86_cpuid_version_set_family(Object *obj, 
> Visitor *v, void *opaque,
>      }
>  }
>  
> +static void x86_cpuid_version_get_model(Object *obj, Visitor *v, void 
> *opaque,
> +                                        const char *name, Error **errp)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +    CPUX86State *env = &cpu->env;
> +    int64_t value;
> +
> +    value = (env->cpuid_version >> 4) & 0xf;
> +    value |= ((env->cpuid_version >> 16) & 0xf) << 4;
> +    visit_type_int(v, &value, name, errp);
> +}
> +

Reviewed-by: Michael Roth <address@hidden>

Just a note though,

The setter code does:

    env->cpuid_version &= ~0xf00f0;
    env->cpuid_version |= ((model & 0xf) << 4) | ((model >> 4) << 16);

So as a result I think there's a potential for the getter to not report bits
that were incorrectly set and exposed to the guest, since we mask off
bits outside the valid range in your code. But that would be a bug in the
setter code/cpudef of course and could be addressed outside this series.

>  static void x86_cpuid_version_set_model(Object *obj, Visitor *v, void 
> *opaque,
>                                          const char *name, Error **errp)
>  {
> @@ -1557,7 +1569,7 @@ static void x86_cpu_initfn(Object *obj)
>                          x86_cpuid_version_get_family,
>                          x86_cpuid_version_set_family, NULL, NULL, NULL);
>      object_property_add(obj, "model", "int",
> -                        NULL,
> +                        x86_cpuid_version_get_model,
>                          x86_cpuid_version_set_model, NULL, NULL, NULL);
>      object_property_add(obj, "stepping", "int",
>                          NULL,
> -- 
> 1.7.7
> 
>