sm: implement accurate request deferral semantics

libraries/libstratosphere/include/stratosphere/sf/hipc/sf_hipc_server_manager.hpp

@@ -128,10 +128,6 @@ namespace ams::sf::hipc {
 
             os::Mutex waitlist_mutex;
             os::WaitableManagerType waitlist;
-
-            os::Mutex deferred_session_mutex;
-            using DeferredSessionList = typename util::IntrusiveListMemberTraits<&ServerSession::deferred_list_node>::ListType;
-            DeferredSessionList deferred_session_list;
         private:
             virtual void RegisterSessionToWaitList(ServerSession *session) override final;
             void RegisterToWaitList(os::WaitableHolderType *holder);
@@ -143,8 +139,6 @@ namespace ams::sf::hipc {
             Result ProcessForMitmServer(os::WaitableHolderType *holder);
             Result ProcessForSession(os::WaitableHolderType *holder);
 
-            void   ProcessDeferredSessions();
-
             template<typename Interface, auto MakeShared>
             void RegisterServerImpl(Handle port_handle, sm::ServiceName service_name, bool managed, cmif::ServiceObjectHolder &&static_holder) {
                 /* Allocate server memory. */
@@ -176,7 +170,7 @@ namespace ams::sf::hipc {
             ServerManagerBase(DomainEntryStorage *entry_storage, size_t entry_count) :
                 ServerDomainSessionManager(entry_storage, entry_count),
                 request_stop_event(os::EventClearMode_ManualClear), notify_event(os::EventClearMode_ManualClear),
-                waitable_selection_mutex(false), waitlist_mutex(false), deferred_session_mutex(false)
+                waitable_selection_mutex(false), waitlist_mutex(false)
             {
                 /* Link waitables. */
                 os::InitializeWaitableManager(std::addressof(this->waitable_manager));

libraries/libstratosphere/include/stratosphere/sf/hipc/sf_hipc_server_session_manager.hpp

@@ -45,7 +45,6 @@ namespace ams::sf::hipc {
         NON_COPYABLE(ServerSession);
         NON_MOVEABLE(ServerSession);
         private:
-            util::IntrusiveListNode deferred_list_node;
             cmif::ServiceObjectHolder srv_obj_holder;
             cmif::PointerAndSize pointer_buffer;
             cmif::PointerAndSize saved_message;

libraries/libstratosphere/source/sf/hipc/sf_hipc_server_manager.cpp

@@ -147,62 +147,14 @@ namespace ams::sf::hipc {
         return ResultSuccess();
     }
 
-    void ServerManagerBase::ProcessDeferredSessions() {
-        /* Iterate over the list of deferred sessions, and see if we can't do anything. */
-        std::scoped_lock lk(this->deferred_session_mutex);
-
-        /* Undeferring a request may undefer another request. We'll continue looping until everything is stable. */
-        bool needs_undefer_all = true;
-        while (needs_undefer_all) {
-            needs_undefer_all = false;
-
-            auto it = this->deferred_session_list.begin();
-            while (it != this->deferred_session_list.end()) {
-                ServerSession *session = static_cast<ServerSession *>(&*it);
-                R_TRY_CATCH(this->ProcessForSession(session)) {
-                    R_CATCH(sf::ResultRequestDeferred) {
-                        /* Session is still deferred, so let's continue. */
-                        it++;
-                        continue;
-                    }
-                    R_CATCH(sf::impl::ResultRequestInvalidated) {
-                        /* Session is no longer deferred! */
-                        it = this->deferred_session_list.erase(it);
-                        needs_undefer_all = true;
-                        continue;
-                    }
-                } R_END_TRY_CATCH_WITH_ABORT_UNLESS;
-
-                /* We succeeded! Remove from deferred list. */
-                it = this->deferred_session_list.erase(it);
-                needs_undefer_all = true;
-            }
-        }
-    }
-
     Result ServerManagerBase::Process(os::WaitableHolderType *holder) {
         switch (static_cast<UserDataTag>(os::GetWaitableHolderUserData(holder))) {
             case UserDataTag::Server:
                 return this->ProcessForServer(holder);
-                break;
             case UserDataTag::MitmServer:
                 return this->ProcessForMitmServer(holder);
-                break;
             case UserDataTag::Session:
-                /* Try to process for session. */
+                return this->ProcessForSession(holder);
-                R_TRY_CATCH(this->ProcessForSession(holder)) {
-                    R_CATCH(sf::ResultRequestDeferred) {
-                        /* The session was deferred, so push it onto the deferred session list. */
-                        std::scoped_lock lk(this->deferred_session_mutex);
-                        this->deferred_session_list.push_back(*static_cast<ServerSession *>(holder));
-                        return ResultSuccess();
-                    }
-                } R_END_TRY_CATCH;
-
-                /* We successfully invoked a command...so let's see if anything can be undeferred. */
-                this->ProcessDeferredSessions();
-                return ResultSuccess();
-                break;
             AMS_UNREACHABLE_DEFAULT_CASE();
         }
     }

stratosphere/sm/source/impl/sm_service_manager.cpp

@@ -15,17 +15,20 @@
  */
 #include <stratosphere.hpp>
 #include "sm_service_manager.hpp"
+#include "sm_wait_list.hpp"
 
 namespace ams::sm::impl {
 
-    /* Anonymous namespace for implementation details. */
     namespace {
+
         /* Constexpr definitions. */
         static constexpr size_t ProcessCountMax      = 0x40;
         static constexpr size_t ServiceCountMax      = 0x100;
         static constexpr size_t FutureMitmCountMax   = 0x20;
         static constexpr size_t AccessControlSizeMax = 0x200;
 
+        constexpr auto InitiallyDeferredServiceName = ServiceName::Encode("fsp-srv");
+
         /* Types. */
         struct ProcessInfo {
             os::ProcessId process_id;
@@ -274,6 +277,9 @@ namespace ams::sm::impl {
                     g_future_mitm_list[i] = InvalidServiceName;
                 }
             }
+
+            /* This might undefer some requests. */
+            TriggerResume(service);
         }
 
         void GetServiceInfoRecord(ServiceRecord *out_record, const ServiceInfo *service_info) {
@@ -347,7 +353,7 @@ namespace ams::sm::impl {
 
             /* This is a mechanism by which certain services will always be deferred until sm:m receives a special command. */
             /* This can be extended with more services as needed at a later date. */
-            return service == ServiceName::Encode("fsp-srv");
+            return service == InitiallyDeferredServiceName;
         }
 
         bool ShouldCloseOnClientDisconnect(ServiceName service) {
@@ -423,6 +429,9 @@ namespace ams::sm::impl {
             free_service->max_sessions = max_sessions;
             free_service->is_light = is_light;
 
+            /* This might undefer some requests. */
+            TriggerResume(service);
+
             return ResultSuccess();
         }
 
@@ -494,8 +503,9 @@ namespace ams::sm::impl {
         R_TRY(impl::HasService(&has_service, service));
 
         /* Wait until we have the service. */
-        R_UNLESS(has_service, sf::ResultRequestDeferredByUser());
+        R_SUCCEED_IF(has_service);
-        return ResultSuccess();
+
+        return StartRegisterRetry(service);
     }
 
     Result GetServiceHandle(Handle *out, os::ProcessId process_id, ServiceName service) {
@@ -519,10 +529,9 @@ namespace ams::sm::impl {
 
         /* Get service info. Check to see if we need to defer this until later. */
         ServiceInfo *service_info = GetServiceInfo(service);
-        R_UNLESS(service_info != nullptr,            sf::ResultRequestDeferredByUser());
+        if (service_info == nullptr || ShouldDeferForInit(service) || HasFutureMitmDeclaration(service) || service_info->mitm_waiting_ack) {
-        R_UNLESS(!ShouldDeferForInit(service),       sf::ResultRequestDeferredByUser());
+            return StartRegisterRetry(service);
-        R_UNLESS(!HasFutureMitmDeclaration(service), sf::ResultRequestDeferredByUser());
+        }
-        R_UNLESS(!service_info->mitm_waiting_ack,    sf::ResultRequestDeferredByUser());
 
         /* Get a handle from the service info. */
         R_TRY_CATCH(GetServiceHandleImpl(out, service_info, process_id)) {
@@ -588,8 +597,9 @@ namespace ams::sm::impl {
         R_TRY(impl::HasMitm(&has_mitm, service));
 
         /* Wait until we have the mitm. */
-        R_UNLESS(has_mitm, sf::ResultRequestDeferredByUser());
+        R_SUCCEED_IF(has_mitm);
-        return ResultSuccess();
+
+        return StartRegisterRetry(service);
     }
 
     Result InstallMitm(Handle *out, Handle *out_query, os::ProcessId process_id, ServiceName service) {
@@ -607,7 +617,9 @@ namespace ams::sm::impl {
         ServiceInfo *service_info = GetServiceInfo(service);
 
         /* If it doesn't exist, defer until it does. */
-        R_UNLESS(service_info != nullptr, sf::ResultRequestDeferredByUser());
+        if (service_info == nullptr) {
+            return StartRegisterRetry(service);
+        }
 
         /* Validate that the service isn't already being mitm'd. */
         R_UNLESS(!IsValidProcessId(service_info->mitm_process_id), sm::ResultAlreadyRegistered());
@@ -637,6 +649,9 @@ namespace ams::sm::impl {
             service_info->mitm_query_h = std::move(mitm_qry_hnd);
             *out = hnd.Move();
             *out_query = qry_hnd.Move();
+
+            /* This might undefer some requests. */
+            TriggerResume(service);
         }
 
         future_guard.Cancel();
@@ -733,6 +748,10 @@ namespace ams::sm::impl {
 
         /* Acknowledge. */
         service_info->AcknowledgeMitmSession(out_info, out_hnd);
+
+        /* Undefer requests to the session. */
+        TriggerResume(service);
+
         return ResultSuccess();
     }
 
@@ -771,6 +790,10 @@ namespace ams::sm::impl {
     /* Deferral extension (works around FS bug). */
     Result EndInitialDefers() {
         g_ended_initial_defers = true;
+
+        /* This might undefer some requests. */
+        TriggerResume(InitiallyDeferredServiceName);
+
         return ResultSuccess();
     }
 

stratosphere/sm/source/impl/sm_wait_list.cpp

@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2018-2020 Atmosphère-NX
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <stratosphere.hpp>
+#include "sm_wait_list.hpp"
+
+namespace ams::sm::impl {
+
+    namespace {
+
+        static constexpr size_t DeferredSessionCountMax = 0x40;
+
+        struct WaitListEntry {
+            sm::ServiceName service;
+            os::WaitableHolderType *session;
+        };
+
+        constinit WaitListEntry g_entries[DeferredSessionCountMax];
+        constinit WaitListEntry *g_processing_entry = nullptr;
+        constinit ServiceName g_triggered_service = InvalidServiceName;
+
+        WaitListEntry *Find(ServiceName service) {
+            for (auto &entry : g_entries) {
+                if (entry.service == service) {
+                    return std::addressof(entry);
+                }
+            }
+
+            return nullptr;
+        }
+
+    }
+
+    Result StartRegisterRetry(ServiceName service) {
+        /* Check that we're not already processing a retry. */
+        AMS_ABORT_UNLESS(g_processing_entry == nullptr);
+
+        /* Find a free entry. */
+        auto *entry = Find(InvalidServiceName);
+        R_UNLESS(entry != nullptr, sm::ResultOutOfProcesses());
+
+        /* Initialize the entry. */
+        entry->service = service;
+        entry->session = nullptr;
+
+        /* Set the processing entry. */
+        g_processing_entry = entry;
+
+        return sf::ResultRequestDeferredByUser();
+    }
+
+    void ProcessRegisterRetry(os::WaitableHolderType *session_holder) {
+        /* Verify that we have a processing entry. */
+        AMS_ABORT_UNLESS(g_processing_entry != nullptr);
+
+        /* Process the session. */
+        g_processing_entry->session = session_holder;
+        g_processing_entry = nullptr;
+    }
+
+    void TriggerResume(ServiceName service) {
+        /* Check that we haven't already triggered a resume. */
+        AMS_ABORT_UNLESS(g_triggered_service == InvalidServiceName);
+
+        /* Set the triggered resume. */
+        g_triggered_service = service;
+    }
+
+    void TestAndResume(ResumeFunction resume_function) {
+        /* If we don't have a triggered service, there's nothing to do. */
+        if (g_triggered_service == InvalidServiceName) {
+            return;
+        }
+
+        /* Process all entries. */
+        for (auto &entry : g_entries) {
+            if (entry.service == g_triggered_service) {
+                /* Get the entry's session. */
+                auto * const session = entry.session;
+
+                /* Clear the entry. */
+                entry.service = InvalidServiceName;
+                entry.session = nullptr;
+
+                /* Resume the request. */
+                R_TRY_CATCH(resume_function(session)) {
+                    R_CATCH(sf::ResultRequestDeferred) {
+                        ProcessRegisterRetry(session);
+                    }
+                } R_END_TRY_CATCH_WITH_ABORT_UNLESS;
+            }
+        }
+    }
+
+}

stratosphere/sm/source/impl/sm_wait_list.hpp

@@ -0,0 +1,29 @@
+/*
+ * Copyright (c) 2018-2020 Atmosphère-NX
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#pragma once
+#include <stratosphere.hpp>
+
+namespace ams::sm::impl {
+
+    using ResumeFunction = Result (*)(os::WaitableHolderType *session_holder);
+
+    Result StartRegisterRetry(ServiceName service);
+    void   ProcessRegisterRetry(os::WaitableHolderType *session_holder);
+
+    void TriggerResume(ServiceName service);
+    void TestAndResume(ResumeFunction resume_function);
+
+}

stratosphere/sm/source/sm_main.cpp

@@ -18,6 +18,7 @@
 #include "sm_manager_service.hpp"
 #include "sm_debug_monitor_service.hpp"
 #include "impl/sm_service_manager.hpp"
+#include "impl/sm_wait_list.hpp"
 
 extern "C" {
     extern u32 __start__;
@@ -86,6 +87,10 @@ namespace {
     constexpr size_t NumServers  = 3;
     sf::hipc::ServerManager<NumServers> g_server_manager;
 
+    ams::Result ResumeImpl(os::WaitableHolderType *session_holder) {
+        return g_server_manager.Process(session_holder);
+    }
+
 }
 
 int main(int argc, char **argv)
@@ -94,13 +99,10 @@ int main(int argc, char **argv)
     os::SetThreadNamePointer(os::GetCurrentThread(), AMS_GET_SYSTEM_THREAD_NAME(sm, Main));
     AMS_ASSERT(os::GetThreadPriority(os::GetCurrentThread()) == AMS_GET_SYSTEM_THREAD_PRIORITY(sm, Main));
 
-    /* NOTE: These handles are manually managed, but we don't save references to them to close on exit. */
-    /* This is fine, because if SM crashes we have much bigger issues. */
-
     /* Create sm:, (and thus allow things to register to it). */
     {
         Handle sm_h;
-        R_ABORT_UNLESS(svcManageNamedPort(&sm_h, "sm:", 0x40));
+        R_ABORT_UNLESS(svc::ManageNamedPort(&sm_h, "sm:", 0x40));
         g_server_manager.RegisterServer<sm::impl::IUserInterface, sm::UserService>(sm_h);
     }
 
@@ -122,8 +124,23 @@ int main(int argc, char **argv)
     /*================================*/
 
     /* Loop forever, servicing our services. */
-    g_server_manager.LoopProcess();
+    while (true) {
+        /* Get the next signaled holder. */
+        auto *holder = g_server_manager.WaitSignaled();
+        AMS_ABORT_UNLESS(holder != nullptr);
 
-    /* Cleanup. */
+        /* Process the holder. */
-    return 0;
+        R_TRY_CATCH(g_server_manager.Process(holder)) {
+            R_CATCH(sf::ResultRequestDeferred) {
+                sm::impl::ProcessRegisterRetry(holder);
+                continue;
+            }
+        } R_END_TRY_CATCH_WITH_ABORT_UNLESS;
+
+        /* Test to see if anything can be undeferred. */
+        sm::impl::TestAndResume(ResumeImpl);
+    }
+
+    /* This can never be reached. */
+    AMS_ASSUME(false);
 }