Re: [Xen-devel] Issue in MPX support in Xen

2016-09-02 Thread Rockosov, Dmitry
Hello Jan,

I have tested your patch, it works fine in situation when cpu_has_mpx = 1 and 
cpu_has_vmx_mpx = 0.

Thank you!

Best regards,
Dmitry Rockosov

> -Original Message-
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Friday, August 26, 2016 11:09 AM
> To: Rockosov, Dmitry 
> Cc: Andrew Cooper ; Sorokin, Georgy
> ; Rechistov, Grigory
> ; Nakajima, Jun ;
> Tian, Kevin ; Kornev, Roman M
> ; Samoylidi, Sergey N
> ; xen-devel@lists.xen.org
> Subject: Re: [Xen-devel] Issue in MPX support in Xen
> 
> >>> On 25.08.16 at 18:53,  wrote:
> > We are working on enabling XEN with WindRiver Simics, which is Intel
> > reference functional simulator for servers.
> >
> > We found the issue in XEN with MPX using.
> > If MPX is supported by CPUID, but MPX is not supported by VMX, XEN is
> > failing on store CPU MSR GUEST_BNDCFGS (file
> > xen-4.7.0/xen/arch/x86/hvm/vmx/vmx.c:798).
> >
> > SDM says in 24.4.1 Guest Register State:
> > IA32_BNDCFGS (64 bits). This field is supported only on processors
> > that support either the 1-setting of the "load IA32_BNDCFGS" VM-entry
> > control or that of the "clear IA32_BNDCFGS"
> > VM-exit control.
> >
> > Looks like XEN doesn't consult VM-entry control or VM-exit control
> > listed in SDM.
> 
> Could you give the attached patch a try? It's against -unstable and will
> require trivial adjustment to two of the vmx.c hunks in order to apply
> to 4.7.
> 
> Andrew, Kevin, Jun - please take a look as well, in particular regarding
> the non-commit-message remark.
> 
> Jan



Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


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


Re: [Xen-devel] Issue in MPX support in Xen

2016-08-31 Thread Jan Beulich
>>> On 26.08.16 at 10:09,  wrote:
 On 25.08.16 at 18:53,  wrote:
>> We are working on enabling XEN with WindRiver Simics, which is Intel 
>> reference functional simulator for servers.
>> 
>> We found the issue in XEN with MPX using.
>> If MPX is supported by CPUID, but MPX is not supported by VMX, XEN is 
>> failing on store CPU MSR GUEST_BNDCFGS (file 
>> xen-4.7.0/xen/arch/x86/hvm/vmx/vmx.c:798).
>> 
>> SDM says in 24.4.1 Guest Register State:
>> IA32_BNDCFGS (64 bits). This field is supported only on processors that 
>> support either the 1-setting of the
>> "load IA32_BNDCFGS" VM-entry control or that of the "clear IA32_BNDCFGS" 
>> VM-exit control.
>> 
>> Looks like XEN doesn't consult VM-entry control or VM-exit control listed in 
>> SDM.
> 
> Could you give the attached patch a try? It's against -unstable and
> will require trivial adjustment to two of the vmx.c hunks in order to
> apply to 4.7.

Ping?

Jan


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


Re: [Xen-devel] Issue in MPX support in Xen

2016-08-29 Thread Tian, Kevin
> From: Jan Beulich [mailto:jbeul...@suse.com]
> Sent: Friday, August 26, 2016 4:09 PM
> 
> >>> On 25.08.16 at 18:53,  wrote:
> > We are working on enabling XEN with WindRiver Simics, which is Intel
> > reference functional simulator for servers.
> >
> > We found the issue in XEN with MPX using.
> > If MPX is supported by CPUID, but MPX is not supported by VMX, XEN is
> > failing on store CPU MSR GUEST_BNDCFGS (file
> > xen-4.7.0/xen/arch/x86/hvm/vmx/vmx.c:798).
> >
> > SDM says in 24.4.1 Guest Register State:
> > IA32_BNDCFGS (64 bits). This field is supported only on processors that
> > support either the 1-setting of the
> > "load IA32_BNDCFGS" VM-entry control or that of the "clear IA32_BNDCFGS"
> > VM-exit control.
> >
> > Looks like XEN doesn't consult VM-entry control or VM-exit control listed in
> > SDM.
> 
> Could you give the attached patch a try? It's against -unstable and
> will require trivial adjustment to two of the vmx.c hunks in order to
> apply to 4.7.
> 
> Andrew, Kevin, Jun - please take a look as well, in particular regarding
> the non-commit-message remark.
> 
> Jan

