[PATCH v2 0/2] MR8591: Draft: ntdll: Fill EnabledFeatures before return.
Followup to 03039ab2ee. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58335 If I observed it right we currently leave `init_xstate_features` on older computers with EnabledFeatures being zero. This leads in `__wine_syscall_dispatcher` to getting the xsave64 getting called with "$rax = 0x0", therefore e.g. xmm6 gets not saved to the stack. But later e.g. xmm6 gets restored from stack in `__wine_syscall_dispatcher` (see [__wine_syscall_dispatcher](https://gitlab.winehq.org/wine/wine/-/blob/wine-10.12/dlls/ntdll/unix/signal...)) -- v2: ntdll/tests: Allow non-AVX CPUs in XState feature tests. https://gitlab.winehq.org/wine/wine/-/merge_requests/8591
From: Bernhard Übelacker <bernhardu(a)mailbox.org> Followup to 03039ab2ee. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=58335 --- dlls/ntdll/unix/system.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index e486da40691..e8758a3df6d 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -436,10 +436,10 @@ static void init_xstate_features( XSTATE_CONFIGURATION *xstate ) TRACE( "XSAVE details %#x, %#x, %#x, %#x.\n", regs[0], regs[1], regs[2], regs[3] ); supported_mask = ((ULONG64)regs[3] << 32) | regs[0]; supported_mask &= do_xgetbv(0) & supported_features; - if (!(supported_mask >> 2)) return; xstate->EnabledFeatures = (1 << XSTATE_LEGACY_FLOATING_POINT) | (1 << XSTATE_LEGACY_SSE) | supported_mask; xstate->EnabledVolatileFeatures = xstate->EnabledFeatures; + if (!(supported_mask >> 2)) return; xstate->AllFeatureSize = regs[1]; do_cpuid( 0x0000000d, 1, regs ); -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8591
From: Bernhard Übelacker <bernhardu(a)mailbox.org> --- dlls/ntdll/tests/virtual.c | 64 +++++++++++++++++++++++++++++--------- 1 file changed, 49 insertions(+), 15 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index c48652f0f65..cce4f0582e6 100644 --- a/dlls/ntdll/tests/virtual.c +++ b/dlls/ntdll/tests/virtual.c @@ -1961,8 +1961,6 @@ static void test_NtMapViewOfSectionEx(void) CloseHandle(process); } -#define SUPPORTED_XSTATE_FEATURES ((1 << XSTATE_LEGACY_FLOATING_POINT) | (1 << XSTATE_LEGACY_SSE) | (1 << XSTATE_AVX)) - static void test_user_shared_data(void) { struct old_xstate_configuration @@ -1974,22 +1972,43 @@ static void test_user_shared_data(void) XSTATE_FEATURE Features[MAXIMUM_XSTATE_FEATURES]; }; - static const ULONG feature_offsets[] = + static const ULONG feature_offsets[2][3] = { + { + /* AVX */ 0, 160, /*offsetof(XMM_SAVE_AREA32, XmmRegisters)*/ 512 /* sizeof(XMM_SAVE_AREA32) */ + offsetof(XSTATE, YmmContext), + }, + { + /* non-AVX */ + 0, + 160, /*offsetof(XMM_SAVE_AREA32, XmmRegisters)*/ + 0, + }, }; - static const ULONG feature_sizes[] = + static const ULONG feature_sizes[2][3] = { + { + /* AVX */ 160, 256, /*sizeof(M128A) * 16 */ sizeof(YMMCONTEXT), + }, + { + /* non-AVX */ + 160, + 256, /*sizeof(M128A) * 16 */ + 0, + }, }; const KUSER_SHARED_DATA *user_shared_data = (void *)0x7ffe0000; XSTATE_CONFIGURATION xstate = user_shared_data->XState; ULONG64 feature_mask; + ULONG64 supported_xstate_features; unsigned int i; + unsigned int feature_set; + size_t xstate_part_size; ok(user_shared_data->NumberOfPhysicalPages == sbi.MmNumberOfPhysicalPages, "Got number of physical pages %#lx, expected %#lx.\n", @@ -2030,6 +2049,21 @@ static void test_user_shared_data(void) return; } + if (xstate.EnabledFeatures & (1 << XSTATE_AVX)) + { + /* AVX */ + feature_set = 0; + xstate_part_size = sizeof(XSTATE); + supported_xstate_features = (1 << XSTATE_LEGACY_FLOATING_POINT) | (1 << XSTATE_LEGACY_SSE) | (1 << XSTATE_AVX); + } + else + { + /* non-AVX */ + feature_set = 1; + xstate_part_size = sizeof(XSTATE) - sizeof(YMMCONTEXT); + supported_xstate_features = (1 << XSTATE_LEGACY_FLOATING_POINT) | (1 << XSTATE_LEGACY_SSE); + } + trace("XState EnabledFeatures %#I64x, EnabledSupervisorFeatures %#I64x, EnabledVolatileFeatures %I64x.\n", xstate.EnabledFeatures, xstate.EnabledSupervisorFeatures, xstate.EnabledVolatileFeatures); feature_mask = pRtlGetEnabledExtendedFeatures(0); @@ -2040,29 +2074,29 @@ static void test_user_shared_data(void) feature_mask = pGetEnabledXStateFeatures(); ok(feature_mask == (xstate.EnabledFeatures | xstate.EnabledSupervisorFeatures), "Got unexpected feature_mask %s.\n", wine_dbgstr_longlong(feature_mask)); - ok((xstate.EnabledFeatures & SUPPORTED_XSTATE_FEATURES) == SUPPORTED_XSTATE_FEATURES, + ok((xstate.EnabledFeatures & supported_xstate_features) == supported_xstate_features, "Got unexpected EnabledFeatures %s.\n", wine_dbgstr_longlong(xstate.EnabledFeatures)); - ok((xstate.EnabledVolatileFeatures & SUPPORTED_XSTATE_FEATURES) == (xstate.EnabledFeatures & SUPPORTED_XSTATE_FEATURES), + ok((xstate.EnabledVolatileFeatures & supported_xstate_features) == (xstate.EnabledFeatures & supported_xstate_features), "Got unexpected EnabledVolatileFeatures %s.\n", wine_dbgstr_longlong(xstate.EnabledVolatileFeatures)); - ok(xstate.Size >= 512 + sizeof(XSTATE), "Got unexpected Size %lu.\n", xstate.Size); + ok(xstate.Size >= 512 + xstate_part_size, "Got unexpected Size %lu.\n", xstate.Size); if (xstate.CompactionEnabled) ok(xstate.OptimizedSave, "Got zero OptimizedSave with compaction enabled.\n"); ok(!xstate.AlignedFeatures, "Got unexpected AlignedFeatures %s.\n", wine_dbgstr_longlong(xstate.AlignedFeatures)); - ok(xstate.AllFeatureSize >= 512 + sizeof(XSTATE) + ok(xstate.AllFeatureSize >= 512 + xstate_part_size || !xstate.AllFeatureSize /* win8 on CPUs without XSAVEC */, "Got unexpected AllFeatureSize %lu.\n", xstate.AllFeatureSize); - for (i = 0; i < ARRAY_SIZE(feature_sizes); ++i) + for (i = 0; i < ARRAY_SIZE(feature_sizes[feature_set]); ++i) { - ok(xstate.AllFeatures[i] == feature_sizes[i] + ok(xstate.AllFeatures[i] == feature_sizes[feature_set][i] || !xstate.AllFeatures[i] /* win8+ on CPUs without XSAVEC */, "Got unexpected AllFeatures[%u] %lu, expected %lu.\n", i, - xstate.AllFeatures[i], feature_sizes[i]); - ok(xstate.Features[i].Size == feature_sizes[i], "Got unexpected Features[%u].Size %lu, expected %lu.\n", i, - xstate.Features[i].Size, feature_sizes[i]); - ok(xstate.Features[i].Offset == feature_offsets[i], "Got unexpected Features[%u].Offset %lu, expected %lu.\n", - i, xstate.Features[i].Offset, feature_offsets[i]); + xstate.AllFeatures[i], feature_sizes[feature_set][i]); + ok(xstate.Features[i].Size == feature_sizes[feature_set][i], "Got unexpected Features[%u].Size %lu, expected %lu.\n", i, + xstate.Features[i].Size, feature_sizes[feature_set][i]); + ok(xstate.Features[i].Offset == feature_offsets[feature_set][i], "Got unexpected Features[%u].Offset %lu, expected %lu.\n", + i, xstate.Features[i].Offset, feature_offsets[feature_set][i]); } } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8591
Hello @bshanks, I added a WIP test that takes the non-AVX CPU into account. This test succeeds on the given laptop with Windows 8.1. Is that going into the right direction? -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8591#note_112551
participants (1)
-
Bernhard Übelacker