Merge pull request #5271 from MerryMage/rm-mem-Special

memory: Remove MemoryHook

src/common/CMakeLists.txt

@@ -135,8 +135,6 @@ add_library(common STATIC
     math_util.h
     memory_detect.cpp
     memory_detect.h
-    memory_hook.cpp
-    memory_hook.h
     microprofile.cpp
     microprofile.h
     microprofileui.h

src/common/memory_hook.cpp

@@ -1,11 +0,0 @@
-// Copyright 2018 Citra Emulator Project
-// Licensed under GPLv2 or any later version
-// Refer to the license.txt file included.
-
-#include "common/memory_hook.h"
-
-namespace Common {
-
-MemoryHook::~MemoryHook() = default;
-
-} // namespace Common

src/common/memory_hook.h

@@ -1,47 +0,0 @@
-// Copyright 2016 Citra Emulator Project
-// Licensed under GPLv2 or any later version
-// Refer to the license.txt file included.
-
-#pragma once
-
-#include <memory>
-#include <optional>
-
-#include "common/common_types.h"
-
-namespace Common {
-
-/**
- * Memory hooks have two purposes:
- * 1. To allow reads and writes to a region of memory to be intercepted. This is used to implement
- *    texture forwarding and memory breakpoints for debugging.
- * 2. To allow for the implementation of MMIO devices.
- *
- * A hook may be mapped to multiple regions of memory.
- *
- * If a std::nullopt or false is returned from a function, the read/write request is passed through
- * to the underlying memory region.
- */
-class MemoryHook {
-public:
-    virtual ~MemoryHook();
-
-    virtual std::optional<bool> IsValidAddress(VAddr addr) = 0;
-
-    virtual std::optional<u8> Read8(VAddr addr) = 0;
-    virtual std::optional<u16> Read16(VAddr addr) = 0;
-    virtual std::optional<u32> Read32(VAddr addr) = 0;
-    virtual std::optional<u64> Read64(VAddr addr) = 0;
-
-    virtual bool ReadBlock(VAddr src_addr, void* dest_buffer, std::size_t size) = 0;
-
-    virtual bool Write8(VAddr addr, u8 data) = 0;
-    virtual bool Write16(VAddr addr, u16 data) = 0;
-    virtual bool Write32(VAddr addr, u32 data) = 0;
-    virtual bool Write64(VAddr addr, u64 data) = 0;
-
-    virtual bool WriteBlock(VAddr dest_addr, const void* src_buffer, std::size_t size) = 0;
-};
-
-using MemoryHookPointer = std::shared_ptr<MemoryHook>;
-} // namespace Common

src/common/page_table.h

@@ -8,7 +8,6 @@
 #include <tuple>
 
 #include "common/common_types.h"
-#include "common/memory_hook.h"
 #include "common/virtual_buffer.h"
 
 namespace Common {
@@ -23,23 +22,6 @@ enum class PageType : u8 {
     RasterizerCachedMemory,
 };
 
-struct SpecialRegion {
-    enum class Type {
-        DebugHook,
-        IODevice,
-    } type;
-
-    MemoryHookPointer handler;
-
-    [[nodiscard]] bool operator<(const SpecialRegion& other) const {
-        return std::tie(type, handler) < std::tie(other.type, other.handler);
-    }
-
-    [[nodiscard]] bool operator==(const SpecialRegion& other) const {
-        return std::tie(type, handler) == std::tie(other.type, other.handler);
-    }
-};
-
 /**
  * A (reasonably) fast way of allowing switchable and remappable process address spaces. It loosely
  * mimics the way a real CPU page table works.

src/core/memory.cpp

@@ -44,27 +44,12 @@ struct Memory::Impl {
         MapPages(page_table, base / PAGE_SIZE, size / PAGE_SIZE, target, Common::PageType::Memory);
     }
 
-    void MapIoRegion(Common::PageTable& page_table, VAddr base, u64 size,
-                     Common::MemoryHookPointer mmio_handler) {
-        UNIMPLEMENTED();
-    }
-
     void UnmapRegion(Common::PageTable& page_table, VAddr base, u64 size) {
         ASSERT_MSG((size & PAGE_MASK) == 0, "non-page aligned size: {:016X}", size);
         ASSERT_MSG((base & PAGE_MASK) == 0, "non-page aligned base: {:016X}", base);
         MapPages(page_table, base / PAGE_SIZE, size / PAGE_SIZE, 0, Common::PageType::Unmapped);
     }
 
-    void AddDebugHook(Common::PageTable& page_table, VAddr base, u64 size,
-                      Common::MemoryHookPointer hook) {
-        UNIMPLEMENTED();
-    }
-
-    void RemoveDebugHook(Common::PageTable& page_table, VAddr base, u64 size,
-                         Common::MemoryHookPointer hook) {
-        UNIMPLEMENTED();
-    }
-
     bool IsValidVirtualAddress(const Kernel::Process& process, const VAddr vaddr) const {
         const auto& page_table = process.PageTable().PageTableImpl();
         const auto [pointer, type] = page_table.pointers[vaddr >> PAGE_BITS].PointerType();
@@ -740,25 +725,10 @@ void Memory::MapMemoryRegion(Common::PageTable& page_table, VAddr base, u64 size
     impl->MapMemoryRegion(page_table, base, size, target);
 }
 
-void Memory::MapIoRegion(Common::PageTable& page_table, VAddr base, u64 size,
-                         Common::MemoryHookPointer mmio_handler) {
-    impl->MapIoRegion(page_table, base, size, std::move(mmio_handler));
-}
-
 void Memory::UnmapRegion(Common::PageTable& page_table, VAddr base, u64 size) {
     impl->UnmapRegion(page_table, base, size);
 }
 
-void Memory::AddDebugHook(Common::PageTable& page_table, VAddr base, u64 size,
-                          Common::MemoryHookPointer hook) {
-    impl->AddDebugHook(page_table, base, size, std::move(hook));
-}
-
-void Memory::RemoveDebugHook(Common::PageTable& page_table, VAddr base, u64 size,
-                             Common::MemoryHookPointer hook) {
-    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);
 }

src/core/memory.h

@@ -8,7 +8,6 @@
 #include <memory>
 #include <string>
 #include "common/common_types.h"
-#include "common/memory_hook.h"
 
 namespace Common {
 struct PageTable;
@@ -77,17 +76,6 @@ public:
      */
     void MapMemoryRegion(Common::PageTable& page_table, VAddr base, u64 size, PAddr target);
 
-    /**
-     * Maps a region of the emulated process address space as a IO region.
-     *
-     * @param page_table   The page table of the emulated process.
-     * @param base         The address to start mapping at. Must be page-aligned.
-     * @param size         The amount of bytes to map. Must be page-aligned.
-     * @param mmio_handler The handler that backs the mapping.
-     */
-    void MapIoRegion(Common::PageTable& page_table, VAddr base, u64 size,
-                     Common::MemoryHookPointer mmio_handler);
-
     /**
      * Unmaps a region of the emulated process address space.
      *
@@ -97,28 +85,6 @@ public:
      */
     void UnmapRegion(Common::PageTable& page_table, VAddr base, u64 size);
 
-    /**
-     * Adds a memory hook to intercept reads and writes to given region of memory.
-     *
-     * @param page_table The page table of the emulated process
-     * @param base       The starting address to apply the hook to.
-     * @param size       The size of the memory region to apply the hook to, in bytes.
-     * @param hook       The hook to apply to the region of memory.
-     */
-    void AddDebugHook(Common::PageTable& page_table, VAddr base, u64 size,
-                      Common::MemoryHookPointer hook);
-
-    /**
-     * Removes a memory hook from a given range of memory.
-     *
-     * @param page_table The page table of the emulated process.
-     * @param base       The starting address to remove the hook from.
-     * @param size       The size of the memory region to remove the hook from, in bytes.
-     * @param hook       The hook to remove from the specified region of memory.
-     */
-    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.

src/tests/CMakeLists.txt

@@ -4,8 +4,6 @@ add_executable(tests
     common/fibers.cpp
     common/param_package.cpp
     common/ring_buffer.cpp
-    core/arm/arm_test_common.cpp
-    core/arm/arm_test_common.h
     core/core_timing.cpp
     tests.cpp
 )

src/tests/core/arm/arm_test_common.cpp

@@ -1,145 +0,0 @@
-// Copyright 2016 Citra Emulator Project
-// Licensed under GPLv2 or any later version
-// Refer to the license.txt file included.
-
-#include <algorithm>
-
-#include "common/page_table.h"
-#include "core/core.h"
-#include "core/hle/kernel/memory/page_table.h"
-#include "core/hle/kernel/process.h"
-#include "core/memory.h"
-#include "tests/core/arm/arm_test_common.h"
-
-namespace ArmTests {
-
-TestEnvironment::TestEnvironment(bool mutable_memory_)
-    : mutable_memory(mutable_memory_),
-      test_memory(std::make_shared<TestMemory>(this)), kernel{Core::System::GetInstance()} {
-    auto& system = Core::System::GetInstance();
-
-    auto process = Kernel::Process::Create(system, "", Kernel::Process::ProcessType::Userland);
-    page_table = &process->PageTable().PageTableImpl();
-
-    system.Memory().MapIoRegion(*page_table, 0x00000000, 0x80000000, test_memory);
-    system.Memory().MapIoRegion(*page_table, 0x80000000, 0x80000000, test_memory);
-
-    kernel.MakeCurrentProcess(process.get());
-}
-
-TestEnvironment::~TestEnvironment() {
-    auto& system = Core::System::GetInstance();
-    system.Memory().UnmapRegion(*page_table, 0x80000000, 0x80000000);
-    system.Memory().UnmapRegion(*page_table, 0x00000000, 0x80000000);
-}
-
-void TestEnvironment::SetMemory64(VAddr vaddr, u64 value) {
-    SetMemory32(vaddr + 0, static_cast<u32>(value));
-    SetMemory32(vaddr + 4, static_cast<u32>(value >> 32));
-}
-
-void TestEnvironment::SetMemory32(VAddr vaddr, u32 value) {
-    SetMemory16(vaddr + 0, static_cast<u16>(value));
-    SetMemory16(vaddr + 2, static_cast<u16>(value >> 16));
-}
-
-void TestEnvironment::SetMemory16(VAddr vaddr, u16 value) {
-    SetMemory8(vaddr + 0, static_cast<u8>(value));
-    SetMemory8(vaddr + 1, static_cast<u8>(value >> 8));
-}
-
-void TestEnvironment::SetMemory8(VAddr vaddr, u8 value) {
-    test_memory->data[vaddr] = value;
-}
-
-std::vector<WriteRecord> TestEnvironment::GetWriteRecords() const {
-    return write_records;
-}
-
-void TestEnvironment::ClearWriteRecords() {
-    write_records.clear();
-}
-
-TestEnvironment::TestMemory::~TestMemory() {}
-
-std::optional<bool> TestEnvironment::TestMemory::IsValidAddress(VAddr addr) {
-    return true;
-}
-
-std::optional<u8> TestEnvironment::TestMemory::Read8(VAddr addr) {
-    const auto iter = data.find(addr);
-
-    if (iter == data.end()) {
-        // Some arbitrary data
-        return static_cast<u8>(addr);
-    }
-
-    return iter->second;
-}
-
-std::optional<u16> TestEnvironment::TestMemory::Read16(VAddr addr) {
-    return *Read8(addr) | static_cast<u16>(*Read8(addr + 1)) << 8;
-}
-
-std::optional<u32> TestEnvironment::TestMemory::Read32(VAddr addr) {
-    return *Read16(addr) | static_cast<u32>(*Read16(addr + 2)) << 16;
-}
-
-std::optional<u64> TestEnvironment::TestMemory::Read64(VAddr addr) {
-    return *Read32(addr) | static_cast<u64>(*Read32(addr + 4)) << 32;
-}
-
-bool TestEnvironment::TestMemory::ReadBlock(VAddr src_addr, void* dest_buffer, std::size_t size) {
-    VAddr addr = src_addr;
-    u8* data = static_cast<u8*>(dest_buffer);
-
-    for (std::size_t i = 0; i < size; i++, addr++, data++) {
-        *data = *Read8(addr);
-    }
-
-    return true;
-}
-
-bool TestEnvironment::TestMemory::Write8(VAddr addr, u8 data) {
-    env->write_records.emplace_back(8, addr, data);
-    if (env->mutable_memory)
-        env->SetMemory8(addr, data);
-    return true;
-}
-
-bool TestEnvironment::TestMemory::Write16(VAddr addr, u16 data) {
-    env->write_records.emplace_back(16, addr, data);
-    if (env->mutable_memory)
-        env->SetMemory16(addr, data);
-    return true;
-}
-
-bool TestEnvironment::TestMemory::Write32(VAddr addr, u32 data) {
-    env->write_records.emplace_back(32, addr, data);
-    if (env->mutable_memory)
-        env->SetMemory32(addr, data);
-    return true;
-}
-
-bool TestEnvironment::TestMemory::Write64(VAddr addr, u64 data) {
-    env->write_records.emplace_back(64, addr, data);
-    if (env->mutable_memory)
-        env->SetMemory64(addr, data);
-    return true;
-}
-
-bool TestEnvironment::TestMemory::WriteBlock(VAddr dest_addr, const void* src_buffer,
-                                             std::size_t size) {
-    VAddr addr = dest_addr;
-    const u8* data = static_cast<const u8*>(src_buffer);
-
-    for (std::size_t i = 0; i < size; i++, addr++, data++) {
-        env->write_records.emplace_back(8, addr, *data);
-        if (env->mutable_memory)
-            env->SetMemory8(addr, *data);
-    }
-
-    return true;
-}
-
-} // namespace ArmTests

src/tests/core/arm/arm_test_common.h

@@ -1,93 +0,0 @@
-// Copyright 2016 Citra Emulator Project
-// Licensed under GPLv2 or any later version
-// Refer to the license.txt file included.
-
-#pragma once
-
-#include <tuple>
-#include <unordered_map>
-#include <vector>
-
-#include "common/common_types.h"
-#include "common/memory_hook.h"
-#include "core/hle/kernel/kernel.h"
-
-namespace Common {
-struct PageTable;
-}
-
-namespace ArmTests {
-
-struct WriteRecord {
-    WriteRecord(std::size_t size, VAddr addr, u64 data) : size(size), addr(addr), data(data) {}
-    std::size_t size;
-    VAddr addr;
-    u64 data;
-    bool operator==(const WriteRecord& o) const {
-        return std::tie(size, addr, data) == std::tie(o.size, o.addr, o.data);
-    }
-};
-
-class TestEnvironment final {
-public:
-    /*
-     * Inititalise test environment
-     * @param mutable_memory If false, writes to memory can never be read back.
-     *                       (Memory is immutable.)
-     */
-    explicit TestEnvironment(bool mutable_memory = false);
-
-    /// Shutdown test environment
-    ~TestEnvironment();
-
-    /// Sets value at memory location vaddr.
-    void SetMemory8(VAddr vaddr, u8 value);
-    void SetMemory16(VAddr vaddr, u16 value);
-    void SetMemory32(VAddr vaddr, u32 value);
-    void SetMemory64(VAddr vaddr, u64 value);
-
-    /**
-     * Whenever Memory::Write{8,16,32,64} is called within the test environment,
-     * a new write-record is made.
-     * @returns A vector of write records made since they were last cleared.
-     */
-    std::vector<WriteRecord> GetWriteRecords() const;
-
-    /// Empties the internal write-record store.
-    void ClearWriteRecords();
-
-private:
-    friend struct TestMemory;
-    struct TestMemory final : Common::MemoryHook {
-        explicit TestMemory(TestEnvironment* env_) : env(env_) {}
-        TestEnvironment* env;
-
-        ~TestMemory() override;
-
-        std::optional<bool> IsValidAddress(VAddr addr) override;
-
-        std::optional<u8> Read8(VAddr addr) override;
-        std::optional<u16> Read16(VAddr addr) override;
-        std::optional<u32> Read32(VAddr addr) override;
-        std::optional<u64> Read64(VAddr addr) override;
-
-        bool ReadBlock(VAddr src_addr, void* dest_buffer, std::size_t size) override;
-
-        bool Write8(VAddr addr, u8 data) override;
-        bool Write16(VAddr addr, u16 data) override;
-        bool Write32(VAddr addr, u32 data) override;
-        bool Write64(VAddr addr, u64 data) override;
-
-        bool WriteBlock(VAddr dest_addr, const void* src_buffer, std::size_t size) override;
-
-        std::unordered_map<VAddr, u8> data;
-    };
-
-    bool mutable_memory;
-    std::shared_ptr<TestMemory> test_memory;
-    std::vector<WriteRecord> write_records;
-    Common::PageTable* page_table = nullptr;
-    Kernel::KernelCore kernel;
-};
-
-} // namespace ArmTests