Memory Read/Write Tracking using Region Handles (#1272)

* WIP Range Tracking - Texture invalidation seems to have large problems - Buffer/Pool invalidation may have problems - Mirror memory tracking puts an additional `add` in compiled code, we likely just want to make HLE access slower if this is the final solution. - Native project is in the messiest possible location. - [HACK] JIT memory access always uses native "fast" path - [HACK] Trying some things with texture invalidation and views. It works :) Still a few hacks, messy things, slow things More work in progress stuff (also move to memory project) Quite a bit faster now. - Unmapping GPU VA and CPU VA will now correctly update write tracking regions, and invalidate textures for the former. - The Virtual range list is now non-overlapping like the physical one. - Fixed some bugs where regions could leak. - Introduced a weird bug that I still need to track down (consistent invalid buffer in MK8 ribbon road) Move some stuff. I think we'll eventually just put the dll and so for this in a nuget package. Fix rebase. [WIP] MultiRegionHandle variable size ranges - Avoid reprotecting regions that change often (needs some tweaking) - There's still a bug in buffers, somehow. - Might want different api for minimum granularity Fix rebase issue Commit everything needed for software only tracking. Remove native components. Remove more native stuff. Cleanup Use a separate window for the background context, update opentk. (fixes linux) Some experimental changes Should get things working up to scratch - still need to try some things with flush/modification and res scale. Include address with the region action. Initial work to make range tracking work Still a ton of bugs Fix some issues with the new stuff. * Fix texture flush instability There's still some weird behaviour, but it's much improved without this. (textures with cpu modified data were flushing over it) * Find the destination texture for Buffer->Texture full copy Greatly improves performance for nvdec videos (with range tracking) * Further improve texture tracking * Disable Memory Tracking for view parents This is a temporary approach to better match behaviour on master (where invalidations would be soaked up by views, rather than trigger twice) The assumption is that when views are created to a texture, they will cover all of its data anyways. Of course, this can easily be improved in future. * Introduce some tracking tests. WIP * Complete base tests. * Add more tests for multiregion, fix existing test. * Cleanup Part 1 * Remove unnecessary code from memory tracking * Fix some inconsistencies with 3D texture rule. * Add dispose tests. * Use a background thread for the background context. Rather than setting and unsetting a context as current, doing the work on a dedicated thread with signals seems to be a bit faster. Also nerf the multithreading test a bit. * Copy to texture with matching alignment This extends the copy to work for some videos with unusual size, such as tutorial videos in SMO. It will only occur if the destination texture already exists at XCount size. * Track reads for buffer copies. Synchronize new buffers before copying overlaps. * Remove old texture flushing mechanisms. Range tracking all the way, baby. * Wake the background thread when disposing. Avoids a deadlock when games are closed. * Address Feedback 1 * Separate TextureCopy instance for background thread Also `BackgroundContextWorker.InBackground` for a more sensible idenfifier for if we're in a background thread. * Add missing XML docs. * Address Feedback * Maybe I should start drinking coffee. * Some more feedback. * Remove flush warning, Refocus window after making background context
author: riperiperi <rhy3756547@hotmail.com> 2020-10-16 21:18:35 +0100
committer: GitHub <noreply@github.com> 2020-10-16 17:18:35 -0300
commit: b4d8d893a4abb20b96fc6171841a858005906445 (patch)
tree: 1ef63c945428132a2c693b259176c2b6e9e328f6 /Ryujinx.Cpu
parent: f02791b20c2841c4f39747e4db243f9992b86846 (diff)
4 files changed, 247 insertions, 113 deletions
diff --git a/Ryujinx.Cpu/MemoryManager.cs b/Ryujinx.Cpu/MemoryManager.cs
index abbeee5f..26cc01c9 100644
--- a/Ryujinx.Cpu/MemoryManager.cs
+++ b/Ryujinx.Cpu/MemoryManager.cs
@@ -1,6 +1,9 @@
-using ARMeilleure.Memory;
+using ARMeilleure.Memory;
+using Ryujinx.Cpu.Tracking;
 using Ryujinx.Memory;
