From 14ce9e15672d03cb6fc067316f90d81471398ebc Mon Sep 17 00:00:00 2001 From: riperiperi Date: Sat, 30 Jul 2022 00:16:29 +0200 Subject: Move partial unmap handler to the native signal handler (#3437) * Initial commit with a lot of testing stuff. * Partial Unmap Cleanup Part 1 * Fix some minor issues, hopefully windows tests. * Disable partial unmap tests on macos for now Weird issue. * Goodbye magic number * Add COMPlus_EnableAlternateStackCheck for tests `COMPlus_EnableAlternateStackCheck` is needed for NullReferenceException handling to work on linux after registering the signal handler, due to how dotnet registers its own signal handler. * Address some feedback * Force retry when memory is mapped in memory tracking This case existed before, but returning `false` no longer retries, so it would crash immediately after unprotecting the memory... Now, we return `true` to deliberately retry. This case existed before (was just broken by this change) and I don't really want to look into fixing the issue right now. Technically, this means that on guest code partial unmaps will retry _due to this_ rather than hitting the handler. I don't expect this to cause any issues. This should fix random crashes in Xenoblade Chronicles 2. * Use IsRangeMapped * Suppress MockMemoryManager.UnmapEvent warning This event is not signalled by the mock memory manager. * Remove 4kb mapping --- Ryujinx.Memory/WindowsShared/PlaceholderManager.cs | 139 +++++++++-------- .../WindowsShared/PlaceholderManager4KB.cs | 170 --------------------- Ryujinx.Memory/WindowsShared/WindowsApi.cs | 3 + 3 files changed, 80 insertions(+), 232 deletions(-) delete mode 100644 Ryujinx.Memory/WindowsShared/PlaceholderManager4KB.cs (limited to 'Ryujinx.Memory/WindowsShared') diff --git a/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs b/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs index 1b425d66..0937d462 100644 --- a/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs +++ b/Ryujinx.Memory/WindowsShared/PlaceholderManager.cs @@ -1,5 +1,7 @@ +using Ryujinx.Common.Memory.PartialUnmaps; using System; using System.Diagnostics; +using System.Runtime.CompilerServices; using System.Runtime.Versioning; using System.Threading; @@ -13,13 +15,10 @@ namespace Ryujinx.Memory.WindowsShared { private const ulong MinimumPageSize = 0x1000; - [ThreadStatic] - private static int _threadLocalPartialUnmapsCount; - private readonly IntervalTree _mappings; private readonly IntervalTree _protections; - private readonly ReaderWriterLock _partialUnmapLock; - private int _partialUnmapsCount; + private readonly IntPtr _partialUnmapStatePtr; + private readonly Thread _partialUnmapTrimThread; /// /// Creates a new instance of the Windows memory placeholder manager. @@ -28,7 +27,35 @@ namespace Ryujinx.Memory.WindowsShared { _mappings = new IntervalTree(); _protections = new IntervalTree(); - _partialUnmapLock = new ReaderWriterLock(); + + _partialUnmapStatePtr = PartialUnmapState.GlobalState; + + _partialUnmapTrimThread = new Thread(TrimThreadLocalMapLoop); + _partialUnmapTrimThread.Name = "CPU.PartialUnmapTrimThread"; + _partialUnmapTrimThread.IsBackground = true; + _partialUnmapTrimThread.Start(); + } + + /// + /// Gets a reference to the partial unmap state struct. + /// + /// A reference to the partial unmap state struct + private unsafe ref PartialUnmapState GetPartialUnmapState() + { + return ref Unsafe.AsRef((void*)_partialUnmapStatePtr); + } + + /// + /// Trims inactive threads from the partial unmap state's thread mapping every few seconds. + /// Should be run in a Background thread so that it doesn't stop the program from closing. + /// + private void TrimThreadLocalMapLoop() + { + while (true) + { + Thread.Sleep(2000); + GetPartialUnmapState().TrimThreads(); + } } /// @@ -98,7 +125,8 @@ namespace Ryujinx.Memory.WindowsShared /// Memory block that owns the mapping public void MapView(IntPtr sharedMemory, ulong srcOffset, IntPtr location, IntPtr size, MemoryBlock owner) { - _partialUnmapLock.AcquireReaderLock(Timeout.Infinite); + ref var partialUnmapLock = ref GetPartialUnmapState().PartialUnmapLock; + partialUnmapLock.AcquireReaderLock(); try { @@ -107,7 +135,7 @@ namespace Ryujinx.Memory.WindowsShared } finally { - _partialUnmapLock.ReleaseReaderLock(); + partialUnmapLock.ReleaseReaderLock(); } } @@ -221,7 +249,8 @@ namespace Ryujinx.Memory.WindowsShared /// Memory block that owns the mapping public void UnmapView(IntPtr sharedMemory, IntPtr location, IntPtr size, MemoryBlock owner) { - _partialUnmapLock.AcquireReaderLock(Timeout.Infinite); + ref var partialUnmapLock = ref GetPartialUnmapState().PartialUnmapLock; + partialUnmapLock.AcquireReaderLock(); try { @@ -229,7 +258,7 @@ namespace Ryujinx.Memory.WindowsShared } finally { - _partialUnmapLock.ReleaseReaderLock(); + partialUnmapLock.ReleaseReaderLock(); } } @@ -265,11 +294,6 @@ namespace Ryujinx.Memory.WindowsShared if (IsMapped(overlap.Value)) { - if (!WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)overlap.Start, 2)) - { - throw new WindowsApiException("UnmapViewOfFile2"); - } - // Tree operations might modify the node start/end values, so save a copy before we modify the tree. ulong overlapStart = overlap.Start; ulong overlapEnd = overlap.End; @@ -291,30 +315,46 @@ namespace Ryujinx.Memory.WindowsShared // This is necessary because Windows does not support partial view unmaps. // That is, you can only fully unmap a view that was previously mapped, you can't just unmap a chunck of it. - LockCookie lockCookie = _partialUnmapLock.UpgradeToWriterLock(Timeout.Infinite); - - _partialUnmapsCount++; + ref var partialUnmapState = ref GetPartialUnmapState(); + ref var partialUnmapLock = ref partialUnmapState.PartialUnmapLock; + partialUnmapLock.UpgradeToWriterLock(); - if (overlapStartsBefore) + try { - ulong remapSize = startAddress - overlapStart; - - MapViewInternal(sharedMemory, overlapValue, (IntPtr)overlapStart, (IntPtr)remapSize); - RestoreRangeProtection(overlapStart, remapSize); + partialUnmapState.PartialUnmapsCount++; + + if (!WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)overlapStart, 2)) + { + throw new WindowsApiException("UnmapViewOfFile2"); + } + + if (overlapStartsBefore) + { + ulong remapSize = startAddress - overlapStart; + + MapViewInternal(sharedMemory, overlapValue, (IntPtr)overlapStart, (IntPtr)remapSize); + RestoreRangeProtection(overlapStart, remapSize); + } + + if (overlapEndsAfter) + { + ulong overlappedSize = endAddress - overlapStart; + ulong remapBackingOffset = overlapValue + overlappedSize; + ulong remapAddress = overlapStart + overlappedSize; + ulong remapSize = overlapEnd - endAddress; + + MapViewInternal(sharedMemory, remapBackingOffset, (IntPtr)remapAddress, (IntPtr)remapSize); + RestoreRangeProtection(remapAddress, remapSize); + } } - - if (overlapEndsAfter) + finally { - ulong overlappedSize = endAddress - overlapStart; - ulong remapBackingOffset = overlapValue + overlappedSize; - ulong remapAddress = overlapStart + overlappedSize; - ulong remapSize = overlapEnd - endAddress; - - MapViewInternal(sharedMemory, remapBackingOffset, (IntPtr)remapAddress, (IntPtr)remapSize); - RestoreRangeProtection(remapAddress, remapSize); + partialUnmapLock.DowngradeFromWriterLock(); } - - _partialUnmapLock.DowngradeFromWriterLock(ref lockCookie); + } + else if (!WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)overlapStart, 2)) + { + throw new WindowsApiException("UnmapViewOfFile2"); } } } @@ -394,7 +434,8 @@ namespace Ryujinx.Memory.WindowsShared /// True if the reprotection was successful, false otherwise public bool ReprotectView(IntPtr address, IntPtr size, MemoryPermission permission) { - _partialUnmapLock.AcquireReaderLock(Timeout.Infinite); + ref var partialUnmapLock = ref GetPartialUnmapState().PartialUnmapLock; + partialUnmapLock.AcquireReaderLock(); try { @@ -402,7 +443,7 @@ namespace Ryujinx.Memory.WindowsShared } finally { - _partialUnmapLock.ReleaseReaderLock(); + partialUnmapLock.ReleaseReaderLock(); } } @@ -659,31 +700,5 @@ namespace Ryujinx.Memory.WindowsShared ReprotectViewInternal((IntPtr)protAddress, (IntPtr)(protEndAddress - protAddress), protection.Value, true); } } - - /// - /// Checks if an access violation handler should retry execution due to a fault caused by partial unmap. - /// - /// - /// Due to Windows limitations, might need to unmap more memory than requested. - /// The additional memory that was unmapped is later remapped, however this leaves a time gap where the - /// memory might be accessed but is unmapped. Users of the API must compensate for that by catching the - /// access violation and retrying if it happened between the unmap and remap operation. - /// This method can be used to decide if retrying in such cases is necessary or not. - /// - /// True if execution should be retried, false otherwise - public bool RetryFromAccessViolation() - { - _partialUnmapLock.AcquireReaderLock(Timeout.Infinite); - - bool retry = _threadLocalPartialUnmapsCount != _partialUnmapsCount; - if (retry) - { - _threadLocalPartialUnmapsCount = _partialUnmapsCount; - } - - _partialUnmapLock.ReleaseReaderLock(); - - return retry; - } } } \ No newline at end of file diff --git a/Ryujinx.Memory/WindowsShared/PlaceholderManager4KB.cs b/Ryujinx.Memory/WindowsShared/PlaceholderManager4KB.cs deleted file mode 100644 index fc056a2f..00000000 --- a/Ryujinx.Memory/WindowsShared/PlaceholderManager4KB.cs +++ /dev/null @@ -1,170 +0,0 @@ -using System; -using System.Runtime.Versioning; - -namespace Ryujinx.Memory.WindowsShared -{ - /// - /// Windows 4KB memory placeholder manager. - /// - [SupportedOSPlatform("windows")] - class PlaceholderManager4KB - { - private const int PageSize = MemoryManagementWindows.PageSize; - - private readonly IntervalTree _mappings; - - /// - /// Creates a new instance of the Windows 4KB memory placeholder manager. - /// - public PlaceholderManager4KB() - { - _mappings = new IntervalTree(); - } - - /// - /// Unmaps the specified range of memory and marks it as mapped internally. - /// - /// - /// Since this marks the range as mapped, the expectation is that the range will be mapped after calling this method. - /// - /// Memory address to unmap and mark as mapped - /// Size of the range in bytes - public void UnmapAndMarkRangeAsMapped(IntPtr location, IntPtr size) - { - ulong startAddress = (ulong)location; - ulong unmapSize = (ulong)size; - ulong endAddress = startAddress + unmapSize; - - var overlaps = Array.Empty>(); - int count = 0; - - lock (_mappings) - { - count = _mappings.Get(startAddress, endAddress, ref overlaps); - } - - for (int index = 0; index < count; index++) - { - var overlap = overlaps[index]; - - // Tree operations might modify the node start/end values, so save a copy before we modify the tree. - ulong overlapStart = overlap.Start; - ulong overlapEnd = overlap.End; - ulong overlapValue = overlap.Value; - - _mappings.Remove(overlap); - - ulong unmapStart = Math.Max(overlapStart, startAddress); - ulong unmapEnd = Math.Min(overlapEnd, endAddress); - - if (overlapStart < startAddress) - { - startAddress = overlapStart; - } - - if (overlapEnd > endAddress) - { - endAddress = overlapEnd; - } - - ulong currentAddress = unmapStart; - while (currentAddress < unmapEnd) - { - WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)currentAddress, 2); - currentAddress += PageSize; - } - } - - _mappings.Add(startAddress, endAddress, 0); - } - - /// - /// Unmaps views at the specified memory range. - /// - /// Address of the range - /// Size of the range in bytes - public void UnmapView(IntPtr location, IntPtr size) - { - ulong startAddress = (ulong)location; - ulong unmapSize = (ulong)size; - ulong endAddress = startAddress + unmapSize; - - var overlaps = Array.Empty>(); - int count = 0; - - lock (_mappings) - { - count = _mappings.Get(startAddress, endAddress, ref overlaps); - } - - for (int index = 0; index < count; index++) - { - var overlap = overlaps[index]; - - // Tree operations might modify the node start/end values, so save a copy before we modify the tree. - ulong overlapStart = overlap.Start; - ulong overlapEnd = overlap.End; - - _mappings.Remove(overlap); - - if (overlapStart < startAddress) - { - _mappings.Add(overlapStart, startAddress, 0); - } - - if (overlapEnd > endAddress) - { - _mappings.Add(endAddress, overlapEnd, 0); - } - - ulong unmapStart = Math.Max(overlapStart, startAddress); - ulong unmapEnd = Math.Min(overlapEnd, endAddress); - - ulong currentAddress = unmapStart; - while (currentAddress < unmapEnd) - { - WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)currentAddress, 2); - currentAddress += PageSize; - } - } - } - - /// - /// Unmaps mapped memory at a given range. - /// - /// Address of the range - /// Size of the range in bytes - public void UnmapRange(IntPtr location, IntPtr size) - { - ulong startAddress = (ulong)location; - ulong unmapSize = (ulong)size; - ulong endAddress = startAddress + unmapSize; - - var overlaps = Array.Empty>(); - int count = 0; - - lock (_mappings) - { - count = _mappings.Get(startAddress, endAddress, ref overlaps); - } - - for (int index = 0; index < count; index++) - { - var overlap = overlaps[index]; - - // Tree operations might modify the node start/end values, so save a copy before we modify the tree. - ulong unmapStart = Math.Max(overlap.Start, startAddress); - ulong unmapEnd = Math.Min(overlap.End, endAddress); - - _mappings.Remove(overlap); - - ulong currentAddress = unmapStart; - while (currentAddress < unmapEnd) - { - WindowsApi.UnmapViewOfFile2(WindowsApi.CurrentProcessHandle, (IntPtr)currentAddress, 2); - currentAddress += PageSize; - } - } - } - } -} \ No newline at end of file diff --git a/Ryujinx.Memory/WindowsShared/WindowsApi.cs b/Ryujinx.Memory/WindowsShared/WindowsApi.cs index 297bd1ee..cbb7d99e 100644 --- a/Ryujinx.Memory/WindowsShared/WindowsApi.cs +++ b/Ryujinx.Memory/WindowsShared/WindowsApi.cs @@ -76,6 +76,9 @@ namespace Ryujinx.Memory.WindowsShared [DllImport("kernel32.dll")] public static extern uint GetLastError(); + [DllImport("kernel32.dll")] + public static extern int GetCurrentThreadId(); + public static MemoryProtection GetProtection(MemoryPermission permission) { return permission switch -- cgit v1.2.3