diff --git a/src/kernel/memory.cpp b/src/kernel/memory.cpp index fb7c38b1..87a9bf94 100644 --- a/src/kernel/memory.cpp +++ b/src/kernel/memory.cpp @@ -21,6 +21,7 @@ #include #include #include +#include #include #if KYTY_PLATFORM == KYTY_PLATFORM_WINDOWS @@ -168,6 +169,7 @@ static bool VirtualRangesOverlap(uint64_t left_start, uint64_t left_size, uint64 #if defined(KYTY_VIRTUAL_MEMORY_ALLOCATION_TESTS) static uint32_t g_test_backing_store_unmaps_before_failure = UINT32_MAX; static callback_func_t g_test_before_backing_map = nullptr; +static callback_func_t g_test_backing_read = nullptr; #endif #include "memoryAddressSpace.inc" @@ -3569,6 +3571,10 @@ void TestBeforeNextBackingMap(callback_func_t callback) { g_test_before_backing_map = callback; } +void TestSetBackingReadCallback(callback_func_t callback) { + g_test_backing_read = callback; +} + void TestFailNextPhysicalMemoryUnmap() { TestFailPhysicalMemoryUnmapAfter(0); } diff --git a/src/kernel/memory.h b/src/kernel/memory.h index 70899896..4b82e41b 100644 --- a/src/kernel/memory.h +++ b/src/kernel/memory.h @@ -192,6 +192,7 @@ bool FreeGuestMemory(uint64_t vaddr, uint64_t size); #if defined(KYTY_VIRTUAL_MEMORY_ALLOCATION_TESTS) void TestBeforeNextBackingMap(callback_func_t callback); +void TestSetBackingReadCallback(callback_func_t callback); void TestFailNextPhysicalMemoryUnmap(); void TestFailPhysicalMemoryUnmapAfter(uint32_t successful_unmaps); void TestFailGuestBackingStoreUnmapAfter(uint32_t successful_unmaps); diff --git a/src/kernel/memoryAddressSpace.inc b/src/kernel/memoryAddressSpace.inc index 346c8b1a..3627366f 100644 --- a/src/kernel/memoryAddressSpace.inc +++ b/src/kernel/memoryAddressSpace.inc @@ -161,30 +161,90 @@ public: if (!IsAvailable() || !RangeOk(backing_offset, size)) { return false; } + std::lock_guard lock(m_mutex); std::memset(m_backing_base + backing_offset, 0, size); return true; } bool TryWriteBacking(uint64_t vaddr, const void* data, uint64_t size) { - if (data == nullptr) { + if (!IsAvailable() || data == nullptr || size == 0 || UINT64_MAX - vaddr < size) { return false; } + + std::lock_guard lock(m_mutex); + const auto end = vaddr + size; + auto first = m_maps.upper_bound(vaddr); + if (first == m_maps.begin()) { + return false; + } + --first; + auto current = vaddr; + for (auto mapping = first; current < end; ++mapping) { + if (mapping == m_maps.end() || current < mapping->second.vaddr || + current >= mapping->second.vaddr + mapping->second.size) { + return false; + } + const auto& entry = mapping->second; + const auto bytes = std::min(end, entry.vaddr + entry.size) - current; + if (!RangeOk(entry.backing_offset + current - entry.vaddr, bytes)) { + return false; + } + current += bytes; + } + const auto* source = static_cast(data); - return TryTransferBacking(vaddr, size, - [source](uint8_t* backing, uint64_t data_offset, size_t bytes) { - std::memcpy(backing, source + data_offset, bytes); - }); + current = vaddr; + for (auto mapping = first; current < end; ++mapping) { + const auto& entry = mapping->second; + const auto bytes = std::min(end, entry.vaddr + entry.size) - current; + const auto offset = entry.backing_offset + current - entry.vaddr; + std::memcpy(m_backing_base + offset, source + (current - vaddr), bytes); + current += bytes; + } + return true; } bool TryReadBacking(uint64_t vaddr, void* data, uint64_t size) { - if (data == nullptr) { + if (!IsAvailable() || data == nullptr || size == 0 || UINT64_MAX - vaddr < size) { return false; } + + std::shared_lock lock(m_mutex); +#if defined(KYTY_VIRTUAL_MEMORY_ALLOCATION_TESTS) + if (g_test_backing_read != nullptr) { + g_test_backing_read(vaddr, size); + } +#endif + const auto end = vaddr + size; + auto first = m_maps.upper_bound(vaddr); + if (first == m_maps.begin()) { + return false; + } + --first; + auto current = vaddr; + for (auto mapping = first; current < end; ++mapping) { + if (mapping == m_maps.end() || current < mapping->second.vaddr || + current >= mapping->second.vaddr + mapping->second.size) { + return false; + } + const auto& entry = mapping->second; + const auto bytes = std::min(end, entry.vaddr + entry.size) - current; + if (!RangeOk(entry.backing_offset + current - entry.vaddr, bytes)) { + return false; + } + current += bytes; + } + auto* destination = static_cast(data); - return TryTransferBacking( - vaddr, size, [destination](uint8_t* backing, uint64_t data_offset, size_t bytes) { - std::memcpy(destination + data_offset, backing, bytes); - }); + current = vaddr; + for (auto mapping = first; current < end; ++mapping) { + const auto& entry = mapping->second; + const auto bytes = std::min(end, entry.vaddr + entry.size) - current; + const auto offset = entry.backing_offset + current - entry.vaddr; + std::memcpy(destination + (current - vaddr), m_backing_base + offset, bytes); + current += bytes; + } + return true; } bool TryReadSparseBacking(uint64_t vaddr, void* data, uint64_t size) { @@ -192,7 +252,12 @@ public: return false; } - std::lock_guard lock(m_mutex); + std::shared_lock lock(m_mutex); +#if defined(KYTY_VIRTUAL_MEMORY_ALLOCATION_TESTS) + if (g_test_backing_read != nullptr) { + g_test_backing_read(vaddr, size); + } +#endif const auto end = vaddr + size; auto first = m_maps.lower_bound(vaddr); if (first != m_maps.begin()) { @@ -437,7 +502,7 @@ public: return false; } - std::lock_guard lock(m_mutex); + std::shared_lock lock(m_mutex); const auto end = vaddr + size; auto current = vaddr; @@ -457,39 +522,6 @@ public: } private: - template - bool TryTransferBacking(uint64_t vaddr, uint64_t size, Transfer transfer) { - if (!IsAvailable() || size == 0 || UINT64_MAX - vaddr < size) { - return false; - } - - std::lock_guard lock(m_mutex); - const auto end = vaddr + size; - auto current = vaddr; - while (current < end) { - MapEntry entry {}; - if (!FindContainingUnlocked(current, 1, &entry)) { - return false; - } - const auto bytes = std::min(end, entry.vaddr + entry.size) - current; - if (!RangeOk(entry.backing_offset + current - entry.vaddr, bytes)) { - return false; - } - current += bytes; - } - - current = vaddr; - while (current < end) { - MapEntry entry {}; - EXIT_IF(!FindContainingUnlocked(current, 1, &entry)); - const auto bytes = std::min(end, entry.vaddr + entry.size) - current; - const auto offset = entry.backing_offset + current - entry.vaddr; - transfer(m_backing_base + offset, current - vaddr, static_cast(bytes)); - current += bytes; - } - return true; - } - struct MapEntry { uint64_t vaddr = 0; uint64_t size = 0; @@ -656,7 +688,7 @@ private: uint8_t* m_backing_base = nullptr; uint64_t m_size = 0; - std::mutex m_mutex; + std::shared_mutex m_mutex; std::map m_maps; }; @@ -727,7 +759,7 @@ public: if (size == 0 || UINT64_MAX - vaddr < size) { return false; } - std::lock_guard lock(m_mutex); + std::shared_lock lock(m_mutex); return std::any_of(m_owned.begin(), m_owned.end(), [vaddr, size](const auto& range) { return vaddr >= range.first && vaddr + size <= range.first + range.second; }); @@ -755,7 +787,7 @@ public: if (!Owns(vaddr, size)) { return false; } - std::lock_guard lock(m_mutex); + std::shared_lock lock(m_mutex); auto mapping = m_mapped.upper_bound(vaddr); if (mapping != m_mapped.begin()) { --mapping; @@ -1381,7 +1413,7 @@ private: #endif } - std::mutex m_mutex; + std::shared_mutex m_mutex; std::map m_free; std::map m_mapped; std::vector> m_owned; diff --git a/tests/VirtualMemoryAllocationTests.cpp b/tests/VirtualMemoryAllocationTests.cpp index bdf9e4b0..15e1553a 100644 --- a/tests/VirtualMemoryAllocationTests.cpp +++ b/tests/VirtualMemoryAllocationTests.cpp @@ -24,6 +24,7 @@ #include #include #include +#include #include #include #include @@ -586,6 +587,107 @@ void TestSparseReadDuringDirectCommit() { std::printf("[host] %-48s ok\n", test); } +std::counting_semaphore<2>* g_backing_reads_entered = nullptr; +std::counting_semaphore<2>* g_continue_backing_reads = nullptr; + +void ParkBackingRead(uintptr_t, size_t) { + g_backing_reads_entered->release(); + g_continue_backing_reads->acquire(); +} + +void TestConcurrentBackingReads() { + using namespace Libs::LibKernel::Memory; + const char* test = "ConcurrentBackingReads"; + constexpr uint64_t original = 0x123456789abcdef0ull; + for (const auto& readers: {std::pair {TryReadBacking, TryReadBacking}, + std::pair {TryReadBacking, TryReadSparseBacking}, + std::pair {TryReadSparseBacking, TryReadSparseBacking}}) { + const auto base = MapNamedFlexible(test, SceKernelPageSize, SceKernelProtCpuRw, + "concurrent_backing_reads"); + std::memcpy(reinterpret_cast(base), &original, sizeof(original)); + std::counting_semaphore<2> entered {0}; + std::counting_semaphore<2> resume {0}; + g_backing_reads_entered = &entered; + g_continue_backing_reads = &resume; + TestSetBackingReadCallback(ParkBackingRead); + std::array values {}; + std::array results {}; + std::thread first([&] { results[0] = readers.first(base, &values[0], sizeof(values[0])); }); + std::thread second( + [&] { results[1] = readers.second(base, &values[1], sizeof(values[1])); }); + const bool concurrent = entered.try_acquire_for(std::chrono::seconds(5)) && + entered.try_acquire_for(std::chrono::seconds(5)); + // Release both threads even if an exclusive read lock prevents the second + // entry. + resume.release(2); + first.join(); + second.join(); + TestSetBackingReadCallback(nullptr); + CheckOk(test, KernelMunmap(base, SceKernelPageSize), "KernelMunmap"); + Check(test, concurrent, "backing reads could not hold their locks concurrently"); + Check(test, results[0] && results[1] && values[0] == original && values[1] == original, + "concurrent backing reads returned different contents"); + } + std::printf("[host] %-48s ok\n", test); +} + +void TestBackingReadExcludesWritesAndUnmap() { + using namespace Libs::LibKernel::Memory; + const char* test = "BackingReadExcludesWritesAndUnmap"; + constexpr uint64_t original = 0x123456789abcdef0ull; + constexpr uint64_t replacement = 0xfedcba9876543210ull; + for (const auto read: {TryReadBacking, TryReadSparseBacking}) { + for (const bool unmap: {false, true}) { + const auto base = MapNamedFlexible(test, SceKernelPageSize, SceKernelProtCpuRw, + "backing_read_exclusion"); + std::memcpy(reinterpret_cast(base), &original, sizeof(original)); + std::counting_semaphore<2> entered {0}; + std::counting_semaphore<2> resume {0}; + g_backing_reads_entered = &entered; + g_continue_backing_reads = &resume; + TestSetBackingReadCallback(ParkBackingRead); + uint64_t snapshot = 0; + bool read_ok = false; + std::thread reader([&] { read_ok = read(base, &snapshot, sizeof(snapshot)); }); + const bool reader_parked = entered.try_acquire_for(std::chrono::seconds(5)); + std::binary_semaphore started {0}; + std::binary_semaphore finished {0}; + bool write_ok = false; + int unmap_result = OK; + std::thread mutation([&] { + started.release(); + if (unmap) { + unmap_result = KernelMunmap(base, SceKernelPageSize); + } else { + write_ok = TryWriteBacking(base, &replacement, sizeof(replacement)); + } + finished.release(); + }); + started.acquire(); + const bool excluded = !finished.try_acquire_for(std::chrono::milliseconds(100)); + resume.release(); + reader.join(); + mutation.join(); + TestSetBackingReadCallback(nullptr); + uint64_t after = 0; + const bool still_backed = TryReadBacking(base, &after, sizeof(after)); + if (!unmap) { + CheckOk(test, KernelMunmap(base, SceKernelPageSize), "KernelMunmap"); + } + Check(test, reader_parked && excluded, + "backing write or unmap completed while a backing read held its " + "lock"); + Check(test, read_ok && snapshot == original, + "backing mutation changed an in-progress read"); + Check(test, + unmap ? unmap_result == OK && !still_backed + : write_ok && still_backed && after == replacement, + "backing mutation failed after the reader released its lock"); + } + } + std::printf("[host] %-48s ok\n", test); +} + void TestGuestAddressSpaceHasNoFixedFallback() { const char* test = "GuestAddressSpaceHasNoFixedFallback"; const auto unowned_address = reinterpret_cast(0x10000); @@ -4296,6 +4398,16 @@ void TestSmallFiberStacksAndMigration() { int main(int argc, char** argv) { InitSubsystems(); + if (argc == 2 && std::strcmp(argv[1], "--backing-transfers-only") == 0) { + RunTest(TestConcurrentBackingReads); + RunTest(TestBackingReadExcludesWritesAndUnmap); + RunTest(TestSparseBackingReadPreservesResidency); + RunTest(TestSparseReadDuringDirectCommit); + RunTest(TestDirectMapQueryOffsetAndPartialMunmap); + RunTest(TestFlexibleMemoryUsesSharedBacking); + RunTest(TestFlexibleMemoryReuseIsZeroFilled); + return g_failed_tests == 0 ? 0 : 1; + } #if defined(__linux__) if (argc == 2 && std::strcmp(argv[1], "--fixed-direct-replacement-only") == 0) { RunTest(TestFixedDirectReplacementPreservesAccess); @@ -4344,6 +4456,8 @@ int main(int argc, char** argv) { RunTest(TestGuestAddressSpaceOwnsReservationsBeforeBacking); RunTest(TestSparseBackingReadPreservesResidency); RunTest(TestSparseReadDuringDirectCommit); + RunTest(TestConcurrentBackingReads); + RunTest(TestBackingReadExcludesWritesAndUnmap); RunTest(TestGuestAddressSpaceHasNoFixedFallback); RunTest(TestGuestFreeRangeSearchDoesNotUnderflow); RunTest(TestFlexibleMemoryCapacityIsBootFixed);