All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates
@ 2013-11-19 10:46 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
  0 siblings, 2 replies; 3+ messages in thread
From: Liu, Jinsong @ 2013-11-19 10:46 UTC (permalink / raw)
  To: keir@xen.org, Jan Beulich, xen-devel@lists.xen.org
  Cc: andrew.cooper3@citrix.com, haoxudong.hao@gmail.com, Ian Campbell

>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

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* 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

end of thread, other threads:[~2013-11-22 13:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.