Unload BSOD - cpuid clobbers RBX

When called from ZwNotifyChangeKey()'s callback, our call to cpuid() would clobber RBX which would manifest as a BSOD on unload time. Builtin intrinsic __cpuidex() does not suffer from this, so we now call it. Additionally, clean up some of the tunables, and ensure they use static memory. Signed-off-by: Jorgen Lundman <[email protected]>
andrewc12 · Apr 25, 2024 · 01aa832 · 01aa832
1 parent 43cce56
commit 01aa832
Show file tree

Hide file tree

Showing 7 changed files with 20 additions and 73 deletions.
diff --git a/include/os/windows/spl/sys/processor.h b/include/os/windows/spl/sys/processor.h
@@ -10,47 +10,6 @@ typedef int	processorid_t;
 
 #if defined(__amd64__) || defined(__x86_64__)
 
-extern int __cpuid_count(unsigned int __level, unsigned int __sublevel,
-	unsigned int __eax, unsigned int __ebx,
-	unsigned int __ecx, unsigned int __edx);
-
-#define	__cpuid_count(level, count, a, b, c, d) \
-	__asm__("xchg{l}\t{%%}ebx, %1\n\t" \
-		"cpuid\n\t" \
-		"xchg{l}\t{%%}ebx, %1\n\t" \
-		: "=a" (a), "=r" (b), "=c" (c), "=d" (d) \
-		: "0" (level), "2" (count))
-
-#define	__cpuid(level, a, b, c, d) \
-	__asm__("xchg{l}\t{%%}ebx, %1\n\t" \
-		"cpuid\n\t" \
-		"xchg{l}\t{%%}ebx, %1\n\t" \
-		: "=a" (a), "=r" (b), "=c" (c), "=d" (d) \
-		: "0" (level))
-
-static inline unsigned int
-__get_cpuid_max(unsigned int __ext, unsigned int *__sig)
-{
-	unsigned int __eax, __ebx, __ecx, __edx;
-	__cpuid(__ext, __eax, __ebx, __ecx, __edx);
-	if (__sig)
-		*__sig = __ebx;
-	return (__eax);
-}
-
-/* macOS does have do_cpuid() macro */
-static inline int
-__get_cpuid(unsigned int __level,
-    unsigned int *__eax, unsigned int *__ebx,
-    unsigned int *__ecx, unsigned int *__edx)
-{
-	unsigned int __ext = __level & 0x80000000;
-	if (__get_cpuid_max(__ext, 0) < __level)
-		return (0);
-	__cpuid(__level, *__eax, *__ebx, *__ecx, *__edx);
-	return (1);
-}
-
 #endif // x86
 
 typedef int	processorid_t;

diff --git a/include/os/windows/spl/sys/simd.h b/include/os/windows/spl/sys/simd.h
@@ -93,10 +93,6 @@ xgetbv(uint32_t c)
 
 #endif
 
-#ifndef ZFS_ASM_BUG
-#define	ZFS_ASM_BUG()	{ ASSERT(0); } break
-#endif
-
 #define	kfpu_allowed()		1
 
 #endif
@@ -134,8 +130,6 @@ extern uint32_t kfpu_state;
 #include <assert.h>
 #endif // KERNEL
 
-#define	ZFS_ASM_BUG()	{ assert(0); } break
-
 /*
  * x86 registers used implicitly by CPUID
  */
