From f943ce2710b3ea57de572645b0fd06fea2b9902f Mon Sep 17 00:00:00 2001 From: squidbus <175574877+squidbus@users.noreply.github.com> Date: Mon, 15 Jul 2024 16:01:34 -0700 Subject: [PATCH] Address review comments around memory and patches. --- src/core/address_space.cpp | 50 +++++++++++++++++++++----------------- src/core/cpu_patches.cpp | 24 +++++++++--------- 2 files changed, 41 insertions(+), 33 deletions(-) diff --git a/src/core/address_space.cpp b/src/core/address_space.cpp index 9288af21..0cccd8c5 100644 --- a/src/core/address_space.cpp +++ b/src/core/address_space.cpp @@ -58,20 +58,28 @@ struct AddressSpace::Impl { static constexpr size_t ReductionOnFail = 1_GB; static constexpr size_t MaxReductions = 10; - system_managed_size = SystemManagedSize; - system_reserved_size = SystemReservedSize + ReductionOnFail; - user_size = UserSize; - for (u32 i = 0; i < MaxReductions && !virtual_base; i++) { - system_reserved_size -= ReductionOnFail; - virtual_base = static_cast( - VirtualAlloc2(process, NULL, system_managed_size + system_reserved_size + user_size, - MEM_RESERVE | MEM_RESERVE_PLACEHOLDER, PAGE_NOACCESS, ¶m, 1)); + size_t reduction = 0; + for (u32 i = 0; i < MaxReductions; i++) { + virtual_base = static_cast(VirtualAlloc2( + process, NULL, SystemManagedSize + SystemReservedSize + UserSize - reduction, + MEM_RESERVE | MEM_RESERVE_PLACEHOLDER, PAGE_NOACCESS, ¶m, 1)); + if (virtual_base) { + break; + } + reduction += ReductionOnFail; } ASSERT_MSG(virtual_base, "Unable to reserve virtual address space!"); system_managed_base = virtual_base; - system_reserved_base = virtual_base + system_managed_size; - user_base = system_reserved_base + system_reserved_size; + system_managed_size = SystemManagedSize - reduction; + system_reserved_base = + virtual_base + (SYSTEM_RESERVED_MIN - SYSTEM_MANAGED_MIN) - reduction; + system_reserved_size = SystemReservedSize; + user_base = virtual_base + (USER_MIN - SYSTEM_MANAGED_MIN) - reduction; + user_size = UserSize; + + ASSERT_MSG(user_base == reinterpret_cast(USER_MIN), + "Unexpected user address space location: {}", fmt::ptr(user_base)); LOG_INFO(Kernel_Vmm, "System managed virtual memory region: {} - {}", fmt::ptr(system_managed_base), @@ -277,23 +285,24 @@ struct AddressSpace::Impl { system_managed_size = SystemManagedSize; system_reserved_size = SystemReservedSize; user_size = UserSize; + + constexpr int protection_flags = PROT_READ | PROT_WRITE; + constexpr int base_map_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE; #ifdef __APPLE__ system_managed_base = reinterpret_cast( - mmap(reinterpret_cast(SYSTEM_MANAGED_MIN), system_managed_size, - PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE | MAP_FIXED, - -1, 0)); + mmap(reinterpret_cast(SYSTEM_MANAGED_MIN), system_managed_size, protection_flags, + base_map_flags | MAP_FIXED, -1, 0)); // Cannot guarantee enough space for these areas at the desired addresses, so not MAP_FIXED. system_reserved_base = reinterpret_cast( mmap(reinterpret_cast(SYSTEM_RESERVED_MIN), system_reserved_size, - PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0)); + protection_flags, base_map_flags, -1, 0)); user_base = reinterpret_cast(mmap(reinterpret_cast(USER_MIN), user_size, - PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE, -1, 0)); + protection_flags, base_map_flags, -1, 0)); #else const auto virtual_size = system_managed_size + system_reserved_size + user_size; - const auto virtual_base = reinterpret_cast( - mmap(reinterpret_cast(SYSTEM_MANAGED_MIN), virtual_size, PROT_READ | PROT_WRITE, - MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE | MAP_FIXED, -1, 0)); + const auto virtual_base = + reinterpret_cast(mmap(reinterpret_cast(SYSTEM_MANAGED_MIN), virtual_size, + protection_flags, base_map_flags | MAP_FIXED, -1, 0)); system_managed_base = virtual_base; system_managed_base = virtual_base + (SYSTEM_RESERVED_MIN - SYSTEM_MANAGED_MIN); user_base = virtual_base + (USER_MIN - SYSTEM_MANAGED_MIN); @@ -331,9 +340,6 @@ struct AddressSpace::Impl { #else madvise(virtual_base, virtual_size, MADV_HUGEPAGE); - const VAddr start_addr = reinterpret_cast(virtual_base); - m_free_regions.insert({start_addr, start_addr + virtual_size}); - backing_fd = memfd_create("BackingDmem", 0); if (backing_fd < 0) { LOG_CRITICAL(Kernel_Vmm, "memfd_create failed: {}", strerror(errno)); diff --git a/src/core/cpu_patches.cpp b/src/core/cpu_patches.cpp index 81a775da..eb062f1e 100644 --- a/src/core/cpu_patches.cpp +++ b/src/core/cpu_patches.cpp @@ -16,6 +16,8 @@ #include #endif +using namespace Xbyak::util; + namespace Core { static Xbyak::Reg ZydisToXbyakRegister(const ZydisRegister reg) { @@ -54,7 +56,7 @@ static Xbyak::Address ZydisToXbyakMemoryOperand(const ZydisDecodedOperand& opera expression = expression + operand.mem.disp.value; } - return Xbyak::util::ptr[expression]; + return ptr[expression]; } static std::unique_ptr ZydisToXbyakOperand(const ZydisDecodedOperand& operand) { @@ -125,8 +127,8 @@ static void SaveRegisters(Xbyak::CodeGenerator& c, const std::initializer_list(register_save_slots[index++] * sizeof(void*)); - c.putSeg(Xbyak::util::gs); - c.mov(Xbyak::util::qword[offset], reg.cvt64()); + c.putSeg(gs); + c.mov(qword[offset], reg.cvt64()); } } @@ -141,8 +143,8 @@ static void RestoreRegisters(Xbyak::CodeGenerator& c, for (const auto& reg : regs) { const auto offset = reinterpret_cast(register_save_slots[index++] * sizeof(void*)); - c.putSeg(Xbyak::util::gs); - c.mov(reg.cvt64(), Xbyak::util::qword[offset]); + c.putSeg(gs); + c.mov(reg.cvt64(), qword[offset]); } } @@ -277,20 +279,20 @@ static void GenerateTcbAccess(const ZydisDecodedOperand* operands, Xbyak::CodeGe const u32 tls_index = slot < TlsMinimumAvailable ? slot : slot - TlsMinimumAvailable; // Load the pointer to the table of TLS slots. - c.putSeg(Xbyak::util::gs); - c.mov(dst, Xbyak::util::ptr[reinterpret_cast(teb_offset)]); + c.putSeg(gs); + c.mov(dst, ptr[reinterpret_cast(teb_offset)]); // Load the pointer to our buffer. - c.mov(dst, Xbyak::util::qword[dst + tls_index * sizeof(LPVOID)]); + c.mov(dst, qword[dst + tls_index * sizeof(LPVOID)]); #elif defined(__APPLE__) // The following logic is based on the Darwin implementation of _os_tsd_get_direct, used by // pthread_getspecific https://github.com/apple/darwin-xnu/blob/main/libsyscall/os/tsd.h#L89-L96 - c.putSeg(Xbyak::util::gs); - c.mov(dst, Xbyak::util::qword[reinterpret_cast(slot * sizeof(void*))]); + c.putSeg(gs); + c.mov(dst, qword[reinterpret_cast(slot * sizeof(void*))]); #else const auto src = ZydisToXbyakMemoryOperand(operands[1]); // Replace fs read with gs read. - c.putSeg(Xbyak::util::gs); + c.putSeg(gs); c.mov(dst, src); #endif }