Improve IRoInterface logic (#809)

* hle: Improve IRoInterface logic

This commit contains a little rewrite of IRoInterface to fix some issues
that we were facing on some recent games (AC3 Remastered & Final Fantasy
VIII Remastered)

Related issues:

- https://github.com/Ryujinx/Ryujinx-Games-List/issues/196

* Address comments

Ryujinx.HLE/HOS/Services/Loader/ResultCode.cs

@@ -6,18 +6,5 @@
         ErrorCodeShift = 9,
 
         Success = 0,
-
-        InvalidMemoryState = (51 << ErrorCodeShift) | ModuleId,
-        InvalidNro         = (52 << ErrorCodeShift) | ModuleId,
-        InvalidNrr         = (53 << ErrorCodeShift) | ModuleId,
-        MaxNro             = (55 << ErrorCodeShift) | ModuleId,
-        MaxNrr             = (56 << ErrorCodeShift) | ModuleId,
-        NroAlreadyLoaded   = (57 << ErrorCodeShift) | ModuleId,
-        NroHashNotPresent  = (54 << ErrorCodeShift) | ModuleId,
-        UnalignedAddress   = (81 << ErrorCodeShift) | ModuleId,
-        BadSize            = (82 << ErrorCodeShift) | ModuleId,
-        BadNroAddress      = (84 << ErrorCodeShift) | ModuleId,
-        BadNrrAddress      = (85 << ErrorCodeShift) | ModuleId,
-        BadInitialization  = (87 << ErrorCodeShift) | ModuleId
     }
 }

Ryujinx.HLE/HOS/Services/Ro/IRoInterface.cs

@@ -5,19 +5,22 @@ using Ryujinx.HLE.HOS.Kernel.Memory;
 using Ryujinx.HLE.HOS.Kernel.Process;
 using Ryujinx.HLE.Loaders.Executables;
 using Ryujinx.HLE.Utilities;
+using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Security.Cryptography;
 
-namespace Ryujinx.HLE.HOS.Services.Loader
+namespace Ryujinx.HLE.HOS.Services.Ro
 {
     [Service("ldr:ro")]
     [Service("ro:1")] // 7.0.0+
-    class IRoInterface : IpcService
+    class IRoInterface : IpcService, IDisposable
     {
-        private const int MaxNrr = 0x40;
+        private const int MaxNrr         = 0x40;
-        private const int MaxNro = 0x40;
+        private const int MaxNro         = 0x40;
+        private const int MaxMapRetries  = 0x200;
+        private const int GuardPagesSize = 0x4000;
 
         private const uint NrrMagic = 0x3052524E;
         private const uint NroMagic = 0x304F524E;
@@ -25,12 +28,15 @@ namespace Ryujinx.HLE.HOS.Services.Loader
         private List<NrrInfo> _nrrInfos;
         private List<NroInfo> _nroInfos;
 
-        private bool _isInitialized;
+        private KProcess _owner;
+
+        private static Random _random = new Random();
 
         public IRoInterface(ServiceCtx context)
         {
             _nrrInfos = new List<NrrInfo>(MaxNrr);
             _nroInfos = new List<NroInfo>(MaxNro);
+            _owner    = null;
         }
 
         private ResultCode ParseNrr(out NrrInfo nrrInfo, ServiceCtx context, long nrrAddress, long nrrSize)
@@ -39,11 +45,11 @@ namespace Ryujinx.HLE.HOS.Services.Loader
 
             if (nrrSize == 0 || nrrAddress + nrrSize <= nrrAddress || (nrrSize & 0xFFF) != 0)
             {
-                return ResultCode.BadSize;
+                return ResultCode.InvalidSize;
             }
             else if ((nrrAddress & 0xFFF) != 0)
             {
-                return ResultCode.UnalignedAddress;
+                return ResultCode.InvalidAddress;
             }
 
             StructReader reader = new StructReader(context.Memory, nrrAddress);
@@ -55,7 +61,7 @@ namespace Ryujinx.HLE.HOS.Services.Loader
             }
             else if (header.NrrSize != nrrSize)
             {
-                return ResultCode.BadSize;
+                return ResultCode.InvalidSize;
             }
 
             List<byte[]> hashes = new List<byte[]>();
@@ -105,19 +111,19 @@ namespace Ryujinx.HLE.HOS.Services.Loader
 
             if (_nroInfos.Count >= MaxNro)
             {
-                return ResultCode.MaxNro;
+                return ResultCode.TooManyNro;
             }
             else if (nroSize == 0 || nroAddress + nroSize <= nroAddress || (nroSize & 0xFFF) != 0)
             {
-                return ResultCode.BadSize;
+                return ResultCode.InvalidSize;
             }
             else if (bssSize != 0 && bssAddress + bssSize <= bssAddress)
             {
-                return ResultCode.BadSize;
+                return ResultCode.InvalidSize;
             }
             else if ((nroAddress & 0xFFF) != 0)
             {
-                return ResultCode.UnalignedAddress;
+                return ResultCode.InvalidAddress;
             }
 
             uint magic       = context.Memory.ReadUInt32((long)nroAddress + 0x10);
@@ -140,12 +146,12 @@ namespace Ryujinx.HLE.HOS.Services.Loader
 
             if (!IsNroHashPresent(nroHash))
             {
-                return ResultCode.NroHashNotPresent;
+                return ResultCode.NotRegistered;
             }
 
             if (IsNroLoaded(nroHash))
             {
-                return ResultCode.NroAlreadyLoaded;
+                return ResultCode.AlreadyLoaded;
             }
 
             stream.Position = 0;
@@ -187,77 +193,120 @@ namespace Ryujinx.HLE.HOS.Services.Loader
             return ResultCode.Success;
         }
 
