From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 2001
From: Liu Jinsong <jinsong.liu@intel.com>
Date: Thu, 28 Nov 2013 05:26:06 +0800
Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle
When MPX supported, a new guest-state field for IA32_BNDCFGS
is added to the VMCS. In addition, two new controls are added:
- a VM-exit control called "clear BNDCFGS"
- a VM-entry control called "load BNDCFGS."
VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS.
Signed-off-by: Xudong Hao <xudong.hao@intel.com>
Reviewed-by: Liu Jinsong <jinsong.liu@intel.com>
Unlikely, but in case VMX support is not available, not expose
MPX to hvm guest.
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Suggested-by: Jan Beulich <jbeulich@suse.com>
Signed-off-by: Liu Jinsong <jinsong.liu@intel.com>
---
xen/arch/x86/hvm/hvm.c | 6 ++++++
xen/arch/x86/hvm/vmx/vmcs.c | 8 ++++++--
xen/include/asm-x86/cpufeature.h | 2 ++
xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++
xen/include/asm-x86/msr-index.h | 2 ++
5 files changed, 18 insertions(+), 2 deletions(-)
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 9c88c73..0f7178b 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -2905,6 +2905,12 @@ void hvm_cpuid(unsigned int input, unsigned int *eax,
unsigned int *ebx,
if ( (count == 0) && !cpu_has_smep )
*ebx &= ~cpufeat_mask(X86_FEATURE_SMEP);
+ /* Don''t expose MPX to hvm when VMX support is not available
*/
+ if ( (count == 0) &&
+ (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
+ !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) )
+ *ebx &= ~cpufeat_mask(X86_FEATURE_MPX);
+
/* Don''t expose INVPCID to non-hap hvm. */
if ( (count == 0) && !hap_enabled(d) )
*ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID);
diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c
index 290b42f..4a1f168 100644
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -270,7 +270,8 @@ static int vmx_init_vmcs_config(void)
}
min = VM_EXIT_ACK_INTR_ON_EXIT;
- opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT;
+ opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT |
+ VM_EXIT_CLEAR_BNDCFGS;
min |= VM_EXIT_IA32E_MODE;
_vmx_vmexit_control = adjust_vmx_controls(
"VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS,
&mismatch);
@@ -284,7 +285,7 @@ static int vmx_init_vmcs_config(void)
_vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT;
min = 0;
- opt = VM_ENTRY_LOAD_GUEST_PAT;
+ opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS;
_vmx_vmentry_control = adjust_vmx_controls(
"VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS,
&mismatch);
@@ -955,6 +956,9 @@ static int construct_vmcs(struct vcpu *v)
vmx_disable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP, MSR_TYPE_R |
MSR_TYPE_W);
if ( paging_mode_hap(d) && (!iommu_enabled || iommu_snoop) )
vmx_disable_intercept_for_msr(v, MSR_IA32_CR_PAT, MSR_TYPE_R |
MSR_TYPE_W);
+ if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) &&
+ (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) )
+ vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS, MSR_TYPE_R |
MSR_TYPE_W);
}
/* I/O access bitmap. */
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index 1cfaf94..930dc9b 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -148,6 +148,7 @@
#define X86_FEATURE_INVPCID (7*32+10) /* Invalidate Process Context ID */
#define X86_FEATURE_RTM (7*32+11) /* Restricted Transactional Memory */
#define X86_FEATURE_NO_FPU_SEL (7*32+13) /* FPU CS/DS stored as zero */
+#define X86_FEATURE_MPX (7*32+14) /* Memory Protection Extensions */
#define X86_FEATURE_SMAP (7*32+20) /* Supervisor Mode Access Prevention */
#define cpu_has(c, bit) test_bit(bit, (c)->x86_capability)
@@ -197,6 +198,7 @@
#define cpu_has_xsave boot_cpu_has(X86_FEATURE_XSAVE)
#define cpu_has_avx boot_cpu_has(X86_FEATURE_AVX)
#define cpu_has_lwp boot_cpu_has(X86_FEATURE_LWP)
+#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX)
#define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON)
diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h
b/xen/include/asm-x86/hvm/vmx/vmcs.h
index ebaba5c..75cd653 100644
--- a/xen/include/asm-x86/hvm/vmx/vmcs.h
+++ b/xen/include/asm-x86/hvm/vmx/vmcs.h
@@ -186,6 +186,7 @@ extern u32 vmx_pin_based_exec_control;
#define VM_EXIT_SAVE_GUEST_EFER 0x00100000
#define VM_EXIT_LOAD_HOST_EFER 0x00200000
#define VM_EXIT_SAVE_PREEMPT_TIMER 0x00400000
+#define VM_EXIT_CLEAR_BNDCFGS 0x00800000
extern u32 vmx_vmexit_control;
#define VM_ENTRY_IA32E_MODE 0x00000200
@@ -194,6 +195,7 @@ extern u32 vmx_vmexit_control;
#define VM_ENTRY_LOAD_PERF_GLOBAL_CTRL 0x00002000
#define VM_ENTRY_LOAD_GUEST_PAT 0x00004000
#define VM_ENTRY_LOAD_GUEST_EFER 0x00008000
+#define VM_ENTRY_LOAD_BNDCFGS 0x00010000
extern u32 vmx_vmentry_control;
#define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001
diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h
index e597a28..ccad1ab 100644
--- a/xen/include/asm-x86/msr-index.h
+++ b/xen/include/asm-x86/msr-index.h
@@ -56,6 +56,8 @@
#define MSR_IA32_DS_AREA 0x00000600
#define MSR_IA32_PERF_CAPABILITIES 0x00000345
+#define MSR_IA32_BNDCFGS 0x00000D90
+
#define MSR_MTRRfix64K_00000 0x00000250
#define MSR_MTRRfix16K_80000 0x00000258
#define MSR_MTRRfix16K_A0000 0x00000259
--
1.7.1
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
On 27/11/13 13:50, Liu, Jinsong wrote:> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 2001 > From: Liu Jinsong <jinsong.liu@intel.com> > Date: Thu, 28 Nov 2013 05:26:06 +0800 > Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle > > When MPX supported, a new guest-state field for IA32_BNDCFGS > is added to the VMCS. In addition, two new controls are added: > - a VM-exit control called "clear BNDCFGS" > - a VM-entry control called "load BNDCFGS." > VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. > > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> > > Unlikely, but in case VMX support is not available, not expose > MPX to hvm guest.You are still missing the point. I as the administrator choose to prevent an HVM guest from using MPX. Perhaps I want to create a heterogeneous pool. Therefore, the bit is disabled in the domains cpuid policy, despite being available on the hardware. ~Andrew> > Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> > --- > xen/arch/x86/hvm/hvm.c | 6 ++++++ > xen/arch/x86/hvm/vmx/vmcs.c | 8 ++++++-- > xen/include/asm-x86/cpufeature.h | 2 ++ > xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++ > xen/include/asm-x86/msr-index.h | 2 ++ > 5 files changed, 18 insertions(+), 2 deletions(-) > > diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c > index 9c88c73..0f7178b 100644 > --- a/xen/arch/x86/hvm/hvm.c > +++ b/xen/arch/x86/hvm/hvm.c > @@ -2905,6 +2905,12 @@ void hvm_cpuid(unsigned int input, unsigned int *eax, unsigned int *ebx, > if ( (count == 0) && !cpu_has_smep ) > *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP); > > + /* Don''t expose MPX to hvm when VMX support is not available */ > + if ( (count == 0) && > + (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) || > + !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) ) > + *ebx &= ~cpufeat_mask(X86_FEATURE_MPX); > + > /* Don''t expose INVPCID to non-hap hvm. */ > if ( (count == 0) && !hap_enabled(d) ) > *ebx &= ~cpufeat_mask(X86_FEATURE_INVPCID); > diff --git a/xen/arch/x86/hvm/vmx/vmcs.c b/xen/arch/x86/hvm/vmx/vmcs.c > index 290b42f..4a1f168 100644 > --- a/xen/arch/x86/hvm/vmx/vmcs.c > +++ b/xen/arch/x86/hvm/vmx/vmcs.c > @@ -270,7 +270,8 @@ static int vmx_init_vmcs_config(void) > } > > min = VM_EXIT_ACK_INTR_ON_EXIT; > - opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT; > + opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT | > + VM_EXIT_CLEAR_BNDCFGS; > min |= VM_EXIT_IA32E_MODE; > _vmx_vmexit_control = adjust_vmx_controls( > "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS, &mismatch); > @@ -284,7 +285,7 @@ static int vmx_init_vmcs_config(void) > _vmx_pin_based_exec_control &= ~ PIN_BASED_POSTED_INTERRUPT; > > min = 0; > - opt = VM_ENTRY_LOAD_GUEST_PAT; > + opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS; > _vmx_vmentry_control = adjust_vmx_controls( > "VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS, &mismatch); > > @@ -955,6 +956,9 @@ static int construct_vmcs(struct vcpu *v) > vmx_disable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP, MSR_TYPE_R | MSR_TYPE_W); > if ( paging_mode_hap(d) && (!iommu_enabled || iommu_snoop) ) > vmx_disable_intercept_for_msr(v, MSR_IA32_CR_PAT, MSR_TYPE_R | MSR_TYPE_W); > + if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) && > + (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) ) > + vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS, MSR_TYPE_R | MSR_TYPE_W); > } > > /* I/O access bitmap. */ > diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h > index 1cfaf94..930dc9b 100644 > --- a/xen/include/asm-x86/cpufeature.h > +++ b/xen/include/asm-x86/cpufeature.h > @@ -148,6 +148,7 @@ > #define X86_FEATURE_INVPCID (7*32+10) /* Invalidate Process Context ID */ > #define X86_FEATURE_RTM (7*32+11) /* Restricted Transactional Memory */ > #define X86_FEATURE_NO_FPU_SEL (7*32+13) /* FPU CS/DS stored as zero */ > +#define X86_FEATURE_MPX (7*32+14) /* Memory Protection Extensions */ > #define X86_FEATURE_SMAP (7*32+20) /* Supervisor Mode Access Prevention */ > > #define cpu_has(c, bit) test_bit(bit, (c)->x86_capability) > @@ -197,6 +198,7 @@ > #define cpu_has_xsave boot_cpu_has(X86_FEATURE_XSAVE) > #define cpu_has_avx boot_cpu_has(X86_FEATURE_AVX) > #define cpu_has_lwp boot_cpu_has(X86_FEATURE_LWP) > +#define cpu_has_mpx boot_cpu_has(X86_FEATURE_MPX) > > #define cpu_has_arch_perfmon boot_cpu_has(X86_FEATURE_ARCH_PERFMON) > > diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h b/xen/include/asm-x86/hvm/vmx/vmcs.h > index ebaba5c..75cd653 100644 > --- a/xen/include/asm-x86/hvm/vmx/vmcs.h > +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h > @@ -186,6 +186,7 @@ extern u32 vmx_pin_based_exec_control; > #define VM_EXIT_SAVE_GUEST_EFER 0x00100000 > #define VM_EXIT_LOAD_HOST_EFER 0x00200000 > #define VM_EXIT_SAVE_PREEMPT_TIMER 0x00400000 > +#define VM_EXIT_CLEAR_BNDCFGS 0x00800000 > extern u32 vmx_vmexit_control; > > #define VM_ENTRY_IA32E_MODE 0x00000200 > @@ -194,6 +195,7 @@ extern u32 vmx_vmexit_control; > #define VM_ENTRY_LOAD_PERF_GLOBAL_CTRL 0x00002000 > #define VM_ENTRY_LOAD_GUEST_PAT 0x00004000 > #define VM_ENTRY_LOAD_GUEST_EFER 0x00008000 > +#define VM_ENTRY_LOAD_BNDCFGS 0x00010000 > extern u32 vmx_vmentry_control; > > #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001 > diff --git a/xen/include/asm-x86/msr-index.h b/xen/include/asm-x86/msr-index.h > index e597a28..ccad1ab 100644 > --- a/xen/include/asm-x86/msr-index.h > +++ b/xen/include/asm-x86/msr-index.h > @@ -56,6 +56,8 @@ > #define MSR_IA32_DS_AREA 0x00000600 > #define MSR_IA32_PERF_CAPABILITIES 0x00000345 > > +#define MSR_IA32_BNDCFGS 0x00000D90 > + > #define MSR_MTRRfix64K_00000 0x00000250 > #define MSR_MTRRfix16K_80000 0x00000258 > #define MSR_MTRRfix16K_A0000 0x00000259
>>> On 27.11.13 at 14:50, "Liu, Jinsong" <jinsong.liu@intel.com> wrote: > From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 2001 > From: Liu Jinsong <jinsong.liu@intel.com> > Date: Thu, 28 Nov 2013 05:26:06 +0800 > Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle > > When MPX supported, a new guest-state field for IA32_BNDCFGS > is added to the VMCS. In addition, two new controls are added: > - a VM-exit control called "clear BNDCFGS" > - a VM-entry control called "load BNDCFGS." > VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. > > Signed-off-by: Xudong Hao <xudong.hao@intel.com> > Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> > > Unlikely, but in case VMX support is not available, not expose > MPX to hvm guest.Good. But this, thinking about it once more, still doesn''t eliminate the need to handle save/restore of the MSR - the VMCS itself isn''t part of the data that''s being migrated, and hence the MSR value needs to be pulled out of it just like vmx_save_vmcs_ctxt() does for various other MSRs. This, btw., would seem to also be missing for at least MSR_CORE_PERF_GLOBAL_CTRL in case vPMU is enabled. Jan
>>> On 27.11.13 at 14:57, Andrew Cooper <andrew.cooper3@citrix.com> wrote: > On 27/11/13 13:50, Liu, Jinsong wrote: >> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 2001 >> From: Liu Jinsong <jinsong.liu@intel.com> >> Date: Thu, 28 Nov 2013 05:26:06 +0800 >> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >> >> When MPX supported, a new guest-state field for IA32_BNDCFGS >> is added to the VMCS. In addition, two new controls are added: >> - a VM-exit control called "clear BNDCFGS" >> - a VM-entry control called "load BNDCFGS." >> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >> >> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >> >> Unlikely, but in case VMX support is not available, not expose >> MPX to hvm guest. > > You are still missing the point. > > I as the administrator choose to prevent an HVM guest from using MPX. > Perhaps I want to create a heterogeneous pool. > > Therefore, the bit is disabled in the domains cpuid policy, despite > being available on the hardware.I think that with this latest version of the patch this is of no concern anymore, or if it was it''d require a per-guest VM entry control setting rather than a global one. Jan
Andrew Cooper wrote:> On 27/11/13 13:50, Liu, Jinsong wrote: >> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 >> 2001 >> From: Liu Jinsong <jinsong.liu@intel.com> >> Date: Thu, 28 Nov 2013 05:26:06 +0800 >> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >> >> When MPX supported, a new guest-state field for IA32_BNDCFGS >> is added to the VMCS. In addition, two new controls are added: >> - a VM-exit control called "clear BNDCFGS" >> - a VM-entry control called "load BNDCFGS." >> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >> >> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >> >> Unlikely, but in case VMX support is not available, not expose >> MPX to hvm guest. > > You are still missing the point. > > I as the administrator choose to prevent an HVM guest from using MPX. > Perhaps I want to create a heterogeneous pool. > > Therefore, the bit is disabled in the domains cpuid policy, despite > being available on the hardware. > > ~Andrew >Could you tell me the reason why choose to prevent HVM from using MPX? Thanks, Jinsong>> >> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >> Suggested-by: Jan Beulich <jbeulich@suse.com> >> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> >> --- >> xen/arch/x86/hvm/hvm.c | 6 ++++++ >> xen/arch/x86/hvm/vmx/vmcs.c | 8 ++++++-- >> xen/include/asm-x86/cpufeature.h | 2 ++ >> xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++ >> xen/include/asm-x86/msr-index.h | 2 ++ >> 5 files changed, 18 insertions(+), 2 deletions(-) >> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >> index 9c88c73..0f7178b 100644 >> --- a/xen/arch/x86/hvm/hvm.c >> +++ b/xen/arch/x86/hvm/hvm.c >> @@ -2905,6 +2905,12 @@ void hvm_cpuid(unsigned int input, unsigned >> int *eax, unsigned int *ebx, if ( (count == 0) && >> !cpu_has_smep ) *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP); >> >> + /* Don''t expose MPX to hvm when VMX support is not >> available */ + if ( (count == 0) && + >> (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) || + >> !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) ) + *ebx >> &= ~cpufeat_mask(X86_FEATURE_MPX); + /* Don''t expose >> INVPCID to non-hap hvm. */ if ( (count == 0) && >> !hap_enabled(d) ) *ebx &>> ~cpufeat_mask(X86_FEATURE_INVPCID); >> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c >> b/xen/arch/x86/hvm/vmx/vmcs.c >> index 290b42f..4a1f168 100644 >> --- a/xen/arch/x86/hvm/vmx/vmcs.c >> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >> @@ -270,7 +270,8 @@ static int vmx_init_vmcs_config(void) } >> >> min = VM_EXIT_ACK_INTR_ON_EXIT; >> - opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT; >> + opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT | >> + VM_EXIT_CLEAR_BNDCFGS; >> min |= VM_EXIT_IA32E_MODE; >> _vmx_vmexit_control = adjust_vmx_controls( >> "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS, >> &mismatch); @@ -284,7 +285,7 @@ static int vmx_init_vmcs_config(void) >> _vmx_pin_based_exec_control &= ~ >> PIN_BASED_POSTED_INTERRUPT; >> >> min = 0; >> - opt = VM_ENTRY_LOAD_GUEST_PAT; >> + opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS; >> _vmx_vmentry_control = adjust_vmx_controls( >> "VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS, >> &mismatch); >> >> @@ -955,6 +956,9 @@ static int construct_vmcs(struct vcpu *v) >> vmx_disable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP, >> MSR_TYPE_R | MSR_TYPE_W); if ( paging_mode_hap(d) && >> (!iommu_enabled || iommu_snoop) ) >> vmx_disable_intercept_for_msr(v, MSR_IA32_CR_PAT, MSR_TYPE_R | >> MSR_TYPE_W); + if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) && + >> (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) ) + >> vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS, MSR_TYPE_R | >> MSR_TYPE_W); } >> >> /* I/O access bitmap. */ >> diff --git a/xen/include/asm-x86/cpufeature.h >> b/xen/include/asm-x86/cpufeature.h >> index 1cfaf94..930dc9b 100644 >> --- a/xen/include/asm-x86/cpufeature.h >> +++ b/xen/include/asm-x86/cpufeature.h >> @@ -148,6 +148,7 @@ >> #define X86_FEATURE_INVPCID (7*32+10) /* Invalidate Process Context >> ID */ #define X86_FEATURE_RTM (7*32+11) /* Restricted >> Transactional Memory */ #define X86_FEATURE_NO_FPU_SEL (7*32+13) >> /* FPU CS/DS stored as zero */ +#define X86_FEATURE_MPX (7*32+14) >> /* Memory Protection Extensions */ #define >> X86_FEATURE_SMAP (7*32+20) /* Supervisor Mode Access Prevention */ >> >> #define cpu_has(c, bit) test_bit(bit, (c)->x86_capability) @@ >> -197,6 +198,7 @@ #define cpu_has_xsave >> boot_cpu_has(X86_FEATURE_XSAVE) #define cpu_has_avx >> boot_cpu_has(X86_FEATURE_AVX) #define cpu_has_lwp >> boot_cpu_has(X86_FEATURE_LWP) +#define cpu_has_mpx >> boot_cpu_has(X86_FEATURE_MPX) >> >> #define cpu_has_arch_perfmon >> boot_cpu_has(X86_FEATURE_ARCH_PERFMON) >> >> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h >> b/xen/include/asm-x86/hvm/vmx/vmcs.h >> index ebaba5c..75cd653 100644 >> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h >> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h >> @@ -186,6 +186,7 @@ extern u32 vmx_pin_based_exec_control; >> #define VM_EXIT_SAVE_GUEST_EFER 0x00100000 >> #define VM_EXIT_LOAD_HOST_EFER 0x00200000 >> #define VM_EXIT_SAVE_PREEMPT_TIMER 0x00400000 >> +#define VM_EXIT_CLEAR_BNDCFGS 0x00800000 >> extern u32 vmx_vmexit_control; >> >> #define VM_ENTRY_IA32E_MODE 0x00000200 >> @@ -194,6 +195,7 @@ extern u32 vmx_vmexit_control; >> #define VM_ENTRY_LOAD_PERF_GLOBAL_CTRL 0x00002000 >> #define VM_ENTRY_LOAD_GUEST_PAT 0x00004000 >> #define VM_ENTRY_LOAD_GUEST_EFER 0x00008000 >> +#define VM_ENTRY_LOAD_BNDCFGS 0x00010000 >> extern u32 vmx_vmentry_control; >> >> #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001 >> diff --git a/xen/include/asm-x86/msr-index.h >> b/xen/include/asm-x86/msr-index.h >> index e597a28..ccad1ab 100644 >> --- a/xen/include/asm-x86/msr-index.h >> +++ b/xen/include/asm-x86/msr-index.h >> @@ -56,6 +56,8 @@ >> #define MSR_IA32_DS_AREA 0x00000600 >> #define MSR_IA32_PERF_CAPABILITIES 0x00000345 >> >> +#define MSR_IA32_BNDCFGS 0x00000D90 >> + >> #define MSR_MTRRfix64K_00000 0x00000250 >> #define MSR_MTRRfix16K_80000 0x00000258 >> #define MSR_MTRRfix16K_A0000 0x00000259
On 27/11/13 14:27, Liu, Jinsong wrote:> Andrew Cooper wrote: >> On 27/11/13 13:50, Liu, Jinsong wrote: >>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 >>> 2001 >>> From: Liu Jinsong <jinsong.liu@intel.com> >>> Date: Thu, 28 Nov 2013 05:26:06 +0800 >>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >>> >>> When MPX supported, a new guest-state field for IA32_BNDCFGS >>> is added to the VMCS. In addition, two new controls are added: >>> - a VM-exit control called "clear BNDCFGS" >>> - a VM-entry control called "load BNDCFGS." >>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >>> >>> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >>> >>> Unlikely, but in case VMX support is not available, not expose >>> MPX to hvm guest. >> You are still missing the point. >> >> I as the administrator choose to prevent an HVM guest from using MPX. >> Perhaps I want to create a heterogeneous pool. >> >> Therefore, the bit is disabled in the domains cpuid policy, despite >> being available on the hardware. >> >> ~Andrew >> > Could you tell me the reason why choose to prevent HVM from using MPX? > > Thanks, > JinsongFor exactly the case I gave - a VM in a heterogeneous pool where one server supports MPX and the other is lacking the MPX feature. ~Andrew> >>> Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com> >>> Suggested-by: Jan Beulich <jbeulich@suse.com> >>> Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> >>> --- >>> xen/arch/x86/hvm/hvm.c | 6 ++++++ >>> xen/arch/x86/hvm/vmx/vmcs.c | 8 ++++++-- >>> xen/include/asm-x86/cpufeature.h | 2 ++ >>> xen/include/asm-x86/hvm/vmx/vmcs.h | 2 ++ >>> xen/include/asm-x86/msr-index.h | 2 ++ >>> 5 files changed, 18 insertions(+), 2 deletions(-) >>> >>> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c >>> index 9c88c73..0f7178b 100644 >>> --- a/xen/arch/x86/hvm/hvm.c >>> +++ b/xen/arch/x86/hvm/hvm.c >>> @@ -2905,6 +2905,12 @@ void hvm_cpuid(unsigned int input, unsigned >>> int *eax, unsigned int *ebx, if ( (count == 0) && >>> !cpu_has_smep ) *ebx &= ~cpufeat_mask(X86_FEATURE_SMEP); >>> >>> + /* Don''t expose MPX to hvm when VMX support is not >>> available */ + if ( (count == 0) && + >>> (!(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) || + >>> !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS)) ) + *ebx >>> &= ~cpufeat_mask(X86_FEATURE_MPX); + /* Don''t expose >>> INVPCID to non-hap hvm. */ if ( (count == 0) && >>> !hap_enabled(d) ) *ebx &>>> ~cpufeat_mask(X86_FEATURE_INVPCID); >>> diff --git a/xen/arch/x86/hvm/vmx/vmcs.c >>> b/xen/arch/x86/hvm/vmx/vmcs.c >>> index 290b42f..4a1f168 100644 >>> --- a/xen/arch/x86/hvm/vmx/vmcs.c >>> +++ b/xen/arch/x86/hvm/vmx/vmcs.c >>> @@ -270,7 +270,8 @@ static int vmx_init_vmcs_config(void) } >>> >>> min = VM_EXIT_ACK_INTR_ON_EXIT; >>> - opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT; >>> + opt = VM_EXIT_SAVE_GUEST_PAT | VM_EXIT_LOAD_HOST_PAT | >>> + VM_EXIT_CLEAR_BNDCFGS; >>> min |= VM_EXIT_IA32E_MODE; >>> _vmx_vmexit_control = adjust_vmx_controls( >>> "VMExit Control", min, opt, MSR_IA32_VMX_EXIT_CTLS, >>> &mismatch); @@ -284,7 +285,7 @@ static int vmx_init_vmcs_config(void) >>> _vmx_pin_based_exec_control &= ~ >>> PIN_BASED_POSTED_INTERRUPT; >>> >>> min = 0; >>> - opt = VM_ENTRY_LOAD_GUEST_PAT; >>> + opt = VM_ENTRY_LOAD_GUEST_PAT | VM_ENTRY_LOAD_BNDCFGS; >>> _vmx_vmentry_control = adjust_vmx_controls( >>> "VMEntry Control", min, opt, MSR_IA32_VMX_ENTRY_CTLS, >>> &mismatch); >>> >>> @@ -955,6 +956,9 @@ static int construct_vmcs(struct vcpu *v) >>> vmx_disable_intercept_for_msr(v, MSR_IA32_SYSENTER_EIP, >>> MSR_TYPE_R | MSR_TYPE_W); if ( paging_mode_hap(d) && >>> (!iommu_enabled || iommu_snoop) ) >>> vmx_disable_intercept_for_msr(v, MSR_IA32_CR_PAT, MSR_TYPE_R | >>> MSR_TYPE_W); + if ( (vmexit_ctl & VM_EXIT_CLEAR_BNDCFGS) && + >>> (vmentry_ctl & VM_ENTRY_LOAD_BNDCFGS) ) + >>> vmx_disable_intercept_for_msr(v, MSR_IA32_BNDCFGS, MSR_TYPE_R | >>> MSR_TYPE_W); } >>> >>> /* I/O access bitmap. */ >>> diff --git a/xen/include/asm-x86/cpufeature.h >>> b/xen/include/asm-x86/cpufeature.h >>> index 1cfaf94..930dc9b 100644 >>> --- a/xen/include/asm-x86/cpufeature.h >>> +++ b/xen/include/asm-x86/cpufeature.h >>> @@ -148,6 +148,7 @@ >>> #define X86_FEATURE_INVPCID (7*32+10) /* Invalidate Process Context >>> ID */ #define X86_FEATURE_RTM (7*32+11) /* Restricted >>> Transactional Memory */ #define X86_FEATURE_NO_FPU_SEL (7*32+13) >>> /* FPU CS/DS stored as zero */ +#define X86_FEATURE_MPX (7*32+14) >>> /* Memory Protection Extensions */ #define >>> X86_FEATURE_SMAP (7*32+20) /* Supervisor Mode Access Prevention */ >>> >>> #define cpu_has(c, bit) test_bit(bit, (c)->x86_capability) @@ >>> -197,6 +198,7 @@ #define cpu_has_xsave >>> boot_cpu_has(X86_FEATURE_XSAVE) #define cpu_has_avx >>> boot_cpu_has(X86_FEATURE_AVX) #define cpu_has_lwp >>> boot_cpu_has(X86_FEATURE_LWP) +#define cpu_has_mpx >>> boot_cpu_has(X86_FEATURE_MPX) >>> >>> #define cpu_has_arch_perfmon >>> boot_cpu_has(X86_FEATURE_ARCH_PERFMON) >>> >>> diff --git a/xen/include/asm-x86/hvm/vmx/vmcs.h >>> b/xen/include/asm-x86/hvm/vmx/vmcs.h >>> index ebaba5c..75cd653 100644 >>> --- a/xen/include/asm-x86/hvm/vmx/vmcs.h >>> +++ b/xen/include/asm-x86/hvm/vmx/vmcs.h >>> @@ -186,6 +186,7 @@ extern u32 vmx_pin_based_exec_control; >>> #define VM_EXIT_SAVE_GUEST_EFER 0x00100000 >>> #define VM_EXIT_LOAD_HOST_EFER 0x00200000 >>> #define VM_EXIT_SAVE_PREEMPT_TIMER 0x00400000 >>> +#define VM_EXIT_CLEAR_BNDCFGS 0x00800000 >>> extern u32 vmx_vmexit_control; >>> >>> #define VM_ENTRY_IA32E_MODE 0x00000200 >>> @@ -194,6 +195,7 @@ extern u32 vmx_vmexit_control; >>> #define VM_ENTRY_LOAD_PERF_GLOBAL_CTRL 0x00002000 >>> #define VM_ENTRY_LOAD_GUEST_PAT 0x00004000 >>> #define VM_ENTRY_LOAD_GUEST_EFER 0x00008000 >>> +#define VM_ENTRY_LOAD_BNDCFGS 0x00010000 >>> extern u32 vmx_vmentry_control; >>> >>> #define SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES 0x00000001 >>> diff --git a/xen/include/asm-x86/msr-index.h >>> b/xen/include/asm-x86/msr-index.h >>> index e597a28..ccad1ab 100644 >>> --- a/xen/include/asm-x86/msr-index.h >>> +++ b/xen/include/asm-x86/msr-index.h >>> @@ -56,6 +56,8 @@ >>> #define MSR_IA32_DS_AREA 0x00000600 >>> #define MSR_IA32_PERF_CAPABILITIES 0x00000345 >>> >>> +#define MSR_IA32_BNDCFGS 0x00000D90 >>> + >>> #define MSR_MTRRfix64K_00000 0x00000250 >>> #define MSR_MTRRfix16K_80000 0x00000258 >>> #define MSR_MTRRfix16K_A0000 0x00000259
Andrew Cooper wrote:> On 27/11/13 14:27, Liu, Jinsong wrote: >> Andrew Cooper wrote: >>> On 27/11/13 13:50, Liu, Jinsong wrote: >>>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 >>>> 2001 From: Liu Jinsong <jinsong.liu@intel.com> >>>> Date: Thu, 28 Nov 2013 05:26:06 +0800 >>>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >>>> >>>> When MPX supported, a new guest-state field for IA32_BNDCFGS >>>> is added to the VMCS. In addition, two new controls are added: >>>> - a VM-exit control called "clear BNDCFGS" >>>> - a VM-entry control called "load BNDCFGS." >>>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >>>> >>>> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >>>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >>>> >>>> Unlikely, but in case VMX support is not available, not expose >>>> MPX to hvm guest. >>> You are still missing the point. >>> >>> I as the administrator choose to prevent an HVM guest from using >>> MPX. Perhaps I want to create a heterogeneous pool. >>> >>> Therefore, the bit is disabled in the domains cpuid policy, despite >>> being available on the hardware. >>> >>> ~Andrew >>> >> Could you tell me the reason why choose to prevent HVM from using >> MPX? >> >> Thanks, >> Jinsong > > For exactly the case I gave - a VM in a heterogeneous pool where one > server supports MPX and the other is lacking the MPX feature. > > ~Andrew >I didn''t see the point of your case to prevent HVM MPX feature. Could you elaborate more of your concern? Thanks, Jinsong
Konrad Rzeszutek Wilk
2013-Nov-27 14:50 UTC
Re: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle
> >>>> Unlikely, but in case VMX support is not available, not expose > >>>> MPX to hvm guest. > >>> You are still missing the point. > >>> > >>> I as the administrator choose to prevent an HVM guest from using > >>> MPX. Perhaps I want to create a heterogeneous pool. > >>> > >>> Therefore, the bit is disabled in the domains cpuid policy, despite > >>> being available on the hardware. > >>> > >>> ~Andrew > >>> > >> Could you tell me the reason why choose to prevent HVM from using > >> MPX? > >> > >> Thanks, > >> Jinsong > > > > For exactly the case I gave - a VM in a heterogeneous pool where one > > server supports MPX and the other is lacking the MPX feature. > > > > ~Andrew > > > > I didn''t see the point of your case to prevent HVM MPX feature. > Could you elaborate more of your concern?B/c the other nodes (that you migrate to) don''t support the MPX and the guest will crash when it tries to use it.
On 27/11/13 14:37, Liu, Jinsong wrote:> Andrew Cooper wrote: >> On 27/11/13 14:27, Liu, Jinsong wrote: >>> Andrew Cooper wrote: >>>> On 27/11/13 13:50, Liu, Jinsong wrote: >>>>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 >>>>> 2001 From: Liu Jinsong <jinsong.liu@intel.com> >>>>> Date: Thu, 28 Nov 2013 05:26:06 +0800 >>>>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >>>>> >>>>> When MPX supported, a new guest-state field for IA32_BNDCFGS >>>>> is added to the VMCS. In addition, two new controls are added: >>>>> - a VM-exit control called "clear BNDCFGS" >>>>> - a VM-entry control called "load BNDCFGS." >>>>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >>>>> >>>>> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >>>>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >>>>> >>>>> Unlikely, but in case VMX support is not available, not expose >>>>> MPX to hvm guest. >>>> You are still missing the point. >>>> >>>> I as the administrator choose to prevent an HVM guest from using >>>> MPX. Perhaps I want to create a heterogeneous pool. >>>> >>>> Therefore, the bit is disabled in the domains cpuid policy, despite >>>> being available on the hardware. >>>> >>>> ~Andrew >>>> >>> Could you tell me the reason why choose to prevent HVM from using >>> MPX? >>> >>> Thanks, >>> Jinsong >> For exactly the case I gave - a VM in a heterogeneous pool where one >> server supports MPX and the other is lacking the MPX feature. >> >> ~Andrew >> > I didn''t see the point of your case to prevent HVM MPX feature. > Could you elaborate more of your concern? > > Thanks, > JinsongIt is very common to have pools of servers made of different generations of CPU. E.g. Ivy Bridge and Haswell. To safely migrate a VM, the feature set the VM can see must be the common subset of the two. ~Andrew
Andrew Cooper wrote:> On 27/11/13 14:37, Liu, Jinsong wrote: >> Andrew Cooper wrote: >>> On 27/11/13 14:27, Liu, Jinsong wrote: >>>> Andrew Cooper wrote: >>>>> On 27/11/13 13:50, Liu, Jinsong wrote: >>>>>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 >>>>>> 2001 From: Liu Jinsong <jinsong.liu@intel.com> >>>>>> Date: Thu, 28 Nov 2013 05:26:06 +0800 >>>>>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >>>>>> >>>>>> When MPX supported, a new guest-state field for IA32_BNDCFGS >>>>>> is added to the VMCS. In addition, two new controls are added: >>>>>> - a VM-exit control called "clear BNDCFGS" >>>>>> - a VM-entry control called "load BNDCFGS." >>>>>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >>>>>> >>>>>> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >>>>>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >>>>>> >>>>>> Unlikely, but in case VMX support is not available, not expose >>>>>> MPX to hvm guest. >>>>> You are still missing the point. >>>>> >>>>> I as the administrator choose to prevent an HVM guest from using >>>>> MPX. Perhaps I want to create a heterogeneous pool. >>>>> >>>>> Therefore, the bit is disabled in the domains cpuid policy, >>>>> despite being available on the hardware. >>>>> >>>>> ~Andrew >>>>> >>>> Could you tell me the reason why choose to prevent HVM from using >>>> MPX? >>>> >>>> Thanks, >>>> Jinsong >>> For exactly the case I gave - a VM in a heterogeneous pool where one >>> server supports MPX and the other is lacking the MPX feature. >>> >>> ~Andrew >>> >> I didn''t see the point of your case to prevent HVM MPX feature. >> Could you elaborate more of your concern? >> >> Thanks, >> Jinsong > > It is very common to have pools of servers made of different > generations of CPU. E.g. Ivy Bridge and Haswell. To safely migrate > a VM, the feature set the VM can see must be the common subset of the > two. > > ~AndrewYes -- but that''s not a reason to prevent MPX feature (or, any new features) -- otherwise you have to prevent any new features. The right place to control cpuid policy of a pool is at higher level, where it has full information of the pool machines and so it''s right place to make decision what cpuid feature set would be proper for the specific pool. Thanks, Jinsong
On 27/11/13 15:02, Liu, Jinsong wrote:> Andrew Cooper wrote: >> On 27/11/13 14:37, Liu, Jinsong wrote: >>> Andrew Cooper wrote: >>>> On 27/11/13 14:27, Liu, Jinsong wrote: >>>>> Andrew Cooper wrote: >>>>>> On 27/11/13 13:50, Liu, Jinsong wrote: >>>>>>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 00:00:00 >>>>>>> 2001 From: Liu Jinsong <jinsong.liu@intel.com> >>>>>>> Date: Thu, 28 Nov 2013 05:26:06 +0800 >>>>>>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >>>>>>> >>>>>>> When MPX supported, a new guest-state field for IA32_BNDCFGS >>>>>>> is added to the VMCS. In addition, two new controls are added: >>>>>>> - a VM-exit control called "clear BNDCFGS" >>>>>>> - a VM-entry control called "load BNDCFGS." >>>>>>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >>>>>>> >>>>>>> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >>>>>>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >>>>>>> >>>>>>> Unlikely, but in case VMX support is not available, not expose >>>>>>> MPX to hvm guest. >>>>>> You are still missing the point. >>>>>> >>>>>> I as the administrator choose to prevent an HVM guest from using >>>>>> MPX. Perhaps I want to create a heterogeneous pool. >>>>>> >>>>>> Therefore, the bit is disabled in the domains cpuid policy, >>>>>> despite being available on the hardware. >>>>>> >>>>>> ~Andrew >>>>>> >>>>> Could you tell me the reason why choose to prevent HVM from using >>>>> MPX? >>>>> >>>>> Thanks, >>>>> Jinsong >>>> For exactly the case I gave - a VM in a heterogeneous pool where one >>>> server supports MPX and the other is lacking the MPX feature. >>>> >>>> ~Andrew >>>> >>> I didn''t see the point of your case to prevent HVM MPX feature. >>> Could you elaborate more of your concern? >>> >>> Thanks, >>> Jinsong >> It is very common to have pools of servers made of different >> generations of CPU. E.g. Ivy Bridge and Haswell. To safely migrate >> a VM, the feature set the VM can see must be the common subset of the >> two. >> >> ~Andrew > Yes -- but that''s not a reason to prevent MPX feature (or, any new features) -- otherwise you have to prevent any new features. > The right place to control cpuid policy of a pool is at higher level, where it has full information of the pool machines and so it''s right place to make decision what cpuid feature set would be proper for the specific pool. > > Thanks, > JinsongThat is exactly a reason to prevent MPX. If the domain cpuid policy (which is set by the toolstack) states that MPX should be disabled, then MPX must be hidden from the HVM guest, even if the hardware supports MPX. ~Andrew
Andrew Cooper wrote:> On 27/11/13 15:02, Liu, Jinsong wrote: >> Andrew Cooper wrote: >>> On 27/11/13 14:37, Liu, Jinsong wrote: >>>> Andrew Cooper wrote: >>>>> On 27/11/13 14:27, Liu, Jinsong wrote: >>>>>> Andrew Cooper wrote: >>>>>>> On 27/11/13 13:50, Liu, Jinsong wrote: >>>>>>>> From 291adaf4ad6174c5641a7239c1801373e92e9975 Mon Sep 17 >>>>>>>> 00:00:00 2001 From: Liu Jinsong <jinsong.liu@intel.com> >>>>>>>> Date: Thu, 28 Nov 2013 05:26:06 +0800 >>>>>>>> Subject: [PATCH 3/4 V3] X86: MPX IA32_BNDCFGS msr handle >>>>>>>> >>>>>>>> When MPX supported, a new guest-state field for IA32_BNDCFGS >>>>>>>> is added to the VMCS. In addition, two new controls are added: >>>>>>>> - a VM-exit control called "clear BNDCFGS" >>>>>>>> - a VM-entry control called "load BNDCFGS." >>>>>>>> VM exits always save IA32_BNDCFGS into BNDCFGS field of VMCS. >>>>>>>> >>>>>>>> Signed-off-by: Xudong Hao <xudong.hao@intel.com> >>>>>>>> Reviewed-by: Liu Jinsong <jinsong.liu@intel.com> >>>>>>>> >>>>>>>> Unlikely, but in case VMX support is not available, not expose >>>>>>>> MPX to hvm guest. >>>>>>> You are still missing the point. >>>>>>> >>>>>>> I as the administrator choose to prevent an HVM guest from using >>>>>>> MPX. Perhaps I want to create a heterogeneous pool. >>>>>>> >>>>>>> Therefore, the bit is disabled in the domains cpuid policy, >>>>>>> despite being available on the hardware. >>>>>>> >>>>>>> ~Andrew >>>>>>> >>>>>> Could you tell me the reason why choose to prevent HVM from >>>>>> using MPX? >>>>>> >>>>>> Thanks, >>>>>> Jinsong >>>>> For exactly the case I gave - a VM in a heterogeneous pool where >>>>> one server supports MPX and the other is lacking the MPX feature. >>>>> >>>>> ~Andrew >>>>> >>>> I didn''t see the point of your case to prevent HVM MPX feature. >>>> Could you elaborate more of your concern? >>>> >>>> Thanks, >>>> Jinsong >>> It is very common to have pools of servers made of different >>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely migrate >>> a VM, the feature set the VM can see must be the common subset of >>> the two. >>> >>> ~Andrew >> Yes -- but that''s not a reason to prevent MPX feature (or, any new >> features) -- otherwise you have to prevent any new features. >> The right place to control cpuid policy of a pool is at higher >> level, where it has full information of the pool machines and so >> it''s right place to make decision what cpuid feature set would be >> proper for the specific pool. >> >> Thanks, >> Jinsong > > That is exactly a reason to prevent MPX. > > If the domain cpuid policy (which is set by the toolstack) states that > MPX should be disabled, then MPX must be hidden from the HVM guest, > even if the hardware supports MPX. > > ~AndrewNo. That''s _not_ a reason to prevent MPX -- toolstack still has the right to disable MPX, no matter h/w support MPX or not. Refer xc_cpuid_set(). Thanks, Jinsong
At 03:17 +0000 on 28 Nov (1385605079), Liu, Jinsong wrote:> Andrew Cooper wrote: > > On 27/11/13 15:02, Liu, Jinsong wrote: > >> Andrew Cooper wrote: > >>> It is very common to have pools of servers made of different > >>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely migrate > >>> a VM, the feature set the VM can see must be the common subset of > >>> the two. > >>> > >>> ~Andrew > >> Yes -- but that''s not a reason to prevent MPX feature (or, any new > >> features) -- otherwise you have to prevent any new features. > >> The right place to control cpuid policy of a pool is at higher > >> level, where it has full information of the pool machines and so > >> it''s right place to make decision what cpuid feature set would be > >> proper for the specific pool. > >> > > That is exactly a reason to prevent MPX. > > > > If the domain cpuid policy (which is set by the toolstack) states that > > MPX should be disabled, then MPX must be hidden from the HVM guest, > > even if the hardware supports MPX. > > No. That''s _not_ a reason to prevent MPX -- toolstack still has the > right to disable MPX, no matter h/w support MPX or not. Refer > xc_cpuid_set().There seems to be a lot of confusion here. As far as I can tell, the only sensible mechanism is: - If the hardware doesn''t support MPX, mask it in guest CPUID. - If the domain cpuid policy masks the MPX feature, disable it. - Otherwise, enable it, and advertise it in guest CPUID. In any case, the CPUID fields seen by the guest _must_ match whether the feature is available. Tim.
Tim Deegan wrote:> At 03:17 +0000 on 28 Nov (1385605079), Liu, Jinsong wrote: >> Andrew Cooper wrote: >>> On 27/11/13 15:02, Liu, Jinsong wrote: >>>> Andrew Cooper wrote: >>>>> It is very common to have pools of servers made of different >>>>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely >>>>> migrate a VM, the feature set the VM can see must be the common >>>>> subset of the two. >>>>> >>>>> ~Andrew >>>> Yes -- but that''s not a reason to prevent MPX feature (or, any new >>>> features) -- otherwise you have to prevent any new features. >>>> The right place to control cpuid policy of a pool is at higher >>>> level, where it has full information of the pool machines and so >>>> it''s right place to make decision what cpuid feature set would be >>>> proper for the specific pool. >>>> >>> That is exactly a reason to prevent MPX. >>> >>> If the domain cpuid policy (which is set by the toolstack) states >>> that MPX should be disabled, then MPX must be hidden from the HVM >>> guest, even if the hardware supports MPX. >> >> No. That''s _not_ a reason to prevent MPX -- toolstack still has the >> right to disable MPX, no matter h/w support MPX or not. Refer >> xc_cpuid_set(). > > There seems to be a lot of confusion here. As far as I can tell, the > only sensible mechanism is: > > - If the hardware doesn''t support MPX, mask it in guest CPUID. > - If the domain cpuid policy masks the MPX feature, disable it. > - Otherwise, enable it, and advertise it in guest CPUID. > > In any case, the CPUID fields seen by the guest _must_ match whether > the feature is available. > > Tim.Do you mean xc_cpuid_set() is some confusion? Yes, it''s some buggy that need got fix at tools side. I take it here as an example just indicate ''toolstack has the right to disable/mask hardware feature, if it want to do so per domain cpuid policy''. Thanks, Jinsong
At 11:12 +0000 on 28 Nov (1385633520), Liu, Jinsong wrote:> Tim Deegan wrote: > > At 03:17 +0000 on 28 Nov (1385605079), Liu, Jinsong wrote: > >> Andrew Cooper wrote: > >>> On 27/11/13 15:02, Liu, Jinsong wrote: > >>>> Andrew Cooper wrote: > >>>>> It is very common to have pools of servers made of different > >>>>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely > >>>>> migrate a VM, the feature set the VM can see must be the common > >>>>> subset of the two. > >>>>> > >>>>> ~Andrew > >>>> Yes -- but that''s not a reason to prevent MPX feature (or, any new > >>>> features) -- otherwise you have to prevent any new features. > >>>> The right place to control cpuid policy of a pool is at higher > >>>> level, where it has full information of the pool machines and so > >>>> it''s right place to make decision what cpuid feature set would be > >>>> proper for the specific pool. > >>>> > >>> That is exactly a reason to prevent MPX. > >>> > >>> If the domain cpuid policy (which is set by the toolstack) states > >>> that MPX should be disabled, then MPX must be hidden from the HVM > >>> guest, even if the hardware supports MPX. > >> > >> No. That''s _not_ a reason to prevent MPX -- toolstack still has the > >> right to disable MPX, no matter h/w support MPX or not. Refer > >> xc_cpuid_set(). > > > > There seems to be a lot of confusion here. As far as I can tell, the > > only sensible mechanism is: > > > > - If the hardware doesn''t support MPX, mask it in guest CPUID. > > - If the domain cpuid policy masks the MPX feature, disable it. > > - Otherwise, enable it, and advertise it in guest CPUID. > > > > In any case, the CPUID fields seen by the guest _must_ match whether > > the feature is available. > > > > Tim. > > Do you mean xc_cpuid_set() is some confusion? Yes, it''s some buggy that need got fix at tools side. > > I take it here as an example just indicate ''toolstack has the right to disable/mask hardware feature, if it want to do so per domain cpuid policy''. >In that case, I think you and Andrew are agreeing with each other. :) The important detail is that if the toolstack has disabled the feature using the CPUID policy, the hypervisor should not expose the feature to the guest. Cheers, Tim.
Tim Deegan wrote:> At 11:12 +0000 on 28 Nov (1385633520), Liu, Jinsong wrote: >> Tim Deegan wrote: >>> At 03:17 +0000 on 28 Nov (1385605079), Liu, Jinsong wrote: >>>> Andrew Cooper wrote: >>>>> On 27/11/13 15:02, Liu, Jinsong wrote: >>>>>> Andrew Cooper wrote: >>>>>>> It is very common to have pools of servers made of different >>>>>>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely >>>>>>> migrate a VM, the feature set the VM can see must be the common >>>>>>> subset of the two. >>>>>>> >>>>>>> ~Andrew >>>>>> Yes -- but that''s not a reason to prevent MPX feature (or, any >>>>>> new features) -- otherwise you have to prevent any new features. >>>>>> The right place to control cpuid policy of a pool is at higher >>>>>> level, where it has full information of the pool machines and so >>>>>> it''s right place to make decision what cpuid feature set would >>>>>> be proper for the specific pool. >>>>>> >>>>> That is exactly a reason to prevent MPX. >>>>> >>>>> If the domain cpuid policy (which is set by the toolstack) states >>>>> that MPX should be disabled, then MPX must be hidden from the HVM >>>>> guest, even if the hardware supports MPX. >>>> >>>> No. That''s _not_ a reason to prevent MPX -- toolstack still has the >>>> right to disable MPX, no matter h/w support MPX or not. Refer >>>> xc_cpuid_set(). >>> >>> There seems to be a lot of confusion here. As far as I can tell, >>> the only sensible mechanism is: >>> >>> - If the hardware doesn''t support MPX, mask it in guest CPUID. >>> - If the domain cpuid policy masks the MPX feature, disable it. >>> - Otherwise, enable it, and advertise it in guest CPUID. >>> >>> In any case, the CPUID fields seen by the guest _must_ match >>> whether the feature is available. >>> >>> Tim. >> >> Do you mean xc_cpuid_set() is some confusion? Yes, it''s some buggy >> that need got fix at tools side. >> >> I take it here as an example just indicate ''toolstack has the right >> to disable/mask hardware feature, if it want to do so per domain >> cpuid policy''. >> > > In that case, I think you and Andrew are agreeing with each other. :) > The important detail is that if the toolstack has disabled the feature > using the CPUID policy, the hypervisor should not expose the feature > to the guest. > > Cheers, > > Tim.Yeah, exactly :) Thanks, Jinsong
On Thu, 2013-11-28 at 12:14 +0100, Tim Deegan wrote:> At 11:12 +0000 on 28 Nov (1385633520), Liu, Jinsong wrote: > > Tim Deegan wrote: > > > At 03:17 +0000 on 28 Nov (1385605079), Liu, Jinsong wrote: > > >> Andrew Cooper wrote: > > >>> On 27/11/13 15:02, Liu, Jinsong wrote: > > >>>> Andrew Cooper wrote: > > >>>>> It is very common to have pools of servers made of different > > >>>>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely > > >>>>> migrate a VM, the feature set the VM can see must be the common > > >>>>> subset of the two. > > >>>>> > > >>>>> ~Andrew > > >>>> Yes -- but that''s not a reason to prevent MPX feature (or, any new > > >>>> features) -- otherwise you have to prevent any new features. > > >>>> The right place to control cpuid policy of a pool is at higher > > >>>> level, where it has full information of the pool machines and so > > >>>> it''s right place to make decision what cpuid feature set would be > > >>>> proper for the specific pool. > > >>>> > > >>> That is exactly a reason to prevent MPX. > > >>> > > >>> If the domain cpuid policy (which is set by the toolstack) states > > >>> that MPX should be disabled, then MPX must be hidden from the HVM > > >>> guest, even if the hardware supports MPX. > > >> > > >> No. That''s _not_ a reason to prevent MPX -- toolstack still has the > > >> right to disable MPX, no matter h/w support MPX or not. Refer > > >> xc_cpuid_set(). > > > > > > There seems to be a lot of confusion here. As far as I can tell, the > > > only sensible mechanism is: > > > > > > - If the hardware doesn''t support MPX, mask it in guest CPUID. > > > - If the domain cpuid policy masks the MPX feature, disable it. > > > - Otherwise, enable it, and advertise it in guest CPUID. > > > > > > In any case, the CPUID fields seen by the guest _must_ match whether > > > the feature is available. > > > > > > Tim. > > > > Do you mean xc_cpuid_set() is some confusion? Yes, it''s some buggy that need got fix at tools side. > > > > I take it here as an example just indicate ''toolstack has the right to disable/mask hardware feature, if it want to do so per domain cpuid policy''. > > > > In that case, I think you and Andrew are agreeing with each other. :) > The important detail is that if the toolstack has disabled the feature > using the CPUID policy, the hypervisor should not expose the feature > to the guest.Do you mean expose as in "present in cpuid" or do you mean expose as in "the instructions/features work (even if disabled in cpuid)"? Ian.
At 11:26 +0000 on 28 Nov (1385634381), Ian Campbell wrote:> On Thu, 2013-11-28 at 12:14 +0100, Tim Deegan wrote: > > At 11:12 +0000 on 28 Nov (1385633520), Liu, Jinsong wrote: > > > Tim Deegan wrote: > > > > At 03:17 +0000 on 28 Nov (1385605079), Liu, Jinsong wrote: > > > >> Andrew Cooper wrote: > > > >>> On 27/11/13 15:02, Liu, Jinsong wrote: > > > >>>> Andrew Cooper wrote: > > > >>>>> It is very common to have pools of servers made of different > > > >>>>> generations of CPU. E.g. Ivy Bridge and Haswell. To safely > > > >>>>> migrate a VM, the feature set the VM can see must be the common > > > >>>>> subset of the two. > > > >>>>> > > > >>>>> ~Andrew > > > >>>> Yes -- but that''s not a reason to prevent MPX feature (or, any new > > > >>>> features) -- otherwise you have to prevent any new features. > > > >>>> The right place to control cpuid policy of a pool is at higher > > > >>>> level, where it has full information of the pool machines and so > > > >>>> it''s right place to make decision what cpuid feature set would be > > > >>>> proper for the specific pool. > > > >>>> > > > >>> That is exactly a reason to prevent MPX. > > > >>> > > > >>> If the domain cpuid policy (which is set by the toolstack) states > > > >>> that MPX should be disabled, then MPX must be hidden from the HVM > > > >>> guest, even if the hardware supports MPX. > > > >> > > > >> No. That''s _not_ a reason to prevent MPX -- toolstack still has the > > > >> right to disable MPX, no matter h/w support MPX or not. Refer > > > >> xc_cpuid_set(). > > > > > > > > There seems to be a lot of confusion here. As far as I can tell, the > > > > only sensible mechanism is: > > > > > > > > - If the hardware doesn''t support MPX, mask it in guest CPUID. > > > > - If the domain cpuid policy masks the MPX feature, disable it. > > > > - Otherwise, enable it, and advertise it in guest CPUID. > > > > > > > > In any case, the CPUID fields seen by the guest _must_ match whether > > > > the feature is available. > > > > > > > > Tim. > > > > > > Do you mean xc_cpuid_set() is some confusion? Yes, it''s some buggy that need got fix at tools side. > > > > > > I take it here as an example just indicate ''toolstack has the right to disable/mask hardware feature, if it want to do so per domain cpuid policy''. > > > > > > > In that case, I think you and Andrew are agreeing with each other. :) > > The important detail is that if the toolstack has disabled the feature > > using the CPUID policy, the hypervisor should not expose the feature > > to the guest. > > Do you mean expose as in "present in cpuid" or do you mean expose as in > "the instructions/features work (even if disabled in cpuid)"?I mean that if the guest does not see the feature in CPUID they should net be able to use it. Tim.
>>> On 28.11.13 at 12:45, Tim Deegan <tim@xen.org> wrote: > I mean that if the guest does not see the feature in CPUID they should > net be able to use it.That would be the ideal thing, but would require more code: We''d need to intercept the MSR just in order to fail accesses to it. And I don''t think we go that far for other CPU features (read: if we do here, we should do so consistently everywhere). Jan