From 2bc949628dfa2efe9a18660b9d662e2a25cef9f9 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Thu, 13 Feb 2020 17:01:44 -0400 Subject: [PATCH] Core: Address Feedback --- src/core/hardware_properties.h | 31 ++++++++++++++++++------- src/core/hle/kernel/scheduler.cpp | 4 ++-- src/core/hle/kernel/server_session.cpp | 2 +- src/core/hle/kernel/svc.cpp | 2 +- src/core/hle/kernel/synchronization.cpp | 21 +++++++++-------- src/core/hle/kernel/synchronization.h | 14 +++++++++-- 6 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/core/hardware_properties.h b/src/core/hardware_properties.h index 62cdf9ef0..947140efb 100644 --- a/src/core/hardware_properties.h +++ b/src/core/hardware_properties.h @@ -8,14 +8,6 @@ namespace Core { -union EmuThreadHandle { - u64 raw; - struct { - u32 host_handle; - u32 guest_handle; - }; -}; - namespace Hardware { // The below clock rate is based on Switch's clockspeed being widely known as 1.020GHz @@ -23,6 +15,29 @@ namespace Hardware { constexpr u64 BASE_CLOCK_RATE = 1019215872; // Switch cpu frequency is 1020MHz un/docked constexpr u64 CNTFREQ = 19200000; // Switch's hardware clock speed constexpr u32 NUM_CPU_CORES = 4; // Number of CPU Cores + } // namespace Hardware +struct EmuThreadHandle { + u32 host_handle; + u32 guest_handle; + + u64 GetRaw() const { + return (static_cast(host_handle) << 32) | guest_handle; + } + + bool operator==(const EmuThreadHandle& rhs) const { + return std::tie(host_handle, guest_handle) == std::tie(rhs.host_handle, rhs.guest_handle); + } + + bool operator!=(const EmuThreadHandle& rhs) const { + return !operator==(rhs); + } + + static constexpr EmuThreadHandle InvalidHandle() { + constexpr u32 invalid_handle = 0xFFFFFFFF; + return {invalid_handle, invalid_handle}; + } +}; + } // namespace Core diff --git a/src/core/hle/kernel/scheduler.cpp b/src/core/hle/kernel/scheduler.cpp index b5ffa5418..86f1421bf 100644 --- a/src/core/hle/kernel/scheduler.cpp +++ b/src/core/hle/kernel/scheduler.cpp @@ -125,7 +125,7 @@ bool GlobalScheduler::YieldThreadAndBalanceLoad(Thread* yielding_thread) { scheduled_queue[core_id].yield(priority); std::array current_threads; - for (u32 i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) { + for (std::size_t i = 0; i < current_threads.size(); i++) { current_threads[i] = scheduled_queue[i].empty() ? nullptr : scheduled_queue[i].front(); } @@ -178,7 +178,7 @@ bool GlobalScheduler::YieldThreadAndWaitForLoadBalancing(Thread* yielding_thread if (scheduled_queue[core_id].empty()) { // Here, "current_threads" is calculated after the ""yield"", unlike yield -1 std::array current_threads; - for (u32 i = 0; i < Core::Hardware::NUM_CPU_CORES; i++) { + for (std::size_t i = 0; i < current_threads.size(); i++) { current_threads[i] = scheduled_queue[i].empty() ? nullptr : scheduled_queue[i].front(); } for (auto& thread : suggested_queue[core_id]) { diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index ca98fd984..4604e35c5 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -57,7 +57,7 @@ bool ServerSession::IsSignaled() const { } // Wait if we have no pending requests, or if we're currently handling a request. - return !(pending_requesting_threads.empty() || currently_handling != nullptr); + return !pending_requesting_threads.empty() && currently_handling == nullptr; } void ServerSession::Acquire(Thread* thread) { diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 86c660cdf..fd91779a3 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -474,7 +474,7 @@ static ResultCode WaitSynchronization(Core::System& system, Handle* index, VAddr objects[i] = object; } auto& synchronization = kernel.Synchronization(); - auto [result, handle_result] = synchronization.WaitFor(objects, nano_seconds); + const auto [result, handle_result] = synchronization.WaitFor(objects, nano_seconds); *index = handle_result; return result; } diff --git a/src/core/hle/kernel/synchronization.cpp b/src/core/hle/kernel/synchronization.cpp index 25afc162f..dc37fad1a 100644 --- a/src/core/hle/kernel/synchronization.cpp +++ b/src/core/hle/kernel/synchronization.cpp @@ -4,6 +4,7 @@ #include "core/core.h" #include "core/hle/kernel/errors.h" +#include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/scheduler.h" #include "core/hle/kernel/synchronization.h" @@ -27,30 +28,30 @@ static bool DefaultThreadWakeupCallback(ThreadWakeupReason reason, std::shared_p thread->SetWaitSynchronizationResult(RESULT_SUCCESS); thread->SetWaitSynchronizationOutput(static_cast(index)); return true; -}; +} Synchronization::Synchronization(Core::System& system) : system{system} {} void Synchronization::SignalObject(SynchronizationObject& obj) const { if (obj.IsSignaled()) { obj.WakeupAllWaitingThreads(); - }; + } } std::pair Synchronization::WaitFor( std::vector>& sync_objects, s64 nano_seconds) { auto* const thread = system.CurrentScheduler().GetCurrentThread(); // Find the first object that is acquirable in the provided list of objects - auto itr = std::find_if(sync_objects.begin(), sync_objects.end(), - [thread](const std::shared_ptr& object) { - return object->IsSignaled(); - }); + const auto itr = std::find_if(sync_objects.begin(), sync_objects.end(), + [thread](const std::shared_ptr& object) { + return object->IsSignaled(); + }); if (itr != sync_objects.end()) { // We found a ready object, acquire it and set the result value SynchronizationObject* object = itr->get(); object->Acquire(thread); - u32 index = static_cast(std::distance(sync_objects.begin(), itr)); + const u32 index = static_cast(std::distance(sync_objects.begin(), itr)); return {RESULT_SUCCESS, index}; } @@ -59,12 +60,12 @@ std::pair Synchronization::WaitFor( // If a timeout value of 0 was provided, just return the Timeout error code instead of // suspending the thread. if (nano_seconds == 0) { - return {RESULT_TIMEOUT, 0}; + return {RESULT_TIMEOUT, InvalidHandle}; } if (thread->IsSyncCancelled()) { thread->SetSyncCancelled(false); - return {ERR_SYNCHRONIZATION_CANCELED, 0}; + return {ERR_SYNCHRONIZATION_CANCELED, InvalidHandle}; } for (auto& object : sync_objects) { @@ -80,7 +81,7 @@ std::pair Synchronization::WaitFor( system.PrepareReschedule(thread->GetProcessorID()); - return {RESULT_TIMEOUT, 0}; + return {RESULT_TIMEOUT, InvalidHandle}; } } // namespace Kernel diff --git a/src/core/hle/kernel/synchronization.h b/src/core/hle/kernel/synchronization.h index 3417a9f13..379f4b1d3 100644 --- a/src/core/hle/kernel/synchronization.h +++ b/src/core/hle/kernel/synchronization.h @@ -6,6 +6,7 @@ #include #include +#include #include "core/hle/kernel/object.h" #include "core/hle/result.h" @@ -16,15 +17,24 @@ class System; namespace Kernel { -class KernelCore; class SynchronizationObject; +/** + * The 'Synchronization' class is an interface for handling synchronization methods + * used by Synchronization objects and synchronization SVCs. This centralizes processing of + * such + */ class Synchronization { public: - Synchronization(Core::System& system); + explicit Synchronization(Core::System& system); + /// Signals a synchronization object, waking up all its waiting threads void SignalObject(SynchronizationObject& obj) const; + /// Tries to see if waiting for any of the sync_objects is necessary, if not + /// it returns Success and the handle index of the signaled sync object. In + /// case not, the current thread will be locked and wait for nano_seconds or + /// for a synchronization object to signal. std::pair WaitFor( std::vector>& sync_objects, s64 nano_seconds);