From e90daa6a4f2f297cf91c7db3927f98ce90639824 Mon Sep 17 00:00:00 2001 From: Subv Date: Tue, 7 Nov 2017 19:58:29 -0500 Subject: [PATCH 1/2] Kernel/IPC: Add a small delay after each SyncRequest to prevent thread starvation. In a real 3DS, threads that call svcSyncRequest are put to sleep until the server responds via svcReplyAndReceive. Our HLE services don't implement this mechanism and are effectively immediate from the 3DS's point of view. This commit makes it so that we at least simulate the IPC delay. Specific HLE handlers might need to put their callers to sleep for a longer period of time to simulate IO timings. This is their responsibility but doing so is currently not implemented. See https://gist.github.com/ds84182/4a7690c5376e045cab9129ca4185bbeb for a test that was not passing before this commit. --- src/core/hle/kernel/server_session.cpp | 25 ++++++++++++++++++++----- 1 file changed, 20 insertions(+), 5 deletions(-) diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index 6ceb67c55..07f54d56d 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -67,13 +67,28 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr thread) { // If this ServerSession has an associated HLE handler, forward the request to it. if (hle_handler != nullptr) { hle_handler->HandleSyncRequest(SharedPtr(this)); - } else { + } + + if (thread->status == THREADSTATUS_RUNNING) { // Put the thread to sleep until the server replies, it will be awoken in - // svcReplyAndReceive. + // svcReplyAndReceive for LLE servers. thread->status = THREADSTATUS_WAIT_IPC; - // Add the thread to the list of threads that have issued a sync request with this - // server. - pending_requesting_threads.push_back(std::move(thread)); + + if (hle_handler != nullptr) { + // For HLE services, we put the request threads to sleep for a short duration to + // simulate IPC overhead, but only if the HLE handler didn't put the thread to sleep for + // other reasons like an async callback. The IPC overhead is needed to prevent + // starvation when a thread only does sync requests to HLE services while a + // lower-priority thread is waiting to run. + + // TODO(Subv): Figure out a good value for this. + static constexpr u64 IPCDelayNanoseconds = 100; + thread->WakeAfterDelay(IPCDelayNanoseconds); + } else { + // Add the thread to the list of threads that have issued a sync request with this + // server. + pending_requesting_threads.push_back(std::move(thread)); + } } // If this ServerSession does not have an HLE implementation, just wake up the threads waiting From 98e38723530eb60e3f2c5ad509174f48f98ae882 Mon Sep 17 00:00:00 2001 From: Subv Date: Wed, 8 Nov 2017 23:14:40 -0500 Subject: [PATCH 2/2] Kernel/IPC: Use 39 microseconds for the SendSyncRequest delay approximation. As measured by the time it takes for to return when performing the SetLcdForceBlack IPC request to the GSP:GPU service in a n3DS with firmware 11.6 See https://gist.github.com/ds84182/ecdbbd25b56a29bd4e5b32a7544b8e92 for the source code of the test. --- src/core/hle/kernel/server_session.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index 07f54d56d..5b61b52cb 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -81,8 +81,11 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr thread) { // starvation when a thread only does sync requests to HLE services while a // lower-priority thread is waiting to run. - // TODO(Subv): Figure out a good value for this. - static constexpr u64 IPCDelayNanoseconds = 100; + // This delay was approximated in a homebrew application by measuring the average time + // it takes for svcSendSyncRequest to return when performing the SetLcdForceBlack IPC + // request to the GSP:GPU service in a n3DS with firmware 11.6. The measured values have + // a high variance and vary between models. + static constexpr u64 IPCDelayNanoseconds = 39000; thread->WakeAfterDelay(IPCDelayNanoseconds); } else { // Add the thread to the list of threads that have issued a sync request with this