@@ -256,14 +250,10 @@ __cpuid_check_feature(const cpuid_feature_desc_t *desc)
 {
 	uint32_t r[CPUID_REG_CNT];
 
-	if (__get_cpuid_max(0, NULL) >= desc->leaf) {
-		/*
-		 * __cpuid_count is needed to properly check
-		 * for AVX2. It is a macro, so return parameters
-		 * are passed by value.
-		 */
-		__cpuid_count(desc->leaf, desc->subleaf,
-		    r[EAX], r[EBX], r[ECX], r[EDX]);
+	__cpuidex((int *)r, 0, 0);
+	// EAX has leaf count
+	if (r[EAX] >= desc->leaf) {
+		__cpuidex((int *)r, desc->leaf, desc->subleaf);
 		return ((r[desc->reg] & desc->flag) == desc->flag);
 	}
 	return (B_FALSE);

diff --git a/module/icp/algs/blake3/blake3_impl.c b/module/icp/algs/blake3/blake3_impl.c
@@ -358,7 +358,7 @@ win32_blake3_param_set(ZFS_MODULE_PARAM_ARGS)
 	*type = ZT_TYPE_STRING;
 
 	if (set == B_FALSE) {
-		char buffer[PAGE_SIZE]; /* Looks like they use page size */
+		static char buffer[PAGE_SIZE];
 		blake3_param_get(buffer, NULL);
 		*ptr = buffer;
 		*len = strlen(buffer);

diff --git a/module/icp/algs/modes/gcm.c b/module/icp/algs/modes/gcm.c
@@ -1031,7 +1031,7 @@ icp_gcm_impl_get(char *buffer, zfs_kernel_param_t *kp)
 int
 win32_icp_gcm_impl_set(ZFS_MODULE_PARAM_ARGS)
 {
-	static char str[1024] = "";
+	static char str[PAGE_SIZE] = "";
 
 	*type = ZT_TYPE_STRING;
 

diff --git a/module/os/windows/driver.c b/module/os/windows/driver.c
@@ -75,7 +75,7 @@ sysctl_os_registry_change(PVOID Parameter);
 void
 OpenZFS_Fini(PDRIVER_OBJECT DriverObject)
 {
-	KdPrintEx((DPFLTR_IHVDRIVER_ID, DPFLTR_INFO_LEVEL, "OpenZFS_Fini\n"));
+	KdPrintEx((DPFLTR_IHVDRIVER_ID, DPFLTR_ERROR_LEVEL, "OpenZFS_Fini\n"));
 
 	zfs_vfsops_fini();
 
@@ -105,6 +105,9 @@ OpenZFS_Fini(PDRIVER_OBJECT DriverObject)
 		STOR_wzvolDriverInfo.zvContextArray = NULL;
 	}
 	ZFSWppCleanup(DriverObject);
+
+	KdPrintEx((DPFLTR_IHVDRIVER_ID, DPFLTR_ERROR_LEVEL,
+	    "OpenZFS: Goodbye.\n"));
 }
 
 /*
@@ -120,9 +123,6 @@ DriverEntry(_In_ PDRIVER_OBJECT DriverObject,
 
 	ZFSWppInit(DriverObject, pRegistryPath);
 
-	KdPrintEx((DPFLTR_IHVDRIVER_ID, DPFLTR_INFO_LEVEL,
-	    "OpenZFS: DriverEntry\n"));
-
 	// Setup global so zfs_ioctl.c can setup devnode
 	WIN_DriverObject = DriverObject;
 

diff --git a/module/os/windows/zfs/sysctl_os.c b/module/os/windows/zfs/sysctl_os.c
@@ -253,8 +253,9 @@ sysctl_os_process(PUNICODE_STRING pRegistryPath, ztunable_t *zt)
 
 			// _CALL style has to 'type', so look it up first.
 			if (zt->zt_perm == ZT_ZMOD_RW) {
-				char **maybestr = NULL;
-				ZT_GET_VALUE(zt, &maybestr, &len, &type);
+				char *maybestr = NULL;
+				ZT_GET_VALUE(zt, (void **)&maybestr, &len,
+				    &type);
 
 				// Set up buffers to SET value.
 				val = &buffer[kv->DataOffset];
@@ -276,8 +277,7 @@ sysctl_os_process(PUNICODE_STRING pRegistryPath, ztunable_t *zt)
 					/* Already set? free it */
 					if (!(zt->zt_flag & ZT_FLAG_STATIC)) {
 
-						if (maybestr != NULL &&
-						    *maybestr != NULL)
+						if (maybestr != NULL)
 							ExFreePool(*maybestr);
 
 						*maybestr = NULL;
@@ -418,7 +418,6 @@ sysctl_os_init(PUNICODE_STRING RegistryPath)
 {
 	// iterate the linker-set
 	ztunable_t **iter = NULL;
-	int count = 0;
 
 	SET_DECLARE(zt, ztunable_t);
 
@@ -427,7 +426,7 @@ sysctl_os_init(PUNICODE_STRING RegistryPath)
 			ztunable_t *zt = *iter;
 
 			sysctl_os_process(RegistryPath, zt);
-			count++;
+
 		}
 	}
 }
@@ -567,13 +566,13 @@ param_set_arc_int(ZFS_MODULE_PARAM_ARGS)
 int
 param_set_active_allocator(ZFS_MODULE_PARAM_ARGS)
 {
-	char buf[16];
+	static char buf[16];
 
 	*type = ZT_TYPE_STRING;
 
 	if (set == B_FALSE) {
 		*ptr = (void *)zfs_active_allocator;
-		*len = strlen(zfs_active_allocator) + 1;
+		*len = strlen(zfs_active_allocator);
 		return (0);
 	}
 
@@ -668,13 +667,13 @@ param_set_deadman_ziotime(ZFS_MODULE_PARAM_ARGS)
 int
 param_set_deadman_failmode(ZFS_MODULE_PARAM_ARGS)
 {
-	char buf[16];
+	static char buf[16];
 
 	*type = ZT_TYPE_STRING;
 
 	if (set == B_FALSE) {
 		*ptr = (void *)zfs_deadman_failmode;
-		*len = strlen(zfs_deadman_failmode) + 1;
+		*len = strlen(zfs_deadman_failmode);
 		return (0);
 	}
 

diff --git a/module/zfs/vdev_raidz_math.c b/module/zfs/vdev_raidz_math.c
@@ -673,8 +673,7 @@ zfs_vdev_raidz_impl_get(char *buffer, zfs_kernel_param_t *kp)
 int
 win32_zfs_vdev_raidz_impl_set(ZFS_MODULE_PARAM_ARGS)
 {
-	uint32_t val;
-	static unsigned char str[1024] = "";
+	static char str[1024] = "";
 
 	*type = ZT_TYPE_STRING;