-        private ResultCode MapNro(ServiceCtx context, NroInfo info, out ulong nroMappedAddress)
+        private ResultCode MapNro(KProcess process, NroInfo info, out ulong nroMappedAddress)
         {
+            KMemoryManager memMgr = process.MemoryManager;
+
+            int retryCount = 0;
+
             nroMappedAddress = 0;
 
-            KMemoryManager memMgr = context.Process.MemoryManager;
+            while (retryCount++ < MaxMapRetries)
-
-            ulong targetAddress = memMgr.GetAddrSpaceBaseAddr();
-
-            while (true)
             {
-                if (targetAddress + info.TotalSize >= memMgr.AddrSpaceEnd)
+                ResultCode result = MapCodeMemoryInProcess(process, info.NroAddress, info.NroSize, out nroMappedAddress);
+
+                if (result != ResultCode.Success)
                 {
-                    return ResultCode.InvalidMemoryState;
+                    return result;
                 }
 
-                KMemoryInfo memInfo = memMgr.QueryMemory(targetAddress);
+                if (info.BssSize > 0)
-
-                if (memInfo.State == MemoryState.Unmapped && memInfo.Size >= info.TotalSize)
                 {
-                    if (!memMgr.InsideHeapRegion (targetAddress, info.TotalSize) &&
+                    KernelResult bssMappingResult = memMgr.MapProcessCodeMemory(nroMappedAddress + info.NroSize, info.BssAddress, info.BssSize);
-                        !memMgr.InsideAliasRegion(targetAddress, info.TotalSize))
+
+                    if (bssMappingResult == KernelResult.InvalidMemState)
+                    {
+                        memMgr.UnmapProcessCodeMemory(nroMappedAddress + info.NroSize, info.BssAddress, info.BssSize);
+                        memMgr.UnmapProcessCodeMemory(nroMappedAddress, info.NroAddress, info.NroSize);
+
+                        continue;
+                    }
+                    else if (bssMappingResult != KernelResult.Success)
+                    {
+                        memMgr.UnmapProcessCodeMemory(nroMappedAddress + info.NroSize, info.BssAddress, info.BssSize);
+                        memMgr.UnmapProcessCodeMemory(nroMappedAddress, info.NroAddress, info.NroSize);
+
+                        return (ResultCode)bssMappingResult;
+                    }
+                }
+
+                if (CanAddGuardRegionsInProcess(process, nroMappedAddress, info.TotalSize))
+                {
+                    return ResultCode.Success;
+                }
+            }
+
+            return ResultCode.InsufficientAddressSpace;
+        }
+
+        private bool CanAddGuardRegionsInProcess(KProcess process, ulong baseAddress, ulong size)
+        {
+            KMemoryManager memMgr = process.MemoryManager;
+
+            KMemoryInfo memInfo = memMgr.QueryMemory(baseAddress - 1);
+
+            if (memInfo.State == MemoryState.Unmapped && baseAddress - GuardPagesSize >= memInfo.Address)
+            {
+                memInfo = memMgr.QueryMemory(baseAddress + size);
+
+                if (memInfo.State == MemoryState.Unmapped)
+                {
+                    return baseAddress + size + GuardPagesSize <= memInfo.Address + memInfo.Size;
+                }
+            }
+            return false;
+        }
+
+        private ResultCode MapCodeMemoryInProcess(KProcess process, ulong baseAddress, ulong size, out ulong targetAddress)
+        {
+            KMemoryManager memMgr = process.MemoryManager;
+
+            targetAddress = 0;
+
+            int retryCount;
+
+            int addressSpacePageLimit = (int)((memMgr.GetAddrSpaceSize() - size) >> 12);
+
+            for (retryCount = 0; retryCount < MaxMapRetries; retryCount++)
+            {
+                while (true)
+                {
+                    targetAddress = memMgr.GetAddrSpaceBaseAddr() + (ulong)(_random.Next(addressSpacePageLimit) << 12);
+
+                    if (memMgr.InsideAddrSpace(targetAddress, size) && !memMgr.InsideHeapRegion(targetAddress, size) && !memMgr.InsideAliasRegion(targetAddress, size))
                     {
                         break;
                     }
                 }
 
-                targetAddress += memInfo.Size;
+                KernelResult result = memMgr.MapProcessCodeMemory(targetAddress, baseAddress, size);
-            }
 
-            KernelResult result = memMgr.MapProcessCodeMemory(targetAddress, info.NroAddress, info.NroSize);
+                if (result == KernelResult.InvalidMemState)
-
-            if (result != KernelResult.Success)
-            {
-                return ResultCode.InvalidMemoryState;
-            }
-
-            ulong bssTargetAddress = targetAddress + info.NroSize;
-
-            if (info.BssSize != 0)
-            {
-                result = memMgr.MapProcessCodeMemory(bssTargetAddress, info.BssAddress, info.BssSize);
-
-                if (result != KernelResult.Success)
                 {
-                    memMgr.UnmapProcessCodeMemory(targetAddress, info.NroAddress, info.NroSize);
+                    continue;
-
-                    return ResultCode.InvalidMemoryState;
                 }
-            }
+                else if (result != KernelResult.Success)
-
-            result = LoadNroIntoMemory(context.Process, info.Executable, targetAddress);
-
-            if (result != KernelResult.Success)
-            {
-                memMgr.UnmapProcessCodeMemory(targetAddress, info.NroAddress, info.NroSize);
-
-                if (info.BssSize != 0)
                 {
-                    memMgr.UnmapProcessCodeMemory(bssTargetAddress, info.BssAddress, info.BssSize);
+                    return (ResultCode)result;
+                }
+
+                if (!CanAddGuardRegionsInProcess(process, targetAddress, size))
+                {
+                    continue;
                 }
 
                 return ResultCode.Success;
             }
 
-            info.NroMappedAddress = targetAddress;
+            if (retryCount == MaxMapRetries)
-            nroMappedAddress      = targetAddress;
+            {
+                return ResultCode.InsufficientAddressSpace;
+            }
 
             return ResultCode.Success;
         }
 
