From 0d6072a31977aa509d04e0b0dc1642508a1ed87b Mon Sep 17 00:00:00 2001 From: Liu Jinsong <jinsong.liu@intel.com> Date: Tue, 19 Nov 2013 18:27:36 +0800 Subject: [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates Nonlazy xstates should be xsaved each time when vcpu_save_fpu. Operation to nonlazy xstates will not trigger #NM exception, so whenever vcpu scheduled in it got restored and whenever scheduled out it should get saved. Currently this bug affects AMD LWP feature, and later Intel MPX feature. With the bugfix both LWP and MPX will work fine. Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> Signed-off-by: Jan Beulich <jbeulich@suse.com> --- xen/arch/x86/i387.c | 30 ++++++++++++++++++++++++++---- 1 files changed, 26 insertions(+), 4 deletions(-) diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c index 7649274..e7a590b 100644 --- a/xen/arch/x86/i387.c +++ b/xen/arch/x86/i387.c @@ -133,10 +133,33 @@ static inline void fpu_frstor(struct vcpu *v) /*******************************/ /* FPU Save Functions */ /*******************************/ + +static inline uint64_t fpu_xsave_mask(struct vcpu *v) +{ + if ( v->fpu_dirtied ) + { + if ( v->arch.nonlazy_xstate_used ) + return XSTATE_ALL; + else + return XSTATE_LAZY; + } + else + { + if ( v->arch.nonlazy_xstate_used ) + return XSTATE_NONLAZY; + else + return 0; + } +} + /* Save x87 extended state */ static inline void fpu_xsave(struct vcpu *v) { bool_t ok; + uint64_t mask = fpu_xsave_mask(v); + + if ( !mask ) + return; ASSERT(v->arch.xsave_area); /* @@ -145,7 +168,7 @@ static inline void fpu_xsave(struct vcpu *v) */ ok = set_xcr0(v->arch.xcr0_accum | XSTATE_FP_SSE); ASSERT(ok); - xsave(v, v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY); + xsave(v, mask); ok = set_xcr0(v->arch.xcr0 ?: XSTATE_FP_SSE); ASSERT(ok); } @@ -257,9 +280,6 @@ void vcpu_restore_fpu_lazy(struct vcpu *v) */ void vcpu_save_fpu(struct vcpu *v) { - if ( !v->fpu_dirtied ) - return; - ASSERT(!is_idle_vcpu(v)); /* This can happen, if a paravirtualised guest OS has set its CR0.TS. */ @@ -267,6 +287,8 @@ void vcpu_save_fpu(struct vcpu *v) if ( cpu_has_xsave ) fpu_xsave(v); + else if ( !v->fpu_dirtied ) + ; /* Nothing */ else if ( cpu_has_fxsr ) fpu_fxsave(v); else -- 1.7.1
Andrew Cooper
2013-Nov-21 14:40 UTC
Re: [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates
On 19/11/13 10:46, Liu, Jinsong wrote:> From 0d6072a31977aa509d04e0b0dc1642508a1ed87b Mon Sep 17 00:00:00 2001 > From: Liu Jinsong <jinsong.liu@intel.com> > Date: Tue, 19 Nov 2013 18:27:36 +0800 > Subject: [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates > > Nonlazy xstates should be xsaved each time when vcpu_save_fpu. > Operation to nonlazy xstates will not trigger #NM exception, > so whenever vcpu scheduled in it got restored and whenever > scheduled out it should get saved. > > Currently this bug affects AMD LWP feature, and later Intel > MPX feature. With the bugfix both LWP and MPX will work fine. > > Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> > Signed-off-by: Jan Beulich <jbeulich@suse.com>Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>> --- > xen/arch/x86/i387.c | 30 ++++++++++++++++++++++++++---- > 1 files changed, 26 insertions(+), 4 deletions(-) > > diff --git a/xen/arch/x86/i387.c b/xen/arch/x86/i387.c > index 7649274..e7a590b 100644 > --- a/xen/arch/x86/i387.c > +++ b/xen/arch/x86/i387.c > @@ -133,10 +133,33 @@ static inline void fpu_frstor(struct vcpu *v) > /*******************************/ > /* FPU Save Functions */ > /*******************************/ > + > +static inline uint64_t fpu_xsave_mask(struct vcpu *v) > +{ > + if ( v->fpu_dirtied ) > + { > + if ( v->arch.nonlazy_xstate_used ) > + return XSTATE_ALL; > + else > + return XSTATE_LAZY; > + } > + else > + { > + if ( v->arch.nonlazy_xstate_used ) > + return XSTATE_NONLAZY; > + else > + return 0; > + } > +} > + > /* Save x87 extended state */ > static inline void fpu_xsave(struct vcpu *v) > { > bool_t ok; > + uint64_t mask = fpu_xsave_mask(v); > + > + if ( !mask ) > + return; > > ASSERT(v->arch.xsave_area); > /* > @@ -145,7 +168,7 @@ static inline void fpu_xsave(struct vcpu *v) > */ > ok = set_xcr0(v->arch.xcr0_accum | XSTATE_FP_SSE); > ASSERT(ok); > - xsave(v, v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY); > + xsave(v, mask); > ok = set_xcr0(v->arch.xcr0 ?: XSTATE_FP_SSE); > ASSERT(ok); > } > @@ -257,9 +280,6 @@ void vcpu_restore_fpu_lazy(struct vcpu *v) > */ > void vcpu_save_fpu(struct vcpu *v) > { > - if ( !v->fpu_dirtied ) > - return; > - > ASSERT(!is_idle_vcpu(v)); > > /* This can happen, if a paravirtualised guest OS has set its CR0.TS. */ > @@ -267,6 +287,8 @@ void vcpu_save_fpu(struct vcpu *v) > > if ( cpu_has_xsave ) > fpu_xsave(v); > + else if ( !v->fpu_dirtied ) > + ; /* Nothing */ > else if ( cpu_has_fxsr ) > fpu_fxsave(v); > else
Nonlazy xstates should be xsaved each time when vcpu_save_fpu. Operation to nonlazy xstates will not trigger #NM exception, so whenever vcpu scheduled in it got restored and whenever scheduled out it should get saved. Currently this bug affects AMD LWP feature, and later Intel MPX feature. With the bugfix both LWP and MPX will work fine. Signed-off-by: Liu Jinsong <jinsong.liu@intel.com> Furthermore, during restore we also need to set nonlazy_xstate_used according to the incoming accumulated XCR0. Also adjust the changes to i387.c such that there won''t be a pointless clts()/stts() pair. Signed-off-by: Jan Beulich <jbeulich@suse.com> --- a/xen/arch/x86/domctl.c +++ b/xen/arch/x86/domctl.c @@ -1146,6 +1146,8 @@ long arch_do_domctl( { v->arch.xcr0 = _xcr0; v->arch.xcr0_accum = _xcr0_accum; + if ( _xcr0_accum & XSTATE_NONLAZY ) + v->arch.nonlazy_xstate_used = 1; memcpy(v->arch.xsave_area, _xsave_area, evc->size - 2 * sizeof(uint64_t)); } --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -1119,6 +1119,8 @@ static int hvm_load_cpu_xsave_states(str v->arch.xcr0 = ctxt->xcr0; v->arch.xcr0_accum = ctxt->xcr0_accum; + if ( ctxt->xcr0_accum & XSTATE_NONLAZY ) + v->arch.nonlazy_xstate_used = 1; memcpy(v->arch.xsave_area, &ctxt->save_area, desc->length - offsetof(struct hvm_hw_cpu_xsave, save_area)); --- a/xen/arch/x86/i387.c +++ b/xen/arch/x86/i387.c @@ -133,11 +133,22 @@ static inline void fpu_frstor(struct vcp /*******************************/ /* FPU Save Functions */ /*******************************/ + +static inline uint64_t vcpu_xsave_mask(const struct vcpu *v) +{ + if ( v->fpu_dirtied ) + return v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY; + + return v->arch.nonlazy_xstate_used ? XSTATE_NONLAZY : 0; +} + /* Save x87 extended state */ static inline void fpu_xsave(struct vcpu *v) { bool_t ok; + uint64_t mask = vcpu_xsave_mask(v); + ASSERT(mask); ASSERT(v->arch.xsave_area); /* * XCR0 normally represents what guest OS set. In case of Xen itself, @@ -145,7 +156,7 @@ static inline void fpu_xsave(struct vcpu */ ok = set_xcr0(v->arch.xcr0_accum | XSTATE_FP_SSE); ASSERT(ok); - xsave(v, v->arch.nonlazy_xstate_used ? XSTATE_ALL : XSTATE_LAZY); + xsave(v, mask); ok = set_xcr0(v->arch.xcr0 ?: XSTATE_FP_SSE); ASSERT(ok); } @@ -257,7 +268,7 @@ void vcpu_restore_fpu_lazy(struct vcpu * */ void vcpu_save_fpu(struct vcpu *v) { - if ( !v->fpu_dirtied ) + if ( !v->fpu_dirtied && !v->arch.nonlazy_xstate_used ) return; ASSERT(!is_idle_vcpu(v)); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel