Re: [PATCH v6 14/16] target-i386: Add setting of Hypervisor leaf extra for known vmare4.

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Sep 24, 2012 at 10:32:16AM -0400, Don Slutz wrote:
> This was taken from:
>   http://article.gmane.org/gmane.comp.emulators.kvm.devel/22643
> 
> Signed-off-by: Don Slutz <Don@CloudSwitch.com>
> ---
>  target-i386/cpu.c |   32 ++++++++++++++++++++++++++++++++
>  1 files changed, 32 insertions(+), 0 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 8bb20c7..b77dbfe 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1135,6 +1135,36 @@ static void x86_cpuid_set_model_id(Object *obj, const char *model_id,
>      }
>  }
>  
> +static void x86_cpuid_set_vmware_extra(Object *obj)
> +{
> +    X86CPU *cpu = X86_CPU(obj);
> +
> +    if ((cpu->env.tsc_khz != 0) &&
> +        (cpu->env.cpuid_hv_level == CPUID_HV_LEVEL_VMWARE_4) &&
> +        (cpu->env.cpuid_hv_vendor1 == CPUID_HV_VENDOR_VMWARE_1) &&
> +        (cpu->env.cpuid_hv_vendor2 == CPUID_HV_VENDOR_VMWARE_2) &&
> +        (cpu->env.cpuid_hv_vendor3 == CPUID_HV_VENDOR_VMWARE_3)) {
> +        const uint32_t apic_khz = 1000000L;
> +
> +        /*
> +         * From article.gmane.org/gmane.comp.emulators.kvm.devel/22643
> +         *
> +         *    Leaf 0x40000010, Timing Information.
> +         *
> +         *    VMware has defined the first generic leaf to provide timing
> +         *    information.  This leaf returns the current TSC frequency and
> +         *    current Bus frequency in kHz.
> +         *
> +         *    # EAX: (Virtual) TSC frequency in kHz.
> +         *    # EBX: (Virtual) Bus (local apic timer) frequency in kHz.
> +         *    # ECX, EDX: RESERVED (Per above, reserved fields are set to zero).
> +         */
> +        cpu->env.cpuid_hv_extra = 0x40000010;
> +        cpu->env.cpuid_hv_extra_a = (uint32_t)cpu->env.tsc_khz;
> +        cpu->env.cpuid_hv_extra_b = apic_khz;
> +    }
> +}
> +
>  static void x86_cpuid_get_tsc_freq(Object *obj, Visitor *v, void *opaque,
>                                     const char *name, Error **errp)
>  {
> @@ -1164,6 +1194,7 @@ static void x86_cpuid_set_tsc_freq(Object *obj, Visitor *v, void *opaque,
>      }
>  
>      cpu->env.tsc_khz = value / 1000;
> +    x86_cpuid_set_vmware_extra(obj);
>  }
>  
>  static void x86_cpuid_get_hv_level(Object *obj, Visitor *v, void *opaque,
> @@ -1277,6 +1308,7 @@ static void x86_cpuid_set_hv_vendor(Object *obj, const char *value,
>          env->cpuid_hv_vendor3 |= ((uint8_t)adj_value[i + 8]) << (8 * i);
>      }
>      env->cpuid_hv_vendor_set = true;
> +    x86_cpuid_set_vmware_extra(obj);
>  }

This is strange. Please have this configuration, that depends on other
properties being set, ordered in x86_cpu_initfn. Say:

    object_property_add(obj, "tsc-frequency", "int",
                        x86_cpuid_get_tsc_freq,
                        x86_cpuid_set_tsc_freq, NULL, NULL, NULL);

    /* depends on tsc frequency */
    object_property_add(obj, "vmware-extra", "int",
                        x86_cpuid_get_vmware_extra,
                        x86_cpuid_set_vmware_extra, NULL, NULL, NULL);

Or something to that effect.