Re-organise the code which manages the owner of the FP state (fpcurproc).

The old code was spread out through the machdep code and was sloppy about
enabling and disabling the FEN bit (which controls access to the FP
register set). This caused a DIAGNOSTIC warning "DANGER WILL ROBINSON:
FEN SET IN cpu_fork!" sometimes when operating under high loads and could
conceivably lead to processes getting incorrect FP results.

The new code is much more strict about the FEN bit and makes sure that
*only* fpcurproc ever has it enabled. This also allows us to remove a
section of code from the exception_return path which might improve
performance marginally.

Reviewed by: gallatin

7 files changed, 164 insertions, 78 deletions

diff --git a/sys/alpha/alpha/fp_emulate.c b/sys/alpha/alpha/fp_emulate.c
index 3b8d6dc..26ecaf7 100644
--- a/sys/alpha/alpha/fp_emulate.c
+++ b/sys/alpha/alpha/fp_emulate.c
@@ -254,16 +254,13 @@ static int fp_emulate(union alpha_instruction ins, struct proc *p)
 		printf("fp_emulate: unhandled opcode = 0x%x, fun = 0x%x\n",ins.common.opcode,ins.f_format.function);
 		return 0;
 	}
+
 	/*
 	 * Dump the float registers into the pcb so we can get at
-	 * them.
+	 * them. We are potentially going to modify the fp state, so
+	 * cancel fpcurproc too.
 	 */
-	if (p == fpcurproc) {
-		alpha_pal_wrfen(1);
-		savefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
-		alpha_pal_wrfen(0);
-		fpcurproc = NULL;
-	}
+	alpha_fpstate_save(p, 1);
 
 	/*
 	 * Decode and execute the instruction.
diff --git a/sys/alpha/alpha/machdep.c b/sys/alpha/alpha/machdep.c
index bdf9660..d582b14 100644
--- a/sys/alpha/alpha/machdep.c
+++ b/sys/alpha/alpha/machdep.c
@@ -1219,12 +1219,7 @@ osendsig(sig_t catcher, int sig, sigset_t *mask, u_long code)
 	ksi.si_sc.sc_regs[R_SP] = alpha_pal_rdusp();
 
 	/* save the floating-point state, if necessary, then copy it. */
-	if (p == fpcurproc) {
-		alpha_pal_wrfen(1);
-		savefpstate(&p->p_addr->u_pcb.pcb_fp);
-		alpha_pal_wrfen(0);
-		fpcurproc = NULL;
-	}
+	alpha_fpstate_save(p, 1);		/* XXX maybe write=0 */
 	ksi.si_sc.sc_ownedfp = p->p_md.md_flags & MDP_FPUSED;
 	bcopy(&p->p_addr->u_pcb.pcb_fp, (struct fpreg *)ksi.si_sc.sc_fpregs,
 	    sizeof(struct fpreg));
@@ -1338,12 +1333,7 @@ sendsig(sig_t catcher, int sig, sigset_t *mask, u_long code)
 	}
 
 	/* save the floating-point state, if necessary, then copy it. */
