Fixes x86 FPU initialization
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 26 Apr 2013 18:54:15 +0000 (11:54 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 26 Apr 2013 18:54:15 +0000 (11:54 -0700)
Need to clear the FPU header state before saving the default copy.  My
guess is the PXE agent / bootloader used FP and left us with a full,
dirty FP stack.  Though perhaps the CPU just decided to start like that.
Either way, that bad state was bleeding through to the SCPs / thread0s,
which increase the likelihood of errors in apps (CFL errors in
fluidanimate, which can also cause page faults).

kern/arch/i686/init.c
kern/arch/i686/kdebug.c
kern/arch/i686/trap.h
kern/src/process.c

index a14db9f..ea248bd 100644 (file)
@@ -57,6 +57,10 @@ static void cons_irq_init(void)
 
 void arch_init()
 {
+       /* need to reinit before saving, in case boot agents used the FPU or it is
+        * o/w dirty.  had this happen on c89, which had a full FP stack after
+        * booting. */
+       asm volatile ("fninit");
        save_fp_state(&x86_default_fpu); /* used in arch/trap.h for fpu init */
        pci_init();
 #ifdef __CONFIG_ENABLE_MPTABLES__
index ded18a0..76d18c5 100644 (file)
@@ -342,3 +342,25 @@ void backtrace(void)
                #endif /* __CONFIG_RESET_STACKS__ */
        }
 }
+
+/* Assumes 32-bit header */
+void print_fpu_state(struct ancillary_state *fpu)
+{
+       printk("fcw:        0x%04x\n", fpu->fp_head_n64.fcw);
+       printk("fsw:        0x%04x\n", fpu->fp_head_n64.fsw);
+       printk("ftw:          0x%02x\n", fpu->fp_head_n64.ftw);
+       printk("fop:        0x%04x\n", fpu->fp_head_n64.fop);
+       printk("fpu_ip: 0x%08x\n", fpu->fp_head_n64.fpu_ip);
+       printk("cs:         0x%04x\n", fpu->fp_head_n64.cs);
+       printk("fpu_dp: 0x%08x\n", fpu->fp_head_n64.fpu_dp);
+       printk("ds:         0x%04x\n", fpu->fp_head_n64.ds);
+       printk("mxcsr:  0x%08x\n", fpu->fp_head_n64.mxcsr);
+       printk("mxcsrm: 0x%08x\n", fpu->fp_head_n64.mxcsr_mask);
+
+       for (int i = 0; i < sizeof(struct ancillary_state); i++) {
+               if (i % 20 == 0)
+                       printk("\n");
+               printk("%02x ", *((uint8_t*)fpu + i));
+       }
+       printk("\n");
+}
index 5163f5c..784fd1c 100644 (file)
@@ -113,11 +113,13 @@ static inline void restore_fp_state(struct ancillary_state *silly)
        asm volatile("fxrstor %0" : : "m"(*silly));
 }
 
-/* A regular fninit will only initialize the x87 part of the FPU, not the XMM
- * registers and the MXCSR state.  So to init, we'll just keep around a copy of
- * the default FPU state, which we grabbed during boot, and can copy that over
- * Alternatively, we can fninit, ldmxcsr with the default value, and 0 out the
- * XMM registers. */
+/* A regular fninit will only initialize the x87 header part of the FPU, not the
+ * st(n) (MMX) registers, the XMM registers, or the MXCSR state.  So to init,
+ * we'll just keep around a copy of the default FPU state, which we grabbed
+ * during boot, and can copy that over.
+ *
+ * Alternatively, we can fninit, ldmxcsr with the default value, and 0 out all
+ * of the registers manually. */
 static inline void init_fp_state(void)
 {
        restore_fp_state(&x86_default_fpu);
index ee796d1..04594fb 100644 (file)
@@ -1747,9 +1747,14 @@ static void __set_curctx_to_vcoreid(struct proc *p, uint32_t vcoreid,
         * state or a brand new init).  Starting a fresh VC is just referring to the
         * GP context we run.  The vcore itself needs to have the FPU state loaded
         * from when it previously ran and was saved (or a fresh FPU if it wasn't
-        * saved).
+        * saved).  For fresh FPUs, the main purpose is for limiting info leakage.
+        * I think VCs that don't need FPU state for some reason (like having a
+        * current_uthread) can handle any sort of FPU state, since it gets sorted
+        * when they pop their next uthread.
         *
-        * Note this can cause a GP fault on x86 if the state is corrupt. */
+        * Note this can cause a GP fault on x86 if the state is corrupt.  In lieu
+        * of reading in the huge FP state and mucking with mxcsr_mask, we should
+        * handle this like a KPF on user code. */
        restore_vc_fp_state(vcpd);
        /* cur_ctx was built above (in actual_ctx), now use it */
        pcpui->cur_ctx = &pcpui->actual_ctx;