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

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

On 13/01/15 18:50, Ed White wrote:
> On 01/12/2015 05:06 AM, Andrew Cooper wrote:
>> 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.
>>
> I definitely can change the default to off, but I don't think it will
> have the effect you're expecting.
>
> This patch simply determines whether the hardware supports enabling
> VMFUNC and #VE, but does not enable them. If a domain enters
> alternate p2m mode through the relevant hypercall, at that point
> VMFUNC will be enabled for vcpu's in that domain; and if a vcpu in
> that domain subsequently registers itself to receive #VE through
> another hypercall, #VE will be enabled for that vcpu. Since both
> features are emulated if the hardware doesn't support them, changing
> the default to off will simply force emulation.

Now you mention this, what feature flag should a VM look for to indicate
the availability of vmfunc?

Looking at the manual, it would appear that guest software's only method
of detecting the absence of support is to attempt the instruction can
catch a #UD.  (I also observe that vmfunc 0 has no cpl0 requirements as
described by its pseudocode.)

One way or another the domain needs something akin to a feature flag. 
While I am loathe to suggest it, I think you need two hvm params to
control this.

One HVM param should probably match the vm-function controls, and
identifies which functions are permitted for use, independent of
hardware support vs emulation.  A missing bit here will cause emulated
attempts to fail with #UD.

A second hvmparam should identify the altp2m mode, as one of {off,
identity, unrestricted}

This allows the host admin quite fine grain control over the options
available including absolutely nothing, out-of-guest-only altp2m,
identity altp2m which plays nicely with most other Xen features, or the
full set.  By explicitly choosing the full set, the host admin has to
take a conscious choice to be incompatible with features such as
passthrough and migration.

~Andrew