-	if (p == fpcurproc) {
-		alpha_pal_wrfen(1);
-		savefpstate(&p->p_addr->u_pcb.pcb_fp);
-		alpha_pal_wrfen(0);
-		fpcurproc = NULL;
-	}
+	alpha_fpstate_save(p, 1);
 	sf.sf_uc.uc_mcontext.mc_ownedfp = p->p_md.md_flags & MDP_FPUSED;
 	bcopy(&p->p_addr->u_pcb.pcb_fp,
 	      (struct fpreg *)sf.sf_uc.uc_mcontext.mc_fpregs,
@@ -1451,8 +1441,7 @@ osigreturn(struct proc *p,
 	alpha_pal_wrusp(ksc.sc_regs[R_SP]);
 
 	/* XXX ksc.sc_ownedfp ? */
-	if (p == fpcurproc)
-		fpcurproc = NULL;
+	alpha_fpstate_drop(p);
 	bcopy((struct fpreg *)ksc.sc_fpregs, &p->p_addr->u_pcb.pcb_fp,
 	    sizeof(struct fpreg));
 	p->p_addr->u_pcb.pcb_fp_control = ksc.sc_fp_control;
@@ -1505,8 +1494,7 @@ sigreturn(struct proc *p,
 	SIG_CANTMASK(p->p_sigmask);
 
 	/* XXX ksc.sc_ownedfp ? */
-	if (p == fpcurproc)
-		fpcurproc = NULL;
+	alpha_fpstate_drop(p);
 	bcopy((struct fpreg *)uc.uc_mcontext.mc_fpregs,
 	      &p->p_addr->u_pcb.pcb_fp, sizeof(struct fpreg));
 	p->p_addr->u_pcb.pcb_fp_control = uc.uc_mcontext.mc_fp_control;
@@ -1566,8 +1554,7 @@ setregs(struct proc *p, u_long entry, u_long stack, u_long ps_strings)
 	tfp->tf_regs[FRAME_T12] = tfp->tf_regs[FRAME_PC];	/* a.k.a. PV */
 
 	p->p_md.md_flags &= ~MDP_FPUSED;
-	if (fpcurproc == p)
-		fpcurproc = NULL;
+	alpha_fpstate_drop(p);
 }
 
 int
@@ -1893,11 +1880,7 @@ fill_fpregs(p, fpregs)
 	struct proc *p;
 	struct fpreg *fpregs;
 {
-	if (p == fpcurproc) {
-		alpha_pal_wrfen(1);
-		savefpstate(&p->p_addr->u_pcb.pcb_fp);
-		alpha_pal_wrfen(0);
-	}
+	alpha_fpstate_save(p, 0);
 
 	bcopy(&p->p_addr->u_pcb.pcb_fp, fpregs, sizeof *fpregs);
 	return (0);
@@ -1908,8 +1891,7 @@ set_fpregs(p, fpregs)
 	struct proc *p;
 	struct fpreg *fpregs;
 {
-	if (p == fpcurproc)
-		fpcurproc = NULL;
+	alpha_fpstate_drop(p);
 
 	bcopy(fpregs, &p->p_addr->u_pcb.pcb_fp, sizeof *fpregs);
 	return (0);
@@ -2004,3 +1986,126 @@ SYSCTL_INT(_machdep, CPU_DISRTCSET, disable_rtc_set,
 
 SYSCTL_INT(_machdep, CPU_WALLCLOCK, wall_cmos_clock,
 	CTLFLAG_RW, &wall_cmos_clock, 0, "");
+
+void
+alpha_fpstate_check(struct proc *p)
+{
+	if (p->p_addr->u_pcb.pcb_hw.apcb_flags & ALPHA_PCB_FLAGS_FEN)
+		if (p != fpcurproc)
+			panic("alpha_check_fpcurproc: bogus");
+}
+
+#define SET_FEN(p) \
+	(p)->p_addr->u_pcb.pcb_hw.apcb_flags |= ALPHA_PCB_FLAGS_FEN
+
+#define CLEAR_FEN(p) \
+	(p)->p_addr->u_pcb.pcb_hw.apcb_flags &= ~ALPHA_PCB_FLAGS_FEN
+
+/*
+ * Save the floating point state in the pcb. Use this to get read-only
+ * access to the floating point state. If write is true, the current
+ * fp process is cleared so that fp state can safely be modified. The
+ * process will automatically reload the changed state by generating a 
+ * FEN trap.
+ */
+void
+alpha_fpstate_save(struct proc *p, int write)
+{
+	if (p == fpcurproc) {
+		/*
+		 * If curproc != fpcurproc, then we need to enable FEN 
+		 * so that we can dump the fp state.
+		 */
+		alpha_pal_wrfen(1);
+
+		/*
+		 * Save the state in the pcb.
+		 */
+		savefpstate(&p->p_addr->u_pcb.pcb_fp);
+
+		if (write) {
+			/*
+			 * If fpcurproc == curproc, just ask the
+			 * PALcode to disable FEN, otherwise we must
+			 * clear the FEN bit in fpcurproc's pcb.
+			 */
+			if (fpcurproc == curproc)
+				alpha_pal_wrfen(0);
+			else
+				CLEAR_FEN(fpcurproc);
+			fpcurproc = NULL;
+		} else {
+			/*
+			 * Make sure that we leave FEN enabled if
+			 * curproc == fpcurproc. We must have at most
+			 * one process with FEN enabled. Note that FEN 
+			 * must already be set in fpcurproc's pcb.
+			 */
+			if (curproc != fpcurproc)
+				alpha_pal_wrfen(0);
+		}
+	}
+}
+
+/*
+ * Relinquish ownership of the FP state. This is called instead of
+ * alpha_save_fpstate() if the entire FP state is being changed
+ * (e.g. on sigreturn).
+ */
+void
+alpha_fpstate_drop(struct proc *p)
+{
+	if (p == fpcurproc) {
+		if (p == curproc) {
+			/*
+			 * Disable FEN via the PALcode. This will
+			 * clear the bit in the pcb as well.
+			 */
+			alpha_pal_wrfen(0);
+		} else {
+			/*
+			 * Clear the FEN bit of the pcb.
+			 */
+			CLEAR_FEN(p);
+		}
+		fpcurproc = NULL;
+	}
+}
+
+/*
+ * Switch the current owner of the fp state to p, reloading the state
+ * from the pcb.
+ */
+void
+alpha_fpstate_switch(struct proc *p)
+{
+	/*
+	 * Enable FEN so that we can access the fp registers.
+	 */
+	alpha_pal_wrfen(1);
+	if (fpcurproc) {
+		/*
+		 * Dump the old fp state if its valid.
+		 */
+		savefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
+		CLEAR_FEN(fpcurproc);
+	}
+
+	/*
+	 * Remember the new FP owner and reload its state.
+	 */
+	fpcurproc = p;
+	restorefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
+
+	/*
+	 * If the new owner is curproc, leave FEN enabled, otherwise
+	 * mark its PCB so that it gets FEN when we context switch to
+	 * it later.
+	 */
+	if (p != curproc) {
+		alpha_pal_wrfen(0);
+		SET_FEN(p);
+	}
+
+	p->p_md.md_flags |= MDP_FPUSED;
+}
diff --git a/sys/alpha/alpha/pmap.c b/sys/alpha/alpha/pmap.c
index 5fca8b3..b265980 100644
--- a/sys/alpha/alpha/pmap.c
+++ b/sys/alpha/alpha/pmap.c
@@ -1054,12 +1054,10 @@ pmap_swapout_proc(p)
 	vm_object_t upobj;
 	vm_page_t m;
 
-	if (p == fpcurproc) {
-		alpha_pal_wrfen(1);
-		savefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
-		fpcurproc = NULL;
-		alpha_pal_wrfen(0);
-	}
+	/*
+	 * Make sure we aren't fpcurproc.
+	 */
+	alpha_fpstate_save(p, 1);
 
 	upobj = p->p_upages_obj;
 	/*
diff --git a/sys/alpha/alpha/swtch.s b/sys/alpha/alpha/swtch.s
index c61cc10..ee191eb 100644
--- a/sys/alpha/alpha/swtch.s
+++ b/sys/alpha/alpha/swtch.s
@@ -267,7 +267,7 @@ Lchkast:
 	beq	t0, Lrestoreregs		/* no: just return */
 
 	ldl	t2, astpending			/* AST pending? */
-	beq	t2, Lsetfpenable		/* no: return & deal with FP */
+	beq	t2, Lrestoreregs		/* no: return */
 
 	/* We've got an AST.  Handle it. */
 	ldiq	a0, ALPHA_PSL_IPL_0		/* drop IPL to zero */
@@ -275,15 +275,6 @@ Lchkast:
 	mov	sp, a0				/* only arg is frame */
 	CALL(ast)
 
-Lsetfpenable:
-	/* enable FPU based on whether the current proc is fpcurproc */
-	ldq	t0, curproc
-	ldq	t1, fpcurproc
-	cmpeq	t0, t1, t0
-	mov	zero, a0
-	cmovne	t0, 1, a0
-	call_pal PAL_OSF1_wrfen
-
 Lrestoreregs:
 	/* set the hae register if this process has specified a value */
 	ldq	t0, curproc
diff --git a/sys/alpha/alpha/trap.c b/sys/alpha/alpha/trap.c
index 1924ddd..ab75b4b 100644
--- a/sys/alpha/alpha/trap.c
+++ b/sys/alpha/alpha/trap.c
@@ -218,6 +218,11 @@ trap(a0, a1, a2, entry, framep)
 		sticks = 0;		/* XXX bogus -Wuninitialized warning */
 	}
 
+#ifdef DIAGNOSTIC
+	if (user)
+		alpha_fpstate_check(p);
+#endif
+
 	switch (entry) {
 	case ALPHA_KENTRY_UNA:
 		/*
@@ -327,14 +332,8 @@ trap(a0, a1, a2, entry, framep)
 				goto dopanic;
 			}
 	
-			alpha_pal_wrfen(1);
-			if (fpcurproc)
-				savefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
-			fpcurproc = p;
-			restorefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
-			alpha_pal_wrfen(0);
-
-			p->p_md.md_flags |= MDP_FPUSED;
+			alpha_fpstate_switch(p);
+
 			goto out;
 
 		default:
@@ -570,6 +569,10 @@ syscall(code, framep)
 	opc = framep->tf_regs[FRAME_PC] - 4;
 	sticks = p->p_sticks;
 
+#ifdef DIAGNOSTIC
+	alpha_fpstate_check(p);
+#endif
+
 	if (p->p_sysent->sv_prepsyscall) {
 		/* (*p->p_sysent->sv_prepsyscall)(framep, args, &code, &params); */
 		panic("prepsyscall");
@@ -745,14 +748,6 @@ const static int reg_to_framereg[32] = {
 #define	frp(p, reg)							\
 	(&(p)->p_addr->u_pcb.pcb_fp.fpr_regs[(reg)])
 
-#define	dump_fp_regs()							\
-	if (p == fpcurproc) {						\
-		alpha_pal_wrfen(1);					\
-		savefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);		\
-		alpha_pal_wrfen(0);					\
-		fpcurproc = NULL;					\
-	}
-
 #define	unaligned_load(storage, ptrf, mod)				\
 	if (copyin((caddr_t)va, &(storage), sizeof (storage)) == 0 &&	\
 	    (regptr = ptrf(p, reg)) != NULL)				\
@@ -777,11 +772,11 @@ const static int reg_to_framereg[32] = {
 	unaligned_store(storage, irp, )
 
 #define	unaligned_load_floating(storage, mod)				\
-	dump_fp_regs();							\
+	alpha_fpstate_save(p, 1);					\
 	unaligned_load(storage, frp, mod)
 
 #define	unaligned_store_floating(storage, mod)				\
-	dump_fp_regs();							\
+	alpha_fpstate_save(p, 0);					\
 	unaligned_store(storage, frp, mod)
 
 unsigned long
diff --git a/sys/alpha/alpha/vm_machdep.c b/sys/alpha/alpha/vm_machdep.c
index 7a96d2b..989be9a 100644
--- a/sys/alpha/alpha/vm_machdep.c
+++ b/sys/alpha/alpha/vm_machdep.c
@@ -133,19 +133,17 @@ cpu_fork(p1, p2)
 	 * Copy floating point state from the FP chip to the PCB
 	 * if this process has state stored there.
 	 */
-	if (p1 == fpcurproc) {
-		alpha_pal_wrfen(1);
-		savefpstate(&fpcurproc->p_addr->u_pcb.pcb_fp);
-		alpha_pal_wrfen(0);
-	}
+	alpha_fpstate_save(p1, 0);
 
 	/*
-	 * Copy pcb and stack from proc p1 to p2.
-	 * We do this as cheaply as possible, copying only the active
-	 * part of the stack.  The stack and pcb need to agree;
+	 * Copy pcb and stack from proc p1 to p2.  We do this as
+	 * cheaply as possible, copying only the active part of the
+	 * stack.  The stack and pcb need to agree. Make sure that the 
+	 * new process has FEN disabled.
 	 */
 	p2->p_addr->u_pcb = p1->p_addr->u_pcb;
 	p2->p_addr->u_pcb.pcb_hw.apcb_usp = alpha_pal_rdusp();
+	p2->p_addr->u_pcb.pcb_hw.apcb_flags &= ~ALPHA_PCB_FLAGS_FEN;
 
 	/*
 	 * Set the floating point state.
@@ -165,8 +163,7 @@ cpu_fork(p1, p2)
 #ifdef DIAGNOSTIC
 	if (p1 != curproc)
 		panic("cpu_fork: curproc");
-	if ((up->u_pcb.pcb_hw.apcb_flags & ALPHA_PCB_FLAGS_FEN) != 0)
-		printf("DANGER WILL ROBINSON: FEN SET IN cpu_fork!\n");
+	alpha_fpstate_check(p1);
 #endif
 
 	/*
@@ -241,8 +238,7 @@ void
 cpu_exit(p)
 	register struct proc *p;
 {
-	if (p == fpcurproc)
-		fpcurproc = NULL;
+	alpha_fpstate_drop(p);
 
 	(void) splhigh();
 	cnt.v_swtch++;
diff --git a/sys/alpha/include/cpu.h b/sys/alpha/include/cpu.h
index cddefe1..b9ff12d 100644
--- a/sys/alpha/include/cpu.h
+++ b/sys/alpha/include/cpu.h
@@ -148,6 +148,10 @@ void	XentSys __P((u_int64_t, u_int64_t, u_int64_t));		/* MAGIC */
 void	XentUna __P((u_int64_t, u_int64_t, u_int64_t));		/* MAGIC */
 void	alpha_init __P((u_long, u_long, u_long, u_long, u_long));
 int	alpha_pa_access __P((u_long));
+void	alpha_fpstate_check __P((struct proc *p));
+void	alpha_fpstate_save __P((struct proc *p, int write));
+void	alpha_fpstate_drop __P((struct proc *p));
+void	alpha_fpstate_switch __P((struct proc *p));
 void	ast __P((struct trapframe *));
 int	badaddr	__P((void *, size_t));
 int	badaddr_read __P((void *, size_t, void *));