+using Ryujinx.Memory.Tracking;
 using System;
+using System.Collections.Generic;
 using System.Runtime.CompilerServices;
 using System.Runtime.InteropServices;
 using System.Threading;
@@ -10,7 +13,7 @@ namespace Ryujinx.Cpu
     /// <summary>
     /// Represents a CPU memory manager.
     /// </summary>
-    public sealed class MemoryManager : IMemoryManager, IDisposable
+    public sealed class MemoryManager : IMemoryManager, IDisposable, IVirtualMemoryManager
     {
         public const int PageBits = 12;
         public const int PageSize = 1 << PageBits;
@@ -35,6 +38,8 @@ namespace Ryujinx.Cpu
         /// </summary>
         public IntPtr PageTablePointer => _pageTable.Pointer;
 
+        public MemoryTracking Tracking { get; }
+
         /// <summary>
         /// Creates a new instance of the memory manager.
         /// </summary>
@@ -58,6 +63,9 @@ namespace Ryujinx.Cpu
             _addressSpaceSize = asSize;
             _backingMemory = backingMemory;
             _pageTable = new MemoryBlock((asSize / PageSize) * PteSize);
+
+            Tracking = new MemoryTracking(this, backingMemory, PageSize);
+            Tracking.EnablePhysicalProtection = false; // Disabled for now, as protection is done in software.
         }
 
         /// <summary>
@@ -71,14 +79,18 @@ namespace Ryujinx.Cpu
         /// <param name="size">Size to be mapped</param>
         public void Map(ulong va, ulong pa, ulong size)
         {
-            while (size != 0)
+            ulong remainingSize = size;
+            ulong oVa = va;
+            ulong oPa = pa;
+            while (remainingSize != 0)
             {
                 _pageTable.Write((va / PageSize) * PteSize, PaToPte(pa));
 
                 va += PageSize;
                 pa += PageSize;
-                size -= PageSize;
+                remainingSize -= PageSize;
             }
+            Tracking.Map(oVa, oPa, size);
         }
 
         /// <summary>
@@ -88,13 +100,16 @@ namespace Ryujinx.Cpu
         /// <param name="size">Size of the range to be unmapped</param>
         public void Unmap(ulong va, ulong size)
         {
-            while (size != 0)
+            ulong remainingSize = size;
+            ulong oVa = va;
+            while (remainingSize != 0)
             {
                 _pageTable.Write((va / PageSize) * PteSize, 0UL);
 
                 va += PageSize;
-                size -= PageSize;
+                remainingSize -= PageSize;
             }
+            Tracking.Unmap(oVa, size);
         }
 
         /// <summary>
@@ -110,6 +125,18 @@ namespace Ryujinx.Cpu
         }
 
         /// <summary>
+        /// Reads data from CPU mapped memory, with read tracking
+        /// </summary>
+        /// <typeparam name="T">Type of the data being read</typeparam>
+        /// <param name="va">Virtual address of the data in memory</param>
+        /// <returns>The data</returns>
+        public T ReadTracked<T>(ulong va) where T : unmanaged
+        {
+            SignalMemoryTracking(va, (ulong)Unsafe.SizeOf<T>(), false);
+            return MemoryMarshal.Cast<byte, T>(GetSpan(va, Unsafe.SizeOf<T>()))[0];
+        }
+
+        /// <summary>
         /// Reads data from CPU mapped memory.
         /// </summary>
         /// <param name="va">Virtual address of the data in memory</param>
@@ -133,7 +160,7 @@ namespace Ryujinx.Cpu
         }
 
         /// <summary>
-        /// Writes data to CPU mapped memory.
+        /// Writes data to CPU mapped memory, with write tracking.
         /// </summary>
         /// <param name="va">Virtual address to write the data into</param>
         /// <param name="data">Data to be written</param>
