From mboxrd@z Thu Jan  1 00:00:00 1970
From: "Jan Beulich" <JBeulich@novell.com>
Subject: Re: [PATCH] FPU LWP 6/8: create lazy and non-lazy FPU
	restore functions
Date: Wed, 04 May 2011 08:09:42 +0100
Message-ID: <4DC117D6020000780003F9CB@vpn.id2.novell.com>
References: <4DC062ED.3070802@amd.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: quoted-printable
Return-path: <xen-devel-bounces@lists.xensource.com>
In-Reply-To: <4DC062ED.3070802@amd.com>
Content-Disposition: inline
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Wei Huang <wei.huang2@amd.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>, KeirFraser <keir@xen.org>
List-Id: xen-devel@lists.xenproject.org

>>> On 03.05.11 at 22:17, Wei Huang <wei.huang2@amd.com> wrote:

Again as pointed out earlier, ...

>--- a/xen/arch/x86/domain.c	Tue May 03 13:49:27 2011 -0500
>+++ b/xen/arch/x86/domain.c	Tue May 03 13:59:37 2011 -0500
>@@ -1578,6 +1578,7 @@
>         memcpy(stack_regs, &n->arch.user_regs, CTXT_SWITCH_STACK_BYTES);
>         if ( xsave_enabled(n) && n->arch.xcr0 !=3D get_xcr0() )
>             set_xcr0(n->arch.xcr0);
>+        vcpu_restore_fpu_eager(n);

... this call is unconditional, ...

>         n->arch.ctxt_switch_to(n);
>     }
>=20
>--- a/xen/arch/x86/i387.c	Tue May 03 13:49:27 2011 -0500
>+++ b/xen/arch/x86/i387.c	Tue May 03 13:59:37 2011 -0500
>@@ -160,10 +160,25 @@
> /*******************************/
> /*       VCPU FPU Functions    */
> /*******************************/
>+/* Restore FPU state whenever VCPU is schduled in. */
>+void vcpu_restore_fpu_eager(struct vcpu *v)
>+{
>+    ASSERT(!is_idle_vcpu(v));
>+
>+    /* Avoid recursion */
>+    clts();
>+   =20
>+    /* save the nonlazy extended state which is not tracked by CR0.TS =
bit */
>+    if ( xsave_enabled(v) )
>+        fpu_xrstor(v, XSTATE_NONLAZY);
>+
>+    stts();

... while here you do an unconditional clts followed by an xrstor only
checking whether xsave is enabled (but not checking whether there's
any non-lazy state to be restored) and, possibly the most expensive
of all, an unconditional write of CR0.

Jan

>+}
>+
> /*=20
>  * Restore FPU state when #NM is triggered.
>  */
>-void vcpu_restore_fpu(struct vcpu *v)
>+void vcpu_restore_fpu_lazy(struct vcpu *v)
> {
>     ASSERT(!is_idle_vcpu(v));
>=20