From 1b2872eebc84ef3b35f8c0456ccb22519059d66a Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 26 Feb 2019 17:20:02 -0500 Subject: [PATCH 1/3] service/vi: Remove use of a module class This didn't really provide much benefit here, especially since the subsequent change requires that the behavior for each service's GetDisplayService differs in a minor detail. This also arguably makes the services nicer to read, since it gets rid of an indirection in the class hierarchy. --- src/core/hle/service/vi/vi.cpp | 21 +++++++-------------- src/core/hle/service/vi/vi.h | 31 ++++++++++++++----------------- src/core/hle/service/vi/vi_m.cpp | 12 ++++++++++-- src/core/hle/service/vi/vi_m.h | 19 ++++++++++++++++--- src/core/hle/service/vi/vi_s.cpp | 12 ++++++++++-- src/core/hle/service/vi/vi_s.h | 19 ++++++++++++++++--- src/core/hle/service/vi/vi_u.cpp | 12 ++++++++++-- src/core/hle/service/vi/vi_u.h | 19 ++++++++++++++++--- 8 files changed, 99 insertions(+), 46 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 7369a09ecc..99340e2ed1 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -24,6 +24,7 @@ #include "core/hle/service/nvdrv/nvdrv.h" #include "core/hle/service/nvflinger/buffer_queue.h" #include "core/hle/service/nvflinger/nvflinger.h" +#include "core/hle/service/service.h" #include "core/hle/service/vi/vi.h" #include "core/hle/service/vi/vi_m.h" #include "core/hle/service/vi/vi_s.h" @@ -1202,26 +1203,18 @@ IApplicationDisplayService::IApplicationDisplayService( RegisterHandlers(functions); } -Module::Interface::Interface(std::shared_ptr module, const char* name, - std::shared_ptr nv_flinger) - : ServiceFramework(name), module(std::move(module)), nv_flinger(std::move(nv_flinger)) {} - -Module::Interface::~Interface() = default; - -void Module::Interface::GetDisplayService(Kernel::HLERequestContext& ctx) { - LOG_WARNING(Service_VI, "(STUBBED) called"); - +void detail::GetDisplayServiceImpl(Kernel::HLERequestContext& ctx, + std::shared_ptr nv_flinger) { IPC::ResponseBuilder rb{ctx, 2, 0, 1}; rb.Push(RESULT_SUCCESS); - rb.PushIpcInterface(nv_flinger); + rb.PushIpcInterface(std::move(nv_flinger)); } void InstallInterfaces(SM::ServiceManager& service_manager, std::shared_ptr nv_flinger) { - auto module = std::make_shared(); - std::make_shared(module, nv_flinger)->InstallAsService(service_manager); - std::make_shared(module, nv_flinger)->InstallAsService(service_manager); - std::make_shared(module, nv_flinger)->InstallAsService(service_manager); + std::make_shared(nv_flinger)->InstallAsService(service_manager); + std::make_shared(nv_flinger)->InstallAsService(service_manager); + std::make_shared(nv_flinger)->InstallAsService(service_manager); } } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi.h b/src/core/hle/service/vi/vi.h index e3963502a7..c5682accc9 100644 --- a/src/core/hle/service/vi/vi.h +++ b/src/core/hle/service/vi/vi.h @@ -4,13 +4,26 @@ #pragma once -#include "core/hle/service/service.h" +#include +#include "common/common_types.h" + +namespace Kernel { +class HLERequestContext; +} namespace Service::NVFlinger { class NVFlinger; } +namespace Service::SM { +class ServiceManager; +} + namespace Service::VI { +namespace detail { +void GetDisplayServiceImpl(Kernel::HLERequestContext& ctx, + std::shared_ptr nv_flinger); +} enum class DisplayResolution : u32 { DockedWidth = 1920, @@ -19,22 +32,6 @@ enum class DisplayResolution : u32 { UndockedHeight = 720, }; -class Module final { -public: - class Interface : public ServiceFramework { - public: - explicit Interface(std::shared_ptr module, const char* name, - std::shared_ptr nv_flinger); - ~Interface() override; - - void GetDisplayService(Kernel::HLERequestContext& ctx); - - protected: - std::shared_ptr module; - std::shared_ptr nv_flinger; - }; -}; - /// Registers all VI services with the specified service manager. void InstallInterfaces(SM::ServiceManager& service_manager, std::shared_ptr nv_flinger); diff --git a/src/core/hle/service/vi/vi_m.cpp b/src/core/hle/service/vi/vi_m.cpp index 207c06b16e..6e3e0bd8f5 100644 --- a/src/core/hle/service/vi/vi_m.cpp +++ b/src/core/hle/service/vi/vi_m.cpp @@ -2,12 +2,14 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include "common/logging/log.h" +#include "core/hle/service/vi/vi.h" #include "core/hle/service/vi/vi_m.h" namespace Service::VI { -VI_M::VI_M(std::shared_ptr module, std::shared_ptr nv_flinger) - : Module::Interface(std::move(module), "vi:m", std::move(nv_flinger)) { +VI_M::VI_M(std::shared_ptr nv_flinger) + : ServiceFramework{"vi:m"}, nv_flinger{std::move(nv_flinger)} { static const FunctionInfo functions[] = { {2, &VI_M::GetDisplayService, "GetDisplayService"}, {3, nullptr, "GetDisplayServiceWithProxyNameExchange"}, @@ -17,4 +19,10 @@ VI_M::VI_M(std::shared_ptr module, std::shared_ptr VI_M::~VI_M() = default; +void VI_M::GetDisplayService(Kernel::HLERequestContext& ctx) { + LOG_WARNING(Service_VI, "(STUBBED) called"); + + detail::GetDisplayServiceImpl(ctx, nv_flinger); +} + } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_m.h b/src/core/hle/service/vi/vi_m.h index 487d58d509..290e066891 100644 --- a/src/core/hle/service/vi/vi_m.h +++ b/src/core/hle/service/vi/vi_m.h @@ -4,14 +4,27 @@ #pragma once -#include "core/hle/service/vi/vi.h" +#include "core/hle/service/service.h" + +namespace Kernel { +class HLERequestContext; +} + +namespace Service::NVFlinger { +class NVFlinger; +} namespace Service::VI { -class VI_M final : public Module::Interface { +class VI_M final : public ServiceFramework { public: - explicit VI_M(std::shared_ptr module, std::shared_ptr nv_flinger); + explicit VI_M(std::shared_ptr nv_flinger); ~VI_M() override; + +private: + void GetDisplayService(Kernel::HLERequestContext& ctx); + + std::shared_ptr nv_flinger; }; } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_s.cpp b/src/core/hle/service/vi/vi_s.cpp index 920e6a1f69..6dd700eaec 100644 --- a/src/core/hle/service/vi/vi_s.cpp +++ b/src/core/hle/service/vi/vi_s.cpp @@ -2,12 +2,14 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include "common/logging/log.h" +#include "core/hle/service/vi/vi.h" #include "core/hle/service/vi/vi_s.h" namespace Service::VI { -VI_S::VI_S(std::shared_ptr module, std::shared_ptr nv_flinger) - : Module::Interface(std::move(module), "vi:s", std::move(nv_flinger)) { +VI_S::VI_S(std::shared_ptr nv_flinger) + : ServiceFramework{"vi:s"}, nv_flinger{std::move(nv_flinger)} { static const FunctionInfo functions[] = { {1, &VI_S::GetDisplayService, "GetDisplayService"}, {3, nullptr, "GetDisplayServiceWithProxyNameExchange"}, @@ -17,4 +19,10 @@ VI_S::VI_S(std::shared_ptr module, std::shared_ptr VI_S::~VI_S() = default; +void VI_S::GetDisplayService(Kernel::HLERequestContext& ctx) { + LOG_WARNING(Service_VI, "(STUBBED) called"); + + detail::GetDisplayServiceImpl(ctx, nv_flinger); +} + } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_s.h b/src/core/hle/service/vi/vi_s.h index bbc31148f4..47804dc0be 100644 --- a/src/core/hle/service/vi/vi_s.h +++ b/src/core/hle/service/vi/vi_s.h @@ -4,14 +4,27 @@ #pragma once -#include "core/hle/service/vi/vi.h" +#include "core/hle/service/service.h" + +namespace Kernel { +class HLERequestContext; +} + +namespace Service::NVFlinger { +class NVFlinger; +} namespace Service::VI { -class VI_S final : public Module::Interface { +class VI_S final : public ServiceFramework { public: - explicit VI_S(std::shared_ptr module, std::shared_ptr nv_flinger); + explicit VI_S(std::shared_ptr nv_flinger); ~VI_S() override; + +private: + void GetDisplayService(Kernel::HLERequestContext& ctx); + + std::shared_ptr nv_flinger; }; } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_u.cpp b/src/core/hle/service/vi/vi_u.cpp index d81e410d6d..ef09a5df5d 100644 --- a/src/core/hle/service/vi/vi_u.cpp +++ b/src/core/hle/service/vi/vi_u.cpp @@ -2,12 +2,14 @@ // Licensed under GPLv2 or any later version // Refer to the license.txt file included. +#include "common/logging/log.h" +#include "core/hle/service/vi/vi.h" #include "core/hle/service/vi/vi_u.h" namespace Service::VI { -VI_U::VI_U(std::shared_ptr module, std::shared_ptr nv_flinger) - : Module::Interface(std::move(module), "vi:u", std::move(nv_flinger)) { +VI_U::VI_U(std::shared_ptr nv_flinger) + : ServiceFramework{"vi:u"}, nv_flinger{std::move(nv_flinger)} { static const FunctionInfo functions[] = { {0, &VI_U::GetDisplayService, "GetDisplayService"}, }; @@ -16,4 +18,10 @@ VI_U::VI_U(std::shared_ptr module, std::shared_ptr VI_U::~VI_U() = default; +void VI_U::GetDisplayService(Kernel::HLERequestContext& ctx) { + LOG_WARNING(Service_VI, "(STUBBED) called"); + + detail::GetDisplayServiceImpl(ctx, nv_flinger); +} + } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_u.h b/src/core/hle/service/vi/vi_u.h index b92f28c92a..19bdb73b0b 100644 --- a/src/core/hle/service/vi/vi_u.h +++ b/src/core/hle/service/vi/vi_u.h @@ -4,14 +4,27 @@ #pragma once -#include "core/hle/service/vi/vi.h" +#include "core/hle/service/service.h" + +namespace Kernel { +class HLERequestContext; +} + +namespace Service::NVFlinger { +class NVFlinger; +} namespace Service::VI { -class VI_U final : public Module::Interface { +class VI_U final : public ServiceFramework { public: - explicit VI_U(std::shared_ptr module, std::shared_ptr nv_flinger); + explicit VI_U(std::shared_ptr nv_flinger); ~VI_U() override; + +private: + void GetDisplayService(Kernel::HLERequestContext& ctx); + + std::shared_ptr nv_flinger; }; } // namespace Service::VI From 254b1e3df731dd47f73b9a0267bfb6b9858fd849 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 26 Feb 2019 18:04:45 -0500 Subject: [PATCH 2/3] core/ipc_helper: Allow popping all signed value types with RequestParser There's no real reason this shouldn't be allowed, given some values sent via a request can be signed. This also makes it less annoying to work with popping enum values, given an enum class with no type specifier will work out of the box now. It's also kind of an oversight to allow popping s64 values, but nothing else. --- src/core/hle/ipc_helpers.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/core/hle/ipc_helpers.h b/src/core/hle/ipc_helpers.h index 90f276ee8f..d0721074dd 100644 --- a/src/core/hle/ipc_helpers.h +++ b/src/core/hle/ipc_helpers.h @@ -362,6 +362,11 @@ inline u32 RequestParser::Pop() { return cmdbuf[index++]; } +template <> +inline s32 RequestParser::Pop() { + return static_cast(Pop()); +} + template void RequestParser::PopRaw(T& value) { std::memcpy(&value, cmdbuf + index, sizeof(T)); @@ -392,6 +397,16 @@ inline u64 RequestParser::Pop() { return msw << 32 | lsw; } +template <> +inline s8 RequestParser::Pop() { + return static_cast(Pop()); +} + +template <> +inline s16 RequestParser::Pop() { + return static_cast(Pop()); +} + template <> inline s64 RequestParser::Pop() { return static_cast(Pop()); From 92ea1c32d608cd258c3fc077f5aaf953536d7f45 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 26 Feb 2019 17:49:32 -0500 Subject: [PATCH 3/3] service/vi: Unstub GetDisplayService This function is also supposed to check its given policy type with the permission of the service itself. This implements the necessary machinery to unstub these functions. Policy::User seems to just be basic access (which is probably why vi:u is restricted to that policy), while the other policy seems to be for extended abilities regarding which displays can be managed and queried, so this is assumed to be for a background compositor (which I've named, appropriately, Policy::Compositor). --- src/core/hle/service/vi/vi.cpp | 25 ++++++++++++++++++++++++- src/core/hle/service/vi/vi.h | 23 +++++++++++++++++++---- src/core/hle/service/vi/vi_m.cpp | 4 ++-- src/core/hle/service/vi/vi_s.cpp | 4 ++-- src/core/hle/service/vi/vi_u.cpp | 4 ++-- 5 files changed, 49 insertions(+), 11 deletions(-) diff --git a/src/core/hle/service/vi/vi.cpp b/src/core/hle/service/vi/vi.cpp index 99340e2ed1..a5ad66a134 100644 --- a/src/core/hle/service/vi/vi.cpp +++ b/src/core/hle/service/vi/vi.cpp @@ -34,6 +34,7 @@ namespace Service::VI { constexpr ResultCode ERR_OPERATION_FAILED{ErrorModule::VI, 1}; +constexpr ResultCode ERR_PERMISSION_DENIED{ErrorModule::VI, 5}; constexpr ResultCode ERR_UNSUPPORTED{ErrorModule::VI, 6}; constexpr ResultCode ERR_NOT_FOUND{ErrorModule::VI, 7}; @@ -1203,8 +1204,30 @@ IApplicationDisplayService::IApplicationDisplayService( RegisterHandlers(functions); } +static bool IsValidServiceAccess(Permission permission, Policy policy) { + if (permission == Permission::User) { + return policy == Policy::User; + } + + if (permission == Permission::System || permission == Permission::Manager) { + return policy == Policy::User || policy == Policy::Compositor; + } + + return false; +} + void detail::GetDisplayServiceImpl(Kernel::HLERequestContext& ctx, - std::shared_ptr nv_flinger) { + std::shared_ptr nv_flinger, + Permission permission) { + IPC::RequestParser rp{ctx}; + const auto policy = rp.PopEnum(); + + if (!IsValidServiceAccess(permission, policy)) { + IPC::ResponseBuilder rb{ctx, 2}; + rb.Push(ERR_PERMISSION_DENIED); + return; + } + IPC::ResponseBuilder rb{ctx, 2, 0, 1}; rb.Push(RESULT_SUCCESS); rb.PushIpcInterface(std::move(nv_flinger)); diff --git a/src/core/hle/service/vi/vi.h b/src/core/hle/service/vi/vi.h index c5682accc9..6b66f8b811 100644 --- a/src/core/hle/service/vi/vi.h +++ b/src/core/hle/service/vi/vi.h @@ -20,10 +20,6 @@ class ServiceManager; } namespace Service::VI { -namespace detail { -void GetDisplayServiceImpl(Kernel::HLERequestContext& ctx, - std::shared_ptr nv_flinger); -} enum class DisplayResolution : u32 { DockedWidth = 1920, @@ -32,6 +28,25 @@ enum class DisplayResolution : u32 { UndockedHeight = 720, }; +/// Permission level for a particular VI service instance +enum class Permission { + User, + System, + Manager, +}; + +/// A policy type that may be requested via GetDisplayService and +/// GetDisplayServiceWithProxyNameExchange +enum class Policy { + User, + Compositor, +}; + +namespace detail { +void GetDisplayServiceImpl(Kernel::HLERequestContext& ctx, + std::shared_ptr nv_flinger, Permission permission); +} // namespace detail + /// Registers all VI services with the specified service manager. void InstallInterfaces(SM::ServiceManager& service_manager, std::shared_ptr nv_flinger); diff --git a/src/core/hle/service/vi/vi_m.cpp b/src/core/hle/service/vi/vi_m.cpp index 6e3e0bd8f5..06070087f8 100644 --- a/src/core/hle/service/vi/vi_m.cpp +++ b/src/core/hle/service/vi/vi_m.cpp @@ -20,9 +20,9 @@ VI_M::VI_M(std::shared_ptr nv_flinger) VI_M::~VI_M() = default; void VI_M::GetDisplayService(Kernel::HLERequestContext& ctx) { - LOG_WARNING(Service_VI, "(STUBBED) called"); + LOG_DEBUG(Service_VI, "called"); - detail::GetDisplayServiceImpl(ctx, nv_flinger); + detail::GetDisplayServiceImpl(ctx, nv_flinger, Permission::Manager); } } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_s.cpp b/src/core/hle/service/vi/vi_s.cpp index 6dd700eaec..57c596cc43 100644 --- a/src/core/hle/service/vi/vi_s.cpp +++ b/src/core/hle/service/vi/vi_s.cpp @@ -20,9 +20,9 @@ VI_S::VI_S(std::shared_ptr nv_flinger) VI_S::~VI_S() = default; void VI_S::GetDisplayService(Kernel::HLERequestContext& ctx) { - LOG_WARNING(Service_VI, "(STUBBED) called"); + LOG_DEBUG(Service_VI, "called"); - detail::GetDisplayServiceImpl(ctx, nv_flinger); + detail::GetDisplayServiceImpl(ctx, nv_flinger, Permission::System); } } // namespace Service::VI diff --git a/src/core/hle/service/vi/vi_u.cpp b/src/core/hle/service/vi/vi_u.cpp index ef09a5df5d..9d5ceb608d 100644 --- a/src/core/hle/service/vi/vi_u.cpp +++ b/src/core/hle/service/vi/vi_u.cpp @@ -19,9 +19,9 @@ VI_U::VI_U(std::shared_ptr nv_flinger) VI_U::~VI_U() = default; void VI_U::GetDisplayService(Kernel::HLERequestContext& ctx) { - LOG_WARNING(Service_VI, "(STUBBED) called"); + LOG_DEBUG(Service_VI, "called"); - detail::GetDisplayServiceImpl(ctx, nv_flinger); + detail::GetDisplayServiceImpl(ctx, nv_flinger, Permission::User); } } // namespace Service::VI