From 067ac434ba90084359babef1638970e849a5f2ce Mon Sep 17 00:00:00 2001
From: Michael Scire <SciresM@gmail.com>
Date: Fri, 22 Jun 2018 00:47:59 -0600
Subject: [PATCH] Kernel/Arbiters: Fix casts, cleanup comments/magic numbers

---
 src/core/hle/kernel/address_arbiter.cpp | 25 ++++++++++++-------------
 src/core/hle/kernel/svc.cpp             |  8 ++++----
 src/core/memory.cpp                     |  4 ++++
 src/core/memory.h                       |  7 +++++++
 4 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/src/core/hle/kernel/address_arbiter.cpp b/src/core/hle/kernel/address_arbiter.cpp
index 972911e421..e9c8369d77 100644
--- a/src/core/hle/kernel/address_arbiter.cpp
+++ b/src/core/hle/kernel/address_arbiter.cpp
@@ -17,7 +17,7 @@ namespace Kernel {
 namespace AddressArbiter {
 
 // Performs actual address waiting logic.
-ResultCode WaitForAddress(VAddr address, s64 timeout) {
+static ResultCode WaitForAddress(VAddr address, s64 timeout) {
     SharedPtr<Thread> current_thread = GetCurrentThread();
     current_thread->arb_wait_address = address;
     current_thread->status = THREADSTATUS_WAIT_ARB;
@@ -26,12 +26,12 @@ ResultCode WaitForAddress(VAddr address, s64 timeout) {
     current_thread->WakeAfterDelay(timeout);
 
     Core::System::GetInstance().CpuCore(current_thread->processor_id).PrepareReschedule();
-    // This should never actually execute.
-    return RESULT_SUCCESS;
+    return RESULT_TIMEOUT;
 }
 
 // Gets the threads waiting on an address.
-void GetThreadsWaitingOnAddress(std::vector<SharedPtr<Thread>>& waiting_threads, VAddr address) {
+static void GetThreadsWaitingOnAddress(std::vector<SharedPtr<Thread>>& waiting_threads,
+                                       VAddr address) {
     auto RetrieveWaitingThreads =
         [](size_t core_index, std::vector<SharedPtr<Thread>>& waiting_threads, VAddr arb_addr) {
             const auto& scheduler = Core::System::GetInstance().Scheduler(core_index);
@@ -56,7 +56,7 @@ void GetThreadsWaitingOnAddress(std::vector<SharedPtr<Thread>>& waiting_threads,
 }
 
 // Wake up num_to_wake (or all) threads in a vector.
-void WakeThreads(std::vector<SharedPtr<Thread>>& waiting_threads, s32 num_to_wake) {
+static void WakeThreads(std::vector<SharedPtr<Thread>>& waiting_threads, s32 num_to_wake) {
     // Only process up to 'target' threads, unless 'target' is <= 0, in which case process
     // them all.
     size_t last = waiting_threads.size();
@@ -64,7 +64,6 @@ void WakeThreads(std::vector<SharedPtr<Thread>>& waiting_threads, s32 num_to_wak
         last = num_to_wake;
 
     // Signal the waiting threads.
-    // TODO: Rescheduling should not occur while waking threads. How can it be prevented?
     for (size_t i = 0; i < last; i++) {
         ASSERT(waiting_threads[i]->status = THREADSTATUS_WAIT_ARB);
         waiting_threads[i]->SetWaitSynchronizationResult(RESULT_SUCCESS);
@@ -90,8 +89,8 @@ ResultCode IncrementAndSignalToAddressIfEqual(VAddr address, s32 value, s32 num_
         return ERR_INVALID_ADDRESS_STATE;
     }
 
-    if ((s32)Memory::Read32(address) == value) {
-        Memory::Write32(address, (u32)(value + 1));
+    if (static_cast<s32>(Memory::Read32(address)) == value) {
+        Memory::Write32(address, static_cast<u32>(value + 1));
     } else {
         return ERR_INVALID_STATE;
     }
@@ -122,8 +121,8 @@ ResultCode ModifyByWaitingCountAndSignalToAddressIfEqual(VAddr address, s32 valu
         updated_value = value;
     }
 
-    if ((s32)Memory::Read32(address) == value) {
-        Memory::Write32(address, (u32)(updated_value));
+    if (static_cast<s32>(Memory::Read32(address)) == value) {
+        Memory::Write32(address, static_cast<u32>(updated_value));
     } else {
         return ERR_INVALID_STATE;
     }
@@ -139,9 +138,9 @@ ResultCode WaitForAddressIfLessThan(VAddr address, s32 value, s64 timeout, bool
         return ERR_INVALID_ADDRESS_STATE;
     }
 
-    s32 cur_value = (s32)Memory::Read32(address);
+    s32 cur_value = static_cast<s32>(Memory::Read32(address));
     if (cur_value < value) {
-        Memory::Write32(address, (u32)(cur_value - 1));
+        Memory::Write32(address, static_cast<u32>(cur_value - 1));
     } else {
         return ERR_INVALID_STATE;
     }
@@ -160,7 +159,7 @@ ResultCode WaitForAddressIfEqual(VAddr address, s32 value, s64 timeout) {
         return ERR_INVALID_ADDRESS_STATE;
     }
     // Only wait for the address if equal.
-    if ((s32)Memory::Read32(address) != value) {
+    if (static_cast<s32>(Memory::Read32(address)) != value) {
         return ERR_INVALID_STATE;
     }
     // Short-circuit without rescheduling, if timeout is zero.
diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp
index 95ce2205ae..1a36e0d027 100644
--- a/src/core/hle/kernel/svc.cpp
+++ b/src/core/hle/kernel/svc.cpp
@@ -695,7 +695,7 @@ static ResultCode WaitForAddress(VAddr address, u32 type, s32 value, s64 timeout
     NGLOG_WARNING(Kernel_SVC, "called, address=0x{:X}, type=0x{:X}, value=0x{:X}, timeout={}",
                   address, type, value, timeout);
     // If the passed address is a kernel virtual address, return invalid memory state.
-    if ((address + 0x8000000000LL) < 0x7FFFE00000LL) {
+    if (Memory::IsKernelVirtualAddress(address)) {
         return ERR_INVALID_ADDRESS_STATE;
     }
     // If the address is not properly aligned to 4 bytes, return invalid address.
@@ -703,7 +703,7 @@ static ResultCode WaitForAddress(VAddr address, u32 type, s32 value, s64 timeout
         return ERR_INVALID_ADDRESS;
     }
 
-    switch ((AddressArbiter::ArbitrationType)type) {
+    switch (static_cast<AddressArbiter::ArbitrationType>(type)) {
     case AddressArbiter::ArbitrationType::WaitIfLessThan:
         return AddressArbiter::WaitForAddressIfLessThan(address, value, timeout, false);
     case AddressArbiter::ArbitrationType::DecrementAndWaitIfLessThan:
@@ -721,7 +721,7 @@ static ResultCode SignalToAddress(VAddr address, u32 type, s32 value, s32 num_to
                   "called, address=0x{:X}, type=0x{:X}, value=0x{:X}, num_to_wake=0x{:X}", address,
                   type, value, num_to_wake);
     // If the passed address is a kernel virtual address, return invalid memory state.
-    if ((address + 0x8000000000LL) < 0x7FFFE00000LL) {
+    if (Memory::IsKernelVirtualAddress(address)) {
         return ERR_INVALID_ADDRESS_STATE;
     }
     // If the address is not properly aligned to 4 bytes, return invalid address.
@@ -729,7 +729,7 @@ static ResultCode SignalToAddress(VAddr address, u32 type, s32 value, s32 num_to
         return ERR_INVALID_ADDRESS;
     }
 
-    switch ((AddressArbiter::SignalType)type) {
+    switch (static_cast<AddressArbiter::SignalType>(type)) {
     case AddressArbiter::SignalType::Signal:
         return AddressArbiter::SignalToAddress(address, num_to_wake);
     case AddressArbiter::SignalType::IncrementAndSignalIfEqual:
diff --git a/src/core/memory.cpp b/src/core/memory.cpp
index 3b81acd631..f070dee7dc 100644
--- a/src/core/memory.cpp
+++ b/src/core/memory.cpp
@@ -241,6 +241,10 @@ bool IsValidVirtualAddress(const VAddr vaddr) {
     return IsValidVirtualAddress(*Core::CurrentProcess(), vaddr);
 }
 
+bool IsKernelVirtualAddress(const VAddr vaddr) {
+    return KERNEL_REGION_VADDR <= vaddr && vaddr < KERNEL_REGION_END;
+}
+
 bool IsValidPhysicalAddress(const PAddr paddr) {
     return GetPhysicalPointer(paddr) != nullptr;
 }
diff --git a/src/core/memory.h b/src/core/memory.h
index 3f56a2c6a6..8d5d017a46 100644
--- a/src/core/memory.h
+++ b/src/core/memory.h
@@ -188,6 +188,11 @@ enum : VAddr {
     MAP_REGION_VADDR = NEW_MAP_REGION_VADDR_END,
     MAP_REGION_SIZE = 0x1000000000,
     MAP_REGION_VADDR_END = MAP_REGION_VADDR + MAP_REGION_SIZE,
+
+    /// Kernel Virtual Address Range
+    KERNEL_REGION_VADDR = 0xFFFFFF8000000000,
+    KERNEL_REGION_SIZE = 0x7FFFE00000,
+    KERNEL_REGION_END = KERNEL_REGION_VADDR + KERNEL_REGION_SIZE,
 };
 
 /// Currently active page table
@@ -197,6 +202,8 @@ PageTable* GetCurrentPageTable();
 /// Determines if the given VAddr is valid for the specified process.
 bool IsValidVirtualAddress(const Kernel::Process& process, const VAddr vaddr);
 bool IsValidVirtualAddress(const VAddr addr);
+/// Determines if the given VAddr is a kernel address
+bool IsKernelVirtualAddress(const VAddr addr);
 
 bool IsValidPhysicalAddress(const PAddr addr);
 
-- 
GitLab