From 586cab617270346c39ecfb340127e0b8edb863d3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 15:24:52 -0400 Subject: 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/vm_manager.cpp | 42 ++++++++++++++++++++------------------ 1 file changed, 22 insertions(+), 20 deletions(-) (limited to 'src/core/hle/kernel/vm_manager.cpp') 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; + if (size > GetCurrentHeapSize()) { + const u64 alloc_size = size - GetCurrentHeapSize(); + + heap_memory->insert(heap_memory->end(), alloc_size, 0); + heap_end = heap_region_base + size; RefreshMemoryBlockMappings(heap_memory.get()); } - ASSERT(heap_end - heap_start == heap_memory->size()); + ASSERT(GetCurrentHeapSize() == heap_memory->size()); - CASCADE_RESULT(auto vma, MapMemoryBlock(target, heap_memory, target - heap_start, size, - MemoryState::Heap)); - Reprotect(vma, perms); + 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()); -- cgit v1.2.3 From 52980df1aa796bb1eba3e5fec921eab1df961e51 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 15:56:07 -0400 Subject: 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/vm_manager.cpp | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) (limited to 'src/core/hle/kernel/vm_manager.cpp') 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; } -- cgit v1.2.3 From abdb81ccaf9e32c8b605af690922599d974a3ee7 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:05:25 -0400 Subject: 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(+) (limited to 'src/core/hle/kernel/vm_manager.cpp') 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); -- cgit v1.2.3 From 99a163478be9ca285280ee59aa7800903b8571c2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:28:04 -0400 Subject: 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/vm_manager.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/core/hle/kernel/vm_manager.cpp') 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; } -- cgit v1.2.3 From 1e92ba27855a40d928265debfdf2478248e40eaf Mon Sep 17 00:00:00 2001 From: Lioncash Date: Sun, 24 Mar 2019 16:30:45 -0400 Subject: 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 +++++++++++++--------------------- 1 file changed, 13 insertions(+), 21 deletions(-) (limited to 'src/core/hle/kernel/vm_manager.cpp') 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{}; -- cgit v1.2.3