Re: [PATCH v2 1/3] Add vmware_hw to xl.cfg

[Don Slutz <dslutz@verizon.com>]

On 09/08/14 09:17, Ian Campbell wrote:
> On Mon, 2014-09-01 at 11:33 -0400, Don Slutz wrote:
>> If non-zero then
>>    Return VMware's cpuid leaves.
>>    Not doing the hardcoded IRQ9 on PIIX4 ACPI PM.
> Please can you say a words about why this is and what the implications
> are?

Duplicate -- windows activation.

>>    Force use of VMware's VGA in QEMU.
>>
>> The support of hypervisor cpuid leaves has not been agreed to.
> Who needs to agree to this? Just us or do we need to be seeking
> consensus with other hypervisors?

Possible consensus with other hypervisors.    The 2 that are an issue
if MicroSoft (Hyper-V, viridian) and VMware.  Xen is not the issue.


>> So based on this, I picked the order:
>>
>> 0x40000000 is viridian, vmware or xen
>> 0x40000100 is vmware or xen
>> 0x40000200 is xen
> Which is another way of saying that the enabled options will be
> presented in the order viridian, vmware, xen.
>

Yes.

>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>> index f1fc906..34fb021 100644
>> --- a/docs/man/xl.cfg.pod.5
>> +++ b/docs/man/xl.cfg.pod.5
>> @@ -1139,6 +1139,12 @@ some other Operating Systems and in some circumstance can prevent
>>   Xen's own paravirtualisation interfaces for HVM guests from being
>>   used.
>>
>> +=item B<vmware_hw=NUMBER>
>> +
>> +Turns on or off the exposure of VMware cpuid.  The number is the
>> +VMware's hardware version number, where 0 is off.  If on it also
>> +forces the use of VMware's VGA in QEMU.
> Do you have a reference of the non-zero values of this field? How can a
> user determine what the correct number to use is?

http://kb.vmware.com/selfservice/microsites/search.do?language=en_US&cmd=displayKC&externalId=1003746

For most uses, any non-zero is good enough.  Where it matters is how
QEMU presents emulated hardware.  VMware changes pci config space
based on this number which is stored in a .vmx file.

Is vmware_hw too short?  Should I use vmware_virtual_hardware_version?

or just adjust the xl.cfg.pod.5 description?




> Other than parroting this value back to the guest in a cpuid leaf does
> this value control anything else? If so then we may want to consider
> something like an enum to allow us to advertise more precisely which
> versions of vmware we are prepared to ape, but at the least we need to
> range check this input somewhere along the way.

See above, mostly just QEMU.


>> diff --git a/tools/libxc/xc_domain_restore.c b/tools/libxc/xc_domain_restore.c
>> index b9a56d5..bc5cd57 100644
>> --- a/tools/libxc/xc_domain_restore.c
>> +++ b/tools/libxc/xc_domain_restore.c
>> diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
>> index 254fdb3..76dc307 100644
>> --- a/tools/libxc/xc_domain_save.c
>> +++ b/tools/libxc/xc_domain_save.c
> There will be a race condition between this series and the new migration
> stuff. Copying Andy for his opinion on how to proceed.
>
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index 103cbca..c79274b 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -542,19 +542,28 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>>               }
>>           }
>>
>> -        switch (b_info->u.hvm.vga.kind) {
>> -        case LIBXL_VGA_INTERFACE_TYPE_STD:
>> -            flexarray_append_pair(dm_args, "-device",
>> -                GCSPRINTF("VGA,vgamem_mb=%d",
>> -                libxl__sizekb_to_mb(b_info->video_memkb)));
>> -            break;
>> -        case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>> +        if (b_info->u.hvm.vmware_hw) {
> Should we have LIBXL_VGA_INTERFACE_TYPE_VMWARE, which is the default for
> hvm.vga.kind if vmware_hw is set?
>
> Apart from letting this code become cleaner this would allow user to
> enable vmware_hw and override the vga to something else (if they so
> wished for some reason).
>

Ok, I will look to adding this.


>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 08a7927..e03f46d 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -371,6 +371,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>                                          ("timeoffset",       string),
>>                                          ("hpet",             libxl_defbool),
>>                                          ("vpt_align",        libxl_defbool),
>> +                                       ("vmware_hw",        UInt(64, init_val = 0)),
> Everything defaults to 0 by default, so you don't need the initval. Also
> you can use "uint64" rather than open coding UInt(64).
>
> You need to add a LIBXL_HAVE_FOO to libxl.h so that applications know
> they can use this. There are a bunch of existing examples.

Will do.

    -Don Slutz


> Ian.
>