From f2b6d41ac6a8ecef2d0495b01792ef73f6e941ae Mon Sep 17 00:00:00 2001 From: IndecisiveTurtle <47210458+raphaelthegreat@users.noreply.github.com> Date: Sat, 9 Nov 2024 15:29:09 +0200 Subject: [PATCH] windows: Address a bunch of address space problems --- src/core/address_space.cpp | 199 ++++++++++++------ src/core/libraries/kernel/memory.cpp | 25 ++- src/core/libraries/kernel/threads/pthread.h | 4 +- src/core/libraries/kernel/threads/stack.cpp | 4 +- src/core/memory.h | 4 + src/core/module.cpp | 2 +- src/video_core/amdgpu/liverpool.cpp | 4 +- src/video_core/page_manager.cpp | 15 +- .../renderer_vulkan/vk_platform.cpp | 2 +- 9 files changed, 163 insertions(+), 96 deletions(-) diff --git a/src/core/address_space.cpp b/src/core/address_space.cpp index 4633cbe2e..b7547867c 100644 --- a/src/core/address_space.cpp +++ b/src/core/address_space.cpp @@ -40,6 +40,12 @@ static constexpr size_t BackingSize = SCE_KERNEL_MAIN_DMEM_SIZE_PRO; } } +struct MemoryRegion { + VAddr base; + size_t size; + bool is_mapped; +}; + struct AddressSpace::Impl { Impl() : process{GetCurrentProcess()} { // Allocate virtual address placeholder for our address space. @@ -75,6 +81,7 @@ struct AddressSpace::Impl { Common::GetLastErrorMsg()); // Take the reduction off of the system managed area, and leave the others unchanged. + reduction = size_t(virtual_base - SYSTEM_MANAGED_MIN); system_managed_base = virtual_base; system_managed_size = SystemManagedSize - reduction; system_reserved_base = reinterpret_cast(SYSTEM_RESERVED_MIN); @@ -95,7 +102,8 @@ struct AddressSpace::Impl { const uintptr_t system_managed_addr = reinterpret_cast(system_managed_base); const uintptr_t system_reserved_addr = reinterpret_cast(system_reserved_base); const uintptr_t user_addr = reinterpret_cast(user_base); - placeholders.insert({system_managed_addr, virtual_size - reduction}); + regions.emplace(system_managed_addr, + MemoryRegion{system_managed_addr, virtual_size - reduction, false}); // Allocate backing file that represents the total physical memory. backing_handle = @@ -132,42 +140,15 @@ struct AddressSpace::Impl { } void* Map(VAddr virtual_addr, PAddr phys_addr, size_t size, ULONG prot, uintptr_t fd = 0) { - const size_t aligned_size = Common::AlignUp(size, 16_KB); - const auto it = placeholders.find(virtual_addr); - ASSERT_MSG(it != placeholders.end(), "Cannot map already mapped region"); - ASSERT_MSG(virtual_addr >= it->lower() && virtual_addr + aligned_size <= it->upper(), - "Map range must be fully contained in a placeholder"); - - // Windows only allows splitting a placeholder into two. - // This means that if the map range is fully - // contained the the placeholder we need to perform two split operations, - // one at the start and at the end. - const VAddr placeholder_start = it->lower(); - const VAddr placeholder_end = it->upper(); - const VAddr virtual_end = virtual_addr + aligned_size; - - // If the placeholder doesn't exactly start at virtual_addr, split it at the start. - if (placeholder_start != virtual_addr) { - VirtualFreeEx(process, reinterpret_cast(placeholder_start), - virtual_addr - placeholder_start, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER); - } - - // If the placeholder doesn't exactly end at virtual_end, split it at the end. - if (placeholder_end != virtual_end) { - VirtualFreeEx(process, reinterpret_cast(virtual_end), - placeholder_end - virtual_end, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER); - } - - // Remove the placeholder. - placeholders.erase({virtual_addr, virtual_end}); - - // Perform the map. + // Before mapping we must carve a placeholder with the exact properties of our mapping. + auto* region = EnsureSplitRegionForMapping(virtual_addr, size); + region->is_mapped = true; void* ptr = nullptr; if (phys_addr != -1) { HANDLE backing = fd ? reinterpret_cast(fd) : backing_handle; if (fd && prot == PAGE_READONLY) { DWORD resultvar; - ptr = VirtualAlloc2(process, reinterpret_cast(virtual_addr), aligned_size, + ptr = VirtualAlloc2(process, reinterpret_cast(virtual_addr), size, MEM_RESERVE | MEM_COMMIT | MEM_REPLACE_PLACEHOLDER, PAGE_READWRITE, nullptr, 0); bool ret = ReadFile(backing, ptr, size, &resultvar, NULL); @@ -176,12 +157,12 @@ struct AddressSpace::Impl { ASSERT_MSG(ret, "VirtualProtect failed. {}", Common::GetLastErrorMsg()); } else { ptr = MapViewOfFile3(backing, process, reinterpret_cast(virtual_addr), - phys_addr, aligned_size, MEM_REPLACE_PLACEHOLDER, prot, + phys_addr, size, MEM_REPLACE_PLACEHOLDER, prot, nullptr, 0); } } else { ptr = - VirtualAlloc2(process, reinterpret_cast(virtual_addr), aligned_size, + VirtualAlloc2(process, reinterpret_cast(virtual_addr), size, MEM_RESERVE | MEM_COMMIT | MEM_REPLACE_PLACEHOLDER, prot, nullptr, 0); } ASSERT_MSG(ptr, "{}", Common::GetLastErrorMsg()); @@ -202,33 +183,114 @@ struct AddressSpace::Impl { // The unmap call will create a new placeholder region. We need to see if we can coalesce it // with neighbors. - VAddr placeholder_start = virtual_addr; - VAddr placeholder_end = virtual_addr + size; + JoinRegionsAfterUnmap(virtual_addr, size); + } + // The following code is inspired from Dolphin's MemArena + // https://github.com/dolphin-emu/dolphin/blob/deee3ee4/Source/Core/Common/MemArenaWin.cpp#L212 + MemoryRegion* EnsureSplitRegionForMapping(VAddr address, size_t size) { + // Find closest region that is <= the given address by using upper bound and decrementing + auto it = regions.upper_bound(address); + ASSERT_MSG(it != regions.begin(), "Invalid address {:#x}", address); + --it; + ASSERT_MSG(!it->second.is_mapped, + "Attempt to map {:#x} with size {:#x} which overlaps with {:#x} mapping", + address, size, it->second.base); + auto& [base, region] = *it; + + const VAddr mapping_address = region.base; + const size_t region_size = region.size; + if (mapping_address == address) { + // If this region is already split up correctly we don't have to do anything + if (region_size == size) { + return ®ion; + } + + ASSERT_MSG(region_size >= size, + "Region with address {:#x} and size {:#x} can't fit {:#x}", mapping_address, + region_size, size); + + // Split the placeholder. + if (!VirtualFreeEx(process, LPVOID(address), size, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER)) { + UNREACHABLE_MSG("Region splitting failed: {}", Common::GetLastErrorMsg()); + return nullptr; + } + + // Update tracked mappings and return the first of the two + region.size = size; + const VAddr new_mapping_start = address + size; + regions.emplace_hint(std::next(it), new_mapping_start, + MemoryRegion(new_mapping_start, region_size - size, false)); + return ®ion; + } + + ASSERT(mapping_address < address); + + // Is there enough space to map this? + const size_t offset_in_region = address - mapping_address; + const size_t minimum_size = size + offset_in_region; + ASSERT(region_size >= minimum_size); + + // Split the placeholder. + if (!VirtualFreeEx(process, LPVOID(address), size, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER)) { + UNREACHABLE_MSG("Region splitting failed: {}", Common::GetLastErrorMsg()); + return nullptr; + } + + // Do we now have two regions or three regions? + if (region_size == minimum_size) { + // Split into two; update tracked mappings and return the second one + region.size = offset_in_region; + it = regions.emplace_hint(std::next(it), address, + MemoryRegion(address, size, false)); + return &it->second; + } else { + // Split into three; update tracked mappings and return the middle one + region.size = offset_in_region; + const VAddr middle_mapping_start = address; + const size_t middle_mapping_size = size; + const VAddr after_mapping_start = address + size; + const size_t after_mapping_size = region_size - minimum_size; + it = regions.emplace_hint(std::next(it), + after_mapping_start, MemoryRegion(after_mapping_start, after_mapping_size, false)); + it = regions.emplace_hint(it, middle_mapping_start, + MemoryRegion(middle_mapping_start, middle_mapping_size, false)); + return &it->second; + } + } + + void JoinRegionsAfterUnmap(VAddr address, size_t size) { + // There should be a mapping that matches the request exactly, find it + auto it = regions.find(address); + ASSERT_MSG(it != regions.end() && it->second.size == size, + "Invalid address/size given to unmap."); + auto& [base, region] = *it; + region.is_mapped = false; + // Check if a placeholder exists right before us. - const auto left_it = placeholders.find(virtual_addr - 1); - if (left_it != placeholders.end()) { - ASSERT_MSG(left_it->upper() == virtual_addr, - "Left placeholder does not end at virtual_addr!"); - placeholder_start = left_it->lower(); - VirtualFreeEx(process, reinterpret_cast(placeholder_start), - placeholder_end - placeholder_start, - MEM_RELEASE | MEM_COALESCE_PLACEHOLDERS); + auto it_prev = it != regions.begin() ? std::prev(it) : regions.end(); + if (it_prev != regions.end() && !it_prev->second.is_mapped) { + const size_t total_size = it_prev->second.size + size; + if (!VirtualFreeEx(process, LPVOID(it_prev->first), total_size, + MEM_RELEASE | MEM_COALESCE_PLACEHOLDERS)) { + UNREACHABLE_MSG("Region coalescing failed: {}", Common::GetLastErrorMsg()); + } + + it_prev->second.size = total_size; + it = regions.erase(it); } // Check if a placeholder exists right after us. - const auto right_it = placeholders.find(placeholder_end + 1); - if (right_it != placeholders.end()) { - ASSERT_MSG(right_it->lower() == placeholder_end, - "Right placeholder does not start at virtual_end!"); - placeholder_end = right_it->upper(); - VirtualFreeEx(process, reinterpret_cast(placeholder_start), - placeholder_end - placeholder_start, - MEM_RELEASE | MEM_COALESCE_PLACEHOLDERS); - } + auto it_next = std::next(it); + if (it_next != regions.end() && !it_next->second.is_mapped) { + const size_t total_size = size + it_next->second.size; + if (!VirtualFreeEx(process, LPVOID(it->first), total_size, MEM_RELEASE | MEM_COALESCE_PLACEHOLDERS)) { + UNREACHABLE_MSG("Region coalescing failed: {}", Common::GetLastErrorMsg()); + } - // Insert the new placeholder. - placeholders.insert({placeholder_start, placeholder_end}); + it->second.size = total_size; + regions.erase(it_next); + } } void Protect(VAddr virtual_addr, size_t size, bool read, bool write, bool execute) { @@ -251,18 +313,21 @@ struct AddressSpace::Impl { return; } - DWORD old_flags{}; - bool success = - VirtualProtect(reinterpret_cast(virtual_addr), size, new_flags, &old_flags); - - if (!success) { - LOG_ERROR(Common_Memory, - "Failed to change virtual memory protection for address {:#x}, size {}", - virtual_addr, size); + const VAddr virtual_end = virtual_addr + size; + auto it = --regions.upper_bound(virtual_addr); + for (; it->first < virtual_end; it++) { + if (!it->second.is_mapped) { + continue; + } + const auto& region = it->second; + const size_t range_addr = std::max(region.base, virtual_addr); + const size_t range_size = std::min(region.base + region.size, virtual_end) - range_addr; + DWORD old_flags{}; + if (!VirtualProtectEx(process, LPVOID(range_addr), range_size, new_flags, &old_flags)) { + UNREACHABLE_MSG("Failed to change virtual memory protection for address {:#x}, size {}", + range_addr, range_size); + } } - - // Use assert to ensure success in debug builds - DEBUG_ASSERT(success && "Failed to change virtual memory protection"); } HANDLE process{}; @@ -275,7 +340,7 @@ struct AddressSpace::Impl { size_t system_reserved_size{}; u8* user_base{}; size_t user_size{}; - boost::icl::separate_interval_set placeholders; + std::map regions; }; #else diff --git a/src/core/libraries/kernel/memory.cpp b/src/core/libraries/kernel/memory.cpp index 0a615d731..9fb6b680d 100644 --- a/src/core/libraries/kernel/memory.cpp +++ b/src/core/libraries/kernel/memory.cpp @@ -5,6 +5,7 @@ #include "common/alignment.h" #include "common/assert.h" +#include "common/scope_exit.h" #include "common/logging/log.h" #include "common/singleton.h" #include "core/file_sys/fs.h" @@ -145,11 +146,6 @@ s32 PS4_SYSV_ABI sceKernelReserveVirtualRange(void** addr, u64 len, int flags, u int PS4_SYSV_ABI sceKernelMapNamedDirectMemory(void** addr, u64 len, int prot, int flags, s64 directMemoryStart, u64 alignment, const char* name) { - LOG_INFO(Kernel_Vmm, - "addr = {}, len = {:#x}, prot = {:#x}, flags = {:#x}, directMemoryStart = {:#x}, " - "alignment = {:#x}", - fmt::ptr(*addr), len, prot, flags, directMemoryStart, alignment); - if (len == 0 || !Common::Is16KBAligned(len)) { LOG_ERROR(Kernel_Vmm, "Map size is either zero or not 16KB aligned!"); return SCE_KERNEL_ERROR_EINVAL; @@ -168,6 +164,13 @@ int PS4_SYSV_ABI sceKernelMapNamedDirectMemory(void** addr, u64 len, int prot, i const VAddr in_addr = reinterpret_cast(*addr); const auto mem_prot = static_cast(prot); const auto map_flags = static_cast(flags); + SCOPE_EXIT { + LOG_INFO(Kernel_Vmm, + "in_addr = {:#x}, out_addr = {}, len = {:#x}, prot = {:#x}, flags = {:#x}, directMemoryStart = {:#x}, " + "alignment = {:#x}", + in_addr, fmt::ptr(*addr), len, prot, flags, directMemoryStart, alignment); + }; + auto* memory = Core::Memory::Instance(); return memory->MapMemory(addr, in_addr, len, mem_prot, map_flags, Core::VMAType::Direct, "", false, directMemoryStart, alignment); @@ -201,13 +204,13 @@ s32 PS4_SYSV_ABI sceKernelMapNamedFlexibleMemory(void** addr_in_out, std::size_t const VAddr in_addr = reinterpret_cast(*addr_in_out); const auto mem_prot = static_cast(prot); const auto map_flags = static_cast(flags); + SCOPE_EXIT { + LOG_INFO(Kernel_Vmm, "in_addr = {:#x}, out_addr = {}, len = {:#x}, prot = {:#x}, flags = {:#x}", + in_addr, fmt::ptr(*addr_in_out), len, prot, flags); + }; auto* memory = Core::Memory::Instance(); - const int ret = memory->MapMemory(addr_in_out, in_addr, len, mem_prot, map_flags, - Core::VMAType::Flexible, name); - - LOG_INFO(Kernel_Vmm, "addr = {}, len = {:#x}, prot = {:#x}, flags = {:#x}", - fmt::ptr(*addr_in_out), len, prot, flags); - return ret; + return memory->MapMemory(addr_in_out, in_addr, len, mem_prot, map_flags, + Core::VMAType::Flexible, name); } s32 PS4_SYSV_ABI sceKernelMapFlexibleMemory(void** addr_in_out, std::size_t len, int prot, diff --git a/src/core/libraries/kernel/threads/pthread.h b/src/core/libraries/kernel/threads/pthread.h index 07fe642f5..ff738a6be 100644 --- a/src/core/libraries/kernel/threads/pthread.h +++ b/src/core/libraries/kernel/threads/pthread.h @@ -169,8 +169,8 @@ struct PthreadAttr { }; using PthreadAttrT = PthreadAttr*; -static constexpr u32 ThrStackDefault = 2_MB; -static constexpr u32 ThrStackInitial = 2 * ThrStackDefault; +static constexpr u32 ThrStackDefault = 1_MB; +static constexpr u32 ThrStackInitial = 2_MB; static constexpr u32 ThrPageSize = 16_KB; static constexpr u32 ThrGuardDefault = ThrPageSize; diff --git a/src/core/libraries/kernel/threads/stack.cpp b/src/core/libraries/kernel/threads/stack.cpp index 33a83b67c..45715482a 100644 --- a/src/core/libraries/kernel/threads/stack.cpp +++ b/src/core/libraries/kernel/threads/stack.cpp @@ -22,8 +22,6 @@ int ThreadState::CreateStack(PthreadAttr* attr) { return 0; } - VAddr stackaddr; - /* * Round up stack size to nearest multiple of _thr_page_size so * that mmap() * will work. If the stack size is not an even @@ -83,7 +81,7 @@ int ThreadState::CreateStack(PthreadAttr* attr) { } /* Allocate a new stack. */ - stackaddr = last_stack - stacksize - guardsize; + VAddr stackaddr = last_stack - stacksize - guardsize; /* * Even if stack allocation fails, we don't want to try to diff --git a/src/core/memory.h b/src/core/memory.h index 35bd457a0..a9a42e1c2 100644 --- a/src/core/memory.h +++ b/src/core/memory.h @@ -133,6 +133,10 @@ public: rasterizer = rasterizer_; } + AddressSpace& GetAddressSpace() { + return impl; + } + u64 GetTotalDirectSize() const { return total_direct_size; } diff --git a/src/core/module.cpp b/src/core/module.cpp index f0ae9a4b1..ef34f25c1 100644 --- a/src/core/module.cpp +++ b/src/core/module.cpp @@ -1,6 +1,6 @@ // SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later -#pragma clang optimize off + #include #include "common/alignment.h" diff --git a/src/video_core/amdgpu/liverpool.cpp b/src/video_core/amdgpu/liverpool.cpp index 759fc99ed..7fbb33ea5 100644 --- a/src/video_core/amdgpu/liverpool.cpp +++ b/src/video_core/amdgpu/liverpool.cpp @@ -1,6 +1,6 @@ // SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later -#pragma clang optimize off + #include "common/assert.h" #include "common/config.h" #include "common/debug.h" @@ -570,7 +570,7 @@ Liverpool::Task Liverpool::ProcessGraphics(std::span dcb, std::spansrc_sel == DmaDataSrc::Gds && dma_data->dst_sel == DmaDataDst::Memory) { - LOG_WARNING(Render_Vulkan, "GDS memory read"); + //LOG_WARNING(Render_Vulkan, "GDS memory read"); } else if (dma_data->src_sel == DmaDataSrc::Memory && dma_data->dst_sel == DmaDataDst::Memory) { rasterizer->InlineData(dma_data->DstAddress(), diff --git a/src/video_core/page_manager.cpp b/src/video_core/page_manager.cpp index a49fff43a..8c20ee6ed 100644 --- a/src/video_core/page_manager.cpp +++ b/src/video_core/page_manager.cpp @@ -7,6 +7,7 @@ #include "common/assert.h" #include "common/error.h" #include "common/signal_context.h" +#include "core/memory.h" #include "core/signals.h" #include "video_core/page_manager.h" #include "video_core/renderer_vulkan/vk_rasterizer.h" @@ -145,15 +146,11 @@ struct PageManager::Impl { ASSERT_MSG(owned_ranges.find(address) != owned_ranges.end(), "Attempted to track non-GPU memory at address {:#x}, size {:#x}.", address, size); -#ifdef _WIN32 - DWORD prot = allow_write ? PAGE_READWRITE : PAGE_READONLY; - DWORD old_prot{}; - BOOL result = VirtualProtect(std::bit_cast(address), size, prot, &old_prot); - ASSERT_MSG(result != 0, "Region protection failed"); -#else - mprotect(reinterpret_cast(address), size, - PROT_READ | (allow_write ? PROT_WRITE : 0)); -#endif + auto* memory = Core::Memory::Instance(); + auto& impl = memory->GetAddressSpace(); + impl.Protect(address, size, + allow_write ? Core::MemoryPermission::ReadWrite + : Core::MemoryPermission::Read); } static bool GuestFaultSignalHandler(void* context, void* fault_address) { diff --git a/src/video_core/renderer_vulkan/vk_platform.cpp b/src/video_core/renderer_vulkan/vk_platform.cpp index 25b62039f..0eb7e0759 100644 --- a/src/video_core/renderer_vulkan/vk_platform.cpp +++ b/src/video_core/renderer_vulkan/vk_platform.cpp @@ -1,6 +1,6 @@ // SPDX-FileCopyrightText: Copyright 2024 shadPS4 Emulator Project // SPDX-License-Identifier: GPL-2.0-or-later -#pragma clang optimize off + // Include the vulkan platform specific header #if defined(ANDROID) #define VK_USE_PLATFORM_ANDROID_KHR