parlib: Fix aggressively-saved FP state for signals
authorBarret Rhoden <brho@cs.berkeley.edu>
Fri, 15 Jun 2018 18:59:07 +0000 (14:59 -0400)
committerBarret Rhoden <brho@cs.berkeley.edu>
Fri, 15 Jun 2018 19:15:00 +0000 (15:15 -0400)
In commit 5562ccb05015 ("parlib: Aggressively save and restore the FPU"),
we did not fix up the signal handling code.

The old __prep_sighandler() code was assuming that if a uthread didn't have
its GP register state saved, then it had FP state in the hardware and the
FPSAVED was not set.  This would be for e.g. HW contexts that got
interrupted.

With the change to aggressively save the FPU, that was not true: it was
saved and the bit was set.  Your GP state could be not saved, but your FP
state would be.

That would lead to other sorts of confusion, since the FPSAVED flag was on
for the signal handler.  That was causing an assertion when we tried to
run_current_uthread, which was a SW ctx (the signal handler).

Signed-off-by: Barret Rhoden <brho@cs.berkeley.edu>
user/parlib/signal.c

index 48f637d..dbe6b42 100644 (file)
@@ -156,10 +156,6 @@ static void __prep_sighandler(struct uthread *uthread,
                stack = get_user_ctx_sp(ctx) - sizeof(struct sigdata);
                assert(stack_ptr_is_sane(stack));
                uthread->sigstate.data = (struct sigdata*)stack;
                stack = get_user_ctx_sp(ctx) - sizeof(struct sigdata);
                assert(stack_ptr_is_sane(stack));
                uthread->sigstate.data = (struct sigdata*)stack;
-               if (uthread->flags & UTHREAD_FPSAVED) {
-                       uthread->sigstate.data->as = uthread->as;
-                       uthread->flags &= ~UTHREAD_FPSAVED;
-               }
        } else {
                assert(current_uthread == uthread);
                ctx = &vcpd_of(vcore_id())->uthread_ctx;
        } else {
                assert(current_uthread == uthread);
                ctx = &vcpd_of(vcore_id())->uthread_ctx;
@@ -167,12 +163,37 @@ static void __prep_sighandler(struct uthread *uthread,
                stack = ROUNDDOWN(stack, __alignof__(struct sigdata));
                assert(stack_ptr_is_sane(stack));
                uthread->sigstate.data = (struct sigdata*)stack;
                stack = ROUNDDOWN(stack, __alignof__(struct sigdata));
                assert(stack_ptr_is_sane(stack));
                uthread->sigstate.data = (struct sigdata*)stack;
-               save_fp_state(&uthread->sigstate.data->as);
        }
        }
+       /* Parlib aggressively saves the FP state for HW and VM ctxs.  SW ctxs
+        * should not have FP state saved. */
+       switch (uthread->u_ctx.type) {
+       case ROS_HW_CTX:
+       case ROS_VM_CTX:
+               assert(uthread->flags & UTHREAD_FPSAVED);
+               /* We need to save the already-saved FP state into the sigstate space.
+                * The sig handler is taking over the uthread and its GP and FP spaces.
+                *
+                * If we ever go back to not aggressively saving the FP state, then for
+                * HW and VM ctxs, the state is in hardware.  Regardless, we still need
+                * to save it in ->as, with something like:
+                *                      save_fp_state(&uthread->sigstate.data->as);
+                * Either way, when we're done with this entire function, the *uthread*
+                * will have ~UTHREAD_FPSAVED, since we will be talking about the SW
+                * context that is running the signal handler. */
+               uthread->sigstate.data->as = uthread->as;
+               uthread->flags &= ~UTHREAD_FPSAVED;
+               break;
+       case ROS_SW_CTX:
+               assert(!(uthread->flags & UTHREAD_FPSAVED));
+               break;
+       };
        if (info != NULL)
                uthread->sigstate.data->info = *info;
 
        init_user_ctx(&uthread->sigstate.data->u_ctx, (uintptr_t)entry, stack);
        if (info != NULL)
                uthread->sigstate.data->info = *info;
 
        init_user_ctx(&uthread->sigstate.data->u_ctx, (uintptr_t)entry, stack);
+       /* The uthread may or may not be UTHREAD_SAVED.  That depends on whether the
+        * uthread was in that state initially.  We're swapping into the location of
+        * 'ctx', which is either in VCPD or the uth itself. */
        swap_user_contexts(ctx, &uthread->sigstate.data->u_ctx);
 }
 
        swap_user_contexts(ctx, &uthread->sigstate.data->u_ctx);
 }