Workaround for readonly memory mapping of files issue

This commit is contained in:
Borchev 2024-08-20 16:10:38 -07:00
parent 32cb3649d3
commit 6596fe091c
4 changed files with 26 additions and 12 deletions

View File

@ -217,7 +217,7 @@ void IOFile::Close() {
file = nullptr; file = nullptr;
#ifdef _WIN64 #ifdef _WIN64
if (file_mapping) { if (file_mapping && file_access_mode == FileAccessMode::ReadWrite) {
CloseHandle(std::bit_cast<HANDLE>(file_mapping)); CloseHandle(std::bit_cast<HANDLE>(file_mapping));
} }
#endif #endif
@ -259,8 +259,7 @@ uintptr_t IOFile::GetFileMapping() {
mapping = CreateFileMapping2(hfile, NULL, FILE_MAP_WRITE, PAGE_READWRITE, SEC_COMMIT, 0, mapping = CreateFileMapping2(hfile, NULL, FILE_MAP_WRITE, PAGE_READWRITE, SEC_COMMIT, 0,
NULL, NULL, 0); NULL, NULL, 0);
} else { } else {
mapping = CreateFileMapping2(hfile, NULL, FILE_MAP_READ, PAGE_READONLY, SEC_COMMIT, 0, NULL, mapping = hfile;
NULL, 0);
} }
file_mapping = std::bit_cast<uintptr_t>(mapping); file_mapping = std::bit_cast<uintptr_t>(mapping);

View File

