Merge pull request #8637 from liamwhite/bad-interrupts

kernel: unlayer CPU interrupt handling

src/core/CMakeLists.txt

@@ -6,8 +6,6 @@ add_library(core STATIC
     announce_multiplayer_session.h
     arm/arm_interface.h
     arm/arm_interface.cpp
-    arm/cpu_interrupt_handler.cpp
-    arm/cpu_interrupt_handler.h
     arm/dynarmic/arm_dynarmic_32.cpp
     arm/dynarmic/arm_dynarmic_32.h
     arm/dynarmic/arm_dynarmic_64.cpp

src/core/arm/arm_interface.h

@@ -27,7 +27,6 @@ namespace Core {
 class System;
 class CPUInterruptHandler;
 
-using CPUInterrupts = std::array<CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES>;
 using WatchpointArray = std::array<Kernel::DebugWatchpoint, Core::Hardware::NUM_WATCHPOINTS>;
 
 /// Generic ARMv8 CPU interface
@@ -36,10 +35,8 @@ public:
     YUZU_NON_COPYABLE(ARM_Interface);
     YUZU_NON_MOVEABLE(ARM_Interface);
 
-    explicit ARM_Interface(System& system_, CPUInterrupts& interrupt_handlers_,
+    explicit ARM_Interface(System& system_, bool uses_wall_clock_)
-                           bool uses_wall_clock_)
+        : system{system_}, uses_wall_clock{uses_wall_clock_} {}
-        : system{system_}, interrupt_handlers{interrupt_handlers_}, uses_wall_clock{
-                                                                        uses_wall_clock_} {}
     virtual ~ARM_Interface() = default;
 
     struct ThreadContext32 {
@@ -181,6 +178,9 @@ public:
     /// Signal an interrupt and ask the core to halt as soon as possible.
     virtual void SignalInterrupt() = 0;
 
+    /// Clear a previous interrupt.
+    virtual void ClearInterrupt() = 0;
+
     struct BacktraceEntry {
         std::string module;
         u64 address;
@@ -208,7 +208,6 @@ public:
 protected:
     /// System context that this ARM interface is running under.
     System& system;
-    CPUInterrupts& interrupt_handlers;
     const WatchpointArray* watchpoints;
     bool uses_wall_clock;
 

src/core/arm/cpu_interrupt_handler.cpp

@@ -1,24 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2020 yuzu Emulator Project
-// SPDX-License-Identifier: GPL-2.0-or-later
-
-#include "common/thread.h"
-#include "core/arm/cpu_interrupt_handler.h"
-
-namespace Core {
-
-CPUInterruptHandler::CPUInterruptHandler() : interrupt_event{std::make_unique<Common::Event>()} {}
-
-CPUInterruptHandler::~CPUInterruptHandler() = default;
-
-void CPUInterruptHandler::SetInterrupt(bool is_interrupted_) {
-    if (is_interrupted_) {
-        interrupt_event->Set();
-    }
-    is_interrupted = is_interrupted_;
-}
-
-void CPUInterruptHandler::AwaitInterrupt() {
-    interrupt_event->Wait();
-}
-
-} // namespace Core

src/core/arm/cpu_interrupt_handler.h

@@ -1,39 +0,0 @@
-// SPDX-FileCopyrightText: Copyright 2020 yuzu Emulator Project
-// SPDX-License-Identifier: GPL-2.0-or-later
-
-#pragma once
-
-#include <atomic>
-#include <memory>
-
-namespace Common {
-class Event;
-}
-
-namespace Core {
-
-class CPUInterruptHandler {
-public:
-    CPUInterruptHandler();
-    ~CPUInterruptHandler();
-
-    CPUInterruptHandler(const CPUInterruptHandler&) = delete;
-    CPUInterruptHandler& operator=(const CPUInterruptHandler&) = delete;
-
-    CPUInterruptHandler(CPUInterruptHandler&&) = delete;
-    CPUInterruptHandler& operator=(CPUInterruptHandler&&) = delete;
-
-    bool IsInterrupted() const {
-        return is_interrupted;
-    }
-
-    void SetInterrupt(bool is_interrupted);
-
-    void AwaitInterrupt();
-
-private:
-    std::unique_ptr<Common::Event> interrupt_event;
-    std::atomic_bool is_interrupted{false};
-};
-
-} // namespace Core

src/core/arm/dynarmic/arm_dynarmic_32.cpp

@@ -11,7 +11,6 @@
 #include "common/logging/log.h"
 #include "common/page_table.h"
 #include "common/settings.h"
-#include "core/arm/cpu_interrupt_handler.h"
 #include "core/arm/dynarmic/arm_dynarmic_32.h"
 #include "core/arm/dynarmic/arm_dynarmic_cp15.h"
 #include "core/arm/dynarmic/arm_exclusive_monitor.h"
@@ -318,11 +317,9 @@ void ARM_Dynarmic_32::RewindBreakpointInstruction() {
     LoadContext(breakpoint_context);
 }
 
-ARM_Dynarmic_32::ARM_Dynarmic_32(System& system_, CPUInterrupts& interrupt_handlers_,
+ARM_Dynarmic_32::ARM_Dynarmic_32(System& system_, bool uses_wall_clock_,
-                                 bool uses_wall_clock_, ExclusiveMonitor& exclusive_monitor_,
+                                 ExclusiveMonitor& exclusive_monitor_, std::size_t core_index_)
-                                 std::size_t core_index_)
+    : ARM_Interface{system_, uses_wall_clock_}, cb(std::make_unique<DynarmicCallbacks32>(*this)),
-    : ARM_Interface{system_, interrupt_handlers_, uses_wall_clock_},
-      cb(std::make_unique<DynarmicCallbacks32>(*this)),
       cp15(std::make_shared<DynarmicCP15>(*this)), core_index{core_index_},
       exclusive_monitor{dynamic_cast<DynarmicExclusiveMonitor&>(exclusive_monitor_)},
       null_jit{MakeJit(nullptr)}, jit{null_jit.get()} {}
@@ -401,6 +398,10 @@ void ARM_Dynarmic_32::SignalInterrupt() {
     jit.load()->HaltExecution(break_loop);
 }
 
+void ARM_Dynarmic_32::ClearInterrupt() {
+    jit.load()->ClearHalt(break_loop);
+}
+
 void ARM_Dynarmic_32::ClearInstructionCache() {
     jit.load()->ClearCache();
 }

src/core/arm/dynarmic/arm_dynarmic_32.h

@@ -28,8 +28,8 @@ class System;
 
 class ARM_Dynarmic_32 final : public ARM_Interface {
 public:
-    ARM_Dynarmic_32(System& system_, CPUInterrupts& interrupt_handlers_, bool uses_wall_clock_,
+    ARM_Dynarmic_32(System& system_, bool uses_wall_clock_, ExclusiveMonitor& exclusive_monitor_,
-                    ExclusiveMonitor& exclusive_monitor_, std::size_t core_index_);
+                    std::size_t core_index_);
     ~ARM_Dynarmic_32() override;
 
     void SetPC(u64 pc) override;
@@ -56,6 +56,7 @@ public:
     void LoadContext(const ThreadContext64& ctx) override {}
 
     void SignalInterrupt() override;
+    void ClearInterrupt() override;
     void ClearExclusiveState() override;
 
     void ClearInstructionCache() override;

src/core/arm/dynarmic/arm_dynarmic_64.cpp

@@ -10,7 +10,6 @@
 #include "common/logging/log.h"
 #include "common/page_table.h"
 #include "common/settings.h"
-#include "core/arm/cpu_interrupt_handler.h"
 #include "core/arm/dynarmic/arm_dynarmic_64.h"
 #include "core/arm/dynarmic/arm_exclusive_monitor.h"
 #include "core/core.h"
@@ -378,10 +377,9 @@ void ARM_Dynarmic_64::RewindBreakpointInstruction() {
     LoadContext(breakpoint_context);
 }
 
-ARM_Dynarmic_64::ARM_Dynarmic_64(System& system_, CPUInterrupts& interrupt_handlers_,
+ARM_Dynarmic_64::ARM_Dynarmic_64(System& system_, bool uses_wall_clock_,
-                                 bool uses_wall_clock_, ExclusiveMonitor& exclusive_monitor_,
+                                 ExclusiveMonitor& exclusive_monitor_, std::size_t core_index_)
-                                 std::size_t core_index_)
+    : ARM_Interface{system_, uses_wall_clock_},
-    : ARM_Interface{system_, interrupt_handlers_, uses_wall_clock_},
       cb(std::make_unique<DynarmicCallbacks64>(*this)), core_index{core_index_},
       exclusive_monitor{dynamic_cast<DynarmicExclusiveMonitor&>(exclusive_monitor_)},
       null_jit{MakeJit(nullptr, 48)}, jit{null_jit.get()} {}
@@ -468,6 +466,10 @@ void ARM_Dynarmic_64::SignalInterrupt() {
     jit.load()->HaltExecution(break_loop);
 }
 
+void ARM_Dynarmic_64::ClearInterrupt() {
+    jit.load()->ClearHalt(break_loop);
+}
+
 void ARM_Dynarmic_64::ClearInstructionCache() {
     jit.load()->ClearCache();
 }

src/core/arm/dynarmic/arm_dynarmic_64.h

@@ -20,14 +20,13 @@ class Memory;
 namespace Core {
 
 class DynarmicCallbacks64;
-class CPUInterruptHandler;
 class DynarmicExclusiveMonitor;
 class System;
 
 class ARM_Dynarmic_64 final : public ARM_Interface {
 public:
-    ARM_Dynarmic_64(System& system_, CPUInterrupts& interrupt_handlers_, bool uses_wall_clock_,
+    ARM_Dynarmic_64(System& system_, bool uses_wall_clock_, ExclusiveMonitor& exclusive_monitor_,
-                    ExclusiveMonitor& exclusive_monitor_, std::size_t core_index_);
+                    std::size_t core_index_);
     ~ARM_Dynarmic_64() override;
 
     void SetPC(u64 pc) override;
@@ -50,6 +49,7 @@ public:
     void LoadContext(const ThreadContext64& ctx) override;
 
     void SignalInterrupt() override;
+    void ClearInterrupt() override;
     void ClearExclusiveState() override;
 
     void ClearInstructionCache() override;

src/core/debugger/debugger.cpp

@@ -15,6 +15,7 @@
 #include "core/debugger/debugger_interface.h"
 #include "core/debugger/gdbstub.h"
 #include "core/hle/kernel/global_scheduler_context.h"
+#include "core/hle/kernel/k_scheduler.h"
 
 template <typename Readable, typename Buffer, typename Callback>
 static void AsyncReceiveInto(Readable& r, Buffer& buffer, Callback&& c) {
@@ -230,13 +231,12 @@ private:
     }
 
     void PauseEmulation() {
+        Kernel::KScopedSchedulerLock sl{system.Kernel()};
+
         // Put all threads to sleep on next scheduler round.
         for (auto* thread : ThreadList()) {
             thread->RequestSuspend(Kernel::SuspendType::Debug);
         }
-
-        // Signal an interrupt so that scheduler will fire.
-        system.Kernel().InterruptAllPhysicalCores();
     }
 
     void ResumeEmulation(Kernel::KThread* except = nullptr) {
@@ -253,7 +253,8 @@ private:
 
     template <typename Callback>
     void MarkResumed(Callback&& cb) {
-        std::scoped_lock lk{connection_lock};
+        Kernel::KScopedSchedulerLock sl{system.Kernel()};
+        std::scoped_lock cl{connection_lock};
         stopped = false;
         cb();
     }

src/core/hle/kernel/kernel.cpp

@@ -17,7 +17,6 @@
 #include "common/thread.h"
 #include "common/thread_worker.h"
 #include "core/arm/arm_interface.h"
-#include "core/arm/cpu_interrupt_handler.h"
 #include "core/arm/exclusive_monitor.h"
 #include "core/core.h"
 #include "core/core_timing.h"
@@ -82,7 +81,7 @@ struct KernelCore::Impl {
 
     void InitializeCores() {
         for (u32 core_id = 0; core_id < Core::Hardware::NUM_CPU_CORES; core_id++) {
-            cores[core_id].Initialize((*current_process).Is64BitProcess());
+            cores[core_id]->Initialize((*current_process).Is64BitProcess());
             system.Memory().SetCurrentPageTable(*current_process, core_id);
         }
     }
@@ -100,7 +99,9 @@ struct KernelCore::Impl {
         next_user_process_id = KProcess::ProcessIDMin;
         next_thread_id = 1;
 
-        cores.clear();
+        for (auto& core : cores) {
+            core = nullptr;
+        }
 
         global_handle_table->Finalize();
         global_handle_table.reset();
@@ -199,7 +200,7 @@ struct KernelCore::Impl {
             const s32 core{static_cast<s32>(i)};
 
             schedulers[i] = std::make_unique<Kernel::KScheduler>(system.Kernel());
-            cores.emplace_back(i, system, *schedulers[i], interrupts);
+            cores[i] = std::make_unique<Kernel::PhysicalCore>(i, system, *schedulers[i]);
 
             auto* main_thread{Kernel::KThread::Create(system.Kernel())};
             main_thread->SetName(fmt::format("MainThread:{}", core));
@@ -761,7 +762,7 @@ struct KernelCore::Impl {
     std::unordered_set<KAutoObject*> registered_in_use_objects;
 
     std::unique_ptr<Core::ExclusiveMonitor> exclusive_monitor;
-    std::vector<Kernel::PhysicalCore> cores;
+    std::array<std::unique_ptr<Kernel::PhysicalCore>, Core::Hardware::NUM_CPU_CORES> cores;
 
     // Next host thead ID to use, 0-3 IDs represent core threads, >3 represent others
     std::atomic<u32> next_host_thread_id{Core::Hardware::NUM_CPU_CORES};
@@ -785,7 +786,6 @@ struct KernelCore::Impl {
     Common::ThreadWorker service_threads_manager;
 
     std::array<KThread*, Core::Hardware::NUM_CPU_CORES> shutdown_threads;
-    std::array<Core::CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES> interrupts{};
     std::array<std::unique_ptr<Kernel::KScheduler>, Core::Hardware::NUM_CPU_CORES> schedulers{};
 
     bool is_multicore{};
@@ -874,11 +874,11 @@ const Kernel::KScheduler& KernelCore::Scheduler(std::size_t id) const {
 }
 
 Kernel::PhysicalCore& KernelCore::PhysicalCore(std::size_t id) {
-    return impl->cores[id];
+    return *impl->cores[id];
 }
 
 const Kernel::PhysicalCore& KernelCore::PhysicalCore(std::size_t id) const {
-    return impl->cores[id];
+    return *impl->cores[id];
 }
 
 size_t KernelCore::CurrentPhysicalCoreIndex() const {
@@ -890,11 +890,11 @@ size_t KernelCore::CurrentPhysicalCoreIndex() const {
 }
 
 Kernel::PhysicalCore& KernelCore::CurrentPhysicalCore() {
-    return impl->cores[CurrentPhysicalCoreIndex()];
+    return *impl->cores[CurrentPhysicalCoreIndex()];
 }
 
 const Kernel::PhysicalCore& KernelCore::CurrentPhysicalCore() const {
-    return impl->cores[CurrentPhysicalCoreIndex()];
+    return *impl->cores[CurrentPhysicalCoreIndex()];
 }
 
 Kernel::KScheduler* KernelCore::CurrentScheduler() {
@@ -906,15 +906,6 @@ Kernel::KScheduler* KernelCore::CurrentScheduler() {
     return impl->schedulers[core_id].get();
 }
 
-std::array<Core::CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES>& KernelCore::Interrupts() {
-    return impl->interrupts;
-}
-
-const std::array<Core::CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES>& KernelCore::Interrupts()
-    const {
-    return impl->interrupts;
-}
-
 Kernel::TimeManager& KernelCore::TimeManager() {
     return impl->time_manager;
 }
@@ -939,24 +930,18 @@ const KAutoObjectWithListContainer& KernelCore::ObjectListContainer() const {
     return *impl->global_object_list_container;
 }
 
-void KernelCore::InterruptAllPhysicalCores() {
-    for (auto& physical_core : impl->cores) {
-        physical_core.Interrupt();
-    }
-}
-
 void KernelCore::InvalidateAllInstructionCaches() {
     for (auto& physical_core : impl->cores) {
-        physical_core.ArmInterface().ClearInstructionCache();
+        physical_core->ArmInterface().ClearInstructionCache();
     }
 }
 
 void KernelCore::InvalidateCpuInstructionCacheRange(VAddr addr, std::size_t size) {
     for (auto& physical_core : impl->cores) {
-        if (!physical_core.IsInitialized()) {
+        if (!physical_core->IsInitialized()) {
             continue;
         }
-        physical_core.ArmInterface().InvalidateCacheRange(addr, size);
+        physical_core->ArmInterface().InvalidateCacheRange(addr, size);
     }
 }
 

src/core/hle/kernel/kernel.h

@@ -9,14 +9,12 @@
 #include <string>
 #include <unordered_map>
 #include <vector>
-#include "core/arm/cpu_interrupt_handler.h"
 #include "core/hardware_properties.h"
 #include "core/hle/kernel/k_auto_object.h"
 #include "core/hle/kernel/k_slab_heap.h"
 #include "core/hle/kernel/svc_common.h"
 
 namespace Core {
-class CPUInterruptHandler;
 class ExclusiveMonitor;
 class System;
 } // namespace Core
@@ -183,12 +181,6 @@ public:
 
     const KAutoObjectWithListContainer& ObjectListContainer() const;
 
-    std::array<Core::CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES>& Interrupts();
-
-    const std::array<Core::CPUInterruptHandler, Core::Hardware::NUM_CPU_CORES>& Interrupts() const;
-
-    void InterruptAllPhysicalCores();
-
     void InvalidateAllInstructionCaches();
 
     void InvalidateCpuInstructionCacheRange(VAddr addr, std::size_t size);

src/core/hle/kernel/physical_core.cpp

@@ -1,7 +1,6 @@
 // SPDX-FileCopyrightText: Copyright 2020 yuzu Emulator Project
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-#include "core/arm/cpu_interrupt_handler.h"
 #include "core/arm/dynarmic/arm_dynarmic_32.h"
 #include "core/arm/dynarmic/arm_dynarmic_64.h"
 #include "core/core.h"
@@ -11,16 +10,14 @@
 
 namespace Kernel {
 
-PhysicalCore::PhysicalCore(std::size_t core_index_, Core::System& system_, KScheduler& scheduler_,
+PhysicalCore::PhysicalCore(std::size_t core_index_, Core::System& system_, KScheduler& scheduler_)
-                           Core::CPUInterrupts& interrupts_)
+    : core_index{core_index_}, system{system_}, scheduler{scheduler_} {
-    : core_index{core_index_}, system{system_}, scheduler{scheduler_},
-      interrupts{interrupts_}, guard{std::make_unique<std::mutex>()} {
 #ifdef ARCHITECTURE_x86_64
     // TODO(bunnei): Initialization relies on a core being available. We may later replace this with
     // a 32-bit instance of Dynarmic. This should be abstracted out to a CPU manager.
     auto& kernel = system.Kernel();
     arm_interface = std::make_unique<Core::ARM_Dynarmic_64>(
-        system, interrupts, kernel.IsMulticore(), kernel.GetExclusiveMonitor(), core_index);
+        system, kernel.IsMulticore(), kernel.GetExclusiveMonitor(), core_index);
 #else
 #error Platform not supported yet.
 #endif
@@ -34,7 +31,7 @@ void PhysicalCore::Initialize([[maybe_unused]] bool is_64_bit) {
     if (!is_64_bit) {
         // We already initialized a 64-bit core, replace with a 32-bit one.
         arm_interface = std::make_unique<Core::ARM_Dynarmic_32>(
-            system, interrupts, kernel.IsMulticore(), kernel.GetExclusiveMonitor(), core_index);
+            system, kernel.IsMulticore(), kernel.GetExclusiveMonitor(), core_index);
     }
 #else
 #error Platform not supported yet.
@@ -47,24 +44,26 @@ void PhysicalCore::Run() {
 }
 
 void PhysicalCore::Idle() {
-    interrupts[core_index].AwaitInterrupt();
+    std::unique_lock lk{guard};
+    on_interrupt.wait(lk, [this] { return is_interrupted; });
 }
 
 bool PhysicalCore::IsInterrupted() const {
-    return interrupts[core_index].IsInterrupted();
+    return is_interrupted;
 }
 
 void PhysicalCore::Interrupt() {
-    guard->lock();
+    std::unique_lock lk{guard};
-    interrupts[core_index].SetInterrupt(true);
+    is_interrupted = true;
     arm_interface->SignalInterrupt();
-    guard->unlock();
+    on_interrupt.notify_all();
 }
 
 void PhysicalCore::ClearInterrupt() {
-    guard->lock();
+    std::unique_lock lk{guard};
-    interrupts[core_index].SetInterrupt(false);
+    is_interrupted = false;
-    guard->unlock();
+    arm_interface->ClearInterrupt();
+    on_interrupt.notify_all();
 }
 
 } // namespace Kernel

src/core/hle/kernel/physical_core.h

@@ -14,7 +14,6 @@ class KScheduler;
 } // namespace Kernel
 
 namespace Core {
-class CPUInterruptHandler;
 class ExclusiveMonitor;
 class System;
 } // namespace Core
@@ -23,15 +22,11 @@ namespace Kernel {
 
 class PhysicalCore {
 public:
-    PhysicalCore(std::size_t core_index_, Core::System& system_, KScheduler& scheduler_,
+    PhysicalCore(std::size_t core_index_, Core::System& system_, KScheduler& scheduler_);
-                 Core::CPUInterrupts& interrupts_);
     ~PhysicalCore();
 
-    PhysicalCore(const PhysicalCore&) = delete;
+    YUZU_NON_COPYABLE(PhysicalCore);
-    PhysicalCore& operator=(const PhysicalCore&) = delete;
+    YUZU_NON_MOVEABLE(PhysicalCore);
-
-    PhysicalCore(PhysicalCore&&) = default;
-    PhysicalCore& operator=(PhysicalCore&&) = delete;
 
     /// Initialize the core for the specified parameters.
     void Initialize(bool is_64_bit);
@@ -86,9 +81,11 @@ private:
     const std::size_t core_index;
     Core::System& system;
     Kernel::KScheduler& scheduler;
-    Core::CPUInterrupts& interrupts;
+
-    std::unique_ptr<std::mutex> guard;
+    std::mutex guard;
+    std::condition_variable on_interrupt;
     std::unique_ptr<Core::ARM_Interface> arm_interface;
+    bool is_interrupted;
 };
 
 } // namespace Kernel