Kernel properly handles floating point (XCC)
authorBarret Rhoden <brho@cs.berkeley.edu>
Wed, 17 Apr 2013 23:07:44 +0000 (16:07 -0700)
committerBarret Rhoden <brho@cs.berkeley.edu>
Wed, 17 Apr 2013 23:07:44 +0000 (16:07 -0700)
The kernel saves and restores floating point rather aggressively.  There
are a couple corner cases where we can avoid saving state, such as when
a uthread makes a syscall and gets preempted before the call returns.

The use of the TS flag is actually quite problematic.  We'd need to
reflect it to userspace via VCPD, and it would end up acting as a taint
if uthreads migrate between vcores.  Keep in mind that userspace can't
reset TS on its own, and any attempt to save callee-saved FP state (like
on a user-level context switch) will result in a trap, forcing the FPU
to be turned on.

Rebuild parlib and all applications.

kern/arch/i686/trap.h
kern/arch/riscv/env.c
kern/arch/sparc/env.c
kern/include/process.h
kern/include/ros/event.h
kern/include/trap.h
kern/src/kthread.c
kern/src/process.c
kern/src/schedule.c
user/parlib/uthread.c

index d60cde9..000139b 100644 (file)
@@ -75,17 +75,23 @@ static inline bool in_kernel(struct hw_trapframe *hw_tf)
        return (hw_tf->tf_cs & ~3) == GD_KT;
 }
 
-/* TODO: (HSS) */
 static inline void save_fp_state(struct ancillary_state *silly)
 {
        asm volatile("fxsave %0" : : "m"(*silly));
 }
 
+/* TODO: this can trigger a GP fault if MXCSR reserved bits are set.  Callers
+ * will need to handle intercepting the kernel fault. */
 static inline void restore_fp_state(struct ancillary_state *silly)
 {
        asm volatile("fxrstor %0" : : "m"(*silly));
 }
 
