From 13a8fde3ad2a4a37cf1bb8dcb367b4c8fc8b4d9b Mon Sep 17 00:00:00 2001 From: Michael Scire Date: Sun, 7 Jul 2019 09:42:54 -0700 Subject: Implement MapPhysicalMemory/UnmapPhysicalMemory This implements svcMapPhysicalMemory/svcUnmapPhysicalMemory for Yuzu, which can be used to map memory at a desired address by games since 3.0.0. It also properly parses SystemResourceSize from NPDM, and makes information available via svcGetInfo. This is needed for games like Super Smash Bros. and Diablo 3 -- this PR's implementation does not run into the "ASCII reads" issue mentioned in the comments of #2626, which was caused by the following bugs in Yuzu's memory management that this PR also addresses: * Yuzu's memory coalescing does not properly merge blocks. This results in a polluted address space/svcQueryMemory results that would be impossible to replicate on hardware, which can lead to game code making the wrong assumptions about memory layout. * This implements better merging for AllocatedMemoryBlocks. * Yuzu's implementation of svcMirrorMemory unprotected the entire virtual memory range containing the range being mirrored. This could lead to games attempting to map data at that unprotected range/attempting to access that range after yuzu improperly unmapped it. * This PR fixes it by simply calling ReprotectRange instead of Reprotect. --- src/core/hle/kernel/svc.cpp | 110 ++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 102 insertions(+), 8 deletions(-) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 332573a95..abb374892 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -729,8 +729,8 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha StackRegionBaseAddr = 14, StackRegionSize = 15, // 3.0.0+ - IsVirtualAddressMemoryEnabled = 16, - PersonalMmHeapUsage = 17, + SystemResourceSize = 16, + SystemResourceUsage = 17, TitleId = 18, // 4.0.0+ PrivilegedProcessId = 19, @@ -756,8 +756,8 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha case GetInfoType::StackRegionSize: case GetInfoType::TotalPhysicalMemoryAvailable: case GetInfoType::TotalPhysicalMemoryUsed: - case GetInfoType::IsVirtualAddressMemoryEnabled: - case GetInfoType::PersonalMmHeapUsage: + case GetInfoType::SystemResourceSize: + case GetInfoType::SystemResourceUsage: case GetInfoType::TitleId: case GetInfoType::UserExceptionContextAddr: case GetInfoType::TotalPhysicalMemoryAvailableWithoutMmHeap: @@ -822,8 +822,22 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha *result = process->GetTotalPhysicalMemoryUsed(); return RESULT_SUCCESS; - case GetInfoType::IsVirtualAddressMemoryEnabled: - *result = process->IsVirtualMemoryEnabled(); + case GetInfoType::SystemResourceSize: + *result = process->GetSystemResourceSize(); + return RESULT_SUCCESS; + + case GetInfoType::SystemResourceUsage: + // On hardware, this returns the amount of system resource memory that has + // been used by the kernel. This is problematic for Yuzu to emulate, because + // system resource memory is used for page tables -- and yuzu doesn't really + // have a way to calculate how much memory is required for page tables for + // the current process at any given time. + // TODO: Is this even worth implementing? No game should ever use it, since + // the amount of remaining page table space should never be relevant except + // for diagnostics. Is returning a value other than zero wise? + LOG_WARNING(Kernel_SVC, + "(STUBBED) Attempted to query system resource usage, returned 0"); + *result = 0; return RESULT_SUCCESS; case GetInfoType::TitleId: @@ -946,6 +960,86 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha } } +/// Maps memory at a desired address +static ResultCode MapPhysicalMemory(Core::System& system, VAddr addr, u64 size) { + LOG_DEBUG(Kernel_SVC, "called, addr=0x{:016X}, size=0x{:X}", addr, size); + + if (!Common::Is4KBAligned(addr)) { + LOG_ERROR(Kernel_SVC, "Address is not aligned to 4KB, 0x{:016X}", addr); + return ERR_INVALID_ADDRESS; + } + + if (!Common::Is4KBAligned(size)) { + LOG_ERROR(Kernel_SVC, "Size is not aligned to 4KB, 0x{:X}", size); + return ERR_INVALID_SIZE; + } + + if (size == 0) { + LOG_ERROR(Kernel_SVC, "Size is zero"); + return ERR_INVALID_SIZE; + } + + if (!(addr < addr + size)) { + LOG_ERROR(Kernel_SVC, "Size causes 64-bit overflow of address"); + return ERR_INVALID_MEMORY_RANGE; + } + + auto* const current_process = Core::CurrentProcess(); + auto& vm_manager = current_process->VMManager(); + + if (current_process->GetSystemResourceSize() == 0) { + LOG_ERROR(Kernel_SVC, "System Resource Size is zero"); + return ERR_INVALID_STATE; + } + + if (!vm_manager.IsWithinMapRegion(addr, size)) { + LOG_ERROR(Kernel_SVC, "Range not within map region"); + return ERR_INVALID_MEMORY_RANGE; + } + + return vm_manager.MapPhysicalMemory(addr, size); +} + +/// Unmaps memory previously mapped via MapPhysicalMemory +static ResultCode UnmapPhysicalMemory(Core::System& system, VAddr addr, u64 size) { + LOG_DEBUG(Kernel_SVC, "called, addr=0x{:016X}, size=0x{:X}", addr, size); + + if (!Common::Is4KBAligned(addr)) { + LOG_ERROR(Kernel_SVC, "Address is not aligned to 4KB, 0x{:016X}", addr); + return ERR_INVALID_ADDRESS; + } + + if (!Common::Is4KBAligned(size)) { + LOG_ERROR(Kernel_SVC, "Size is not aligned to 4KB, 0x{:X}", size); + return ERR_INVALID_SIZE; + } + + if (size == 0) { + LOG_ERROR(Kernel_SVC, "Size is zero"); + return ERR_INVALID_SIZE; + } + + if (!(addr < addr + size)) { + LOG_ERROR(Kernel_SVC, "Size causes 64-bit overflow of address"); + return ERR_INVALID_MEMORY_RANGE; + } + + auto* const current_process = Core::CurrentProcess(); + auto& vm_manager = current_process->VMManager(); + + if (current_process->GetSystemResourceSize() == 0) { + LOG_ERROR(Kernel_SVC, "System Resource Size is zero"); + return ERR_INVALID_STATE; + } + + if (!vm_manager.IsWithinMapRegion(addr, size)) { + LOG_ERROR(Kernel_SVC, "Range not within map region"); + return ERR_INVALID_MEMORY_RANGE; + } + + return vm_manager.UnmapPhysicalMemory(addr, size); +} + /// Sets the thread activity static ResultCode SetThreadActivity(Core::System& system, Handle handle, u32 activity) { LOG_DEBUG(Kernel_SVC, "called, handle=0x{:08X}, activity=0x{:08X}", handle, activity); @@ -2303,8 +2397,8 @@ static const FunctionDef SVC_Table[] = { {0x29, SvcWrap, "GetInfo"}, {0x2A, nullptr, "FlushEntireDataCache"}, {0x2B, nullptr, "FlushDataCache"}, - {0x2C, nullptr, "MapPhysicalMemory"}, - {0x2D, nullptr, "UnmapPhysicalMemory"}, + {0x2C, SvcWrap, "MapPhysicalMemory"}, + {0x2D, SvcWrap, "UnmapPhysicalMemory"}, {0x2E, nullptr, "GetFutureThreadInfo"}, {0x2F, nullptr, "GetLastThreadInfo"}, {0x30, SvcWrap, "GetResourceLimitLimitValue"}, -- cgit v1.2.3 From 1689784c198f6a7f3c389d4cd5edeccc74c1d9f3 Mon Sep 17 00:00:00 2001 From: Michael Scire Date: Sun, 7 Jul 2019 11:48:11 -0700 Subject: address review commentary --- src/core/hle/kernel/svc.cpp | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index abb374892..85e4512f0 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -737,8 +737,8 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha // 5.0.0+ UserExceptionContextAddr = 20, // 6.0.0+ - TotalPhysicalMemoryAvailableWithoutMmHeap = 21, - TotalPhysicalMemoryUsedWithoutMmHeap = 22, + TotalPhysicalMemoryAvailableWithoutSystemResource = 21, + TotalPhysicalMemoryUsedWithoutSystemResource = 22, }; const auto info_id_type = static_cast(info_id); @@ -760,8 +760,8 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha case GetInfoType::SystemResourceUsage: case GetInfoType::TitleId: case GetInfoType::UserExceptionContextAddr: - case GetInfoType::TotalPhysicalMemoryAvailableWithoutMmHeap: - case GetInfoType::TotalPhysicalMemoryUsedWithoutMmHeap: { + case GetInfoType::TotalPhysicalMemoryAvailableWithoutSystemResource: + case GetInfoType::TotalPhysicalMemoryUsedWithoutSystemResource: { if (info_sub_id != 0) { return ERR_INVALID_ENUM_VALUE; } @@ -827,17 +827,9 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha return RESULT_SUCCESS; case GetInfoType::SystemResourceUsage: - // On hardware, this returns the amount of system resource memory that has - // been used by the kernel. This is problematic for Yuzu to emulate, because - // system resource memory is used for page tables -- and yuzu doesn't really - // have a way to calculate how much memory is required for page tables for - // the current process at any given time. - // TODO: Is this even worth implementing? No game should ever use it, since - // the amount of remaining page table space should never be relevant except - // for diagnostics. Is returning a value other than zero wise? LOG_WARNING(Kernel_SVC, - "(STUBBED) Attempted to query system resource usage, returned 0"); - *result = 0; + "(STUBBED) Attempted to query system resource usage"); + *result = process->GetSystemResourceUsage(); return RESULT_SUCCESS; case GetInfoType::TitleId: @@ -850,12 +842,12 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha *result = 0; return RESULT_SUCCESS; - case GetInfoType::TotalPhysicalMemoryAvailableWithoutMmHeap: - *result = process->GetTotalPhysicalMemoryAvailable(); + case GetInfoType::TotalPhysicalMemoryAvailableWithoutSystemResource: + *result = process->GetTotalPhysicalMemoryAvailableWithoutSystemResource(); return RESULT_SUCCESS; - case GetInfoType::TotalPhysicalMemoryUsedWithoutMmHeap: - *result = process->GetTotalPhysicalMemoryUsedWithoutMmHeap(); + case GetInfoType::TotalPhysicalMemoryUsedWithoutSystemResource: + *result = process->GetTotalPhysicalMemoryUsedWithoutSystemResource(); return RESULT_SUCCESS; default: @@ -984,7 +976,7 @@ static ResultCode MapPhysicalMemory(Core::System& system, VAddr addr, u64 size) return ERR_INVALID_MEMORY_RANGE; } - auto* const current_process = Core::CurrentProcess(); + Process* const current_process = system.Kernel().CurrentProcess(); auto& vm_manager = current_process->VMManager(); if (current_process->GetSystemResourceSize() == 0) { @@ -1024,7 +1016,7 @@ static ResultCode UnmapPhysicalMemory(Core::System& system, VAddr addr, u64 size return ERR_INVALID_MEMORY_RANGE; } - auto* const current_process = Core::CurrentProcess(); + Process* const current_process = system.Kernel().CurrentProcess(); auto& vm_manager = current_process->VMManager(); if (current_process->GetSystemResourceSize() == 0) { -- cgit v1.2.3 From b901cd584e7c63ace5feb095279e1671fc605129 Mon Sep 17 00:00:00 2001 From: Michael Scire Date: Sun, 7 Jul 2019 12:08:29 -0700 Subject: clang-format fixes --- src/core/hle/kernel/svc.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/core/hle/kernel/svc.cpp') diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 85e4512f0..f9fe7dce0 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -827,8 +827,7 @@ static ResultCode GetInfo(Core::System& system, u64* result, u64 info_id, u64 ha return RESULT_SUCCESS; case GetInfoType::SystemResourceUsage: - LOG_WARNING(Kernel_SVC, - "(STUBBED) Attempted to query system resource usage"); + LOG_WARNING(Kernel_SVC, "(STUBBED) Attempted to query system resource usage"); *result = process->GetSystemResourceUsage(); return RESULT_SUCCESS; -- cgit v1.2.3