Core: Memory Fixes (#2872)

* Fix VirtualQuery behavior on low addresses.

* Fix VirtualQuery struct

Somewhere in our BitField and array use, the size of our VirtualQuery struct became larger than the struct used on real hardware.
Fixing this fixes some data corruption visible in the name parameter during my tests.

* Default name to anon

On real hardware, nameless mappings are given the name "anon:address" where address appears to be the address that made the memory call.
For simplicity sake, I'll stick to the name "anon" for now.

* Place an upper bound on returns from SearchFree

Right now, this upper bound is set based on the limitations of our GPU buffer cache and page table.
Someone with more experience in that area of code should probably fix that at some point.

* More anons

* Clang

* Fix name in sceKernelMapNamedDirectMemory

* strncpy instead of strcpy

Hardcoded the constant size for now, I need to review how real hardware behaves here to determine if anything else is necessary for this to be accurate.

* Fix name behavior

All memory naming functions restrict the name size to a 31 character limit, and return `ORBIS_KERNEL_ERROR_ENAMETOOLONG` if that limit is exceeded.
Since this value is constant for all functions involving names, I've defined it as a constant in kernel's memory.h, and used that in place of any hardcoded 32 character limits.

* Error logging

Hopefully this helps in catching the UFC regression?

* Increase address space upper bound

Probably needs heavy testing, especially on Mac/Windows.
This increases the address space, as needed to accommodate strange memory behaviors seen in UFC.

* VirtualQuery fix

Due to limitations of certain platforms, we initialize our vma_map with 3 separate free mappings.
As such, we need to use a while loop here to accurately query mappings with high addresses

* Fix mappings to high addresses

The PS4's GPU can only handle 40bit addresses. Our texture cache and buffer cache were designed around these limits, and mapping to higher addresses would cause segmentation faults and access violations.
To fix these crashes, only map to the GPU if the mapping is fully contained within the address space the GPU should access.
I'm open to suggestions on how to make this cleaner

* Revert "Increase address space upper bound"

This reverts commit 3d50eeeebb.

* Revert VirtualQuery while loop

Windows wasn't happy with this, again.
Will try to debug and properly fix this when I have a good chance.

* Fix asserts

FindVMA, due to the way it's programmed, never actually returns vma_map.end(), the furthest it ever returns is the last valid memory area. All those asserts we involving vma_map.end() never actually trigger due to this.
This commit removes redundant asserts, adds messages to asserts that were lacking them, and fixes all asserts designed to detect out of bounds memory accesses so they actually trigger.
I've also fixed some potential memory safety issues.

* Proper error behavior in QueryProtection

Might as well handle this properly while I'm here.

* Clang

* More information about ReserveVirtualRange results

Should help debug issues like the one in The Order: 1886 (CUSA00076)

* Fix assert message

* Update assert message

Extra space

* Fix my bug

Oh hey, finally something that's my fault.

* Fix rasterizer unmaps

Should use adjusted_size here, otherwise we could unmap too much.
Thanks to diegolix29 for spotting this.

* Fix edge case in MapMemory

Code comments explain everything.
This should fix some memory asserts.

* Fix fix

Avoid running the code path if it's unnecessary, since there are many additional edge cases to handle when the VMA map is small.

* Fix fix fix

Should prevent infinite loops, haven't tested properly yet though.

* Split logging for inputs and out_addr in ReserveVirtualRange

Addresses review comments.

src/core/memory.cpp

@@ -75,7 +75,8 @@ u64 MemoryManager::ClampRangeSize(VAddr virtual_addr, u64 size) {
 
     // Clamp size to the remaining size of the current VMA.
     auto vma = FindVMA(virtual_addr);
-    ASSERT_MSG(vma != vma_map.end(), "Attempted to access invalid GPU address {:#x}", virtual_addr);
+    ASSERT_MSG(vma->second.Contains(virtual_addr, 0),
+               "Attempted to access invalid GPU address {:#x}", virtual_addr);
     u64 clamped_size = vma->second.base + vma->second.size - virtual_addr;
     ++vma;
 
@@ -96,6 +97,8 @@ u64 MemoryManager::ClampRangeSize(VAddr virtual_addr, u64 size) {
 bool MemoryManager::TryWriteBacking(void* address, const void* data, u32 num_bytes) {
     const VAddr virtual_addr = std::bit_cast<VAddr>(address);
     const auto& vma = FindVMA(virtual_addr)->second;
+    ASSERT_MSG(vma.Contains(virtual_addr, 0),
+               "Attempting to access out of bounds memory at address {:#x}", virtual_addr);
     if (vma.type != VMAType::Direct) {
         return false;
     }
@@ -145,10 +148,12 @@ PAddr MemoryManager::Allocate(PAddr search_start, PAddr search_end, size_t size,
     auto mapping_end = mapping_start + size;
 
     // Find the first free, large enough dmem area in the range.
-    while ((!dmem_area->second.is_free || dmem_area->second.GetEnd() < mapping_end) &&
-           dmem_area != dmem_map.end()) {
+    while (!dmem_area->second.is_free || dmem_area->second.GetEnd() < mapping_end) {
         // The current dmem_area isn't suitable, move to the next one.
         dmem_area++;
+        if (dmem_area == dmem_map.end()) {
+            break;
+        }
 
         // Update local variables based on the new dmem_area
         mapping_start = Common::AlignUp(dmem_area->second.base, alignment);
@@ -172,7 +177,6 @@ void MemoryManager::Free(PAddr phys_addr, size_t size) {
     std::scoped_lock lk{mutex};
 
     auto dmem_area = CarveDmemArea(phys_addr, size);
-    ASSERT(dmem_area != dmem_map.end() && dmem_area->second.size >= size);
 
     // Release any dmem mappings that reference this physical block.
     std::vector<std::pair<VAddr, u64>> remove_list;
@@ -216,12 +220,18 @@ int MemoryManager::PoolReserve(void** out_addr, VAddr virtual_addr, size_t size,
             vma = FindVMA(mapped_addr)->second;
         }
         const size_t remaining_size = vma.base + vma.size - mapped_addr;
-        ASSERT_MSG(vma.type == VMAType::Free && remaining_size >= size);
+        ASSERT_MSG(vma.type == VMAType::Free && remaining_size >= size,
+                   "Memory region {:#x} to {:#x} is not large enough to reserve {:#x} to {:#x}",
+                   vma.base, vma.base + vma.size, virtual_addr, virtual_addr + size);
     }
 
     // Find the first free area starting with provided virtual address.
     if (False(flags & MemoryMapFlags::Fixed)) {
         mapped_addr = SearchFree(mapped_addr, size, alignment);
+        if (mapped_addr == -1) {
+            // No suitable memory areas to map to
+            return ORBIS_KERNEL_ERROR_ENOMEM;
+        }
     }
 
     // Add virtual memory area
@@ -229,7 +239,7 @@ int MemoryManager::PoolReserve(void** out_addr, VAddr virtual_addr, size_t size,
     auto& new_vma = new_vma_handle->second;
     new_vma.disallow_merge = True(flags & MemoryMapFlags::NoCoalesce);
     new_vma.prot = MemoryProt::NoAccess;
-    new_vma.name = "";
+    new_vma.name = "anon";
     new_vma.type = VMAType::PoolReserved;
     MergeAdjacent(vma_map, new_vma_handle);
 
@@ -247,19 +257,25 @@ int MemoryManager::Reserve(void** out_addr, VAddr virtual_addr, size_t size, Mem
 
     // Fixed mapping means the virtual address must exactly match the provided one.
     if (True(flags & MemoryMapFlags::Fixed)) {
-        auto& vma = FindVMA(mapped_addr)->second;
+        auto vma = FindVMA(mapped_addr)->second;
         // If the VMA is mapped, unmap the region first.
         if (vma.IsMapped()) {
             UnmapMemoryImpl(mapped_addr, size);
             vma = FindVMA(mapped_addr)->second;
         }
         const size_t remaining_size = vma.base + vma.size - mapped_addr;
-        ASSERT_MSG(vma.type == VMAType::Free && remaining_size >= size);
+        ASSERT_MSG(vma.type == VMAType::Free && remaining_size >= size,
+                   "Memory region {:#x} to {:#x} is not large enough to reserve {:#x} to {:#x}",
+                   vma.base, vma.base + vma.size, virtual_addr, virtual_addr + size);
     }
 
     // Find the first free area starting with provided virtual address.
     if (False(flags & MemoryMapFlags::Fixed)) {
         mapped_addr = SearchFree(mapped_addr, size, alignment);
+        if (mapped_addr == -1) {
+            // No suitable memory areas to map to
+            return ORBIS_KERNEL_ERROR_ENOMEM;
+        }
     }
 
     // Add virtual memory area
@@ -267,7 +283,7 @@ int MemoryManager::Reserve(void** out_addr, VAddr virtual_addr, size_t size, Mem
     auto& new_vma = new_vma_handle->second;
     new_vma.disallow_merge = True(flags & MemoryMapFlags::NoCoalesce);
     new_vma.prot = MemoryProt::NoAccess;
-    new_vma.name = "";
+    new_vma.name = "anon";
     new_vma.type = VMAType::Reserved;
     MergeAdjacent(vma_map, new_vma_handle);
 
@@ -288,7 +304,9 @@ int MemoryManager::PoolCommit(VAddr virtual_addr, size_t size, MemoryProt prot)
     // This should return SCE_KERNEL_ERROR_ENOMEM but shouldn't normally happen.
     const auto& vma = FindVMA(mapped_addr)->second;
     const size_t remaining_size = vma.base + vma.size - mapped_addr;
-    ASSERT_MSG(!vma.IsMapped() && remaining_size >= size);
+    ASSERT_MSG(!vma.IsMapped() && remaining_size >= size,
+               "Memory region {:#x} to {:#x} isn't free enough to map region {:#x} to {:#x}",
+               vma.base, vma.base + vma.size, virtual_addr, virtual_addr + size);
 
     // Perform the mapping.
     void* out_addr = impl.Map(mapped_addr, size, alignment, -1, false);
@@ -302,7 +320,10 @@ int MemoryManager::PoolCommit(VAddr virtual_addr, size_t size, MemoryProt prot)
     new_vma.is_exec = false;
     new_vma.phys_base = 0;
 
-    rasterizer->MapMemory(mapped_addr, size);
+    if (IsValidGpuMapping(mapped_addr, size)) {
+        rasterizer->MapMemory(mapped_addr, size);
+    }
+
     return ORBIS_OK;
 }
 
@@ -325,15 +346,34 @@ int MemoryManager::MapMemory(void** out_addr, VAddr virtual_addr, size_t size, M
 
     // Fixed mapping means the virtual address must exactly match the provided one.
     if (True(flags & MemoryMapFlags::Fixed)) {
-        // This should return SCE_KERNEL_ERROR_ENOMEM but shouldn't normally happen.
-        const auto& vma = FindVMA(mapped_addr)->second;
-        const size_t remaining_size = vma.base + vma.size - mapped_addr;
-        ASSERT_MSG(!vma.IsMapped() && remaining_size >= size);
+        auto vma = FindVMA(mapped_addr)->second;
+        size_t remaining_size = vma.base + vma.size - mapped_addr;
+        // There's a possible edge case where we're mapping to a partially reserved range.
+        // To account for this, unmap any reserved areas within this mapping range first.
+        auto unmap_addr = mapped_addr;
+        auto unmap_size = size;
+        while (!vma.IsMapped() && unmap_addr < mapped_addr + size && remaining_size < size) {
+            auto unmapped = UnmapBytesFromEntry(unmap_addr, vma, unmap_size);
+            unmap_addr += unmapped;
+            unmap_size -= unmapped;
+            vma = FindVMA(unmap_addr)->second;
+        }
+
+        // This should return SCE_KERNEL_ERROR_ENOMEM but rarely happens.
+        vma = FindVMA(mapped_addr)->second;
+        remaining_size = vma.base + vma.size - mapped_addr;
+        ASSERT_MSG(!vma.IsMapped() && remaining_size >= size,
+                   "Memory region {:#x} to {:#x} isn't free enough to map region {:#x} to {:#x}",
+                   vma.base, vma.base + vma.size, virtual_addr, virtual_addr + size);
     }
 
     // Find the first free area starting with provided virtual address.
     if (False(flags & MemoryMapFlags::Fixed)) {
         mapped_addr = SearchFree(mapped_addr, size, alignment);
+        if (mapped_addr == -1) {
+            // No suitable memory areas to map to
+            return ORBIS_KERNEL_ERROR_ENOMEM;
+        }
     }
 
     // Perform the mapping.
@@ -353,7 +393,10 @@ int MemoryManager::MapMemory(void** out_addr, VAddr virtual_addr, size_t size, M
     if (type == VMAType::Flexible) {
         flexible_usage += size;
     }
-    rasterizer->MapMemory(mapped_addr, size);
+
+    if (IsValidGpuMapping(mapped_addr, size)) {
+        rasterizer->MapMemory(mapped_addr, size);
+    }
 
     return ORBIS_OK;
 }
@@ -366,12 +409,18 @@ int MemoryManager::MapFile(void** out_addr, VAddr virtual_addr, size_t size, Mem
     // Find first free area to map the file.
     if (False(flags & MemoryMapFlags::Fixed)) {
         mapped_addr = SearchFree(mapped_addr, size_aligned, 1);
+        if (mapped_addr == -1) {
+            // No suitable memory areas to map to
+            return ORBIS_KERNEL_ERROR_ENOMEM;
+        }
     }
 
     if (True(flags & MemoryMapFlags::Fixed)) {
         const auto& vma = FindVMA(virtual_addr)->second;
         const size_t remaining_size = vma.base + vma.size - virtual_addr;
-        ASSERT_MSG(!vma.IsMapped() && remaining_size >= size);
+        ASSERT_MSG(!vma.IsMapped() && remaining_size >= size,
+                   "Memory region {:#x} to {:#x} isn't free enough to map region {:#x} to {:#x}",
+                   vma.base, vma.base + vma.size, virtual_addr, virtual_addr + size);
     }
 
     // Map the file.
@@ -404,7 +453,9 @@ void MemoryManager::PoolDecommit(VAddr virtual_addr, size_t size) {
     const auto start_in_vma = virtual_addr - vma_base_addr;
     const auto type = vma_base.type;
 
-    rasterizer->UnmapMemory(virtual_addr, size);
+    if (IsValidGpuMapping(virtual_addr, size)) {
+        rasterizer->UnmapMemory(virtual_addr, size);
+    }
 
     // Mark region as free and attempt to coalesce it with neighbours.
     const auto new_it = CarveVMA(virtual_addr, size);
@@ -444,7 +495,10 @@ u64 MemoryManager::UnmapBytesFromEntry(VAddr virtual_addr, VirtualMemoryArea vma
     if (type == VMAType::Flexible) {
         flexible_usage -= adjusted_size;
     }
-    rasterizer->UnmapMemory(virtual_addr, adjusted_size);
+
+    if (IsValidGpuMapping(virtual_addr, adjusted_size)) {
+        rasterizer->UnmapMemory(virtual_addr, adjusted_size);
+    }
 
     // Mark region as free and attempt to coalesce it with neighbours.
     const auto new_it = CarveVMA(virtual_addr, adjusted_size);
@@ -471,6 +525,8 @@ s32 MemoryManager::UnmapMemoryImpl(VAddr virtual_addr, u64 size) {
     do {
         auto it = FindVMA(virtual_addr + unmapped_bytes);
         auto& vma_base = it->second;
+        ASSERT_MSG(vma_base.Contains(virtual_addr + unmapped_bytes, 0),
+                   "Address {:#x} is out of bounds", virtual_addr + unmapped_bytes);
         auto unmapped =
             UnmapBytesFromEntry(virtual_addr + unmapped_bytes, vma_base, size - unmapped_bytes);
         ASSERT_MSG(unmapped > 0, "Failed to unmap memory, progress is impossible");
@@ -485,7 +541,10 @@ int MemoryManager::QueryProtection(VAddr addr, void** start, void** end, u32* pr
 
     const auto it = FindVMA(addr);
     const auto& vma = it->second;
-    ASSERT_MSG(vma.type != VMAType::Free, "Provided address is not mapped");
+    if (!vma.Contains(addr, 0) || vma.IsFree()) {
+        LOG_ERROR(Kernel_Vmm, "Address {:#x} is not mapped", addr);
+        return ORBIS_KERNEL_ERROR_EACCES;
+    }
 
     if (start != nullptr) {
         *start = reinterpret_cast<void*>(vma.base);
@@ -555,6 +614,8 @@ s32 MemoryManager::Protect(VAddr addr, size_t size, MemoryProt prot) {
     do {
         auto it = FindVMA(addr + protected_bytes);
         auto& vma_base = it->second;
+        ASSERT_MSG(vma_base.Contains(addr + protected_bytes, 0), "Address {:#x} is out of bounds",
+                   addr + protected_bytes);
         auto result = 0;
         result = ProtectBytes(addr + protected_bytes, vma_base, size - protected_bytes, prot);
         if (result < 0) {
@@ -571,8 +632,16 @@ int MemoryManager::VirtualQuery(VAddr addr, int flags,
                                 ::Libraries::Kernel::OrbisVirtualQueryInfo* info) {
     std::scoped_lock lk{mutex};
 
-    auto it = FindVMA(addr);
-    if (it->second.type == VMAType::Free && flags == 1) {
+    // FindVMA on addresses before the vma_map return garbage data.
+    auto query_addr =
+        addr < impl.SystemManagedVirtualBase() ? impl.SystemManagedVirtualBase() : addr;
+    if (addr < query_addr && flags == 0) {
+        LOG_WARNING(Kernel_Vmm, "VirtualQuery on free memory region");
+        return ORBIS_KERNEL_ERROR_EACCES;
+    }
+    auto it = FindVMA(query_addr);
+
+    while (it->second.type == VMAType::Free && flags == 1 && it != --vma_map.end()) {
         ++it;
     }
     if (it->second.type == VMAType::Free) {
@@ -585,15 +654,17 @@ int MemoryManager::VirtualQuery(VAddr addr, int flags,
     info->end = vma.base + vma.size;
     info->offset = vma.phys_base;
     info->protection = static_cast<s32>(vma.prot);
-    info->is_flexible.Assign(vma.type == VMAType::Flexible);
-    info->is_direct.Assign(vma.type == VMAType::Direct);
-    info->is_stack.Assign(vma.type == VMAType::Stack);
-    info->is_pooled.Assign(vma.type == VMAType::PoolReserved || vma.type == VMAType::Pooled);
-    info->is_committed.Assign(vma.IsMapped());
-    vma.name.copy(info->name.data(), std::min(info->name.size(), vma.name.size()));
+    info->is_flexible = vma.type == VMAType::Flexible ? 1 : 0;
+    info->is_direct = vma.type == VMAType::Direct ? 1 : 0;
+    info->is_stack = vma.type == VMAType::Stack ? 1 : 0;
+    info->is_pooled = vma.type == VMAType::PoolReserved || vma.type == VMAType::Pooled ? 1 : 0;
+    info->is_committed = vma.IsMapped() ? 1 : 0;
+
+    strncpy(info->name, vma.name.data(), ::Libraries::Kernel::ORBIS_KERNEL_MAXIMUM_NAME_LENGTH);
+
     if (vma.type == VMAType::Direct) {
         const auto dmem_it = FindDmemArea(vma.phys_base);
-        ASSERT(dmem_it != dmem_map.end());
+        ASSERT_MSG(vma.phys_base <= dmem_it->second.GetEnd(), "vma.phys_base is not in dmem_map!");
         info->memory_type = dmem_it->second.memory_type;
     } else {
         info->memory_type = ::Libraries::Kernel::SCE_KERNEL_WB_ONION;
@@ -607,11 +678,11 @@ int MemoryManager::DirectMemoryQuery(PAddr addr, bool find_next,
     std::scoped_lock lk{mutex};
 
     auto dmem_area = FindDmemArea(addr);
-    while (dmem_area != dmem_map.end() && dmem_area->second.is_free && find_next) {
+    while (dmem_area != --dmem_map.end() && dmem_area->second.is_free && find_next) {
         dmem_area++;
     }
 
-    if (dmem_area == dmem_map.end() || dmem_area->second.is_free) {
+    if (dmem_area->second.is_free) {
         LOG_ERROR(Core, "Unable to find allocated direct memory region to query!");
         return ORBIS_KERNEL_ERROR_EACCES;
     }
@@ -691,36 +762,56 @@ VAddr MemoryManager::SearchFree(VAddr virtual_addr, size_t size, u32 alignment)
         virtual_addr = min_search_address;
     }
 
+    // If the requested address is beyond the maximum our code can handle, throw an assert
+    auto max_search_address = impl.UserVirtualBase() + impl.UserVirtualSize();
+    ASSERT_MSG(virtual_addr <= max_search_address, "Input address {:#x} is out of bounds",
+               virtual_addr);
+
     auto it = FindVMA(virtual_addr);
-    ASSERT_MSG(it != vma_map.end(), "Specified mapping address was not found!");
 
     // If the VMA is free and contains the requested mapping we are done.
     if (it->second.IsFree() && it->second.Contains(virtual_addr, size)) {
         return virtual_addr;
     }
+
     // Search for the first free VMA that fits our mapping.
-    const auto is_suitable = [&] {
+    while (it != vma_map.end()) {
         if (!it->second.IsFree()) {
-            return false;
+            it++;
+            continue;
         }
+
         const auto& vma = it->second;
         virtual_addr = Common::AlignUp(vma.base, alignment);
         // Sometimes the alignment itself might be larger than the VMA.
         if (virtual_addr > vma.base + vma.size) {
-            return false;
+            it++;
+            continue;
         }
+
+        // Make sure the address is within our defined bounds
+        if (virtual_addr >= max_search_address) {
+            // There are no free mappings within our safely usable address space.
+            break;
+        }
+
+        // If there's enough space in the VMA, return the address.
         const size_t remaining_size = vma.base + vma.size - virtual_addr;
-        return remaining_size >= size;
-    };
-    while (!is_suitable()) {
-        ++it;
+        if (remaining_size >= size) {
+            return virtual_addr;
+        }
+        it++;
     }
-    return virtual_addr;
+
+    // Couldn't find a suitable VMA, return an error.
+    LOG_ERROR(Kernel_Vmm, "Couldn't find a free mapping for address {:#x}, size {:#x}",
+              virtual_addr, size);
+    return -1;
 }
 
 MemoryManager::VMAHandle MemoryManager::CarveVMA(VAddr virtual_addr, size_t size) {
     auto vma_handle = FindVMA(virtual_addr);
-    ASSERT_MSG(vma_handle != vma_map.end(), "Virtual address not in vm_map");
+    ASSERT_MSG(vma_handle->second.Contains(virtual_addr, 0), "Virtual address not in vm_map");
 
     const VirtualMemoryArea& vma = vma_handle->second;
     ASSERT_MSG(vma.base <= virtual_addr, "Adding a mapping to already mapped region");
@@ -749,7 +840,7 @@ MemoryManager::VMAHandle MemoryManager::CarveVMA(VAddr virtual_addr, size_t size
 
 MemoryManager::DMemHandle MemoryManager::CarveDmemArea(PAddr addr, size_t size) {
     auto dmem_handle = FindDmemArea(addr);
-    ASSERT_MSG(dmem_handle != dmem_map.end(), "Physical address not in dmem_map");
+    ASSERT_MSG(addr <= dmem_handle->second.GetEnd(), "Physical address not in dmem_map");
 
     const DirectMemoryArea& area = dmem_handle->second;
     ASSERT_MSG(area.base <= addr, "Adding an allocation to already allocated region");
@@ -804,7 +895,7 @@ int MemoryManager::GetDirectMemoryType(PAddr addr, int* directMemoryTypeOut,
 
     auto dmem_area = FindDmemArea(addr);
 
-    if (dmem_area == dmem_map.end() || dmem_area->second.is_free) {
+    if (addr > dmem_area->second.GetEnd() || dmem_area->second.is_free) {
         LOG_ERROR(Core, "Unable to find allocated direct memory region to check type!");
         return ORBIS_KERNEL_ERROR_ENOENT;
     }