* Re: [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates
2013-11-19 10:46 [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates Liu, Jinsong
@ 2013-11-21 14:40 ` Andrew Cooper
2013-11-22 13:23 ` [PATCH v2 1/5] x86/xsave: fix nonlazy state handling Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Cooper @ 2013-11-21 14:40 UTC (permalink / raw)
To: Liu, Jinsong
Cc: Ian Campbell, keir@xen.org, haoxudong.hao@gmail.com, Jan Beulich,
xen-devel@lists.xen.org
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
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH v2 1/5] x86/xsave: fix nonlazy state handling
2013-11-19 10:46 [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates Liu, Jinsong
2013-11-21 14:40 ` Andrew Cooper
@ 2013-11-22 13:23 ` Jan Beulich
1 sibling, 0 replies; 3+ messages in thread
From: Jan Beulich @ 2013-11-22 13:23 UTC (permalink / raw)
To: Jinsong Liu, xen-devel
Cc: andrew.cooper3@citrix.com, keir@xen.org, Ian Campbell,
haoxudong.hao@gmail.com
[-- Attachment #1: Type: text/plain, Size: 2869 bytes --]
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));
[-- Attachment #2: x86-xsave-lazy.patch --]
[-- Type: text/plain, Size: 2904 bytes --]
x86/xsave: fix nonlazy state handling
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));
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 3+ messages in thread