@@ -145,13 +172,13 @@ namespace Ryujinx.Cpu
                 return;
             }
 
-            MarkRegionAsModified(va, (ulong)data.Length);
+            SignalMemoryTracking(va, (ulong)data.Length, true);
 
             WriteImpl(va, data);
         }
 
         /// <summary>
-        /// Writes data to CPU mapped memory, without tracking.
+        /// Writes data to CPU mapped memory, without write tracking.
         /// </summary>
         /// <param name="va">Virtual address to write the data into</param>
         /// <param name="data">Data to be written</param>
@@ -222,15 +249,21 @@ namespace Ryujinx.Cpu
         /// </remarks>
         /// <param name="va">Virtual address of the data</param>
         /// <param name="size">Size of the data</param>
+        /// <param name="tracked">True if read tracking is triggered on the span</param>
         /// <returns>A read-only span of the data</returns>
         /// <exception cref="InvalidMemoryRegionException">Throw for unhandled invalid or unmapped memory accesses</exception>
-        public ReadOnlySpan<byte> GetSpan(ulong va, int size)
+        public ReadOnlySpan<byte> GetSpan(ulong va, int size, bool tracked = false)
         {
             if (size == 0)
             {
                 return ReadOnlySpan<byte>.Empty;
             }
 
+            if (tracked)
+            {
+                SignalMemoryTracking(va, (ulong)size, false);
+            }
+
             if (IsContiguousAndMapped(va, size))
             {
                 return _backingMemory.GetSpan(GetPhysicalAddressInternal(va), size);
@@ -295,7 +328,7 @@ namespace Ryujinx.Cpu
                 ThrowMemoryNotContiguous();
             }
 
-            MarkRegionAsModified(va, (ulong)Unsafe.SizeOf<T>());
+            SignalMemoryTracking(va, (ulong)Unsafe.SizeOf<T>(), true);
 
             return ref _backingMemory.GetRef<T>(GetPhysicalAddressInternal(va));
         }
@@ -337,6 +370,56 @@ namespace Ryujinx.Cpu
             return true;
         }
 
+        /// <summary>
+        /// Gets the physical regions that make up the given virtual address region.
+        /// If any part of the virtual region is unmapped, null is returned.
+        /// </summary>
+        /// <param name="va">Virtual address of the range</param>
+        /// <param name="size">Size of the range</param>
+        /// <returns>Array of physical regions</returns>
+        public (ulong address, ulong size)[] GetPhysicalRegions(ulong va, ulong size)
+        {
+            if (!ValidateAddress(va))
+            {
+                return null;
+            }
+
+            ulong endVa = (va + size + PageMask) & ~(ulong)PageMask;
+
+            va &= ~(ulong)PageMask;
+
+            int pages = (int)((endVa - va) / PageSize);
+
+            List<(ulong, ulong)> regions = new List<(ulong, ulong)>();
+
+            ulong regionStart = GetPhysicalAddressInternal(va);
+            ulong regionSize = PageSize;
+
+            for (int page = 0; page < pages - 1; page++)
+            {
+                if (!ValidateAddress(va + PageSize))
+                {
+                    return null;
+                }
+
+                ulong newPa = GetPhysicalAddressInternal(va + PageSize);
+
+                if (GetPhysicalAddressInternal(va) + PageSize != newPa)
+                {
+                    regions.Add((regionStart, regionSize));
+                    regionStart = newPa;
+                    regionSize = 0;
+                }
+
+                va += PageSize;
+                regionSize += PageSize;
+            }
+
+            regions.Add((regionStart, regionSize));
+
+            return regions.ToArray();
+        }
+
         private void ReadImpl(ulong va, Span<byte> data)
         {
             if (data.Length == 0)
@@ -378,99 +461,6 @@ namespace Ryujinx.Cpu
         }
 
         /// <summary>
