Remove unused physical region tracking (#2085)

* Remove unused physical region tracking

* Update comments

Ryujinx.Cpu/MemoryManager.cs

@@ -115,9 +115,9 @@ namespace Ryujinx.Cpu
             AssertValidAddressAndSize(va, size);
 
             UnmapEvent?.Invoke(va, size);
+            Tracking.Unmap(va, size);
 
             ulong remainingSize = size;
-            ulong oVa = va;
             while (remainingSize != 0)
             {
                 _pageTable.Write((va / PageSize) * PteSize, 0UL);
@@ -125,7 +125,6 @@ namespace Ryujinx.Cpu
                 va += PageSize;
                 remainingSize -= PageSize;
             }
-            Tracking.Unmap(oVa, size);
         }
 
         /// <summary>

Ryujinx.Memory.Tests/MultiRegionTrackingTests.cs

@@ -256,12 +256,12 @@ namespace Ryujinx.Memory.Tests
             const int pageCount = 32;
             const int overlapStart = 16;
 
-            Assert.AreEqual((0, 0), _tracking.GetRegionCounts());
+            Assert.AreEqual(0, _tracking.GetRegionCount());
 
             IMultiRegionHandle handleLow = GetGranular(smart, 0, PageSize * pageCount, PageSize);
             PreparePages(handleLow, pageCount);
 
-            Assert.AreEqual((pageCount, pageCount), _tracking.GetRegionCounts());
+            Assert.AreEqual(pageCount, _tracking.GetRegionCount());
 
             IMultiRegionHandle handleHigh = GetGranular(smart, PageSize * overlapStart, PageSize * pageCount, PageSize);
             PreparePages(handleHigh, pageCount, PageSize * overlapStart);
@@ -269,15 +269,15 @@ namespace Ryujinx.Memory.Tests
             // Combined pages (and assuming overlapStart <= pageCount) should be pageCount after overlapStart.
             int totalPages = overlapStart + pageCount;
 
-            Assert.AreEqual((totalPages, totalPages), _tracking.GetRegionCounts());
+            Assert.AreEqual(totalPages, _tracking.GetRegionCount());
 
             handleLow.Dispose(); // After disposing one, the pages for the other remain.
 
-            Assert.AreEqual((pageCount, pageCount), _tracking.GetRegionCounts());
+            Assert.AreEqual(pageCount, _tracking.GetRegionCount());
 
             handleHigh.Dispose(); // After disposing the other, there are no pages left.
 
-            Assert.AreEqual((0, 0), _tracking.GetRegionCounts());
+            Assert.AreEqual(0, _tracking.GetRegionCount());
         }
     }
 }

Ryujinx.Memory.Tests/TrackingTests.cs

@@ -32,17 +32,12 @@ namespace Ryujinx.Memory.Tests
             _memoryBlock.Dispose();
         }
 
-        private bool TestSingleWrite(RegionHandle handle, ulong address, ulong size, bool physical = false)
+        private bool TestSingleWrite(RegionHandle handle, ulong address, ulong size)
         {
             handle.Reprotect();
-            if (physical)
+
-            {
-                _tracking.PhysicalMemoryEvent(address, true);
-            }
-            else
-            {
             _tracking.VirtualMemoryEvent(address, size, true);
-            }
+
             return handle.Dirty;
         }
 
@@ -97,9 +92,6 @@ namespace Ryujinx.Memory.Tests
 
             bool dirtyAfterDispose = TestSingleWrite(handle, 0, 4);
             Assert.False(dirtyAfterDispose); // Handle cannot be triggered when disposed
-
-            bool dirtyAfterDispose2 = TestSingleWrite(handle, 0, 4, true);
-            Assert.False(dirtyAfterDispose2);
         }
 
         [Test]
@@ -362,33 +354,6 @@ namespace Ryujinx.Memory.Tests
             Assert.AreEqual(registeredCount, triggeredCount + 1);
         }
 
