[PATCH v14 0/2] MR8594: Draft: ntdll: Add environment to override cpuid for debugging and test.

[Bernhard Übelacker (＠bernhardu)]

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.

--
  v14: gitlab: Add test for non-avx code path in __wine_syscall_dispatcher.

https://gitlab.winehq.org/wine/wine/-/merge_requests/8594

[PATCH v14 1/2] ntdll: Add environment to override cpuid for debugging and test.

[Bernhard Übelacker]

From: Bernhard Übelacker

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..2bfb800de74 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(__arm64ec__)
+    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;

[PATCH v14 2/2] gitlab: Add test for non-avx code path in __wine_syscall_dispatcher.

[Bernhard Übelacker]

From: Bernhard Übelacker

---
 tools/gitlab/test.yml | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/tools/gitlab/test.yml b/tools/gitlab/test.yml
index 2b03a05264f..077a80cfb53 100644
--- a/tools/gitlab/test.yml
+++ b/tools/gitlab/test.yml
@@ -59,6 +59,36 @@
     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
+    # Show current cpu flags
+    - lscpu
+    # Simulate older CPU
+    - curl --silent --location https://github.com/twosigma/libvirtcpuid/archive/3b549c732adffb566342693eaf8... --output libvirtcpuid.zip
+    - unzip libvirtcpuid.zip
+    - mv libvirtcpuid-3b549c732adffb566342693eaf86eb9b192510f1 libvirtcpuid
+    - cd libvirtcpuid
+    - curl --silent --location -O https://musl.libc.org/releases/musl-1.1.24.tar.gz
+    - make -j --silent
+    - cd ..
+    - export LD_PRELOAD=$PWD/libvirtcpuid/libvirtcpuid.so
+    - export VIRT_CPUID_MASK=3dnowprefetch,abm,adx,aes,arat,avic,avx,avx2,bmi1,bmi2,bpext,clflushopt,clzero,cmp_legacy,cr8_legacy,decodeassists,extapic,f16c,flushbyasid,fma,fsgsbase,fxsr_opt,irperf,lbrv,misalignsse,mmxext,movbe,mwaitx,npt,nrip_save,osvw,overflow_recov,pausefilter,pclmulqdq,pdpe1gb,perfctr_core,perfctr_llc,perfctr_nb,pfthreshold,popcnt,rdrand,rdseed,rdtscp,sha_ni,skinit,smap,smca,smep,sse4_1,sse4_2,sse4a,succor,svm,svm_lock,tce,topoext,tsc_scale,v_vmsave_vmload,vgif,vmcb_clean,wdt,xgetbv1,xsavec,xsaveerptr,xsaveopt
+    - 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:

[Bernhard Übelacker (＠bernhardu)]

> We could use https://github.com/twosigma/libvirtcpuid, seems it's commonly used in tandem with CRIU.

Looks like the gitlab runner currently is not able to use CPUID faulting ([pipeline #45703](https://gitlab.winehq.org/bernhardu/wine/-/pipelines/45703)), maybe because of the CPU being an AMD one. It looks like support for it may get available for AMD CPUs in the next kernel releases (https://www.phoronix.com/news/AMD-CPUID-Faulting-Linux-6.17).
> 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 seems quite easy to bitrot: we would have to ensure that _all_ places that call CPUID directly stay up to date. Otherwise the CPUID view would be inconsistent across callsites—debugging that doesn't sound straightforward.

As said, this is not expected to fix anything when users add it. It would be more the other way around: The user reports an unexplainable bug, the developer queries the cpuid environment from the user, and may be able to reproduce the issue with his own more feature-rich CPU.

Or make such regression early visible before or after it got merged.