My feeling is below:

- Looks checking hvm_cpuid is a natural condition since MPX in hvm_cpuid 
is set only when both situations are true (CPU MPX and VMX MPX)

- For save/load_msr, it's purely about VMCS structure. So checking VMX
MPX alone should be enough.

Thanks
Kevin

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


Re: [Xen-devel] Issue in MPX support in Xen

2016-08-26 Thread Jan Beulich
>>> On 25.08.16 at 18:53,  wrote:
> We are working on enabling XEN with WindRiver Simics, which is Intel 
> reference functional simulator for servers.
> 
> We found the issue in XEN with MPX using.
> If MPX is supported by CPUID, but MPX is not supported by VMX, XEN is 
> failing on store CPU MSR GUEST_BNDCFGS (file 
> xen-4.7.0/xen/arch/x86/hvm/vmx/vmx.c:798).
> 
> SDM says in 24.4.1 Guest Register State:
> IA32_BNDCFGS (64 bits). This field is supported only on processors that 
> support either the 1-setting of the
> "load IA32_BNDCFGS" VM-entry control or that of the "clear IA32_BNDCFGS" 
> VM-exit control.
> 
> Looks like XEN doesn't consult VM-entry control or VM-exit control listed in 
> SDM.

Could you give the attached patch a try? It's against -unstable and
will require trivial adjustment to two of the vmx.c hunks in order to
apply to 4.7.

Andrew, Kevin, Jun - please take a look as well, in particular regarding
the non-commit-message remark.

Jan

VMX: correct feature checks for MPX and XSAVES

Their VMCS fields aren't tied to the respective base CPU feature flags
but instead to VMX specific ones.

Note that while the VMCS GUEST_BNDCFGS field exists if either of the
two respective features is available, MPX continues to get exposed to
guests only with both features present.

Also add the so far missing handling of
- GUEST_BNDCFGS in construct_vmcs()
- MSR_IA32_BNDCFGS in vmx_msr_{read,write}_intercept()
and mirror the extra correctness checks during MSR write to
vmx_load_msr().

Reported-by: Rockosov, Dmitry 
Signed-off-by: Jan Beulich 
---
Question of course is whether the checks in
vmx_msr_{read,write}_intercept() (and then also the xsaves ones in
hvm_msr_{read,write}_intercept()) shouldn't rather consult hvm_cpuid().
In that case it would seem natural to move the BNDCFGS handling to
hvm_msr_{read,write}_intercept().

