FPU LWP 6/8: create lazy and non-lazy FPU restore functions

Quoted 1 line

Quoted 1 line · Wei Huang

xen/arch/x86/domain.c +0 -0

        n->arch.ctxt_switch_to(n);
    }
 

xen/arch/x86/i387.c +3 -0

@@ -160,10 +160,25 @@
 /*******************************/
 /*       VCPU FPU Functions    */
 /*******************************/
+/* Restore FPU state whenever VCPU is schduled in. */
+void vcpu_restore_fpu_eager(struct vcpu *v)
+{

xen/arch/x86/i387.c +3 -1

+}
+
 /* 
 * 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));
 

Quoted 1 line

Quoted 1 line

Quoted 1 line · Wei Huang

Again as pointed out earlier, ...

xen/arch/x86/domain.c line 1581

Quoted 7 lines

xen/arch/x86/domain.c +1 -0

@@ -1578,6 +1578,7 @@
         memcpy(stack_regs,&n->arch.user_regs, CTXT_SWITCH_STACK_BYTES);
         if ( xsave_enabled(n)&&  n->arch.xcr0 != get_xcr0() )
             set_xcr0(n->arch.xcr0);
+        vcpu_restore_fpu_eager(n);

... this call is unconditional, ...

Quoted 12 lines

xen/arch/x86/domain.c +0 -0

         n->arch.ctxt_switch_to(n);
     }
 

xen/arch/x86/i387.c +3 -0

@@ -160,10 +160,25 @@
 /*******************************/
 /*       VCPU FPU Functions    */
 /*******************************/
+/* Restore FPU state whenever VCPU is schduled in. */
+void vcpu_restore_fpu_eager(struct vcpu *v)
+{

xen/arch/x86/i387.c line 175

Quoted 10 lines

xen/arch/x86/i387.c +10 -0

+    ASSERT(!is_idle_vcpu(v));
+
+    /* Avoid recursion */
+    clts();
+
+    /* 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

Quoted 10 lines

xen/arch/x86/i387.c +3 -1

+}
+
 /*
  * 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));
 

Quoted 1 line

Quoted 1 line · Wei Huang

Checking whether there is a non-lazy state to save is architectural specific and very messy. For instance, we need to read LWP_CBADDR to confirm LWP's dirty state. This MSR is AMD specific and we don't want to add it here. Plus reading data from LWP_CBADDR MSR might be as expensive as clts/stts.

My previous email showed that the overhead with LWP is around 1%-2% of __context_switch(). For non lwp-capable CPU, this overhead should be much smaller (only clts and stts) because xfeature_mask[LWP] is 0.

Yes, clts() and stts() don't have to called every time. How about this one?

/* Restore FPU state whenever VCPU is schduled in. */ void vcpu_restore_fpu_eager(struct vcpu *v)

{
     ASSERT(!is_idle_vcpu(v));


     /* save the nonlazy extended state which is not tracked by CR0.TS bit */
     if ( xsave_enabled(v) )
     {
         /* Avoid recursion */
         clts();
         fpu_xrstor(v, XSTATE_NONLAZY);
         stts();
     }

Quoted 10 lines

Quoted 1 line

Quoted 1 line · Wei Huang

Checking whether there is a non-lazy state to save is architectural specific and very messy. For instance, we need to read LWP_CBADDR to confirm LWP's dirty state. This MSR is AMD specific and we don't want to add it here. Plus reading data from LWP_CBADDR MSR might be as expensive as clts/stts.

My previous email showed that the overhead with LWP is around 1%-2% of __context_switch(). For non lwp-capable CPU, this overhead should be much smaller (only clts and stts) because xfeature_mask[LWP] is 0.

I wasn't talking about determining whether LWP state is dirty, but much rather about LWP not being in use at all.

Quoted 4 lines

Yes, clts() and stts() don't have to called every time. How about this one?

/* Restore FPU state whenever VCPU is schduled in. */ void vcpu_restore_fpu_eager(struct vcpu *v)

Quoted 12 lines

{
      ASSERT(!is_idle_vcpu(v));


      /* save the nonlazy extended state which is not tracked by CR0.TS bit */
      if ( xsave_enabled(v) )
      {
          /* Avoid recursion */
          clts();
          fpu_xrstor(v, XSTATE_NONLAZY);
          stts();
      }

That's certainly better, but I'd still like to see the xsave_enabled() check to be replaced by some form of lwp_enabled() or lazy_xsave_needed() or some such (which will at once exclude all pv guests until you care to add support for them).

Jan

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Quoted 1 line

Quoted 1 line · Wei Huang

Hi Jan,

If we want to make LWP restore optional in vcpu_restore_fpu_eager(), we have to change vcpu_save_fpu() as well. Otherwise, the extended state will become inconsistent for non-LWP VCPUs (because save and restore is asymmetric). There are two approaches:

1. In vcpu_save_fpu(), clean physical CPU's extended state for VCPU which is being scheduled in. This prevents messy states from causing problems. The disadvantage is the cleaning cost, which would out-weight the benefits.

2. Add a new variable in VCPU to track whether nonlazy state is dirty. I think this is better. See the attached file.

Let me know if it is what you want. After that, I will re-spin the patches.