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

2015-01-14 Thread Andrew Cooper
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 
>>> ---
>>>  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)
>>> +> `= `
>>> +
>>> +> Default: `true`
>>> +
>>> +Use VMFUNC and #VE support if available.
>>> +
>>>  ### vpid (Intel)
>>>  > `= `
>>>  
>>> 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


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

2015-01-13 Thread Ed White
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 
>> ---
>>  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)
>> +> `= `
>> +
>> +> Default: `true`
>> +
>> +Use VMFUNC and #VE support if available.
>> +
>>  ### vpid (Intel)
>>  > `= `
>>  
>> 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.

In its current state, I suspect that this patch series will cause
problems for migration either way, as I noted in the cover letter.

As regards making vmfunc a subopt of vmx: I can look into that, but
then what happens if AMD implements vmfunc?

Ed


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


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

2015-01-12 Thread Andrew Cooper
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 
> ---
>  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)
> +> `= `
> +
> +> Default: `true`
> +
> +Use VMFUNC and #VE support if available.
> +
>  ### vpid (Intel)
>  > `= `
>  
> 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


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel


[Xen-devel] [PATCH 01/11] VMX: VMFUNC and #VE definitions and detection.

2015-01-09 Thread Ed White
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 
---
 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)
+> `= `
+
+> Default: `true`
+
+Use VMFUNC and #VE support if available.
+
 ### vpid (Intel)
 > `= `
 
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);
+
 /*
  * These two parameters are used to config the controls for Pause-Loop Exiting:
  * ple_gap:upper bound on the amount of time between two successive
@@ -71,6 +74,8 @@ u32 vmx_secondary_exec_control __read_mostly;
 u32 vmx_vmexit_control __read_mostly;
 u32 vmx_vmentry_control __read_mostly;
 u64 vmx_ept_vpid_cap __read_mostly;
+u64 vmx_vmfunc __read_mostly;
+bool_t vmx_virt_exception __read_mostly;
 
 const u32 vmx_introspection_force_enabled_msrs[] = {
 MSR_IA32_SYSENTER_EIP,
@@ -110,6 +115,8 @@ static void __init vmx_display_features(void)
 P(cpu_has_vmx_virtual_intr_delivery, "Virtual Interrupt Delivery");
 P(cpu_has_vmx_posted_intr_processing, "Posted Interrupt Processing");
 P(cpu_has_vmx_vmcs_shadowing, "VMCS shadowing");
+P(cpu_has_vmx_vmfunc, "VM Functions");
+P(cpu_has_vmx_virt_exceptions, "Virtualization Exceptions");
 #undef P
 
 if ( !printed )
@@ -154,6 +161,7 @@ static int vmx_init_vmcs_config(void)
 u64 _vmx_misc_cap = 0;
 u32 _vmx_vmexit_control;
 u32 _vmx_vmentry_control;
+u64 _vmx_vmfunc = 0;
 bool_t mismatch = 0;
 
 rdmsr(MSR_IA32_VMX_BASIC, vmx_basic_msr_low, vmx_basic_msr_high);
@@ -207,6 +215,9 @@ static int vmx_init_vmcs_config(void)
 opt |= SECONDARY_EXEC_ENABLE_VPID;
 if ( opt_unrestricted_guest_enabled )
 opt |= SECONDARY_EXEC_UNRESTRICTED_GUEST;
+if ( opt_vmfunc_enabled )
+opt |= SECONDARY_EXEC_ENABLE_VM_FUNCTIONS |
+   SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS;
 
 /*
  * "APIC Register Virtualization" and "Virtual Interrupt Delivery"
@@ -296,6 +307,24 @@ static int vmx_init_vmcs_config(void)
   || !(_vmx_vmexit_control & VM_EXIT_ACK_INTR_ON_EXIT) )
 _vmx_pin_based_exec_control  &= ~ PIN_BASED_POSTED_INTERRUPT;
 
+/* The IA32_VMX_VMFUNC MSR exists only when VMFUNC is available */
+if ( _vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS )
+{
+rdmsrl(MSR_IA32_VMX_VMFUNC, _vmx_vmfunc);
+
+/*
+ * VMFUNC leaf 0 (EPTP switching) must be supported.
+ *
+ * Or we just don't use VMFUNC.
+ */
+if ( !(_vmx_vmfunc & VMX_VMFUNC_EPTP_SWITCHING) )
+_vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VM_FUNCTIONS;
+}
+
+/* Virtualization exceptions are only enabled if VMFUNC is enabled */
+if ( !(_vmx_secondary_exec_control & SECONDARY_EXEC_ENABLE_VM_FUNCTIONS) )
+_vmx_secondary_exec_control &= ~SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS;
+
 min = 0;
 opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS;
 _vmx_vmentry_control = adjust_vmx_controls(
@@ -316,6 +345,9 @@ static int vmx_init_vmcs_config(void)
 vmx_vmentry_control= _vmx_vmentry_control;
 vmx_basic_msr  = ((u64)vmx_basic_msr_high << 32) |
  vmx_basic_msr_low;
+vmx_vmfunc = _vmx_vmfunc;
+vmx_virt_exception = !!(_vmx_secondary_exec_control &
+   SECONDARY_EXEC_ENABLE_VIRT_EXCEPTIONS);
 vmx_display_features();
 
 /* IA-32 SDM Vol 3B: VMCS size is never greater than 4kB. */
@@ -352,6 +384,9 @@ static int vmx_init_vmcs_config(void)
 mismatch |=