From 01aa832b99a8a39594b15e78767b4388c09ce82a Mon Sep 17 00:00:00 2001 From: Jorgen Lundman Date: Thu, 25 Apr 2024 10:45:15 +0900 Subject: [PATCH] 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 --- include/os/windows/spl/sys/processor.h | 41 -------------------------- include/os/windows/spl/sys/simd.h | 18 +++-------- module/icp/algs/blake3/blake3_impl.c | 2 +- module/icp/algs/modes/gcm.c | 2 +- module/os/windows/driver.c | 8 ++--- module/os/windows/zfs/sysctl_os.c | 19 ++++++------ module/zfs/vdev_raidz_math.c | 3 +- 7 files changed, 20 insertions(+), 73 deletions(-) diff --git a/include/os/windows/spl/sys/processor.h b/include/os/windows/spl/sys/processor.h index 600dadf17a69..6c663688dec2 100644 --- 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 index 5a607904df0f..626fd1a28bdf 100644 --- 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 #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 index df3d6922752e..5eed7ce0a619 100644 --- 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 index 1fdfcc2a1844..b7cadd6a2af9 100644 --- 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 index 8df9010f0c0a..f1df2426e05c 100644 --- 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 index 62a7dcc2eaf5..29ebac5ad584 100644 --- 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 index fea93aa8b255..3856264b7e16 100644 --- 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;