Re: [PATCH 01/11] VMX: VMFUNC and #VE definitions and detection.

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 09/01/15 21:26, Ed White wrote:
> Currently, neither is enabled globally but may be enabled on a per-VCPU
> basis by the altp2m code.
>
> Everything can be force-disabled globally by specifying vmfunc=0 on the
> Xen command line.
>
> Remove the check for EPTE bit 63 == zero in ept_split_super_page(), as
> that bit is now hardware-defined.
>
> Signed-off-by: Ed White <edmund.h.white@intel.com>
> ---
>  docs/misc/xen-command-line.markdown |  7 +++++++
>  xen/arch/x86/hvm/vmx/vmcs.c         | 40 +++++++++++++++++++++++++++++++++++++
>  xen/arch/x86/mm/p2m-ept.c           |  1 -
>  xen/include/asm-x86/hvm/vmx/vmcs.h  | 16 +++++++++++++++
>  xen/include/asm-x86/hvm/vmx/vmx.h   | 13 +++++++++++-
>  xen/include/asm-x86/msr-index.h     |  1 +
>  6 files changed, 76 insertions(+), 2 deletions(-)
>
> diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
> index 152ae03..00fbae7 100644
> --- a/docs/misc/xen-command-line.markdown
> +++ b/docs/misc/xen-command-line.markdown
> @@ -1305,6 +1305,13 @@ The optional `keep` parameter causes Xen to continue using the vga
>  console even after dom0 has been started.  The default behaviour is to
>  relinquish control to dom0.
>  
> +### vmfunc (Intel)
> +> `= <boolean>`
> +
> +> Default: `true`
> +
> +Use VMFUNC and #VE support if available.
> +
>  ### vpid (Intel)
>  > `= <boolean>`
>  
> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
> index 9d8033e..4274e92 100644
> --- a/xen/arch/x86/hvm/vmx/vmcs.c
> +++ b/xen/arch/x86/hvm/vmx/vmcs.c
> @@ -50,6 +50,9 @@ boolean_param("unrestricted_guest", opt_unrestricted_guest_enabled);
>  static bool_t __read_mostly opt_apicv_enabled = 1;
>  boolean_param("apicv", opt_apicv_enabled);
>  
> +static bool_t __read_mostly opt_vmfunc_enabled = 1;
> +boolean_param("vmfunc", opt_vmfunc_enabled);

Please can experimental features be off by default.  (I am specifically
looking to avoid the issues we had with apicv getting into stable
releases despite reliably causing problems for migration).

I suspect you will have many interested testers for this featureset, and
it is fine to patch the default later when the feature gets declared stable.

I also wonder whether it might be better to have a "vmx=" command line
parameter with "vmfunc" as a subopt, to save gaining an ever increasing
set of related top level parameters?

Other than this, the content of the rest of the patch appears fine.

~Andrew