Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/arch/i386/isa Since we always run with CR0.NE set (inter...



details:   https://anonhg.NetBSD.org/src/rev/fb19fb6d97a6
branches:  trunk
changeset: 326573:fb19fb6d97a6
user:      dsl <dsl%NetBSD.org@localhost>
date:      Mon Feb 03 23:00:32 2014 +0000

description:
Since we always run with CR0.NE set (internal fpu using vector 0x10)
npxintr() is only generated when a process executes an FP instruction
and the TS set interrupt takes precedence - so we know the current lwp
owns the fp registers.
It is also then impossible to get a splurious error while saving
the registers.
Ths lets the npxintr() code be simplified somewhat.
XXX: I'm not at all sure it really DTRT if the process actually wished
to fixup anything in the signal handler (or even if the signal is masked).

diffstat:

 sys/arch/i386/isa/npx.c |  162 +++++++++++++++++++++--------------------------
 1 files changed, 72 insertions(+), 90 deletions(-)

diffs (241 lines):

diff -r 040b771aced8 -r fb19fb6d97a6 sys/arch/i386/isa/npx.c
--- a/sys/arch/i386/isa/npx.c   Mon Feb 03 21:22:21 2014 +0000
+++ b/sys/arch/i386/isa/npx.c   Mon Feb 03 23:00:32 2014 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: npx.c,v 1.150 2014/02/02 22:41:20 dsl Exp $    */
+/*     $NetBSD: npx.c,v 1.151 2014/02/03 23:00:32 dsl Exp $    */
 
 /*-
  * Copyright (c) 2008 The NetBSD Foundation, Inc.
@@ -96,7 +96,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: npx.c,v 1.150 2014/02/02 22:41:20 dsl Exp $");
+__KERNEL_RCSID(0, "$NetBSD: npx.c,v 1.151 2014/02/03 23:00:32 dsl Exp $");
 
 #if 0
 #define IPRINTF(x)     printf x
@@ -127,6 +127,11 @@
  * an external fpu, only the one that is part of the cpu fabric.
  * A 486 might not have an fpu, but the 487 is a full 486.
  *
+ * Since we set CR0.NE a FP exception can only happen when a user process
+ * executes a FP instruction. The DNA exception takes precedence, so
+ * the execption can only happen when the lwp already owns the fpu.
+ * In particular the exceptions won't happen in-kernel while saving state.
+ *
  * We do lazy initialization and switching using the TS bit in cr0 and the
  * MDL_USEDFPU bit in mdlwp.
  *
@@ -161,19 +166,6 @@
 
 struct npx_softc               *npx_softc;
 
-static inline void
-fpu_save(union savefpu *addr)
-{
-       if (i386_use_fxsave)
-       {
-                fxsave(&addr->sv_xmm);
-
-               /* FXSAVE doesn't FNINIT like FNSAVE does -- so do it here. */
-               fninit();
-       } else
-               fnsave(&addr->sv_87);
-}
-
 #ifndef XEN
 /* Initialise fpu, might be boot cpu or a later cpu coming online */
 void
@@ -221,7 +213,11 @@
  * best a handler could do an fninit followed by an fldcw of a static value.
  * fnclex would be of little use because it would leave junk on the FPU stack.
  *
- * Only called dircetly from i386_trap.S
+ * Only called directly from i386_trap.S (with interrupts disabled)
+ *
+ * Since we have CR0.NE set this can only happen as a result of
+ * executing an FP instruction (and after CR0.TS has been checked).
+ * This means this must be from userspace on the current lwp.
  */
 int npxintr(void *, struct intrframe *);
 int
@@ -231,27 +227,25 @@
        struct lwp *l = ci->ci_fpcurlwp;
        union savefpu *addr;
        struct pcb *pcb;
+       uint32_t statbits;
        ksiginfo_t ksi;
 
+       if (!USERMODE(frame->if_cs, frame->if_eflags))
+               panic("fpu trap from kernel\n");
+
        kpreempt_disable();
 #ifndef XEN
        KASSERT((x86_read_psl() & PSL_I) == 0);
        x86_enable_intr();
 #endif
 
-       curcpu()->ci_data.cpu_ntrap++;
        IPRINTF(("%s: fp intr\n", device_xname(ci->ci_dev)));
 
        /*
-        * If we're saving, ignore the interrupt.  The FPU will generate
-        * another one when we restore the state later.
+        * At this point, fpcurlwp should be curlwp.  If it wasn't, the TS
+        * bit should be set, and we should have gotten a DNA exception.
         */
