From 6dfb400106d416bb310e15eb5a66dab9502bab9a Mon Sep 17 00:00:00 2001 From: dfr Date: Wed, 10 Nov 1999 21:14:25 +0000 Subject: [PATCH] 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 --- sys/alpha/alpha/fp_emulate.c | 11 +-- sys/alpha/alpha/machdep.c | 155 ++++++++++++++++++++++++++----- sys/alpha/alpha/pmap.c | 10 +- sys/alpha/alpha/swtch.s | 11 +-- sys/alpha/alpha/trap.c | 29 +++--- sys/alpha/alpha/vm_machdep.c | 20 ++-- sys/alpha/include/cpu.h | 4 + sys/powerpc/aim/vm_machdep.c | 20 ++-- sys/powerpc/powerpc/vm_machdep.c | 20 ++-- 9 files changed, 179 insertions(+), 101 deletions(-) diff --git a/sys/alpha/alpha/fp_emulate.c b/sys/alpha/alpha/fp_emulate.c index 3b8d6dc1f656..26ecaf7d13ef 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 bdf9660beb6b..d582b14a05ba 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 5fca8b3f0dbf..b265980f538c 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 c61cc1018be0..ee191ebd6264 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 1924ddd39b70..ab75b4b18def 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); + alpha_fpstate_switch(p); - p->p_md.md_flags |= MDP_FPUSED; 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, ¶ms); */ 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 7a96d2b76075..989be9a4e038 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 cddefe118ede..b9ff12de5b60 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 *)); diff --git a/sys/powerpc/aim/vm_machdep.c b/sys/powerpc/aim/vm_machdep.c index 7a96d2b76075..989be9a4e038 100644 --- a/sys/powerpc/aim/vm_machdep.c +++ b/sys/powerpc/aim/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/powerpc/powerpc/vm_machdep.c b/sys/powerpc/powerpc/vm_machdep.c index 7a96d2b76075..989be9a4e038 100644 --- a/sys/powerpc/powerpc/vm_machdep.c +++ b/sys/powerpc/powerpc/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++;