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

Marcelo Tosatti
2012-10-09T19:11:47Z

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.