Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 18 Apr 2023 23:22:57 GMT
From:      Konstantin Belousov <kib@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: 617a11eab633 - main - x86: initialize use_xsave once
Message-ID:  <202304182322.33INMvuQ011715@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=617a11eab6337693eae9d160453adf1943ab6a37

commit 617a11eab6337693eae9d160453adf1943ab6a37
Author:     Konstantin Belousov <kib@FreeBSD.org>
AuthorDate: 2023-04-18 15:50:26 +0000
Commit:     Konstantin Belousov <kib@FreeBSD.org>
CommitDate: 2023-04-18 23:22:28 +0000

    x86: initialize use_xsave once
    
    The explanation from https://reviews.freebsd.org/D39637 by stevek:
    The "use_xsave" variable is a global and that is only supposed to be
    initialized early before scheduling gets started. However, with the way
    the ifuncs for "fpusave" and "fpurestore" are implemented, the value
    could be changed at runtime when scheduling is active if "use_xsave"
    was set to 0 by the tunable. This leaves a window of opportunity where
    "use_xsave" gets re-initialized to 1 and a context switch could occur
    with a thread that was not set up to be able to use xsave functionality.
    This can lead to an "privileged instruction fault".
    
    The fix is to protect "use_xsave" from being initialized more than once.
    
    Reported and reviewed by:       stevek
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D39660
---
 sys/amd64/amd64/fpu.c     | 16 ----------------
 sys/amd64/amd64/machdep.c |  5 +++++
 sys/i386/i386/machdep.c   |  6 ++++++
 sys/i386/i386/npx.c       | 14 --------------
 4 files changed, 11 insertions(+), 30 deletions(-)

diff --git a/sys/amd64/amd64/fpu.c b/sys/amd64/amd64/fpu.c
index 64974a7210a9..a3421cf0d496 100644
--- a/sys/amd64/amd64/fpu.c
+++ b/sys/amd64/amd64/fpu.c
@@ -238,22 +238,8 @@ fpurestore_fxrstor(void *addr)
 	fxrstor((char *)addr);
 }
 
-static void
-init_xsave(void)
-{
-
-	if (use_xsave)
-		return;
-	if ((cpu_feature2 & CPUID2_XSAVE) == 0)
-		return;
-	use_xsave = 1;
-	TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
-}
-
 DEFINE_IFUNC(, void, fpusave, (void *))
 {
-
-	init_xsave();
 	if (!use_xsave)
 		return (fpusave_fxsave);
 	if ((cpu_stdext_feature & CPUID_EXTSTATE_XSAVEOPT) != 0) {
@@ -266,8 +252,6 @@ DEFINE_IFUNC(, void, fpusave, (void *))
 
 DEFINE_IFUNC(, void, fpurestore, (void *))
 {
-
-	init_xsave();
 	if (!use_xsave)
 		return (fpurestore_fxrstor);
 	return ((cpu_stdext_feature & CPUID_STDEXT_NFPUSG) != 0 ?
diff --git a/sys/amd64/amd64/machdep.c b/sys/amd64/amd64/machdep.c
index 480db1ed2c31..29a0bd6f9db4 100644
--- a/sys/amd64/amd64/machdep.c
+++ b/sys/amd64/amd64/machdep.c
@@ -1345,6 +1345,11 @@ hammer_time(u_int64_t modulep, u_int64_t physfree)
 	    &pmap_pcid_invlpg_workaround_uena);
 	cpu_init_small_core();
 
+	if ((cpu_feature2 & CPUID2_XSAVE) != 0) {
+		use_xsave = 1;
+		TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
+	}
+
 	link_elf_ireloc(kmdp);
 
 	/*
diff --git a/sys/i386/i386/machdep.c b/sys/i386/i386/machdep.c
index d59e95ed1a99..1003dbe539c2 100644
--- a/sys/i386/i386/machdep.c
+++ b/sys/i386/i386/machdep.c
@@ -1545,6 +1545,11 @@ init386(int first)
 		i386_kdb_init();
 	}
 
+	if (cpu_fxsr && (cpu_feature2 & CPUID2_XSAVE) != 0) {
+		use_xsave = 1;
+		TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
+	}
+
 	kmdp = preload_search_by_type("elf kernel");
 	link_elf_ireloc(kmdp);
 
@@ -1565,6 +1570,7 @@ init386(int first)
 
 	msgbufinit(msgbufp, msgbufsize);
 	npxinit(true);
+
 	/*
 	 * Set up thread0 pcb after npxinit calculated pcb + fpu save
 	 * area size.  Zero out the extended state header in fpu save
diff --git a/sys/i386/i386/npx.c b/sys/i386/i386/npx.c
index 3d4f2f2a60c8..91967b4be843 100644
--- a/sys/i386/i386/npx.c
+++ b/sys/i386/i386/npx.c
@@ -320,22 +320,8 @@ fpusave_fnsave(union savefpu *addr)
 	fnsave((char *)addr);
 }
 
-static void
-init_xsave(void)
-{
-
-	if (use_xsave)
-		return;
-	if (!cpu_fxsr || (cpu_feature2 & CPUID2_XSAVE) == 0)
-		return;
-	use_xsave = 1;
-	TUNABLE_INT_FETCH("hw.use_xsave", &use_xsave);
-}
-
 DEFINE_IFUNC(, void, fpusave, (union savefpu *))
 {
-
-	init_xsave();
 	if (use_xsave)
 		return ((cpu_stdext_feature & CPUID_EXTSTATE_XSAVEOPT) != 0 ?
 		    fpusave_xsaveopt : fpusave_xsave);



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