Author: kib
Date: Tue Jul 24 19:22:52 2018
New Revision: 336683
URL: https://svnweb.freebsd.org/changeset/base/336683

Log:
  Extend ranges of the critical sections to ensure that context switch
  code never sees FPU pcb flags not consistent with the hardware state.
  
  This is uncovered by the eager FPU switch mode.
  
  Analyzed, reviewed and tested by:     gleb
  Sponsored by: The FreeBSD Foundation
  MFC after:    1 week

Modified:
  head/sys/amd64/amd64/fpu.c
  head/sys/amd64/amd64/machdep.c
  head/sys/i386/i386/machdep.c
  head/sys/i386/i386/npx.c

Modified: head/sys/amd64/amd64/fpu.c
==============================================================================
--- head/sys/amd64/amd64/fpu.c  Tue Jul 24 19:21:11 2018        (r336682)
+++ head/sys/amd64/amd64/fpu.c  Tue Jul 24 19:22:52 2018        (r336683)
@@ -783,22 +783,22 @@ fpugetregs(struct thread *td)
        int max_ext_n, i, owned;
 
        pcb = td->td_pcb;
+       critical_enter();
        if ((pcb->pcb_flags & PCB_USERFPUINITDONE) == 0) {
                bcopy(fpu_initialstate, get_pcb_user_save_pcb(pcb),
                    cpu_max_ext_state_size);
                get_pcb_user_save_pcb(pcb)->sv_env.en_cw =
                    pcb->pcb_initial_fpucw;
                fpuuserinited(td);
+               critical_exit();
                return (_MC_FPOWNED_PCB);
        }
-       critical_enter();
        if (td == PCPU_GET(fpcurthread) && PCB_USER_FPU(pcb)) {
                fpusave(get_pcb_user_save_pcb(pcb));
                owned = _MC_FPOWNED_FPU;
        } else {
                owned = _MC_FPOWNED_PCB;
        }
