Merge pull request #4617 from wwylele/hle-service-clean

HLE/IPC: clean up System::GetInstance

src/core/hle/kernel/hle_ipc.cpp

@@ -48,7 +48,7 @@ SharedPtr<Event> HLERequestContext::SleepClientThread(SharedPtr<Thread> thread,
         // the translation might need to read from it in order to retrieve the StaticBuffer
         // target addresses.
         std::array<u32_le, IPC::COMMAND_BUFFER_LENGTH + 2 * IPC::MAX_STATIC_BUFFERS> cmd_buff;
-        Memory::MemorySystem& memory = Core::System::GetInstance().Memory();
+        Memory::MemorySystem& memory = context.kernel.memory;
         memory.ReadBlock(*process, thread->GetCommandBufferAddress(), cmd_buff.data(),
                          cmd_buff.size() * sizeof(u32));
         context.WriteToOutgoingCommandBuffer(cmd_buff.data(), *process);
@@ -57,8 +57,7 @@ SharedPtr<Event> HLERequestContext::SleepClientThread(SharedPtr<Thread> thread,
                           cmd_buff.size() * sizeof(u32));
     };
 
-    auto event = Core::System::GetInstance().Kernel().CreateEvent(Kernel::ResetType::OneShot,
-                                                                  "HLE Pause Event: " + reason);
+    auto event = kernel.CreateEvent(Kernel::ResetType::OneShot, "HLE Pause Event: " + reason);
     thread->status = ThreadStatus::WaitHleEvent;
     thread->wait_objects = {event};
     event->AddWaitingThread(thread);
@@ -69,8 +68,8 @@ SharedPtr<Event> HLERequestContext::SleepClientThread(SharedPtr<Thread> thread,
     return event;
 }
 