-        /// Checks if a specified virtual memory region has been modified by the CPU since the last call.
-        /// </summary>
-        /// <param name="va">Virtual address of the region</param>
-        /// <param name="size">Size of the region</param>
-        /// <param name="id">Resource identifier number (maximum is 15)</param>
-        /// <param name="modifiedRanges">Optional array where the modified ranges should be written</param>
-        /// <returns>The number of modified ranges</returns>
-        [MethodImpl(MethodImplOptions.AggressiveInlining)]
-        public int QueryModified(ulong va, ulong size, int id, (ulong, ulong)[] modifiedRanges = null)
-        {
-            if (!ValidateAddress(va))
-            {
-                return 0;
-            }
-
-            ulong maxSize = _addressSpaceSize - va;
-
-            if (size > maxSize)
-            {
-                size = maxSize;
-            }
-
-            // We need to ensure that the tagged pointer value is negative,
-            // JIT generated code checks that to take the slow paths and call the MemoryManager Read/Write methods.
-            long tag = (0x8000L | (1L << id)) << 48;
-
-            ulong endVa = (va + size + PageMask) & ~(ulong)PageMask;
-
-            va &= ~(ulong)PageMask;
-
-            ulong rgStart = va;
-            ulong rgSize = 0;
-
-            int rangeIndex = 0;
-
-            for (; va < endVa; va += PageSize)
-            {
-                while (true)
-                {
-                    ref long pte = ref _pageTable.GetRef<long>((va >> PageBits) * PteSize);
-
-                    long pteValue = pte;
-
-                    // If the PTE value is 0, that means that the page is unmapped.
-                    // We behave as if the page was not modified, since modifying a page
-                    // that is not even mapped is impossible.
-                    if ((pteValue & tag) == tag || pteValue == 0)
-                    {
-                        if (rgSize != 0)
-                        {
-                            if (modifiedRanges != null && rangeIndex < modifiedRanges.Length)
-                            {
-                                modifiedRanges[rangeIndex] = (rgStart, rgSize);
-                            }
-
-                            rangeIndex++;
-
-                            rgSize = 0;
-                        }
-
-                        break;
-                    }
-                    else
-                    {
-                        if (Interlocked.CompareExchange(ref pte, pteValue | tag, pteValue) == pteValue)
-                        {
-                            if (rgSize == 0)
-                            {
-                                rgStart = va;
-                            }
-
-                            rgSize += PageSize;
-
-                            break;
-                        }
-                    }
-                }
-            }
-
-            if (rgSize != 0)
-            {
-                if (modifiedRanges != null && rangeIndex < modifiedRanges.Length)
-                {
-                    modifiedRanges[rangeIndex] = (rgStart, rgSize);
-                }
-
-                rangeIndex++;
-            }
-
-            return rangeIndex;
-        }
-
-        /// <summary>
         /// Checks if the page at a given CPU virtual address.
         /// </summary>
         /// <param name="va">Virtual address to check</param>
@@ -516,13 +506,24 @@ namespace Ryujinx.Cpu
         }
 
         /// <summary>
-        /// Marks a region of memory as modified by the CPU.
+        /// Reprotect a region of virtual memory for tracking. Sets software protection bits.
         /// </summary>