@ -2,6 +2,7 @@
// SPDX-License-Identifier: GPL-2.0-or-later // SPDX-License-Identifier: GPL-2.0-or-later
#include <boost/icl/separate_interval_set.hpp> #include <boost/icl/separate_interval_set.hpp>
#include "common/alignment.h"
#include "common/assert.h" #include "common/assert.h"
#include "common/error.h" #include "common/error.h"
#include "core/address_space.h" #include "core/address_space.h"
@ -129,9 +130,10 @@ struct AddressSpace::Impl {
} }
void* Map(VAddr virtual_addr, PAddr phys_addr, size_t size, ULONG prot, uintptr_t fd = 0) { 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); const auto it = placeholders.find(virtual_addr);
ASSERT_MSG(it != placeholders.end(), "Cannot map already mapped region"); ASSERT_MSG(it != placeholders.end(), "Cannot map already mapped region");
ASSERT_MSG(virtual_addr >= it->lower() && virtual_addr + size <= it->upper(), ASSERT_MSG(virtual_addr >= it->lower() && virtual_addr + aligned_size <= it->upper(),
"Map range must be fully contained in a placeholder"); "Map range must be fully contained in a placeholder");
// Windows only allows splitting a placeholder into two. // Windows only allows splitting a placeholder into two.
@ -140,7 +142,7 @@ struct AddressSpace::Impl {
// one at the start and at the end. // one at the start and at the end.
const VAddr placeholder_start = it->lower(); const VAddr placeholder_start = it->lower();
const VAddr placeholder_end = it->upper(); const VAddr placeholder_end = it->upper();
const VAddr virtual_end = virtual_addr + size; const VAddr virtual_end = virtual_addr + aligned_size;
// If the placeholder doesn't exactly start at virtual_addr, split it at the start. // If the placeholder doesn't exactly start at virtual_addr, split it at the start.
if (placeholder_start != virtual_addr) { if (placeholder_start != virtual_addr) {
@ -161,11 +163,23 @@ struct AddressSpace::Impl {
void* ptr = nullptr; void* ptr = nullptr;
if (phys_addr != -1) { if (phys_addr != -1) {
HANDLE backing = fd ? reinterpret_cast<HANDLE>(fd) : backing_handle; HANDLE backing = fd ? reinterpret_cast<HANDLE>(fd) : backing_handle;
ptr = MapViewOfFile3(backing, process, reinterpret_cast<PVOID>(virtual_addr), phys_addr, if (fd && prot == PAGE_READONLY) {
size, MEM_REPLACE_PLACEHOLDER, prot, nullptr, 0); DWORD resultvar;
ptr = VirtualAlloc2(process, reinterpret_cast<PVOID>(virtual_addr), aligned_size,
MEM_RESERVE | MEM_COMMIT | MEM_REPLACE_PLACEHOLDER,
PAGE_READWRITE, nullptr, 0);
bool ret = ReadFile(backing, ptr, size, &resultvar, NULL);
ASSERT_MSG(ret, "ReadFile failed. {}", Common::GetLastErrorMsg());
ret = VirtualProtect(ptr, size, prot, &resultvar);
ASSERT_MSG(ret, "VirtualProtect failed. {}", Common::GetLastErrorMsg());
} else {
ptr = MapViewOfFile3(backing, process, reinterpret_cast<PVOID>(virtual_addr),
phys_addr, aligned_size, MEM_REPLACE_PLACEHOLDER, prot,
nullptr, 0);
}
} else { } else {
ptr = ptr =
VirtualAlloc2(process, reinterpret_cast<PVOID>(virtual_addr), size, VirtualAlloc2(process, reinterpret_cast<PVOID>(virtual_addr), aligned_size,
MEM_RESERVE | MEM_COMMIT | MEM_REPLACE_PLACEHOLDER, prot, nullptr, 0); MEM_RESERVE | MEM_COMMIT | MEM_REPLACE_PLACEHOLDER, prot, nullptr, 0);
} }
ASSERT_MSG(ptr, "{}", Common::GetLastErrorMsg()); ASSERT_MSG(ptr, "{}", Common::GetLastErrorMsg());
@ -455,12 +469,12 @@ void* AddressSpace::MapFile(VAddr virtual_addr, size_t size, size_t offset, u32
} }
void AddressSpace::Unmap(VAddr virtual_addr, size_t size, VAddr start_in_vma, VAddr end_in_vma, void AddressSpace::Unmap(VAddr virtual_addr, size_t size, VAddr start_in_vma, VAddr end_in_vma,
PAddr phys_base, bool is_exec, bool has_backing) { PAddr phys_base, bool is_exec, bool has_backing, bool readonly) {
#ifdef _WIN32 #ifdef _WIN32
// There does not appear to be comparable support for partial unmapping on Windows. // There does not appear to be comparable support for partial unmapping on Windows.
// Unfortunately, a least one title was found to require this. The workaround is to unmap // Unfortunately, a least one title was found to require this. The workaround is to unmap
// the entire allocation and remap the portions outside of the requested unmapping range. // the entire allocation and remap the portions outside of the requested unmapping range.
impl->Unmap(virtual_addr, size, has_backing); impl->Unmap(virtual_addr, size, has_backing && !readonly);
// TODO: Determine if any titles require partial unmapping support for flexible allocations. // TODO: Determine if any titles require partial unmapping support for flexible allocations.
ASSERT_MSG(has_backing || (start_in_vma == 0 && end_in_vma == size), ASSERT_MSG(has_backing || (start_in_vma == 0 && end_in_vma == size),

View File

@ -92,7 +92,7 @@ public:
/// Unmaps specified virtual memory area. /// Unmaps specified virtual memory area.
void Unmap(VAddr virtual_addr, size_t size, VAddr start_in_vma, VAddr end_in_vma, void Unmap(VAddr virtual_addr, size_t size, VAddr start_in_vma, VAddr end_in_vma,
PAddr phys_base, bool is_exec, bool has_backing); PAddr phys_base, bool is_exec, bool has_backing, bool readonly);
void Protect(VAddr virtual_addr, size_t size, MemoryPermission perms); void Protect(VAddr virtual_addr, size_t size, MemoryPermission perms);

View File

@ -242,10 +242,11 @@ void MemoryManager::UnmapMemory(VAddr virtual_addr, size_t size) {
vma.disallow_merge = false; vma.disallow_merge = false;
vma.name = ""; vma.name = "";
MergeAdjacent(vma_map, new_it); MergeAdjacent(vma_map, new_it);
bool readonly = vma.prot == MemoryProt::CpuRead;
// Unmap the memory region. // Unmap the memory region.
impl.Unmap(vma_base_addr, vma_base_size, start_in_vma, start_in_vma + size, phys_base, is_exec, impl.Unmap(vma_base_addr, vma_base_size, start_in_vma, start_in_vma + size, phys_base, is_exec,
has_backing); has_backing, readonly);
TRACK_FREE(virtual_addr, "VMEM"); TRACK_FREE(virtual_addr, "VMEM");
} }