From 8e0a421264df4eadbe35e2e63fcb81a02ae04ba4 Mon Sep 17 00:00:00 2001 From: gdkchan Date: Sat, 9 Jan 2021 20:11:31 -0300 Subject: [PATCH] Fix remap when handle is 0 (#1882) * Nvservices cleanup and attempt to fix remap * Unmap if remap handle is 0 * Remove mapped pool add from Remap --- Ryujinx.Graphics.Gpu/Memory/MemoryManager.cs | 8 +- .../Decoders/OpCodeConditional.cs | 2 +- .../NvHostAsGpu/NvHostAsGpuDeviceFile.cs | 92 +++++++++---------- .../NvHostAsGpu/Types/AddressSpaceContext.cs | 84 ++++++++--------- .../Nv/NvDrvServices/NvMap/NvMapDeviceFile.cs | 19 ++-- 5 files changed, 88 insertions(+), 117 deletions(-) diff --git a/Ryujinx.Graphics.Gpu/Memory/MemoryManager.cs b/Ryujinx.Graphics.Gpu/Memory/MemoryManager.cs index b1b1a1a8bb..cf49fd93f0 100644 --- a/Ryujinx.Graphics.Gpu/Memory/MemoryManager.cs +++ b/Ryujinx.Graphics.Gpu/Memory/MemoryManager.cs @@ -129,11 +129,11 @@ namespace Ryujinx.Graphics.Gpu.Memory } /// - /// Frees memory that was previously allocated by a map or reserved. + /// Unmaps a given range of pages at the specified GPU virtual memory region. /// - /// GPU virtual address to free - /// Size in bytes of the region being freed - public void Free(ulong va, ulong size) + /// GPU virtual address to unmap + /// Size in bytes of the region being unmapped + public void Unmap(ulong va, ulong size) { lock (_pageTable) { diff --git a/Ryujinx.Graphics.Shader/Decoders/OpCodeConditional.cs b/Ryujinx.Graphics.Shader/Decoders/OpCodeConditional.cs index f9110707f8..219eec89d5 100644 --- a/Ryujinx.Graphics.Shader/Decoders/OpCodeConditional.cs +++ b/Ryujinx.Graphics.Shader/Decoders/OpCodeConditional.cs @@ -6,7 +6,7 @@ namespace Ryujinx.Graphics.Shader.Decoders { public Condition Condition { get; } - public new static OpCode Create(InstEmitter emitter, ulong address, long opCode) => new OpCodeExit(emitter, address, opCode); + public new static OpCode Create(InstEmitter emitter, ulong address, long opCode) => new OpCodeConditional(emitter, address, opCode); public OpCodeConditional(InstEmitter emitter, ulong address, long opCode) : base(emitter, address, opCode) { diff --git a/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/NvHostAsGpuDeviceFile.cs b/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/NvHostAsGpuDeviceFile.cs index c013cefae0..ac0c4ab10d 100644 --- a/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/NvHostAsGpuDeviceFile.cs +++ b/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/NvHostAsGpuDeviceFile.cs @@ -1,5 +1,4 @@ -using Ryujinx.Common.Collections; -using Ryujinx.Common.Logging; +using Ryujinx.Common.Logging; using Ryujinx.Graphics.Gpu.Memory; using Ryujinx.HLE.HOS.Kernel.Process; using Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types; @@ -16,8 +15,8 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu private NvMemoryAllocator _memoryAllocator; public NvHostAsGpuDeviceFile(ServiceCtx context, IVirtualMemoryManager memory, long owner) : base(context, owner) - { - _memoryAllocator = context.Device.MemoryAllocator; + { + _memoryAllocator = context.Device.MemoryAllocator; } public override NvInternalResult Ioctl(NvIoctl command, Span arguments) @@ -123,7 +122,7 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu arguments.Offset = address; } - if (arguments.Offset < 0) + if (arguments.Offset == NvMemoryAllocator.PteUnmapped) { arguments.Offset = 0; @@ -153,7 +152,7 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu if (addressSpaceContext.RemoveReservation(arguments.Offset)) { _memoryAllocator.DeallocateRange(arguments.Offset, size); - addressSpaceContext.Gmm.Free(arguments.Offset, size); + addressSpaceContext.Gmm.Unmap(arguments.Offset, size); } else { @@ -178,7 +177,7 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu if (size != 0) { _memoryAllocator.DeallocateRange(arguments.Offset, size); - addressSpaceContext.Gmm.Free(arguments.Offset, size); + addressSpaceContext.Gmm.Unmap(arguments.Offset, size); } } else @@ -196,22 +195,6 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu AddressSpaceContext addressSpaceContext = GetAddressSpaceContext(Context); - NvMapHandle map = NvMapDeviceFile.GetMapFromHandle(Owner, arguments.NvMapHandle, true); - - if (map == null) - { - Logger.Warning?.Print(LogClass.ServiceNv, $"Invalid NvMap handle 0x{arguments.NvMapHandle:x8}!"); - - return NvInternalResult.InvalidInput; - } - - ulong pageSize = (ulong)arguments.PageSize; - - if (pageSize == 0) - { - pageSize = (ulong)map.Align; - } - ulong physicalAddress; if ((arguments.Flags & AddressSpaceFlags.RemapSubRange) != 0) @@ -225,15 +208,6 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu physicalAddress += arguments.BufferOffset; addressSpaceContext.Gmm.Map(physicalAddress, virtualAddress, arguments.MappingSize); - if (virtualAddress < 0) - { - string message = string.Format(mapErrorMsg, virtualAddress, arguments.MappingSize, pageSize); - - Logger.Warning?.Print(LogClass.ServiceNv, message); - - return NvInternalResult.InvalidInput; - } - return NvInternalResult.Success; } else @@ -245,6 +219,22 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu } } + NvMapHandle map = NvMapDeviceFile.GetMapFromHandle(Owner, arguments.NvMapHandle); + + if (map == null) + { + Logger.Warning?.Print(LogClass.ServiceNv, $"Invalid NvMap handle 0x{arguments.NvMapHandle:x8}!"); + + return NvInternalResult.InvalidInput; + } + + ulong pageSize = (ulong)arguments.PageSize; + + if (pageSize == 0) + { + pageSize = (ulong)map.Align; + } + physicalAddress = map.Address + arguments.BufferOffset; ulong size = arguments.MappingSize; @@ -284,12 +274,12 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu { _memoryAllocator.AllocateRange(va, size, freeAddressStartPosition); } - + addressSpaceContext.Gmm.Map(physicalAddress, va, size); arguments.Offset = va; } - if (arguments.Offset < 0) + if (arguments.Offset == NvMemoryAllocator.PteUnmapped) { arguments.Offset = 0; @@ -322,32 +312,32 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu private NvInternalResult Remap(Span arguments) { + AddressSpaceContext addressSpaceContext = GetAddressSpaceContext(Context); + for (int index = 0; index < arguments.Length; index++) { MemoryManager gmm = GetAddressSpaceContext(Context).Gmm; - NvMapHandle map = NvMapDeviceFile.GetMapFromHandle(Owner, arguments[index].NvMapHandle, true); + ulong mapOffs = (ulong)arguments[index].MapOffset << 16; + ulong gpuVa = (ulong)arguments[index].GpuOffset << 16; + ulong size = (ulong)arguments[index].Pages << 16; - if (map == null) + if (arguments[index].NvMapHandle == 0) { - Logger.Warning?.Print(LogClass.ServiceNv, $"Invalid NvMap handle 0x{arguments[index].NvMapHandle:x8}!"); - - return NvInternalResult.InvalidInput; + gmm.Unmap(gpuVa, size); } - - ulong shiftedGpuOffset = ((ulong)arguments[index].GpuOffset << 16); - - gmm.Map( - ((ulong)arguments[index].MapOffset << 16) + map.Address, - shiftedGpuOffset, - (ulong)arguments[index].Pages << 16); - - if (shiftedGpuOffset < 0) + else { - Logger.Warning?.Print(LogClass.ServiceNv, - $"Page 0x{arguments[index].GpuOffset:x16} size 0x{arguments[index].Pages:x16} not allocated!"); + NvMapHandle map = NvMapDeviceFile.GetMapFromHandle(Owner, arguments[index].NvMapHandle); - return NvInternalResult.InvalidInput; + if (map == null) + { + Logger.Warning?.Print(LogClass.ServiceNv, $"Invalid NvMap handle 0x{arguments[index].NvMapHandle:x8}!"); + + return NvInternalResult.InvalidInput; + } + + gmm.Map(mapOffs + map.Address, gpuVa, size); } } diff --git a/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/Types/AddressSpaceContext.cs b/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/Types/AddressSpaceContext.cs index b784ca514d..4e7f82ef89 100644 --- a/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/Types/AddressSpaceContext.cs +++ b/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvHostAsGpu/Types/AddressSpaceContext.cs @@ -1,6 +1,4 @@ using Ryujinx.Graphics.Gpu.Memory; -using Ryujinx.HLE.HOS.Kernel.Process; -using System; using System.Collections.Generic; namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types @@ -9,37 +7,33 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types { private class Range { - public ulong Start { get; private set; } - public ulong End { get; private set; } + public ulong Start { get; } + public ulong End { get; } - public Range(ulong position, ulong size) + public Range(ulong address, ulong size) { - Start = position; + Start = address; End = size + Start; } } private class MappedMemory : Range { - public ulong PhysicalAddress { get; private set; } - public bool VaAllocated { get; private set; } + public ulong PhysicalAddress { get; } + public bool VaAllocated { get; } - public MappedMemory( - ulong position, - ulong size, - ulong physicalAddress, - bool vaAllocated) : base(position, size) + public MappedMemory(ulong address, ulong size, ulong physicalAddress, bool vaAllocated) : base(address, size) { PhysicalAddress = physicalAddress; VaAllocated = vaAllocated; } } - private SortedList _maps; - private SortedList _reservations; - public MemoryManager Gmm { get; } + private readonly SortedList _maps; + private readonly SortedList _reservations; + public AddressSpaceContext(ServiceCtx context) { Gmm = context.Device.Gpu.MemoryManager; @@ -48,24 +42,24 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types _reservations = new SortedList(); } - public bool ValidateFixedBuffer(ulong position, ulong size, ulong alignment) + public bool ValidateFixedBuffer(ulong address, ulong size, ulong alignment) { - ulong mapEnd = position + size; + ulong mapEnd = address + size; // Check if size is valid (0 is also not allowed). - if (mapEnd <= position) + if (mapEnd <= address) { return false; } // Check if address is aligned. - if ((position & (alignment - 1)) != 0) + if ((address & (alignment - 1)) != 0) { return false; } // Check if region is reserved. - if (BinarySearch(_reservations, position) == null) + if (BinarySearch(_reservations, address) == null) { return false; } @@ -73,7 +67,7 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types // Check for overlap with already mapped buffers. Range map = BinarySearchLt(_maps, mapEnd); - if (map != null && map.End > position) + if (map != null && map.End > address) { return false; } @@ -81,20 +75,16 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types return true; } - public void AddMap( - ulong position, - ulong size, - ulong physicalAddress, - bool vaAllocated) + public void AddMap(ulong gpuVa, ulong size, ulong physicalAddress, bool vaAllocated) { - _maps.Add(position, new MappedMemory(position, size, physicalAddress, vaAllocated)); + _maps.Add(gpuVa, new MappedMemory(gpuVa, size, physicalAddress, vaAllocated)); } - public bool RemoveMap(ulong position, out ulong size) + public bool RemoveMap(ulong gpuVa, out ulong size) { size = 0; - if (_maps.Remove(position, out Range value)) + if (_maps.Remove(gpuVa, out Range value)) { MappedMemory map = (MappedMemory)value; @@ -109,36 +99,34 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types return false; } - public bool TryGetMapPhysicalAddress(ulong position, out ulong physicalAddress) + public bool TryGetMapPhysicalAddress(ulong gpuVa, out ulong physicalAddress) { - Range map = BinarySearch(_maps, position); + Range map = BinarySearch(_maps, gpuVa); if (map != null) { physicalAddress = ((MappedMemory)map).PhysicalAddress; - return true; } physicalAddress = 0; - return false; } - public void AddReservation(ulong position, ulong size) + public void AddReservation(ulong gpuVa, ulong size) { - _reservations.Add(position, new Range(position, size)); + _reservations.Add(gpuVa, new Range(gpuVa, size)); } - public bool RemoveReservation(ulong position) + public bool RemoveReservation(ulong gpuVa) { - return _reservations.Remove(position); + return _reservations.Remove(gpuVa); } - private Range BinarySearch(SortedList lst, ulong position) + private Range BinarySearch(SortedList list, ulong address) { int left = 0; - int right = lst.Count - 1; + int right = list.Count - 1; while (left <= right) { @@ -146,14 +134,14 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types int middle = left + (size >> 1); - Range rg = lst.Values[middle]; + Range rg = list.Values[middle]; - if (position >= rg.Start && position < rg.End) + if (address >= rg.Start && address < rg.End) { return rg; } - if (position < rg.Start) + if (address < rg.Start) { right = middle - 1; } @@ -166,12 +154,12 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types return null; } - private Range BinarySearchLt(SortedList lst, ulong position) + private Range BinarySearchLt(SortedList list, ulong address) { Range ltRg = null; int left = 0; - int right = lst.Count - 1; + int right = list.Count - 1; while (left <= right) { @@ -179,9 +167,9 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types int middle = left + (size >> 1); - Range rg = lst.Values[middle]; + Range rg = list.Values[middle]; - if (position < rg.Start) + if (address < rg.Start) { right = middle - 1; } @@ -189,7 +177,7 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvHostAsGpu.Types { left = middle + 1; - if (position > rg.Start) + if (address > rg.Start) { ltRg = rg; } diff --git a/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvMap/NvMapDeviceFile.cs b/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvMap/NvMapDeviceFile.cs index 8218aca8d6..7f6f6068fb 100644 --- a/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvMap/NvMapDeviceFile.cs +++ b/Ryujinx.HLE/HOS/Services/Nv/NvDrvServices/NvMap/NvMapDeviceFile.cs @@ -232,14 +232,7 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvMap private int CreateHandleFromMap(NvMapHandle map) { - IdDictionary dict = _maps.GetOrAdd(Owner, (key) => - { - IdDictionary newDict = new IdDictionary(); - - newDict.Add(0, new NvMapHandle()); - - return newDict; - }); + IdDictionary dict = _maps.GetOrAdd(Owner, (key) => new IdDictionary()); return dict.Add(map); } @@ -254,14 +247,14 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvMap return false; } - public static void IncrementMapRefCount(long pid, int handle, bool allowHandleZero = false) + public static void IncrementMapRefCount(long pid, int handle) { - GetMapFromHandle(pid, handle, allowHandleZero)?.IncrementRefCount(); + GetMapFromHandle(pid, handle)?.IncrementRefCount(); } public static bool DecrementMapRefCount(long pid, int handle) { - NvMapHandle map = GetMapFromHandle(pid, handle, false); + NvMapHandle map = GetMapFromHandle(pid, handle); if (map == null) { @@ -282,9 +275,9 @@ namespace Ryujinx.HLE.HOS.Services.Nv.NvDrvServices.NvMap } } - public static NvMapHandle GetMapFromHandle(long pid, int handle, bool allowHandleZero = false) + public static NvMapHandle GetMapFromHandle(long pid, int handle) { - if ((allowHandleZero || handle != 0) && _maps.TryGetValue(pid, out IdDictionary dict)) + if (_maps.TryGetValue(pid, out IdDictionary dict)) { return dict.GetData(handle); }