-       if (ci->ci_fpsaving) {
-               kpreempt_enable();
-               return (1);
-       }
-
-       if (l == NULL || !i386_fpu_present) {
+       if (l != curlwp || !i386_fpu_present) {
                printf("npxintr: l = %p, curproc = %p, fpu_present = %d\n",
                    l, curproc, i386_fpu_present);
                printf("npxintr: came from nowhere");
@@ -259,89 +253,77 @@
                return 1;
        }
 
-       /*
-        * At this point, fpcurlwp should be curlwp.  If it wasn't, the TS
-        * bit should be set, and we should have gotten a DNA exception.
-        */
-       KASSERT(l == curlwp);
+       /* Find the address of fpcurproc's saved FPU state. */
        pcb = lwp_getpcb(l);
-
-       /*
-        * Find the address of fpcurproc's saved FPU state.  (Given the
-        * invariant above, this is always the one in curpcb.)
-        */
        addr = &pcb->pcb_savefpu;
 
        /*
-        * Save state.  This does an implied fninit.  It had better not halt
-        * the CPU or we'll hang.
+        * XXX: (dsl) I think this is all borked.
+        * We need to save the status word (which contains the cause)
+        * of the fault, and clear the relevant error bits so that
+        * the fp instruction doesn't trap again when the signal handler
+        * returns (or if the signal is ignored).
+        * What the code actually does is to reset the FPU, this clears
+        * the FP stack pointer so I suspect the code then gets the
+        * wrong register values for an later operations.
+        * Any code that enabled the stack under/overflow traps is doomed.
+        *
+        * I think this code should just save the status word and clear
+        * the pending errors.
+        * If the signal is generated then the FP state can be saved in
+        * the context stashed on the user stack, and restrored from
+        * there later. So this code need not do a fsave or finit.
         */
-       fpu_save(addr);
-       fwait();
-        if (i386_use_fxsave) {
+       if (i386_use_fxsave) {
+               fxsave(&addr->sv_xmm);
+               /* FXSAVE doesn't FNINIT like FNSAVE does -- so do it here. */
+               fninit();
+               fwait();
                fldcw(&addr->sv_xmm.fx_cw);
                /*
                 * FNINIT doesn't affect MXCSR or the XMM registers;
                 * no need to re-load MXCSR here.
                 */
-        } else
-                fldcw(&addr->sv_87.s87_cw);
+               statbits = addr->sv_xmm.fx_sw;
+       } else {
+               fnsave(&addr->sv_87);
+               /* fnsave has done an fninit() */
+               fwait();
+               fldcw(&addr->sv_87.s87_cw);
+               statbits = addr->sv_87.s87_sw;
+       }
        fwait();
-       /*
-        * Remember the exception status word and tag word.  The current
-        * (almost fninit'ed) fpu state is in the fpu and the exception
-        * state just saved will soon be junk.  However, the implied fninit
-        * doesn't change the error pointers or register contents, and we
-        * preserved the control word and will copy the status and tag
-        * words, so the complete exception state can be recovered.
-        */
-        if (i386_use_fxsave) {
-               addr->sv_xmm.sv_ex_sw = addr->sv_xmm.fx_sw;
-               addr->sv_xmm.sv_ex_tw = addr->sv_xmm.fx_tw;
-       } else {
-               addr->sv_87.s87_ex_sw = addr->sv_87.s87_sw;
-               addr->sv_87.s87_ex_tw = addr->sv_87.s87_tw;
-       }
+
        /*
         * Pass exception to process.
         */
-       if (USERMODE(frame->if_cs, frame->if_eflags)) {
-               /*
-                * Interrupt is essentially a trap, so we can afford to call
-                * the SIGFPE handler (if any) as soon as the interrupt
-                * returns.
-                *
-                * XXX little or nothing is gained from this, and plenty is
-                * lost - the interrupt frame has to contain the trap frame
-                * (this is otherwise only necessary for the rescheduling trap
-                * in doreti, and the frame for that could easily be set up
-                * just before it is used).
-                */
-               l->l_md.md_regs = (struct trapframe *)&frame->if_gs;
 
-               KSI_INIT_TRAP(&ksi);
-               ksi.ksi_signo = SIGFPE;
-               ksi.ksi_addr = (void *)frame->if_eip;
+       /*
+        * Interrupt is essentially a trap, so we can afford to call
+        * the SIGFPE handler (if any) as soon as the interrupt
+        * returns.
+        *
+        * XXX little or nothing is gained from this, and plenty is
+        * lost - the interrupt frame has to contain the trap frame
+        * (this is otherwise only necessary for the rescheduling trap
+        * in doreti, and the frame for that could easily be set up
+        * just before it is used).
+        */
+       l->l_md.md_regs = (struct trapframe *)&frame->if_gs;
 
-               /*
-                * Encode the appropriate code for detailed information on
-                * this exception.
-                */
+       KSI_INIT_TRAP(&ksi);
+       ksi.ksi_signo = SIGFPE;
+       ksi.ksi_addr = (void *)frame->if_eip;
 
-               if (i386_use_fxsave) {
-                       ksi.ksi_code =
-                               x86fpflags_to_ksiginfo(addr->sv_xmm.sv_ex_sw);
-                       ksi.ksi_trap = (int)addr->sv_xmm.sv_ex_sw;
-               } else {
-                       ksi.ksi_code =
-                               x86fpflags_to_ksiginfo(addr->sv_87.s87_ex_sw);
-                       ksi.ksi_trap = (int)addr->sv_87.s87_ex_sw;
-               }
+       /*
+        * Encode the appropriate code for detailed information on
+        * this exception.
+        */
 
-               trapsignal(l, &ksi);
-       } else {
-               panic("fpu trap from kernel\n");
-       }
+       ksi.ksi_code = x86fpflags_to_ksiginfo(statbits);
+       ksi.ksi_trap = statbits;
+
+       trapsignal(l, &ksi);
 
        kpreempt_enable();
        return (1);



Home | Main Index | Thread Index | Old Index