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

@@ -129,11 +129,11 @@ namespace Ryujinx.Graphics.Gpu.Memory
         }
 
         /// <summary>
-        /// Frees memory that was previously allocated by a map or reserved.
+        /// Unmaps a given range of pages at the specified GPU virtual memory region.
         /// </summary>
-        /// <param name="va">GPU virtual address to free</param>
-        /// <param name="size">Size in bytes of the region being freed</param>
-        public void Free(ulong va, ulong size)
+        /// <param name="va">GPU virtual address to unmap</param>
+        /// <param name="size">Size in bytes of the region being unmapped</param>
+        public void Unmap(ulong va, ulong size)
         {
             lock (_pageTable)
             {

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)
         {

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<byte> 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<RemapArguments> 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);
                 }
+                else
+                {
+                    NvMapHandle map = NvMapDeviceFile.GetMapFromHandle(Owner, arguments[index].NvMapHandle);
 
-                ulong shiftedGpuOffset = ((ulong)arguments[index].GpuOffset << 16);
-
-                gmm.Map(
-                    ((ulong)arguments[index].MapOffset << 16) + map.Address,
-                     shiftedGpuOffset,
-                     (ulong)arguments[index].Pages     << 16);
+                    if (map == null)
+                    {
+                        Logger.Warning?.Print(LogClass.ServiceNv, $"Invalid NvMap handle 0x{arguments[index].NvMapHandle:x8}!");
 
-                if (shiftedGpuOffset < 0)
-                {
-                    Logger.Warning?.Print(LogClass.ServiceNv,
-                        $"Page 0x{arguments[index].GpuOffset:x16} size 0x{arguments[index].Pages:x16} not allocated!");
+                        return NvInternalResult.InvalidInput;
+                    }
 
-                    return NvInternalResult.InvalidInput;
+                    gmm.Map(mapOffs + map.Address, gpuVa, size);
                 }
             }
 

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 MappedMemory(
-                ulong position,
-                ulong size,
-                ulong physicalAddress,
-                bool vaAllocated) : base(position, size)
+            public ulong PhysicalAddress { get; }
+            public bool  VaAllocated     { get; }
+
+            public MappedMemory(ulong address, ulong size, ulong physicalAddress, bool vaAllocated) : base(address, size)
             {
                 PhysicalAddress = physicalAddress;
                 VaAllocated     = vaAllocated;
             }
         }
 
-        private SortedList<ulong, Range> _maps;
-        private SortedList<ulong, Range> _reservations;
-
         public MemoryManager Gmm { get; }
 
+        private readonly SortedList<ulong, Range> _maps;
+        private readonly SortedList<ulong, Range> _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<ulong, Range>();
         }
 
-        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<ulong, Range> lst, ulong position)
+        private Range BinarySearch(SortedList<ulong, Range> 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<ulong, Range> lst, ulong position)
+        private Range BinarySearchLt(SortedList<ulong, Range> 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;
                     }

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<NvMapHandle>(handle);
             }