From mboxrd@z Thu Jan  1 00:00:00 1970
From: Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH 1/5] X86: Fix xsave bug for nonlazy xstates
Date: Thu, 21 Nov 2013 14:40:30 +0000
Message-ID: <528E1B5E.5000705@citrix.com>
References: <DE8DF0795D48FD4CA783C40EC8292335013CC7E6@SHSMSX101.ccr.corp.intel.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <DE8DF0795D48FD4CA783C40EC8292335013CC7E6@SHSMSX101.ccr.corp.intel.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: "Liu, Jinsong" <jinsong.liu@intel.com>
Cc: Ian Campbell <Ian.Campbell@citrix.com>, "keir@xen.org" <keir@xen.org>, "haoxudong.hao@gmail.com" <haoxudong.hao@gmail.com>, Jan Beulich <JBeulich@suse.com>, "xen-devel@lists.xen.org" <xen-devel@lists.xen.org>
List-Id: xen-devel@lists.xenproject.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