hle: kernel: Improve MapSharedMemory and implement UnmapSharedMemory.

src/core/hle/kernel/k_memory_block.h

@@ -134,6 +134,10 @@ enum class KMemoryPermission : u8 {
 };
 DECLARE_ENUM_FLAG_OPERATORS(KMemoryPermission);
 
+constexpr KMemoryPermission ConvertToKMemoryPermission(Svc::MemoryPermission perm) {
+    return static_cast<KMemoryPermission>(perm);
+}
+
 enum class KMemoryAttribute : u8 {
     None = 0x00,
     Mask = 0x7F,

src/core/hle/kernel/k_page_table.cpp

@@ -641,6 +641,45 @@ ResultCode KPageTable::MapPages(VAddr addr, KPageLinkedList& page_linked_list, K
     return RESULT_SUCCESS;
 }
 
+ResultCode KPageTable::UnmapPages(VAddr addr, const KPageLinkedList& page_linked_list) {
+    VAddr cur_addr{addr};
+
+    for (const auto& node : page_linked_list.Nodes()) {
+        const std::size_t num_pages{(addr - cur_addr) / PageSize};
+        if (const auto result{
+                Operate(addr, num_pages, KMemoryPermission::None, OperationType::Unmap)};
+            result.IsError()) {
+            return result;
+        }
+
+        cur_addr += node.GetNumPages() * PageSize;
+    }
+
+    return RESULT_SUCCESS;
+}
+
+ResultCode KPageTable::UnmapPages(VAddr addr, KPageLinkedList& page_linked_list,
+                                  KMemoryState state) {
+    std::lock_guard lock{page_table_lock};
+
+    const std::size_t num_pages{page_linked_list.GetNumPages()};
+    const std::size_t size{num_pages * PageSize};
+
+    if (!CanContain(addr, size, state)) {
+        return ResultInvalidCurrentMemory;
+    }
+
+    if (IsRegionMapped(addr, num_pages * PageSize)) {
+        return ResultInvalidCurrentMemory;
+    }
+
+    CASCADE_CODE(UnmapPages(addr, page_linked_list));
+
+    block_manager->Update(addr, num_pages, state, KMemoryPermission::None);
+
+    return RESULT_SUCCESS;
+}
+
 ResultCode KPageTable::SetCodeMemoryPermission(VAddr addr, std::size_t size,
                                                KMemoryPermission perm) {
 

src/core/hle/kernel/k_page_table.h

@@ -40,6 +40,7 @@ public:
     ResultCode Unmap(VAddr dst_addr, VAddr src_addr, std::size_t size);
     ResultCode MapPages(VAddr addr, KPageLinkedList& page_linked_list, KMemoryState state,
                         KMemoryPermission perm);
+    ResultCode UnmapPages(VAddr addr, KPageLinkedList& page_linked_list, KMemoryState state);
     ResultCode SetCodeMemoryPermission(VAddr addr, std::size_t size, KMemoryPermission perm);
     KMemoryInfo QueryInfo(VAddr addr);
     ResultCode ReserveTransferMemory(VAddr addr, std::size_t size, KMemoryPermission perm);
@@ -63,6 +64,8 @@ public:
         return page_table_impl;
     }
 
+    constexpr bool CanContain(VAddr addr, std::size_t size, KMemoryState state) const;
+
 private:
     enum class OperationType : u32 {
         Map,
@@ -79,6 +82,7 @@ private:
     ResultCode InitializeMemoryLayout(VAddr start, VAddr end);
     ResultCode MapPages(VAddr addr, const KPageLinkedList& page_linked_list,
                         KMemoryPermission perm);
+    ResultCode UnmapPages(VAddr addr, const KPageLinkedList& page_linked_list);
     void MapPhysicalMemory(KPageLinkedList& page_linked_list, VAddr start, VAddr end);
     bool IsRegionMapped(VAddr address, u64 size);
     bool IsRegionContiguous(VAddr addr, u64 size) const;
@@ -92,7 +96,6 @@ private:
                        OperationType operation, PAddr map_addr = 0);
     constexpr VAddr GetRegionAddress(KMemoryState state) const;
     constexpr std::size_t GetRegionSize(KMemoryState state) const;
-    constexpr bool CanContain(VAddr addr, std::size_t size, KMemoryState state) const;
 
     constexpr ResultCode CheckMemoryState(const KMemoryInfo& info, KMemoryState state_mask,
                                           KMemoryState state, KMemoryPermission perm_mask,

src/core/hle/kernel/k_process.cpp

@@ -21,6 +21,7 @@
 #include "core/hle/kernel/k_resource_limit.h"
 #include "core/hle/kernel/k_scheduler.h"
 #include "core/hle/kernel/k_scoped_resource_reservation.h"
+#include "core/hle/kernel/k_shared_memory.h"
 #include "core/hle/kernel/k_slab_heap.h"
 #include "core/hle/kernel/k_thread.h"
 #include "core/hle/kernel/kernel.h"
@@ -247,6 +248,30 @@ void KProcess::UnpinCurrentThread() {
     KScheduler::SetSchedulerUpdateNeeded(kernel);
 }
 
+ResultCode KProcess::AddSharedMemory(KSharedMemory* shmem, [[maybe_unused]] VAddr address,
+                                     [[maybe_unused]] size_t size) {
+    // Lock ourselves, to prevent concurrent access.
+    KScopedLightLock lk(state_lock);
+
+    // TODO(bunnei): Manage KSharedMemoryInfo list here.
+
+    // Open a reference to the shared memory.
+    shmem->Open();
+
+    return RESULT_SUCCESS;
+}
+
+void KProcess::RemoveSharedMemory(KSharedMemory* shmem, [[maybe_unused]] VAddr address,
+                                  [[maybe_unused]] size_t size) {
+    // Lock ourselves, to prevent concurrent access.
+    KScopedLightLock lk(state_lock);
+
+    // TODO(bunnei): Manage KSharedMemoryInfo list here.
+
+    // Close a reference to the shared memory.
+    shmem->Close();
+}
+
 void KProcess::RegisterThread(const KThread* thread) {
     thread_list.push_back(thread);
 }

src/core/hle/kernel/k_process.h

@@ -352,6 +352,9 @@ public:
         return state_lock;
     }
 
+    ResultCode AddSharedMemory(KSharedMemory* shmem, VAddr address, size_t size);
+    void RemoveSharedMemory(KSharedMemory* shmem, VAddr address, size_t size);
+
     ///////////////////////////////////////////////////////////////////////////////////////////////
     // Thread-local storage management
 

src/core/hle/kernel/k_shared_memory.cpp

@@ -20,9 +20,10 @@ KSharedMemory::~KSharedMemory() {
 
 ResultCode KSharedMemory::Initialize(KernelCore& kernel_, Core::DeviceMemory& device_memory_,
                                      KProcess* owner_process_, KPageLinkedList&& page_list_,
-                                     KMemoryPermission owner_permission_,
+                                     Svc::MemoryPermission owner_permission_,
-                                     KMemoryPermission user_permission_, PAddr physical_address_,
+                                     Svc::MemoryPermission user_permission_,
-                                     std::size_t size_, std::string name_) {
+                                     PAddr physical_address_, std::size_t size_,
+                                     std::string name_) {
     // Set members.
     owner_process = owner_process_;
     device_memory = &device_memory_;
@@ -58,14 +59,6 @@ ResultCode KSharedMemory::Initialize(KernelCore& kernel_, Core::DeviceMemory& de
 }
 
 void KSharedMemory::Finalize() {
-    ///* Get the number of pages. */
-    // const size_t num_pages = m_page_group.GetNumPages();
-    // const size_t size = num_pages * PageSize;
-
-    ///* Close and finalize the page group. */
-    // m_page_group.Close();
-    // m_page_group.Finalize();
-
     // Release the memory reservation.
     resource_limit->Release(LimitableResource::PhysicalMemory, size);
     resource_limit->Close();
@@ -75,14 +68,14 @@ void KSharedMemory::Finalize() {
 }
 
 ResultCode KSharedMemory::Map(KProcess& target_process, VAddr address, std::size_t size,
-                              KMemoryPermission permissions) {
+                              Svc::MemoryPermission permissions) {
     const u64 page_count{(size + PageSize - 1) / PageSize};
 
     if (page_list.GetNumPages() != page_count) {
         UNIMPLEMENTED_MSG("Page count does not match");
     }
 
-    const KMemoryPermission expected =
+    const Svc::MemoryPermission expected =
         &target_process == owner_process ? owner_permission : user_permission;
 
     if (permissions != expected) {
@@ -90,7 +83,17 @@ ResultCode KSharedMemory::Map(KProcess& target_process, VAddr address, std::size
     }
 
     return target_process.PageTable().MapPages(address, page_list, KMemoryState::Shared,
-                                               permissions);
+                                               ConvertToKMemoryPermission(permissions));
+}
+
+ResultCode KSharedMemory::Unmap(KProcess& target_process, VAddr address, std::size_t size) {
+    const u64 page_count{(size + PageSize - 1) / PageSize};
+
+    if (page_list.GetNumPages() != page_count) {
+        UNIMPLEMENTED_MSG("Page count does not match");
+    }
+
+    return target_process.PageTable().UnmapPages(address, page_list, KMemoryState::Shared);
 }
 
 } // namespace Kernel

src/core/hle/kernel/k_shared_memory.h

@@ -29,8 +29,9 @@ public:
 
     ResultCode Initialize(KernelCore& kernel_, Core::DeviceMemory& device_memory_,
                           KProcess* owner_process_, KPageLinkedList&& page_list_,
-                          KMemoryPermission owner_permission_, KMemoryPermission user_permission_,
+                          Svc::MemoryPermission owner_permission_,
-                          PAddr physical_address_, std::size_t size_, std::string name_);
+                          Svc::MemoryPermission user_permission_, PAddr physical_address_,
+                          std::size_t size_, std::string name_);
 
     /**
      * Maps a shared memory block to an address in the target process' address space
@@ -40,7 +41,15 @@ public:
      * @param permissions Memory block map permissions (specified by SVC field)
      */
     ResultCode Map(KProcess& target_process, VAddr address, std::size_t size,
-                   KMemoryPermission permissions);
+                   Svc::MemoryPermission permissions);
+
+    /**
+     * Unmaps a shared memory block from an address in the target process' address space
+     * @param target_process Process on which to unmap the memory block
+     * @param address Address in system memory to unmap shared memory block
+     * @param size Size of the shared memory block to unmap
+     */
+    ResultCode Unmap(KProcess& target_process, VAddr address, std::size_t size);
 
     /**
      * Gets a pointer to the shared memory block
@@ -71,8 +80,8 @@ private:
     Core::DeviceMemory* device_memory;
     KProcess* owner_process{};
     KPageLinkedList page_list;
-    KMemoryPermission owner_permission{};
+    Svc::MemoryPermission owner_permission{};
-    KMemoryPermission user_permission{};
+    Svc::MemoryPermission user_permission{};
     PAddr physical_address{};
     std::size_t size{};
     KResourceLimit* resource_limit{};

src/core/hle/kernel/kernel.cpp

@@ -599,21 +599,21 @@ struct KernelCore::Impl {
         time_shared_mem = KSharedMemory::Create(system.Kernel());
 
         hid_shared_mem->Initialize(system.Kernel(), system.DeviceMemory(), nullptr,
-                                   {hid_phys_addr, hid_size / PageSize}, KMemoryPermission::None,
+                                   {hid_phys_addr, hid_size / PageSize},
-                                   KMemoryPermission::Read, hid_phys_addr, hid_size,
+                                   Svc::MemoryPermission::None, Svc::MemoryPermission::Read,
-                                   "HID:SharedMemory");
+                                   hid_phys_addr, hid_size, "HID:SharedMemory");
         font_shared_mem->Initialize(system.Kernel(), system.DeviceMemory(), nullptr,
-                                    {font_phys_addr, font_size / PageSize}, KMemoryPermission::None,
+                                    {font_phys_addr, font_size / PageSize},
-                                    KMemoryPermission::Read, font_phys_addr, font_size,
+                                    Svc::MemoryPermission::None, Svc::MemoryPermission::Read,
-                                    "Font:SharedMemory");
+                                    font_phys_addr, font_size, "Font:SharedMemory");
         irs_shared_mem->Initialize(system.Kernel(), system.DeviceMemory(), nullptr,
-                                   {irs_phys_addr, irs_size / PageSize}, KMemoryPermission::None,
+                                   {irs_phys_addr, irs_size / PageSize},
-                                   KMemoryPermission::Read, irs_phys_addr, irs_size,
+                                   Svc::MemoryPermission::None, Svc::MemoryPermission::Read,
-                                   "IRS:SharedMemory");
+                                   irs_phys_addr, irs_size, "IRS:SharedMemory");
         time_shared_mem->Initialize(system.Kernel(), system.DeviceMemory(), nullptr,
-                                    {time_phys_addr, time_size / PageSize}, KMemoryPermission::None,
+                                    {time_phys_addr, time_size / PageSize},
-                                    KMemoryPermission::Read, time_phys_addr, time_size,
+                                    Svc::MemoryPermission::None, Svc::MemoryPermission::Read,
-                                    "Time:SharedMemory");
+                                    time_phys_addr, time_size, "Time:SharedMemory");
     }
 
     void InitializePageSlab() {

src/core/hle/kernel/svc.cpp

@@ -1183,79 +1183,97 @@ static u32 GetCurrentProcessorNumber32(Core::System& system) {
     return GetCurrentProcessorNumber(system);
 }
 
-static ResultCode MapSharedMemory(Core::System& system, Handle shared_memory_handle, VAddr addr,
+constexpr bool IsValidSharedMemoryPermission(Svc::MemoryPermission perm) {
-                                  u64 size, u32 permissions) {
+    switch (perm) {
-    std::lock_guard lock{HLE::g_hle_lock};
+    case Svc::MemoryPermission::Read:
-    LOG_TRACE(Kernel_SVC,
+    case Svc::MemoryPermission::ReadWrite:
-              "called, shared_memory_handle=0x{:X}, addr=0x{:X}, size=0x{:X}, permissions=0x{:08X}",
+        return true;
-              shared_memory_handle, addr, size, permissions);
+    default:
-
+        return false;
-    if (!Common::Is4KBAligned(addr)) {
-        LOG_ERROR(Kernel_SVC, "Address is not aligned to 4KB, addr=0x{:016X}", addr);
-        return ResultInvalidAddress;
     }
-
-    if (size == 0) {
-        LOG_ERROR(Kernel_SVC, "Size is 0");
-        return ResultInvalidSize;
-    }
-
-    if (!Common::Is4KBAligned(size)) {
-        LOG_ERROR(Kernel_SVC, "Size is not aligned to 4KB, size=0x{:016X}", size);
-        return ResultInvalidSize;
-    }
-
-    if (!IsValidAddressRange(addr, size)) {
-        LOG_ERROR(Kernel_SVC, "Region is not a valid address range, addr=0x{:016X}, size=0x{:016X}",
-                  addr, size);
-        return ResultInvalidCurrentMemory;
-    }
-
-    const auto permission_type = static_cast<MemoryPermission>(permissions);
-    if ((permission_type | MemoryPermission::Write) != MemoryPermission::ReadWrite) {
-        LOG_ERROR(Kernel_SVC, "Expected Read or ReadWrite permission but got permissions=0x{:08X}",
-                  permissions);
-        return ResultInvalidNewMemoryPermission;
-    }
-
-    auto* const current_process{system.Kernel().CurrentProcess()};
-    auto& page_table{current_process->PageTable()};
-
-    if (page_table.IsInvalidRegion(addr, size)) {
-        LOG_ERROR(Kernel_SVC,
-                  "Addr does not fit within the valid region, addr=0x{:016X}, "
-                  "size=0x{:016X}",
-                  addr, size);
-        return ResultInvalidMemoryRegion;
-    }
-
-    if (page_table.IsInsideHeapRegion(addr, size)) {
-        LOG_ERROR(Kernel_SVC,
-                  "Addr does not fit within the heap region, addr=0x{:016X}, "
-                  "size=0x{:016X}",
-                  addr, size);
-        return ResultInvalidMemoryRegion;
-    }
-
-    if (page_table.IsInsideAliasRegion(addr, size)) {
-        LOG_ERROR(Kernel_SVC,
-                  "Address does not fit within the map region, addr=0x{:016X}, "
-                  "size=0x{:016X}",
-                  addr, size);
-        return ResultInvalidMemoryRegion;
-    }
-
-    auto shared_memory{
-        current_process->GetHandleTable().GetObject<KSharedMemory>(shared_memory_handle)};
-    R_UNLESS(shared_memory.IsNotNull(), ResultInvalidHandle);
-
-    return shared_memory->Map(*current_process, addr, size,
-                              static_cast<KMemoryPermission>(permission_type));
 }
 
-static ResultCode MapSharedMemory32(Core::System& system, Handle shared_memory_handle, u32 addr,
+constexpr bool IsValidRemoteSharedMemoryPermission(Svc::MemoryPermission perm) {
-                                    u32 size, u32 permissions) {
+    return IsValidSharedMemoryPermission(perm) || perm == Svc::MemoryPermission::DontCare;
-    return MapSharedMemory(system, shared_memory_handle, addr, size, permissions);
+}
+
+static ResultCode MapSharedMemory(Core::System& system, Handle shmem_handle, VAddr address,
+                                  u64 size, Svc::MemoryPermission map_perm) {
+    LOG_TRACE(Kernel_SVC,
+              "called, shared_memory_handle=0x{:X}, addr=0x{:X}, size=0x{:X}, permissions=0x{:08X}",
+              shmem_handle, address, size, map_perm);
+
+    // Validate the address/size.
+    R_UNLESS(Common::IsAligned(address, PageSize), ResultInvalidAddress);
+    R_UNLESS(Common::IsAligned(size, PageSize), ResultInvalidSize);
+    R_UNLESS(size > 0, ResultInvalidSize);
+    R_UNLESS((address < address + size), ResultInvalidCurrentMemory);
+
+    // Validate the permission.
+    R_UNLESS(IsValidSharedMemoryPermission(map_perm), ResultInvalidNewMemoryPermission);
+
+    // Get the current process.
+    auto& process = *system.Kernel().CurrentProcess();
+    auto& page_table = process.PageTable();
+
+    // Get the shared memory.
+    KScopedAutoObject shmem = process.GetHandleTable().GetObject<KSharedMemory>(shmem_handle);
+    R_UNLESS(shmem.IsNotNull(), ResultInvalidHandle);
+
+    // Verify that the mapping is in range.
+    R_UNLESS(page_table.CanContain(address, size, KMemoryState::Shared), ResultInvalidMemoryRegion);
+
+    // Add the shared memory to the process.
+    R_TRY(process.AddSharedMemory(shmem.GetPointerUnsafe(), address, size));
+
+    // Ensure that we clean up the shared memory if we fail to map it.
+    auto guard =
+        SCOPE_GUARD({ process.RemoveSharedMemory(shmem.GetPointerUnsafe(), address, size); });
+
+    // Map the shared memory.
+    R_TRY(shmem->Map(process, address, size, map_perm));
+
+    // We succeeded.
+    guard.Cancel();
+    return RESULT_SUCCESS;
+}
+
+static ResultCode MapSharedMemory32(Core::System& system, Handle shmem_handle, u32 address,
+                                    u32 size, Svc::MemoryPermission map_perm) {
+    return MapSharedMemory(system, shmem_handle, address, size, map_perm);
+}
+
+static ResultCode UnmapSharedMemory(Core::System& system, Handle shmem_handle, VAddr address,
+                                    u64 size) {
+    // Validate the address/size.
+    R_UNLESS(Common::IsAligned(address, PageSize), ResultInvalidAddress);
+    R_UNLESS(Common::IsAligned(size, PageSize), ResultInvalidSize);
+    R_UNLESS(size > 0, ResultInvalidSize);
+    R_UNLESS((address < address + size), ResultInvalidCurrentMemory);
+
+    // Get the current process.
+    auto& process = *system.Kernel().CurrentProcess();
+    auto& page_table = process.PageTable();
+
+    // Get the shared memory.
+    KScopedAutoObject shmem = process.GetHandleTable().GetObject<KSharedMemory>(shmem_handle);
+    R_UNLESS(shmem.IsNotNull(), ResultInvalidHandle);
+
+    // Verify that the mapping is in range.
+    R_UNLESS(page_table.CanContain(address, size, KMemoryState::Shared), ResultInvalidMemoryRegion);
+
+    // Unmap the shared memory.
+    R_TRY(shmem->Unmap(process, address, size));
+
+    // Remove the shared memory from the process.
+    process.RemoveSharedMemory(shmem.GetPointerUnsafe(), address, size);
+
+    return RESULT_SUCCESS;
+}
+
+static ResultCode UnmapSharedMemory32(Core::System& system, Handle shmem_handle, u32 address,
+                                      u32 size) {
+    return UnmapSharedMemory(system, shmem_handle, address, size);
 }
 
 static ResultCode QueryProcessMemory(Core::System& system, VAddr memory_info_address,
@@ -2297,7 +2315,7 @@ static const FunctionDef SVC_Table_32[] = {
     {0x11, SvcWrap32<SignalEvent32>, "SignalEvent32"},
     {0x12, SvcWrap32<ClearEvent32>, "ClearEvent32"},
     {0x13, SvcWrap32<MapSharedMemory32>, "MapSharedMemory32"},
-    {0x14, nullptr, "UnmapSharedMemory32"},
+    {0x14, SvcWrap32<UnmapSharedMemory32>, "UnmapSharedMemory32"},
     {0x15, SvcWrap32<CreateTransferMemory32>, "CreateTransferMemory32"},
     {0x16, SvcWrap32<CloseHandle32>, "CloseHandle32"},
     {0x17, SvcWrap32<ResetSignal32>, "ResetSignal32"},
@@ -2492,7 +2510,7 @@ static const FunctionDef SVC_Table_64[] = {
     {0x11, SvcWrap64<SignalEvent>, "SignalEvent"},
     {0x12, SvcWrap64<ClearEvent>, "ClearEvent"},
     {0x13, SvcWrap64<MapSharedMemory>, "MapSharedMemory"},
-    {0x14, nullptr, "UnmapSharedMemory"},
+    {0x14, SvcWrap64<UnmapSharedMemory>, "UnmapSharedMemory"},
     {0x15, SvcWrap64<CreateTransferMemory>, "CreateTransferMemory"},
     {0x16, SvcWrap64<CloseHandle>, "CloseHandle"},
     {0x17, SvcWrap64<ResetSignal>, "ResetSignal"},

src/core/hle/kernel/svc_wrap.h

@@ -232,10 +232,11 @@ void SvcWrap64(Core::System& system) {
         func(system, Param(system, 0), Param(system, 1), static_cast<u32>(Param(system, 2))).raw);
 }
 
-template <ResultCode func(Core::System&, u32, u64, u64, u32)>
+// Used by MapSharedMemory
+template <ResultCode func(Core::System&, Handle, u64, u64, Svc::MemoryPermission)>
 void SvcWrap64(Core::System& system) {
-    FuncReturn(system, func(system, static_cast<u32>(Param(system, 0)), Param(system, 1),
+    FuncReturn(system, func(system, static_cast<Handle>(Param(system, 0)), Param(system, 1),
-                            Param(system, 2), static_cast<u32>(Param(system, 3)))
+                            Param(system, 2), static_cast<Svc::MemoryPermission>(Param(system, 3)))
                            .raw);
 }
 
@@ -553,6 +554,16 @@ void SvcWrap32(Core::System& system) {
     FuncReturn(system, retval);
 }
 
+// Used by MapSharedMemory32
+template <ResultCode func(Core::System&, Handle, u32, u32, Svc::MemoryPermission)>
+void SvcWrap32(Core::System& system) {
+    const u32 retval =
+        func(system, static_cast<Handle>(Param(system, 0)), static_cast<u32>(Param(system, 1)), static_cast<u32>(Param(system, 2)),
+                            static_cast<Svc::MemoryPermission>(Param(system, 3)))
+            .raw;
+    FuncReturn(system, retval);
+}
+
 // Used by SetThreadCoreMask32
 template <ResultCode func(Core::System&, Handle, s32, u32, u32)>
 void SvcWrap32(Core::System& system) {