From 815819767c5794624e3e7bc2bebcabe8ea4de0f6 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Tue, 7 Nov 2023 13:24:10 -0300 Subject: [PATCH] Force all exclusive memory accesses to be ordered on AppleHv (#5898) * Implement reprotect method on virtual memory manager (currently stubbed) * Force all exclusive memory accesses to be ordered on AppleHv * Format whitespace * Fix test build * Fix comment copy/paste * Fix wrong bit for isLoad * Update src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs Co-authored-by: riperiperi --------- Co-authored-by: riperiperi --- src/Ryujinx.Cpu/AppleHv/HvCodePatcher.cs | 62 +++++++++++++++++++ src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs | 33 +++++----- src/Ryujinx.Cpu/Jit/MemoryManager.cs | 23 ++----- .../Jit/MemoryManagerHostMapped.cs | 6 ++ .../HOS/Kernel/Memory/KMemoryPermission.cs | 46 ++++++++++++++ .../HOS/Kernel/Memory/KPageTable.cs | 10 +-- .../HOS/Kernel/Memory/KPageTableBase.cs | 6 +- .../HOS/Kernel/Memory/MemoryPermission.cs | 20 ------ src/Ryujinx.Memory/AddressSpaceManager.cs | 5 ++ src/Ryujinx.Memory/IVirtualMemoryManager.cs | 14 +++++ .../MockVirtualMemoryManager.cs | 5 ++ 11 files changed, 171 insertions(+), 59 deletions(-) create mode 100644 src/Ryujinx.Cpu/AppleHv/HvCodePatcher.cs create mode 100644 src/Ryujinx.HLE/HOS/Kernel/Memory/KMemoryPermission.cs delete mode 100644 src/Ryujinx.HLE/HOS/Kernel/Memory/MemoryPermission.cs diff --git a/src/Ryujinx.Cpu/AppleHv/HvCodePatcher.cs b/src/Ryujinx.Cpu/AppleHv/HvCodePatcher.cs new file mode 100644 index 00000000..876597b7 --- /dev/null +++ b/src/Ryujinx.Cpu/AppleHv/HvCodePatcher.cs @@ -0,0 +1,62 @@ +using System; +using System.Runtime.InteropServices; +using System.Runtime.Intrinsics; + +namespace Ryujinx.Cpu.AppleHv +{ + static class HvCodePatcher + { + private const uint XMask = 0x3f808000u; + private const uint XValue = 0x8000000u; + + private const uint ZrIndex = 31u; + + public static void RewriteUnorderedExclusiveInstructions(Span code) + { + Span codeUint = MemoryMarshal.Cast(code); + Span> codeVector = MemoryMarshal.Cast>(code); + + Vector128 mask = Vector128.Create(XMask); + Vector128 value = Vector128.Create(XValue); + + for (int index = 0; index < codeVector.Length; index++) + { + Vector128 v = codeVector[index]; + + if (Vector128.EqualsAny(Vector128.BitwiseAnd(v, mask), value)) + { + int baseIndex = index * 4; + + for (int instIndex = baseIndex; instIndex < baseIndex + 4; instIndex++) + { + ref uint inst = ref codeUint[instIndex]; + + if ((inst & XMask) != XValue) + { + continue; + } + + bool isPair = (inst & (1u << 21)) != 0; + bool isLoad = (inst & (1u << 22)) != 0; + + uint rt2 = (inst >> 10) & 0x1fu; + uint rs = (inst >> 16) & 0x1fu; + + if (isLoad && rs != ZrIndex) + { + continue; + } + + if (!isPair && rt2 != ZrIndex) + { + continue; + } + + // Set the ordered flag. + inst |= 1u << 15; + } + } + } + } + } +} diff --git a/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs b/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs index d5ce817a..947c3710 100644 --- a/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs +++ b/src/Ryujinx.Cpu/AppleHv/HvMemoryManager.cs @@ -128,21 +128,6 @@ namespace Ryujinx.Cpu.AppleHv } } -#pragma warning disable IDE0051 // Remove unused private member - /// - /// Ensures the combination of virtual address and size is part of the addressable space and fully mapped. - /// - /// Virtual address of the range - /// Size of the range in bytes - private void AssertMapped(ulong va, ulong size) - { - if (!ValidateAddressAndSize(va, size) || !IsRangeMappedImpl(va, size)) - { - throw new InvalidMemoryRegionException($"Not mapped: va=0x{va:X16}, size=0x{size:X16}"); - } - } -#pragma warning restore IDE0051 - /// public void Map(ulong va, ulong pa, ulong size, MemoryMapFlags flags) { @@ -736,6 +721,24 @@ namespace Ryujinx.Cpu.AppleHv return (int)(vaSpan / PageSize); } + /// + public void Reprotect(ulong va, ulong size, MemoryPermission protection) + { + if (protection.HasFlag(MemoryPermission.Execute)) + { + // Some applications use unordered exclusive memory access instructions + // where it is not valid to do so, leading to memory re-ordering that + // makes the code behave incorrectly on some CPUs. + // To work around this, we force all such accesses to be ordered. + + using WritableRegion writableRegion = GetWritableRegion(va, (int)size); + + HvCodePatcher.RewriteUnorderedExclusiveInstructions(writableRegion.Memory.Span); + } + + // TODO + } + /// public void TrackingReprotect(ulong va, ulong size, MemoryPermission protection) { diff --git a/src/Ryujinx.Cpu/Jit/MemoryManager.cs b/src/Ryujinx.Cpu/Jit/MemoryManager.cs index 1c27e97f..912e3f7e 100644 --- a/src/Ryujinx.Cpu/Jit/MemoryManager.cs +++ b/src/Ryujinx.Cpu/Jit/MemoryManager.cs @@ -575,24 +575,17 @@ namespace Ryujinx.Cpu.Jit } } -#pragma warning disable IDE0051 // Remove unused private member - private ulong GetPhysicalAddress(ulong va) - { - // We return -1L if the virtual address is invalid or unmapped. - if (!ValidateAddress(va) || !IsMapped(va)) - { - return ulong.MaxValue; - } - - return GetPhysicalAddressInternal(va); - } -#pragma warning restore IDE0051 - private ulong GetPhysicalAddressInternal(ulong va) { return PteToPa(_pageTable.Read((va / PageSize) * PteSize) & ~(0xffffUL << 48)) + (va & PageMask); } + /// + public void Reprotect(ulong va, ulong size, MemoryPermission protection) + { + // TODO + } + /// public void TrackingReprotect(ulong va, ulong size, MemoryPermission protection) { @@ -698,9 +691,5 @@ namespace Ryujinx.Cpu.Jit /// Disposes of resources used by the memory manager. /// protected override void Destroy() => _pageTable.Dispose(); - -#pragma warning disable IDE0051 // Remove unused private member - private static void ThrowInvalidMemoryRegionException(string message) => throw new InvalidMemoryRegionException(message); -#pragma warning restore IDE0051 } } diff --git a/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs b/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs index 010a0bc2..6d32787a 100644 --- a/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs +++ b/src/Ryujinx.Cpu/Jit/MemoryManagerHostMapped.cs @@ -615,6 +615,12 @@ namespace Ryujinx.Cpu.Jit return (int)(vaSpan / PageSize); } + /// + public void Reprotect(ulong va, ulong size, MemoryPermission protection) + { + // TODO + } + /// public void TrackingReprotect(ulong va, ulong size, MemoryPermission protection) { diff --git a/src/Ryujinx.HLE/HOS/Kernel/Memory/KMemoryPermission.cs b/src/Ryujinx.HLE/HOS/Kernel/Memory/KMemoryPermission.cs new file mode 100644 index 00000000..32734574 --- /dev/null +++ b/src/Ryujinx.HLE/HOS/Kernel/Memory/KMemoryPermission.cs @@ -0,0 +1,46 @@ +using Ryujinx.Memory; +using System; + +namespace Ryujinx.HLE.HOS.Kernel.Memory +{ + [Flags] + enum KMemoryPermission : uint + { + None = 0, + UserMask = Read | Write | Execute, + Mask = uint.MaxValue, + + Read = 1 << 0, + Write = 1 << 1, + Execute = 1 << 2, + DontCare = 1 << 28, + + ReadAndWrite = Read | Write, + ReadAndExecute = Read | Execute, + } + + static class KMemoryPermissionExtensions + { + public static MemoryPermission Convert(this KMemoryPermission permission) + { + MemoryPermission output = MemoryPermission.None; + + if (permission.HasFlag(KMemoryPermission.Read)) + { + output = MemoryPermission.Read; + } + + if (permission.HasFlag(KMemoryPermission.Write)) + { + output |= MemoryPermission.Write; + } + + if (permission.HasFlag(KMemoryPermission.Execute)) + { + output |= MemoryPermission.Execute; + } + + return output; + } + } +} diff --git a/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTable.cs b/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTable.cs index dcfc8f4f..4cd3e6fd 100644 --- a/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTable.cs +++ b/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTable.cs @@ -203,15 +203,17 @@ namespace Ryujinx.HLE.HOS.Kernel.Memory /// protected override Result Reprotect(ulong address, ulong pagesCount, KMemoryPermission permission) { - // TODO. + _cpuMemory.Reprotect(address, pagesCount * PageSize, permission.Convert()); + return Result.Success; } /// - protected override Result ReprotectWithAttributes(ulong address, ulong pagesCount, KMemoryPermission permission) + protected override Result ReprotectAndFlush(ulong address, ulong pagesCount, KMemoryPermission permission) { - // TODO. - return Result.Success; + // TODO: Flush JIT cache. + + return Reprotect(address, pagesCount, permission); } /// diff --git a/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTableBase.cs b/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTableBase.cs index 2b00f802..2b6d4e4e 100644 --- a/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTableBase.cs +++ b/src/Ryujinx.HLE/HOS/Kernel/Memory/KPageTableBase.cs @@ -1255,7 +1255,7 @@ namespace Ryujinx.HLE.HOS.Kernel.Memory if ((oldPermission & KMemoryPermission.Execute) != 0) { - result = ReprotectWithAttributes(address, pagesCount, permission); + result = ReprotectAndFlush(address, pagesCount, permission); } else { @@ -3036,13 +3036,13 @@ namespace Ryujinx.HLE.HOS.Kernel.Memory protected abstract Result Reprotect(ulong address, ulong pagesCount, KMemoryPermission permission); /// - /// Changes the permissions of a given virtual memory region. + /// Changes the permissions of a given virtual memory region, while also flushing the cache. /// /// Virtual address of the region to have the permission changes /// Number of pages to have their permissions changed /// New permission /// Result of the permission change operation - protected abstract Result ReprotectWithAttributes(ulong address, ulong pagesCount, KMemoryPermission permission); + protected abstract Result ReprotectAndFlush(ulong address, ulong pagesCount, KMemoryPermission permission); /// /// Alerts the memory tracking that a given region has been read from or written to. diff --git a/src/Ryujinx.HLE/HOS/Kernel/Memory/MemoryPermission.cs b/src/Ryujinx.HLE/HOS/Kernel/Memory/MemoryPermission.cs deleted file mode 100644 index 068cdbb8..00000000 --- a/src/Ryujinx.HLE/HOS/Kernel/Memory/MemoryPermission.cs +++ /dev/null @@ -1,20 +0,0 @@ -using System; - -namespace Ryujinx.HLE.HOS.Kernel.Memory -{ - [Flags] - enum KMemoryPermission : uint - { - None = 0, - UserMask = Read | Write | Execute, - Mask = uint.MaxValue, - - Read = 1 << 0, - Write = 1 << 1, - Execute = 1 << 2, - DontCare = 1 << 28, - - ReadAndWrite = Read | Write, - ReadAndExecute = Read | Execute, - } -} diff --git a/src/Ryujinx.Memory/AddressSpaceManager.cs b/src/Ryujinx.Memory/AddressSpaceManager.cs index 65b4d48f..b8d48365 100644 --- a/src/Ryujinx.Memory/AddressSpaceManager.cs +++ b/src/Ryujinx.Memory/AddressSpaceManager.cs @@ -455,6 +455,11 @@ namespace Ryujinx.Memory return _pageTable.Read(va) + (nuint)(va & PageMask); } + /// + public void Reprotect(ulong va, ulong size, MemoryPermission protection) + { + } + /// public void TrackingReprotect(ulong va, ulong size, MemoryPermission protection) { diff --git a/src/Ryujinx.Memory/IVirtualMemoryManager.cs b/src/Ryujinx.Memory/IVirtualMemoryManager.cs index edbfc885..8c9ca168 100644 --- a/src/Ryujinx.Memory/IVirtualMemoryManager.cs +++ b/src/Ryujinx.Memory/IVirtualMemoryManager.cs @@ -104,6 +104,12 @@ namespace Ryujinx.Memory /// True if the data was changed, false otherwise bool WriteWithRedundancyCheck(ulong va, ReadOnlySpan data); + /// + /// Fills the specified memory region with the value specified in . + /// + /// Virtual address to fill the value into + /// Size of the memory region to fill + /// Value to fill with void Fill(ulong va, ulong size, byte value) { const int MaxChunkSize = 1 << 24; @@ -194,6 +200,14 @@ namespace Ryujinx.Memory /// Optional ID of the handles that should not be signalled void SignalMemoryTracking(ulong va, ulong size, bool write, bool precise = false, int? exemptId = null); + /// + /// Reprotect a region of virtual memory for guest access. + /// + /// Virtual address base + /// Size of the region to protect + /// Memory protection to set + void Reprotect(ulong va, ulong size, MemoryPermission protection); + /// /// Reprotect a region of virtual memory for tracking. /// diff --git a/src/Ryujinx.Tests.Memory/MockVirtualMemoryManager.cs b/src/Ryujinx.Tests.Memory/MockVirtualMemoryManager.cs index 59dc1a52..435bb35a 100644 --- a/src/Ryujinx.Tests.Memory/MockVirtualMemoryManager.cs +++ b/src/Ryujinx.Tests.Memory/MockVirtualMemoryManager.cs @@ -102,6 +102,11 @@ namespace Ryujinx.Tests.Memory throw new NotImplementedException(); } + public void Reprotect(ulong va, ulong size, MemoryPermission protection) + { + throw new NotImplementedException(); + } + public void TrackingReprotect(ulong va, ulong size, MemoryPermission protection) { OnProtect?.Invoke(va, size, protection);