From 36c8e1d7a9eddfef7f06cd6e234b9d63e3286643 Mon Sep 17 00:00:00 2001 From: Weiyi Wang Date: Sat, 2 Feb 2019 15:18:01 -0500 Subject: [PATCH 1/4] HLE/IPC: move command buffer translation into kernel as TODO says --- src/core/hle/kernel/hle_ipc.h | 10 ++++---- src/core/hle/kernel/server_session.cpp | 22 ++++++++++++++++-- src/core/hle/kernel/server_session.h | 1 + src/core/hle/service/service.cpp | 32 ++++---------------------- src/core/hle/service/service.h | 2 +- 5 files changed, 31 insertions(+), 36 deletions(-) diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index 4e0c31d98..55753dce2 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -27,6 +27,7 @@ class HandleTable; class Process; class Thread; class Event; +class HLERequestContext; /** * Interface implemented by HLE Session handlers. @@ -39,13 +40,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 server_session) = 0; + virtual void HandleSyncRequest(Kernel::HLERequestContext& context) = 0; /** * Signals that a client has just connected to this HLE handler and keeps the diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index 83e2ec4fa..c07bc9840 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/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,25 @@ 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)); + // TODO(wwylele): avoid GetPointer + u32* cmd_buf = + reinterpret_cast(kernel.memory.GetPointer(thread->GetCommandBufferAddress())); + + Kernel::Process* current_process = thread->owner_process; + + Kernel::HLERequestContext context(this); + context.PopulateFromIncomingCommandBuffer(cmd_buf, *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, *current_process); + } } if (thread->status == ThreadStatus::Running) { diff --git a/src/core/hle/kernel/server_session.h b/src/core/hle/kernel/server_session.h index 956deba3f..8fe9a6552 100644 --- a/src/core/hle/kernel/server_session.h +++ b/src/core/hle/kernel/server_session.h @@ -102,6 +102,7 @@ private: std::string name = "Unknown"); friend class KernelSystem; + KernelSystem& kernel; }; } // namespace Kernel diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index d88ce203f..97ff14338 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -169,39 +169,17 @@ void ServiceFrameworkBase::ReportUnimplementedFunction(u32* cmd_buf, const Funct cmd_buf[1] = 0; } -void ServiceFrameworkBase::HandleSyncRequest( - Kernel::SharedPtr server_session) { - Kernel::KernelSystem& kernel = Core::System::GetInstance().Kernel(); - auto thread = kernel.GetThreadManager().GetCurrentThread(); - // TODO(wwylele): avoid GetPointer - u32* cmd_buf = reinterpret_cast( - 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 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); - } } //////////////////////////////////////////////////////////////////////////////////////////////////// diff --git a/src/core/hle/service/service.h b/src/core/hle/service/service.h index 5f36f98de..4aa1cb600 100644 --- a/src/core/hle/service/service.h +++ b/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 server_session) override; + void HandleSyncRequest(Kernel::HLERequestContext& context) override; protected: /// Member-function pointer type of SyncRequest handlers. From 0a424b86d20cf7de81cb458625d5717707dd0000 Mon Sep 17 00:00:00 2001 From: Weiyi Wang Date: Sat, 2 Feb 2019 15:39:54 -0500 Subject: [PATCH 2/4] ServerSession: replace GetPointer with block copy for HLE translation Do it in the same way as HLERequestContext::SleepClientThread callback and avoid unsafe GetPointer --- src/core/hle/kernel/server_session.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index c07bc9840..27923ac5d 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -66,14 +66,13 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr thread) { // If this ServerSession has an associated HLE handler, forward the request to it. if (hle_handler != nullptr) { - // TODO(wwylele): avoid GetPointer - u32* cmd_buf = - reinterpret_cast(kernel.memory.GetPointer(thread->GetCommandBufferAddress())); - + std::array 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(this); - context.PopulateFromIncomingCommandBuffer(cmd_buf, *current_process); + context.PopulateFromIncomingCommandBuffer(cmd_buf.data(), *current_process); hle_handler->HandleSyncRequest(context); @@ -83,7 +82,9 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr thread) { // 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); + context.WriteToOutgoingCommandBuffer(cmd_buf.data(), *current_process); + kernel.memory.WriteBlock(*current_process, thread->GetCommandBufferAddress(), + cmd_buf.data(), cmd_buf.size() * sizeof(u32)); } } From 3f86be88f0107d2b29010bf0a9a2e962aa50a6b9 Mon Sep 17 00:00:00 2001 From: Weiyi Wang Date: Sat, 2 Feb 2019 15:55:45 -0500 Subject: [PATCH 3/4] HLE/IPC: pass in kernel & memory reference from parent to avoid global state reference --- src/core/hle/kernel/hle_ipc.cpp | 29 ++++++++++++-------------- src/core/hle/kernel/hle_ipc.h | 12 +++++++++-- src/core/hle/kernel/server_session.cpp | 2 +- src/tests/core/hle/kernel/hle_ipc.cpp | 4 ++-- 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp index 746da245e..618a9d210 100644 --- a/src/core/hle/kernel/hle_ipc.cpp +++ b/src/core/hle/kernel/hle_ipc.cpp @@ -48,7 +48,7 @@ SharedPtr HLERequestContext::SleepClientThread(SharedPtr thread, // the translation might need to read from it in order to retrieve the StaticBuffer // target addresses. std::array 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 HLERequestContext::SleepClientThread(SharedPtr 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 HLERequestContext::SleepClientThread(SharedPtr thread, return event; } -HLERequestContext::HLERequestContext(SharedPtr session) - : session(std::move(session)) { +HLERequestContext::HLERequestContext(KernelSystem& kernel, SharedPtr 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 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(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(offset), - dest_buffer, size); + memory->ReadBlock(*process, address + static_cast(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(offset), - src_buffer, size); + memory->WriteBlock(*process, address + static_cast(offset), src_buffer, size); } } // namespace Kernel diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index 55753dce2..78c459a9e 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -21,6 +21,10 @@ namespace Service { class ServiceFrameworkBase; } +namespace Memory { +class MemorySystem; +} + namespace Kernel { class HandleTable; @@ -28,6 +32,7 @@ class Process; class Thread; class Event; class HLERequestContext; +class KernelSystem; /** * Interface implemented by HLE Session handlers. @@ -93,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); @@ -113,6 +119,7 @@ public: private: friend class HLERequestContext; + Memory::MemorySystem* memory; u32 id; VAddr address; const Process* process; @@ -151,7 +158,7 @@ private: */ class HLERequestContext { public: - HLERequestContext(SharedPtr session); + HLERequestContext(KernelSystem& kernel, SharedPtr session); ~HLERequestContext(); /// Returns a pointer to the IPC command buffer for this request. @@ -228,6 +235,7 @@ public: ResultCode WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf, Process& dst_process) const; private: + KernelSystem& kernel; std::array cmd_buf; SharedPtr session; // TODO(yuriks): Check common usage of this and optimize size accordingly diff --git a/src/core/hle/kernel/server_session.cpp b/src/core/hle/kernel/server_session.cpp index 27923ac5d..dd940b724 100644 --- a/src/core/hle/kernel/server_session.cpp +++ b/src/core/hle/kernel/server_session.cpp @@ -71,7 +71,7 @@ ResultCode ServerSession::HandleSyncRequest(SharedPtr thread) { kernel.memory.ReadBlock(*current_process, thread->GetCommandBufferAddress(), cmd_buf.data(), cmd_buf.size() * sizeof(u32)); - Kernel::HLERequestContext context(this); + Kernel::HLERequestContext context(kernel, this); context.PopulateFromIncomingCommandBuffer(cmd_buf.data(), *current_process); hle_handler->HandleSyncRequest(context); diff --git a/src/tests/core/hle/kernel/hle_ipc.cpp b/src/tests/core/hle/kernel/hle_ipc.cpp index c22c66837..04937d9ea 100644 --- a/src/tests/core/hle/kernel/hle_ipc.cpp +++ b/src/tests/core/hle/kernel/hle_ipc.cpp @@ -26,7 +26,7 @@ TEST_CASE("HLERequestContext::PopulateFromIncomingCommandBuffer", "[core][kernel auto memory = std::make_unique(); Kernel::KernelSystem kernel(*memory, 0); auto session = std::get>(kernel.CreateSessionPair()); - HLERequestContext context(std::move(session)); + HLERequestContext context(kernel, std::move(session)); auto process = kernel.CreateProcess(kernel.CreateCodeSet("", 0)); @@ -239,7 +239,7 @@ TEST_CASE("HLERequestContext::WriteToOutgoingCommandBuffer", "[core][kernel]") { auto memory = std::make_unique(); Kernel::KernelSystem kernel(*memory, 0); auto session = std::get>(kernel.CreateSessionPair()); - HLERequestContext context(std::move(session)); + HLERequestContext context(kernel, std::move(session)); auto process = kernel.CreateProcess(kernel.CreateCodeSet("", 0)); auto* input = context.CommandBuffer(); From e6feef96c1c6588b5febaa76f26130473fad10b1 Mon Sep 17 00:00:00 2001 From: Weiyi Wang Date: Thu, 14 Feb 2019 22:16:50 -0500 Subject: [PATCH 4/4] Service: clear IPC header for unimplemented function stub --- src/core/hle/service/service.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index 97ff14338..3eb4dfcaa 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -166,6 +166,9 @@ 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; }