--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -168,8 +168,7 @@ static void __init calculate_hvm_feature
  */
 if ( cpu_has_vmx )
 {
-if ( !(vmx_vmexit_control & VM_EXIT_CLEAR_BNDCFGS) ||
- !(vmx_vmentry_control & VM_ENTRY_LOAD_BNDCFGS) )
+if ( !cpu_has_vmx_mpx )
 __clear_bit(X86_FEATURE_MPX, hvm_featureset);
 
 if ( !cpu_has_vmx_xsaves )
--- a/xen/arch/x86/hvm/vmx/vmcs.c
+++ b/xen/arch/x86/hvm/vmx/vmcs.c
@@ -1261,6 +1261,8 @@ static int construct_vmcs(struct vcpu *v
 __vmwrite(HOST_PAT, host_pat);
 __vmwrite(GUEST_PAT, guest_pat);
 }
+if ( cpu_has_vmx_mpx )
+__vmwrite(GUEST_BNDCFGS, 0);
 if ( cpu_has_vmx_xsaves )
 __vmwrite(XSS_EXIT_BITMAP, 0);
 
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -788,14 +788,15 @@ static int vmx_load_vmcs_ctxt(struct vcp
 
 static unsigned int __init vmx_init_msr(void)
 {
-return !!cpu_has_mpx + !!cpu_has_xsaves;
+return (cpu_has_mpx && cpu_has_vmx_mpx) +
+   (cpu_has_xsaves && cpu_has_vmx_xsaves);
 }
 
 static void vmx_save_msr(struct vcpu *v, struct hvm_msr *ctxt)
 {
 vmx_vmcs_enter(v);
 
-if ( cpu_has_mpx )
+if ( cpu_has_mpx && cpu_has_vmx_mpx )
 {
 __vmread(GUEST_BNDCFGS, &ctxt->msr[ctxt->count].val);
 if ( ctxt->msr[ctxt->count].val )
@@ -804,7 +805,7 @@ static void vmx_save_msr(struct vcpu *v,
 
 vmx_vmcs_exit(v);
 
-if ( cpu_has_xsaves )
+if ( cpu_has_xsaves && cpu_has_vmx_xsaves )
 {
 ctxt->msr[ctxt->count].val = v->arch.hvm_vcpu.msr_xss;
 if ( ctxt->msr[ctxt->count].val )
@@ -824,13 +825,15 @@ static int vmx_load_msr(struct vcpu *v,
 switch ( ctxt->msr[i].index )
 {
 case MSR_IA32_BNDCFGS:
-if ( cpu_has_mpx )
+if ( cpu_has_mpx && cpu_has_vmx_mpx &&
+ is_canonical_address(ctxt->msr[i].val) &&
+ !(ctxt->msr[i].val & IA32_BNDCFGS_RESERVED) )
 __vmwrite(GUEST_BNDCFGS, ctxt->msr[i].val);
 else if ( ctxt->msr[i].val )
 err = -ENXIO;
 break;
 case MSR_IA32_XSS:
-if ( cpu_has_xsaves )
+if ( cpu_has_xsaves && cpu_has_vmx_xsaves )
 v->arch.hvm_vcpu.msr_xss = ctxt->msr[i].val;
 else
 err = -ENXIO;
@@ -2643,6 +2643,11 @@ static int vmx_msr_read_intercept(unsign
 case MSR_IA32_DEBUGCTLMSR:
 __vmread(GUEST_IA32_DEBUGCTL, msr_content);
 break;
+case MSR_IA32_BNDCFGS:
+if ( !cpu_has_mpx || !cpu_has_vmx_mpx )
+goto gp_fault;
+__vmread(GUEST_BNDCFGS, msr_content);
+break;
 case MSR_IA32_FEATURE_CONTROL:
 case MSR_IA32_VMX_BASIC...MSR_IA32_VMX_VMFUNC:
 if ( !nvmx_msr_read_intercept(msr, msr_content) )
@@ -2869,6 +2872,13 @@ static int vmx_msr_write_intercept(unsig
 
 break;
 }
+case MSR_IA32_BNDCFGS:
+if ( !cpu_has_mpx || !cpu_has_vmx_mpx ||
+ !is_canonical_address(msr_content) ||
+ (msr_content & IA32_BNDCFGS_RESERV

[Xen-devel] Issue in MPX support in Xen

2016-08-25 Thread Rockosov, Dmitry
Hello Xen team,

We are working on enabling XEN with WindRiver Simics, which is Intel reference 
functional simulator for servers.

We found the issue in XEN with MPX using.
If MPX is supported by CPUID, but MPX is not supported by VMX, XEN is failing 
on store CPU MSR GUEST_BNDCFGS (file xen-4.7.0/xen/arch/x86/hvm/vmx/vmx.c:798).

SDM says in 24.4.1 Guest Register State:
IA32_BNDCFGS (64 bits). This field is supported only on processors that support 
either the 1-setting of the
"load IA32_BNDCFGS" VM-entry control or that of the "clear IA32_BNDCFGS" 
VM-exit control.

Looks like XEN doesn't consult VM-entry control or VM-exit control listed in 
SDM.

Best regards,
Dmitry Rockosov
SSG Simics server modeling team (STC, Moscow)



Joint Stock Company Intel A/O
Registered legal address: Krylatsky Hills Business Park,
17 Krylatskaya Str., Bldg 4, Moscow 121614,
Russian Federation

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel