From fdd3263e31f8bf352a21e05703d0a6a82c800995 Mon Sep 17 00:00:00 2001 From: riperiperi Date: Thu, 14 Mar 2024 22:38:27 +0000 Subject: Separate guest/host tracking + unaligned protection (#6486) * WIP: Separate guest/host tracking + unaligned protection Allow memory manager to define support for single byte guest tracking * Formatting * Improve docs * Properly handle cases where the address space bits are too low * Address feedback --- src/Ryujinx.Memory/Tracking/MemoryTracking.cs | 137 +++++++++++++++++------ src/Ryujinx.Memory/Tracking/MultiRegionHandle.cs | 9 +- src/Ryujinx.Memory/Tracking/RegionFlags.cs | 21 ++++ src/Ryujinx.Memory/Tracking/RegionHandle.cs | 68 +++++++++-- src/Ryujinx.Memory/Tracking/VirtualRegion.cs | 10 +- 5 files changed, 194 insertions(+), 51 deletions(-) create mode 100644 src/Ryujinx.Memory/Tracking/RegionFlags.cs (limited to 'src/Ryujinx.Memory/Tracking') diff --git a/src/Ryujinx.Memory/Tracking/MemoryTracking.cs b/src/Ryujinx.Memory/Tracking/MemoryTracking.cs index 6febcbbb..96cb2c5f 100644 --- a/src/Ryujinx.Memory/Tracking/MemoryTracking.cs +++ b/src/Ryujinx.Memory/Tracking/MemoryTracking.cs @@ -14,9 +14,14 @@ namespace Ryujinx.Memory.Tracking // Only use these from within the lock. private readonly NonOverlappingRangeList _virtualRegions; + // Guest virtual regions are a subset of the normal virtual regions, with potentially different protection + // and expanded area of effect on platforms that don't support misaligned page protection. + private readonly NonOverlappingRangeList _guestVirtualRegions; private readonly int _pageSize; + private readonly bool _singleByteGuestTracking; + /// /// This lock must be obtained when traversing or updating the region-handle hierarchy. /// It is not required when reading dirty flags. @@ -27,16 +32,27 @@ namespace Ryujinx.Memory.Tracking /// Create a new tracking structure for the given "physical" memory block, /// with a given "virtual" memory manager that will provide mappings and virtual memory protection. /// + /// + /// If is true, the memory manager must also support protection on partially + /// unmapped regions without throwing exceptions or dropping protection on the mapped portion. + /// /// Virtual memory manager - /// Physical memory block /// Page size of the virtual memory space - public MemoryTracking(IVirtualMemoryManager memoryManager, int pageSize, InvalidAccessHandler invalidAccessHandler = null) + /// Method to call for invalid memory accesses + /// True if the guest only signals writes for the first byte + public MemoryTracking( + IVirtualMemoryManager memoryManager, + int pageSize, + InvalidAccessHandler invalidAccessHandler = null, + bool singleByteGuestTracking = false) { _memoryManager = memoryManager; _pageSize = pageSize; _invalidAccessHandler = invalidAccessHandler; + _singleByteGuestTracking = singleByteGuestTracking; _virtualRegions = new NonOverlappingRangeList(); + _guestVirtualRegions = new NonOverlappingRangeList(); } private (ulong address, ulong size) PageAlign(ulong address, ulong size) @@ -62,20 +78,25 @@ namespace Ryujinx.Memory.Tracking { ref var overlaps = ref ThreadStaticArray.Get(); - int count = _virtualRegions.FindOverlapsNonOverlapping(va, size, ref overlaps); - - for (int i = 0; i < count; i++) + for (int type = 0; type < 2; type++) { - VirtualRegion region = overlaps[i]; + NonOverlappingRangeList regions = type == 0 ? _virtualRegions : _guestVirtualRegions; + + int count = regions.FindOverlapsNonOverlapping(va, size, ref overlaps); - // If the region has been fully remapped, signal that it has been mapped again. - bool remapped = _memoryManager.IsRangeMapped(region.Address, region.Size); - if (remapped) + for (int i = 0; i < count; i++) { - region.SignalMappingChanged(true); - } + VirtualRegion region = overlaps[i]; + + // If the region has been fully remapped, signal that it has been mapped again. + bool remapped = _memoryManager.IsRangeMapped(region.Address, region.Size); + if (remapped) + { + region.SignalMappingChanged(true); + } - region.UpdateProtection(); + region.UpdateProtection(); + } } } } @@ -95,27 +116,58 @@ namespace Ryujinx.Memory.Tracking { ref var overlaps = ref ThreadStaticArray.Get(); - int count = _virtualRegions.FindOverlapsNonOverlapping(va, size, ref overlaps); - - for (int i = 0; i < count; i++) + for (int type = 0; type < 2; type++) { - VirtualRegion region = overlaps[i]; + NonOverlappingRangeList regions = type == 0 ? _virtualRegions : _guestVirtualRegions; + + int count = regions.FindOverlapsNonOverlapping(va, size, ref overlaps); - region.SignalMappingChanged(false); + for (int i = 0; i < count; i++) + { + VirtualRegion region = overlaps[i]; + + region.SignalMappingChanged(false); + } } } } + /// + /// Alter a tracked memory region to properly capture unaligned accesses. + /// For most memory manager modes, this does nothing. + /// + /// Original region address + /// Original region size + /// A new address and size for tracking unaligned accesses + internal (ulong newAddress, ulong newSize) GetUnalignedSafeRegion(ulong address, ulong size) + { + if (_singleByteGuestTracking) + { + // The guest only signals the first byte of each memory access with the current memory manager. + // To catch unaligned access properly, we need to also protect the page before the address. + + // Assume that the address and size are already aligned. + + return (address - (ulong)_pageSize, size + (ulong)_pageSize); + } + else + { + return (address, size); + } + } + /// /// Get a list of virtual regions that a handle covers. /// /// Starting virtual memory address of the handle /// Size of the handle's memory region + /// True if getting handles for guest protection, false otherwise /// A list of virtual regions within the given range - internal List GetVirtualRegionsForHandle(ulong va, ulong size) + internal List GetVirtualRegionsForHandle(ulong va, ulong size, bool guest) { List result = new(); - _virtualRegions.GetOrAddRegions(result, va, size, (va, size) => new VirtualRegion(this, va, size)); + NonOverlappingRangeList regions = guest ? _guestVirtualRegions : _virtualRegions; + regions.GetOrAddRegions(result, va, size, (va, size) => new VirtualRegion(this, va, size, guest)); return result; } @@ -126,7 +178,14 @@ namespace Ryujinx.Memory.Tracking /// Region to remove internal void RemoveVirtual(VirtualRegion region) { - _virtualRegions.Remove(region); + if (region.Guest) + { + _guestVirtualRegions.Remove(region); + } + else + { + _virtualRegions.Remove(region); + } } /// @@ -137,10 +196,11 @@ namespace Ryujinx.Memory.Tracking /// Handles to inherit state from or reuse. When none are present, provide null /// Desired granularity of write tracking /// Handle ID + /// Region flags /// The memory tracking handle - public MultiRegionHandle BeginGranularTracking(ulong address, ulong size, IEnumerable handles, ulong granularity, int id) + public MultiRegionHandle BeginGranularTracking(ulong address, ulong size, IEnumerable handles, ulong granularity, int id, RegionFlags flags = RegionFlags.None) { - return new MultiRegionHandle(this, address, size, handles, granularity, id); + return new MultiRegionHandle(this, address, size, handles, granularity, id, flags); } /// @@ -164,15 +224,16 @@ namespace Ryujinx.Memory.Tracking /// CPU virtual address of the region /// Size of the region /// Handle ID + /// Region flags /// The memory tracking handle - public RegionHandle BeginTracking(ulong address, ulong size, int id) + public RegionHandle BeginTracking(ulong address, ulong size, int id, RegionFlags flags = RegionFlags.None) { var (paAddress, paSize) = PageAlign(address, size); lock (TrackingLock) { bool mapped = _memoryManager.IsRangeMapped(address, size); - RegionHandle handle = new(this, paAddress, paSize, address, size, id, mapped); + RegionHandle handle = new(this, paAddress, paSize, address, size, id, flags, mapped); return handle; } @@ -186,15 +247,16 @@ namespace Ryujinx.Memory.Tracking /// The bitmap owning the dirty flag for this handle /// The bit of this handle within the dirty flag /// Handle ID + /// Region flags /// The memory tracking handle - internal RegionHandle BeginTrackingBitmap(ulong address, ulong size, ConcurrentBitmap bitmap, int bit, int id) + internal RegionHandle BeginTrackingBitmap(ulong address, ulong size, ConcurrentBitmap bitmap, int bit, int id, RegionFlags flags = RegionFlags.None) { var (paAddress, paSize) = PageAlign(address, size); lock (TrackingLock) { bool mapped = _memoryManager.IsRangeMapped(address, size); - RegionHandle handle = new(this, paAddress, paSize, address, size, bitmap, bit, id, mapped); + RegionHandle handle = new(this, paAddress, paSize, address, size, bitmap, bit, id, flags, mapped); return handle; } @@ -202,6 +264,7 @@ namespace Ryujinx.Memory.Tracking /// /// Signal that a virtual memory event happened at the given location. + /// The memory event is assumed to be triggered by guest code. /// /// Virtual address accessed /// Size of the region affected in bytes @@ -209,7 +272,7 @@ namespace Ryujinx.Memory.Tracking /// True if the event triggered any tracking regions, false otherwise public bool VirtualMemoryEvent(ulong address, ulong size, bool write) { - return VirtualMemoryEvent(address, size, write, precise: false, null); + return VirtualMemoryEvent(address, size, write, precise: false, exemptId: null, guest: true); } /// @@ -222,8 +285,9 @@ namespace Ryujinx.Memory.Tracking /// Whether the region was written to or read /// True if the access is precise, false otherwise /// Optional ID that of the handles that should not be signalled + /// True if the access is from the guest, false otherwise /// True if the event triggered any tracking regions, false otherwise - public bool VirtualMemoryEvent(ulong address, ulong size, bool write, bool precise, int? exemptId = null) + public bool VirtualMemoryEvent(ulong address, ulong size, bool write, bool precise, int? exemptId = null, bool guest = false) { // Look up the virtual region using the region list. // Signal up the chain to relevant handles. @@ -234,7 +298,9 @@ namespace Ryujinx.Memory.Tracking { ref var overlaps = ref ThreadStaticArray.Get(); - int count = _virtualRegions.FindOverlapsNonOverlapping(address, size, ref overlaps); + NonOverlappingRangeList regions = guest ? _guestVirtualRegions : _virtualRegions; + + int count = regions.FindOverlapsNonOverlapping(address, size, ref overlaps); if (count == 0 && !precise) { @@ -242,7 +308,7 @@ namespace Ryujinx.Memory.Tracking { // TODO: There is currently the possibility that a page can be protected after its virtual region is removed. // This code handles that case when it happens, but it would be better to find out how this happens. - _memoryManager.TrackingReprotect(address & ~(ulong)(_pageSize - 1), (ulong)_pageSize, MemoryPermission.ReadAndWrite); + _memoryManager.TrackingReprotect(address & ~(ulong)(_pageSize - 1), (ulong)_pageSize, MemoryPermission.ReadAndWrite, guest); return true; // This memory _should_ be mapped, so we need to try again. } else @@ -252,6 +318,12 @@ namespace Ryujinx.Memory.Tracking } else { + if (guest && _singleByteGuestTracking) + { + // Increase the access size to trigger handles with misaligned accesses. + size += (ulong)_pageSize; + } + for (int i = 0; i < count; i++) { VirtualRegion region = overlaps[i]; @@ -285,9 +357,10 @@ namespace Ryujinx.Memory.Tracking /// /// Region to reprotect /// Memory permission to protect with - internal void ProtectVirtualRegion(VirtualRegion region, MemoryPermission permission) + /// True if the protection is for guest access, false otherwise + internal void ProtectVirtualRegion(VirtualRegion region, MemoryPermission permission, bool guest) { - _memoryManager.TrackingReprotect(region.Address, region.Size, permission); + _memoryManager.TrackingReprotect(region.Address, region.Size, permission, guest); } /// diff --git a/src/Ryujinx.Memory/Tracking/MultiRegionHandle.cs b/src/Ryujinx.Memory/Tracking/MultiRegionHandle.cs index 1c1b48b3..6fdca69f 100644 --- a/src/Ryujinx.Memory/Tracking/MultiRegionHandle.cs +++ b/src/Ryujinx.Memory/Tracking/MultiRegionHandle.cs @@ -37,7 +37,8 @@ namespace Ryujinx.Memory.Tracking ulong size, IEnumerable handles, ulong granularity, - int id) + int id, + RegionFlags flags) { _handles = new RegionHandle[(size + granularity - 1) / granularity]; Granularity = granularity; @@ -62,7 +63,7 @@ namespace Ryujinx.Memory.Tracking // Fill any gap left before this handle. while (i < startIndex) { - RegionHandle fillHandle = tracking.BeginTrackingBitmap(address + (ulong)i * granularity, granularity, _dirtyBitmap, i, id); + RegionHandle fillHandle = tracking.BeginTrackingBitmap(address + (ulong)i * granularity, granularity, _dirtyBitmap, i, id, flags); fillHandle.Parent = this; _handles[i++] = fillHandle; } @@ -83,7 +84,7 @@ namespace Ryujinx.Memory.Tracking while (i < endIndex) { - RegionHandle splitHandle = tracking.BeginTrackingBitmap(address + (ulong)i * granularity, granularity, _dirtyBitmap, i, id); + RegionHandle splitHandle = tracking.BeginTrackingBitmap(address + (ulong)i * granularity, granularity, _dirtyBitmap, i, id, flags); splitHandle.Parent = this; splitHandle.Reprotect(handle.Dirty); @@ -106,7 +107,7 @@ namespace Ryujinx.Memory.Tracking // Fill any remaining space with new handles. while (i < _handles.Length) { - RegionHandle handle = tracking.BeginTrackingBitmap(address + (ulong)i * granularity, granularity, _dirtyBitmap, i, id); + RegionHandle handle = tracking.BeginTrackingBitmap(address + (ulong)i * granularity, granularity, _dirtyBitmap, i, id, flags); handle.Parent = this; _handles[i++] = handle; } diff --git a/src/Ryujinx.Memory/Tracking/RegionFlags.cs b/src/Ryujinx.Memory/Tracking/RegionFlags.cs new file mode 100644 index 00000000..ceb8e56a --- /dev/null +++ b/src/Ryujinx.Memory/Tracking/RegionFlags.cs @@ -0,0 +1,21 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Ryujinx.Memory.Tracking +{ + [Flags] + public enum RegionFlags + { + None = 0, + + /// + /// Access to the resource is expected to occasionally be unaligned. + /// With some memory managers, guest protection must extend into the previous page to cover unaligned access. + /// If this is not expected, protection is not altered, which can avoid unintended resource dirty/flush. + /// + UnalignedAccess = 1, + } +} diff --git a/src/Ryujinx.Memory/Tracking/RegionHandle.cs b/src/Ryujinx.Memory/Tracking/RegionHandle.cs index df3d9c31..a94ffa43 100644 --- a/src/Ryujinx.Memory/Tracking/RegionHandle.cs +++ b/src/Ryujinx.Memory/Tracking/RegionHandle.cs @@ -55,6 +55,8 @@ namespace Ryujinx.Memory.Tracking private RegionSignal _preAction; // Action to perform before a read or write. This will block the memory access. private PreciseRegionSignal _preciseAction; // Action to perform on a precise read or write. private readonly List _regions; + private readonly List _guestRegions; + private readonly List _allRegions; private readonly MemoryTracking _tracking; private bool _disposed; @@ -99,6 +101,7 @@ namespace Ryujinx.Memory.Tracking /// The bitmap the dirty flag for this handle is stored in /// The bit index representing the dirty flag for this handle /// Handle ID + /// Region flags /// True if the region handle starts mapped internal RegionHandle( MemoryTracking tracking, @@ -109,6 +112,7 @@ namespace Ryujinx.Memory.Tracking ConcurrentBitmap bitmap, int bit, int id, + RegionFlags flags, bool mapped = true) { Bitmap = bitmap; @@ -128,11 +132,12 @@ namespace Ryujinx.Memory.Tracking RealEndAddress = realAddress + realSize; _tracking = tracking; - _regions = tracking.GetVirtualRegionsForHandle(address, size); - foreach (var region in _regions) - { - region.Handles.Add(this); - } + + _regions = tracking.GetVirtualRegionsForHandle(address, size, false); + _guestRegions = GetGuestRegions(tracking, address, size, flags); + _allRegions = new List(_regions.Count + _guestRegions.Count); + + InitializeRegions(); } /// @@ -145,8 +150,9 @@ namespace Ryujinx.Memory.Tracking /// The real, unaligned address of the handle /// The real, unaligned size of the handle /// Handle ID + /// Region flags /// True if the region handle starts mapped - internal RegionHandle(MemoryTracking tracking, ulong address, ulong size, ulong realAddress, ulong realSize, int id, bool mapped = true) + internal RegionHandle(MemoryTracking tracking, ulong address, ulong size, ulong realAddress, ulong realSize, int id, RegionFlags flags, bool mapped = true) { Bitmap = new ConcurrentBitmap(1, mapped); @@ -163,8 +169,37 @@ namespace Ryujinx.Memory.Tracking RealEndAddress = realAddress + realSize; _tracking = tracking; - _regions = tracking.GetVirtualRegionsForHandle(address, size); - foreach (var region in _regions) + + _regions = tracking.GetVirtualRegionsForHandle(address, size, false); + _guestRegions = GetGuestRegions(tracking, address, size, flags); + _allRegions = new List(_regions.Count + _guestRegions.Count); + + InitializeRegions(); + } + + private List GetGuestRegions(MemoryTracking tracking, ulong address, ulong size, RegionFlags flags) + { + ulong guestAddress; + ulong guestSize; + + if (flags.HasFlag(RegionFlags.UnalignedAccess)) + { + (guestAddress, guestSize) = tracking.GetUnalignedSafeRegion(address, size); + } + else + { + (guestAddress, guestSize) = (address, size); + } + + return tracking.GetVirtualRegionsForHandle(guestAddress, guestSize, true); + } + + private void InitializeRegions() + { + _allRegions.AddRange(_regions); + _allRegions.AddRange(_guestRegions); + + foreach (var region in _allRegions) { region.Handles.Add(this); } @@ -321,7 +356,7 @@ namespace Ryujinx.Memory.Tracking lock (_tracking.TrackingLock) { - foreach (VirtualRegion region in _regions) + foreach (VirtualRegion region in _allRegions) { protectionChanged |= region.UpdateProtection(); } @@ -379,7 +414,7 @@ namespace Ryujinx.Memory.Tracking { lock (_tracking.TrackingLock) { - foreach (VirtualRegion region in _regions) + foreach (VirtualRegion region in _allRegions) { region.UpdateProtection(); } @@ -414,7 +449,16 @@ namespace Ryujinx.Memory.Tracking /// Virtual region to add as a child internal void AddChild(VirtualRegion region) { - _regions.Add(region); + if (region.Guest) + { + _guestRegions.Add(region); + } + else + { + _regions.Add(region); + } + + _allRegions.Add(region); } /// @@ -469,7 +513,7 @@ namespace Ryujinx.Memory.Tracking lock (_tracking.TrackingLock) { - foreach (VirtualRegion region in _regions) + foreach (VirtualRegion region in _allRegions) { region.RemoveHandle(this); } diff --git a/src/Ryujinx.Memory/Tracking/VirtualRegion.cs b/src/Ryujinx.Memory/Tracking/VirtualRegion.cs index 538e94fe..bb087e9a 100644 --- a/src/Ryujinx.Memory/Tracking/VirtualRegion.cs +++ b/src/Ryujinx.Memory/Tracking/VirtualRegion.cs @@ -13,10 +13,14 @@ namespace Ryujinx.Memory.Tracking private readonly MemoryTracking _tracking; private MemoryPermission _lastPermission; - public VirtualRegion(MemoryTracking tracking, ulong address, ulong size, MemoryPermission lastPermission = MemoryPermission.Invalid) : base(address, size) + public bool Guest { get; } + + public VirtualRegion(MemoryTracking tracking, ulong address, ulong size, bool guest, MemoryPermission lastPermission = MemoryPermission.Invalid) : base(address, size) { _lastPermission = lastPermission; _tracking = tracking; + + Guest = guest; } /// @@ -103,7 +107,7 @@ namespace Ryujinx.Memory.Tracking if (_lastPermission != permission) { - _tracking.ProtectVirtualRegion(this, permission); + _tracking.ProtectVirtualRegion(this, permission, Guest); _lastPermission = permission; return true; @@ -131,7 +135,7 @@ namespace Ryujinx.Memory.Tracking public override INonOverlappingRange Split(ulong splitAddress) { - VirtualRegion newRegion = new(_tracking, splitAddress, EndAddress - splitAddress, _lastPermission); + VirtualRegion newRegion = new(_tracking, splitAddress, EndAddress - splitAddress, Guest, _lastPermission); Size = splitAddress - Address; // The new region inherits all of our parents. -- cgit v1.2.3