[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-4.5 v6 02/16] tools: Add vmware_hw support

On 09/22/14 09:34, Ian Campbell wrote:
On Sat, 2014-09-20 at 14:07 -0400, Don Slutz wrote:
This is used to set HVM_PARAM_VMWARE_HW. It is set to the VMware
virtual hardware version.

Currently 0, 3-4, 6-11 are good values.  However the code only
checks for == 0 or != 0.

If non-zero then
   default VGA to VMware's VGA.

Also now allows vga=vmware

Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
       Anything looking for Xen according to the Xen cpuid instructions...
         Adjusted doc to new wording.

  docs/man/xl.cfg.pod.5               | 21 +++++++++++++++++++--
  docs/misc/hypervisor-cpuid.markdown | 28 ++++++++++++++++++++++++++++
  tools/libxc/xc_domain_restore.c     | 14 ++++++++++++++
  tools/libxc/xc_domain_save.c        | 11 +++++++++++
  tools/libxc/xg_save_restore.h       |  2 ++
  tools/libxl/libxl.h                 | 10 ++++++++++
  tools/libxl/libxl_create.c          |  4 +++-
  tools/libxl/libxl_dm.c              | 10 +++++++++-
  tools/libxl/libxl_dom.c             |  2 ++
  tools/libxl/libxl_types.idl         |  2 ++
  tools/libxl/xl_cmdimpl.c            | 11 ++++++++++-
  11 files changed, 110 insertions(+), 5 deletions(-)
  create mode 100644 docs/misc/hypervisor-cpuid.markdown

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 517ae2f..367b401 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1147,6 +1147,23 @@ some other Operating Systems and in some circumstance 
can prevent
  Xen's own paravirtualisation interfaces for HVM guests from being
+=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 not zero it
+changes the default VGA to VMware's VGA.
"is the VMware's" => "is VMware's".

Will do.

@@ -1185,8 +1202,8 @@ This option is deprecated, use vga="stdvga" instead.
=item B<vga="STRING"> -Selects the emulated video card (none|stdvga|cirrus).
-The default is cirrus.
+Selects the emulated video card (none|stdvga|cirrus|vmware).
+The default is cirrus (or vmware if B<vmware_hw> is not zero).
"The default is cirrus unless B<vmware_hw> is non-zero in which case it
is vmware." ?


=item B<vnc=BOOLEAN> diff --git a/docs/misc/hypervisor-cpuid.markdown b/docs/misc/hypervisor-cpuid.markdown
new file mode 100644
index 0000000..901a4e1
--- /dev/null
+++ b/docs/misc/hypervisor-cpuid.markdown
@@ -0,0 +1,28 @@
+Hypervisor Cpuid
+The support of hypervisor cpuid leaves has not been agreed to.

"the general hypervisor community" perhaps?

Perhaps a better way of putting this would be "There is no agreed
standard for the use of hypervisor cpuid leaves" or some such.


+Other then the range 0x40000000 to 0x400000ff can be used by
s/then/than/ I think.

I am not sure, so I will change it.

+MicroSoft Hyper-V (AKA viridian) currently must be at 0x40000000.
+VMware currently must be at 0x40000000.
+KVM currently must be at 0x40000000 (from Seabios).
+Xen can be found at the first otherwise unused 0x100 aligned
+offset between 0x40000000 and 0x40010000.
I think you should add " leaves" after each hypervisor name.


@@ -378,6 +379,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)),
There is no need for an explicitly 0 init_val, it's the default default.

Will switch to uint64.

                                         ("timer_mode",       libxl_timer_mode),
                                         ("nested_hvm",       libxl_defbool),
                                         ("smbios_firmware",  string),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 698b3bc..2119bd6 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1038,6 +1038,8 @@ static void parse_config_data(const char *config_source,
          xlu_cfg_get_defbool(config, "hpet", &b_info->u.hvm.hpet, 0);
          xlu_cfg_get_defbool(config, "vpt_align", &b_info->u.hvm.vpt_align, 0);
+ if (!xlu_cfg_get_long(config, "vmware_hw", &l, 1))
+            b_info->u.hvm.vmware_hw = l;
          if (!xlu_cfg_get_long(config, "timer_mode", &l, 1)) {
              const char *s = libxl_timer_mode_to_string(l);
              fprintf(stderr, "WARNING: specifying \"timer_mode\" as an integer is 
deprecated. "
@@ -1676,13 +1678,20 @@ skip_vfb:
                  b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
              } else if (!strcmp(buf, "none")) {
                  b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
+            } else if (!strcmp(buf, "vmware")) {
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_VMWARE;
              } else {
                  fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
          } else if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
              b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
-                                         LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+                b_info->u.hvm.vmware_hw ? LIBXL_VGA_INTERFACE_TYPE_VMWARE :
+                                          LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
I don't think this is a good idea. stdvga = 1 in the config file should
still mean stdvga, not conditionally vmware. Likewise stdvga = 0 should
always be cirrus.

I think you are miss reading this.  For "stdvga = 1", l === 1, so it
is not conditionally vmware.  However for "stdvga = 0" it is conditionally
vmware.  I will drop the "stdvga = 0" conditionally vmware.

And add to the xl.cfg.pod.5 the additional statement that
the deprecated "stdvga=0" prevents the usage of vmware by default.

Someone who wants to force vmware should use vga=vmware and not specify
stdvga at all.

This should work.  If VGA is specified, stdvga is ignored.

(NB: stdvga is deprecated synonym, the man page advises using vga=

+        else
+            b_info->u.hvm.vga.kind =
+                b_info->u.hvm.vmware_hw ? LIBXL_VGA_INTERFACE_TYPE_VMWARE :
+                                          LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
This else clause shouldn't be here, update
libxl__domain_build_info_setdefault instead where it currently says:
         if (!b_info->u.hvm.vga.kind)
             b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;

note that this code should only set vga.kind if it is currently zero
(which indicates to libxl "pick a default")

Will do.

   -Don Slutz

xlu_cfg_replace_string (config, "keymap", &b_info->u.hvm.keymap, 0);
          xlu_cfg_get_defbool (config, "spice", &b_info->u.hvm.spice.enable, 0);

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.