Re: [Xen-devel] [PATCH v11 3/9] tools: Add vmware_hwver support
On 06/04/15 11:17, Ian Campbell wrote:
> On Fri, 2015-05-22 at 11:50 -0400, Don Slutz wrote:
>> [...]
>> +=item B
>> +
>> +Turns on or off the exposure of VMware cpuid. The number is
>> +VMware's hardware version number, where 0 is off. A number >= 7
>> +is needed to enable exposure of VMware cpuid.
>> +
>> +The hardware version number (vmware_hwver) come from VMware config files.
>
> "comes"
>
Will fix.
>> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
>> index 38d065f..4362d5d 100644
>> --- a/tools/libxc/xc_domain.c
>> +++ b/tools/libxc/xc_domain.c
>> @@ -64,7 +64,7 @@ int xc_domain_create(xc_interface *xch,
>> memset(&config, 0, sizeof(config));
>>
>> #if defined (__i386) || defined(__x86_64__)
>> -/* No arch-specific configuration for now */
>> +/* No arch-specific default configuration for now */
>
> I'm not sure this hunk has anything to do with this patch, nor what the
> semantic difference between the old and new text is supposed to be.
>
I do not have to make this change. However I feel the comment is
misleading, and I changed it. The real change is later:
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 651b338..fd7dafa 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -5,8 +5,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
libxl_domain_config *d_config,
xc_domain_configuration_t *xc_config)
{
-/* Note: will be changed in a later patch */
-xc_config->vmware_hwver = 0;
+xc_config->vmware_hwver = d_config->c_info.vmware_hwver;
return 0;
}
which is where "arch-specific configuration" is done. This is where the
default "arch-specific configuration" is done.
-Don Slutz
> Ian.
>
>
>
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
>
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v11 3/9] tools: Add vmware_hwver support
On 06/04/15 11:15, Ian Campbell wrote: > On Wed, 2015-06-03 at 15:53 +0100, George Dunlap wrote: >> On 05/22/2015 04:50 PM, Don Slutz wrote: >>> This is used to set xen_arch_domainconfig vmware_hw. It is set to >>> the emulated VMware virtual hardware version. >>> >>> Currently 0, 3-4, 6-11 are good values. However the code only >>> checks for == 0, != 0, or < 7. >>> >>> Signed-off-by: Don Slutz >> >> Ian, >> >> It looks like you gave a pre-approved Ack to something almost identical >> to v10. > > In v9 I indicated that LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE and > LIBXL_HAVE_BUILDINFO_HVM_VMWARE_HWVER could be covered by a single ack > (introducing vmware support generally). > > In v11 this seems to have morphed into only > LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE being provided, which is > clearly not an appropriate umbrella #define. > Only in PATCH 1/9 -- Which in v11 is now completely independent. I only kept it in the series since in v10 it was not fully independent. > I'm also not sure if there is more stuff later in the series, if so then > unless it is all committed together an umbrella option may not work, > unless it is added right at the end, in which case I suppose having some > "unadvertised" functionality in the midst of a dev cycle would be ok. > Releasing like that would be a mistake though. > There is one later in the series 7/9. to which you said (in a different thread): >> +#define LIBXL_HAVE_CREATEINFO_VMWARE 1 > > Lets just have a single one of these indicating support for vmware, it > should be added at the end of the series after all the baseline vmware > functionality is in place. I think that means hwver, vga=vmware and this > port stuff. > > (Future incremental changes will of course require their own flags). If I am reading this correctly, you want PATCH 1/9 to not be completely independent. -Don Slutz > Ian. > ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v11 3/9] tools: Add vmware_hwver support
On Fri, 2015-05-22 at 11:50 -0400, Don Slutz wrote: > [...] > +=item B > + > +Turns on or off the exposure of VMware cpuid. The number is > +VMware's hardware version number, where 0 is off. A number >= 7 > +is needed to enable exposure of VMware cpuid. > + > +The hardware version number (vmware_hwver) come from VMware config files. "comes" > diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c > index 38d065f..4362d5d 100644 > --- a/tools/libxc/xc_domain.c > +++ b/tools/libxc/xc_domain.c > @@ -64,7 +64,7 @@ int xc_domain_create(xc_interface *xch, > memset(&config, 0, sizeof(config)); > > #if defined (__i386) || defined(__x86_64__) > -/* No arch-specific configuration for now */ > +/* No arch-specific default configuration for now */ I'm not sure this hunk has anything to do with this patch, nor what the semantic difference between the old and new text is supposed to be. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v11 3/9] tools: Add vmware_hwver support
On Wed, 2015-06-03 at 15:53 +0100, George Dunlap wrote: > On 05/22/2015 04:50 PM, Don Slutz wrote: > > This is used to set xen_arch_domainconfig vmware_hw. It is set to > > the emulated VMware virtual hardware version. > > > > Currently 0, 3-4, 6-11 are good values. However the code only > > checks for == 0, != 0, or < 7. > > > > Signed-off-by: Don Slutz > > Ian, > > It looks like you gave a pre-approved Ack to something almost identical > to v10. In v9 I indicated that LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE and LIBXL_HAVE_BUILDINFO_HVM_VMWARE_HWVER could be covered by a single ack (introducing vmware support generally). In v11 this seems to have morphed into only LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE being provided, which is clearly not an appropriate umbrella #define. I'm also not sure if there is more stuff later in the series, if so then unless it is all committed together an umbrella option may not work, unless it is added right at the end, in which case I suppose having some "unadvertised" functionality in the midst of a dev cycle would be ok. Releasing like that would be a mistake though. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH v11 3/9] tools: Add vmware_hwver support
On 05/22/2015 04:50 PM, Don Slutz wrote:
> This is used to set xen_arch_domainconfig vmware_hw. It is set to
> the emulated VMware virtual hardware version.
>
> Currently 0, 3-4, 6-11 are good values. However the code only
> checks for == 0, != 0, or < 7.
>
> Signed-off-by: Don Slutz
Ian,
It looks like you gave a pre-approved Ack to something almost identical
to v10. The only thing in v10 not in your pre-approval specification
was removing the thing automatically setting vga=vmware. Instead in v10
he added a patch to take 'vmware' as an explicit argument to vga.
Patch 2 is reviewed-by Andy Cooper (and can have an Ack from me if it's
wanted), and patches 1-3 constitute a sensible chunk of functionality --
if you could take a quick look over 1 and 3 and give them a thumbs-up, I
think it might make sense to go ahead and check those in.
(Still working through the rest of the patches.)
-George
> ---
> v11:
> Dropped "If non-zero then default VGA to VMware's VGA"
>
> v10:
> LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE &
> LIBXL_HAVE_BUILDINFO_HVM_VMWARE_HWVER are arriving together
> a single umbrella could be used.
> Since I split the LIBXL_VGA_INTERFACE_TYPE_VMWARE into
> it's own patch, this is not longer true.
> But I did use 1 for the 2 c_info changes.
> Please use GCSPRINTF.
> Remove vga=vmware from here.
>
> v9:
> I assumed that s/vmware_hw/vmware_hwver/ is not a big enough
> change to drop the Reviewed-by. Did a minor edit to the
> commit message to add 7 to the list of values checked.
>
> v7:
> Default handling of hvm.vga.kind bad.
> Fixed.
> Default of vmware_port should be based on vmware_hw.
> Done.
>
> v5:
> Anything looking for Xen according to the Xen cpuid instructions...
> Adjusted doc to new wording.
>
> docs/man/xl.cfg.pod.5 | 17 +
> tools/libxc/xc_domain.c | 2 +-
> tools/libxl/libxl_create.c | 4 +++-
> tools/libxl/libxl_types.idl | 1 +
> tools/libxl/libxl_x86.c | 3 +--
> tools/libxl/xl_cmdimpl.c| 2 ++
> 6 files changed, 25 insertions(+), 4 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 84078f6..eaad4bf 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1348,6 +1348,23 @@ The viridian option can be specified as a boolean. A
> value of true (1)
> is equivalent to the list [ "defaults" ], and a value of false (0) is
> equivalent to an empty list.
>
> +=item B
> +
> +Turns on or off the exposure of VMware cpuid. The number is
> +VMware's hardware version number, where 0 is off. A number >= 7
> +is needed to enable exposure of VMware cpuid.
> +
> +The hardware version number (vmware_hwver) come from VMware config files.
> +
> +=over 4
> +
> +In a .vmx it is virtualHW.version
> +
> +In a .ovf it is part of the value of vssd:VirtualSystemType.
> +For vssd:VirtualSystemType == vmx-07, vmware_hwver = 7.
> +
> +=back
> +
> =back
>
> =head3 Emulated VGA Graphics Device
> diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
> index 38d065f..4362d5d 100644
> --- a/tools/libxc/xc_domain.c
> +++ b/tools/libxc/xc_domain.c
> @@ -64,7 +64,7 @@ int xc_domain_create(xc_interface *xch,
> memset(&config, 0, sizeof(config));
>
> #if defined (__i386) || defined(__x86_64__)
> -/* No arch-specific configuration for now */
> +/* No arch-specific default configuration for now */
> #elif defined (__arm__) || defined(__aarch64__)
> config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT;
> config.nr_spis = 0;
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 86384d2..895577f 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -464,7 +464,7 @@ int libxl__domain_build(libxl__gc *gc,
> vments[4] = "start_time";
> vments[5] = libxl__sprintf(gc, "%lu.%02d",
> start_time.tv_sec,(int)start_time.tv_usec/1);
>
> -localents = libxl__calloc(gc, 9, sizeof(char *));
> +localents = libxl__calloc(gc, 11, sizeof(char *));
> i = 0;
> localents[i++] = "platform/acpi";
> localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
> @@ -472,6 +472,8 @@ int libxl__domain_build(libxl__gc *gc,
> localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
> localents[i++] = "platform/acpi_s4";
> localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
> +localents[i++] = "platform/vmware_hwver";
> +localents[i++] = GCSPRINTF("%"PRId64, d_config->c_info.vmware_hwver);
> if (info->u.hvm.mmio_hole_memkb) {
> uint64_t max_ram_below_4g =
> (1ULL << 32) - (info->u.hvm.mmio_hole_memkb << 10);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 6cab732..c8a1345 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
>
[Xen-devel] [PATCH v11 3/9] tools: Add vmware_hwver support
This is used to set xen_arch_domainconfig vmware_hw. It is set to
the emulated VMware virtual hardware version.
Currently 0, 3-4, 6-11 are good values. However the code only
checks for == 0, != 0, or < 7.
Signed-off-by: Don Slutz
---
v11:
Dropped "If non-zero then default VGA to VMware's VGA"
v10:
LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE &
LIBXL_HAVE_BUILDINFO_HVM_VMWARE_HWVER are arriving together
a single umbrella could be used.
Since I split the LIBXL_VGA_INTERFACE_TYPE_VMWARE into
it's own patch, this is not longer true.
But I did use 1 for the 2 c_info changes.
Please use GCSPRINTF.
Remove vga=vmware from here.
v9:
I assumed that s/vmware_hw/vmware_hwver/ is not a big enough
change to drop the Reviewed-by. Did a minor edit to the
commit message to add 7 to the list of values checked.
v7:
Default handling of hvm.vga.kind bad.
Fixed.
Default of vmware_port should be based on vmware_hw.
Done.
v5:
Anything looking for Xen according to the Xen cpuid instructions...
Adjusted doc to new wording.
docs/man/xl.cfg.pod.5 | 17 +
tools/libxc/xc_domain.c | 2 +-
tools/libxl/libxl_create.c | 4 +++-
tools/libxl/libxl_types.idl | 1 +
tools/libxl/libxl_x86.c | 3 +--
tools/libxl/xl_cmdimpl.c| 2 ++
6 files changed, 25 insertions(+), 4 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 84078f6..eaad4bf 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1348,6 +1348,23 @@ The viridian option can be specified as a boolean. A
value of true (1)
is equivalent to the list [ "defaults" ], and a value of false (0) is
equivalent to an empty list.
+=item B
+
+Turns on or off the exposure of VMware cpuid. The number is
+VMware's hardware version number, where 0 is off. A number >= 7
+is needed to enable exposure of VMware cpuid.
+
+The hardware version number (vmware_hwver) come from VMware config files.
+
+=over 4
+
+In a .vmx it is virtualHW.version
+
+In a .ovf it is part of the value of vssd:VirtualSystemType.
+For vssd:VirtualSystemType == vmx-07, vmware_hwver = 7.
+
+=back
+
=back
=head3 Emulated VGA Graphics Device
diff --git a/tools/libxc/xc_domain.c b/tools/libxc/xc_domain.c
index 38d065f..4362d5d 100644
--- a/tools/libxc/xc_domain.c
+++ b/tools/libxc/xc_domain.c
@@ -64,7 +64,7 @@ int xc_domain_create(xc_interface *xch,
memset(&config, 0, sizeof(config));
#if defined (__i386) || defined(__x86_64__)
-/* No arch-specific configuration for now */
+/* No arch-specific default configuration for now */
#elif defined (__arm__) || defined(__aarch64__)
config.gic_version = XEN_DOMCTL_CONFIG_GIC_DEFAULT;
config.nr_spis = 0;
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 86384d2..895577f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -464,7 +464,7 @@ int libxl__domain_build(libxl__gc *gc,
vments[4] = "start_time";
vments[5] = libxl__sprintf(gc, "%lu.%02d",
start_time.tv_sec,(int)start_time.tv_usec/1);
-localents = libxl__calloc(gc, 9, sizeof(char *));
+localents = libxl__calloc(gc, 11, sizeof(char *));
i = 0;
localents[i++] = "platform/acpi";
localents[i++] = libxl_defbool_val(info->u.hvm.acpi) ? "1" : "0";
@@ -472,6 +472,8 @@ int libxl__domain_build(libxl__gc *gc,
localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s3) ? "1" : "0";
localents[i++] = "platform/acpi_s4";
localents[i++] = libxl_defbool_val(info->u.hvm.acpi_s4) ? "1" : "0";
+localents[i++] = "platform/vmware_hwver";
+localents[i++] = GCSPRINTF("%"PRId64, d_config->c_info.vmware_hwver);
if (info->u.hvm.mmio_hole_memkb) {
uint64_t max_ram_below_4g =
(1ULL << 32) - (info->u.hvm.mmio_hole_memkb << 10);
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 6cab732..c8a1345 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -343,6 +343,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
("run_hotplug_scripts",libxl_defbool),
("pvh", libxl_defbool),
("driver_domain",libxl_defbool),
+("vmware_hwver", uint64),
], dir=DIR_IN)
libxl_domain_restore_params = Struct("domain_restore_params", [
diff --git a/tools/libxl/libxl_x86.c b/tools/libxl/libxl_x86.c
index 651b338..fd7dafa 100644
--- a/tools/libxl/libxl_x86.c
+++ b/tools/libxl/libxl_x86.c
@@ -5,8 +5,7 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
libxl_domain_config *d_config,
xc_domain_configuration_t *xc_config)
{
-/* Note: will be changed in a later patch */
-xc_config->vmware_hwver = 0;
+xc_config->vmware_hwver = d_config->c_info.vmware_hwver;
return 0;
}
diff --git a/tools/libxl/xl_cmdimpl.c b/to
