From e58748fd802dc069e90928d12d4db9ff994a869d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 26 Nov 2019 13:46:41 -0500 Subject: [PATCH] core/memory: Migrate over address checking functions to the new Memory class A fairly straightforward migration. These member functions can just be mostly moved verbatim with minor changes. We already have the necessary plumbing in places that they're used. IsKernelVirtualAddress() can remain a non-member function, since it doesn't rely on class state in any form. --- src/core/gdbstub/gdbstub.cpp | 18 +++++---- src/core/hle/kernel/address_arbiter.cpp | 8 ++-- src/core/hle/kernel/svc.cpp | 4 +- src/core/hle/kernel/thread.cpp | 4 +- src/core/memory.cpp | 51 +++++++++++++++---------- src/core/memory.h | 24 ++++++++++-- 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/src/core/gdbstub/gdbstub.cpp b/src/core/gdbstub/gdbstub.cpp index 54ed680db9..78e44f3bdb 100644 --- a/src/core/gdbstub/gdbstub.cpp +++ b/src/core/gdbstub/gdbstub.cpp @@ -969,7 +969,8 @@ static void ReadMemory() { SendReply("E01"); } - if (!Memory::IsValidVirtualAddress(addr)) { + const auto& memory = Core::System::GetInstance().Memory(); + if (!memory.IsValidVirtualAddress(addr)) { return SendReply("E00"); } @@ -984,22 +985,23 @@ static void ReadMemory() { /// Modify location in memory with data received from the gdb client. static void WriteMemory() { auto start_offset = command_buffer + 1; - auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); - VAddr addr = HexToLong(start_offset, static_cast(addr_pos - start_offset)); + const auto addr_pos = std::find(start_offset, command_buffer + command_length, ','); + const VAddr addr = HexToLong(start_offset, static_cast(addr_pos - start_offset)); start_offset = addr_pos + 1; - auto len_pos = std::find(start_offset, command_buffer + command_length, ':'); - u64 len = HexToLong(start_offset, static_cast(len_pos - start_offset)); + const auto len_pos = std::find(start_offset, command_buffer + command_length, ':'); + const u64 len = HexToLong(start_offset, static_cast(len_pos - start_offset)); - if (!Memory::IsValidVirtualAddress(addr)) { + auto& system = Core::System::GetInstance(); + const auto& memory = system.Memory(); + if (!memory.IsValidVirtualAddress(addr)) { return SendReply("E00"); } std::vector data(len); - GdbHexToMem(data.data(), len_pos + 1, len); Memory::WriteBlock(addr, data.data(), len); - Core::System::GetInstance().InvalidateCpuInstructionCaches(); + system.InvalidateCpuInstructionCaches(); SendReply("OK"); } diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp index 4859954cbd..7f9a559d2f 100644 --- a/src/core/hle/kernel/address_arbiter.cpp +++ b/src/core/hle/kernel/address_arbiter.cpp @@ -68,7 +68,7 @@ ResultCode AddressArbiter::SignalToAddressOnly(VAddr address, s32 num_to_wake) { ResultCode AddressArbiter::IncrementAndSignalToAddressIfEqual(VAddr address, s32 value, s32 num_to_wake) { // Ensure that we can write to the address. - if (!Memory::IsValidVirtualAddress(address)) { + if (!system.Memory().IsValidVirtualAddress(address)) { return ERR_INVALID_ADDRESS_STATE; } @@ -83,7 +83,7 @@ ResultCode AddressArbiter::IncrementAndSignalToAddressIfEqual(VAddr address, s32 ResultCode AddressArbiter::ModifyByWaitingCountAndSignalToAddressIfEqual(VAddr address, s32 value, s32 num_to_wake) { // Ensure that we can write to the address. - if (!Memory::IsValidVirtualAddress(address)) { + if (!system.Memory().IsValidVirtualAddress(address)) { return ERR_INVALID_ADDRESS_STATE; } @@ -135,7 +135,7 @@ ResultCode AddressArbiter::WaitForAddress(VAddr address, ArbitrationType type, s ResultCode AddressArbiter::WaitForAddressIfLessThan(VAddr address, s32 value, s64 timeout, bool should_decrement) { // Ensure that we can read the address. - if (!Memory::IsValidVirtualAddress(address)) { + if (!system.Memory().IsValidVirtualAddress(address)) { return ERR_INVALID_ADDRESS_STATE; } @@ -158,7 +158,7 @@ ResultCode AddressArbiter::WaitForAddressIfLessThan(VAddr address, s32 value, s6 ResultCode AddressArbiter::WaitForAddressIfEqual(VAddr address, s32 value, s64 timeout) { // Ensure that we can read the address. - if (!Memory::IsValidVirtualAddress(address)) { + if (!system.Memory().IsValidVirtualAddress(address)) { return ERR_INVALID_ADDRESS_STATE; } // Only wait for the address if equal. diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 9928b3a26d..eddafaf601 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -332,7 +332,7 @@ static ResultCode UnmapMemory(Core::System& system, VAddr dst_addr, VAddr src_ad /// Connect to an OS service given the port name, returns the handle to the port to out static ResultCode ConnectToNamedPort(Core::System& system, Handle* out_handle, VAddr port_name_address) { - if (!Memory::IsValidVirtualAddress(port_name_address)) { + if (!system.Memory().IsValidVirtualAddress(port_name_address)) { LOG_ERROR(Kernel_SVC, "Port Name Address is not a valid virtual address, port_name_address=0x{:016X}", port_name_address); @@ -452,7 +452,7 @@ static ResultCode WaitSynchronization(Core::System& system, Handle* index, VAddr LOG_TRACE(Kernel_SVC, "called handles_address=0x{:X}, handle_count={}, nano_seconds={}", handles_address, handle_count, nano_seconds); - if (!Memory::IsValidVirtualAddress(handles_address)) { + if (!system.Memory().IsValidVirtualAddress(handles_address)) { LOG_ERROR(Kernel_SVC, "Handle address is not a valid virtual address, handle_address=0x{:016X}", handles_address); diff --git a/src/core/hle/kernel/thread.cpp b/src/core/hle/kernel/thread.cpp index 735019d962..e84e5ce0d0 100644 --- a/src/core/hle/kernel/thread.cpp +++ b/src/core/hle/kernel/thread.cpp @@ -162,13 +162,13 @@ ResultVal> Thread::Create(KernelCore& kernel, std::strin return ERR_INVALID_PROCESSOR_ID; } - if (!Memory::IsValidVirtualAddress(owner_process, entry_point)) { + auto& system = Core::System::GetInstance(); + if (!system.Memory().IsValidVirtualAddress(owner_process, entry_point)) { LOG_ERROR(Kernel_SVC, "(name={}): invalid entry {:016X}", name, entry_point); // TODO (bunnei): Find the correct error code to use here return RESULT_UNKNOWN; } - auto& system = Core::System::GetInstance(); std::shared_ptr thread = std::make_shared(kernel); thread->thread_id = kernel.CreateNewThreadID(); diff --git a/src/core/memory.cpp b/src/core/memory.cpp index 28b65ca5eb..4c13ea1e7a 100644 --- a/src/core/memory.cpp +++ b/src/core/memory.cpp @@ -75,6 +75,29 @@ struct Memory::Impl { std::make_pair(interval, std::set{region})); } + bool IsValidVirtualAddress(const Kernel::Process& process, const VAddr vaddr) const { + const auto& page_table = process.VMManager().page_table; + + const u8* const page_pointer = page_table.pointers[vaddr >> PAGE_BITS]; + if (page_pointer != nullptr) { + return true; + } + + if (page_table.attributes[vaddr >> PAGE_BITS] == Common::PageType::RasterizerCachedMemory) { + return true; + } + + if (page_table.attributes[vaddr >> PAGE_BITS] != Common::PageType::Special) { + return false; + } + + return false; + } + + bool IsValidVirtualAddress(VAddr vaddr) const { + return IsValidVirtualAddress(*system.CurrentProcess(), vaddr); + } + /** * Maps a region of pages as a specific type. * @@ -148,6 +171,14 @@ void Memory::RemoveDebugHook(Common::PageTable& page_table, VAddr base, u64 size impl->RemoveDebugHook(page_table, base, size, std::move(hook)); } +bool Memory::IsValidVirtualAddress(const Kernel::Process& process, const VAddr vaddr) const { + return impl->IsValidVirtualAddress(process, vaddr); +} + +bool Memory::IsValidVirtualAddress(const VAddr vaddr) const { + return impl->IsValidVirtualAddress(vaddr); +} + void SetCurrentPageTable(Kernel::Process& process) { current_page_table = &process.VMManager().page_table; @@ -256,26 +287,6 @@ void Write(const VAddr vaddr, const T data) { } } -bool IsValidVirtualAddress(const Kernel::Process& process, const VAddr vaddr) { - const auto& page_table = process.VMManager().page_table; - - const u8* page_pointer = page_table.pointers[vaddr >> PAGE_BITS]; - if (page_pointer) - return true; - - if (page_table.attributes[vaddr >> PAGE_BITS] == Common::PageType::RasterizerCachedMemory) - return true; - - if (page_table.attributes[vaddr >> PAGE_BITS] != Common::PageType::Special) - return false; - - return false; -} - -bool IsValidVirtualAddress(const VAddr vaddr) { - return IsValidVirtualAddress(*Core::System::GetInstance().CurrentProcess(), vaddr); -} - bool IsKernelVirtualAddress(const VAddr vaddr) { return KERNEL_REGION_VADDR <= vaddr && vaddr < KERNEL_REGION_END; } diff --git a/src/core/memory.h b/src/core/memory.h index 87ed3b6960..cacf4fb1a6 100644 --- a/src/core/memory.h +++ b/src/core/memory.h @@ -111,6 +111,27 @@ public: void RemoveDebugHook(Common::PageTable& page_table, VAddr base, u64 size, Common::MemoryHookPointer hook); + /** + * Checks whether or not the supplied address is a valid virtual + * address for the given process. + * + * @param process The emulated process to check the address against. + * @param vaddr The virtual address to check the validity of. + * + * @returns True if the given virtual address is valid, false otherwise. + */ + bool IsValidVirtualAddress(const Kernel::Process& process, VAddr vaddr) const; + + /** + * Checks whether or not the supplied address is a valid virtual + * address for the current process. + * + * @param vaddr The virtual address to check the validity of. + * + * @returns True if the given virtual address is valid, false otherwise. + */ + bool IsValidVirtualAddress(VAddr vaddr) const; + private: struct Impl; std::unique_ptr impl; @@ -120,9 +141,6 @@ private: /// the given process instance. void SetCurrentPageTable(Kernel::Process& process); -/// Determines if the given VAddr is valid for the specified process. -bool IsValidVirtualAddress(const Kernel::Process& process, VAddr vaddr); -bool IsValidVirtualAddress(VAddr vaddr); /// Determines if the given VAddr is a kernel address bool IsKernelVirtualAddress(VAddr vaddr);