-HLERequestContext::HLERequestContext(SharedPtr<ServerSession> session)
-    : session(std::move(session)) {
+HLERequestContext::HLERequestContext(KernelSystem& kernel, SharedPtr<ServerSession> session)
+    : kernel(kernel), session(std::move(session)) {
     cmd_buf[0] = 0;
 }
 
@@ -143,8 +142,7 @@ ResultCode HLERequestContext::PopulateFromIncomingCommandBuffer(const u32_le* sr
 
             // Copy the input buffer into our own vector and store it.
             std::vector<u8> data(buffer_info.size);
-            Core::System::GetInstance().Memory().ReadBlock(src_process, source_address, data.data(),
-                                                           data.size());
+            kernel.memory.ReadBlock(src_process, source_address, data.data(), data.size());
 
             AddStaticBuffer(buffer_info.buffer_id, std::move(data));
             cmd_buf[i++] = source_address;
@@ -152,7 +150,8 @@ ResultCode HLERequestContext::PopulateFromIncomingCommandBuffer(const u32_le* sr
         }
         case IPC::DescriptorType::MappedBuffer: {
             u32 next_id = static_cast<u32>(request_mapped_buffers.size());
-            request_mapped_buffers.emplace_back(src_process, descriptor, src_cmdbuf[i], next_id);
+            request_mapped_buffers.emplace_back(kernel.memory, src_process, descriptor,
+                                                src_cmdbuf[i], next_id);
             cmd_buf[i++] = next_id;
             break;
         }
@@ -211,8 +210,7 @@ ResultCode HLERequestContext::WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf,
 
             ASSERT_MSG(target_descriptor.size >= data.size(), "Static buffer data is too big");
 
-            Core::System::GetInstance().Memory().WriteBlock(dst_process, target_address,
-                                                            data.data(), data.size());
+            kernel.memory.WriteBlock(dst_process, target_address, data.data(), data.size());
 
             dst_cmdbuf[i++] = target_address;
             break;
@@ -235,8 +233,9 @@ MappedBuffer& HLERequestContext::GetMappedBuffer(u32 id_from_cmdbuf) {
     return request_mapped_buffers[id_from_cmdbuf];
 }
 
-MappedBuffer::MappedBuffer(const Process& process, u32 descriptor, VAddr address, u32 id)
-    : id(id), address(address), process(&process) {
+MappedBuffer::MappedBuffer(Memory::MemorySystem& memory, const Process& process, u32 descriptor,
+                           VAddr address, u32 id)
+    : memory(&memory), id(id), address(address), process(&process) {
     IPC::MappedBufferDescInfo desc{descriptor};
     size = desc.size;
     perms = desc.perms;
@@ -245,15 +244,13 @@ MappedBuffer::MappedBuffer(const Process& process, u32 descriptor, VAddr address
 void MappedBuffer::Read(void* dest_buffer, std::size_t offset, std::size_t size) {
     ASSERT(perms & IPC::R);
     ASSERT(offset + size <= this->size);
-    Core::System::GetInstance().Memory().ReadBlock(*process, address + static_cast<VAddr>(offset),
-                                                   dest_buffer, size);
+    memory->ReadBlock(*process, address + static_cast<VAddr>(offset), dest_buffer, size);
 }
 
 void MappedBuffer::Write(const void* src_buffer, std::size_t offset, std::size_t size) {
     ASSERT(perms & IPC::W);
     ASSERT(offset + size <= this->size);
-    Core::System::GetInstance().Memory().WriteBlock(*process, address + static_cast<VAddr>(offset),
-                                                    src_buffer, size);
+    memory->WriteBlock(*process, address + static_cast<VAddr>(offset), src_buffer, size);
 }
 
 } // namespace Kernel

src/core/hle/kernel/hle_ipc.h

@@ -21,12 +21,18 @@ namespace Service {
 class ServiceFrameworkBase;
 }
 
+namespace Memory {
+class MemorySystem;
+}
+
 namespace Kernel {
 
 class HandleTable;
 class Process;
 class Thread;
 class Event;
+class HLERequestContext;
+class KernelSystem;
 
 /**
  * Interface implemented by HLE Session handlers.
@@ -39,13 +45,10 @@ public:
 
     /**
      * Handles a sync request from the emulated application.
-     * @param server_session The ServerSession that was triggered for this sync request,
-     * it should be used to differentiate which client (As in ClientSession) we're answering to.
-     * TODO(Subv): Use a wrapper structure to hold all the information relevant to
-     * this request (ServerSession, Originator thread, Translated command buffer, etc).
-     * @returns ResultCode the result code of the translate operation.
+     * @param context holds all the information relevant to his request (ServerSession, Translated
+     * command buffer, etc).
      */
-    virtual void HandleSyncRequest(SharedPtr<ServerSession> server_session) = 0;
+    virtual void HandleSyncRequest(Kernel::HLERequestContext& context) = 0;
 
     /**
      * Signals that a client has just connected to this HLE handler and keeps the
@@ -95,7 +98,8 @@ protected:
 
 class MappedBuffer {
 public:
-    MappedBuffer(const Process& process, u32 descriptor, VAddr address, u32 id);
+    MappedBuffer(Memory::MemorySystem& memory, const Process& process, u32 descriptor,
+                 VAddr address, u32 id);
 
     // interface for service
     void Read(void* dest_buffer, std::size_t offset, std::size_t size);
@@ -115,6 +119,7 @@ public:
 
 private:
     friend class HLERequestContext;
+    Memory::MemorySystem* memory;
     u32 id;
     VAddr address;
     const Process* process;
@@ -153,7 +158,7 @@ private:
  */
 class HLERequestContext {
 public:
-    HLERequestContext(SharedPtr<ServerSession> session);
+    HLERequestContext(KernelSystem& kernel, SharedPtr<ServerSession> session);
     ~HLERequestContext();
 
     /// Returns a pointer to the IPC command buffer for this request.
@@ -230,6 +235,7 @@ public:
     ResultCode WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf, Process& dst_process) const;
 
 private:
+    KernelSystem& kernel;
     std::array<u32, IPC::COMMAND_BUFFER_LENGTH> cmd_buf;
     SharedPtr<ServerSession> session;
     // TODO(yuriks): Check common usage of this and optimize size accordingly

src/core/hle/kernel/server_session.cpp

@@ -13,7 +13,7 @@
 
 namespace Kernel {
 
-ServerSession::ServerSession(KernelSystem& kernel) : WaitObject(kernel) {}
+ServerSession::ServerSession(KernelSystem& kernel) : WaitObject(kernel), kernel(kernel) {}
 ServerSession::~ServerSession() {
     // This destructor will be called automatically when the last ServerSession handle is closed by
     // the emulated application.
@@ -66,7 +66,26 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr<Thread> thread) {
 
     // If this ServerSession has an associated HLE handler, forward the request to it.
     if (hle_handler != nullptr) {
-        hle_handler->HandleSyncRequest(SharedPtr<ServerSession>(this));
+        std::array<u32_le, IPC::COMMAND_BUFFER_LENGTH + 2 * IPC::MAX_STATIC_BUFFERS> cmd_buf;
+        Kernel::Process* current_process = thread->owner_process;
+        kernel.memory.ReadBlock(*current_process, thread->GetCommandBufferAddress(), cmd_buf.data(),
+                                cmd_buf.size() * sizeof(u32));
+
+        Kernel::HLERequestContext context(kernel, this);
+        context.PopulateFromIncomingCommandBuffer(cmd_buf.data(), *current_process);
+
+        hle_handler->HandleSyncRequest(context);
+
+        ASSERT(thread->status == Kernel::ThreadStatus::Running ||
+               thread->status == Kernel::ThreadStatus::WaitHleEvent);
+        // Only write the response immediately if the thread is still running. If the HLE handler
+        // put the thread to sleep then the writing of the command buffer will be deferred to the
+        // wakeup callback.
+        if (thread->status == Kernel::ThreadStatus::Running) {
+            context.WriteToOutgoingCommandBuffer(cmd_buf.data(), *current_process);
+            kernel.memory.WriteBlock(*current_process, thread->GetCommandBufferAddress(),
+                                     cmd_buf.data(), cmd_buf.size() * sizeof(u32));
+        }
     }
 
     if (thread->status == ThreadStatus::Running) {

src/core/hle/kernel/server_session.h

@@ -102,6 +102,7 @@ private:
                                                       std::string name = "Unknown");
 
     friend class KernelSystem;
+    KernelSystem& kernel;
 };
 
 } // namespace Kernel

src/core/hle/service/service.cpp

@@ -166,42 +166,23 @@ void ServiceFrameworkBase::ReportUnimplementedFunction(u32* cmd_buf, const Funct
 
     LOG_ERROR(Service, "unknown / unimplemented {}", fmt::to_string(buf));
     // TODO(bunnei): Hack - ignore error
+    header.normal_params_size.Assign(1);
+    header.translate_params_size.Assign(0);
+    cmd_buf[0] = header.raw;
     cmd_buf[1] = 0;
 }
 
-void ServiceFrameworkBase::HandleSyncRequest(
-    Kernel::SharedPtr<Kernel::ServerSession> server_session) {
-    Kernel::KernelSystem& kernel = Core::System::GetInstance().Kernel();
-    auto thread = kernel.GetThreadManager().GetCurrentThread();
-    // TODO(wwylele): avoid GetPointer
-    u32* cmd_buf = reinterpret_cast<u32*>(
-        Core::System::GetInstance().Memory().GetPointer(thread->GetCommandBufferAddress()));
-
-    u32 header_code = cmd_buf[0];
+void ServiceFrameworkBase::HandleSyncRequest(Kernel::HLERequestContext& context) {
+    u32 header_code = context.CommandBuffer()[0];
     auto itr = handlers.find(header_code);
     const FunctionInfoBase* info = itr == handlers.end() ? nullptr : &itr->second;
     if (info == nullptr || info->handler_callback == nullptr) {
-        return ReportUnimplementedFunction(cmd_buf, info);
+        return ReportUnimplementedFunction(context.CommandBuffer(), info);
     }
 
-    Kernel::SharedPtr<Kernel::Process> current_process = kernel.GetCurrentProcess();
-
-    // TODO(yuriks): The kernel should be the one handling this as part of translation after
-    // everything else is migrated
-    Kernel::HLERequestContext context(std::move(server_session));
-    context.PopulateFromIncomingCommandBuffer(cmd_buf, *current_process);
-
-    LOG_TRACE(Service, "{}", MakeFunctionString(info->name, GetServiceName().c_str(), cmd_buf));
+    LOG_TRACE(Service, "{}",
+              MakeFunctionString(info->name, GetServiceName().c_str(), context.CommandBuffer()));
     handler_invoker(this, info->handler_callback, context);
-
-    ASSERT(thread->status == Kernel::ThreadStatus::Running ||
-           thread->status == Kernel::ThreadStatus::WaitHleEvent);
-    // Only write the response immediately if the thread is still running. If the HLE handler put
-    // the thread to sleep then the writing of the command buffer will be deferred to the wakeup
-    // callback.
-    if (thread->status == Kernel::ThreadStatus::Running) {
-        context.WriteToOutgoingCommandBuffer(cmd_buf, *current_process);
-    }
 }
 
 ////////////////////////////////////////////////////////////////////////////////////////////////////

src/core/hle/service/service.h

@@ -61,7 +61,7 @@ public:
     /// Creates a port pair and registers it on the kernel's global port registry.
     void InstallAsNamedPort(Kernel::KernelSystem& kernel);
 
-    void HandleSyncRequest(Kernel::SharedPtr<Kernel::ServerSession> server_session) override;
+    void HandleSyncRequest(Kernel::HLERequestContext& context) override;
 
 protected:
     /// Member-function pointer type of SyncRequest handlers.

src/tests/core/hle/kernel/hle_ipc.cpp

@@ -25,7 +25,7 @@ TEST_CASE("HLERequestContext::PopulateFromIncomingCommandBuffer", "[core][kernel
     Memory::MemorySystem memory;
     Kernel::KernelSystem kernel(memory, timing, [] {}, 0);
     auto session = std::get<SharedPtr<ServerSession>>(kernel.CreateSessionPair());
-    HLERequestContext context(std::move(session));
+    HLERequestContext context(kernel, std::move(session));
 
     auto process = kernel.CreateProcess(kernel.CreateCodeSet("", 0));
 
@@ -237,7 +237,7 @@ TEST_CASE("HLERequestContext::WriteToOutgoingCommandBuffer", "[core][kernel]") {
     Memory::MemorySystem memory;
     Kernel::KernelSystem kernel(memory, timing, [] {}, 0);
     auto session = std::get<SharedPtr<ServerSession>>(kernel.CreateSessionPair());
-    HLERequestContext context(std::move(session));
+    HLERequestContext context(kernel, std::move(session));
 
     auto process = kernel.CreateProcess(kernel.CreateCodeSet("", 0));
     auto* input = context.CommandBuffer();