Date: Sun, 15 Sep 2002 19:43:38 -0700 (PDT) From: Jonathan Mini <mini@FreeBSD.org> To: Perforce Change Reviews <perforce@freebsd.org> Subject: PERFORCE change 17551 for review Message-ID: <200209160243.g8G2hc0w005586@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
http://people.freebsd.org/~peter/p4db/chv.cgi?CH=17551 Change 17551 by mini@mini_stylus on 2002/09/15 19:43:22 Add Bruce's fix for two NPX problems: 1) a panic in npxdna() where emulation was enabled, but we owned the coprocessor. 2) a trap 9 in fxrstor, caused by unaligned access. Affected files ... .. //depot/projects/kse/sys/i386/i386/machdep.c#59 edit Differences ... ==== //depot/projects/kse/sys/i386/i386/machdep.c#59 (text+ko) ==== @@ -917,6 +917,20 @@ */ load_cr0(rcr0() | CR0_MP | CR0_TS); + /* Initialize the npx (if any) for the current process. */ + /* + * XXX the above load_cr0() also initializes it and is a layering + * violation if NPX is configured. It drops the npx partially + * and this would be fatal if we were interrupted now, and decided + * to force the state to the pcb, and checked the invariant + * (CR0_TS clear) if and only if PCPU_GET(fpcurthread) != NULL). + * ALL of this can happen except the check. The check used to + * happen and be fatal later when we didn't complete the drop + * before returning to user mode. This should be fixed properly + * soon. + */ + fpstate_drop(td); + /* * XXX - Linux emulator * Make sure sure edx is 0x0 on entry. Linux binaries depend @@ -2139,7 +2153,32 @@ mcp->mc_fpformat = _MC_FPFMT_NODEV; mcp->mc_ownedfp = _MC_FPOWNED_NONE; #else - mcp->mc_ownedfp = npxgetregs(td, (union savefpu *)&mcp->mc_fpstate); + union savefpu *addr; + + /* + * XXX mc_fpstate might be misaligned, since its declaration is not + * unportabilized using __attribute__((aligned(16))) like the + * declaration of struct savemm, and anyway, alignment doesn't work + * for auto variables since we don't use gcc's pessimal stack + * alignment. Work around this by abusing the spare fields after + * mcp->mc_fpstate. + * + * XXX unpessimize most cases by only aligning when fxsave might be + * called, although this requires knowing too much about + * npxgetregs()'s internals. + */ + addr = (union savefpu *)&mcp->mc_fpstate; + if (td == PCPU_GET(fpcurthread) && cpu_fxsr && + ((uintptr_t)(void *)addr & 0xF)) { + do + addr = (void *)((char *)addr + 4); + while ((uintptr_t)(void *)addr & 0xF); + } + mcp->mc_ownedfp = npxgetregs(td, addr); + if (addr != (union savefpu *)&mcp->mc_fpstate) { + bcopy(addr, &mcp->mc_fpstate, sizeof(mcp->mc_fpstate)); + bzero(&mcp->mc_spare2, sizeof(mcp->mc_spare2)); + } mcp->mc_fpformat = npxformat(); #endif } @@ -2147,19 +2186,40 @@ static int set_fpcontext(struct thread *td, const mcontext_t *mcp) { + union savefpu *addr; if (mcp->mc_fpformat == _MC_FPFMT_NODEV) return (0); - else if ((mcp->mc_fpformat != _MC_FPFMT_387) && - ((mcp->mc_fpformat != _MC_FPFMT_XMM))) + else if (mcp->mc_fpformat != _MC_FPFMT_387 && + mcp->mc_fpformat != _MC_FPFMT_XMM) return (EINVAL); else if (mcp->mc_ownedfp == _MC_FPOWNED_NONE) /* We don't care what state is left in the FPU or PCB. */ fpstate_drop(td); else if (mcp->mc_ownedfp == _MC_FPOWNED_FPU || - mcp->mc_ownedfp == _MC_FPOWNED_PCB) - npxsetregs(td, (union savefpu *)&mcp->mc_fpstate); - else + mcp->mc_ownedfp == _MC_FPOWNED_PCB) { + /* XXX align as above. */ + addr = (union savefpu *)&mcp->mc_fpstate; + if (td == PCPU_GET(fpcurthread) && cpu_fxsr && + ((uintptr_t)(void *)addr & 0xF)) { + do + addr = (void *)((char *)addr + 4); + while ((uintptr_t)(void *)addr & 0xF); + bcopy(&mcp->mc_fpstate, addr, sizeof(mcp->mc_fpstate)); + } +#ifdef DEV_NPX + /* + * XXX we violate the dubious requirement that npxsetregs() + * be called with interrupts disabled. + */ + npxsetregs(td, addr); +#endif + /* + * Don't bother putting things back where they were in the + * misaligned case, since we know that the caller won't use + * them again. + */ + } else return (EINVAL); return (0); } @@ -2170,8 +2230,21 @@ register_t s; s = intr_disable(); +#ifdef DEV_NPX if (PCPU_GET(fpcurthread) == td) npxdrop(); +#endif + /* + * XXX force a full drop of the npx. The above only drops it if we + * owned it. npxgetregs() has the same bug in the !cpu_fxsr case. + * + * XXX I don't much like npxgetregs()'s semantics of doing a full + * drop. Dropping only to the pcb matches fnsave's behaviour. + * We only need to drop to !PCB_INITDONE in sendsig(). But + * sendsig() is the only caller of npxgetregs()... perhaps we just + * have too many layers. + */ + curthread->td_pcb->pcb_flags &= ~PCB_NPXINITDONE; intr_restore(s); } To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe p4-projects" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200209160243.g8G2hc0w005586>