[PATCH v3 0/1] 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...)) -- v3: ntdll: Fix XState data initialisation with non-AVX CPUs. 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/tests/virtual.c | 48 +++++++++++++++++++++++++++++++------- dlls/ntdll/unix/system.c | 4 ++-- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/dlls/ntdll/tests/virtual.c b/dlls/ntdll/tests/virtual.c index c48652f0f65..8bd5897fdcc 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,21 +1972,43 @@ static void test_user_shared_data(void) XSTATE_FEATURE Features[MAXIMUM_XSTATE_FEATURES]; }; - static const ULONG feature_offsets[] = + static const ULONG all_feature_offsets[2][3] = { + { + /* AVX present */ 0, 160, /*offsetof(XMM_SAVE_AREA32, XmmRegisters)*/ 512 /* sizeof(XMM_SAVE_AREA32) */ + offsetof(XSTATE, YmmContext), + }, + { + /* AVX not present */ + 0, + 160, /*offsetof(XMM_SAVE_AREA32, XmmRegisters)*/ + 0, + }, }; - static const ULONG feature_sizes[] = + static const ULONG all_feature_sizes[2][3] = { + { + /* AVX present */ 160, 256, /*sizeof(M128A) * 16 */ sizeof(YMMCONTEXT), + }, + { + /* AVX not present */ + 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 = (1 << XSTATE_LEGACY_FLOATING_POINT) | (1 << XSTATE_LEGACY_SSE) | (1 << XSTATE_AVX); + const ULONG *feature_sizes = all_feature_sizes[0]; + const ULONG *feature_offsets = all_feature_offsets[0]; + ULONG xstate_part_size = sizeof(XSTATE); unsigned int i; ok(user_shared_data->NumberOfPhysicalPages == sbi.MmNumberOfPhysicalPages, @@ -2030,6 +2050,15 @@ static void test_user_shared_data(void) return; } + if (!(xstate.EnabledFeatures & (1 << XSTATE_AVX))) + { + trace("AVX not present\n"); + feature_sizes = all_feature_sizes[1]; + feature_offsets = all_feature_offsets[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,20 +2069,21 @@ 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, expected %lu.\n", xstate.Size, (ULONG)(512 + xstate_part_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(all_feature_sizes[0]); ++i) { ok(xstate.AllFeatures[i] == feature_sizes[i] || !xstate.AllFeatures[i] /* win8+ on CPUs without XSAVEC */, diff --git a/dlls/ntdll/unix/system.c b/dlls/ntdll/unix/system.c index e486da40691..342e443f5e7 100644 --- a/dlls/ntdll/unix/system.c +++ b/dlls/ntdll/unix/system.c @@ -436,7 +436,6 @@ 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; @@ -469,7 +468,8 @@ static void init_xstate_features( XSTATE_CONFIGURATION *xstate ) xstate->Features[i].Offset, xstate->Features[i].Size, !!(regs[2] & 2) ); } - xstate->Size = xstate->CompactionEnabled ? off : xstate->Features[i - 1].Offset + xstate->Features[i - 1].Size; + xstate->Size = xstate->CompactionEnabled ? off : + offsetof( XSAVE_FORMAT, XmmRegisters ) + xstate->Features[i - 1].Offset + xstate->Features[i - 1].Size; TRACE( "xstate size %x, compacted %d, optimized %d.\n", xstate->Size, xstate->CompactionEnabled, xstate->OptimizedSave ); } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/8591
v2/3: - Modified test to succeed with non-AVX CPUs too. - Removed problematic return from init_xstate_features completely. This version of the patch succeeds on an Intel T4500 in Windows 8.1 and in Linux on top of wine git 91d3874, and [this is a testbot run](https://testbot.winehq.org/JobDetails.pl?Key=159385). -- https://gitlab.winehq.org/wine/wine/-/merge_requests/8591#note_112578
On Mon Aug 11 21:17:52 2025 +0000, Bernhard Übelacker wrote:
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? I'm not real knowledgeable on XState, but this looks good to me. The tests pass with Wine and Win10 on an Ivy Bridge Xeon and Coffee Lake laptop (testing both AVX disabled and enabled).
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/8591#note_112639
participants (2)
-
Bernhard Übelacker -
Brendan Shanks (＠bshanks)