From 69a2003a8e904399a84ec9d9573d77cdccb359d2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 15 Apr 2019 20:17:15 -0400 Subject: kernel/thread: Update thread processor ID flags Adds the missing flags to the enum and documents them. --- src/core/hle/kernel/svc.cpp | 2 +- src/core/hle/kernel/thread.h | 21 +++++++++++++++------ 2 files changed, 16 insertions(+), 7 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index e5d4d6b55..bb12f9ac7 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1776,7 +1776,7 @@ static ResultCode SetThreadCoreMask(Core::System& system, Handle thread_handle, if (core == OnlyChangeMask) { core = thread->GetIdealCore(); - } else if (core >= Core::NUM_CPU_CORES && core != static_cast(-1)) { + } else if (core >= Core::NUM_CPU_CORES && core != static_cast(THREADPROCESSORID_DONT_UPDATE)) { LOG_ERROR(Kernel_SVC, "Invalid core specified, got {}", core); return ERR_INVALID_PROCESSOR_ID; } diff --git a/src/core/hle/kernel/thread.h b/src/core/hle/kernel/thread.h index 83c83e45a..c340a801d 100644 --- a/src/core/hle/kernel/thread.h +++ b/src/core/hle/kernel/thread.h @@ -30,12 +30,21 @@ enum ThreadPriority : u32 { }; enum ThreadProcessorId : s32 { - THREADPROCESSORID_IDEAL = -2, ///< Run thread on the ideal core specified by the process. - THREADPROCESSORID_0 = 0, ///< Run thread on core 0 - THREADPROCESSORID_1 = 1, ///< Run thread on core 1 - THREADPROCESSORID_2 = 2, ///< Run thread on core 2 - THREADPROCESSORID_3 = 3, ///< Run thread on core 3 - THREADPROCESSORID_MAX = 4, ///< Processor ID must be less than this + /// Indicates that no particular processor core is preferred. + THREADPROCESSORID_DONT_CARE = -1, + + /// Run thread on the ideal core specified by the process. + THREADPROCESSORID_IDEAL = -2, + + /// Indicates that the preferred processor ID shouldn't be updated in + /// a core mask setting operation. + THREADPROCESSORID_DONT_UPDATE = -3, + + THREADPROCESSORID_0 = 0, ///< Run thread on core 0 + THREADPROCESSORID_1 = 1, ///< Run thread on core 1 + THREADPROCESSORID_2 = 2, ///< Run thread on core 2 + THREADPROCESSORID_3 = 3, ///< Run thread on core 3 + THREADPROCESSORID_MAX = 4, ///< Processor ID must be less than this /// Allowed CPU mask THREADPROCESSORID_DEFAULT_MASK = (1 << THREADPROCESSORID_0) | (1 << THREADPROCESSORID_1) | -- cgit v1.2.3 From d672c6e759c560dbeff04c1b92ce46bb7a2f8ed8 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 15 Apr 2019 20:34:55 -0400 Subject: kernel/svc: Reorganize svcSetThreadCoreMask() Makes the code much nicer to follow in terms of behavior and control flow. It also fixes a few bugs in the implementation. Notably, the thread's owner process shouldn't be accessed in order to retrieve the core mask or ideal core. This should be done through the current running process. The only reason this bug wasn't encountered yet is because we currently only support running one process, and thus every owner process will be the current process. We also weren't checking against the process' CPU core mask to see if an allowed core is specified or not. With this out of the way, it'll be less noisy to implement proper handling of the affinity flags internally within the kernel thread instances. --- src/core/hle/kernel/svc.cpp | 71 +++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 32 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index bb12f9ac7..5ed00d451 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1744,52 +1744,59 @@ static ResultCode GetThreadCoreMask(Core::System& system, Handle thread_handle, } static ResultCode SetThreadCoreMask(Core::System& system, Handle thread_handle, u32 core, - u64 mask) { - LOG_DEBUG(Kernel_SVC, "called, handle=0x{:08X}, mask=0x{:016X}, core=0x{:X}", thread_handle, - mask, core); + u64 affinity_mask) { + LOG_DEBUG(Kernel_SVC, "called, handle=0x{:08X}, core=0x{:X}, affinity_mask=0x{:016X}", + thread_handle, core, affinity_mask); - const auto& handle_table = system.Kernel().CurrentProcess()->GetHandleTable(); - const SharedPtr thread = handle_table.Get(thread_handle); - if (!thread) { - LOG_ERROR(Kernel_SVC, "Thread handle does not exist, thread_handle=0x{:08X}", - thread_handle); - return ERR_INVALID_HANDLE; - } + const auto* const current_process = system.Kernel().CurrentProcess(); if (core == static_cast(THREADPROCESSORID_IDEAL)) { - const u8 ideal_cpu_core = thread->GetOwnerProcess()->GetIdealCore(); + const u8 ideal_cpu_core = current_process->GetIdealCore(); ASSERT(ideal_cpu_core != static_cast(THREADPROCESSORID_IDEAL)); // Set the target CPU to the ideal core specified by the process. core = ideal_cpu_core; - mask = 1ULL << core; - } - - if (mask == 0) { - LOG_ERROR(Kernel_SVC, "Mask is 0"); - return ERR_INVALID_COMBINATION; - } + affinity_mask = 1ULL << core; + } else { + const u64 core_mask = current_process->GetCoreMask(); + + if ((core_mask | affinity_mask) != core_mask) { + LOG_ERROR( + Kernel_SVC, + "Invalid processor ID specified (core_mask=0x{:08X}, affinity_mask=0x{:016X})", + core_mask, affinity_mask); + return ERR_INVALID_PROCESSOR_ID; + } - /// This value is used to only change the affinity mask without changing the current ideal core. - static constexpr u32 OnlyChangeMask = static_cast(-3); + if (affinity_mask == 0) { + LOG_ERROR(Kernel_SVC, "Specfified affinity mask is zero."); + return ERR_INVALID_COMBINATION; + } - if (core == OnlyChangeMask) { - core = thread->GetIdealCore(); - } else if (core >= Core::NUM_CPU_CORES && core != static_cast(THREADPROCESSORID_DONT_UPDATE)) { - LOG_ERROR(Kernel_SVC, "Invalid core specified, got {}", core); - return ERR_INVALID_PROCESSOR_ID; + if (core < Core::NUM_CPU_CORES) { + if ((affinity_mask & (1ULL << core)) == 0) { + LOG_ERROR(Kernel_SVC, + "Core is not enabled for the current mask, core={}, mask={:016X}", core, + affinity_mask); + return ERR_INVALID_COMBINATION; + } + } else if (core != static_cast(THREADPROCESSORID_DONT_CARE) && + core != static_cast(THREADPROCESSORID_DONT_UPDATE)) { + LOG_ERROR(Kernel_SVC, "Invalid processor ID specified (core={}).", core); + return ERR_INVALID_PROCESSOR_ID; + } } - // Error out if the input core isn't enabled in the input mask. - if (core < Core::NUM_CPU_CORES && (mask & (1ull << core)) == 0) { - LOG_ERROR(Kernel_SVC, "Core is not enabled for the current mask, core={}, mask={:016X}", - core, mask); - return ERR_INVALID_COMBINATION; + const auto& handle_table = current_process->GetHandleTable(); + const SharedPtr thread = handle_table.Get(thread_handle); + if (!thread) { + LOG_ERROR(Kernel_SVC, "Thread handle does not exist, thread_handle=0x{:08X}", + thread_handle); + return ERR_INVALID_HANDLE; } - thread->ChangeCore(core, mask); - + thread->ChangeCore(core, affinity_mask); return RESULT_SUCCESS; } -- cgit v1.2.3 From 19632d24211df1a0f6a347f201e4b815b6715630 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Mon, 15 Apr 2019 21:33:07 -0400 Subject: kernel/svc: Make svcCreateThread/svcStartThread/svcSleepThread/svcExitThread calls show up in the debug log These are actually quite important indicators of thread lifetimes, so they should be going into the debug log, rather than being treated as misc info and delegated to the trace log. --- src/core/hle/kernel/svc.cpp | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 5ed00d451..a5bd92433 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -1208,7 +1208,7 @@ static void ExitProcess(Core::System& system) { /// Creates a new thread static ResultCode CreateThread(Core::System& system, Handle* out_handle, VAddr entry_point, u64 arg, VAddr stack_top, u32 priority, s32 processor_id) { - LOG_TRACE(Kernel_SVC, + LOG_DEBUG(Kernel_SVC, "called entrypoint=0x{:08X}, arg=0x{:08X}, stacktop=0x{:08X}, " "threadpriority=0x{:08X}, processorid=0x{:08X} : created handle=0x{:08X}", entry_point, arg, stack_top, priority, processor_id, *out_handle); @@ -1266,7 +1266,7 @@ static ResultCode CreateThread(Core::System& system, Handle* out_handle, VAddr e /// Starts the thread for the provided handle static ResultCode StartThread(Core::System& system, Handle thread_handle) { - LOG_TRACE(Kernel_SVC, "called thread=0x{:08X}", thread_handle); + LOG_DEBUG(Kernel_SVC, "called thread=0x{:08X}", thread_handle); const auto& handle_table = system.Kernel().CurrentProcess()->GetHandleTable(); const SharedPtr thread = handle_table.Get(thread_handle); @@ -1289,7 +1289,7 @@ static ResultCode StartThread(Core::System& system, Handle thread_handle) { /// Called when a thread exits static void ExitThread(Core::System& system) { - LOG_TRACE(Kernel_SVC, "called, pc=0x{:08X}", system.CurrentArmInterface().GetPC()); + LOG_DEBUG(Kernel_SVC, "called, pc=0x{:08X}", system.CurrentArmInterface().GetPC()); auto* const current_thread = system.CurrentScheduler().GetCurrentThread(); current_thread->Stop(); @@ -1299,7 +1299,7 @@ static void ExitThread(Core::System& system) { /// Sleep the current thread static void SleepThread(Core::System& system, s64 nanoseconds) { - LOG_TRACE(Kernel_SVC, "called nanoseconds={}", nanoseconds); + LOG_DEBUG(Kernel_SVC, "called nanoseconds={}", nanoseconds); enum class SleepType : s64 { YieldWithoutLoadBalancing = 0, -- cgit v1.2.3