-        [Test]
-        public void PhysicalMemoryMapping()
-        {
-            // Tracking is done in the virtual space usually, but we also support tracking on physical regions.
-            // The physical regions that make up a virtual region are determined when the region is created,
-            // or when a mapping changes.
-
-            // These tests verify that the region cannot be signalled after unmapping, and can after remapping.
-
-            RegionHandle handle = _tracking.BeginTracking(PageSize, PageSize);
-
-            Assert.True(handle.Dirty);
-
-            bool trackedWriteTriggers = TestSingleWrite(handle, PageSize, 1, true);
-            Assert.True(trackedWriteTriggers);
-
-            _memoryManager.NoMappings = true;
-            _tracking.Unmap(PageSize, PageSize);
-            bool unmappedWriteTriggers = TestSingleWrite(handle, PageSize, 1, true);
-            Assert.False(unmappedWriteTriggers);
-
-            _memoryManager.NoMappings = false;
-            _tracking.Map(PageSize, PageSize, PageSize);
-            bool remappedWriteTriggers = TestSingleWrite(handle, PageSize, 1, true);
-            Assert.True(remappedWriteTriggers);
-        }
-
         [Test]
         public void DisposeHandles()
         {
@@ -397,11 +362,11 @@ namespace Ryujinx.Memory.Tests
             RegionHandle handle = _tracking.BeginTracking(0, PageSize);
             handle.Reprotect();
 
-            Assert.AreEqual((1, 1), _tracking.GetRegionCounts());
+            Assert.AreEqual(1, _tracking.GetRegionCount());
 
             handle.Dispose();
 
-            Assert.AreEqual((0, 0), _tracking.GetRegionCounts());
+            Assert.AreEqual(0, _tracking.GetRegionCount());
 
             // Two handles, small entirely contains big.
             // We expect there to be three regions after creating both, one for the small region and two covering the big one around it.
@@ -410,16 +375,16 @@ namespace Ryujinx.Memory.Tests
             RegionHandle handleSmall = _tracking.BeginTracking(PageSize, PageSize);
             RegionHandle handleBig = _tracking.BeginTracking(0, PageSize * 4);
 
-            Assert.AreEqual((3, 3), _tracking.GetRegionCounts());
+            Assert.AreEqual(3, _tracking.GetRegionCount());
 
             // After disposing the big region, only the small one will remain.
             handleBig.Dispose();
 
-            Assert.AreEqual((1, 1), _tracking.GetRegionCounts());
+            Assert.AreEqual(1, _tracking.GetRegionCount());
 
             handleSmall.Dispose();
 
-            Assert.AreEqual((0, 0), _tracking.GetRegionCounts());
+            Assert.AreEqual(0, _tracking.GetRegionCount());
         }
 
         [Test]

Ryujinx.Memory/Tracking/MemoryTracking.cs

@@ -13,11 +13,9 @@ namespace Ryujinx.Memory.Tracking
 
         // Only use these from within the lock.
         private readonly NonOverlappingRangeList<VirtualRegion> _virtualRegions;
-        private readonly NonOverlappingRangeList<PhysicalRegion> _physicalRegions;
 
         // Only use these from within the lock.
         private readonly VirtualRegion[] _virtualResults = new VirtualRegion[10];
-        private readonly PhysicalRegion[] _physicalResults = new PhysicalRegion[10];
 
         private readonly int _pageSize;
 
@@ -43,7 +41,6 @@ namespace Ryujinx.Memory.Tracking
             _pageSize = pageSize;
 
             _virtualRegions = new NonOverlappingRangeList<VirtualRegion>();
-            _physicalRegions = new NonOverlappingRangeList<PhysicalRegion>();
         }
 
         private (ulong address, ulong size) PageAlign(ulong address, ulong size)
@@ -82,7 +79,6 @@ namespace Ryujinx.Memory.Tracking
                         region.SignalMappingChanged(true);
                     }
 
-                    region.RecalculatePhysicalChildren();
                     region.UpdateProtection();
                 }
             }
@@ -90,14 +86,14 @@ namespace Ryujinx.Memory.Tracking
 
         /// <summary>
         /// Indicate that a virtual region has been unmapped.
-        /// Should be called after the unmapping is complete.
+        /// Should be called before the unmapping is complete.
         /// </summary>
         /// <param name="va">Virtual memory address</param>
         /// <param name="size">Size to be unmapped</param>
         public void Unmap(ulong va, ulong size)
         {
             // An unmapping may mean we need to re-evaluate each VirtualRegion's affected area.
-            // Find all handles that overlap with the range, we need to recalculate their physical regions
+            // Find all handles that overlap with the range, we need to notify them that the region was unmapped.
 
             lock (TrackingLock)
             {
@@ -107,8 +103,8 @@ namespace Ryujinx.Memory.Tracking
                 for (int i = 0; i < count; i++)
                 {
                     VirtualRegion region = results[i];
+
                     region.SignalMappingChanged(false);
-                    region.RecalculatePhysicalChildren();
                 }
             }
         }
