From 20e5abb30807d4e0e34c79c049252f0872e47ca7 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 8 Jun 2017 23:52:30 -0700 Subject: [PATCH 1/9] ServiceFramework: Use separate copy of command buffer Copy the IPC command buffer to/from the request context before/after the handler is invoked. This is part of a move away from using global data for handling IPC requests. --- src/core/hle/ipc.h | 3 +++ src/core/hle/kernel/hle_ipc.h | 9 ++++++--- src/core/hle/service/service.cpp | 26 ++++++++++++++++++++------ 3 files changed, 29 insertions(+), 9 deletions(-) diff --git a/src/core/hle/ipc.h b/src/core/hle/ipc.h index 303ca090d..f7f96125a 100644 --- a/src/core/hle/ipc.h +++ b/src/core/hle/ipc.h @@ -44,6 +44,9 @@ inline u32* GetStaticBuffers(const int offset = 0) { namespace IPC { +/// Size of the command buffer area, in 32-bit words. +constexpr size_t COMMAND_BUFFER_LENGTH = 0x100 / sizeof(u32); + // These errors are commonly returned by invalid IPC translations, so alias them here for // convenience. // TODO(yuriks): These will probably go away once translation is implemented inside the kernel. diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index c30184eab..aa0046001 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -4,8 +4,11 @@ #pragma once +#include #include #include +#include "common/common_types.h" +#include "core/hle/ipc.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/server_session.h" @@ -65,8 +68,8 @@ public: ~HLERequestContext(); /// Returns a pointer to the IPC command buffer for this request. - u32* CommandBuffer() const { - return cmd_buf; + u32* CommandBuffer() { + return cmd_buf.data(); } /** @@ -80,7 +83,7 @@ public: private: friend class Service::ServiceFrameworkBase; - u32* cmd_buf = nullptr; + std::array cmd_buf; SharedPtr session; }; diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index d34968428..35582b0ff 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -2,9 +2,12 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include #include +#include "common/assert.h" #include "common/logging/log.h" #include "common/string_util.h" +#include "core/hle/ipc.h" #include "core/hle/kernel/client_port.h" #include "core/hle/kernel/server_port.h" #include "core/hle/kernel/server_session.h" @@ -160,12 +163,6 @@ void ServiceFrameworkBase::ReportUnimplementedFunction(u32* cmd_buf, const Funct void ServiceFrameworkBase::HandleSyncRequest(SharedPtr server_session) { u32* cmd_buf = Kernel::GetCommandBuffer(); - // TODO(yuriks): The kernel should be the one handling this as part of translation after - // everything else is migrated - Kernel::HLERequestContext context; - context.cmd_buf = cmd_buf; - context.session = std::move(server_session); - u32 header_code = cmd_buf[0]; auto itr = handlers.find(header_code); const FunctionInfoBase* info = itr == handlers.end() ? nullptr : &itr->second; @@ -173,9 +170,26 @@ void ServiceFrameworkBase::HandleSyncRequest(SharedPtr server_ses return ReportUnimplementedFunction(cmd_buf, info); } + // TODO(yuriks): The kernel should be the one handling this as part of translation after + // everything else is migrated + IPC::Header request_header{cmd_buf[0]}; + size_t request_size = + 1 + request_header.normal_params_size + request_header.translate_params_size; + ASSERT(request_size <= IPC::COMMAND_BUFFER_LENGTH); // TODO(yuriks): Return error + + Kernel::HLERequestContext context; + std::copy_n(cmd_buf, request_size, context.cmd_buf.begin()); + context.session = std::move(server_session); + LOG_TRACE(Service, "%s", MakeFunctionString(info->name, GetServiceName().c_str(), cmd_buf).c_str()); handler_invoker(this, info->handler_callback, context); + + IPC::Header response_header{context.cmd_buf[0]}; + size_t response_size = + 1 + response_header.normal_params_size + response_header.translate_params_size; + ASSERT(response_size <= IPC::COMMAND_BUFFER_LENGTH); + std::copy_n(context.cmd_buf.begin(), response_size, cmd_buf); } //////////////////////////////////////////////////////////////////////////////////////////////////// From 05fee702117dd5000109257ac35efe28c820d828 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 8 Jun 2017 23:55:18 -0700 Subject: [PATCH 2/9] Kernel: Add methods in HLERequestContext abstracting handle creation --- src/core/hle/kernel/hle_ipc.cpp | 9 +++++++++ src/core/hle/kernel/hle_ipc.h | 3 +++ 2 files changed, 12 insertions(+) diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp index a60b8ef00..5a74645c7 100644 --- a/src/core/hle/kernel/hle_ipc.cpp +++ b/src/core/hle/kernel/hle_ipc.cpp @@ -5,6 +5,7 @@ #include #include "common/assert.h" #include "common/common_types.h" +#include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/hle_ipc.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/server_session.h" @@ -23,4 +24,12 @@ void SessionRequestHandler::ClientDisconnected(SharedPtr server_s HLERequestContext::~HLERequestContext() = default; +SharedPtr HLERequestContext::GetIncomingHandle(Handle id_from_cmdbuf) const { + return Kernel::g_handle_table.GetGeneric(id_from_cmdbuf); +} + +Handle HLERequestContext::AddOutgoingHandle(SharedPtr object) { + return Kernel::g_handle_table.Create(object).Unwrap(); +} + } // namespace Kernel diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index aa0046001..f23daa7ea 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -80,6 +80,9 @@ public: return session; } + SharedPtr GetIncomingHandle(Handle id_from_cmdbuf) const; + Handle AddOutgoingHandle(SharedPtr object); + private: friend class Service::ServiceFrameworkBase; From 1c4b0ebb1f5b5c7f2079f3460b4b468816b6c746 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Thu, 8 Jun 2017 21:30:39 -0700 Subject: [PATCH 3/9] IPC: Add basic HLERequestContext support to RequestParser/Builder --- src/core/hle/ipc_helpers.h | 33 ++++++++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h index d7348c09d..a3abc102e 100644 --- a/src/core/hle/ipc_helpers.h +++ b/src/core/hle/ipc_helpers.h @@ -6,17 +6,22 @@ #include "core/hle/ipc.h" #include "core/hle/kernel/handle_table.h" +#include "core/hle/kernel/hle_ipc.h" #include "core/hle/kernel/kernel.h" namespace IPC { class RequestHelperBase { protected: + Kernel::HLERequestContext* context = nullptr; u32* cmdbuf; ptrdiff_t index = 1; Header header; public: + RequestHelperBase(Kernel::HLERequestContext& context, Header desired_header) + : context(&context), cmdbuf(context.CommandBuffer()), header(desired_header) {} + RequestHelperBase(u32* command_buffer, Header command_header) : cmdbuf(command_buffer), header(command_header) {} @@ -51,12 +56,24 @@ public: class RequestBuilder : public RequestHelperBase { public: + RequestBuilder(Kernel::HLERequestContext& context, Header command_header) + : RequestHelperBase(context, command_header) { + cmdbuf[0] = header.raw; + } + + RequestBuilder(Kernel::HLERequestContext& context, u16 command_id, unsigned normal_params_size, + unsigned translate_params_size) + : RequestBuilder( + context, Header{MakeHeader(command_id, normal_params_size, translate_params_size)}) {} + RequestBuilder(u32* command_buffer, Header command_header) : RequestHelperBase(command_buffer, command_header) { cmdbuf[0] = header.raw; } + explicit RequestBuilder(u32* command_buffer, u32 command_header) : RequestBuilder(command_buffer, Header{command_header}) {} + RequestBuilder(u32* command_buffer, u16 command_id, unsigned normal_params_size, unsigned translate_params_size) : RequestBuilder(command_buffer, @@ -171,10 +188,21 @@ inline void RequestBuilder::PushMappedBuffer(VAddr buffer_vaddr, u32 size, class RequestParser : public RequestHelperBase { public: + RequestParser(Kernel::HLERequestContext& context, Header desired_header) + : RequestHelperBase(context, desired_header) {} + + RequestParser(Kernel::HLERequestContext& context, u16 command_id, unsigned normal_params_size, + unsigned translate_params_size) + : RequestParser(context, + Header{MakeHeader(command_id, normal_params_size, translate_params_size)}) { + } + RequestParser(u32* command_buffer, Header command_header) : RequestHelperBase(command_buffer, command_header) {} + explicit RequestParser(u32* command_buffer, u32 command_header) : RequestParser(command_buffer, Header{command_header}) {} + RequestParser(u32* command_buffer, u16 command_id, unsigned normal_params_size, unsigned translate_params_size) : RequestParser(command_buffer, @@ -186,7 +214,10 @@ public: ValidateHeader(); Header builderHeader{ MakeHeader(header.command_id, normal_params_size, translate_params_size)}; - return {cmdbuf, builderHeader}; + if (context != nullptr) + return {*context, builderHeader}; + else + return {cmdbuf, builderHeader}; } template From 21436f5ef7b833fd29871a815aa4e69197f4f730 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 9 Jun 2017 00:51:18 -0700 Subject: [PATCH 4/9] IPC: Add Pop/PushObjects methods to RequestParser/Builder These use the context functions to create and look-up handles for the user. --- src/core/hle/ipc_helpers.h | 113 +++++++++++++++++++++++++++++++++---- 1 file changed, 103 insertions(+), 10 deletions(-) diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h index a3abc102e..5f370bc3b 100644 --- a/src/core/hle/ipc_helpers.h +++ b/src/core/hle/ipc_helpers.h @@ -4,6 +4,10 @@ #pragma once +#include +#include +#include +#include #include "core/hle/ipc.h" #include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/hle_ipc.h" @@ -105,6 +109,9 @@ public: template void PushMoveHandles(H... handles); + template + void PushObjects(Kernel::SharedPtr... pointers); + void PushCurrentPIDHandle(); void PushStaticBuffer(VAddr buffer_vaddr, u32 size, u8 buffer_id); @@ -170,6 +177,11 @@ inline void RequestBuilder::PushMoveHandles(H... handles) { Push(static_cast(handles)...); } +template +inline void RequestBuilder::PushObjects(Kernel::SharedPtr... pointers) { + PushMoveHandles(context->AddOutgoingHandle(std::move(pointers))...); +} + inline void RequestBuilder::PushCurrentPIDHandle() { Push(CallingPidDesc()); Push(u32(0)); @@ -229,10 +241,52 @@ public: template void Pop(First& first_value, Other&... other_values); + /// Equivalent to calling `PopHandles<1>()[0]`. Kernel::Handle PopHandle(); + /** + * Pops a descriptor containing `N` handles. The handles are returned as an array. The + * descriptor must contain exactly `N` handles, it is not permitted to, for example, call + * PopHandles<1>() twice to read a multi-handle descriptor with 2 handles, or to make a single + * PopHandles<2>() call to read 2 single-handle descriptors. + */ + template + std::array PopHandles(); + + /// Convenience wrapper around PopHandles() which assigns the handles to the passed references. template - void PopHandles(H&... handles); + void PopHandles(H&... handles) { + std::tie(handles...) = PopHandles(); + } + + /// Equivalent to calling `PopGenericObjects<1>()[0]`. + Kernel::SharedPtr PopGenericObject(); + + /// Equivalent to calling `std::get<0>(PopObjects())`. + template + Kernel::SharedPtr PopObject(); + + /** + * Pop a descriptor containing `N` handles and resolves them to Kernel::Object pointers. If a + * handle is invalid, null is returned for that object instead. The same caveats from + * PopHandles() apply regarding `N` matching the number of handles in the descriptor. + */ + template + std::array, N> PopGenericObjects(); + + /** + * Resolves handles to Kernel::Objects as in PopGenericsObjects(), but then also casts them to + * the passed `T` types, while verifying that the cast is valid. If the type of an object does + * not match, null is returned instead. + */ + template + std::tuple...> PopObjects(); + + /// Convenience wrapper around PopObjects() which assigns the handles to the passed references. + template + void PopObjects(Kernel::SharedPtr&... pointers) { + std::tie(pointers...) = PopObjects(); + } /** * @brief Pops the static buffer vaddr @@ -344,15 +398,54 @@ inline Kernel::Handle RequestParser::PopHandle() { return Pop(); } -template -void RequestParser::PopHandles(H&... handles) { - const u32 handle_descriptor = Pop(); - const int handles_number = sizeof...(H); - DEBUG_ASSERT_MSG(IsHandleDescriptor(handle_descriptor), - "Tried to pop handle(s) but the descriptor is not a handle descriptor"); - DEBUG_ASSERT_MSG(handles_number == HandleNumberFromDesc(handle_descriptor), - "Number of handles doesn't match the descriptor"); - Pop(static_cast(handles)...); +template +std::array RequestParser::PopHandles() { + u32 handle_descriptor = Pop(); + ASSERT_MSG(IsHandleDescriptor(handle_descriptor), + "Tried to pop handle(s) but the descriptor is not a handle descriptor"); + ASSERT_MSG(N == HandleNumberFromDesc(handle_descriptor), + "Number of handles doesn't match the descriptor"); + + std::array handles{}; + for (Kernel::Handle& handle : handles) { + handle = Pop(); + } + return handles; +} + +inline Kernel::SharedPtr RequestParser::PopGenericObject() { + Kernel::Handle handle = PopHandle(); + return context->GetIncomingHandle(handle); +} + +template +Kernel::SharedPtr RequestParser::PopObject() { + return Kernel::DynamicObjectCast(PopGenericObject()); +} + +template +inline std::array, N> RequestParser::PopGenericObjects() { + std::array handles = PopHandles(); + std::array, N> pointers; + for (int i = 0; i < N; ++i) { + pointers[i] = context->GetIncomingHandle(handles[i]); + } + return pointers; +} + +namespace detail { +template +std::tuple...> PopObjectsHelper( + std::array, sizeof...(T)>&& pointers, + std::index_sequence) { + return std::make_tuple(Kernel::DynamicObjectCast(std::move(pointers[I]))...); +} +} // namespace detail + +template +inline std::tuple...> RequestParser::PopObjects() { + return detail::PopObjectsHelper(PopGenericObjects(), + std::index_sequence_for{}); } inline VAddr RequestParser::PopStaticBuffer(size_t* data_size, bool useStaticBuffersToGetVaddr) { From 7656d83df5e1f2d1f71af03341c23691522067b8 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 9 Jun 2017 00:54:08 -0700 Subject: [PATCH 5/9] Service/sm: Convert srv: to use IPC helpers --- src/core/hle/service/sm/srv.cpp | 105 +++++++++++++++++--------------- 1 file changed, 56 insertions(+), 49 deletions(-) diff --git a/src/core/hle/service/sm/srv.cpp b/src/core/hle/service/sm/srv.cpp index b8b62b068..74a1256e0 100644 --- a/src/core/hle/service/sm/srv.cpp +++ b/src/core/hle/service/sm/srv.cpp @@ -7,9 +7,11 @@ #include "common/common_types.h" #include "common/logging/log.h" #include "core/hle/ipc.h" +#include "core/hle/ipc_helpers.h" #include "core/hle/kernel/client_port.h" #include "core/hle/kernel/client_session.h" -#include "core/hle/kernel/handle_table.h" +#include "core/hle/kernel/errors.h" +#include "core/hle/kernel/hle_ipc.h" #include "core/hle/kernel/semaphore.h" #include "core/hle/kernel/server_session.h" #include "core/hle/service/sm/sm.h" @@ -30,15 +32,18 @@ constexpr int MAX_PENDING_NOTIFICATIONS = 16; * 1: ResultCode */ void SRV::RegisterClient(Kernel::HLERequestContext& ctx) { - u32* cmd_buff = ctx.CommandBuffer(); + IPC::RequestParser rp(ctx, 0x1, 0, 2); - if (cmd_buff[1] != IPC::CallingPidDesc()) { - cmd_buff[0] = IPC::MakeHeader(0x0, 0x1, 0); // 0x40 - cmd_buff[1] = IPC::ERR_INVALID_BUFFER_DESCRIPTOR.raw; + u32 pid_descriptor = rp.Pop(); + if (pid_descriptor != IPC::CallingPidDesc()) { + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(IPC::ERR_INVALID_BUFFER_DESCRIPTOR); return; } - cmd_buff[0] = IPC::MakeHeader(0x1, 0x1, 0); // 0x10040 - cmd_buff[1] = RESULT_SUCCESS.raw; // No error + u32 caller_pid = rp.Pop(); + + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(RESULT_SUCCESS); LOG_WARNING(Service_SRV, "(STUBBED) called"); } @@ -53,15 +58,14 @@ void SRV::RegisterClient(Kernel::HLERequestContext& ctx) { * 3: Handle to semaphore signaled on process notification */ void SRV::EnableNotification(Kernel::HLERequestContext& ctx) { - u32* cmd_buff = ctx.CommandBuffer(); + IPC::RequestParser rp(ctx, 0x2, 0, 0); notification_semaphore = Kernel::Semaphore::Create(0, MAX_PENDING_NOTIFICATIONS, "SRV:Notification").Unwrap(); - cmd_buff[0] = IPC::MakeHeader(0x2, 0x1, 0x2); // 0x20042 - cmd_buff[1] = RESULT_SUCCESS.raw; // No error - cmd_buff[2] = IPC::CopyHandleDesc(1); - cmd_buff[3] = Kernel::g_handle_table.Create(notification_semaphore).MoveFrom(); + IPC::RequestBuilder rb = rp.MakeBuilder(1, 2); + rb.Push(RESULT_SUCCESS); + rb.PushObjects(notification_semaphore); LOG_WARNING(Service_SRV, "(STUBBED) called"); } @@ -77,43 +81,49 @@ void SRV::EnableNotification(Kernel::HLERequestContext& ctx) { * 3: Service handle */ void SRV::GetServiceHandle(Kernel::HLERequestContext& ctx) { - ResultCode res = RESULT_SUCCESS; - u32* cmd_buff = ctx.CommandBuffer(); + IPC::RequestParser rp(ctx, 0x5, 4, 0); + auto name_buf = rp.PopRaw>(); + size_t name_len = rp.Pop(); + u32 flags = rp.Pop(); + + bool return_port_on_failure = (flags & 1) == 0; - size_t name_len = cmd_buff[3]; if (name_len > Service::kMaxPortSize) { - cmd_buff[1] = ERR_INVALID_NAME_SIZE.raw; - LOG_ERROR(Service_SRV, "called name_len=0x%X, failed with code=0x%08X", name_len, - cmd_buff[1]); + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(ERR_INVALID_NAME_SIZE); + LOG_ERROR(Service_SRV, "called name_len=0x%X -> ERR_INVALID_NAME_SIZE", name_len); return; } - std::string name(reinterpret_cast(&cmd_buff[1]), name_len); - bool return_port_on_failure = (cmd_buff[4] & 1) == 0; + std::string name(name_buf.data(), name_len); // TODO(yuriks): Permission checks go here auto client_port = service_manager->GetServicePort(name); if (client_port.Failed()) { - cmd_buff[1] = client_port.Code().raw; - LOG_ERROR(Service_SRV, "called service=%s, failed with code=0x%08X", name.c_str(), - cmd_buff[1]); + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(client_port.Code()); + LOG_ERROR(Service_SRV, "called service=%s -> error 0x%08X", name.c_str(), + client_port.Code().raw); return; } auto session = client_port.Unwrap()->Connect(); - cmd_buff[1] = session.Code().raw; if (session.Succeeded()) { - cmd_buff[3] = Kernel::g_handle_table.Create(session.MoveFrom()).MoveFrom(); - LOG_DEBUG(Service_SRV, "called service=%s, session handle=0x%08X", name.c_str(), - cmd_buff[3]); + LOG_DEBUG(Service_SRV, "called service=%s -> session=%u", name.c_str(), + (*session)->GetObjectId()); + IPC::RequestBuilder rb = rp.MakeBuilder(1, 2); + rb.Push(session.Code()); + rb.PushObjects(session.MoveFrom()); } else if (session.Code() == Kernel::ERR_MAX_CONNECTIONS_REACHED && return_port_on_failure) { - cmd_buff[1] = ERR_MAX_CONNECTIONS_REACHED.raw; - cmd_buff[3] = Kernel::g_handle_table.Create(client_port.MoveFrom()).MoveFrom(); - LOG_WARNING(Service_SRV, "called service=%s, *port* handle=0x%08X", name.c_str(), - cmd_buff[3]); + LOG_WARNING(Service_SRV, "called service=%s -> ERR_MAX_CONNECTIONS_REACHED, *port*=%u", + name.c_str(), (*client_port)->GetObjectId()); + IPC::RequestBuilder rb = rp.MakeBuilder(1, 2); + rb.Push(ERR_MAX_CONNECTIONS_REACHED); + rb.PushObjects(client_port.MoveFrom()); } else { - LOG_ERROR(Service_SRV, "called service=%s, failed with code=0x%08X", name.c_str(), - cmd_buff[1]); + LOG_ERROR(Service_SRV, "called service=%s -> error 0x%08X", name.c_str(), session.Code()); + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(session.Code()); } } @@ -127,12 +137,11 @@ void SRV::GetServiceHandle(Kernel::HLERequestContext& ctx) { * 1: ResultCode */ void SRV::Subscribe(Kernel::HLERequestContext& ctx) { - u32* cmd_buff = ctx.CommandBuffer(); + IPC::RequestParser rp(ctx, 0x9, 1, 0); + u32 notification_id = rp.Pop(); - u32 notification_id = cmd_buff[1]; - - cmd_buff[0] = IPC::MakeHeader(0x9, 0x1, 0); // 0x90040 - cmd_buff[1] = RESULT_SUCCESS.raw; // No error + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(RESULT_SUCCESS); LOG_WARNING(Service_SRV, "(STUBBED) called, notification_id=0x%X", notification_id); } @@ -146,12 +155,11 @@ void SRV::Subscribe(Kernel::HLERequestContext& ctx) { * 1: ResultCode */ void SRV::Unsubscribe(Kernel::HLERequestContext& ctx) { - u32* cmd_buff = ctx.CommandBuffer(); + IPC::RequestParser rp(ctx, 0xA, 1, 0); + u32 notification_id = rp.Pop(); - u32 notification_id = cmd_buff[1]; - - cmd_buff[0] = IPC::MakeHeader(0xA, 0x1, 0); // 0xA0040 - cmd_buff[1] = RESULT_SUCCESS.raw; // No error + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(RESULT_SUCCESS); LOG_WARNING(Service_SRV, "(STUBBED) called, notification_id=0x%X", notification_id); } @@ -166,13 +174,12 @@ void SRV::Unsubscribe(Kernel::HLERequestContext& ctx) { * 1: ResultCode */ void SRV::PublishToSubscriber(Kernel::HLERequestContext& ctx) { - u32* cmd_buff = ctx.CommandBuffer(); + IPC::RequestParser rp(ctx, 0xC, 2, 0); + u32 notification_id = rp.Pop(); + u8 flags = rp.Pop(); - u32 notification_id = cmd_buff[1]; - u8 flags = cmd_buff[2] & 0xFF; - - cmd_buff[0] = IPC::MakeHeader(0xC, 0x1, 0); // 0xC0040 - cmd_buff[1] = RESULT_SUCCESS.raw; // No error + IPC::RequestBuilder rb = rp.MakeBuilder(1, 0); + rb.Push(RESULT_SUCCESS); LOG_WARNING(Service_SRV, "(STUBBED) called, notification_id=0x%X, flags=%u", notification_id, flags); } From 8cb65fe65a48988d40dd3ca2c4f673629a275b4b Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Fri, 9 Jun 2017 05:23:13 -0700 Subject: [PATCH 6/9] Kernel: Basic support for IPC translation for HLE services --- src/core/hle/kernel/hle_ipc.cpp | 94 ++++++++++++++++++++++++++++++-- src/core/hle/kernel/hle_ipc.h | 37 ++++++++++++- src/core/hle/service/service.cpp | 17 ++---- 3 files changed, 130 insertions(+), 18 deletions(-) diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp index 5a74645c7..656405dd6 100644 --- a/src/core/hle/kernel/hle_ipc.cpp +++ b/src/core/hle/kernel/hle_ipc.cpp @@ -8,6 +8,7 @@ #include "core/hle/kernel/handle_table.h" #include "core/hle/kernel/hle_ipc.h" #include "core/hle/kernel/kernel.h" +#include "core/hle/kernel/process.h" #include "core/hle/kernel/server_session.h" namespace Kernel { @@ -24,12 +25,97 @@ void SessionRequestHandler::ClientDisconnected(SharedPtr server_s HLERequestContext::~HLERequestContext() = default; -SharedPtr HLERequestContext::GetIncomingHandle(Handle id_from_cmdbuf) const { - return Kernel::g_handle_table.GetGeneric(id_from_cmdbuf); +SharedPtr HLERequestContext::GetIncomingHandle(u32 id_from_cmdbuf) const { + ASSERT(id_from_cmdbuf < request_handles.size()); + return request_handles[id_from_cmdbuf]; } -Handle HLERequestContext::AddOutgoingHandle(SharedPtr object) { - return Kernel::g_handle_table.Create(object).Unwrap(); +u32 HLERequestContext::AddOutgoingHandle(SharedPtr object) { + request_handles.push_back(std::move(object)); + return request_handles.size() - 1; +} + +ResultCode HLERequestContext::PopulateFromIncomingCommandBuffer(const u32_le* src_cmdbuf, + Process& src_process, + HandleTable& src_table) { + IPC::Header header{src_cmdbuf[0]}; + + size_t untranslated_size = 1u + header.normal_params_size; + size_t command_size = untranslated_size + header.translate_params_size; + ASSERT(command_size <= IPC::COMMAND_BUFFER_LENGTH); // TODO(yuriks): Return error + + std::copy_n(src_cmdbuf, untranslated_size, cmd_buf.begin()); + + size_t i = untranslated_size; + while (i < command_size) { + u32 descriptor = cmd_buf[i] = src_cmdbuf[i]; + i += 1; + + switch (IPC::GetDescriptorType(descriptor)) { + case IPC::DescriptorType::CopyHandle: + case IPC::DescriptorType::MoveHandle: { + u32 num_handles = IPC::HandleNumberFromDesc(descriptor); + ASSERT(i + num_handles <= command_size); // TODO(yuriks): Return error + for (u32 j = 0; j < num_handles; ++j) { + Handle handle = src_cmdbuf[i]; + SharedPtr object = src_table.GetGeneric(handle); + ASSERT(object != nullptr); // TODO(yuriks): Return error + if (descriptor == IPC::DescriptorType::MoveHandle) { + src_table.Close(handle); + } + + cmd_buf[i++] = AddOutgoingHandle(std::move(object)); + } + break; + } + case IPC::DescriptorType::CallingPid: { + cmd_buf[i++] = src_process.process_id; + break; + } + default: + UNIMPLEMENTED_MSG("Unsupported handle translation: 0x%08X", descriptor); + } + } + + return RESULT_SUCCESS; +} + +ResultCode HLERequestContext::WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf, Process& dst_process, + HandleTable& dst_table) const { + IPC::Header header{cmd_buf[0]}; + + size_t untranslated_size = 1u + header.normal_params_size; + size_t command_size = untranslated_size + header.translate_params_size; + ASSERT(command_size <= IPC::COMMAND_BUFFER_LENGTH); + + std::copy_n(cmd_buf.begin(), untranslated_size, dst_cmdbuf); + + size_t i = untranslated_size; + while (i < command_size) { + u32 descriptor = dst_cmdbuf[i] = cmd_buf[i]; + i += 1; + + switch (IPC::GetDescriptorType(descriptor)) { + case IPC::DescriptorType::CopyHandle: + case IPC::DescriptorType::MoveHandle: { + // HLE services don't use handles, so we treat both CopyHandle and MoveHandle equally + u32 num_handles = IPC::HandleNumberFromDesc(descriptor); + ASSERT(i + num_handles <= command_size); + for (u32 j = 0; j < num_handles; ++j) { + SharedPtr object = GetIncomingHandle(cmd_buf[i]); + + // TODO(yuriks): Figure out the proper error handling for if this fails + Handle handle = dst_table.Create(object).Unwrap(); + dst_cmdbuf[i++] = handle; + } + break; + } + default: + UNIMPLEMENTED_MSG("Unsupported handle translation: 0x%08X", descriptor); + } + } + + return RESULT_SUCCESS; } } // namespace Kernel diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index f23daa7ea..d6ebf113c 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -8,6 +8,7 @@ #include #include #include "common/common_types.h" +#include "common/swap.h" #include "core/hle/ipc.h" #include "core/hle/kernel/kernel.h" #include "core/hle/kernel/server_session.h" @@ -18,6 +19,9 @@ class ServiceFrameworkBase; namespace Kernel { +class HandleTable; +class Process; + /** * Interface implemented by HLE Session handlers. * This can be provided to a ServerSession in order to hook into several relevant events @@ -62,6 +66,20 @@ protected: * Class containing information about an in-flight IPC request being handled by an HLE service * implementation. Services should avoid using old global APIs (e.g. Kernel::GetCommandBuffer()) and * when possible use the APIs in this class to service the request. + * + * HLE handle protocol + * =================== + * + * To avoid needing HLE services to keep a separate handle table, or having to directly modify the + * requester's table, a tweaked protocol is used to receive and send handles in requests. The kernel + * will decode the incoming handles into object pointers and insert a id in the buffer where the + * handle would normally be. The service then calls GetIncomingHandle() with that id to get the + * pointer to the object. Similarly, instead of inserting a handle into the command buffer, the + * service calls AddOutgoingHandle() and stores the returned id where the handle would normally go. + * + * The end result is similar to just giving services their own real handle tables, but since these + * ids are local to a specific context, it avoids requiring services to manage handles for objects + * across multiple calls and ensuring that unneeded handles are cleaned up. */ class HLERequestContext { public: @@ -80,14 +98,29 @@ public: return session; } - SharedPtr GetIncomingHandle(Handle id_from_cmdbuf) const; - Handle AddOutgoingHandle(SharedPtr object); + /** + * Resolves a object id from the request command buffer into a pointer to an object. See the + * "HLE handle protocol" section in the class documentation for more details. + */ + SharedPtr GetIncomingHandle(u32 id_from_cmdbuf) const; + + /** + * Adds an outgoing object to the response, returning the id which should be used to reference + * it. See the "HLE handle protocol" section in the class documentation for more details. + */ + u32 AddOutgoingHandle(SharedPtr object); private: friend class Service::ServiceFrameworkBase; + ResultCode PopulateFromIncomingCommandBuffer(const u32_le* src_cmdbuf, Process& src_process, + HandleTable& src_table); + ResultCode WriteToOutgoingCommandBuffer(u32_le* dst_cmdbuf, Process& dst_process, + HandleTable& dst_table) const; + std::array cmd_buf; SharedPtr session; + std::vector> request_handles; }; } // namespace Kernel diff --git a/src/core/hle/service/service.cpp b/src/core/hle/service/service.cpp index 35582b0ff..791a65c19 100644 --- a/src/core/hle/service/service.cpp +++ b/src/core/hle/service/service.cpp @@ -9,6 +9,7 @@ #include "common/string_util.h" #include "core/hle/ipc.h" #include "core/hle/kernel/client_port.h" +#include "core/hle/kernel/process.h" #include "core/hle/kernel/server_port.h" #include "core/hle/kernel/server_session.h" #include "core/hle/service/ac/ac.h" @@ -172,24 +173,16 @@ void ServiceFrameworkBase::HandleSyncRequest(SharedPtr server_ses // TODO(yuriks): The kernel should be the one handling this as part of translation after // everything else is migrated - IPC::Header request_header{cmd_buf[0]}; - size_t request_size = - 1 + request_header.normal_params_size + request_header.translate_params_size; - ASSERT(request_size <= IPC::COMMAND_BUFFER_LENGTH); // TODO(yuriks): Return error - Kernel::HLERequestContext context; - std::copy_n(cmd_buf, request_size, context.cmd_buf.begin()); context.session = std::move(server_session); + context.PopulateFromIncomingCommandBuffer(cmd_buf, *Kernel::g_current_process, + Kernel::g_handle_table); LOG_TRACE(Service, "%s", MakeFunctionString(info->name, GetServiceName().c_str(), cmd_buf).c_str()); handler_invoker(this, info->handler_callback, context); - - IPC::Header response_header{context.cmd_buf[0]}; - size_t response_size = - 1 + response_header.normal_params_size + response_header.translate_params_size; - ASSERT(response_size <= IPC::COMMAND_BUFFER_LENGTH); - std::copy_n(context.cmd_buf.begin(), response_size, cmd_buf); + context.WriteToOutgoingCommandBuffer(cmd_buf, *Kernel::g_current_process, + Kernel::g_handle_table); } //////////////////////////////////////////////////////////////////////////////////////////////////// From 92ca422088eb49c31e8b6146872548c456e56f3e Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sat, 10 Jun 2017 17:57:08 -0700 Subject: [PATCH 7/9] Kernel: Allow clearing request_objects to re-use buffer space Reduces the necessary allocation to max(in_handles, out_handles) rather than (in_handles + out_handles). --- src/core/hle/ipc_helpers.h | 3 +++ src/core/hle/kernel/hle_ipc.cpp | 4 ++++ src/core/hle/kernel/hle_ipc.h | 7 +++++++ 3 files changed, 14 insertions(+) diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h index 5f370bc3b..f0d89cffe 100644 --- a/src/core/hle/ipc_helpers.h +++ b/src/core/hle/ipc_helpers.h @@ -62,6 +62,9 @@ class RequestBuilder : public RequestHelperBase { public: RequestBuilder(Kernel::HLERequestContext& context, Header command_header) : RequestHelperBase(context, command_header) { + // From this point we will start overwriting the existing command buffer, so it's safe to + // release all previous incoming Object pointers since they won't be usable anymore. + context.ClearIncomingObjects(); cmdbuf[0] = header.raw; } diff --git a/src/core/hle/kernel/hle_ipc.cpp b/src/core/hle/kernel/hle_ipc.cpp index 656405dd6..6cf1886cf 100644 --- a/src/core/hle/kernel/hle_ipc.cpp +++ b/src/core/hle/kernel/hle_ipc.cpp @@ -35,6 +35,10 @@ u32 HLERequestContext::AddOutgoingHandle(SharedPtr object) { return request_handles.size() - 1; } +void HLERequestContext::ClearIncomingObjects() { + request_handles.clear(); +} + ResultCode HLERequestContext::PopulateFromIncomingCommandBuffer(const u32_le* src_cmdbuf, Process& src_process, HandleTable& src_table) { diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index d6ebf113c..1022dece8 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -110,6 +110,13 @@ public: */ u32 AddOutgoingHandle(SharedPtr object); + /** + * Discards all Objects from the context, invalidating all ids. This may be called after reading + * out all incoming objects, so that the buffer memory can be re-used for outgoing handles, but + * this is not required. + */ + void ClearIncomingObjects(); + private: friend class Service::ServiceFrameworkBase; From 60d70c4f43f64a147e30396108d5ae2d16aabeaf Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sun, 11 Jun 2017 12:47:03 -0700 Subject: [PATCH 8/9] Externals: Upgrade bundled Boost to 1.64 --- externals/boost | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/externals/boost b/externals/boost index 351972396..3abc84aba 160000 --- a/externals/boost +++ b/externals/boost @@ -1 +1 @@ -Subproject commit 351972396392c97a659b9a02f34ce9269293d21f +Subproject commit 3abc84abaf63a068cb59a9f9b5675c1947bc6fd9 From d8f6000f5434b94412ac5738f8f72ba7ad9f8497 Mon Sep 17 00:00:00 2001 From: Yuri Kunde Schlesner Date: Sun, 11 Jun 2017 12:51:05 -0700 Subject: [PATCH 9/9] Kernel/IPC: Use boost::small_vector for HLE context objects --- src/core/hle/kernel/hle_ipc.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/core/hle/kernel/hle_ipc.h b/src/core/hle/kernel/hle_ipc.h index 1022dece8..cbb109d8f 100644 --- a/src/core/hle/kernel/hle_ipc.h +++ b/src/core/hle/kernel/hle_ipc.h @@ -7,6 +7,7 @@ #include #include #include +#include #include "common/common_types.h" #include "common/swap.h" #include "core/hle/ipc.h" @@ -127,7 +128,8 @@ private: std::array cmd_buf; SharedPtr session; - std::vector> request_handles; + // TODO(yuriks): Check common usage of this and optimize size accordingly + boost::container::small_vector, 8> request_handles; }; } // namespace Kernel