Re: [Xen-devel] [PATCH 01/11] VMX: VMFUNC and #VE definitions and detection.
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.
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.
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.
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 |=