@@ -127,31 +123,6 @@ namespace Ryujinx.Memory.Tracking
             return result;
         }
 
-        /// <summary>
-        /// Get a list of physical regions that a virtual region covers.
-        /// Note that this becomes outdated if the virtual or physical regions are unmapped or remapped.
-        /// </summary>
-        /// <param name="va">Virtual memory address</param>
-        /// <param name="size">Size of the virtual region</param>
-        /// <returns>A list of physical regions the virtual region covers</returns>
-        internal List<PhysicalRegion> GetPhysicalRegionsForVirtual(ulong va, ulong size)
-        {
-            List<PhysicalRegion> result = new List<PhysicalRegion>();
-
-            // Get a list of physical regions for this virtual region, from our injected virtual mapping function.
-            (ulong Address, ulong Size)[] physicalRegions = _memoryManager.GetPhysicalRegions(va, size);
-
-            if (physicalRegions != null)
-            {
-                foreach (var region in physicalRegions)
-                {
-                    _physicalRegions.GetOrAddRegions(result, region.Address, region.Size, (pa, size) => new PhysicalRegion(this, pa, size));
-                }
-            }
-
-            return result;
-        }
-
         /// <summary>
         /// Remove a virtual region from the range list. This assumes that the lock has been acquired.
         /// </summary>
@@ -161,15 +132,6 @@ namespace Ryujinx.Memory.Tracking
             _virtualRegions.Remove(region);
         }
 
-        /// <summary>
-        /// Remove a physical region from the range list. This assumes that the lock has been acquired.
-        /// </summary>
-        /// <param name="region">Region to remove</param>
-        internal void RemovePhysical(PhysicalRegion region)
-        {
-            _physicalRegions.Remove(region);
-        }
-
         /// <summary>
         /// Obtains a memory tracking handle for the given virtual region, with a specified granularity. This should be disposed when finished with.
         /// </summary>
@@ -216,38 +178,6 @@ namespace Ryujinx.Memory.Tracking
             }
         }
 
-        /// <summary>
-        /// Signal that a physical memory event happened at the given location.
-        /// </summary>
-        /// <param name="address">Physical address accessed</param>
-        /// <param name="write">Whether the region was written to or read</param>
-        /// <returns>True if the event triggered any tracking regions, false otherwise</returns>
-        public bool PhysicalMemoryEvent(ulong address, bool write)
-        {
-            // Look up the physical region using the region list.
-            // Signal up the chain to relevant handles.
-
-            lock (TrackingLock)
-            {
-                var results = _physicalResults;
-                int count = _physicalRegions.FindOverlapsNonOverlapping(address, 1, ref results); // TODO: get/use the actual access size?
-
-                if (count == 0)
-                {
-                    _block.Reprotect(address & ~(ulong)(_pageSize - 1), (ulong)_pageSize, MemoryPermission.ReadAndWrite);
-                    return false; // We can't handle this - unprotect and return.
-                }
-
-                for (int i = 0; i < count; i++)
-                {
-                    PhysicalRegion region = results[i];
-                    region.Signal(address, 1, write);
-                }
-            }
-
-            return true;
-        }
-
         /// <summary>
         /// Signal that a virtual memory event happened at the given location (one byte).
         /// </summary>
@@ -292,19 +222,6 @@ namespace Ryujinx.Memory.Tracking
             return true;
         }
 
-        /// <summary>
-        /// Reprotect a given physical region, if enabled. This is protected on the memory block provided during initialization.
-        /// </summary>
-        /// <param name="region">Region to reprotect</param>
-        /// <param name="permission">Memory permission to protect with</param>
-        internal void ProtectPhysicalRegion(PhysicalRegion region, MemoryPermission permission)
-        {
-            if (EnablePhysicalProtection)
-            {
-                _block.Reprotect(region.Address, region.Size, permission);
-            }
-        }
-
         /// <summary>
         /// Reprotect a given virtual region. The virtual memory manager will handle this.
         /// </summary>
