From 586cab617270346c39ecfb340127e0b8edb863d3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 15:24:52 -0400 Subject: [PATCH 1/6] kernel/vm_manager: Tidy up heap allocation code Another holdover from citra that can be tossed out is the notion of the heap needing to be allocated in different addresses. On the switch, the base address of the heap will always be managed by the memory allocator in the kernel, so this doesn't need to be specified in the function's interface itself. The heap on the switch is always allocated with read/write permissions, so we don't need to add specifying the memory permissions as part of the heap allocation itself either. This also corrects the error code returned from within the function. If the size of the heap is larger than the entire heap region, then the kernel will report an out of memory condition. --- src/core/hle/kernel/svc.cpp | 6 ++-- src/core/hle/kernel/vm_manager.cpp | 46 ++++++++++++++++-------------- src/core/hle/kernel/vm_manager.h | 16 +++++++++-- 3 files changed, 39 insertions(+), 29 deletions(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index a6a17efe7..59bc8d9f8 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -174,10 +174,8 @@ static ResultCode SetHeapSize(VAddr* heap_addr, u64 heap_size) { return ERR_INVALID_SIZE; } - auto& vm_manager = Core::CurrentProcess()->VMManager(); - const VAddr heap_base = vm_manager.GetHeapRegionBaseAddress(); - const auto alloc_result = - vm_manager.HeapAllocate(heap_base, heap_size, VMAPermission::ReadWrite); + auto& vm_manager = Core::System::GetInstance().Kernel().CurrentProcess()->VMManager(); + const auto alloc_result = vm_manager.HeapAllocate(heap_size); if (alloc_result.Failed()) { return alloc_result.Code(); diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 22bf55ce7..9848a8ac6 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -256,39 +256,37 @@ ResultCode VMManager::ReprotectRange(VAddr target, u64 size, VMAPermission new_p return RESULT_SUCCESS; } -ResultVal VMManager::HeapAllocate(VAddr target, u64 size, VMAPermission perms) { - if (!IsWithinHeapRegion(target, size)) { - return ERR_INVALID_ADDRESS; +ResultVal VMManager::HeapAllocate(u64 size) { + if (size > GetHeapRegionSize()) { + return ERR_OUT_OF_MEMORY; } if (heap_memory == nullptr) { // Initialize heap - heap_memory = std::make_shared>(); - heap_start = heap_end = target; + heap_memory = std::make_shared>(size); + heap_end = heap_region_base + size; } else { - UnmapRange(heap_start, heap_end - heap_start); + UnmapRange(heap_region_base, GetCurrentHeapSize()); } // If necessary, expand backing vector to cover new heap extents. - if (target < heap_start) { - heap_memory->insert(begin(*heap_memory), heap_start - target, 0); - heap_start = target; - RefreshMemoryBlockMappings(heap_memory.get()); - } - if (target + size > heap_end) { - heap_memory->insert(end(*heap_memory), (target + size) - heap_end, 0); - heap_end = target + size; - RefreshMemoryBlockMappings(heap_memory.get()); - } - ASSERT(heap_end - heap_start == heap_memory->size()); + if (size > GetCurrentHeapSize()) { + const u64 alloc_size = size - GetCurrentHeapSize(); - CASCADE_RESULT(auto vma, MapMemoryBlock(target, heap_memory, target - heap_start, size, - MemoryState::Heap)); - Reprotect(vma, perms); + heap_memory->insert(heap_memory->end(), alloc_size, 0); + heap_end = heap_region_base + size; + RefreshMemoryBlockMappings(heap_memory.get()); + } + ASSERT(GetCurrentHeapSize() == heap_memory->size()); + + const auto mapping_result = + MapMemoryBlock(heap_region_base, heap_memory, 0, size, MemoryState::Heap); + if (mapping_result.Failed()) { + return mapping_result.Code(); + } heap_used = size; - - return MakeResult(heap_end - size); + return MakeResult(heap_region_base); } ResultCode VMManager::HeapFree(VAddr target, u64 size) { @@ -778,6 +776,10 @@ u64 VMManager::GetHeapRegionSize() const { return heap_region_end - heap_region_base; } +u64 VMManager::GetCurrentHeapSize() const { + return heap_end - heap_region_base; +} + bool VMManager::IsWithinHeapRegion(VAddr address, u64 size) const { return IsInsideAddressRange(address, size, GetHeapRegionBaseAddress(), GetHeapRegionEndAddress()); diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 7cdff6094..23edd6582 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -380,7 +380,7 @@ public: /// Changes the permissions of a range of addresses, splitting VMAs as necessary. ResultCode ReprotectRange(VAddr target, u64 size, VMAPermission new_perms); - ResultVal HeapAllocate(VAddr target, u64 size, VMAPermission perms); + ResultVal HeapAllocate(u64 size); ResultCode HeapFree(VAddr target, u64 size); ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state); @@ -469,6 +469,13 @@ public: /// Gets the total size of the heap region in bytes. u64 GetHeapRegionSize() const; + /// Gets the total size of the current heap in bytes. + /// + /// @note This is the current allocated heap size, not the size + /// of the region it's allowed to exist within. + /// + u64 GetCurrentHeapSize() const; + /// Determines whether or not the specified range is within the heap region. bool IsWithinHeapRegion(VAddr address, u64 size) const; @@ -628,9 +635,12 @@ private: // This makes deallocation and reallocation of holes fast and keeps process memory contiguous // in the emulator address space, allowing Memory::GetPointer to be reasonably safe. std::shared_ptr> heap_memory; - // The left/right bounds of the address space covered by heap_memory. - VAddr heap_start = 0; + + // The end of the currently allocated heap. This is not an inclusive + // end of the range. This is essentially 'base_address + current_size'. VAddr heap_end = 0; + + // Indicates how many bytes from the current heap are currently used. u64 heap_used = 0; }; } // namespace Kernel From 52980df1aa796bb1eba3e5fec921eab1df961e51 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 15:56:07 -0400 Subject: [PATCH 2/6] kernel/vm_manager: Remove unnecessary heap_used data member This isn't required anymore, as all the kernel ever queries is the size of the current heap, not the total usage of it. --- src/core/hle/kernel/svc.cpp | 2 +- src/core/hle/kernel/vm_manager.cpp | 7 +------ src/core/hle/kernel/vm_manager.h | 6 ------ 3 files changed, 2 insertions(+), 13 deletions(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index 59bc8d9f8..f689f745f 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -806,7 +806,7 @@ static ResultCode GetInfo(u64* result, u64 info_id, u64 handle, u64 info_sub_id) return RESULT_SUCCESS; case GetInfoType::TotalHeapUsage: - *result = process->VMManager().GetTotalHeapUsage(); + *result = process->VMManager().GetCurrentHeapSize(); return RESULT_SUCCESS; case GetInfoType::IsVirtualAddressMemoryEnabled: diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 9848a8ac6..04d58bbf7 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -285,7 +285,6 @@ ResultVal VMManager::HeapAllocate(u64 size) { return mapping_result.Code(); } - heap_used = size; return MakeResult(heap_region_base); } @@ -303,7 +302,6 @@ ResultCode VMManager::HeapFree(VAddr target, u64 size) { return result; } - heap_used -= size; return RESULT_SUCCESS; } @@ -596,6 +594,7 @@ void VMManager::InitializeMemoryRegionRanges(FileSys::ProgramAddressSpaceType ty heap_region_base = map_region_end; heap_region_end = heap_region_base + heap_region_size; + heap_end = heap_region_base; new_map_region_base = heap_region_end; new_map_region_end = new_map_region_base + new_map_region_size; @@ -690,10 +689,6 @@ u64 VMManager::GetTotalMemoryUsage() const { return 0xF8000000; } -u64 VMManager::GetTotalHeapUsage() const { - return heap_used; -} - VAddr VMManager::GetAddressSpaceBaseAddress() const { return address_space_base; } diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index 23edd6582..cb864ab31 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -418,9 +418,6 @@ public: /// Gets the total memory usage, used by svcGetInfo u64 GetTotalMemoryUsage() const; - /// Gets the total heap usage, used by svcGetInfo - u64 GetTotalHeapUsage() const; - /// Gets the address space base address VAddr GetAddressSpaceBaseAddress() const; @@ -639,8 +636,5 @@ private: // The end of the currently allocated heap. This is not an inclusive // end of the range. This is essentially 'base_address + current_size'. VAddr heap_end = 0; - - // Indicates how many bytes from the current heap are currently used. - u64 heap_used = 0; }; } // namespace Kernel From 9f63acac0f30bdb864746da84c021d05b4e8fa08 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:00:22 -0400 Subject: [PATCH 3/6] kernel/vm_manager: Remove unused class variables Over time these have fallen out of use due to refactoring, so these can be removed. --- src/core/hle/kernel/vm_manager.h | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index cb864ab31..ac5c33087 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -621,9 +621,6 @@ private: VAddr new_map_region_base = 0; VAddr new_map_region_end = 0; - VAddr main_code_region_base = 0; - VAddr main_code_region_end = 0; - VAddr tls_io_region_base = 0; VAddr tls_io_region_end = 0; From abdb81ccaf9e32c8b605af690922599d974a3ee7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:05:25 -0400 Subject: [PATCH 4/6] kernel/vm_manager: Handle case of identical calls to HeapAllocate In cases where HeapAllocate is called with the same size of the current heap, we can simply do nothing and return successfully. This avoids doing work where we otherwise don't have to. This is also what the kernel itself does in this scenario. --- src/core/hle/kernel/vm_manager.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 04d58bbf7..16f48471e 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -261,6 +261,11 @@ ResultVal VMManager::HeapAllocate(u64 size) { return ERR_OUT_OF_MEMORY; } + // No need to do any additional work if the heap is already the given size. + if (size == GetCurrentHeapSize()) { + return MakeResult(heap_region_base); + } + if (heap_memory == nullptr) { // Initialize heap heap_memory = std::make_shared>(size); From 99a163478be9ca285280ee59aa7800903b8571c2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:28:04 -0400 Subject: [PATCH 5/6] kernel/vm_manager: Rename HeapAllocate to SetHeapSize Makes it more obvious that this function is intending to stand in for the actual supervisor call itself, and not acting as a general heap allocation function. Also the following change will merge the freeing behavior of HeapFree into this function, so leaving it as HeapAllocate would be misleading. --- src/core/hle/kernel/svc.cpp | 3 +-- src/core/hle/kernel/vm_manager.cpp | 2 +- src/core/hle/kernel/vm_manager.h | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/core/hle/kernel/svc.cpp b/src/core/hle/kernel/svc.cpp index f689f745f..6a8960c8d 100644 --- a/src/core/hle/kernel/svc.cpp +++ b/src/core/hle/kernel/svc.cpp @@ -175,8 +175,7 @@ static ResultCode SetHeapSize(VAddr* heap_addr, u64 heap_size) { } auto& vm_manager = Core::System::GetInstance().Kernel().CurrentProcess()->VMManager(); - const auto alloc_result = vm_manager.HeapAllocate(heap_size); - + const auto alloc_result = vm_manager.SetHeapSize(heap_size); if (alloc_result.Failed()) { return alloc_result.Code(); } diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 16f48471e..523fe63e9 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -256,7 +256,7 @@ ResultCode VMManager::ReprotectRange(VAddr target, u64 size, VMAPermission new_p return RESULT_SUCCESS; } -ResultVal VMManager::HeapAllocate(u64 size) { +ResultVal VMManager::SetHeapSize(u64 size) { if (size > GetHeapRegionSize()) { return ERR_OUT_OF_MEMORY; } diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index ac5c33087..cab748364 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -380,7 +380,7 @@ public: /// Changes the permissions of a range of addresses, splitting VMAs as necessary. ResultCode ReprotectRange(VAddr target, u64 size, VMAPermission new_perms); - ResultVal HeapAllocate(u64 size); + ResultVal SetHeapSize(u64 size); ResultCode HeapFree(VAddr target, u64 size); ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state); From 1e92ba27855a40d928265debfdf2478248e40eaf Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:30:45 -0400 Subject: [PATCH 6/6] kernel/vm_manager: Handle shrinking of the heap size within SetHeapSize() One behavior that we weren't handling properly in our heap allocation process was the ability for the heap to be shrunk down in size if a larger size was previously requested. This adds the basic behavior to do so and also gets rid of HeapFree, as it's no longer necessary now that we have allocations and deallocations going through the same API function. While we're at it, fully document the behavior that this function performs. --- src/core/hle/kernel/vm_manager.cpp | 34 +++++++++++----------------- src/core/hle/kernel/vm_manager.h | 36 +++++++++++++++++++++++++++--- 2 files changed, 46 insertions(+), 24 deletions(-) diff --git a/src/core/hle/kernel/vm_manager.cpp b/src/core/hle/kernel/vm_manager.cpp index 523fe63e9..ec0a480ce 100644 --- a/src/core/hle/kernel/vm_manager.cpp +++ b/src/core/hle/kernel/vm_manager.cpp @@ -274,14 +274,23 @@ ResultVal VMManager::SetHeapSize(u64 size) { UnmapRange(heap_region_base, GetCurrentHeapSize()); } - // If necessary, expand backing vector to cover new heap extents. - if (size > GetCurrentHeapSize()) { - const u64 alloc_size = size - GetCurrentHeapSize(); + // If necessary, expand backing vector to cover new heap extents in + // the case of allocating. Otherwise, shrink the backing memory, + // if a smaller heap has been requested. + const u64 old_heap_size = GetCurrentHeapSize(); + if (size > old_heap_size) { + const u64 alloc_size = size - old_heap_size; heap_memory->insert(heap_memory->end(), alloc_size, 0); - heap_end = heap_region_base + size; + RefreshMemoryBlockMappings(heap_memory.get()); + } else if (size < old_heap_size) { + heap_memory->resize(size); + heap_memory->shrink_to_fit(); + RefreshMemoryBlockMappings(heap_memory.get()); } + + heap_end = heap_region_base + size; ASSERT(GetCurrentHeapSize() == heap_memory->size()); const auto mapping_result = @@ -293,23 +302,6 @@ ResultVal VMManager::SetHeapSize(u64 size) { return MakeResult(heap_region_base); } -ResultCode VMManager::HeapFree(VAddr target, u64 size) { - if (!IsWithinHeapRegion(target, size)) { - return ERR_INVALID_ADDRESS; - } - - if (size == 0) { - return RESULT_SUCCESS; - } - - const ResultCode result = UnmapRange(target, size); - if (result.IsError()) { - return result; - } - - return RESULT_SUCCESS; -} - MemoryInfo VMManager::QueryMemory(VAddr address) const { const auto vma = FindVMA(address); MemoryInfo memory_info{}; diff --git a/src/core/hle/kernel/vm_manager.h b/src/core/hle/kernel/vm_manager.h index cab748364..6f484b7bf 100644 --- a/src/core/hle/kernel/vm_manager.h +++ b/src/core/hle/kernel/vm_manager.h @@ -380,11 +380,41 @@ public: /// Changes the permissions of a range of addresses, splitting VMAs as necessary. ResultCode ReprotectRange(VAddr target, u64 size, VMAPermission new_perms); - ResultVal SetHeapSize(u64 size); - ResultCode HeapFree(VAddr target, u64 size); - ResultCode MirrorMemory(VAddr dst_addr, VAddr src_addr, u64 size, MemoryState state); + /// Attempts to allocate a heap with the given size. + /// + /// @param size The size of the heap to allocate in bytes. + /// + /// @note If a heap is currently allocated, and this is called + /// with a size that is equal to the size of the current heap, + /// then this function will do nothing and return the current + /// heap's starting address, as there's no need to perform + /// any additional heap allocation work. + /// + /// @note If a heap is currently allocated, and this is called + /// with a size less than the current heap's size, then + /// this function will attempt to shrink the heap. + /// + /// @note If a heap is currently allocated, and this is called + /// with a size larger than the current heap's size, then + /// this function will attempt to extend the size of the heap. + /// + /// @returns A result indicating either success or failure. + ///

+ /// If successful, this function will return a result + /// containing the starting address to the allocated heap. + ///

+ /// If unsuccessful, this function will return a result + /// containing an error code. + /// + /// @pre The given size must lie within the allowable heap + /// memory region managed by this VMManager instance. + /// Failure to abide by this will result in ERR_OUT_OF_MEMORY + /// being returned as the result. + /// + ResultVal SetHeapSize(u64 size); + /// Queries the memory manager for information about the given address. /// /// @param address The address to query the memory manager about for information.