CVS commit: src/sys/arch/x86/x86

[Taylor R Campbell]

Module Name:    src
Committed By:   riastradh
Date:           Mon Jul  6 01:08:15 UTC 2020

Modified Files:
        src/sys/arch/x86/x86: fpu.c

Log Message:
Fix race in fpu save with fpu_kern_enter in softint.

Likely source of:

https://mail-index.netbsd.org/current-users/2020/07/02/msg039051.html


To generate a diff of this commit:
cvs rdiff -u -r1.65 -r1.66 src/sys/arch/x86/x86/fpu.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.


Modified files:

Index: src/sys/arch/x86/x86/fpu.c
diff -u src/sys/arch/x86/x86/fpu.c:1.65 src/sys/arch/x86/x86/fpu.c:1.66
--- src/sys/arch/x86/x86/fpu.c:1.65	Sun Jun 14 16:12:05 2020
+++ src/sys/arch/x86/x86/fpu.c	Mon Jul  6 01:08:15 2020
@@ -1,4 +1,4 @@
-/*	$NetBSD: fpu.c,v 1.65 2020/06/14 16:12:05 riastradh Exp $	*/
+/*	$NetBSD: fpu.c,v 1.66 2020/07/06 01:08:15 riastradh Exp $	*/
 
 /*
  * Copyright (c) 2008, 2019 The NetBSD Foundation, Inc.  All
@@ -96,7 +96,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.65 2020/06/14 16:12:05 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.66 2020/07/06 01:08:15 riastradh Exp $");
 
 #include "opt_multiprocessor.h"
 
@@ -126,6 +126,8 @@ __KERNEL_RCSID(0, "$NetBSD: fpu.c,v 1.65
 #define stts() HYPERVISOR_fpu_taskswitch(1)
 #endif
 
+static void fpu_area_do_save(void *, uint64_t);
+
 void fpu_handle_deferred(void);
 void fpu_switch(struct lwp *, struct lwp *);
 
@@ -155,8 +157,24 @@ fpu_save_lwp(struct lwp *l)
 	kpreempt_disable();
 	if (l->l_md.md_flags & MDL_FPU_IN_CPU) {
 		KASSERT((l->l_flag & LW_SYSTEM) == 0);
-		fpu_area_save(area, x86_xsave_features);
+
+		/*
+		 * Order is important, in case we are interrupted and
+		 * the interrupt calls fpu_kern_enter, triggering
+		 * reentry of fpu_save_lwp:
+		 *
+		 * 1. Save FPU state.
+		 * 2. Note FPU state has been saved.
+		 * 3. Disable FPU access so the kernel doesn't
+		 *    accidentally use it.
+		 *
+		 * Steps (1) and (2) are both idempotent until step
+		 * (3), after which point attempting to save the FPU
+		 * state will trigger #NM/fpudna fault.
+		 */
+		fpu_area_do_save(area, x86_xsave_features);
 		l->l_md.md_flags &= ~MDL_FPU_IN_CPU;
+		stts();
 	}
 	kpreempt_enable();
 }
@@ -245,8 +263,8 @@ fpu_errata_amd(void)
 	fldummy();
 }
 
-void
-fpu_area_save(void *area, uint64_t xsave_features)
+static void
+fpu_area_do_save(void *area, uint64_t xsave_features)
 {
 	switch (x86_fpu_save) {
 	case FPU_SAVE_FSAVE:
@@ -262,7 +280,13 @@ fpu_area_save(void *area, uint64_t xsave
 		xsaveopt(area, xsave_features);
 		break;
 	}
+}
+
+void
+fpu_area_save(void *area, uint64_t xsave_features)
+{
 
+	fpu_area_do_save(area, xsave_features);
 	stts();
 }