@@ -316,15 +233,15 @@ namespace Ryujinx.Memory.Tracking
         }
 
         /// <summary>
-        /// Returns the number of virtual and physical regions currently being tracked.
+        /// Returns the number of virtual regions currently being tracked.
         /// Useful for tests and metrics.
         /// </summary>
-        /// <returns>The number of virtual regions, and the number of physical regions</returns>
+        /// <returns>The number of virtual regions</returns>
-        public (int VirtualCount, int PhysicalCount) GetRegionCounts()
+        public int GetRegionCount()
         {
             lock (TrackingLock)
             {
-                return (_virtualRegions.Count, _physicalRegions.Count);
+                return _virtualRegions.Count;
             }
         }
     }

Ryujinx.Memory/Tracking/PhysicalRegion.cs

@@ -1,97 +0,0 @@
-using Ryujinx.Memory.Range;
-using System.Collections.Generic;
-
-namespace Ryujinx.Memory.Tracking
-{
-    /// <summary>
-    /// A region of physical memory.
-    /// </summary>
-    class PhysicalRegion : AbstractRegion
-    {
-        public List<VirtualRegion> VirtualParents = new List<VirtualRegion>();
-        public MemoryPermission Protection { get; private set; }
-        public MemoryTracking Tracking;
-
-        public PhysicalRegion(MemoryTracking tracking, ulong address, ulong size) : base(address, size)
-        {
-            Tracking = tracking;
-            Protection = MemoryPermission.ReadAndWrite;
-        }
-
-        public override void Signal(ulong address, ulong size, bool write)
-        {
-            Protection = MemoryPermission.ReadAndWrite;
-            Tracking.ProtectPhysicalRegion(this, MemoryPermission.ReadAndWrite); // Remove our protection immedately.
-            foreach (var parent in VirtualParents)
-            {
-                parent.Signal(address, size, write);
-            }
-        }
-
-        /// <summary>
-        /// Update the protection of this region, based on our parent's requested protection.
-        /// </summary>
-        public void UpdateProtection()
-        {
-            // Re-evaluate protection, and commit to the block.
-
-            lock (Tracking.TrackingLock)
-            {
-                MemoryPermission result = MemoryPermission.ReadAndWrite;
-                foreach (var parent in VirtualParents)
-                {
-                    result &= parent.GetRequiredPermission();
-                    if (result == 0) break;
-                }
-
-                if (Protection != result)
-                {
-                    Protection = result;
-                    Tracking.ProtectPhysicalRegion(this, result);
-                }
-            }
-        }
-
-        public override INonOverlappingRange Split(ulong splitAddress)
-        {
-            PhysicalRegion newRegion = new PhysicalRegion(Tracking, splitAddress, EndAddress - splitAddress);
-            Size = splitAddress - Address;
-
-            // The new region inherits all of our parents.
-            newRegion.VirtualParents = new List<VirtualRegion>(VirtualParents);
-            foreach (var parent in VirtualParents)
-            {
-                parent.AddChild(newRegion);
-            }
-
-            return newRegion;
-        }
-
-        /// <summary>
-        /// Remove a parent virtual region from this physical region. Assumes that the tracking lock has been obtained.
-        /// </summary>
-        /// <param name="region">Region to remove</param>
-        /// <returns>True if there are no more parents and we should be removed, false otherwise.</returns>
-        public bool RemoveParent(VirtualRegion region)
-        {
-            VirtualParents.Remove(region);
-            UpdateProtection();
-            if (VirtualParents.Count == 0)
-            {
-                return true;
-            }
-            return false;
-        }
-
-        /// <summary>
-        /// Deletes this physical region if there are no more virtual parents.
-        /// </summary>
-        public void TryDelete()
-        {
-            if (VirtualParents.Count == 0)
-            {
-                Tracking.RemovePhysical(this);
-            }
-        }
-    }
-}

Ryujinx.Memory/Tracking/RegionHandle.cs

@@ -159,7 +159,7 @@ namespace Ryujinx.Memory.Tracking
         }
 
         /// <summary>
