Re: [PATCH v11 7/9] tools: Add vmware_port support

[George Dunlap <george.dunlap@eu.citrix.com>]

On 05/22/2015 04:50 PM, Don Slutz wrote:
> This new libxl_domain_create_info field is used to set
> XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK in the xc_domain_configuration_t
> for x86.
> 
> In xen it is is_vmware_port_enabled.
> 
> If is_vmware_port_enabled then
>   enable a limited support of VMware's hyper-call.
> 
> VMware's hyper-call is also known as VMware Backdoor I/O Port.
> 
> if vmware_port is not specified in the config file, let
> "vmware_hwver != 0" be the default value.  This means that only
> vmware_hwver = 7 needs to be specified to enable both features.
> 
> vmware_hwver = 7 is special because that is what controls the
> enable of CPUID leaves for VMware (vmware_hwver >= 7).
> 
> Note: vmware_port and nestedhvm cannot be specified at the
> same time.
> 
> Signed-off-by: Don Slutz <dslutz@verizon.com>

Ian:

So I *think* it may be the case that this patch only depends on patch 5
to apply.  I also think that patches 5 and 7 together add another useful
"chunk" of functionality (core vmport functionality for guest OSes).
Patch 5 already has Andy's Reviewed-by (and can have my Ack as well),
and it seemed like previous versions of this patch were close to being
acceptable to you.

So if you wanted to give this a once-over, we could probably apply these
two without much trouble as well.

Then I think this series would also be off your plate. :-)

 -George


> ---
> v11:
>   Dropped "If non-zero then default VGA to VMware's VGA"
> 
> v10:
>     If..." at the start of the sentence ...
>     Also, why is 7 special?
> 
> 
>  docs/man/xl.cfg.pod.5       | 15 +++++++++++++++
>  tools/libxl/libxl.h         |  5 +++++
>  tools/libxl/libxl_create.c  |  9 +++++++++
>  tools/libxl/libxl_types.idl |  1 +
>  tools/libxl/libxl_x86.c     |  2 ++
>  tools/libxl/xl_cmdimpl.c    |  1 +
>  6 files changed, 33 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index eaad4bf..00aa78f 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1354,6 +1354,8 @@ 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.
>  
> +If not zero it changes the default for vmware_port to on.
> +
>  The hardware version number (vmware_hwver) come from VMware config files.
>  
>  =over 4
> @@ -1365,6 +1367,19 @@ For vssd:VirtualSystemType == vmx-07, vmware_hwver = 7.
>  
>  =back
>  
> +=item B<vmware_port=BOOLEAN>
> +
> +Turns on or off the exposure of VMware port.  This is known as
> +vmport in QEMU.  Also called VMware Backdoor I/O Port.  Not all
> +defined VMware backdoor commands are implemented.  All of the
> +ones that Linux kernel uses are defined.
> +
> +Defaults to enabled if vmware_hwver is non-zero (i.e. enabled)
> +otherwise defaults to disabled.
> +
> +Note: vmware_port and nestedhvm cannot be specified at the
> +same time.
> +
>  =back
>  
>  =head3 Emulated VGA Graphics Device
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> index 86164a7..fcce7c3 100644
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -205,6 +205,11 @@
>  #define LIBXL_HAVE_LIBXL_VGA_INTERFACE_TYPE_VMWARE 1
>  
>  /*
> + * libxl_domain_create_info has the vmware_hwver and vmware_port field.
> + */
> +#define LIBXL_HAVE_CREATEINFO_VMWARE 1
> +
> +/*
>   * libxl ABI compatibility
>   *
>   * The only guarantee which libxl makes regarding ABI compatibility
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 895577f..ac05ecc 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -41,6 +41,7 @@ int libxl__domain_create_info_setdefault(libxl__gc *gc,
>          libxl_defbool_setdefault(&c_info->hap, libxl_defbool_val(c_info->pvh));
>      }
>  
> +    libxl_defbool_setdefault(&c_info->vmware_port, c_info->vmware_hwver != 0);
>      libxl_defbool_setdefault(&c_info->run_hotplug_scripts, true);
>      libxl_defbool_setdefault(&c_info->driver_domain, false);
>  
> @@ -917,6 +918,14 @@ static void initiate_domain_create(libxl__egc *egc,
>      ret = libxl__domain_build_info_setdefault(gc, &d_config->b_info);
>      if (ret) goto error_out;
>  
> +    if (d_config->c_info.type == LIBXL_DOMAIN_TYPE_HVM &&
> +        libxl_defbool_val(d_config->b_info.u.hvm.nested_hvm) &&
> +        libxl_defbool_val(d_config->c_info.vmware_port)) {
> +        LOG(ERROR,
> +            "vmware_port and nestedhvm cannot be enabled simultaneously\n");
> +        ret = ERROR_INVAL;
> +        goto error_out;
> +    }
>      if (!sched_params_valid(gc, domid, &d_config->b_info.sched_params)) {
>          LOG(ERROR, "Invalid scheduling parameters\n");
>          ret = ERROR_INVAL;
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index c8a1345..c7af74b 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -344,6 +344,7 @@ libxl_domain_create_info = Struct("domain_create_info",[
>      ("pvh",          libxl_defbool),
>      ("driver_domain",libxl_defbool),
>      ("vmware_hwver", uint64),
> +    ("vmware_port",  libxl_defbool),
>      ], 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 fd7dafa..404904a 100644
> --- a/tools/libxl/libxl_x86.c
> +++ b/tools/libxl/libxl_x86.c
> @@ -6,6 +6,8 @@ int libxl__arch_domain_prepare_config(libxl__gc *gc,
>                                        xc_domain_configuration_t *xc_config)
>  {
>      xc_config->vmware_hwver = d_config->c_info.vmware_hwver;
> +    if (libxl_defbool_val(d_config->c_info.vmware_port))
> +        xc_config->arch_flags |= XEN_DOMCTL_CONFIG_VMWARE_PORT_MASK;
>      return 0;
>  }
>  
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index e79a9d0..b3fe0cd 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1230,6 +1230,7 @@ static void parse_config_data(const char *config_source,
>      }
>  
>      xlu_cfg_get_defbool(config, "oos", &c_info->oos, 0);
> +    xlu_cfg_get_defbool(config, "vmware_port", &c_info->vmware_port, 0);
>  
>      if (!xlu_cfg_get_string (config, "pool", &buf, 0))
>          xlu_cfg_replace_string(config, "pool", &c_info->pool_name, 0);
>