[PATCH v4 0/2] MR8594: Draft: ntdll: Add environment to override cpuid for debugging and test.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58335 This should help to demonstrate the issue in above bug and probably add a way check to such an issue in gitlab. -- v4: gitlab: Add test for non-avx code path in __wine_syscall_dispatcher. ntdll: Add environment to override cpuid for debugging and test. https://gitlab.winehq.org/wine/wine/-/merge_requests/8594
From: Bernhard Übelacker <bernhardu(a)mailbox.org> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58335 --- dlls/ntdll/tests/virtual.c | 48 ++++++++++++++++++++++++++++++++++++++ dlls/ntdll/unix/system.c | 32 ++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 1 deletion(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index c48652f0f65..362f8e16da4 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -3209,6 +3209,48 @@ static void test_exec_memory_writes(void) RtlRemoveVectoredExceptionHandler( handler ); } +static void test_cpuid_child(void) +{ +#if defined(__x86_64__) && !defined(__aarch64__) + HMODULE module = GetModuleHandleW( L"ntdll.dll" ); + NTSTATUS (WINAPI *pNtClose)(HANDLE); + int val[] = {0xabcdef00, 0x12345678, 0xfedcba00, 0x87654321}; + + __asm__ __volatile__ ( "movaps %0,%%xmm6" : : "m" (val) ); + /* just some call to get through __wine_syscall_dispatcher */ + pNtClose = (void *)GetProcAddress( module, "NtClose" ); + pNtClose(INVALID_HANDLE_VALUE); + __asm__ __volatile__ ( "movaps %%xmm6,%0" : "=m" (val) ); + + ok(val[0] == 0xabcdef00 && val[1] == 0x12345678 && val[2] == 0xfedcba00 && val[3] == 0x87654321, + "register xmm6 has unexpected values 0x%08x, 0x%08x, 0x%08x, 0x%08x\n", val[0], val[1], val[2], val[3]); +#endif +} + +static void test_cpuid(void) +{ +#if defined(__x86_64__) && !defined(__aarch64__) + PROCESS_INFORMATION info; + STARTUPINFOA startup; + char **argv; + char path_name[MAX_PATH]; + DWORD ret; + + winetest_get_mainargs(&argv); + memset(&startup, 0, sizeof(startup)); + startup.cb = sizeof(startup); + sprintf(path_name, "%s %s cpuid", argv[0], argv[1]); + + ret = CreateProcessA(NULL, path_name, NULL, NULL, FALSE, 0, NULL, NULL, &startup, &info); + ok(ret, "Failed to create target process.\n"); + + wait_child_process(info.hProcess); + + CloseHandle(info.hProcess); + CloseHandle(info.hThread); +#endif +} + START_TEST(virtual) { HMODULE mod; @@ -3224,6 +3266,11 @@ START_TEST(virtual) Sleep(5000); /* spawned process runs for at most 5 seconds */ return; } + if (!strcmp(argv[2], "cpuid")) + { + test_cpuid_child(); + return; + } return; } @@ -3262,4 +3309,5 @@ START_TEST(virtual) test_query_region_information(); test_query_image_information(); test_exec_memory_writes(); + test_cpuid(); } diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index e486da40691..431d9b808b1 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -79,6 +79,9 @@ #include "wine/debug.h" WINE_DEFAULT_DEBUG_CHANNEL(ntdll); +#if defined(__i386__) || defined(__x86_64__) +WINE_DECLARE_DEBUG_CHANNEL(cpuid); +#endif #pragma pack(push,1) @@ -321,11 +324,34 @@ void copy_xstate( XSAVE_AREA_HEADER *dst, XSAVE_AREA_HEADER *src, UINT64 mask ) } } -static inline void do_cpuid( unsigned int ax, unsigned int cx, unsigned int *p ) +static inline void do_cpuid_( unsigned int ax, unsigned int cx, unsigned int *p ) { __asm__ ( "cpuid" : "=a" (p[0]), "=b" (p[1]), "=c" (p[2]), "=d" (p[3]) : "a" (ax), "c" (cx) ); } +static inline void do_cpuid( unsigned int ax, unsigned int cx, unsigned int *p ) +{ + char buf[30]; + char *override; + + memset(p, 0, sizeof(*p)*4); + do_cpuid_(ax, cx, p); + TRACE_(cpuid)("cpuid returned WINE_CPUID_%08x_%08x=%08x_%08x_%08x_%08x\n", + ax, cx, p[0], p[1], p[2], p[3]); + + snprintf(buf, sizeof(buf), "WINE_CPUID_%08x_%08x", ax, cx); + override = getenv(buf); + if (override && strlen(override) == 35) + { + sscanf(override, "%08x", &p[0]); + sscanf(override+9, "%08x", &p[1]); + sscanf(override+18, "%08x", &p[2]); + sscanf(override+27, "%08x", &p[3]); + ERR_(cpuid)("overriding with WINE_CPUID_%08x_%08x=%08x_%08x_%08x_%08x\n", + ax, cx, p[0], p[1], p[2], p[3]); + } +} + static inline UINT64 do_xgetbv( unsigned int cx ) { UINT low, high; @@ -432,6 +458,8 @@ static void init_xstate_features( XSTATE_CONFIGURATION *xstate ) ULONG64 supported_mask; unsigned int i, off, regs[4]; + TRACE_(cpuid)("init_xstate_features reached.\n"); + do_cpuid( 0x0000000d, 0, regs ); TRACE( "XSAVE details %#x, %#x, %#x, %#x.\n", regs[0], regs[1], regs[2], regs[3] ); supported_mask = ((ULONG64)regs[3] << 32) | regs[0]; @@ -479,6 +507,8 @@ void init_shared_data_cpuinfo( KUSER_SHARED_DATA *data ) BOOLEAN *features = data->ProcessorFeatures; unsigned int regs[4]; + TRACE_(cpuid)("init_shared_data_cpuinfo reached.\n"); + features[PF_FASTFAIL_AVAILABLE] = TRUE; features[PF_COMPARE_EXCHANGE_DOUBLE] = TRUE; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8594
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- tools/gitlab/test.yml | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tools/gitlab/test.yml b/tools/gitlab/test.yml index 2b03a05264f..2bba830edcb 100644 --- a/tools/gitlab/test.yml +++ b/tools/gitlab/test.yml @@ -59,6 +59,34 @@ CI_COMMIT_MESSAGE: "" GITLAB_USER_NAME: "" +test-linux-64-non-avx: + extends: .wine-test + variables: + INCLUDE_TESTS: "ntdll:virtual" + rules: + - if: $CI_PIPELINE_SOURCE == 'merge_request_event' + needs: + - job: build-linux + script: + - export WINETEST_COLOR=1 + # Pentium(R) Dual-Core CPU T4500 @ 2.30GHz + - export WINE_CPUID_00000000_00000000=0000000d_756e6547_6c65746e_49656e69 + - export WINE_CPUID_00000001_00000000=0001067a_01020800_0c00e39d_bfebfbff + - export WINE_CPUID_00000007_00000000=00000000_00000000_00000000_00000000 + - export WINE_CPUID_0000000d_00000000=00000003_00000240_00000240_00000000 + - export WINE_CPUID_80000000_00000000=80000008_00000000_00000000_00000000 + - export WINE_CPUID_80000001_00000000=00000000_00000000_00000001_20100800 + - export WINE_CPUID_80000002_00000000=746e6550_286d7569_44202952_2d6c6175 + - export WINE_CPUID_80000003_00000000=65726f43_55504320_20202020_54202020 + - export WINE_CPUID_80000004_00000000=30303534_20402020_30332e32_007a4847 + - wine usr/local/lib/wine/x86_64-windows/winetest.exe -q -q -o - -J winetest.xml $INCLUDE_TESTS + artifacts: + when: always + paths: + - winetest.xml + reports: + junit: winetest.xml + test-linux-64: extends: .wine-test variables: -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8594
_WIN64 is defined on arm64, isn't it? movaps is not a thing on arm64. Do those envs affect if the app does its own cpuid? If no, the names feel kinda misleading. Maybe call it WINE_NTDLL_HIDE_AVX_SUPPORT=1 instead? Or do you have future plans for other cpuid overrides, like avx512? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8594#note_110298
On Fri Jul 18 12:25:03 2025 +0000, Alfred Agrell wrote:
_WIN64 is defined on arm64, isn't it? movaps is not a thing on arm64. Do those envs affect if the app does its own cpuid? If no, the names feel kinda misleading. Maybe call it WINE_NTDLL_HIDE_AVX_SUPPORT=1 instead? Or do you have future plans for other cpuid overrides, like avx512? Thanks for looking at it. I replaced _WIN64 already, but still fails, and still trying to sort this out. I don't understand why even with `#if defined(__x86_64__) && !defined(__aarch64__)` this movaps is failing for `make: *** [Makefile:300035: dlls/ntdll/tests/arm64ec-windows/virtual.o] Error 1`.
The idea behind is to enable a developer with a recent CPU to be able to debug issues just showing up in older CPUs. Or like in this merge request make this a CI test to avoid future issues like 58335. It was not specifically intended to disable avx support but to see how Wine would behave on such a CPU. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8594#note_110300
On Fri Jul 18 12:25:03 2025 +0000, Bernhard Übelacker wrote:
Thanks for looking at it. I replaced _WIN64 already, but still fails, and still trying to sort this out. I don't understand why even with `#if defined(__x86_64__) && !defined(__aarch64__)` this movaps is failing for `make: *** [Makefile:300035: dlls/ntdll/tests/arm64ec-windows/virtual.o] Error 1`. The idea behind is to enable a developer with a recent CPU to be able to debug issues just showing up in older CPUs. Or like in this merge request make this a CI test to avoid future issues like 58335. It was not specifically intended to disable avx support but to see how Wine would behave on such a CPU. [On MSVC](https://techcommunity.microsoft.com/blog/windowsosplatform/getting-to-know-a...), arm64ec defines _M_AMD64 and does not define _M_ARM64.
I'd guess your compiler does something similar, and you need to explicitly look for an arm64ec define. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8594#note_110301
IMHO this option is confusing to users, since this envvar only affects the CPUID as viewed by Wine, not by the program or any of our Unix-side dependencies (e.g., glibc / gstreamer / ...) that may invoke CPUID directly. It's reasonable to expect that some users find that the envvar works around their issue, share their "fixes," and we'd get a bug reports from a bunch of confused users that the envvar doesn't actually "override the CPUID." Also, this seem quite easy to bitrot: we would have to ensure that *all* places that might call CPUID directly stays up to date. Otherwise the CPUID view would be inconsistent across callsites—debugging that doesn't sound straightforward. I wonder, instead, if we can use (or make) a separate LD\_PRELOAD library that hooks cpuid instruction via ARCH\_SET\_CPUID. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8594#note_110302
On Fri Jul 18 12:51:01 2025 +0000, Jinoh Kang wrote:
IMHO this option is confusing to users, since this envvar only affects the CPUID as viewed by Wine, not by the program or any of our Unix-side dependencies (e.g., glibc / gstreamer / ...) that may invoke CPUID directly. It's reasonable to expect that some users find that the envvar works around their issue, share their "fixes," and we'd get a bug reports from a bunch of confused users that the envvar doesn't actually "override the CPUID." Also, this seem quite easy to bitrot: we would have to ensure that *all* places that might call CPUID directly stays up to date. Otherwise the CPUID view would be inconsistent across callsites—debugging that doesn't sound straightforward. I wonder, instead, if we can use (or make) a separate LD\_PRELOAD library that hooks cpuid instruction via ARCH\_SET\_CPUID. Thanks for looking into it. This is not intended for users, more for developers trying to reproduce bug reports. (Except enabling the cpuid debug channel and providing the values to the developer.)
The ARCH_SET_CPUID looks interesting, I will try to generalize this, thanks. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8594#note_110304
participants (3)
-
Alfred Agrell (＠Alcaro) -
Bernhard Übelacker
-
Jinoh Kang (＠iamahuman)