-        /// Dispose the handle. Within the tracking lock, this removes references from virtual and physical regions.
+        /// Dispose the handle. Within the tracking lock, this removes references from virtual regions.
         /// </summary>
         public void Dispose()
         {

Ryujinx.Memory/Tracking/VirtualRegion.cs

@@ -9,15 +9,12 @@ namespace Ryujinx.Memory.Tracking
     class VirtualRegion : AbstractRegion
     {
         public List<RegionHandle> Handles = new List<RegionHandle>();
-        private List<PhysicalRegion> _physicalChildren;
 
         private readonly MemoryTracking _tracking;
 
         public VirtualRegion(MemoryTracking tracking, ulong address, ulong size) : base(address, size)
         {
             _tracking = tracking;
-
-            UpdatePhysicalChildren();
         }
 
         public override void Signal(ulong address, ulong size, bool write)
@@ -30,42 +27,6 @@ namespace Ryujinx.Memory.Tracking
             UpdateProtection();
         }
 
-        /// <summary>
-        /// Clears all physical children of this region. Assumes that the tracking lock has been obtained.
-        /// </summary>
-        private void ClearPhysicalChildren()
-        {
-            if (_physicalChildren != null)
-            {
-                foreach (PhysicalRegion child in _physicalChildren)
-                {
-                    child.RemoveParent(this);
-                }
-            }
-        }
-
-        /// <summary>
-        /// Updates the physical children of this region, assuming that they are clear and that the tracking lock has been obtained.
-        /// </summary>
-        private void UpdatePhysicalChildren()
-        {
-            _physicalChildren = _tracking.GetPhysicalRegionsForVirtual(Address, Size);
-
-            foreach (PhysicalRegion child in _physicalChildren)
-            {
-                child.VirtualParents.Add(this);
-            }
-        }
-
-        /// <summary>
-        /// Recalculates the physical children for this virtual region. Assumes that the tracking lock has been obtained.
-        /// </summary>
-        public void RecalculatePhysicalChildren()
-        {
-            ClearPhysicalChildren();
-            UpdatePhysicalChildren();
-        }
-
         /// <summary>
         /// Signal that this region has been mapped or unmapped.
         /// </summary>
@@ -98,20 +59,11 @@ namespace Ryujinx.Memory.Tracking
         }
 
         /// <summary>
-        /// Updates the protection for this virtual region, and all child physical regions.
+        /// Updates the protection for this virtual region.
         /// </summary>
         public void UpdateProtection()
         {
-            // Re-evaluate protection for all physical children.
-
             _tracking.ProtectVirtualRegion(this, GetRequiredPermission());
-            lock (_tracking.TrackingLock)
-            {
-                foreach (var child in _physicalChildren)
-                {
-                    child.UpdateProtection();
-                }
-            }
         }
 
         /// <summary>
@@ -120,7 +72,6 @@ namespace Ryujinx.Memory.Tracking
         /// <param name="handle">Handle to remove</param>
         public void RemoveHandle(RegionHandle handle)
         {
-            bool removedRegions = false;
             lock (_tracking.TrackingLock)
             {
                 Handles.Remove(handle);
@@ -128,41 +79,14 @@ namespace Ryujinx.Memory.Tracking
                 if (Handles.Count == 0)
                 {
                     _tracking.RemoveVirtual(this);
-                    foreach (var child in _physicalChildren)
-                    {
-                        removedRegions |= child.RemoveParent(this);
                 }
             }
         }
 
-            if (removedRegions)
-            {
-                // The first lock will unprotect any regions that have been removed. This second lock will remove them.
-                lock (_tracking.TrackingLock)
-                {
-                    foreach (var child in _physicalChildren)
-                    {
-                        child.TryDelete();
-                    }
-                }
-            }
-        }
-
-        /// <summary>
-        /// Add a child physical region to this virtual region. Assumes that the tracking lock has been obtained.
-        /// </summary>
-        /// <param name="region">Physical region to add as a child</param>
-        public void AddChild(PhysicalRegion region)
-        {
-            _physicalChildren.Add(region);
-        }
-
         public override INonOverlappingRange Split(ulong splitAddress)
         {
-            ClearPhysicalChildren();
             VirtualRegion newRegion = new VirtualRegion(_tracking, splitAddress, EndAddress - splitAddress);
             Size = splitAddress - Address;
-            UpdatePhysicalChildren();
 
             // The new region inherits all of our parents.
             newRegion.Handles = new List<RegionHandle>(Handles);