-        /// <param name="va">Virtual address of the region</param>
-        /// <param name="size">Size of the region</param>
-        public void MarkRegionAsModified(ulong va, ulong size)
+        /// <param name="va">Virtual address base</param>
+        /// <param name="size">Size of the region to protect</param>
+        /// <param name="protection">Memory protection to set</param>
+        public void TrackingReprotect(ulong va, ulong size, MemoryPermission protection)
         {
+            // Protection is inverted on software pages, since the default value is 0.
+            protection = (~protection) & MemoryPermission.ReadAndWrite;
+
+            long tag = (long)protection << 48;
+            if (tag > 0)
+            {
+                tag |= long.MinValue; // If any protection is present, the whole pte is negative.
+            }
+
             ulong endVa = (va + size + PageMask) & ~(ulong)PageMask;
+            long invTagMask = ~(0xffffL << 48);
 
             while (va < endVa)
             {
@@ -533,13 +534,77 @@ namespace Ryujinx.Cpu
                 do
                 {
                     pte = Volatile.Read(ref pageRef);
+                }
+                while (Interlocked.CompareExchange(ref pageRef, (pte & invTagMask) | tag, pte) != pte);
 
-                    if (pte >= 0)
-                    {
-                        break;
-                    }
+                va += PageSize;
+            }
+        }
+
+        /// <summary>
+        /// Obtains a memory tracking handle for the given virtual region. This should be disposed when finished with.
+        /// </summary>
+        /// <param name="address">CPU virtual address of the region</param>
+        /// <param name="size">Size of the region</param>
+        /// <returns>The memory tracking handle</returns>
+        public CpuRegionHandle BeginTracking(ulong address, ulong size)
+        {
+            return new CpuRegionHandle(Tracking.BeginTracking(address, size));
+        }
+
+        /// <summary>
+        /// Obtains a memory tracking handle for the given virtual region, with a specified granularity. This should be disposed when finished with.
+        /// </summary>
+        /// <param name="address">CPU virtual address of the region</param>
+        /// <param name="size">Size of the region</param>
+        /// <param name="granularity">Desired granularity of write tracking</param>
+        /// <returns>The memory tracking handle</returns>
+        public CpuMultiRegionHandle BeginGranularTracking(ulong address, ulong size, ulong granularity)
+        {
+            return new CpuMultiRegionHandle(Tracking.BeginGranularTracking(address, size, granularity));
+        }
+
+        /// <summary>
+        /// Obtains a smart memory tracking handle for the given virtual region, with a specified granularity. This should be disposed when finished with.
+        /// </summary>
+        /// <param name="address">CPU virtual address of the region</param>
+        /// <param name="size">Size of the region</param>
+        /// <param name="granularity">Desired granularity of write tracking</param>
+        /// <returns>The memory tracking handle</returns>
+        public CpuSmartMultiRegionHandle BeginSmartGranularTracking(ulong address, ulong size, ulong granularity)
+        {
+            return new CpuSmartMultiRegionHandle(Tracking.BeginSmartGranularTracking(address, size, granularity));
+        }
+
+        /// <summary>
+        /// Alerts the memory tracking that a given region has been read from or written to.
+        /// This should be called before read/write is performed.
+        /// </summary>
+        /// <param name="va">Virtual address of the region</param>
+        /// <param name="size">Size of the region</param>
+        public void SignalMemoryTracking(ulong va, ulong size, bool write)
+        {
+            // We emulate guard pages for software memory access. This makes for an easy transition to
+            // tracking using host guard pages in future, but also supporting platforms where this is not possible.
+
+            // Write tag includes read protection, since we don't have any read actions that aren't performed before write too.
+            long tag = (write ? 3L : 1L) << 48;
+
+            ulong endVa = (va + size + PageMask) & ~(ulong)PageMask;
+
+            while (va < endVa)
+            {
+                ref long pageRef = ref _pageTable.GetRef<long>((va >> PageBits) * PteSize);
+
+                long pte;
+
+                pte = Volatile.Read(ref pageRef);
+
+                if ((pte & tag) != 0)
+                {
+                    Tracking.VirtualMemoryEvent(va, size, write);
+                    break;
                 }
-                while (Interlocked.CompareExchange(ref pageRef, pte & ~(0xffffL << 48), pte) != pte);
 
                 va += PageSize;
             }
diff --git a/Ryujinx.Cpu/Tracking/CpuMultiRegionHandle.cs b/Ryujinx.Cpu/Tracking/CpuMultiRegionHandle.cs
new file mode 100644
index 00000000..f76410b4
--- /dev/null
+++ b/Ryujinx.Cpu/Tracking/CpuMultiRegionHandle.cs
@@ -0,0 +1,23 @@
+using Ryujinx.Memory.Tracking;
+using System;
+
+namespace Ryujinx.Cpu.Tracking
+{
+    public class CpuMultiRegionHandle : IMultiRegionHandle
+    {
+        private readonly MultiRegionHandle _impl;
+
+        public bool Dirty => _impl.Dirty;
+
+        internal CpuMultiRegionHandle(MultiRegionHandle impl)
+        {
+            _impl = impl;
+        }
+
+        public void Dispose() => _impl.Dispose();
+        public void QueryModified(Action<ulong, ulong> modifiedAction) => _impl.QueryModified(modifiedAction);
+        public void QueryModified(ulong address, ulong size, Action<ulong, ulong> modifiedAction) => _impl.QueryModified(address, size, modifiedAction);
+        public void QueryModified(ulong address, ulong size, Action<ulong, ulong> modifiedAction, int sequenceNumber) => _impl.QueryModified(address, size, modifiedAction, sequenceNumber);
+        public void SignalWrite() => _impl.SignalWrite();
+    }
+}
diff --git a/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs b/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs
new file mode 100644
index 00000000..9dbdbfcb
--- /dev/null
+++ b/Ryujinx.Cpu/Tracking/CpuRegionHandle.cs
@@ -0,0 +1,23 @@
+using Ryujinx.Memory.Tracking;
+
+namespace Ryujinx.Cpu.Tracking
+{
+    public class CpuRegionHandle : IRegionHandle
+    {
+        private readonly RegionHandle _impl;
+
+        public bool Dirty => _impl.Dirty;
+        public ulong Address => _impl.Address;
+        public ulong Size => _impl.Size;
+        public ulong EndAddress => _impl.EndAddress;
+
+        internal CpuRegionHandle(RegionHandle impl)
+        {
+            _impl = impl;
+        }
+
+        public void Dispose() => _impl.Dispose();
+        public void RegisterAction(RegionSignal action) => _impl.RegisterAction(action);
+        public void Reprotect() => _impl.Reprotect();
+    }
+}
diff --git a/Ryujinx.Cpu/Tracking/CpuSmartMultiRegionHandle.cs b/Ryujinx.Cpu/Tracking/CpuSmartMultiRegionHandle.cs
new file mode 100644
index 00000000..ddeeab0a
--- /dev/null
+++ b/Ryujinx.Cpu/Tracking/CpuSmartMultiRegionHandle.cs
@@ -0,0 +1,23 @@
+using Ryujinx.Memory.Tracking;
+using System;
+
+namespace Ryujinx.Cpu.Tracking
+{
+    public class CpuSmartMultiRegionHandle : IMultiRegionHandle
+    {
+        private readonly SmartMultiRegionHandle _impl;
+
+        public bool Dirty => _impl.Dirty;
+
+        internal CpuSmartMultiRegionHandle(SmartMultiRegionHandle impl)
+        {
+            _impl = impl;
+        }
+
+        public void Dispose() => _impl.Dispose();
+        public void QueryModified(Action<ulong, ulong> modifiedAction) => _impl.QueryModified(modifiedAction);
+        public void QueryModified(ulong address, ulong size, Action<ulong, ulong> modifiedAction) => _impl.QueryModified(address, size, modifiedAction);
+        public void QueryModified(ulong address, ulong size, Action<ulong, ulong> modifiedAction, int sequenceNumber) => _impl.QueryModified(address, size, modifiedAction, sequenceNumber);
+        public void SignalWrite() => _impl.SignalWrite();
+    }
+}
author	riperiperi <rhy3756547@hotmail.com>	2020-10-16 21:18:35 +0100
committer	GitHub <noreply@github.com>	2020-10-16 17:18:35 -0300
commit	b4d8d893a4abb20b96fc6171841a858005906445 (patch)
tree	1ef63c945428132a2c693b259176c2b6e9e328f6 /Ryujinx.Cpu
parent	f02791b20c2841c4f39747e4db243f9992b86846 (diff)