From b14d087cd4cd9584a19ee3139b0fb4724feda70e Mon Sep 17 00:00:00 2001 From: umang Date: Mon, 12 Oct 2026 19:59:43 +0530 Subject: [PATCH 1/2] fix(agc): defer write tracking for fresh Windows shared imports --- core/libs/CMakeLists.txt | 10 + .../Execution/src/GuestMemory.cpp | 83 +++---- .../libSceAgcDriver/tests/WriteTracking.cpp | 60 +++++ core/libs/prx/libc/include/GuestArena.hpp | 2 + .../libs/prx/libc/include/WindowsMappings.hpp | 38 +++- core/libs/prx/libc/src/GuestArena.cpp | 8 + core/libs/tests/WindowsWriteWatch.cpp | 208 ++++++++++++++++++ 7 files changed, 357 insertions(+), 52 deletions(-) create mode 100644 core/libs/tests/WindowsWriteWatch.cpp diff --git a/core/libs/CMakeLists.txt b/core/libs/CMakeLists.txt index ca374ef2e5..07e9890094 100644 --- a/core/libs/CMakeLists.txt +++ b/core/libs/CMakeLists.txt @@ -203,6 +203,16 @@ if(BUILD_TESTING) if(APPLE) add_subdirectory(tests/write_watch) endif() + if(WIN32) + add_test_executable(windows_write_watch_tests tests/WindowsWriteWatch.cpp) + target_include_directories(windows_write_watch_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) + target_compile_definitions(windows_write_watch_tests PRIVATE _WIN32_WINNT=0x0A00) + if(MINGW) + target_compile_options(windows_write_watch_tests PRIVATE -fasynchronous-unwind-tables) + endif() + add_test(NAME windows_write_watch COMMAND windows_write_watch_tests) + set_tests_properties(windows_write_watch PROPERTIES TIMEOUT 20) + endif() add_test_executable(guest_offline_imports_tests tests/GuestOfflineImports.cpp) target_include_directories(guest_offline_imports_tests PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp index 555baf8583..f19c67b284 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp @@ -1025,7 +1025,6 @@ bool sharedCollectMemo() { return shared; } -#ifndef _WIN32 struct StampRuns { WriteTracker& tracker; StampKind kind; @@ -1046,59 +1045,44 @@ void stampWrittenRun(void* context, std::uintptr_t begin, std::uintptr_t end) { } else { for (auto block = tracker.blockOf(begin); block <= tracker.blockOf(end - 1); ++block) tracker.stamp(block, generation, kind); } +#ifndef _WIN32 collectDirtyRuns.fetch_add(1, std::memory_order_relaxed); +#endif +} + +#ifdef _WIN32 +bool collectWindowsWrites(StampRuns& runs, std::uint64_t first, std::uint64_t stop, bool armedOnly) { + auto& tracker = runs.tracker; + const bool fresh = runs.kind == StampKind::Fresh; + const auto collect = fresh ? &GuestArena::GuestArenaCollectFreshWrites_nid_postfix : armedOnly ? &GuestArena::GuestArenaCollectArmedWrites_nid_postfix : &GuestArena::GuestArenaCollectWrites_nid_postfix; + static const bool singlePass = std::getenv("APS5_NO_SINGLE_PASS_COLLECT") == nullptr; + const auto walk = [&](bool clear) { + auto cursor = first; + while (cursor < stop) { + std::size_t count = tracker.pages.size(); + if (!collect(cursor, static_cast(stop - cursor), tracker.pages.data(), &count, clear)) return false; + for (std::size_t i = 0; i < count; ++i) { + const auto page = reinterpret_cast(tracker.pages[i]); + stampWrittenRun(&runs, page, page + WritePageBytes); + } + if (count < tracker.pages.size()) break; + cursor = reinterpret_cast(tracker.pages[count - 1]) + WritePageBytes; + } + return true; + }; + if (!walk(singlePass || fresh)) return false; + if (runs.stamped != 0) collectDirty.fetch_add(1, std::memory_order_relaxed); + return fresh || singlePass || runs.stamped == 0 || walk(true); } #endif bool walkWrites(WriteTracker& tracker, std::uint64_t first, std::uint64_t stop, StampKind kind, bool armedOnly = false, const WindowCopy* window = nullptr) { ++tracker.generation; + StampRuns runs{tracker, kind, 0, window}; #ifdef _WIN32 - static_cast(armedOnly); - constexpr std::uint64_t page = 4096; - // One resetting walk: the kernel reports and clears a page's dirty bit together, a write landing - // after the walk passed a page is reported by the next walk, and a clean page is not touched by the - // reset, so a probe pass first (which doubled the walk of every dirty range) buys nothing. - // APS5_NO_SINGLE_PASS_COLLECT=1 restores the probe pass followed by a resetting pass when dirty. - static const bool singlePass = std::getenv("APS5_NO_SINGLE_PASS_COLLECT") == nullptr; - auto cursor = first; - bool dirty = false; - while (cursor < stop) { - std::size_t count = tracker.pages.size(); - DWORD granularity = 4096; - if (!GuestArena::GuestArenaCollectWrites_nid_postfix(cursor, static_cast(stop - cursor), tracker.pages.data(), &count, singlePass)) { - // Uncommitted pages inside the range make the call fail; such ranges are compared instead. - return false; - } - if (count != 0) dirty = true; - for (ULONG_PTR i = 0; i < count; ++i) { - const auto page = reinterpret_cast(tracker.pages[i]); - tracker.stamp(tracker.blockOf(page), tracker.generation, window != nullptr && window->Holds(page) ? StampKind::ImportWindow : kind); - if (kind != StampKind::Driver) tracker.noteCpuStore(page, page + WritePageBytes, tracker.generation); - } - if (count < tracker.pages.size()) break; - cursor = reinterpret_cast(tracker.pages[count - 1]) + (granularity != 0 ? granularity : page); - } - if (dirty) collectDirty.fetch_add(1, std::memory_order_relaxed); - if (dirty && !singlePass) { - // The resetting pass reports pages again, so a write racing the first pass is stamped too. - cursor = first; - while (cursor < stop) { - std::size_t count = tracker.pages.size(); - DWORD granularity = 4096; - if (!GuestArena::GuestArenaCollectWrites_nid_postfix(cursor, static_cast(stop - cursor), tracker.pages.data(), &count, true)) return false; - for (ULONG_PTR i = 0; i < count; ++i) { - const auto page = reinterpret_cast(tracker.pages[i]); - tracker.stamp(tracker.blockOf(page), tracker.generation, window != nullptr && window->Holds(page) ? StampKind::ImportWindow : kind); - if (kind != StampKind::Driver) tracker.noteCpuStore(page, page + WritePageBytes, tracker.generation); - } - if (count < tracker.pages.size()) break; - cursor = reinterpret_cast(tracker.pages[count - 1]) + (granularity != 0 ? granularity : page); - } - } - return true; + return collectWindowsWrites(runs, first, stop, armedOnly); #else const auto dirtyRuns = collectDirtyRuns.load(std::memory_order_relaxed); - StampRuns runs{tracker, kind, 0, window}; const auto collect = armedOnly ? &GuestWriteWatch::GuestWriteWatchCollectArmed_nid_postfix : &GuestWriteWatch::GuestWriteWatchCollect_nid_postfix; const bool complete = collect(static_cast(first), static_cast(stop - first), &stampWrittenRun, &runs); if (collectDirtyRuns.load(std::memory_order_relaxed) != dirtyRuns) collectDirty.fetch_add(1, std::memory_order_relaxed); @@ -1333,19 +1317,22 @@ std::uint64_t MarkWritten(std::uint64_t address, std::size_t bytes) { tracker.noteUnstampedStore(address, address + bytes); return 0; } -#ifndef _WIN32 { const auto blockFirst = address & ~(std::uint64_t{WriteBlockBytes} - 1); const auto blockStop = (address + bytes + WriteBlockBytes - 1) & ~(std::uint64_t{WriteBlockBytes} - 1); StampRuns runs{tracker, StampKind::Fresh}; - if (!GuestWriteWatch::GuestWriteWatchCollectFresh_nid_postfix(static_cast(blockFirst), static_cast(blockStop - blockFirst), &stampWrittenRun, &runs)) { +#ifdef _WIN32 + const bool complete = collectWindowsWrites(runs, blockFirst, blockStop, false); +#else + const bool complete = GuestWriteWatch::GuestWriteWatchCollectFresh_nid_postfix(static_cast(blockFirst), static_cast(blockStop - blockFirst), &stampWrittenRun, &runs); +#endif + if (!complete) { unwatchLocked(tracker, blockFirst, static_cast(blockStop - blockFirst)); tracker.noteUnstampedStore(address, address + bytes); return 0; } if (runs.stamped != 0) ++tracker.generation; } -#endif const auto first = tracker.blockOf(address); const auto last = tracker.blockOf(address + bytes - 1); for (auto block = first; block <= last; ++block) { diff --git a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp index 29dafa7323..f839edd79d 100644 --- a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp @@ -184,6 +184,65 @@ void CheckNewestStamp() { } #ifdef _WIN32 +void CheckDeferredSharedImports() { + constexpr std::size_t bytes = 4 * Block; + HANDLE section = CreateFileMappingW(INVALID_HANDLE_VALUE, nullptr, PAGE_EXECUTE_READWRITE | SEC_RESERVE, 0, bytes, nullptr); + Require(section != nullptr, "cannot create shared import backing"); + auto* memory = static_cast(GuestArena::GuestArenaAllocate_nid_postfix(bytes, Block)); + Require(memory != nullptr, "cannot allocate shared import address"); + GuestArena::GuestArenaMap_nid_postfix(memory, bytes, section, 0, PAGE_READWRITE); + const auto base = reinterpret_cast(memory); + const auto protection = [&](std::size_t offset) { + MEMORY_BASIC_INFORMATION info{}; + Require(VirtualQuery(memory + offset, &info, sizeof(info)) == sizeof(info), "cannot query shared import protection"); + return info.Protect; + }; + Require(ImportWatched(base, bytes, [] { return true; }), "fresh shared import failed"); + Require(protection(0) == PAGE_READWRITE && protection(bytes - 1) == PAGE_READWRITE, "fresh import armed shared pages"); + const auto gpu = MarkWritten(base + 4096, 4); + Require(gpu != 0 && protection(0) == PAGE_READONLY && protection(Block) == PAGE_READWRITE, "GPU write did not activate only its block"); + CollectWritesUncached(base, Block); + Require(!WrittenSince(base + 4096, 4, gpu) && !StoredOver(base + 4096, 4, gpu), "first collect overwrote a GPU generation"); + static_cast(memory)[4096] = 0x44; + CollectWritesUncached(base, Block); + Require(WrittenSince(base + 4096, 4, gpu) && StoredOver(base + 4096, 4, gpu), "CPU store after GPU write was missed"); + + const auto second = base + Block; + const auto before = CollectWritesUncached(second, 4096); + Require(MarkWritten(second, 4) != 0, "GPU write next to fresh pages failed"); + Require(UnchangedSinceCollected(second, 4, before), "fresh neighbors were stamped as new CPU writes over a collected page"); + Require(StoredOver(second + 8 * 4096, 4096, before), "fresh neighbor history was lost"); + const std::array generations{before}; + std::array changed{}; + std::array cpu{}; + Require(ChangedBlocks(second, Block, generations, changed, cpu) && changed[0] == BlockWritten && cpu[0] == 0, "fresh collection set the block's CPU stamp"); + const auto generation = TrackerGeneration(); + Require(MarkWritten(second, 4) == generation + 1, "an already collected GPU block advanced twice"); + + const auto third = base + 2 * Block; + const auto cached = CollectWritesUncached(third, Block); + Require(ImportWatched(third, Block, [&] { + static_cast(memory)[2 * Block] = 0x55; + return true; + }), "tracked import window failed"); + Require(!UnchangedSince(third, Block, cached) && WrittenSince(third, Block, cached), "a store of new bytes within a tracked import window was missed"); + const auto afterImport = CollectWritesUncached(third, Block); + static_cast(memory)[2 * Block] = 0x66; + Require(ImportWatched(third, Block, [] { return true; }) && WrittenSince(third, Block, afterImport), "pre-import CPU store was not collected"); + + GuestArena::GuestArenaReset_nid_postfix(memory + 3 * Block, 16384); + const auto edge = base + 3 * Block + 16384; + Require(ImportWatched(edge, Block - 16384, [] { return true; }), "unaligned shared import failed"); + const auto edgeGpu = MarkWritten(edge, 4); + Require(edgeGpu != 0, "fresh collection rejected an unmapped block neighbor"); + CollectWritesUncached(edge, Block - 16384); + Require(!WrittenSince(edge, 4, edgeGpu), "edge block collection overwrote GPU results"); + GuestArena::GuestArenaReset_nid_postfix(memory, bytes); + GuestArena::GuestArenaRelease_nid_postfix(memory, bytes); + CloseHandle(section); +} + + void CheckPrivateMappingReuse() { void* memory = AllocateWatched(3 * Block); const auto base = reinterpret_cast(memory); @@ -520,6 +579,7 @@ int main() { CheckUnwatch(); CheckNewestStamp(); #ifdef _WIN32 + CheckDeferredSharedImports(); CheckPrivateMappingReuse(); CheckHostHoleInChild(); #endif diff --git a/core/libs/prx/libc/include/GuestArena.hpp b/core/libs/prx/libc/include/GuestArena.hpp index b42cd1951e..5029a7717a 100644 --- a/core/libs/prx/libc/include/GuestArena.hpp +++ b/core/libs/prx/libc/include/GuestArena.hpp @@ -34,6 +34,8 @@ void GuestArenaPinWritable_nid_postfix(const void* pointer, std::size_t bytes); void GuestArenaUnpinWritable_nid_postfix(const void* pointer, std::size_t bytes); bool GuestArenaProtection_nid_postfix(std::uintptr_t address, std::uint32_t* protection); bool GuestArenaCollectWrites_nid_postfix(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear); +bool GuestArenaCollectArmedWrites_nid_postfix(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear); +bool GuestArenaCollectFreshWrites_nid_postfix(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear); bool GuestArenaHostRegionOverlaps_nid_postfix(std::uintptr_t address, std::size_t bytes); std::uint64_t GuestArenaCommitGeneration_nid_postfix(); void GuestArenaSetPrivateMappingObserver_nid_postfix(void (*callback)(std::uintptr_t address, std::size_t bytes, std::uint64_t generation)); diff --git a/core/libs/prx/libc/include/WindowsMappings.hpp b/core/libs/prx/libc/include/WindowsMappings.hpp index e73cf34e18..2e3691659f 100644 --- a/core/libs/prx/libc/include/WindowsMappings.hpp +++ b/core/libs/prx/libc/include/WindowsMappings.hpp @@ -20,6 +20,8 @@ namespace GuestArena { class WindowsMappings { public: + enum class Collection { All, Armed, Fresh }; + static WindowsMappings& Get() { static WindowsMappings mappings; return mappings; @@ -220,12 +222,19 @@ public: return true; } - bool Collect(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear) { + bool Collect(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear, Collection collection = Collection::All) { std::lock_guard lock(mutex); const auto capacity = *count; *count = 0; const auto end = address + bytes; - if (clear && !collectable(address, end)) return false; + if (collection == Collection::Fresh) { + for (auto it = views.lower_bound(address & ~(pageBytes - 1)); it != views.end() && it->first < end; ++it) { + if (it->second.protection != PAGE_NOACCESS) continue; + for (auto at = std::max(address, it->first); at < std::min(end, it->first + pageBytes); at += 4096) { + if ((it->second.collected & (1u << ((at - it->first) / 4096))) == 0) return false; + } + } + } else if (clear && !collectable(address, end)) return false; for (auto cursor = address; cursor < end;) { const auto nextClean = cleanRanges.upper_bound(cursor); if (nextClean != cleanRanges.begin()) { @@ -239,7 +248,21 @@ public: const auto found = views.find(base); if (found != views.end()) { auto& view = found->second; - const auto stop = std::min(std::min(end, base + pageBytes), nextClean == cleanRanges.end() ? end : nextClean->first); + auto stop = std::min(std::min(end, base + pageBytes), nextClean == cleanRanges.end() ? end : nextClean->first); + if (collection != Collection::All) { + const auto selected = [&](std::uintptr_t at) { + const bool collected = (view.collected & (1u << ((at - base) / 4096))) != 0; + return collection == Collection::Armed ? collected : !collected; + }; + while (cursor < stop && !selected(cursor)) cursor = std::min(stop, cursor + 4096); + if (cursor == stop) continue; + for (auto at = cursor + 4096; at < stop; at += 4096) { + if (!selected(at)) { + stop = at; + break; + } + } + } if (view.protection == PAGE_NOACCESS) return false; const bool pinned = view.page->pins != 0; std::uint8_t settled = 0; @@ -249,7 +272,7 @@ public: const auto from = std::max(cursor, base + sub * subpageBytes); const auto to = std::min(stop, base + (sub + 1) * subpageBytes); if (from >= to) continue; - if (pinned || view.seen[sub] != view.page->generation[sub]) { + if (collection == Collection::Fresh || pinned || view.seen[sub] != view.page->generation[sub]) { if (*count == capacity) { full = true; break; @@ -259,6 +282,7 @@ public: settled |= static_cast(1u << sub); settledEnd = to; } + if (clear) view.collected |= settled; if (clear && !pinned && settled != 0) { std::uint8_t untracked = 0; for (const auto alias : view.page->aliases) { @@ -292,6 +316,11 @@ public: if (full) return true; cursor = stop; } else { + if (collection == Collection::Fresh) { + const auto next = views.lower_bound(cursor); + cursor = next == views.end() ? end : std::min(end, next->first); + continue; + } const auto memory = query(cursor); if (memory.State != MEM_COMMIT || memory.Type != MEM_PRIVATE) return false; const auto stop = std::min(end, reinterpret_cast(memory.BaseAddress) + memory.RegionSize); @@ -352,6 +381,7 @@ private: std::shared_ptr
section; std::uint64_t offset; std::uint32_t hostWrites; + std::uint8_t collected = 0; }; void forgetClean(std::uintptr_t start, std::uintptr_t end) { auto it = cleanRanges.lower_bound(start); diff --git a/core/libs/prx/libc/src/GuestArena.cpp b/core/libs/prx/libc/src/GuestArena.cpp index 025f4faaa9..5ff89ebc9c 100644 --- a/core/libs/prx/libc/src/GuestArena.cpp +++ b/core/libs/prx/libc/src/GuestArena.cpp @@ -338,6 +338,14 @@ bool GuestArenaCollectWrites_nid_postfix(std::uintptr_t address, std::size_t byt return WindowsMappings::Get().Collect(address, bytes, pages, count, clear); } +bool GuestArenaCollectArmedWrites_nid_postfix(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear) { + return WindowsMappings::Get().Collect(address, bytes, pages, count, clear, WindowsMappings::Collection::Armed); +} + +bool GuestArenaCollectFreshWrites_nid_postfix(std::uintptr_t address, std::size_t bytes, void** pages, std::size_t* count, bool clear) { + return WindowsMappings::Get().Collect(address, bytes, pages, count, clear, WindowsMappings::Collection::Fresh); +} + bool GuestArenaHostRegionOverlaps_nid_postfix(std::uintptr_t address, std::size_t bytes) { return Arena::Get().OverlapsHostRegion(address, bytes); } diff --git a/core/libs/tests/WindowsWriteWatch.cpp b/core/libs/tests/WindowsWriteWatch.cpp new file mode 100644 index 0000000000..08b6d4ea12 --- /dev/null +++ b/core/libs/tests/WindowsWriteWatch.cpp @@ -0,0 +1,208 @@ +#define NOMINMAX +#include "prx/libc/include/WindowsMappings.hpp" +#include +#include +#include +#include +#include + +namespace { +using Mappings = GuestArena::WindowsMappings; +using Collection = Mappings::Collection; +constexpr std::size_t Small = 4096; +constexpr std::size_t Bytes = 65536; +std::atomic faults{0}; + +void Require(bool condition, const char* message) { + if (!condition) throw std::runtime_error(message); +} + +LONG CALLBACK HandleWrite(EXCEPTION_POINTERS* exception) { + const auto* record = exception->ExceptionRecord; + if (record->ExceptionCode != EXCEPTION_ACCESS_VIOLATION || record->NumberParameters < 2 || record->ExceptionInformation[0] != 1) return EXCEPTION_CONTINUE_SEARCH; + if (!Mappings::Get().HandleWrite(record->ExceptionInformation[1])) return EXCEPTION_CONTINUE_SEARCH; + ++faults; + return EXCEPTION_CONTINUE_EXECUTION; +} + +struct Region { + HANDLE section = CreateFileMappingW(INVALID_HANDLE_VALUE, nullptr, PAGE_EXECUTE_READWRITE | SEC_RESERVE, 0, Bytes, nullptr); + unsigned char* first = static_cast(Mappings::Get().Reserve(nullptr, Bytes)); + unsigned char* alias = static_cast(Mappings::Get().Reserve(nullptr, Bytes)); + + Region() { + Require(section && first && alias, "cannot allocate shared test memory"); + Mappings::Get().Map(first, Bytes, section, 0, PAGE_READWRITE); + Mappings::Get().Map(alias, Bytes, section, 0, PAGE_READWRITE); + } + ~Region() { + Mappings::Get().Reset(first, Bytes); + Mappings::Get().Reset(alias, Bytes); + VirtualFree(first, 0, MEM_RELEASE); + VirtualFree(alias, 0, MEM_RELEASE); + CloseHandle(section); + } + void Write(std::size_t page, unsigned char value) { + static_cast(first)[page * Small] = value; + } + std::set Collect(Collection mode, std::size_t begin = 0, std::size_t count = 16, bool clear = true, bool useAlias = false, std::size_t capacity = 32) { + auto* base = useAlias ? alias : first; + const auto address = reinterpret_cast(base); + auto cursor = address + begin * Small; + const auto end = cursor + count * Small; + std::set result; + while (cursor < end) { + std::array pages{}; + auto available = capacity; + Require(Mappings::Get().Collect(cursor, end - cursor, pages.data(), &available, clear, mode), "shared collection failed"); + for (std::size_t i = 0; i < available; ++i) result.insert((reinterpret_cast(pages[i]) - address) / Small); + if (available < capacity) break; + cursor = reinterpret_cast(pages[available - 1]) + Small; + } + return result; + } + DWORD Protection(std::size_t page, bool useAlias = false) { + MEMORY_BASIC_INFORMATION info{}; + Require(VirtualQuery((useAlias ? alias : first) + page * Small, &info, sizeof(info)) == sizeof(info), "protection query failed"); + return info.Protect; + } +}; + +std::set Pages(std::size_t begin, std::size_t end) { + std::set result; + for (auto page = begin; page < end; ++page) result.insert(page); + return result; +} + +void CheckArmed() { + Region region; + Require(region.Collect(Collection::Armed).empty(), "import collected fresh memory"); + const auto before = faults.load(); + region.Write(0, 1); + region.Write(4, 2); + Require(faults == before, "import armed fresh memory"); + Require(region.Collect(Collection::All, 0, 1) == Pages(0, 1), "first partial collect failed"); + Require(region.Protection(0) == PAGE_READONLY && region.Protection(3) == PAGE_READWRITE, "partial collect did not protect only its page"); + region.Write(0, 3); + region.Write(4, 4); + Require(region.Collect(Collection::Armed) == Pages(0, 1), "armed collect lost a write or consumed fresh pages"); + Require(region.Protection(4) == PAGE_READWRITE, "armed collect protected another fresh view"); + Require(region.Collect(Collection::All) == Pages(1, 16), "ordinary collect lost fresh pages"); + Require(region.Collect(Collection::All).empty(), "cleared memory reports writes"); +} + +void CheckFresh() { + Region region; + Require(region.Collect(Collection::All, 0, 1) == Pages(0, 1), "cannot collect initial page"); + region.Write(0, 7); + Require(region.Collect(Collection::Fresh) == Pages(1, 16), "fresh collect consumed an existing dirty page"); + Require(region.Collect(Collection::Fresh).empty(), "fresh collect repeated initial writes"); + Require(region.Collect(Collection::Armed) == Pages(0, 1), "fresh collect cleared an existing write"); + region.Write(9, 9); + Require(region.Collect(Collection::Armed) == Pages(9, 10), "write after activation was missed"); +} + +void CheckAliasesAndCapacity() { + Region region; + Require(region.Collect(Collection::All, 0, 1) == Pages(0, 1), "initial collect failed"); + Require(region.Protection(0, true) == PAGE_READONLY, "alias was not protected"); + Require(region.Collect(Collection::Armed, 0, 16, true, true).empty(), "alias protection was mistaken for collection history"); + Require(region.Collect(Collection::Fresh, 0, 16, true, true, 1) == Pages(0, 16), "small output buffers lost fresh pages"); + Require(region.Collect(Collection::Fresh, 0, 16, true, true).empty(), "small output buffers did not finish fresh collection"); + static_cast(region.alias)[0] = 8; + Require(region.Collect(Collection::Armed) == Pages(0, 1), "alias write was missed or fresh neighbors were consumed"); + Require(region.Collect(Collection::Fresh) == Pages(1, 16), "alias collection changed another view's history"); + Mappings::Get().Reset(region.first, Bytes); + Mappings::Get().Map(region.first, Bytes, region.section, 0, PAGE_READWRITE); + Require(region.Collect(Collection::Armed).empty(), "remapped address kept old collection history"); + Require(region.Collect(Collection::Fresh) == Pages(0, 16), "remapped memory was not fresh"); +} + +void CheckPinsAndHostWrites() { + Region region; + const auto address = reinterpret_cast(region.first); + Mappings::Get().Pin(address, 16384); + Require(region.Collect(Collection::Fresh) == Pages(0, 16), "pinned fresh collection failed"); + Require(region.Protection(0) == PAGE_READWRITE, "pinned page was protected"); + Require(region.Collect(Collection::Armed) == Pages(0, 4), "pinned writes were trusted as clean"); + Mappings::Get().Unpin(address, 16384); + Require(region.Collect(Collection::Armed) == Pages(0, 4), "unpin was not invalidated"); + Require(region.Collect(Collection::Armed).empty(), "unpin did not restore tracking"); + Require(Mappings::Get().BeginHostWrite(address, 16384), "host write refused"); + region.Write(0, 3); + region.Collect(Collection::Armed); + region.Write(0, 4); + Mappings::Get().EndHostWrite(address, 16384); + Require(region.Collect(Collection::Armed) == Pages(0, 4), "host write completion was missed"); + Require(region.Collect(Collection::Armed).empty(), "completed host write stayed dirty"); +} + +void CheckFailureAndHoles() { + Region region; + const auto address = reinterpret_cast(region.first); + DWORD previous; + Mappings::Get().SetProtection(address + 16384, 16384, PAGE_NOACCESS); + Require(VirtualProtect(region.first + 16384, 16384, PAGE_NOACCESS, &previous), "cannot protect test page"); + std::array pages{}; + auto count = pages.size(); + Require(!Mappings::Get().Collect(address, Bytes, pages.data(), &count, true, Collection::Fresh), "fresh inaccessible page accepted"); + Require(region.Protection(0) == PAGE_READWRITE, "failed collect changed an earlier page"); + Mappings::Get().SetProtection(address + 16384, 16384, PAGE_READWRITE); + Require(VirtualProtect(region.first + 16384, 16384, PAGE_READWRITE, &previous), "cannot restore test page"); + Mappings::Get().Reset(region.first + 16384, 16384); + Require(region.Collect(Collection::Fresh) == std::set({0, 1, 2, 3, 8, 9, 10, 11, 12, 13, 14, 15}), "fresh collection did not tolerate an unmapped block neighbor"); + Require(region.Collect(Collection::Fresh).empty(), "holes caused fresh pages to repeat"); +} + +void CheckPrivate() { + auto& mappings = Mappings::Get(); + auto* memory = static_cast(mappings.Reserve(nullptr, Bytes)); + Require(memory != nullptr, "private reservation failed"); + mappings.Commit(memory, Bytes, PAGE_READWRITE, Bytes, true); + const auto address = reinterpret_cast(memory); + std::array pages{}; + std::size_t count = pages.size(); + mappings.Collect(address, Bytes, pages.data(), &count, true); + static_cast(memory)[Small] = 3; + count = pages.size(); + Require(mappings.Collect(address, Bytes, pages.data(), &count, true, Collection::Fresh) && count == 0, "fresh collect touched private memory"); + count = pages.size(); + Require(mappings.Collect(address, Bytes, pages.data(), &count, true, Collection::Armed) && count == 1 && pages[0] == memory + Small, "private write watch changed"); + mappings.Reset(memory, Bytes); + VirtualFree(memory, 0, MEM_RELEASE); +} + +void CheckConcurrentWrites() { + Region region; + std::atomic start{false}; + std::thread writer([&] { + while (!start.load(std::memory_order_acquire)) std::this_thread::yield(); + for (std::size_t page = 0; page < 16; ++page) region.Write(page, static_cast(page + 1)); + }); + start.store(true, std::memory_order_release); + region.Collect(Collection::Fresh); + writer.join(); + for (std::size_t page = 0; page < 16; ++page) Require(region.first[page * Small] == page + 1, "activation lost a concurrent write"); + region.Collect(Collection::Armed); + Require(region.Collect(Collection::Armed).empty(), "concurrent writes did not settle"); +} +} + +int main() { + auto* handler = AddVectoredExceptionHandler(1, HandleWrite); + try { + Require(handler != nullptr, "cannot install test handler"); + CheckArmed(); + CheckFresh(); + CheckAliasesAndCapacity(); + CheckPinsAndHostWrites(); + CheckFailureAndHoles(); + CheckPrivate(); + CheckConcurrentWrites(); + std::cout << "Windows deferred write-watch tests passed\n"; + } catch (const std::exception& error) { + std::cerr << error.what() << '\n'; + return 1; + } + RemoveVectoredExceptionHandler(handler); +} From b24e1cc46f0663192481a9d4a464f83b17e58242 Mon Sep 17 00:00:00 2001 From: umang Date: Tue, 13 Oct 2026 09:04:09 +0530 Subject: [PATCH 2/2] perf(agc): return from fresh Windows collection when nothing is fresh MarkWritten collects the fresh portions of the written blocks before each GPU stamp. On Windows that walked every shared view of those blocks twice under the mappings mutex even when all of them had been collected before. Keep the never-collected portions as address ranges, as the Linux tracker keeps _fresh, and return after one lookup when none overlaps the request. Co-Authored-By: Claude Opus 5.5 --- .../libs/prx/libc/include/WindowsMappings.hpp | 52 +++++++++++++------ 1 file changed, 37 insertions(+), 15 deletions(-) diff --git a/core/libs/prx/libc/include/WindowsMappings.hpp b/core/libs/prx/libc/include/WindowsMappings.hpp index 2e3691659f..73b71a87d2 100644 --- a/core/libs/prx/libc/include/WindowsMappings.hpp +++ b/core/libs/prx/libc/include/WindowsMappings.hpp @@ -99,6 +99,7 @@ public: const auto base = cursor + done; shared->aliases.push_back(base); views.emplace(base, View{shared, protection, {}, 0, owned, offset + done, 0}); + remember(freshRanges, base, base + pageBytes); invalidate(*shared); } } @@ -228,6 +229,8 @@ public: *count = 0; const auto end = address + bytes; if (collection == Collection::Fresh) { + const auto fresh = freshRanges.upper_bound(address); + if ((fresh == freshRanges.begin() || std::prev(fresh)->second <= address) && (fresh == freshRanges.end() || fresh->first >= end)) return true; for (auto it = views.lower_bound(address & ~(pageBytes - 1)); it != views.end() && it->first < end; ++it) { if (it->second.protection != PAGE_NOACCESS) continue; for (auto at = std::max(address, it->first); at < std::min(end, it->first + pageBytes); at += 4096) { @@ -282,7 +285,14 @@ public: settled |= static_cast(1u << sub); settledEnd = to; } - if (clear) view.collected |= settled; + if (clear) { + for (std::size_t sub = 0; sub < subpages; ++sub) { + const auto bit = static_cast(1u << sub); + if ((settled & bit) == 0 || (view.collected & bit) != 0) continue; + view.collected |= bit; + forget(freshRanges, base + sub * subpageBytes, base + (sub + 1) * subpageBytes); + } + } if (clear && !pinned && settled != 0) { std::uint8_t untracked = 0; for (const auto alias : view.page->aliases) { @@ -383,27 +393,37 @@ private: std::uint32_t hostWrites; std::uint8_t collected = 0; }; - void forgetClean(std::uintptr_t start, std::uintptr_t end) { - auto it = cleanRanges.lower_bound(start); - if (it != cleanRanges.begin() && std::prev(it)->second > start) --it; - while (it != cleanRanges.end() && it->first < end) { + using Ranges = std::map; + + static void forget(Ranges& ranges, std::uintptr_t start, std::uintptr_t end) { + auto it = ranges.lower_bound(start); + if (it != ranges.begin() && std::prev(it)->second > start) --it; + while (it != ranges.end() && it->first < end) { const auto first = it->first; const auto last = it->second; - it = cleanRanges.erase(it); - if (first < start) cleanRanges.emplace(first, start); - if (last > end) it = cleanRanges.emplace(end, last).first; + it = ranges.erase(it); + if (first < start) ranges.emplace(first, start); + if (last > end) it = ranges.emplace(end, last).first; } } - void rememberClean(std::uintptr_t start, std::uintptr_t end) { - auto it = cleanRanges.lower_bound(start); - if (it != cleanRanges.begin() && std::prev(it)->second >= start) --it; - while (it != cleanRanges.end() && it->first <= end) { + static void remember(Ranges& ranges, std::uintptr_t start, std::uintptr_t end) { + auto it = ranges.lower_bound(start); + if (it != ranges.begin() && std::prev(it)->second >= start) --it; + while (it != ranges.end() && it->first <= end) { start = std::min(start, it->first); end = std::max(end, it->second); - it = cleanRanges.erase(it); + it = ranges.erase(it); } - cleanRanges.emplace(start, end); + ranges.emplace(start, end); + } + + void forgetClean(std::uintptr_t start, std::uintptr_t end) { + forget(cleanRanges, start, end); + } + + void rememberClean(std::uintptr_t start, std::uintptr_t end) { + remember(cleanRanges, start, end); } void invalidate(SharedPage& page, std::uint8_t mask = allSubpages) { @@ -505,6 +525,7 @@ private: if (found != views.end()) { std::erase(found->second.page->aliases, cursor); views.erase(found); + forget(freshRanges, cursor, cursor + pageBytes); } } else if (memory.Type == MEM_PRIVATE) { if (!VirtualFree(reinterpret_cast(cursor), allocationEnd - cursor, MEM_RELEASE | MEM_PRESERVE_PLACEHOLDER)) fail("release private guest memory"); @@ -521,7 +542,8 @@ private: if (query(address).RegionSize != bytes && !VirtualFree(reinterpret_cast(address), bytes, MEM_RELEASE | MEM_COALESCE_PLACEHOLDERS)) fail("coalesce guest placeholders"); } - std::map cleanRanges; + Ranges cleanRanges; + Ranges freshRanges; std::map views; std::map, std::weak_ptr> physical; std::mutex mutex;