-        private KernelResult LoadNroIntoMemory(KProcess process, IExecutable relocatableObject, ulong baseAddress)
+        private KernelResult SetNroMemoryPermissions(KProcess process, IExecutable relocatableObject, ulong baseAddress)
         {
             ulong textStart = baseAddress + (ulong)relocatableObject.TextOffset;
             ulong roStart   = baseAddress + (ulong)relocatableObject.RoOffset;
@@ -304,10 +353,10 @@ namespace Ryujinx.HLE.HOS.Services.Loader
                 }
             }
 
-            return ResultCode.BadNrrAddress;
+            return ResultCode.NotLoaded;
         }
 
-        private ResultCode RemoveNroInfo(ServiceCtx context, ulong nroMappedAddress)
+        private ResultCode RemoveNroInfo(ulong nroMappedAddress)
         {
             foreach (NroInfo info in _nroInfos)
             {
@@ -315,49 +364,64 @@ namespace Ryujinx.HLE.HOS.Services.Loader
                 {
                     _nroInfos.Remove(info);
 
-                    ulong textSize = (ulong)info.Executable.Text.Length;
+                    return UnmapNroFromInfo(info);
-                    ulong roSize   = (ulong)info.Executable.Ro.Length;
-                    ulong dataSize = (ulong)info.Executable.Data.Length;
-                    ulong bssSize  = (ulong)info.Executable.BssSize;
-
-                    KernelResult result = KernelResult.Success;
-
-                    if (info.Executable.BssSize != 0)
-                    {
-                        result = context.Process.MemoryManager.UnmapProcessCodeMemory(
-                            info.NroMappedAddress + textSize + roSize + dataSize,
-                            info.Executable.BssAddress,
-                            bssSize);
-                    }
-
-                    if (result == KernelResult.Success)
-                    {
-                        result = context.Process.MemoryManager.UnmapProcessCodeMemory(
-                            info.NroMappedAddress         + textSize + roSize,
-                            info.Executable.SourceAddress + textSize + roSize,
-                            dataSize);
-
-                        if (result == KernelResult.Success)
-                        {
-                            result = context.Process.MemoryManager.UnmapProcessCodeMemory(
-                                info.NroMappedAddress,
-                                info.Executable.SourceAddress,
-                                textSize + roSize);
-                        }
-                    }
-
-                    return (ResultCode)result;
                 }
             }
 
-            return ResultCode.BadNroAddress;
+            return ResultCode.NotLoaded;
+        }
+
+        private ResultCode UnmapNroFromInfo(NroInfo info)
+        {
+            ulong textSize = (ulong)info.Executable.Text.Length;
+            ulong roSize   = (ulong)info.Executable.Ro.Length;
+            ulong dataSize = (ulong)info.Executable.Data.Length;
+            ulong bssSize  = (ulong)info.Executable.BssSize;
+
+            KernelResult result = KernelResult.Success;
+
+            if (info.Executable.BssSize != 0)
+            {
+                result = _owner.MemoryManager.UnmapProcessCodeMemory(
+                    info.NroMappedAddress + textSize + roSize + dataSize,
+                    info.Executable.BssAddress,
+                    bssSize);
+            }
+
+            if (result == KernelResult.Success)
+            {
+                result = _owner.MemoryManager.UnmapProcessCodeMemory(
+                    info.NroMappedAddress + textSize + roSize,
+                    info.Executable.SourceAddress + textSize + roSize,
+                    dataSize);
+
+                if (result == KernelResult.Success)
+                {
+                    result = _owner.MemoryManager.UnmapProcessCodeMemory(
+                        info.NroMappedAddress,
+                        info.Executable.SourceAddress,
+                        textSize + roSize);
+                }
+            }
+
+            return (ResultCode)result;
+        }
+
+        private ResultCode IsInitialized(KProcess process)
+        {
+            if (_owner != null && _owner.Pid == process.Pid)
+            {
+                return ResultCode.Success;
+            }
+
+            return ResultCode.InvalidProcess;
         }
 
         [Command(0)]
         // LoadNro(u64, u64, u64, u64, u64, pid) -> u64
         public ResultCode LoadNro(ServiceCtx context)
         {
-            ResultCode result = ResultCode.BadInitialization;
+            ResultCode result = IsInitialized(context.Process);
 
             // Zero
             context.RequestData.ReadUInt64();
@@ -369,19 +433,24 @@ namespace Ryujinx.HLE.HOS.Services.Loader
 
             ulong nroMappedAddress = 0;
 
-            if (_isInitialized)
+            if (result == ResultCode.Success)
             {
                 NroInfo info;
 
                 result = ParseNro(out info, context, nroHeapAddress, nroSize, bssHeapAddress, bssSize);
 
-                if (result == 0)
+                if (result == ResultCode.Success)
                 {
-                    result = MapNro(context, info, out nroMappedAddress);
+                    result = MapNro(context.Process, info, out nroMappedAddress);
 
-                    if (result == 0)
+                    if (result == ResultCode.Success)
                     {
-                        _nroInfos.Add(info);
+                        result = (ResultCode)SetNroMemoryPermissions(context.Process, info.Executable, nroMappedAddress);
+
+                        if (result == ResultCode.Success)
+                        {
+                            _nroInfos.Add(info);
+                        }
                     }
                 }
             }
@@ -395,21 +464,21 @@ namespace Ryujinx.HLE.HOS.Services.Loader
         // UnloadNro(u64, u64, pid)
         public ResultCode UnloadNro(ServiceCtx context)
         {
-            ResultCode result = ResultCode.BadInitialization;
+            ResultCode result = IsInitialized(context.Process);
 
             // Zero
             context.RequestData.ReadUInt64();
 
             ulong nroMappedAddress = context.RequestData.ReadUInt64();
 
-            if (_isInitialized)
+            if (result == ResultCode.Success)
             {
                 if ((nroMappedAddress & 0xFFF) != 0)
                 {
-                    return ResultCode.UnalignedAddress;
+                    return ResultCode.InvalidAddress;
                 }
 
-                result = RemoveNroInfo(context, nroMappedAddress);
+                result = RemoveNroInfo(nroMappedAddress);
             }
 
             return result;
@@ -419,24 +488,24 @@ namespace Ryujinx.HLE.HOS.Services.Loader
         // LoadNrr(u64, u64, u64, pid)
         public ResultCode LoadNrr(ServiceCtx context)
         {
-            ResultCode result = ResultCode.BadInitialization;
+            ResultCode result = IsInitialized(context.Process);
 
-            // Zero
+            // pid placeholder, zero
             context.RequestData.ReadUInt64();
 
             long nrrAddress = context.RequestData.ReadInt64();
             long nrrSize    = context.RequestData.ReadInt64();
 
-            if (_isInitialized)
+            if (result == ResultCode.Success)
             {
                 NrrInfo info;
                 result = ParseNrr(out info, context, nrrAddress, nrrSize);
 
-                if (result == 0)
+                if (result == ResultCode.Success)
                 {
                     if (_nrrInfos.Count >= MaxNrr)
                     {
-                        result = ResultCode.MaxNrr;
+                        result = ResultCode.NotLoaded;
                     }
                     else
                     {
@@ -452,18 +521,18 @@ namespace Ryujinx.HLE.HOS.Services.Loader
         // UnloadNrr(u64, u64, pid)
         public ResultCode UnloadNrr(ServiceCtx context)
         {
-            ResultCode result = ResultCode.BadInitialization;
+            ResultCode result = IsInitialized(context.Process);
 
-            // Zero
+            // pid placeholder, zero
             context.RequestData.ReadUInt64();
 
             long nrrHeapAddress = context.RequestData.ReadInt64();
 
-            if (_isInitialized)
+            if (result == ResultCode.Success)
             {
                 if ((nrrHeapAddress & 0xFFF) != 0)
                 {
-                    return ResultCode.UnalignedAddress;
+                    return ResultCode.InvalidAddress;
                 }
 
                 result = RemoveNrrInfo(nrrHeapAddress);
@@ -476,10 +545,24 @@ namespace Ryujinx.HLE.HOS.Services.Loader
         // Initialize(u64, pid, KObject)
         public ResultCode Initialize(ServiceCtx context)
         {
-            // TODO: we actually ignore the pid and process handle receive, we will need to use them when we will have multi process support.
+            if (_owner != null)
-            _isInitialized = true;
+            {
+                return ResultCode.InvalidSession;
+            }
+
+            _owner = context.Process;
 
             return ResultCode.Success;
         }
+
+        public void Dispose()
+        {
+            foreach (NroInfo info in _nroInfos)
+            {
+                UnmapNroFromInfo(info);
+            }
+
+            _nroInfos.Clear();
+        }
     }
 }

Ryujinx.HLE/HOS/Services/Ro/ResultCode.cs

@@ -0,0 +1,27 @@
+namespace Ryujinx.HLE.HOS.Services.Ro
+{
+    enum ResultCode
+    {
+        ModuleId       = 22,
+        ErrorCodeShift = 22,
+
+        Success = 0,
+
+        InsufficientAddressSpace = (2 << ErrorCodeShift) | ModuleId,
+        AlreadyLoaded            = (3 << ErrorCodeShift) | ModuleId,
+        InvalidNro               = (4 << ErrorCodeShift) | ModuleId,
+        InvalidNrr               = (6 << ErrorCodeShift) | ModuleId,
+        TooManyNro               = (7 << ErrorCodeShift) | ModuleId,
+        TooManyNrr               = (8 << ErrorCodeShift) | ModuleId,
+        NotAuthorized            = (9 << ErrorCodeShift) | ModuleId,
+
+        InvalidNrrType           = (10 << ErrorCodeShift) | ModuleId,
+
+        InvalidAddress           = (1025 << ErrorCodeShift) | ModuleId,
+        InvalidSize              = (1026 << ErrorCodeShift) | ModuleId,
+        NotLoaded                = (1028 << ErrorCodeShift) | ModuleId,
+        NotRegistered            = (1029 << ErrorCodeShift) | ModuleId,
+        InvalidSession           = (1030 << ErrorCodeShift) | ModuleId,
+        InvalidProcess           = (1031 << ErrorCodeShift) | ModuleId,
+    }
+}

Ryujinx.HLE/HOS/Services/Ro/Types/NroInfo.cs

@@ -1,6 +1,6 @@
 using Ryujinx.HLE.Loaders.Executables;
 
-namespace Ryujinx.HLE.HOS.Services.Loader
+namespace Ryujinx.HLE.HOS.Services.Ro
 {
     class NroInfo
     {

Ryujinx.HLE/HOS/Services/Ro/Types/NrrHeader.cs

@@ -1,6 +1,6 @@
 using System.Runtime.InteropServices;
 
-namespace Ryujinx.HLE.HOS.Services.Loader
+namespace Ryujinx.HLE.HOS.Services.Ro
 {
     [StructLayout(LayoutKind.Explicit, Size = 0x350)]
     unsafe struct NrrHeader

Ryujinx.HLE/HOS/Services/Ro/Types/NrrInfo.cs

@@ -1,6 +1,6 @@
 using System.Collections.Generic;
 
-namespace Ryujinx.HLE.HOS.Services.Loader
+namespace Ryujinx.HLE.HOS.Services.Ro
 {
     class NrrInfo
     {