-       critical_exit();
        if (use_xsave) {
                /*
                 * Handle partially saved state.
@@ -818,6 +818,7 @@ fpugetregs(struct thread *td)
                        *xstate_bv |= bit;
                }
        }
+       critical_exit();
        return (owned);
 }
 
@@ -826,6 +827,7 @@ fpuuserinited(struct thread *td)
 {
        struct pcb *pcb;
 
+       CRITICAL_ASSERT(td);
        pcb = td->td_pcb;
        if (PCB_USER_FPU(pcb))
                set_pcb_flags(pcb,
@@ -884,26 +886,25 @@ fpusetregs(struct thread *td, struct savefpu *addr, ch
 
        addr->sv_env.en_mxcsr &= cpu_mxcsr_mask;
        pcb = td->td_pcb;
+       error = 0;
        critical_enter();
        if (td == PCPU_GET(fpcurthread) && PCB_USER_FPU(pcb)) {
                error = fpusetxstate(td, xfpustate, xfpustate_size);
-               if (error != 0) {
-                       critical_exit();
-                       return (error);
+               if (error == 0) {
+                       bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
+                       fpurestore(get_pcb_user_save_td(td));
+                       set_pcb_flags(pcb, PCB_FPUINITDONE |
+                           PCB_USERFPUINITDONE);
                }
-               bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
-               fpurestore(get_pcb_user_save_td(td));
-               critical_exit();
-               set_pcb_flags(pcb, PCB_FPUINITDONE | PCB_USERFPUINITDONE);
        } else {
-               critical_exit();
                error = fpusetxstate(td, xfpustate, xfpustate_size);
-               if (error != 0)
-                       return (error);
-               bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
-               fpuuserinited(td);
+               if (error == 0) {
+                       bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
+                       fpuuserinited(td);
+               }
        }
-       return (0);
+       critical_exit();
+       return (error);
 }
 
 /*
@@ -1077,6 +1078,7 @@ fpu_kern_enter(struct thread *td, struct fpu_kern_ctx 
                ctx->flags = FPU_KERN_CTX_DUMMY | FPU_KERN_CTX_INUSE;
                return;
        }
+       critical_enter();
        KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
            get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
        ctx->flags = FPU_KERN_CTX_INUSE;
@@ -1087,7 +1089,7 @@ fpu_kern_enter(struct thread *td, struct fpu_kern_ctx 
        pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
        set_pcb_flags(pcb, PCB_KERNFPU);
        clear_pcb_flags(pcb, PCB_FPUINITDONE);
-       return;
+       critical_exit();
 }
 
 int
@@ -1105,7 +1107,6 @@ fpu_kern_leave(struct thread *td, struct fpu_kern_ctx 
 
                clear_pcb_flags(pcb,  PCB_FPUNOSAVE | PCB_FPUINITDONE);
                start_emulating();
-               critical_exit();
        } else {
                KASSERT((ctx->flags & FPU_KERN_CTX_INUSE) != 0,
                    ("leaving not inuse ctx"));
@@ -1119,7 +1120,6 @@ fpu_kern_leave(struct thread *td, struct fpu_kern_ctx 
                critical_enter();
                if (curthread == PCPU_GET(fpcurthread))
                        fpudrop();
-               critical_exit();
                pcb->pcb_save = ctx->prev;
        }
 
@@ -1136,6 +1136,7 @@ fpu_kern_leave(struct thread *td, struct fpu_kern_ctx 
                        clear_pcb_flags(pcb, PCB_FPUINITDONE);
                KASSERT(!PCB_USER_FPU(pcb), ("unpaired fpu_kern_leave"));
        }
+       critical_exit();
        return (0);
 }
 

Modified: head/sys/amd64/amd64/machdep.c
==============================================================================
--- head/sys/amd64/amd64/machdep.c      Tue Jul 24 19:21:11 2018        
(r336682)
+++ head/sys/amd64/amd64/machdep.c      Tue Jul 24 19:22:52 2018        
(r336683)
@@ -2171,8 +2171,10 @@ int
 set_fpregs(struct thread *td, struct fpreg *fpregs)
 {
 
+       critical_enter();
        set_fpregs_xmm(fpregs, get_pcb_user_save_td(td));
        fpuuserinited(td);
+       critical_exit();
        return (0);
 }
 

Modified: head/sys/i386/i386/machdep.c
==============================================================================
--- head/sys/i386/i386/machdep.c        Tue Jul 24 19:21:11 2018        
(r336682)
+++ head/sys/i386/i386/machdep.c        Tue Jul 24 19:22:52 2018        
(r336683)
@@ -2874,6 +2874,7 @@ int
 set_fpregs(struct thread *td, struct fpreg *fpregs)
 {
 
+       critical_enter();
        if (cpu_fxsr)
                npx_set_fpregs_xmm((struct save87 *)fpregs,
                    &get_pcb_user_save_td(td)->sv_xmm);
@@ -2881,6 +2882,7 @@ set_fpregs(struct thread *td, struct fpreg *fpregs)
                bcopy(fpregs, &get_pcb_user_save_td(td)->sv_87,
                    sizeof(*fpregs));
        npxuserinited(td);
+       critical_exit();
        return (0);
 }
 

Modified: head/sys/i386/i386/npx.c
==============================================================================
--- head/sys/i386/i386/npx.c    Tue Jul 24 19:21:11 2018        (r336682)
+++ head/sys/i386/i386/npx.c    Tue Jul 24 19:22:52 2018        (r336683)
@@ -966,14 +966,15 @@ npxgetregs(struct thread *td)
                return (_MC_FPOWNED_NONE);
 
        pcb = td->td_pcb;
+       critical_enter();
        if ((pcb->pcb_flags & PCB_NPXINITDONE) == 0) {
                bcopy(npx_initialstate, get_pcb_user_save_pcb(pcb),
                    cpu_max_ext_state_size);
                SET_FPU_CW(get_pcb_user_save_pcb(pcb), pcb->pcb_initial_npxcw);
                npxuserinited(td);
+               critical_exit();
                return (_MC_FPOWNED_PCB);
        }
-       critical_enter();
        if (td == PCPU_GET(fpcurthread)) {
                fpusave(get_pcb_user_save_pcb(pcb));
                if (!cpu_fxsr)
@@ -987,7 +988,6 @@ npxgetregs(struct thread *td)
        } else {
                owned = _MC_FPOWNED_PCB;
        }
-       critical_exit();
        if (use_xsave) {
                /*
                 * Handle partially saved state.
@@ -1010,6 +1010,7 @@ npxgetregs(struct thread *td)
                        *xstate_bv |= bit;
                }
        }
+       critical_exit();
        return (owned);
 }
 
@@ -1018,6 +1019,7 @@ npxuserinited(struct thread *td)
 {
        struct pcb *pcb;
 
+       CRITICAL_ASSERT(td);
        pcb = td->td_pcb;
        if (PCB_USER_FPU(pcb))
                pcb->pcb_flags |= PCB_NPXINITDONE;
@@ -1075,28 +1077,26 @@ npxsetregs(struct thread *td, union savefpu *addr, cha
        if (cpu_fxsr)
                addr->sv_xmm.sv_env.en_mxcsr &= cpu_mxcsr_mask;
        pcb = td->td_pcb;
+       error = 0;
        critical_enter();
        if (td == PCPU_GET(fpcurthread) && PCB_USER_FPU(pcb)) {
                error = npxsetxstate(td, xfpustate, xfpustate_size);
-               if (error != 0) {
-                       critical_exit();
-                       return (error);
+               if (error == 0) {
+                       if (!cpu_fxsr)
+                               fnclex();       /* As in npxdrop(). */
+                       bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
+                       fpurstor(get_pcb_user_save_td(td));
+                       pcb->pcb_flags |= PCB_NPXUSERINITDONE | PCB_NPXINITDONE;
                }
