From c7de7890b67e9507d05497dacf9081956ca75a69 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Thu, 11 Mar 2021 23:12:12 +0530 Subject: [PATCH] Improve KMemory + Fix Service Implementation Warning * Improve KMemory Comments * Add parameter prefix 'p-' to `KPrivateMemory::UpdatePermission` * Fix the missing trailing double quote in missing service prints, this was due to `stringName` being padded with extra 0s --- app/src/main/cpp/skyline/kernel/svc.cpp | 2 +- .../skyline/kernel/types/KPrivateMemory.cpp | 26 +++++++++---------- .../cpp/skyline/kernel/types/KPrivateMemory.h | 11 +++++--- .../skyline/kernel/types/KSharedMemory.cpp | 18 +++++++------ .../cpp/skyline/kernel/types/KSharedMemory.h | 4 +-- .../skyline/kernel/types/KTransferMemory.h | 2 +- .../skyline/services/sm/IUserInterface.cpp | 2 +- 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/app/src/main/cpp/skyline/kernel/svc.cpp b/app/src/main/cpp/skyline/kernel/svc.cpp index 2bd70100..1d1ea6f9 100644 --- a/app/src/main/cpp/skyline/kernel/svc.cpp +++ b/app/src/main/cpp/skyline/kernel/svc.cpp @@ -1017,7 +1017,7 @@ namespace skyline::kernel::svc { if (memory) { auto item{static_pointer_cast(memory->item)}; auto initialSize{item->size}; - if (item->memState == memory::states::Heap) { + if (item->memoryState == memory::states::Heap) { if (item->ptr >= pointer) { if (item->size <= size) { item->Resize(0); diff --git a/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.cpp b/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.cpp index 95a2cf51..c16297e4 100644 --- a/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.cpp @@ -8,7 +8,7 @@ #include "KProcess.h" namespace skyline::kernel::type { - KPrivateMemory::KPrivateMemory(const DeviceState &state, u8 *ptr, size_t size, memory::Permission permission, memory::MemoryState memState) : ptr(ptr), size(size), permission(permission), memState(memState), KMemory(state, KType::KPrivateMemory) { + KPrivateMemory::KPrivateMemory(const DeviceState &state, u8 *ptr, size_t size, memory::Permission permission, memory::MemoryState memState) : ptr(ptr), size(size), permission(permission), memoryState(memState), KMemory(state, KType::KPrivateMemory) { if (!state.process->memory.base.IsInside(ptr) || !state.process->memory.base.IsInside(ptr + size)) throw exception("KPrivateMemory allocation isn't inside guest address space: 0x{:X} - 0x{:X}", ptr, ptr + size); if (!util::PageAligned(ptr) || !util::PageAligned(size)) @@ -40,7 +40,7 @@ namespace skyline::kernel::type { .ptr = ptr + size, .size = nSize - size, .permission = permission, - .state = memState, + .state = memoryState, }); } @@ -60,22 +60,22 @@ namespace skyline::kernel::type { throw exception("An occurred while remapping private memory: {}", strerror(errno)); } - void KPrivateMemory::UpdatePermission(u8 *ptr, size_t size, memory::Permission permission) { - ptr = std::clamp(ptr, this->ptr, this->ptr + this->size); - size = std::min(size, static_cast((this->ptr + this->size) - ptr)); + void KPrivateMemory::UpdatePermission(u8 *pPtr, size_t pSize, memory::Permission pPermission) { + pPtr = std::clamp(pPtr, ptr, ptr + size); + pSize = std::min(pSize, static_cast((ptr + size) - pPtr)); - if (ptr && !util::PageAligned(ptr)) - throw exception("KPrivateMemory permission updated with a non-page-aligned address: 0x{:X}", ptr); + if (pPtr && !util::PageAligned(pPtr)) + throw exception("KPrivateMemory permission updated with a non-page-aligned address: 0x{:X}", pPtr); // If a static code region has been mapped as writable it needs to be changed to mutable - if (memState == memory::states::CodeStatic && permission.w) - memState = memory::states::CodeMutable; + if (memoryState == memory::states::CodeStatic && pPermission.w) + memoryState = memory::states::CodeMutable; state.process->memory.InsertChunk(ChunkDescriptor{ - .ptr = ptr, - .size = size, - .permission = permission, - .state = memState, + .ptr = pPtr, + .size = pSize, + .permission = pPermission, + .state = memoryState, }); } diff --git a/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.h b/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.h index a836ef1d..13801968 100644 --- a/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.h +++ b/app/src/main/cpp/skyline/kernel/types/KPrivateMemory.h @@ -8,13 +8,14 @@ namespace skyline::kernel::type { /** * @brief KPrivateMemory is used to map memory local to the guest process + * @note This does not reflect a kernel object in Horizon OS, it is an abstraction which makes things simpler to manage in Skyline instead */ class KPrivateMemory : public KMemory { public: u8 *ptr{}; size_t size{}; memory::Permission permission; - memory::MemoryState memState; + memory::MemoryState memoryState; /** * @param permission The permissions for the allocated memory (As reported to the application, host memory permissions aren't reflected by this) @@ -22,10 +23,14 @@ namespace skyline::kernel::type { */ KPrivateMemory(const DeviceState &state, u8 *ptr, size_t size, memory::Permission permission, memory::MemoryState memState); + /** + * @note There is no check regarding if any expansions will cause the memory mapping to leak into other mappings + * @note Any extensions will have the same permissions and memory state as the initial mapping as opposed to extending the end + */ void Resize(size_t size); /** - * @note Only contents of any overlapping regions will be retained + * @note This does not copy over anything, only contents of any overlapping regions will be retained */ void Remap(u8 *ptr, size_t size); @@ -33,7 +38,7 @@ namespace skyline::kernel::type { return span(ptr, size); } - void UpdatePermission(u8 *ptr, size_t size, memory::Permission permission) override; + void UpdatePermission(u8 *pPtr, size_t pSize, memory::Permission pPermission) override; /** * @brief The destructor of private memory, it deallocates the memory diff --git a/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp b/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp index 9d96b9e6..0f6c63df 100644 --- a/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp +++ b/app/src/main/cpp/skyline/kernel/types/KSharedMemory.cpp @@ -8,14 +8,15 @@ #include "KProcess.h" namespace skyline::kernel::type { - KSharedMemory::KSharedMemory(const DeviceState &state, size_t size, memory::MemoryState memState, KType type) : initialState(memState), KMemory(state, type) { + KSharedMemory::KSharedMemory(const DeviceState &state, size_t size, memory::MemoryState memState, KType type) : memoryState(memState), KMemory(state, type) { fd = ASharedMemory_create("KSharedMemory", size); if (fd < 0) throw exception("An error occurred while creating shared memory: {}", fd); kernel.ptr = reinterpret_cast(mmap(nullptr, size, PROT_READ | PROT_WRITE | PROT_EXEC, MAP_SHARED, fd, 0)); if (kernel.ptr == MAP_FAILED) - throw exception("An occurred while mapping shared memory: {}", strerror(errno)); + [[unlikely]] + throw exception("An occurred while mapping shared memory: {}", strerror(errno)); kernel.size = size; } @@ -28,14 +29,15 @@ namespace skyline::kernel::type { guest.ptr = reinterpret_cast(mmap(ptr, size, permission.Get(), MAP_SHARED | (ptr ? MAP_FIXED : 0), fd, 0)); if (guest.ptr == MAP_FAILED) - throw exception("An error occurred while mapping shared memory in guest"); + [[unlikely]] + throw exception("An error occurred while mapping shared memory in guest: {}", strerror(errno)); guest.size = size; state.process->memory.InsertChunk(ChunkDescriptor{ .ptr = guest.ptr, .size = size, .permission = permission, - .state = initialState, + .state = memoryState, }); return guest.ptr; @@ -47,15 +49,15 @@ namespace skyline::kernel::type { if (guest.Valid()) { mprotect(ptr, size, permission.Get()); - if (guest.ptr == MAP_FAILED) - throw exception("An error occurred while updating shared memory's permissions in guest"); + [[unlikely]] + throw exception("An error occurred while updating shared memory's permissions in guest: {}", strerror(errno)); state.process->memory.InsertChunk(ChunkDescriptor{ .ptr = ptr, .size = size, .permission = permission, - .state = initialState, + .state = memoryState, }); } } @@ -65,7 +67,7 @@ namespace skyline::kernel::type { munmap(kernel.ptr, kernel.size); if (state.process && guest.Valid()) { - mmap(guest.ptr, guest.size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); + mmap(guest.ptr, guest.size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0); // As this is the destructor, we cannot throw on this failing state.process->memory.InsertChunk(ChunkDescriptor{ .ptr = guest.ptr, .size = guest.size, diff --git a/app/src/main/cpp/skyline/kernel/types/KSharedMemory.h b/app/src/main/cpp/skyline/kernel/types/KSharedMemory.h index 47a01c47..24bdac5f 100644 --- a/app/src/main/cpp/skyline/kernel/types/KSharedMemory.h +++ b/app/src/main/cpp/skyline/kernel/types/KSharedMemory.h @@ -7,12 +7,12 @@ namespace skyline::kernel::type { /** - * @brief KSharedMemory is used to retain two mappings of the same underlying memory, allowing persistence of the memory + * @brief KSharedMemory is used to retain two mappings of the same underlying memory, allowing sharing memory between two processes */ class KSharedMemory : public KMemory { private: int fd; //!< A file descriptor to the underlying shared memory - memory::MemoryState initialState; + memory::MemoryState memoryState; //!< The state of the memory as supplied initially, this is retained for any mappings public: struct MapInfo { diff --git a/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h b/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h index 1ad2614d..c5b65b0c 100644 --- a/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h +++ b/app/src/main/cpp/skyline/kernel/types/KTransferMemory.h @@ -7,7 +7,7 @@ namespace skyline::kernel::type { /** - * @brief KTransferMemory is used to transfer memory from one application to another on HOS, we emulate this abstraction using KSharedMemory as it's functionally indistinguishable for the guest and allows access from the kernel regardless of if it's mapped on the guest + * @brief KTransferMemory is used to transfer memory from one application to another on HOS, we emulate this abstraction using KSharedMemory as it's essentially the same with the main difference being that KSharedMemory is allocated by the kernel while KTransferMemory is created from memory that's been allocated by the guest beforehand */ class KTransferMemory : public KSharedMemory { public: diff --git a/app/src/main/cpp/skyline/services/sm/IUserInterface.cpp b/app/src/main/cpp/skyline/services/sm/IUserInterface.cpp index 8c9fb66f..b8ce0c83 100644 --- a/app/src/main/cpp/skyline/services/sm/IUserInterface.cpp +++ b/app/src/main/cpp/skyline/services/sm/IUserInterface.cpp @@ -20,7 +20,7 @@ namespace skyline::service::sm { manager.NewService(name, session, response); return {}; } catch (std::out_of_range &) { - std::string_view stringName(reinterpret_cast(&name), sizeof(u64)); + std::string_view stringName(span(reinterpret_cast(&name), sizeof(u64)).as_string(true)); state.logger->Warn("Service has not been implemented: \"{}\"", stringName); return result::InvalidServiceName; }