From a3a63d43948b79450d1a0ee963ea4796cb3532a0 Mon Sep 17 00:00:00 2001 From: jhorv <38920027+jhorv@users.noreply.github.com> Date: Sat, 9 Mar 2024 19:01:51 -0500 Subject: [PATCH] Refactor memory managers to a common base class, consolidate Read() method logic (#6360) * - add new abstract class `VirtualMemoryManagerBase` - rename `MemoryManagerBase` to `VirtualMemoryManagerRefCountedBase` and derive from `VirtualMemoryManagerBase` - change `AddressSpaceManager`, `HvMemoryManager`, `MemoryManager`, and `MemoryManagerHostMapped` to implement abstract members and use the inherited `void VirtualMemoryManagerBase.Read(TVirtual va, Span data)` implementation. * move property `AddressSpaceSize` up by the other properties --- src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs | 114 ++++-------------- src/Ryujinx.Cpu/Jit/MemoryManager.cs | 107 ++++------------ .../Jit/MemoryManagerHostMapped.cs | 54 ++------- ... => VirtualMemoryManagerRefCountedBase.cs} | 5 +- src/Ryujinx.Memory/AddressSpaceManager.cs | 85 ++----------- .../VirtualMemoryManagerBase.cs | 91 ++++++++++++++ 6 files changed, 161 insertions(+), 295 deletions(-) rename src/Ryujinx.Cpu/{MemoryManagerBase.cs => VirtualMemoryManagerRefCountedBase.cs} (70%) create mode 100644 src/Ryujinx.Memory/VirtualMemoryManagerBase.cs diff --git a/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs b/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs index 2f9743ab4..6e864f4ca 100644 --- a/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs +++ b/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs @@ -16,12 +16,8 @@ namespace Ryujinx.Cpu.AppleHv /// Represents a CPU memory manager which maps guest virtual memory directly onto the Hypervisor page table. /// [SupportedOSPlatform("macos")] - public class HvMemoryManager : MemoryManagerBase, IMemoryManager, IVirtualMemoryManagerTracked, IWritableBlock + public class HvMemoryManager : VirtualMemoryManagerRefCountedBase, IMemoryManager, IVirtualMemoryManagerTracked, IWritableBlock { - public const int PageBits = 12; - public const int PageSize = 1 << PageBits; - public const int PageMask = PageSize - 1; - public const int PageToPteShift = 5; // 32 pages (2 bits each) in one ulong page table entry. public const ulong BlockMappedMask = 0x5555555555555555; // First bit of each table entry set. @@ -39,8 +35,6 @@ namespace Ryujinx.Cpu.AppleHv private readonly InvalidAccessHandler _invalidAccessHandler; - private readonly ulong _addressSpaceSize; - private readonly HvAddressSpace _addressSpace; internal HvAddressSpace AddressSpace => _addressSpace; @@ -62,6 +56,8 @@ namespace Ryujinx.Cpu.AppleHv public event Action UnmapEvent; + protected override ulong AddressSpaceSize { get; } + /// /// Creates a new instance of the Hypervisor memory manager. /// @@ -73,7 +69,7 @@ namespace Ryujinx.Cpu.AppleHv _backingMemory = backingMemory; _pageTable = new PageTable(); _invalidAccessHandler = invalidAccessHandler; - _addressSpaceSize = addressSpaceSize; + AddressSpaceSize = addressSpaceSize; ulong asSize = PageSize; int asBits = PageBits; @@ -92,42 +88,6 @@ namespace Ryujinx.Cpu.AppleHv Tracking = new MemoryTracking(this, PageSize, invalidAccessHandler); } - /// - /// Checks if the virtual address is part of the addressable space. - /// - /// Virtual address - /// True if the virtual address is part of the addressable space - private bool ValidateAddress(ulong va) - { - return va < _addressSpaceSize; - } - - /// - /// Checks if the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// True if the combination of virtual address and size is part of the addressable space - private bool ValidateAddressAndSize(ulong va, ulong size) - { - ulong endVa = va + size; - return endVa >= va && endVa >= size && endVa <= _addressSpaceSize; - } - - /// - /// Ensures the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// Throw when the memory region specified outside the addressable space - private void AssertValidAddressAndSize(ulong va, ulong size) - { - if (!ValidateAddressAndSize(va, size)) - { - throw new InvalidMemoryRegionException($"va=0x{va:X16}, size=0x{size:X16}"); - } - } - /// public void Map(ulong va, ulong pa, ulong size, MemoryMapFlags flags) { @@ -209,9 +169,19 @@ namespace Ryujinx.Cpu.AppleHv } /// - public void Read(ulong va, Span data) + public override void Read(ulong va, Span data) { - ReadImpl(va, data); + try + { + base.Read(va, data); + } + catch (InvalidMemoryRegionException) + { + if (_invalidAccessHandler == null || !_invalidAccessHandler(va)) + { + throw; + } + } } /// @@ -340,7 +310,7 @@ namespace Ryujinx.Cpu.AppleHv { Span data = new byte[size]; - ReadImpl(va, data); + base.Read(va, data); return data; } @@ -367,7 +337,7 @@ namespace Ryujinx.Cpu.AppleHv { Memory memory = new byte[size]; - ReadImpl(va, memory.Span); + base.Read(va, memory.Span); return new WritableRegion(this, va, memory); } @@ -576,48 +546,6 @@ namespace Ryujinx.Cpu.AppleHv return regions; } - private void ReadImpl(ulong va, Span data) - { - if (data.Length == 0) - { - return; - } - - try - { - AssertValidAddressAndSize(va, (ulong)data.Length); - - int offset = 0, size; - - if ((va & PageMask) != 0) - { - ulong pa = GetPhysicalAddressChecked(va); - - size = Math.Min(data.Length, PageSize - (int)(va & PageMask)); - - _backingMemory.GetSpan(pa, size).CopyTo(data[..size]); - - offset += size; - } - - for (; offset < data.Length; offset += size) - { - ulong pa = GetPhysicalAddressChecked(va + (ulong)offset); - - size = Math.Min(data.Length - offset, PageSize); - - _backingMemory.GetSpan(pa, size).CopyTo(data.Slice(offset, size)); - } - } - catch (InvalidMemoryRegionException) - { - if (_invalidAccessHandler == null || !_invalidAccessHandler(va)) - { - throw; - } - } - } - /// /// /// This function also validates that the given range is both valid and mapped, and will throw if it is not. @@ -936,6 +864,10 @@ namespace Ryujinx.Cpu.AppleHv _addressSpace.Dispose(); } - private static void ThrowInvalidMemoryRegionException(string message) => throw new InvalidMemoryRegionException(message); + protected override Span GetPhysicalAddressSpan(ulong pa, int size) + => _backingMemory.GetSpan(pa, size); + + protected override ulong TranslateVirtualAddressForRead(ulong va) + => GetPhysicalAddressChecked(va); } } diff --git a/src/Ryujinx.Cpu/Jit/MemoryManager.cs b/src/Ryujinx.Cpu/Jit/MemoryManager.cs index b9a547025..bbfdf536e 100644 --- a/src/Ryujinx.Cpu/Jit/MemoryManager.cs +++ b/src/Ryujinx.Cpu/Jit/MemoryManager.cs @@ -14,12 +14,8 @@ namespace Ryujinx.Cpu.Jit /// /// Represents a CPU memory manager. /// - public sealed class MemoryManager : MemoryManagerBase, IMemoryManager, IVirtualMemoryManagerTracked, IWritableBlock + public sealed class MemoryManager : VirtualMemoryManagerRefCountedBase, IMemoryManager, IVirtualMemoryManagerTracked, IWritableBlock { - public const int PageBits = 12; - public const int PageSize = 1 << PageBits; - public const int PageMask = PageSize - 1; - private const int PteSize = 8; private const int PointerTagBit = 62; @@ -35,8 +31,6 @@ namespace Ryujinx.Cpu.Jit /// public int AddressSpaceBits { get; } - private readonly ulong _addressSpaceSize; - private readonly MemoryBlock _pageTable; /// @@ -50,6 +44,8 @@ namespace Ryujinx.Cpu.Jit public event Action UnmapEvent; + protected override ulong AddressSpaceSize { get; } + /// /// Creates a new instance of the memory manager. /// @@ -71,7 +67,7 @@ namespace Ryujinx.Cpu.Jit } AddressSpaceBits = asBits; - _addressSpaceSize = asSize; + AddressSpaceSize = asSize; _pageTable = new MemoryBlock((asSize / PageSize) * PteSize); Tracking = new MemoryTracking(this, PageSize); @@ -153,9 +149,19 @@ namespace Ryujinx.Cpu.Jit } /// - public void Read(ulong va, Span data) + public override void Read(ulong va, Span data) { - ReadImpl(va, data); + try + { + base.Read(va, data); + } + catch (InvalidMemoryRegionException) + { + if (_invalidAccessHandler == null || !_invalidAccessHandler(va)) + { + throw; + } + } } /// @@ -290,7 +296,7 @@ namespace Ryujinx.Cpu.Jit { Span data = new byte[size]; - ReadImpl(va, data); + base.Read(va, data); return data; } @@ -462,48 +468,6 @@ namespace Ryujinx.Cpu.Jit return regions; } - private void ReadImpl(ulong va, Span data) - { - if (data.Length == 0) - { - return; - } - - try - { - AssertValidAddressAndSize(va, (ulong)data.Length); - - int offset = 0, size; - - if ((va & PageMask) != 0) - { - ulong pa = GetPhysicalAddressInternal(va); - - size = Math.Min(data.Length, PageSize - (int)(va & PageMask)); - - _backingMemory.GetSpan(pa, size).CopyTo(data[..size]); - - offset += size; - } - - for (; offset < data.Length; offset += size) - { - ulong pa = GetPhysicalAddressInternal(va + (ulong)offset); - - size = Math.Min(data.Length - offset, PageSize); - - _backingMemory.GetSpan(pa, size).CopyTo(data.Slice(offset, size)); - } - } - catch (InvalidMemoryRegionException) - { - if (_invalidAccessHandler == null || !_invalidAccessHandler(va)) - { - throw; - } - } - } - /// public bool IsRangeMapped(ulong va, ulong size) { @@ -544,37 +508,6 @@ namespace Ryujinx.Cpu.Jit return _pageTable.Read((va / PageSize) * PteSize) != 0; } - private bool ValidateAddress(ulong va) - { - return va < _addressSpaceSize; - } - - /// - /// Checks if the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// True if the combination of virtual address and size is part of the addressable space - private bool ValidateAddressAndSize(ulong va, ulong size) - { - ulong endVa = va + size; - return endVa >= va && endVa >= size && endVa <= _addressSpaceSize; - } - - /// - /// Ensures the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// Throw when the memory region specified outside the addressable space - private void AssertValidAddressAndSize(ulong va, ulong size) - { - if (!ValidateAddressAndSize(va, size)) - { - throw new InvalidMemoryRegionException($"va=0x{va:X16}, size=0x{size:X16}"); - } - } - private ulong GetPhysicalAddressInternal(ulong va) { return PteToPa(_pageTable.Read((va / PageSize) * PteSize) & ~(0xffffUL << 48)) + (va & PageMask); @@ -691,5 +624,11 @@ namespace Ryujinx.Cpu.Jit /// Disposes of resources used by the memory manager. /// protected override void Destroy() => _pageTable.Dispose(); + + protected override Span GetPhysicalAddressSpan(ulong pa, int size) + => _backingMemory.GetSpan(pa, size); + + protected override ulong TranslateVirtualAddressForRead(ulong va) + => GetPhysicalAddressInternal(va); } } diff --git a/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs b/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs index 2b315e841..0b6ba260a 100644 --- a/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs +++ b/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs @@ -13,12 +13,8 @@ namespace Ryujinx.Cpu.Jit /// /// Represents a CPU memory manager which maps guest virtual memory directly onto a host virtual region. /// - public sealed class MemoryManagerHostMapped : MemoryManagerBase, IMemoryManager, IVirtualMemoryManagerTracked, IWritableBlock + public sealed class MemoryManagerHostMapped : VirtualMemoryManagerRefCountedBase, IMemoryManager, IVirtualMemoryManagerTracked, IWritableBlock { - public const int PageBits = 12; - public const int PageSize = 1 << PageBits; - public const int PageMask = PageSize - 1; - public const int PageToPteShift = 5; // 32 pages (2 bits each) in one ulong page table entry. public const ulong BlockMappedMask = 0x5555555555555555; // First bit of each table entry set. @@ -39,8 +35,6 @@ namespace Ryujinx.Cpu.Jit private readonly AddressSpace _addressSpace; - public ulong AddressSpaceSize { get; } - private readonly PageTable _pageTable; private readonly MemoryEhMeilleure _memoryEh; @@ -60,6 +54,8 @@ namespace Ryujinx.Cpu.Jit public event Action UnmapEvent; + protected override ulong AddressSpaceSize { get; } + /// /// Creates a new instance of the host mapped memory manager. /// @@ -91,42 +87,6 @@ namespace Ryujinx.Cpu.Jit _memoryEh = new MemoryEhMeilleure(_addressSpace.Base, _addressSpace.Mirror, Tracking); } - /// - /// Checks if the virtual address is part of the addressable space. - /// - /// Virtual address - /// True if the virtual address is part of the addressable space - private bool ValidateAddress(ulong va) - { - return va < AddressSpaceSize; - } - - /// - /// Checks if the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// True if the combination of virtual address and size is part of the addressable space - private bool ValidateAddressAndSize(ulong va, ulong size) - { - ulong endVa = va + size; - return endVa >= va && endVa >= size && endVa <= AddressSpaceSize; - } - - /// - /// Ensures the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// Throw when the memory region specified outside the addressable space - private void AssertValidAddressAndSize(ulong va, ulong size) - { - if (!ValidateAddressAndSize(va, size)) - { - throw new InvalidMemoryRegionException($"va=0x{va:X16}, size=0x{size:X16}"); - } - } - /// /// Ensures the combination of virtual address and size is part of the addressable space and fully mapped. /// @@ -235,7 +195,7 @@ namespace Ryujinx.Cpu.Jit } /// - public void Read(ulong va, Span data) + public override void Read(ulong va, Span data) { try { @@ -816,6 +776,10 @@ namespace Ryujinx.Cpu.Jit _memoryEh.Dispose(); } - private static void ThrowInvalidMemoryRegionException(string message) => throw new InvalidMemoryRegionException(message); + protected override Span GetPhysicalAddressSpan(ulong pa, int size) + => _addressSpace.Mirror.GetSpan(pa, size); + + protected override ulong TranslateVirtualAddressForRead(ulong va) + => va; } } diff --git a/src/Ryujinx.Cpu/MemoryManagerBase.cs b/src/Ryujinx.Cpu/VirtualMemoryManagerRefCountedBase.cs similarity index 70% rename from src/Ryujinx.Cpu/MemoryManagerBase.cs rename to src/Ryujinx.Cpu/VirtualMemoryManagerRefCountedBase.cs index 3288e3a49..c2d8cfb1a 100644 --- a/src/Ryujinx.Cpu/MemoryManagerBase.cs +++ b/src/Ryujinx.Cpu/VirtualMemoryManagerRefCountedBase.cs @@ -1,10 +1,13 @@ using Ryujinx.Memory; using System.Diagnostics; +using System.Numerics; using System.Threading; namespace Ryujinx.Cpu { - public abstract class MemoryManagerBase : IRefCounted + public abstract class VirtualMemoryManagerRefCountedBase : VirtualMemoryManagerBase, IRefCounted + where TVirtual : IBinaryInteger + where TPhysical : IBinaryInteger { private int _referenceCount; diff --git a/src/Ryujinx.Memory/AddressSpaceManager.cs b/src/Ryujinx.Memory/AddressSpaceManager.cs index 021d33663..b953eb306 100644 --- a/src/Ryujinx.Memory/AddressSpaceManager.cs +++ b/src/Ryujinx.Memory/AddressSpaceManager.cs @@ -11,12 +11,8 @@ namespace Ryujinx.Memory /// Represents a address space manager. /// Supports virtual memory region mapping, address translation and read/write access to mapped regions. /// - public sealed class AddressSpaceManager : IVirtualMemoryManager, IWritableBlock + public sealed class AddressSpaceManager : VirtualMemoryManagerBase, IVirtualMemoryManager, IWritableBlock { - public const int PageBits = PageTable.PageBits; - public const int PageSize = PageTable.PageSize; - public const int PageMask = PageTable.PageMask; - /// public bool Supports4KBPages => true; @@ -25,11 +21,11 @@ namespace Ryujinx.Memory /// public int AddressSpaceBits { get; } - private readonly ulong _addressSpaceSize; - private readonly MemoryBlock _backingMemory; private readonly PageTable _pageTable; + protected override ulong AddressSpaceSize { get; } + /// /// Creates a new instance of the memory manager. /// @@ -47,7 +43,7 @@ namespace Ryujinx.Memory } AddressSpaceBits = asBits; - _addressSpaceSize = asSize; + AddressSpaceSize = asSize; _backingMemory = backingMemory; _pageTable = new PageTable(); } @@ -102,12 +98,6 @@ namespace Ryujinx.Memory return MemoryMarshal.Cast(GetSpan(va, Unsafe.SizeOf()))[0]; } - /// - public void Read(ulong va, Span data) - { - ReadImpl(va, data); - } - /// public void Write(ulong va, T value) where T : unmanaged { @@ -174,7 +164,7 @@ namespace Ryujinx.Memory { Span data = new byte[size]; - ReadImpl(va, data); + Read(va, data); return data; } @@ -346,34 +336,6 @@ namespace Ryujinx.Memory return regions; } - private void ReadImpl(ulong va, Span data) - { - if (data.Length == 0) - { - return; - } - - AssertValidAddressAndSize(va, (ulong)data.Length); - - int offset = 0, size; - - if ((va & PageMask) != 0) - { - size = Math.Min(data.Length, PageSize - (int)(va & PageMask)); - - GetHostSpanContiguous(va, size).CopyTo(data[..size]); - - offset += size; - } - - for (; offset < data.Length; offset += size) - { - size = Math.Min(data.Length - offset, PageSize); - - GetHostSpanContiguous(va + (ulong)offset, size).CopyTo(data.Slice(offset, size)); - } - } - /// [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool IsMapped(ulong va) @@ -414,37 +376,6 @@ namespace Ryujinx.Memory return true; } - private bool ValidateAddress(ulong va) - { - return va < _addressSpaceSize; - } - - /// - /// Checks if the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// True if the combination of virtual address and size is part of the addressable space - private bool ValidateAddressAndSize(ulong va, ulong size) - { - ulong endVa = va + size; - return endVa >= va && endVa >= size && endVa <= _addressSpaceSize; - } - - /// - /// Ensures the combination of virtual address and size is part of the addressable space. - /// - /// Virtual address of the range - /// Size of the range in bytes - /// Throw when the memory region specified outside the addressable space - private void AssertValidAddressAndSize(ulong va, ulong size) - { - if (!ValidateAddressAndSize(va, size)) - { - throw new InvalidMemoryRegionException($"va=0x{va:X16}, size=0x{size:X16}"); - } - } - private unsafe Span GetHostSpanContiguous(ulong va, int size) { return new Span((void*)GetHostAddress(va), size); @@ -471,5 +402,11 @@ namespace Ryujinx.Memory { // Only the ARM Memory Manager has tracking for now. } + + protected override unsafe Span GetPhysicalAddressSpan(nuint pa, int size) + => new((void*)pa, size); + + protected override nuint TranslateVirtualAddressForRead(ulong va) + => GetHostAddress(va); } } diff --git a/src/Ryujinx.Memory/VirtualMemoryManagerBase.cs b/src/Ryujinx.Memory/VirtualMemoryManagerBase.cs new file mode 100644 index 000000000..cbec88cc5 --- /dev/null +++ b/src/Ryujinx.Memory/VirtualMemoryManagerBase.cs @@ -0,0 +1,91 @@ +using System; +using System.Numerics; + +namespace Ryujinx.Memory +{ + public abstract class VirtualMemoryManagerBase + where TVirtual : IBinaryInteger + where TPhysical : IBinaryInteger + { + public const int PageBits = 12; + public const int PageSize = 1 << PageBits; + public const int PageMask = PageSize - 1; + + protected abstract TVirtual AddressSpaceSize { get; } + + public virtual void Read(TVirtual va, Span data) + { + if (data.Length == 0) + { + return; + } + + AssertValidAddressAndSize(va, TVirtual.CreateChecked(data.Length)); + + int offset = 0, size; + + if ((int.CreateTruncating(va) & PageMask) != 0) + { + TPhysical pa = TranslateVirtualAddressForRead(va); + + size = Math.Min(data.Length, PageSize - ((int.CreateTruncating(va) & PageMask))); + + GetPhysicalAddressSpan(pa, size).CopyTo(data[..size]); + + offset += size; + } + + for (; offset < data.Length; offset += size) + { + TPhysical pa = TranslateVirtualAddressForRead(va + TVirtual.CreateChecked(offset)); + + size = Math.Min(data.Length - offset, PageSize); + + GetPhysicalAddressSpan(pa, size).CopyTo(data.Slice(offset, size)); + } + } + + /// + /// Ensures the combination of virtual address and size is part of the addressable space. + /// + /// Virtual address of the range + /// Size of the range in bytes + /// Throw when the memory region specified outside the addressable space + protected void AssertValidAddressAndSize(TVirtual va, TVirtual size) + { + if (!ValidateAddressAndSize(va, size)) + { + throw new InvalidMemoryRegionException($"va=0x{va:X16}, size=0x{size:X16}"); + } + } + + protected abstract Span GetPhysicalAddressSpan(TPhysical pa, int size); + + protected abstract TPhysical TranslateVirtualAddressForRead(TVirtual va); + + /// + /// Checks if the virtual address is part of the addressable space. + /// + /// Virtual address + /// True if the virtual address is part of the addressable space + protected bool ValidateAddress(TVirtual va) + { + return va < AddressSpaceSize; + } + + /// + /// Checks if the combination of virtual address and size is part of the addressable space. + /// + /// Virtual address of the range + /// Size of the range in bytes + /// True if the combination of virtual address and size is part of the addressable space + protected bool ValidateAddressAndSize(TVirtual va, TVirtual size) + { + TVirtual endVa = va + size; + return endVa >= va && endVa >= size && endVa <= AddressSpaceSize; + } + + protected static void ThrowInvalidMemoryRegionException(string message) + => throw new InvalidMemoryRegionException(message); + } +}