-               if (!cpu_fxsr)
-                       fnclex();       /* As in npxdrop(). */
-               bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
-               fpurstor(get_pcb_user_save_td(td));
-               critical_exit();
-               pcb->pcb_flags |= PCB_NPXUSERINITDONE | PCB_NPXINITDONE;
        } else {
-               critical_exit();
                error = npxsetxstate(td, xfpustate, xfpustate_size);
-               if (error != 0)
-                       return (error);
-               bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
-               npxuserinited(td);
+               if (error == 0) {
+                       bcopy(addr, get_pcb_user_save_td(td), sizeof(*addr));
+                       npxuserinited(td);
+               }
        }
-       return (0);
+       critical_exit();
+       return (error);
 }
 
 static void
@@ -1364,6 +1364,7 @@ fpu_kern_enter(struct thread *td, struct fpu_kern_ctx 
                return;
        }
        pcb = td->td_pcb;
+       critical_enter();
        KASSERT(!PCB_USER_FPU(pcb) || pcb->pcb_save ==
            get_pcb_user_save_pcb(pcb), ("mangled pcb_save"));
        ctx->flags = FPU_KERN_CTX_INUSE;
@@ -1374,7 +1375,7 @@ fpu_kern_enter(struct thread *td, struct fpu_kern_ctx 
        pcb->pcb_save = fpu_kern_ctx_savefpu(ctx);
        pcb->pcb_flags |= PCB_KERNNPX;
        pcb->pcb_flags &= ~PCB_NPXINITDONE;
-       return;
+       critical_exit();
 }
 
 int
@@ -1392,7 +1393,6 @@ fpu_kern_leave(struct thread *td, struct fpu_kern_ctx 
        critical_enter();
        if (curthread == PCPU_GET(fpcurthread))
                npxdrop();
-       critical_exit();
        pcb->pcb_save = ctx->prev;
        if (pcb->pcb_save == get_pcb_user_save_pcb(pcb)) {
                if ((pcb->pcb_flags & PCB_NPXUSERINITDONE) != 0)
@@ -1407,6 +1407,7 @@ fpu_kern_leave(struct thread *td, struct fpu_kern_ctx 
                        pcb->pcb_flags &= ~PCB_NPXINITDONE;
                KASSERT(!PCB_USER_FPU(pcb), ("unpaired fpu_kern_leave"));
        }
+       critical_exit();
        return (0);
 }
 
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "[email protected]"

Reply via email to