Re: [Xen-devel] [PATCH for-4.5 v7 1/7] xen: Add support for VMware cpuid leaves

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.01.15 at 22:00, <dslutz@xxxxxxxxxxx> wrote:
> On 01/15/15 11:42, Jan Beulich wrote:
>>>>> On 02.10.14 at 23:30, <dslutz@xxxxxxxxxxx> wrote:
>>> --- /dev/null
>>> +++ b/xen/arch/x86/hvm/vmware/cpuid.c
>> 
>> Whether adding another subdirectory here is really the way to go
>> heavily depends on how much of this new code we really want to
>> take into the tree. There sheer size of the series makes me
>> hesitant to consider taking it all.
>> 
> 
> Not sure what I can say here to help.

Much will depend on the discussion of the subsequent much bigger
patch, so nothing specific to say here for the moment.

>>> +/*
>>> + * VMware hardware version 7 defines some of these cpuid levels,
>>> + * below is a brief description about those.
>>> + *
>>> + *     Leaf 0x40000000, Hypervisor CPUID information
>>> + * # EAX: The maximum input value for hypervisor CPUID info (0x40000010).
>>> + * # EBX, ECX, EDX: Hypervisor vendor ID signature. E.g. "VMwareVMware"
>>> + *
>>> + *     Leaf 0x40000010, Timing information.
>>> + * # EAX: (Virtual) TSC frequency in kHz.
>>> + * # EBX: (Virtual) Bus (local apic timer) frequency in kHz.
>>> + * # ECX, EDX: RESERVED
>>> + */
>>> +
>>> +int cpuid_vmware_leaves(uint32_t idx, uint32_t *eax, uint32_t *ebx,
>>> +                        uint32_t *ecx, uint32_t *edx)
>>> +{
>>> +    struct domain *d = current->domain;
>>> +
>>> +    if ( !is_vmware_domain(d) )
>>> +        return 0;
>>> +
>>> +    switch ( idx - 0x40000000 )
>>> +    {
>>> +    case 0x0:
>>> +        if ( d->arch.hvm_domain.params[HVM_PARAM_VMWARE_HW] >= 7 )
>>> +        {
>>> +            *eax = 0x40000010;  /* Largest leaf */
>>> +            *ebx = 0x61774d56;  /* "VMwa" */
>>> +            *ecx = 0x4d566572;  /* "reVM" */
>>> +            *edx = 0x65726177;  /* "ware" */
>>> +            break;
>>> +        }
>>> +        /* fallthrough */
>>> +    case 0x10:
>>> +        if ( d->arch.hvm_domain.params[HVM_PARAM_VMWARE_HW] >= 7 )
>>> +        {
>>> +            /* (Virtual) TSC frequency in kHz. */
>>> +            *eax =  d->arch.tsc_khz;
>>> +            /* (Virtual) Bus (local apic timer) frequency in kHz. */
>>> +            *ebx = 1000000ull / APIC_BUS_CYCLE_NS;
>>> +            *ecx = 0;          /* Reserved */
>>> +            *edx = 0;          /* Reserved */
>>> +            break;
>>> +        }
>>> +        /* fallthrough */
>> 
>> So for versions < 7 there's effectively no CPUID support at all?
>> Wouldn't it then make more sense to check for the version together
>> with the is_vmware_domain() check at the top?
>> 
> 
> Nope, when version is > 0 & < 7, all zeros are returned for these.
> This is why there is a fallthrough comment.
> 
> Doing the zero returns is part of making the environment look as much
> like VMware as possible.  I feel this is better, but I can be pushed
> into including this check at the top (since the VMware KB article
> referenced just has an equal statement).

But afaict zeros get returned even when you bail from the function
right at the top, thanks to the subsequent domain_cpuid() invocation
in the caller (unless overridden in the guest config, which surely
would be dubious).

>>> --- a/xen/include/public/hvm/params.h
>>> +++ b/xen/include/public/hvm/params.h
>>> @@ -189,6 +189,9 @@
>>>  /* Location of the VM Generation ID in guest physical address space. */
>>>  #define HVM_PARAM_VM_GENERATION_ID_ADDR 34
>>>  
>>> -#define HVM_NR_PARAMS          35
>>> +/* Params for VMware */
>>> +#define HVM_PARAM_VMWARE_HW                 35
>> 
>> The comment seems wrong - after all it's the version, not some
>> arbitrary parameters/flags.
>> 
> 
> It use to be a set of Params.  Will fix, thinking of:
> 
> /* VMware Hardware Version */

... emulated ...

> Did you what me to rename HVM_PARAM_VMWARE_HW also to new name (maybe
> HVM_PARAM_VMWARE_HARDWARE_VERSION)?

Perhaps a good idea; if it turns out too long,
HVM_PARAM_VMWARE_HWVER would also seem fine.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel