From 64f6e5e597f621f09bef271479d9004b49335fde Mon Sep 17 00:00:00 2001 From: Weiyi Wang Date: Fri, 1 Feb 2019 14:50:25 -0500 Subject: [PATCH] ARM: pass MemorySystem separately in the constructor and make System optional So that unit test can test CPU without constructing the entire system. Also remove hacks in the System class --- src/core/arm/dynarmic/arm_dynarmic.cpp | 13 +++++----- src/core/arm/dynarmic/arm_dynarmic.h | 4 ++- src/core/arm/dyncom/arm_dyncom.cpp | 13 +++++++--- src/core/arm/dyncom/arm_dyncom.h | 9 +++++-- .../arm/dyncom/arm_dyncom_interpreter.cpp | 7 +++--- src/core/arm/skyeye_common/armstate.cpp | 25 +++++++++++-------- src/core/arm/skyeye_common/armstate.h | 10 ++++++-- src/core/core.cpp | 6 ++--- src/core/core.h | 2 -- src/tests/core/arm/arm_test_common.cpp | 24 ++++++------------ src/tests/core/arm/arm_test_common.h | 8 +++++- .../core/arm/dyncom/arm_dyncom_vfp_tests.cpp | 2 +- src/tests/core/hle/kernel/hle_ipc.cpp | 14 +++++------ src/tests/core/memory/memory.cpp | 8 +++--- 14 files changed, 80 insertions(+), 65 deletions(-) diff --git a/src/core/arm/dynarmic/arm_dynarmic.cpp b/src/core/arm/dynarmic/arm_dynarmic.cpp index 62c63f9ac..f494b5228 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.cpp +++ b/src/core/arm/dynarmic/arm_dynarmic.cpp @@ -73,7 +73,7 @@ class DynarmicUserCallbacks final : public Dynarmic::A32::UserCallbacks { public: explicit DynarmicUserCallbacks(ARM_Dynarmic& parent) : parent(parent), timing(parent.system.CoreTiming()), svc_context(parent.system), - memory(parent.system.Memory()) {} + memory(parent.memory) {} ~DynarmicUserCallbacks() = default; std::uint8_t MemoryRead8(VAddr vaddr) override { @@ -163,9 +163,10 @@ public: Memory::MemorySystem& memory; }; -ARM_Dynarmic::ARM_Dynarmic(Core::System& system, PrivilegeMode initial_mode) - : system(system), cb(std::make_unique(*this)) { - interpreter_state = std::make_shared(system, initial_mode); +ARM_Dynarmic::ARM_Dynarmic(Core::System* system, Memory::MemorySystem& memory, + PrivilegeMode initial_mode) + : system(*system), memory(memory), cb(std::make_unique(*this)) { + interpreter_state = std::make_shared(system, memory, initial_mode); PageTableChanged(); } @@ -174,7 +175,7 @@ ARM_Dynarmic::~ARM_Dynarmic() = default; MICROPROFILE_DEFINE(ARM_Jit, "ARM JIT", "ARM JIT", MP_RGB(255, 64, 64)); void ARM_Dynarmic::Run() { - ASSERT(system.Memory().GetCurrentPageTable() == current_page_table); + ASSERT(memory.GetCurrentPageTable() == current_page_table); MICROPROFILE_SCOPE(ARM_Jit); jit->Run(); @@ -281,7 +282,7 @@ void ARM_Dynarmic::InvalidateCacheRange(u32 start_address, std::size_t length) { } void ARM_Dynarmic::PageTableChanged() { - current_page_table = system.Memory().GetCurrentPageTable(); + current_page_table = memory.GetCurrentPageTable(); auto iter = jits.find(current_page_table); if (iter != jits.end()) { diff --git a/src/core/arm/dynarmic/arm_dynarmic.h b/src/core/arm/dynarmic/arm_dynarmic.h index 0a73c05ac..537784e08 100644 --- a/src/core/arm/dynarmic/arm_dynarmic.h +++ b/src/core/arm/dynarmic/arm_dynarmic.h @@ -13,6 +13,7 @@ namespace Memory { struct PageTable; +class MemorySystem; } // namespace Memory namespace Core { @@ -23,7 +24,7 @@ class DynarmicUserCallbacks; class ARM_Dynarmic final : public ARM_Interface { public: - ARM_Dynarmic(Core::System& system, PrivilegeMode initial_mode); + ARM_Dynarmic(Core::System* system, Memory::MemorySystem& memory, PrivilegeMode initial_mode); ~ARM_Dynarmic(); void Run() override; @@ -55,6 +56,7 @@ public: private: friend class DynarmicUserCallbacks; Core::System& system; + Memory::MemorySystem& memory; std::unique_ptr cb; std::unique_ptr MakeJit(); diff --git a/src/core/arm/dyncom/arm_dyncom.cpp b/src/core/arm/dyncom/arm_dyncom.cpp index 4ee22dfdd..d54b0cb95 100644 --- a/src/core/arm/dyncom/arm_dyncom.cpp +++ b/src/core/arm/dyncom/arm_dyncom.cpp @@ -68,14 +68,17 @@ private: u32 fpexc; }; -ARM_DynCom::ARM_DynCom(Core::System& system, PrivilegeMode initial_mode) : system(system) { - state = std::make_unique(system, initial_mode); +ARM_DynCom::ARM_DynCom(Core::System* system, Memory::MemorySystem& memory, + PrivilegeMode initial_mode) + : system(system) { + state = std::make_unique(system, memory, initial_mode); } ARM_DynCom::~ARM_DynCom() {} void ARM_DynCom::Run() { - ExecuteInstructions(std::max(system.CoreTiming().GetDowncount(), 0)); + DEBUG_ASSERT(system != nullptr); + ExecuteInstructions(std::max(system->CoreTiming().GetDowncount(), 0)); } void ARM_DynCom::Step() { @@ -146,7 +149,9 @@ void ARM_DynCom::SetCP15Register(CP15Register reg, u32 value) { void ARM_DynCom::ExecuteInstructions(u64 num_instructions) { state->NumInstrsToExecute = num_instructions; unsigned ticks_executed = InterpreterMainLoop(state.get()); - system.CoreTiming().AddTicks(ticks_executed); + if (system != nullptr) { + system->CoreTiming().AddTicks(ticks_executed); + } state->ServeBreak(); } diff --git a/src/core/arm/dyncom/arm_dyncom.h b/src/core/arm/dyncom/arm_dyncom.h index 83366e09d..16c332d3b 100644 --- a/src/core/arm/dyncom/arm_dyncom.h +++ b/src/core/arm/dyncom/arm_dyncom.h @@ -14,9 +14,14 @@ namespace Core { struct System; } +namespace Memory { +class MemorySystem; +} + class ARM_DynCom final : public ARM_Interface { public: - explicit ARM_DynCom(Core::System& system, PrivilegeMode initial_mode); + explicit ARM_DynCom(Core::System* system, Memory::MemorySystem& memory, + PrivilegeMode initial_mode); ~ARM_DynCom(); void Run() override; @@ -48,6 +53,6 @@ public: private: void ExecuteInstructions(u64 num_instructions); - Core::System& system; + Core::System* system; std::unique_ptr state; }; diff --git a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp index 0a50d31b6..9de8dddab 100644 --- a/src/core/arm/dyncom/arm_dyncom_interpreter.cpp +++ b/src/core/arm/dyncom/arm_dyncom_interpreter.cpp @@ -811,7 +811,7 @@ MICROPROFILE_DEFINE(DynCom_Decode, "DynCom", "Decode", MP_RGB(255, 64, 64)); static unsigned int InterpreterTranslateInstruction(const ARMul_State* cpu, const u32 phys_addr, ARM_INST_PTR& inst_base) { u32 inst_size = 4; - u32 inst = cpu->system.Memory().Read32(phys_addr & 0xFFFFFFFC); + u32 inst = cpu->memory.Read32(phys_addr & 0xFFFFFFFC); // If we are in Thumb mode, we'll translate one Thumb instruction to the corresponding ARM // instruction @@ -3859,12 +3859,13 @@ SUB_INST : { } SWI_INST : { if (inst_base->cond == ConditionCode::AL || CondPassed(cpu, inst_base->cond)) { + DEBUG_ASSERT(cpu->system != nullptr); swi_inst* const inst_cream = (swi_inst*)inst_base->component; - cpu->system.CoreTiming().AddTicks(num_instrs); + cpu->system->CoreTiming().AddTicks(num_instrs); cpu->NumInstrsToExecute = num_instrs >= cpu->NumInstrsToExecute ? 0 : cpu->NumInstrsToExecute - num_instrs; num_instrs = 0; - Kernel::SVCContext{cpu->system}.CallSVC(inst_cream->num & 0xFFFF); + Kernel::SVCContext{*cpu->system}.CallSVC(inst_cream->num & 0xFFFF); // The kernel would call ERET to get here, which clears exclusive memory state. cpu->UnsetExclusiveMemoryAddress(); } diff --git a/src/core/arm/skyeye_common/armstate.cpp b/src/core/arm/skyeye_common/armstate.cpp index 27a7540cc..3bd4d28f1 100644 --- a/src/core/arm/skyeye_common/armstate.cpp +++ b/src/core/arm/skyeye_common/armstate.cpp @@ -10,7 +10,9 @@ #include "core/core.h" #include "core/memory.h" -ARMul_State::ARMul_State(Core::System& system, PrivilegeMode initial_mode) : system(system) { +ARMul_State::ARMul_State(Core::System* system, Memory::MemorySystem& memory, + PrivilegeMode initial_mode) + : system(system), memory(memory) { Reset(); ChangePrivilegeMode(initial_mode); } @@ -191,13 +193,13 @@ static void CheckMemoryBreakpoint(u32 address, GDBStub::BreakpointType type) { u8 ARMul_State::ReadMemory8(u32 address) const { CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - return system.Memory().Read8(address); + return memory.Read8(address); } u16 ARMul_State::ReadMemory16(u32 address) const { CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - u16 data = system.Memory().Read16(address); + u16 data = memory.Read16(address); if (InBigEndianMode()) data = Common::swap16(data); @@ -208,7 +210,7 @@ u16 ARMul_State::ReadMemory16(u32 address) const { u32 ARMul_State::ReadMemory32(u32 address) const { CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - u32 data = system.Memory().Read32(address); + u32 data = memory.Read32(address); if (InBigEndianMode()) data = Common::swap32(data); @@ -219,7 +221,7 @@ u32 ARMul_State::ReadMemory32(u32 address) const { u64 ARMul_State::ReadMemory64(u32 address) const { CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Read); - u64 data = system.Memory().Read64(address); + u64 data = memory.Read64(address); if (InBigEndianMode()) data = Common::swap64(data); @@ -230,7 +232,7 @@ u64 ARMul_State::ReadMemory64(u32 address) const { void ARMul_State::WriteMemory8(u32 address, u8 data) { CheckMemoryBreakpoint(address, GDBStub::BreakpointType::Write); - system.Memory().Write8(address, data); + memory.Write8(address, data); } void ARMul_State::WriteMemory16(u32 address, u16 data) { @@ -239,7 +241,7 @@ void ARMul_State::WriteMemory16(u32 address, u16 data) { if (InBigEndianMode()) data = Common::swap16(data); - system.Memory().Write16(address, data); + memory.Write16(address, data); } void ARMul_State::WriteMemory32(u32 address, u32 data) { @@ -248,7 +250,7 @@ void ARMul_State::WriteMemory32(u32 address, u32 data) { if (InBigEndianMode()) data = Common::swap32(data); - system.Memory().Write32(address, data); + memory.Write32(address, data); } void ARMul_State::WriteMemory64(u32 address, u64 data) { @@ -257,7 +259,7 @@ void ARMul_State::WriteMemory64(u32 address, u64 data) { if (InBigEndianMode()) data = Common::swap64(data); - system.Memory().Write64(address, data); + memory.Write64(address, data); } // Reads from the CP15 registers. Used with implementation of the MRC instruction. @@ -603,8 +605,9 @@ void ARMul_State::ServeBreak() { if (last_bkpt_hit) { Reg[15] = last_bkpt.address; } - Kernel::Thread* thread = system.Kernel().GetThreadManager().GetCurrentThread(); - system.CPU().SaveContext(thread->context); + DEBUG_ASSERT(system != nullptr); + Kernel::Thread* thread = system->Kernel().GetThreadManager().GetCurrentThread(); + system->CPU().SaveContext(thread->context); if (last_bkpt_hit || GDBStub::GetCpuStepFlag()) { last_bkpt_hit = false; GDBStub::Break(); diff --git a/src/core/arm/skyeye_common/armstate.h b/src/core/arm/skyeye_common/armstate.h index 9327dc88c..eedc68c69 100644 --- a/src/core/arm/skyeye_common/armstate.h +++ b/src/core/arm/skyeye_common/armstate.h @@ -27,6 +27,10 @@ namespace Core { class System; } +namespace Memory { +class MemorySystem; +} + // Signal levels enum { LOW = 0, HIGH = 1, LOWHIGH = 1, HIGHLOW = 2 }; @@ -143,7 +147,8 @@ enum { struct ARMul_State final { public: - explicit ARMul_State(Core::System& system, PrivilegeMode initial_mode); + explicit ARMul_State(Core::System* system, Memory::MemorySystem& memory, + PrivilegeMode initial_mode); void ChangePrivilegeMode(u32 new_mode); void Reset(); @@ -201,7 +206,8 @@ public: void ServeBreak(); - Core::System& system; + Core::System* system; + Memory::MemorySystem& memory; std::array Reg{}; // The current register file std::array Reg_usr{}; diff --git a/src/core/core.cpp b/src/core/core.cpp index df0ff93bd..9473b5c0c 100644 --- a/src/core/core.cpp +++ b/src/core/core.cpp @@ -179,13 +179,13 @@ System::ResultStatus System::Init(EmuWindow& emu_window, u32 system_mode) { if (Settings::values.use_cpu_jit) { #ifdef ARCHITECTURE_x86_64 - cpu_core = std::make_unique(*this, USER32MODE); + cpu_core = std::make_unique(this, *memory, USER32MODE); #else - cpu_core = std::make_unique(*this, USER32MODE); + cpu_core = std::make_unique(this, *memory, USER32MODE); LOG_WARNING(Core, "CPU JIT requested, but Dynarmic not available"); #endif } else { - cpu_core = std::make_unique(*this, USER32MODE); + cpu_core = std::make_unique(this, *memory, USER32MODE); } kernel->GetThreadManager().SetCPU(*cpu_core); diff --git a/src/core/core.h b/src/core/core.h index b8552fe3b..2e0c9ce7f 100644 --- a/src/core/core.h +++ b/src/core/core.h @@ -270,8 +270,6 @@ private: std::unique_ptr archive_manager; -public: // HACK: this is temporary exposed for tests, - // due to WIP kernel refactor causing desync state in memory std::unique_ptr memory; std::unique_ptr kernel; std::unique_ptr timing; diff --git a/src/tests/core/arm/arm_test_common.cpp b/src/tests/core/arm/arm_test_common.cpp index e1ef67a52..3c8ac9878 100644 --- a/src/tests/core/arm/arm_test_common.cpp +++ b/src/tests/core/arm/arm_test_common.cpp @@ -15,15 +15,9 @@ static Memory::PageTable* page_table = nullptr; TestEnvironment::TestEnvironment(bool mutable_memory_) : mutable_memory(mutable_memory_), test_memory(std::make_shared(this)) { - // HACK: some memory functions are currently referring kernel from the global instance, - // so we need to create the kernel object there. - // Change this when all global states are eliminated. - Core::System::GetInstance().timing = std::make_unique(); - Core::System::GetInstance().memory = std::make_unique(); - Memory::MemorySystem& memory = *Core::System::GetInstance().memory; - Core::System::GetInstance().kernel = std::make_unique( - memory, *Core::System::GetInstance().timing, [] {}, 0); - kernel = Core::System::GetInstance().kernel.get(); + timing = std::make_unique(); + memory = std::make_unique(); + kernel = std::make_unique(*memory, *timing, [] {}, 0); kernel->SetCurrentProcess(kernel->CreateProcess(kernel->CreateCodeSet("", 0))); page_table = &kernel->GetCurrentProcess()->vm_manager.page_table; @@ -31,17 +25,15 @@ TestEnvironment::TestEnvironment(bool mutable_memory_) page_table->pointers.fill(nullptr); page_table->attributes.fill(Memory::PageType::Unmapped); - memory.MapIoRegion(*page_table, 0x00000000, 0x80000000, test_memory); - memory.MapIoRegion(*page_table, 0x80000000, 0x80000000, test_memory); + memory->MapIoRegion(*page_table, 0x00000000, 0x80000000, test_memory); + memory->MapIoRegion(*page_table, 0x80000000, 0x80000000, test_memory); - memory.SetCurrentPageTable(page_table); + memory->SetCurrentPageTable(page_table); } TestEnvironment::~TestEnvironment() { - Memory::MemorySystem& memory = *Core::System::GetInstance().memory; - memory.UnmapRegion(*page_table, 0x80000000, 0x80000000); - memory.UnmapRegion(*page_table, 0x00000000, 0x80000000); - Core::System::GetInstance().kernel.reset(); + memory->UnmapRegion(*page_table, 0x80000000, 0x80000000); + memory->UnmapRegion(*page_table, 0x00000000, 0x80000000); } void TestEnvironment::SetMemory64(VAddr vaddr, u64 value) { diff --git a/src/tests/core/arm/arm_test_common.h b/src/tests/core/arm/arm_test_common.h index b52d26e3c..604880d21 100644 --- a/src/tests/core/arm/arm_test_common.h +++ b/src/tests/core/arm/arm_test_common.h @@ -49,6 +49,10 @@ public: /// Empties the internal write-record store. void ClearWriteRecords(); + Memory::MemorySystem& GetMemory() { + return *memory; + } + private: friend struct TestMemory; struct TestMemory final : Memory::MMIORegion { @@ -80,7 +84,9 @@ private: std::shared_ptr test_memory; std::vector write_records; - Kernel::KernelSystem* kernel; + std::unique_ptr timing; + std::unique_ptr memory; + std::unique_ptr kernel; }; } // namespace ArmTests diff --git a/src/tests/core/arm/dyncom/arm_dyncom_vfp_tests.cpp b/src/tests/core/arm/dyncom/arm_dyncom_vfp_tests.cpp index 6eb1a3b7f..5fadaf85e 100644 --- a/src/tests/core/arm/dyncom/arm_dyncom_vfp_tests.cpp +++ b/src/tests/core/arm/dyncom/arm_dyncom_vfp_tests.cpp @@ -23,7 +23,7 @@ TEST_CASE("ARM_DynCom (vfp): vadd", "[arm_dyncom]") { test_env.SetMemory32(0, 0xEE321A03); // vadd.f32 s2, s4, s6 test_env.SetMemory32(4, 0xEAFFFFFE); // b +#0 - ARM_DynCom dyncom(Core::System::GetInstance(), USER32MODE); + ARM_DynCom dyncom(nullptr, test_env.GetMemory(), USER32MODE); std::vector test_cases{{ #include "vfp_vadd_f32.inc" diff --git a/src/tests/core/hle/kernel/hle_ipc.cpp b/src/tests/core/hle/kernel/hle_ipc.cpp index 3df0a4468..02ba3589b 100644 --- a/src/tests/core/hle/kernel/hle_ipc.cpp +++ b/src/tests/core/hle/kernel/hle_ipc.cpp @@ -21,10 +21,9 @@ static SharedPtr MakeObject(Kernel::KernelSystem& kernel) { } TEST_CASE("HLERequestContext::PopulateFromIncomingCommandBuffer", "[core][kernel]") { - // HACK: see comments of member timing - Core::System::GetInstance().timing = std::make_unique(); - auto memory = std::make_unique(); - Kernel::KernelSystem kernel(*memory, *Core::System::GetInstance().timing, [] {}, 0); + Core::Timing timing; + Memory::MemorySystem memory; + Kernel::KernelSystem kernel(memory, timing, [] {}, 0); auto session = std::get>(kernel.CreateSessionPair()); HLERequestContext context(std::move(session)); @@ -234,10 +233,9 @@ TEST_CASE("HLERequestContext::PopulateFromIncomingCommandBuffer", "[core][kernel } TEST_CASE("HLERequestContext::WriteToOutgoingCommandBuffer", "[core][kernel]") { - // HACK: see comments of member timing - Core::System::GetInstance().timing = std::make_unique(); - auto memory = std::make_unique(); - Kernel::KernelSystem kernel(*memory, *Core::System::GetInstance().timing, [] {}, 0); + Core::Timing timing; + Memory::MemorySystem memory; + Kernel::KernelSystem kernel(memory, timing, [] {}, 0); auto session = std::get>(kernel.CreateSessionPair()); HLERequestContext context(std::move(session)); diff --git a/src/tests/core/memory/memory.cpp b/src/tests/core/memory/memory.cpp index 9ccfe4587..4a6d54bf7 100644 --- a/src/tests/core/memory/memory.cpp +++ b/src/tests/core/memory/memory.cpp @@ -11,11 +11,9 @@ #include "core/memory.h" TEST_CASE("Memory::IsValidVirtualAddress", "[core][memory]") { - // HACK: see comments of member timing - Core::System::GetInstance().timing = std::make_unique(); - Core::System::GetInstance().memory = std::make_unique(); - Kernel::KernelSystem kernel(*Core::System::GetInstance().memory, - *Core::System::GetInstance().timing, [] {}, 0); + Core::Timing timing; + Memory::MemorySystem memory; + Kernel::KernelSystem kernel(memory, timing, [] {}, 0); SECTION("these regions should not be mapped on an empty process") { auto process = kernel.CreateProcess(kernel.CreateCodeSet("", 0)); CHECK(Memory::IsValidVirtualAddress(*process, Memory::PROCESS_IMAGE_VADDR) == false);