+static inline void init_fp_state(void)
+{
+       asm volatile("fninit");
+}
+
 static inline void __attribute__((always_inline))
 set_stack_pointer(uintptr_t sp)
 {
index e2b2ac9..366b6fc 100644 (file)
@@ -14,6 +14,7 @@ env_push_ancillary_state(env_t* e)
 void
 save_fp_state(ancillary_state_t* silly)
 {
+       /* TODO: save FP state! */
        return; // don't save FP state for now
        uintptr_t sr = mfpcr(PCR_SR);
        mtpcr(PCR_SR, sr | SR_EF);
@@ -108,6 +109,11 @@ restore_fp_state(ancillary_state_t* silly)
        mtpcr(PCR_SR, sr);
 }
 
+void init_fp_state(void)
+{
+       /* TODO: implement me! */
+}
+
 static int
 user_mem_walk_recursive(env_t* e, uintptr_t start, size_t len,
                         mem_walk_callback_t callback, void* arg,
index 71c2271..2bc8c49 100644 (file)
@@ -89,6 +89,11 @@ restore_fp_state(ancillary_state_t* silly)
        write_psr(read_psr() & ~PSR_EF);
 }
 
+void init_fp_state(void)
+{
+       /* TODO: implement me! */
+}
+
 // Flush all mapped pages in the user portion of the address space
 // TODO: only supports L3 user pages
 int
index 042ed69..545970b 100644 (file)
@@ -75,6 +75,7 @@ void proc_destroy(struct proc *p);
 void proc_signal_parent(struct proc *child);
 int __proc_disown_child(struct proc *parent, struct proc *child);
 int proc_change_to_m(struct proc *p);
+void __proc_save_fpu_s(struct proc *p);
 void __proc_save_context_s(struct proc *p, struct user_context *ctx);
 void proc_yield(struct proc *SAFE p, bool being_nice);
 void proc_notify(struct proc *p, uint32_t vcoreid);
index 2168b9a..125d181 100644 (file)
@@ -105,6 +105,9 @@ struct event_queue_big {
 #define VC_UTHREAD_STEALING            0x008                           /* Uthread being stolen */
 #define VC_SCP_NOVCCTX                 0x010                           /* can't go into vc ctx */
 
+/* Racy flags, where we don't need the atomics */
+#define VC_FPU_SAVED                   0x1000                          /* valid FPU state in anc */
+
 /* Per-core data about preemptions and notifications */
 struct preempt_data {
        struct user_context                     vcore_ctx;                      /* for preemptions */
@@ -112,6 +115,7 @@ struct preempt_data {
        struct user_context                     uthread_ctx;            /* for preempts or notifs */
        uintptr_t                                       transition_stack;       /* advertised by the user */
        atomic_t                                        flags;
+       int                                                     rflags;                         /* racy flags */
        bool                                            notif_disabled;         /* vcore unwilling to recv*/
        bool                                            notif_pending;          /* notif k_msg on the way */
        struct event_mbox                       ev_mbox_public;         /* can be read remotely */
index c4fb1d1..6fa9605 100644 (file)
@@ -52,6 +52,7 @@ extern void sysenter_handler();
 
 extern inline void save_fp_state(struct ancillary_state *silly);
 extern inline void restore_fp_state(struct ancillary_state *silly);
+extern inline void init_fp_state(void);
 /* Set stacktop for the current core to be the stack the kernel will start on
  * when trapping/interrupting from userspace */
 void set_stack_top(uintptr_t stacktop);
index cb07a82..a20bd38 100644 (file)
@@ -91,25 +91,11 @@ static void __launch_kthread(uint32_t srcid, long a0, long a1, long a2)
                 * kthread.  This means the _M is getting interrupted or otherwise
                 * delayed.  If we want to do something other than run it (like send the
                 * kmsg to another pcore, or ship the context from here to somewhere
-                * else/deschedule it (like for an _S)), do it here. */
+                * else/deschedule it (like for an _S)), do it here.
+                *
+                * If you want to do something here, call out to the ksched, then
+                * abandon_core(). */
                cmb();  /* do nothing/placeholder */
-               #if 0
-               /* example of something to do (wrap up and schedule an _S).  Note this
-                * might not work perfectly, but is just an example.  One thing to be
-                * careful of is that spin_lock() can't be called if __launch isn't
-                * ROUTINE (which it is right now). */
-               if (pcpui->owning_proc->state == PROC_RUNNING_S) {
-                       spin_lock(&pcpui->owning_proc->proc_lock);
-                       /* Wrap up / yield the _S proc */
-                       __proc_set_state(pcpui->owning_proc, PROC_WAITING);
-                       __proc_save_context_s(pcpui->owning_proc, current_ctx);
-                       spin_unlock(&pcpui->owning_proc->proc_lock);
-                       proc_wakeup(p);
-                       abandon_core();
-                       /* prob need to clear the owning proc?  this is some old shit, so
-                        * don't just uncomment it. */
-               }
-               #endif
        }
        /* o/w, just run the kthread.  any trapframes that are supposed to run or
         * were interrupted will run whenever the kthread smp_idles() or otherwise
index 5554556..ee796d1 100644 (file)
@@ -39,6 +39,8 @@ static uint32_t try_get_pcoreid(struct proc *p, uint32_t vcoreid);
 static uint32_t get_pcoreid(struct proc *p, uint32_t vcoreid);
 static void __proc_free(struct kref *kref);
 static bool scp_is_vcctx_ready(struct preempt_data *vcpd);
+static void save_vc_fp_state(struct preempt_data *vcpd);
+static void restore_vc_fp_state(struct preempt_data *vcpd);
 
 /* PID management. */
 #define PID_MAX 32767 // goes from 0 to 32767, with 0 reserved
@@ -481,7 +483,7 @@ void proc_run_s(struct proc *p)
                        assert(!pcpui->owning_proc);
                        pcpui->owning_proc = p;
                        pcpui->owning_vcoreid = 0; /* TODO (VC#) */
-                       /* TODO: (HSS) set FP state here (__startcore does it instantly) */
+                       restore_vc_fp_state(vcpd);
                        /* similar to the old __startcore, start them in vcore context if
                         * they have notifs and aren't already in vcore context.  o/w, start
                         * them wherever they were before (could be either vc ctx or not) */
@@ -616,17 +618,6 @@ static void __proc_startcore(struct proc *p, struct user_context *ctx)
 {
        assert(!irq_is_enabled());
        __set_proc_current(p);
-       /* need to load our silly state, preferably somewhere other than here so we
-        * can avoid the case where the context was just running here.  it's not
-        * sufficient to do it in the "new process" if-block above (could be things
-        * like page faults that cause us to keep the same process, but want a
-        * different context.
-        * for now, we load this silly state here. (TODO) (HSS)
-        * We also need this to be per trapframe, and not per process...
-        * For now / OSDI, only load it when in _S mode.  _M mode was handled in
-        * __startcore.  */
-       if (p->state == PROC_RUNNING_S)
-               env_pop_ancillary_state(p);
        /* Clear the current_ctx, since it is no longer used */
        current_ctx = 0;        /* TODO: might not need this... */
        proc_pop_ctx(ctx);
@@ -812,7 +803,7 @@ int proc_change_to_m(struct proc *p)
                        /* Copy uthread0's context to VC 0's uthread slot */
                        vcpd->uthread_ctx = *current_ctx;
                        clear_owning_proc(core_id());   /* so we don't restart */
-                       save_fp_state(&vcpd->preempt_anc);
+                       save_vc_fp_state(vcpd);
                        /* Userspace needs to not fuck with notif_disabled before
                         * transitioning to _M. */
                        if (vcpd->notif_disabled) {
@@ -860,6 +851,7 @@ error_out:
  * by the proc. */
 uint32_t __proc_change_to_s(struct proc *p, uint32_t *pc_arr)
 {
+       struct preempt_data *vcpd = &p->procdata->vcore_preempt_data[0];
        uint32_t num_revoked;
        printk("[kernel] trying to transition _M -> _S (deprecated)!\n");
        assert(p->state == PROC_RUNNING_M); // TODO: (ACR) async core req
@@ -867,7 +859,7 @@ uint32_t __proc_change_to_s(struct proc *p, uint32_t *pc_arr)
        assert(current_ctx);
        p->scp_ctx = *current_ctx;
        clear_owning_proc(core_id());   /* so we don't restart */
-       env_push_ancillary_state(p); // TODO: (HSS)
+       save_vc_fp_state(vcpd);
        /* sending death, since it's not our job to save contexts or anything in
         * this case. */
        num_revoked = __proc_take_allcores(p, pc_arr, FALSE);
@@ -906,14 +898,51 @@ static uint32_t get_pcoreid(struct proc *p, uint32_t vcoreid)
        return try_get_pcoreid(p, vcoreid);
 }
 
-/* Helper: saves the SCP's tf state and unmaps vcore 0.  In the future, we'll
- * probably use vc0's space for scp_ctx and the silly state.  If we ever do
- * that, we'll need to stop using scp_ctx (soon to be in VCPD) as a location for
- * pcpui->cur_ctx to point (dangerous) */
+/* Saves the FP state of the calling core into VCPD.  Pairs with
+ * restore_vc_fp_state().  On x86, the best case overhead of the flags:
+ *             FNINIT: 36 ns
+ *             FXSAVE: 46 ns
+ *             FXRSTR: 42 ns
+ *             Flagged FXSAVE: 50 ns
+ *             Flagged FXRSTR: 66 ns
+ *             Excess flagged FXRSTR: 42 ns
+ * If we don't do it, we'll need to initialize every VCPD at process creation
+ * time with a good FPU state (x86 control words are initialized as 0s, like the
+ * rest of VCPD). */
+static void save_vc_fp_state(struct preempt_data *vcpd)
+{
+       save_fp_state(&vcpd->preempt_anc);
+       vcpd->rflags |= VC_FPU_SAVED;
+}
+
+/* Conditionally restores the FP state from VCPD.  If the state was not valid,
+ * we don't bother restoring and just initialize the FPU. */
+static void restore_vc_fp_state(struct preempt_data *vcpd)
+{
+       if (vcpd->rflags & VC_FPU_SAVED) {
+               restore_fp_state(&vcpd->preempt_anc);
+               vcpd->rflags &= ~VC_FPU_SAVED;
+       } else {
+               init_fp_state();
+       }
+}
+
+/* Helper for SCPs, saves the core's FPU state into the VCPD vc0 slot */
+void __proc_save_fpu_s(struct proc *p)
+{
+       struct preempt_data *vcpd = &p->procdata->vcore_preempt_data[0];
+       save_vc_fp_state(vcpd);
+}
+
+/* Helper: saves the SCP's GP tf state and unmaps vcore 0.  This does *not* save
+ * the FPU state.
+ *
+ * In the future, we'll probably use vc0's space for scp_ctx and the silly
+ * state.  If we ever do that, we'll need to stop using scp_ctx (soon to be in
+ * VCPD) as a location for pcpui->cur_ctx to point (dangerous) */
 void __proc_save_context_s(struct proc *p, struct user_context *ctx)
 {
        p->scp_ctx = *ctx;
-       env_push_ancillary_state(p);                    /* TODO: (HSS) */
        __unmap_vcore(p, 0);    /* VC# keep in sync with proc_run_s */
 }
 
@@ -1703,18 +1732,25 @@ static void __set_curctx_to_vcoreid(struct proc *p, uint32_t vcoreid,
         * they'll restart in vcore context.  It's just a matter of whether or not
         * it is the old, interrupted vcore context. */
        if (vcpd->notif_disabled) {
-               restore_fp_state(&vcpd->preempt_anc);
                /* copy-in the tf we'll pop, then set all security-related fields */
                pcpui->actual_ctx = vcpd->vcore_ctx;
                proc_secure_ctx(&pcpui->actual_ctx);
        } else { /* not restarting from a preemption, use a fresh vcore */
                assert(vcpd->transition_stack);
-               /* TODO: consider 0'ing the FP state.  We're probably leaking. */
                proc_init_ctx(&pcpui->actual_ctx, vcoreid, p->env_entry,
                              vcpd->transition_stack);
                /* Disable/mask active notifications for fresh vcores */
                vcpd->notif_disabled = TRUE;
        }
+       /* Regardless of whether or not we have a 'fresh' VC, we need to restore the
+        * FPU state for the VC according to VCPD (which means either a saved FPU
+        * 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).
+        *
+        * Note this can cause a GP fault on x86 if the state is corrupt. */
+       restore_vc_fp_state(vcpd);
        /* cur_ctx was built above (in actual_ctx), now use it */
        pcpui->cur_ctx = &pcpui->actual_ctx;
        /* this cur_ctx will get run when the kernel returns / idles */
@@ -1793,11 +1829,13 @@ int proc_change_to_vcore(struct proc *p, uint32_t new_vcoreid,
                /* if they set this flag, then the vcore can just restart from scratch,
                 * and we don't care about either the uthread_ctx or the vcore_ctx. */
                caller_vcpd->notif_disabled = FALSE;
+               /* Don't need to save the FPU.  There should be no uthread or other
+                * reason to return to the FPU state. */
        } else {
                /* need to set up the calling vcore's ctx so that it'll get restarted by
                 * __startcore, to make the caller look like it was preempted. */
                caller_vcpd->vcore_ctx = *current_ctx;
-               save_fp_state(&caller_vcpd->preempt_anc);
+               save_vc_fp_state(caller_vcpd);
                /* Mark our core as preempted (for userspace recovery). */
                atomic_or(&caller_vcpd->flags, VC_PREEMPTED);
        }
@@ -1956,8 +1994,16 @@ void __preempt(uint32_t srcid, long a0, long a1, long a2)
                vcpd->vcore_ctx = *pcpui->cur_ctx;
        else
                vcpd->uthread_ctx = *pcpui->cur_ctx;
-       /* either way, we save the silly state (FP) */
-       save_fp_state(&vcpd->preempt_anc);
+       /* Userspace in a preemption handler on another core might be copying FP
+        * state from memory (VCPD) at the moment, and if so we don't want to
+        * clobber it.  In this rare case, our current core's FPU state should be
+        * the same as whatever is in VCPD, so this shouldn't be necessary, but the
+        * arch-specific save function might do something other than write out
+        * bit-for-bit the exact same data.  Checking STEALING suffices, since we
+        * hold the K_LOCK (preventing userspace from starting a fresh STEALING
+        * phase concurrently). */
+       if (!(atomic_read(&vcpd->flags) & VC_UTHREAD_STEALING))
+               save_vc_fp_state(vcpd);
        /* Mark the vcore as preempted and unlock (was locked by the sender). */
        atomic_or(&vcpd->flags, VC_PREEMPTED);
        atomic_and(&vcpd->flags, ~VC_K_LOCK);
index 3aa3218..9fd3051 100644 (file)
@@ -371,6 +371,9 @@ static bool __schedule_scp(void)
                        /* locking just to be safe */
                        spin_lock(&p->proc_lock);
                        __proc_set_state(pcpui->owning_proc, PROC_RUNNABLE_S);
+                       /* Saving FP state aggressively.  Odds are, the SCP was hit by an
+                        * IRQ and has a HW ctx, in which case we must save. */
+                       __proc_save_fpu_s(pcpui->owning_proc);
                        __proc_save_context_s(pcpui->owning_proc, pcpui->cur_ctx);
                        spin_unlock(&p->proc_lock);
                        /* round-robin the SCPs (inserts at the end of the queue) */
index 9f86ead..2047c64 100644 (file)
@@ -577,8 +577,6 @@ void uth_enable_notifs(void)
 static bool start_uth_stealing(struct preempt_data *vcpd)
 {
        long old_flags;
-       /* Might not need to bother with the K_LOCK, we aren't talking to the kernel
-        * in these two helpers. */
        do {
                old_flags = atomic_read(&vcpd->flags);
                /* Spin if the kernel is mucking with the flags */