Skip site navigation (1)Skip section navigation (2) 
Date:      Tue, 24 Jul 2018 19:22:52 +0000 (UTC)
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r336683 - in head/sys: amd64/amd64 i386/i386
Message-ID:  <201807241922.w6OJMqoH021911@repo.freebsd.org>
next in thread | raw e-mail | index | archive | help 
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);
 }

Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201807241922.w6OJMqoH021911>