From 0beed6a9b62bed31566f4fde90011c234788838b Mon Sep 17 00:00:00 2001 From: uytvbn Date: Tue, 24 Oct 2017 23:12:33 +0200 Subject: [PATCH 1/5] [Linux] Implement virtual memory allocation --- src/xenia/base/memory_posix.cc | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index 68fdcd61b..1e55f99d9 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -40,7 +40,13 @@ void* AllocFixed(void* base_address, size_t length, AllocationType allocation_type, PageAccess access) { // mmap does not support reserve / commit, so ignore allocation_type. uint32_t prot = ToPosixProtectFlags(access); - return mmap(base_address, length, prot, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + void* result = mmap(base_address, length, prot, + MAP_PRIVATE | MAP_FIXED | MAP_ANONYMOUS, -1, 0); + if (result == MAP_FAILED) { + return nullptr; + } else { + return result; + } } bool DeallocFixed(void* base_address, size_t length, @@ -90,7 +96,7 @@ FileMappingHandle CreateFileMappingHandle(std::wstring path, size_t length, } void CloseFileMappingHandle(FileMappingHandle handle) { - close((intptr_t)handle); + close(static_cast(reinterpret_cast(handle))); } void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, From cbbb69273c637e32ad9c6e98709b26ced5f9bfed Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Wed, 10 Jul 2019 22:31:16 -0400 Subject: [PATCH 2/5] [memory] Move "Local\\" prefix to win impl CreateFileMappingHandle now takes shared memory name without a prefix. The doc of shm_open recommends not using slashes and prefixing with "/". The prefixing has been moved to the os implementation layer. Invocations of CreateFileMappingHandle were all using "Local\\" so these prefixes were removed. --- src/xenia/base/memory_posix.cc | 3 ++- src/xenia/base/memory_win.cc | 3 ++- src/xenia/cpu/backend/x64/x64_code_cache.cc | 2 +- src/xenia/memory.cc | 2 +- 4 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index 1e55f99d9..f315d6576 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -87,7 +87,8 @@ FileMappingHandle CreateFileMappingHandle(std::wstring path, size_t length, } oflag |= O_CREAT; - int ret = shm_open(xe::to_string(path).c_str(), oflag, 0777); + std::string full_path = "/" + xe::to_string(path); + int ret = shm_open(full_path.c_str(), oflag, 0777); if (ret > 0) { ftruncate64(ret, length); } diff --git a/src/xenia/base/memory_win.cc b/src/xenia/base/memory_win.cc index 0113c5f6d..583dd67f2 100644 --- a/src/xenia/base/memory_win.cc +++ b/src/xenia/base/memory_win.cc @@ -146,9 +146,10 @@ FileMappingHandle CreateFileMappingHandle(std::wstring path, size_t length, PageAccess access, bool commit) { DWORD protect = ToWin32ProtectFlags(access) | (commit ? SEC_COMMIT : SEC_RESERVE); + std::wstring full_path = L"Local\\" + path; return CreateFileMappingW(INVALID_HANDLE_VALUE, NULL, protect, static_cast(length >> 32), - static_cast(length), path.c_str()); + static_cast(length), full_path.c_str()); } void CloseFileMappingHandle(FileMappingHandle handle) { CloseHandle(handle); } diff --git a/src/xenia/cpu/backend/x64/x64_code_cache.cc b/src/xenia/cpu/backend/x64/x64_code_cache.cc index e4a23248e..3eb109527 100644 --- a/src/xenia/cpu/backend/x64/x64_code_cache.cc +++ b/src/xenia/cpu/backend/x64/x64_code_cache.cc @@ -61,7 +61,7 @@ bool X64CodeCache::Initialize() { } // Create mmap file. This allows us to share the code cache with the debugger. - file_name_ = std::wstring(L"Local\\xenia_code_cache_") + + file_name_ = std::wstring(L"xenia_code_cache_") + std::to_wstring(Clock::QueryHostTickCount()); mapping_ = xe::memory::CreateFileMappingHandle( file_name_, kGeneratedCodeSize, xe::memory::PageAccess::kExecuteReadWrite, diff --git a/src/xenia/memory.cc b/src/xenia/memory.cc index c0a59dcc3..826e2a1bd 100644 --- a/src/xenia/memory.cc +++ b/src/xenia/memory.cc @@ -122,7 +122,7 @@ Memory::~Memory() { } bool Memory::Initialize() { - file_name_ = std::wstring(L"Local\\xenia_memory_") + + file_name_ = std::wstring(L"xenia_memory_") + std::to_wstring(Clock::QueryHostTickCount()); // Create main page file-backed mapping. This is all reserved but From 220c65782374b0f6c72a13497d85f8d261ddd02e Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 09:44:42 -0400 Subject: [PATCH 3/5] [memory linux] Properly unlink shared memory shm_unlink(name) is the proper way to close a shared memory in linux. Prior to this, xenia was creating and not cleaning up shared memory handle which would accumulate in /dev/shm. shm_unlink is the proper way of doing this. Add filename to CloseFileMappingHandle signature. Add simple test to open and close. --- src/xenia/base/memory.h | 2 +- src/xenia/base/memory_posix.cc | 5 +++-- src/xenia/base/memory_win.cc | 4 +++- src/xenia/base/testing/memory_test.cc | 7 +++++++ src/xenia/cpu/backend/x64/x64_code_cache.cc | 2 +- src/xenia/memory.cc | 2 +- 6 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/xenia/base/memory.h b/src/xenia/base/memory.h index 3819f9dbe..18eb6a243 100644 --- a/src/xenia/base/memory.h +++ b/src/xenia/base/memory.h @@ -99,7 +99,7 @@ typedef void* FileMappingHandle; FileMappingHandle CreateFileMappingHandle(std::wstring path, size_t length, PageAccess access, bool commit); -void CloseFileMappingHandle(FileMappingHandle handle); +void CloseFileMappingHandle(FileMappingHandle handle, std::wstring path); void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, PageAccess access, size_t file_offset); bool UnmapFileView(FileMappingHandle handle, void* base_address, size_t length); diff --git a/src/xenia/base/memory_posix.cc b/src/xenia/base/memory_posix.cc index f315d6576..13484dd29 100644 --- a/src/xenia/base/memory_posix.cc +++ b/src/xenia/base/memory_posix.cc @@ -96,8 +96,9 @@ FileMappingHandle CreateFileMappingHandle(std::wstring path, size_t length, return ret <= 0 ? nullptr : reinterpret_cast(ret); } -void CloseFileMappingHandle(FileMappingHandle handle) { - close(static_cast(reinterpret_cast(handle))); +void CloseFileMappingHandle(FileMappingHandle handle, std::wstring path) { + std::string full_path = "/" + xe::to_string(path); + shm_unlink(full_path.c_str()); } void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, diff --git a/src/xenia/base/memory_win.cc b/src/xenia/base/memory_win.cc index 583dd67f2..72fcb084b 100644 --- a/src/xenia/base/memory_win.cc +++ b/src/xenia/base/memory_win.cc @@ -152,7 +152,9 @@ FileMappingHandle CreateFileMappingHandle(std::wstring path, size_t length, static_cast(length), full_path.c_str()); } -void CloseFileMappingHandle(FileMappingHandle handle) { CloseHandle(handle); } +void CloseFileMappingHandle(FileMappingHandle handle, std::wstring path) { + CloseHandle(handle); +} void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, PageAccess access, size_t file_offset) { diff --git a/src/xenia/base/testing/memory_test.cc b/src/xenia/base/testing/memory_test.cc index 5433dc118..cb30e8778 100644 --- a/src/xenia/base/testing/memory_test.cc +++ b/src/xenia/base/testing/memory_test.cc @@ -414,6 +414,13 @@ TEST_CASE("copy_and_swap_16_in_32_unaligned", "Copy and Swap") { REQUIRE(true == true); } +TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { + auto memory = xe::memory::CreateFileMappingHandle( + L"test", 0x100, xe::memory::PageAccess::kReadWrite, false); + REQUIRE(memory); + xe::memory::CloseFileMappingHandle(memory, L"test"); +} + } // namespace test } // namespace base } // namespace xe diff --git a/src/xenia/cpu/backend/x64/x64_code_cache.cc b/src/xenia/cpu/backend/x64/x64_code_cache.cc index 3eb109527..37a4f6a04 100644 --- a/src/xenia/cpu/backend/x64/x64_code_cache.cc +++ b/src/xenia/cpu/backend/x64/x64_code_cache.cc @@ -42,7 +42,7 @@ X64CodeCache::~X64CodeCache() { if (mapping_) { xe::memory::UnmapFileView(mapping_, generated_code_base_, kGeneratedCodeSize); - xe::memory::CloseFileMappingHandle(mapping_); + xe::memory::CloseFileMappingHandle(mapping_, file_name_); mapping_ = nullptr; } } diff --git a/src/xenia/memory.cc b/src/xenia/memory.cc index 826e2a1bd..e41ad4d71 100644 --- a/src/xenia/memory.cc +++ b/src/xenia/memory.cc @@ -112,7 +112,7 @@ Memory::~Memory() { // Unmap all views and close mapping. if (mapping_) { UnmapViews(); - xe::memory::CloseFileMappingHandle(mapping_); + xe::memory::CloseFileMappingHandle(mapping_, file_name_); mapping_base_ = nullptr; mapping_ = nullptr; } From fe2bb74d7cc70defbbda51affeac7261d6dffe43 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 10:26:59 -0400 Subject: [PATCH 4/5] [memory] Add Memory mapping view tests Add test for mapping and for mapping with access. --- src/xenia/base/testing/memory_test.cc | 42 +++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/xenia/base/testing/memory_test.cc b/src/xenia/base/testing/memory_test.cc index cb30e8778..9083eef40 100644 --- a/src/xenia/base/testing/memory_test.cc +++ b/src/xenia/base/testing/memory_test.cc @@ -421,6 +421,48 @@ TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") { xe::memory::CloseFileMappingHandle(memory, L"test"); } +TEST_CASE("map_view", "Virtual Memory Mapping") { + const size_t length = 0x100; + auto memory = xe::memory::CreateFileMappingHandle( + L"test", length, xe::memory::PageAccess::kReadWrite, false); + REQUIRE(memory); + + uintptr_t address = 0x100000000; + auto view = + xe::memory::MapFileView(memory, reinterpret_cast(address), length, + xe::memory::PageAccess::kReadWrite, 0); + REQUIRE(reinterpret_cast(view) == address); + + xe::memory::UnmapFileView(memory, reinterpret_cast(address), length); + xe::memory::CloseFileMappingHandle(memory, L"test"); +} + +TEST_CASE("read_write_view", "Virtual Memory Mapping") { + const size_t length = 0x100; + auto memory = xe::memory::CreateFileMappingHandle( + L"test", length, xe::memory::PageAccess::kReadWrite, false); + REQUIRE(memory); + + uintptr_t address = 0x100000000; + auto view = + xe::memory::MapFileView(memory, reinterpret_cast(address), length, + xe::memory::PageAccess::kReadWrite, 0); + REQUIRE(reinterpret_cast(view) == address); + + for (uint32_t i = 0; i < length; i += sizeof(uint8_t)) { + auto p_value = reinterpret_cast(address + i); + *p_value = i; + } + for (uint32_t i = 0; i < length; i += sizeof(uint8_t)) { + auto p_value = reinterpret_cast(address + i); + uint8_t value = *p_value; + REQUIRE(value == i); + } + + xe::memory::UnmapFileView(memory, reinterpret_cast(address), length); + xe::memory::CloseFileMappingHandle(memory, L"test"); +} + } // namespace test } // namespace base } // namespace xe From 05b3680b225f91254e30226a7f187ccee7b01b11 Mon Sep 17 00:00:00 2001 From: Sandy Carter Date: Thu, 11 Jul 2019 22:00:07 -0400 Subject: [PATCH 5/5] [kernel] Define param order between compilers Fix issue in clang where args were inverted last to first due to the way c++ implements function calls. The function make_tuple, being a function, has undefined ordering of resolution of the values of its parameters `Ps(init)` and would vary between compilers. Namely clang would resolve them in order and msvc would resolve them in reverse order. This normally would not matter except for the fact that init maintains a mutable state which is affected my the order of operations: init.ordinal is a counter and also defines where in memory a value is stored. The C++ standard doesn't define an order of resolution of parameters in a function but it will define an order in a brace-initializer. Switching make_tuple for a brace-initializer enforces an order which is the same between all compilers (tested gcc, clang and msvc). Prior code was written to decrement ordinal due to the reverse traversal. This has been switched to incrementing thanks to the in-order traversal. --- src/xenia/kernel/util/shim_utils.h | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/xenia/kernel/util/shim_utils.h b/src/xenia/kernel/util/shim_utils.h index 57d5d1e98..15e783022 100644 --- a/src/xenia/kernel/util/shim_utils.h +++ b/src/xenia/kernel/util/shim_utils.h @@ -108,7 +108,7 @@ class Param { protected: Param() : ordinal_(-1) {} - explicit Param(Init& init) : ordinal_(--init.ordinal) {} + explicit Param(Init& init) : ordinal_(init.ordinal++) {} template void LoadValue(Init& init, V* out_value) { @@ -473,10 +473,13 @@ xe::cpu::Export* RegisterExport(R (*fn)(Ps&...), const char* name, ++export_entry->function_data.call_count; Param::Init init = { ppc_context, - sizeof...(Ps), 0, }; - auto params = std::make_tuple(Ps(init)...); + // Using braces initializer instead of make_tuple because braces + // enforce execution order across compilers. + // The make_tuple order is undefined per the C++ standard and + // cause inconsitencies between msvc and clang. + std::tuple params = {Ps(init)...}; if (export_entry->tags & xe::cpu::ExportTag::kLog && (!(export_entry->tags & xe::cpu::ExportTag::kHighFrequency) || cvars::log_high_frequency_kernel_calls)) { @@ -508,9 +511,13 @@ xe::cpu::Export* RegisterExport(void (*fn)(Ps&...), const char* name, ++export_entry->function_data.call_count; Param::Init init = { ppc_context, - sizeof...(Ps), + 0, }; - auto params = std::make_tuple(Ps(init)...); + // Using braces initializer instead of make_tuple because braces + // enforce execution order across compilers. + // The make_tuple order is undefined per the C++ standard and + // cause inconsitencies between msvc and clang. + std::tuple params = {Ps(init)...}; if (export_entry->tags & xe::cpu::ExportTag::kLog && (!(export_entry->tags & xe::cpu::ExportTag::kHighFrequency) || cvars::log_high_frequency_kernel_calls)) {