From 93df5c42a1b3ddd9362d7fc8541f811eac730248 Mon Sep 17 00:00:00 2001 From: LeandroLP23 Date: Thu, 15 Oct 2026 12:43:58 +0200 Subject: [PATCH 1/7] fix(agc): run the unwatch hook when a watched import leaves the write watch An import that should stay watched still drops its range from the watch when the range was only partly watched or its import window cannot be resolved. That fallback skipped the unwatch hook, so a storage image with results pending in the range lost the watch without its guest bytes saved, and its next store threw "storing storage image results pending over memory outside the write watch without the guest bytes saved before them is not implemented". Since #6060 settled private imports on Windows take this path, and ProsperoPuzzles (PPSA99006) stopped 5 s in (#6301). The fallback now releases the tracker lock, runs the hook over the page range, and unwatches it. Co-Authored-By: Claude Opus 5.5 --- .../Execution/src/GuestMemory.cpp | 7 ++++++- .../libSceAgcDriver/tests/WriteTracking.cpp | 21 +++++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp index 8850453b340d..26e7dff7d2dd 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp @@ -1345,7 +1345,12 @@ bool ImportWatched(std::uint64_t address, std::size_t bytes, const std::function return true; } const WindowCopy window{first, saved, slots}; - if (!before || !walkWrites(tracker, first, stop, StampKind::Cpu, true, &window)) unwatchLocked(tracker, first, static_cast(stop - first)); + if (!before || !walkWrites(tracker, first, stop, StampKind::Cpu, true, &window)) { + lock.unlock(); + beforeUnwatch(first, static_cast(stop - first)); + lock.lock(); + unwatchLocked(tracker, first, static_cast(stop - first)); + } return true; } diff --git a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp index 86da33486da4..ee6c53b38827 100644 --- a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp @@ -570,6 +570,26 @@ void CheckImportWindow() { Require(!UnchangedSince(base, Block, after), "a store after the import window was missed"); } +std::uint64_t hookedBegin = 0; +std::uint64_t hookedEnd = 0; + +void CheckImportWindowOverUnwatchedPart() { + void* memory = AllocateWatched(3 * Block); + const auto base = reinterpret_cast(memory); + std::memset(memory, 0x11, 3 * Block); + static_cast(CollectWritesUncached(base, 3 * Block)); + Unwatch(base + Block, Block); + SetUnwatchHook([](std::uint64_t address, std::size_t bytes) { + hookedBegin = address; + hookedEnd = address + bytes; + }); + const bool imported = ImportWatched(base, 3 * Block, [] { return true; }); + SetUnwatchHook(nullptr); + Require(imported, "an import over a partly unwatched range failed"); + Require(!Watched(base, Block) && !Watched(base + 2 * Block, Block), "an import over a partly unwatched range left its watched blocks watched"); + Require(hookedBegin <= base && hookedEnd >= base + 3 * Block, "an import that left the write watch did not run the unwatch hook over its range"); +} + void CheckImportWindowOtherThread() { void* memory = AllocateWatched(2 * Block); const auto base = reinterpret_cast(memory); @@ -848,6 +868,7 @@ int main() { CheckImportWindow(); CheckImportWindowZeroPages(); CheckImportWindowOtherThread(); + CheckImportWindowOverUnwatchedPart(); CheckUnwatchedImport(); CheckCommittedRanges(); } catch (const std::exception& error) { From 0205c3672fed5551239ec4c5138688533966d4c2 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Thu, 15 Oct 2026 22:29:02 +0200 Subject: [PATCH 2/7] test(agc): store results pending when an import over a partly unwatched range drops it A storage image has results pending over a block whose neighbour block already left the write watch. An import of both blocks that should stay watched cannot vouch for its pages, so the range leaves the watch, and the image's next store has to find the guest bytes the unwatch hook saved. agc_storage_unwalked_import (Linux) runs that flow through HostImportFor. The host import of the first creation is refused through the import budget so that the import under test is the first one over the block. staleGenerationTests runs the same flow for an import that does not stay watched; it now exists for both. --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 4 ++ .../prx/libSceAgcDriver/tests/Recorder.cpp | 47 ++++++++++++++----- 2 files changed, 38 insertions(+), 13 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index a6de94a52ff5..81879eb4d437 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -1180,6 +1180,10 @@ if(BUILD_TESTING) set_tests_properties(agc_storage_kept_unit_settle PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() + if(NOT WIN32) + add_test(NAME agc_storage_unwalked_import COMMAND agc_driver_recorder_tests --unwalked-import-only) + set_tests_properties(agc_storage_unwalked_import PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 30) + endif() add_test(NAME agc_storage_baseline_settled COMMAND agc_driver_recorder_tests --settled-baseline-only) add_test(NAME agc_storage_integer_clear COMMAND agc_driver_recorder_tests --integer-clear-only) set_tests_properties(agc_storage_integer_clear PROPERTIES SKIP_RETURN_CODE 77) diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 7c4bd1b701b5..9135f5697441 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -4444,41 +4444,51 @@ void importWatchTests(const Device& device) { #endif } -void staleGenerationTests(const Device& device, Recorder& recorder) { +template +bool staleGenerationTests(const Device& device, Recorder& recorder) { const auto& base = device.GetContext(); #ifdef _WIN32 static_cast(recorder); static_cast(base); std::cout << "a range leaving the watch: Linux write watch only\n"; + return false; #else if (base.hostImportAlignment == 0) { std::cout << "host imports unavailable: a range leaving the watch not tested\n"; - return; + return false; } constexpr std::uint32_t side = 256; constexpr std::size_t surfaceBytes = side * side * 4; constexpr std::size_t keyCount = surfaceBytes / 256; - constexpr std::size_t bytes = surfaceBytes + 65536; + constexpr std::size_t lead = StaysWatched ? 65536 : 0; + constexpr std::size_t bytes = lead + surfaceBytes + 65536; void* block = AllocateWatched(bytes, 65536); if (block == nullptr) { std::cout << "no write watching: a range leaving the watch not tested\n"; - return; + return false; } struct Release { void* block; ~Release() { ReleaseWatched(block, bytes); } } release{block}; const auto decided = PrepareImportWatch(base); - SetImportWatch(base, ImportWatch::Unwatch); + SetImportWatch(base, StaysWatched ? ImportWatch::Watch : ImportWatch::Unwatch); struct Restore { const Context& context; ImportWatch decided; ~Restore() { SetImportWatch(context, decided); } } restore{base, decided}; - auto* texels = static_cast(block); + struct Budget { + bool set; + ~Budget() { + if (set) SetHostImportBudget(0); + } + } budget{StaysWatched}; + auto* texels = static_cast(block) + lead; std::memset(texels, 0x55, surfaceBytes); std::memset(texels + surfaceBytes, 0x00, keyCount); - const auto address = reinterpret_cast(block); + const auto blockAddress = reinterpret_cast(block); + const auto address = blockAddress + lead; { GuestAllocations::Mutation mutation; mutation.Add(block, bytes, true, true, true); @@ -4494,7 +4504,7 @@ void staleGenerationTests(const Device& device, Recorder& recorder) { } HostImportFor(context, address, bytes); } - } unregister{base, block, address}; + } unregister{base, block, blockAddress}; TextureDetiler detiler(base); auto context = base; context.detiler = &detiler; @@ -4512,7 +4522,8 @@ void staleGenerationTests(const Device& device, Recorder& recorder) { resource.dstSelW = 7; resource.dccAddress = address + surfaceBytes; { - Require(AgcDriver::GuestMemory::Watched(address, bytes), "(s) the block is not watched before the storage image takes its generation"); + Require(AgcDriver::GuestMemory::Watched(blockAddress, bytes), "(s) the block is not watched before the storage image takes its generation"); + if constexpr (StaysWatched) SetHostImportBudget(1); auto image = StorageTexture::Create(context, detiler, resource, 0); const VkImageSubresourceRange range{VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1}; const auto commands = recorder.Commands(); @@ -4523,13 +4534,20 @@ void staleGenerationTests(const Device& device, Recorder& recorder) { context.Function("vkCmdClearColorImage")(commands, image->Image(), VK_IMAGE_LAYOUT_GENERAL, &red, 1, &range); RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_TRANSFER_READ_BIT | VK_ACCESS_TRANSFER_WRITE_BIT); image->MarkDirty(); - const auto* import = HostImportFor(base, address, bytes); + if constexpr (StaysWatched) { + SetHostImportBudget(0); + Require(!image->GuestSnapshotValid(), "(s) the pending results had saved guest bytes while their memory was watched"); + AgcDriver::GuestMemory::Unwatch(blockAddress, lead); + Require(!AgcDriver::GuestMemory::Watched(blockAddress, lead) && AgcDriver::GuestMemory::Watched(address, bytes - lead), "(s) the block before the surface did not leave the watch alone"); + } + const auto* import = HostImportFor(base, blockAddress, bytes); if (import == nullptr) { std::cout << "host import of the stale generation block refused: a range leaving the watch not tested\n"; recorder.Sync(); - return; + return false; } - Require(!AgcDriver::GuestMemory::Watched(address, bytes), "(s) the imported range stayed watched"); + Require(!AgcDriver::GuestMemory::Watched(blockAddress, bytes), "(s) the imported range stayed watched"); + if constexpr (StaysWatched) Require(image->GuestSnapshotValid(), "(s) the results pending when a walk gave up the watch saved no guest bytes"); image->Refresh(); Buffer readback(context, surfaceBytes, VK_BUFFER_USAGE_TRANSFER_DST_BIT); const auto copyCommands = recorder.Commands(); @@ -4553,6 +4571,7 @@ void staleGenerationTests(const Device& device, Recorder& recorder) { for (std::size_t i = 0; i < surfaceBytes; ++i) Require(texels[i] == expected[i % 4], "(s) results pending when their memory left the watch did not reach it"); } recorder.Sync(); + return true; #endif } @@ -8801,6 +8820,7 @@ int main(int argc, char** argv) { settledBaselineTests(device, recorder); return 0; } + if (argc == 2 && std::string_view(argv[1]) == "--unwalked-import-only") return staleGenerationTests(device, recorder) ? 0 : 77; if (argc == 2 && std::string_view(argv[1]) == "--completion-labels-only") { completionCountTests(device, recorder); afterRecordedWorkTests(device, recorder); @@ -8997,7 +9017,8 @@ int main(int argc, char** argv) { keptUnitSettleTests(device, recorder); settledBaselineTests(device, recorder); importWatchTests(device); - staleGenerationTests(device, recorder); + staleGenerationTests(device, recorder); + staleGenerationTests(device, recorder); importWindowTests(device, recorder); dataWordPositionsTests(); dataRefreshTests(device, recorder); From c20686e11410a33fe490a9e9d5446945ddabcd90 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Thu, 15 Oct 2026 22:29:04 +0200 Subject: [PATCH 3/7] fix(agc): run the unwatch hook before the import when the page walk already fails An import that should stay watched runs the unwatch hook only when its range leaves the write watch, after the import. When the range is partly outside the watch or the walk before the import fails, the range leaves the watch whatever the import does, so the hook can run first, as it does for the import that does not stay watched. After the import, the walk that tells an import's own page marks from CPU writes (the window compare) is skipped in this case, so the hook collects those marks as CPU writes. A driver that marks the pages of an import written (the NVIDIA driver on Windows does, see Pages under an import window) makes the hook keep the blocks they cover for the CPU unless their bytes are proven unchanged, and those blocks do not receive the GPU results. A failure of the walk after the import still runs the hook after it. The check of #6308 also requires the hook to have run before the import. --- .../prx/libSceAgcDriver/Execution/src/GuestMemory.cpp | 11 ++++++++--- core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp | 7 ++++++- docs/dev/debt/libSceAgcDriver.md | 2 +- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp index 8d7f33e03680..265951de0421 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp @@ -1409,6 +1409,13 @@ bool ImportWatched(std::uint64_t address, std::size_t bytes, const std::function } } const bool before = covered && walkWrites(tracker, first, stop, StampKind::Cpu, true); + const auto saveBytes = [&] { + lock.unlock(); + beforeUnwatch(first, static_cast(stop - first)); + lock.lock(); + }; + const bool savedEarly = staysWatched && !before; + if (savedEarly) saveBytes(); bool imported = false; { #ifdef __APPLE__ @@ -1424,9 +1431,7 @@ bool ImportWatched(std::uint64_t address, std::size_t bytes, const std::function } const WindowCopy window{first, saved, slots}; if (!before || !walkWrites(tracker, first, stop, StampKind::Cpu, true, &window)) { - lock.unlock(); - beforeUnwatch(first, static_cast(stop - first)); - lock.lock(); + if (!savedEarly) saveBytes(); unwatchLocked(tracker, first, static_cast(stop - first)); } return true; diff --git a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp index 63491a8619e5..f7607574bcd9 100644 --- a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp @@ -793,11 +793,16 @@ void CheckImportWindowOverUnwatchedPart() { hookedBegin = address; hookedEnd = address + bytes; }); - const bool imported = ImportWatched(base, 3 * Block, [] { return true; }); + bool hookedBeforeImport = false; + const bool imported = ImportWatched(base, 3 * Block, [&] { + hookedBeforeImport = hookedEnd != 0; + return true; + }); SetUnwatchHook(nullptr); Require(imported, "an import over a partly unwatched range failed"); Require(!Watched(base, Block) && !Watched(base + 2 * Block, Block), "an import over a partly unwatched range left its watched blocks watched"); Require(hookedBegin <= base && hookedEnd >= base + 3 * Block, "an import that left the write watch did not run the unwatch hook over its range"); + Require(hookedBeforeImport, "an import over a partly unwatched range ran the unwatch hook after the import"); } void CheckImportWindowOtherThread() { diff --git a/docs/dev/debt/libSceAgcDriver.md b/docs/dev/debt/libSceAgcDriver.md index 01fbd5d3e528..e340ce1f71d9 100644 --- a/docs/dev/debt/libSceAgcDriver.md +++ b/docs/dev/debt/libSceAgcDriver.md @@ -79,9 +79,9 @@ - On a device without VK_EXT_robustness2's `nullDescriptor`, such as MoltenVK, a null image descriptor in a typed heap is a [placeholder image](../../../core/libs/prx/libSceAgcDriver/Graphics/src/PlaceholderImages.cpp) of that heap's type, 1×1 and cleared to zero, which every null descriptor of the heap shares. Reads give zeros, as `nullDescriptor` allows, but stores and atomics to it are kept, so a later read of a null storage image can return them, and a depth comparison compares against depth 0. A heap whose placeholder the device cannot create, for example 1D or 3D depth, 64-bit atomics without 64-bit image atomics, or multisampled storage without `shaderStorageImageMultisample`, throws when a null descriptor reaches it. - [Draw input snapshots](../../../core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp) (libSceAgcDriver): `PrepareDrawBindings` caps read-only input buffer snapshots at 64 MiB (overridable via `APS5_DRAW_SNAPSHOT_MAX_MIB`). Larger inputs (streaming heaps, bindless pools) or draws exceeding the cumulative budget skip the per-draw snapshot and bind in place, avoiding multi-gigabyte CPU copy stalls (such as Ratchet & Clank: Rift Apart PPSA01474 stalling at frame ~380 copying 36 GB every 5 s). If the title rewrites those large ranges on the CPU before GPU completion, the in-place binding observes the modification, as verified by `drawSnapshotCapTests`. - [Pending storage-image results](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver) are stored when something reads their memory, not when the GPU work that wrote them completes. A title that waits for that work, frees the image and reuses its memory can have the new data overwritten by the late store (#1970). A title's own CPU reads are not seen either, so a title that reads the results after waiting for a label reads stale memory. `APS5_STORE_AT_FLIP=1` stores every pending result at each flip and `APS5_STORE_AT_LABEL=1` before each label the GPU writes; both store every pending image, not only the ones the waited-for work wrote, and neither is on by default. -- [Pending storage-image results over memory outside the write watch](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver) leave to the CPU the 64 KiB blocks whose bytes differ from the ones saved at the upload, the fill, the image copy, the previous store or the moment their range left the watch (`Unwatch`, or a host import that does not stay watched), and the blocks the watch saw the CPU write before that moment. A store made on the GPU saves its bytes through a host-visible copy that becomes the saved bytes when its batch completes; a store or a refresh that needs them earlier waits for that batch. A unit without a write generation saves the guest bytes when results land in it outside the watch. One whose range leaves the watch while its results are pending, and ranges that leave the watch through a failed collect, have no saved bytes; storing such results throws. - Windows host-pointer imports of the guest mapping itself are compared instead of write-watched, except writable imports of private `MEM_WRITE_WATCH` memory when the import probe keeps the write watch and a settle queue exists: those of 1 MiB or more stay watched, and smaller ones return to the watch once their settle has completed (see Pages under an import window): a driver can mark imported pages dirty after the synchronous import window. Shared memory imported through a separate writable alias retains protection-based tracking on the guest views; driver access to the import alias does not disarm those views. [GuestMemory](../../../core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp) excludes every overlapping 64 KiB tracker block while its backing is retained. A full block replaced by fresh private backing regains tracking and gets a new generation; protection changes, partial replacements and shared remappings do not restore it. Excluded blocks keep their write-watch marks as a hint only (`UntrackedHint`): a storage image or a sampled texture snapshot over them compares its bytes again after a mark in its range, a driver store, a mapping change or a recorded GPU write instead of at every collect epoch or lookup, and a mark the driver makes without a CPU store costs one comparison. This preserves CPU edits through byte comparisons but limits snapshot reuse on excluded blocks to draws of one collect epoch and disables unit shadows for them; restoring tracking on import retirement without replacing the backing remains unimplemented. - [Pages under an import window](../../../core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp) (libSceAgcDriver) whose write protection was resolved while a host import that leaves its range watched was made are compared with the bytes they held before the import and count as stored over only when those differ. A CPU store of the bytes already there within the window cannot be told apart from an import that resolves the write protection without storing, as the NVIDIA driver (610.57.04) does for every imported page, so such a store does not win over pending results. Where host-pointer and dma-buf imports differ in whether they stay watched, a failed host-pointer import and the dma-buf import after it are two windows, and the pages the failed one resolved count as stored over. On Windows, the NVIDIA driver (617.42) marks every page of a host import of private `MEM_WRITE_WATCH` memory written, asynchronously, until a GPU use of the import completes; a 4-byte read of it on the settle queue ends that even while main-queue work that used the import first is still running, and an earlier import of the same pages whose first use runs after that read marks none of them (`agc_import_settle_first_use`, `agc_import_settle_earlier_import`). Such an import stays watched only after a 4-byte read of it on the settle queue has completed inside the window. Each one waits for that read, which keeps its window open for one round trip on the settle queue, so a same-value CPU store from another thread in that time counts as unchanged too. An import made where no settle queue exists is compared instead. An import of under 1 MiB is not settled inside its window, because the settle blocks the importing thread and a title that imports a 64 KiB buffer around every submission lost about 19 % of its frame rate to that wait (Dripl1x, #6060). It is compared until its read, submitted in a batch by a later import lookup under `GpuMutex` while the imports match the registry, has completed; a later lookup then returns its whole blocks to the write watch, each stamped as a CPU store, so whatever was derived from them while they were compared reads as changed once. Its blocks stay compared while storage results are pending over them, and those of an import retired before that stay compared. +- [Pending storage-image results over memory outside the write watch](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver) leave to the CPU the 64 KiB blocks whose bytes differ from the ones saved at the upload, the fill, the image copy, the previous store or the moment their range left the watch (`Unwatch`, or a host import that does not stay watched or whose page walk fails), and the blocks the watch saw the CPU write before that moment. A store made on the GPU saves its bytes through a host-visible copy that becomes the saved bytes when its batch completes; a store or a refresh that needs them earlier waits for that batch. A unit without a write generation saves the guest bytes when results land in it outside the watch. One whose range leaves the watch while its results are pending, and ranges that leave the watch through the failed collect of a driver store or, on Linux, because their direct memory is mapped a second time, have no saved bytes; storing such results throws. - [Settled dma-buf imports](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp) (libSceAgcDriver): on Linux a dma-buf import of 1 MiB or more is read by a 4-byte copy on the settle queue right after it is made (`settleImport`), so the kernel maps its pages for the GPU there and not in the first submission that uses it (about 2 s for a 3 GiB import on RADV). The import's buffer is shared with the settle queue's family. An import made at map time settles on the guest thread that maps the range, inside its map call, so that thread waits while the kernel maps the pages: 0.47 to 2.2 s for the 3 GiB range of Hades (PPSA03355) and 0.49 to 4.3 s for the 4.5 GiB range of ASTRO BOT (PPSA21567) on RADV. Both titles map these ranges at start-up; a title that maps large ranges during gameplay stalls its mapping thread for as long. An import made from a submission settles under the GPU mutex, so its frame still pays for it; a smaller import, and every import on a device with no settle queue (one without a copy-capable queue besides the main one), pays in its first submission. A range imported through a host pointer, as lavapipe imports shared memory, is not settled - [Sparse dma-buf imports](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp) (libSceAgcDriver): on Linux a shared-backed range larger than one 64 MiB udmabuf buffer is imported as one sparse buffer bound to the 64 MiB buffers, on a queue of its own when the device has a second family that binds sparse memory. The path is decided before anything is imported, and a failure after that throws. A range takes the single udmabuf import instead, which udmabuf's `size_limit_mb` refuses above 64 MiB by default so the range is copied, when its offset in the backing file is not a multiple of the buffer's sparse block size (64 KiB on RADV), when the sparse buffer does not accept every memory type a chunk import can have, when the backing file ends inside its last chunk, or when it is imported at map time on a device with no second family (the bind then needs the GPU mutex); the NVIDIA proprietary driver imports no dma-buf memory - [Unnormalized S# coordinates](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestSamplerResource.cpp) (libSceAgcDriver) - `FORCE_UNNORMALIZED` uses a Vulkan `unnormalizedCoordinates` sampler, so the S# LOD range, LOD bias and mip filter are not applied, which is exact only on a one-level view. From aaf19708ff31c4d28dea3bfaf0cc04dfbb62e110 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Sun, 18 Oct 2026 10:47:31 +0200 Subject: [PATCH 4/7] test(agc): cover the unwatch hook on each exit that drops a range from the write watch CheckUnwatchHooks runs Unwatch, an import that does not stay watched, an import whose page walk vouches for its pages and, on Linux, an import whose range loses a block during the import. The hook has to run once, over the dropped range, while the range is still watched: before the import for the first two, after it for the last, and not at all for the vouched import. --- .../libSceAgcDriver/tests/WriteTracking.cpp | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp index f7607574bcd9..7f8fc018eedf 100644 --- a/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/WriteTracking.cpp @@ -938,6 +938,80 @@ void CheckUnwatchedImport() { #endif } +struct HookCall { + std::uint64_t address; + std::size_t bytes; + bool watched; +}; + +std::vector hookCalls; +std::uint64_t hookProbe = 0; +std::size_t hookProbeBytes = 0; + +void RecordHook(std::uint64_t address, std::size_t bytes) { + hookCalls.push_back({address, bytes, Watched(hookProbe, hookProbeBytes)}); +} + +struct RecordedHook { + RecordedHook() { + hookCalls.clear(); + SetUnwatchHook(&RecordHook); + } + ~RecordedHook() { SetUnwatchHook(nullptr); } + RecordedHook(const RecordedHook&) = delete; + RecordedHook& operator=(const RecordedHook&) = delete; +}; + +void RequireHookOver(std::uint64_t address, std::size_t bytes, const std::string& path) { + Require(hookCalls.size() == 1, path + " did not run the unwatch hook once"); + const auto& call = hookCalls.front(); + Require(call.address <= address && address + bytes <= call.address + call.bytes, path + " gave the unwatch hook a range that misses its own"); + Require(call.watched, path + " ran the unwatch hook after its range left the write watch"); +} + +void CheckUnwatchHooks() { + void* memory = AllocateWatched(8 * Block); + const auto base = reinterpret_cast(memory); + std::memset(memory, 0x11, 8 * Block); + Require(CollectWritesUncached(base, 8 * Block) != 0, "the hooked block is not collected"); + const RecordedHook recorded; + hookProbe = base; + hookProbeBytes = Block; + Unwatch(base, Block); + RequireHookOver(base, Block, "Unwatch"); + Require(!Watched(base, Block), "Unwatch left its range watched"); + + hookCalls.clear(); + hookProbe = base + Block; + bool hookedBeforeImport = false; + Require(ImportWatched(base + Block, Block, [&] { + hookedBeforeImport = hookCalls.size() == 1; + return true; + }, false), "an import that does not stay watched failed"); + RequireHookOver(base + Block, Block, "an import that does not stay watched"); + Require(hookedBeforeImport, "an import that does not stay watched ran the unwatch hook after the import"); + Require(!Watched(base + Block, Block), "an import that does not stay watched left its range watched"); + + hookCalls.clear(); + Require(ImportWatched(base + 2 * Block, Block, [] { return true; }), "an import that stays watched failed"); + Require(hookCalls.empty() && Watched(base + 2 * Block, Block), "an import whose page walk vouched for its pages ran the unwatch hook or gave up its range"); + +#ifndef _WIN32 + hookCalls.clear(); + const auto late = base + 4 * Block; + hookProbe = late; + bool hookedBeforeLoss = false; + Require(ImportWatched(late, 3 * Block, [&] { + GuestWriteWatch::GuestWriteWatchUnregister_nid_postfix(reinterpret_cast(late + 2 * Block), Block); + hookedBeforeLoss = !hookCalls.empty(); + return true; + }), "an import whose range lost a block during the import failed"); + RequireHookOver(late, 3 * Block, "an import whose page walk failed after the import"); + Require(!hookedBeforeLoss, "an import whose page walk failed after the import ran the unwatch hook before it"); + Require(!Watched(late, Block), "an import whose page walk failed after the import left its range watched"); +#endif +} + class PartlyCommitted { public: static constexpr std::size_t Bytes = 4 * Block; @@ -1111,6 +1185,7 @@ int main() { CheckImportWindowOverUnwatchedPart(); CheckStoreAcrossImports(); CheckUnwatchedImport(); + CheckUnwatchHooks(); CheckCommittedRanges(); } catch (const std::exception& error) { std::cerr << "write tracking test failed: " << error.what() << "\n"; From 05bd606cf78b3a013a923ce7e06b397d3bbe8d26 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:17:12 +0200 Subject: [PATCH 5/7] docs(agc): drop the duplicate pending-results debt entry left by the merge --- docs/dev/debt/libSceAgcDriver.md | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/dev/debt/libSceAgcDriver.md b/docs/dev/debt/libSceAgcDriver.md index b195d88cd251..6c8d150f312a 100644 --- a/docs/dev/debt/libSceAgcDriver.md +++ b/docs/dev/debt/libSceAgcDriver.md @@ -109,7 +109,6 @@ - [Pending storage-image results over memory outside the write watch](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver) leave to the CPU the 64 KiB blocks whose bytes differ from the ones saved at the upload, the fill, the image copy, the previous store or the moment their range left the watch (`Unwatch`, or a host import that does not stay watched or whose page walk fails), and the blocks the watch saw the CPU write before that moment. A store made on the GPU saves its bytes through a host-visible copy that becomes the saved bytes when its batch completes; a store or a refresh that needs them earlier waits for that batch. A unit without a write generation saves the guest bytes when results land in it outside the watch. One whose range leaves the watch while its results are pending saves its bytes through the unwatch hook, which also runs when a failed collect takes a range out of the watch. A range that leaves the watch without the hook running over it leaves its pending results without saved bytes; storing such results throws. A unit whose results were stored while its blocks were watched keeps only a generation; when its blocks leave the watch while nothing is pending, the hook saves nothing, and results that land in it afterwards have neither saved bytes nor a watch, so storing them throws too. - Windows host-pointer imports of the guest mapping itself are compared instead of write-watched, except writable imports of private `MEM_WRITE_WATCH` memory when the import probe keeps the write watch and a settle queue exists: those of 1 MiB or more stay watched, and smaller ones return to the watch once their settle has completed, unless a storage image overlaps them (see Pages under an import window): a driver can mark imported pages dirty after the synchronous import window. Shared memory imported through a separate writable alias retains protection-based tracking on the guest views; driver access to the import alias does not disarm those views. [GuestMemory](../../../core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp) excludes every overlapping 64 KiB tracker block while its backing is retained. A full block replaced by fresh private backing regains tracking and gets a new generation; protection changes, partial replacements and shared remappings do not restore it. Excluded blocks keep their write-watch marks as a hint only (`UntrackedHint`): a storage image or a sampled texture snapshot over them compares its bytes again after a mark in its range, a driver store, a mapping change or a recorded GPU write instead of at every collect epoch or lookup, and a mark the driver makes without a CPU store costs one comparison. This preserves CPU edits through byte comparisons but limits snapshot reuse on excluded blocks to draws of one collect epoch and disables unit shadows for them; restoring tracking on import retirement without replacing the backing remains unimplemented. - [Pages under an import window](../../../core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp) (libSceAgcDriver) whose write protection was resolved while a host import that leaves its range watched was made are compared with the bytes they held before the import and count as stored over only when those differ. A CPU store of the bytes already there within the window cannot be told apart from an import that resolves the write protection without storing, as the NVIDIA driver (610.57.04) does for every imported page, so such a store does not win over pending results. Where host-pointer and dma-buf imports differ in whether they stay watched, a failed host-pointer import and the dma-buf import after it are two windows, and the pages the failed one resolved count as stored over. On Windows, the NVIDIA driver (617.42) marks every page of a host import of private `MEM_WRITE_WATCH` memory written, asynchronously, until a GPU use of the import completes; a 4-byte read of it on the settle queue ends that even while main-queue work that used the import first is still running, and an earlier import of the same pages whose first use runs after that read marks none of them (`agc_import_settle_first_use`, `agc_import_settle_earlier_import`). Such an import stays watched only after a 4-byte read of it on the settle queue has completed inside the window. Each one waits for that read, which keeps its window open for one round trip on the settle queue, so a same-value CPU store from another thread in that time counts as unchanged too. An import made where no settle queue exists is compared instead. An import of under 1 MiB is not settled inside its window, because the settle blocks the importing thread and a title that imports a 64 KiB buffer around every submission lost about 19 % of its frame rate to that wait (Dripl1x, #6060). It is compared until its read, submitted in a batch by a later import lookup under `GpuMutex` while the imports match the registry, has completed; a later lookup then returns its whole blocks to the write watch, each stamped as a CPU store, so whatever was derived from them while they were compared reads as changed once. Its blocks stay compared while storage results are pending over them, and those of an import retired before that stay compared. -- [Pending storage-image results over memory outside the write watch](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver) leave to the CPU the 64 KiB blocks whose bytes differ from the ones saved at the upload, the fill, the image copy, the previous store or the moment their range left the watch (`Unwatch`, or a host import that does not stay watched or whose page walk fails), and the blocks the watch saw the CPU write before that moment. A store made on the GPU saves its bytes through a host-visible copy that becomes the saved bytes when its batch completes; a store or a refresh that needs them earlier waits for that batch. A unit without a write generation saves the guest bytes when results land in it outside the watch. One whose range leaves the watch while its results are pending, and ranges that leave the watch through the failed collect of a driver store or, on Linux, because their direct memory is mapped a second time, have no saved bytes; storing such results throws. - [Settled dma-buf imports](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp) (libSceAgcDriver): on Linux a dma-buf import of 1 MiB or more is read by a 4-byte copy on the settle queue right after it is made (`settleImport`), so the kernel maps its pages for the GPU there and not in the first submission that uses it (about 2 s for a 3 GiB import on RADV). The import's buffer is shared with the settle queue's family. An import made at map time settles on the guest thread that maps the range, inside its map call, so that thread waits while the kernel maps the pages: 0.47 to 2.2 s for the 3 GiB range of Hades (PPSA03355) and 0.49 to 4.3 s for the 4.5 GiB range of ASTRO BOT (PPSA21567) on RADV. Both titles map these ranges at start-up; a title that maps large ranges during gameplay stalls its mapping thread for as long. An import made from a submission settles under the GPU mutex, so its frame still pays for it; a smaller import, and every import on a device with no settle queue (one without a copy-capable queue besides the main one), pays in its first submission. A range imported through a host pointer, as lavapipe imports shared memory, is not settled - [Sparse dma-buf imports](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp) (libSceAgcDriver): on Linux a shared-backed range larger than one 64 MiB udmabuf buffer is imported as one sparse buffer bound to the 64 MiB buffers, on a queue of its own when the device has a second family that binds sparse memory. The path is decided before anything is imported, and a failure after that throws. A range whose offset in the backing file is not a multiple of 64 KiB starts its sparse buffer at the 64 KiB block below it (`HostImport::lead`). A range takes the single udmabuf import instead, which udmabuf's `size_limit_mb` refuses above 64 MiB by default so the range is copied, when the buffer's sparse block size is larger than 64 KiB (it is 64 KiB on RADV), when the sparse buffer does not accept every memory type a chunk import can have, when the backing file ends inside its last chunk, or when it is imported at map time on a device with no second family (the bind then needs the GPU mutex); the NVIDIA proprietary driver imports no dma-buf memory - [Unnormalized S# coordinates](../../../core/libs/prx/libSceAgcDriver/Graphics/src/GuestSamplerResource.cpp) (libSceAgcDriver) - `FORCE_UNNORMALIZED` uses a Vulkan `unnormalizedCoordinates` sampler, so the S# LOD range, LOD bias and mip filter are not applied, which is exact only on a one-level view. From bae91578fe7a414b6a00749231fbe7bd4353dacd Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Wed, 21 Oct 2026 23:48:39 +0200 Subject: [PATCH 6/7] test(agc): drop the unwalked-import storage test whose partly unwatched range now stays watched --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 4 -- .../prx/libSceAgcDriver/tests/Recorder.cpp | 47 +++++-------------- 2 files changed, 13 insertions(+), 38 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index ab0803d7cfc6..4a6440642db5 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -1497,10 +1497,6 @@ if(BUILD_TESTING) set_tests_properties(agc_storage_kept_unit_settle PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() - if(NOT WIN32) - add_test(NAME agc_storage_unwalked_import COMMAND agc_driver_recorder_tests --unwalked-import-only) - set_tests_properties(agc_storage_unwalked_import PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 30) - endif() add_test(NAME agc_storage_baseline_settled COMMAND agc_driver_recorder_tests --settled-baseline-only) add_test(NAME agc_storage_clear_hand_over COMMAND agc_driver_recorder_tests --clear-hand-over-only) set_tests_properties(agc_storage_clear_hand_over PROPERTIES SKIP_RETURN_CODE 77) diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 46233dc94fc6..97f02e7f2f2d 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -5352,51 +5352,41 @@ void importWatchTests(const Device& device) { #endif } -template -bool staleGenerationTests(const Device& device, Recorder& recorder) { +void staleGenerationTests(const Device& device, Recorder& recorder) { const auto& base = device.GetContext(); #ifdef _WIN32 static_cast(recorder); static_cast(base); std::cout << "a range leaving the watch: Linux write watch only\n"; - return false; #else if (base.hostImportAlignment == 0) { std::cout << "host imports unavailable: a range leaving the watch not tested\n"; - return false; + return; } constexpr std::uint32_t side = 256; constexpr std::size_t surfaceBytes = side * side * 4; constexpr std::size_t keyCount = surfaceBytes / 256; - constexpr std::size_t lead = StaysWatched ? 65536 : 0; - constexpr std::size_t bytes = lead + surfaceBytes + 65536; + constexpr std::size_t bytes = surfaceBytes + 65536; void* block = AllocateWatched(bytes, 65536); if (block == nullptr) { std::cout << "no write watching: a range leaving the watch not tested\n"; - return false; + return; } struct Release { void* block; ~Release() { ReleaseWatched(block, bytes); } } release{block}; const auto decided = PrepareImportWatch(base); - SetImportWatch(base, StaysWatched ? ImportWatch::Watch : ImportWatch::Unwatch); + SetImportWatch(base, ImportWatch::Unwatch); struct Restore { const Context& context; ImportWatch decided; ~Restore() { SetImportWatch(context, decided); } } restore{base, decided}; - struct Budget { - bool set; - ~Budget() { - if (set) SetHostImportBudget(0); - } - } budget{StaysWatched}; - auto* texels = static_cast(block) + lead; + auto* texels = static_cast(block); std::memset(texels, 0x55, surfaceBytes); std::memset(texels + surfaceBytes, 0x00, keyCount); - const auto blockAddress = reinterpret_cast(block); - const auto address = blockAddress + lead; + const auto address = reinterpret_cast(block); { GuestAllocations::Mutation mutation; mutation.Add(block, bytes, true, true, true); @@ -5412,7 +5402,7 @@ bool staleGenerationTests(const Device& device, Recorder& recorder) { } HostImportFor(context, address, bytes); } - } unregister{base, block, blockAddress}; + } unregister{base, block, address}; TextureDetiler detiler(base); auto context = base; context.detiler = &detiler; @@ -5430,8 +5420,7 @@ bool staleGenerationTests(const Device& device, Recorder& recorder) { resource.dstSelW = 7; resource.dccAddress = address + surfaceBytes; { - Require(AgcDriver::GuestMemory::Watched(blockAddress, bytes), "(s) the block is not watched before the storage image takes its generation"); - if constexpr (StaysWatched) SetHostImportBudget(1); + Require(AgcDriver::GuestMemory::Watched(address, bytes), "(s) the block is not watched before the storage image takes its generation"); auto image = StorageTexture::Create(context, detiler, resource, 0); const VkImageSubresourceRange range{VK_IMAGE_ASPECT_COLOR_BIT, 0, 1, 0, 1}; const auto commands = recorder.Commands(); @@ -5442,20 +5431,13 @@ bool staleGenerationTests(const Device& device, Recorder& recorder) { context.Function("vkCmdClearColorImage")(commands, image->Image(), VK_IMAGE_LAYOUT_GENERAL, &red, 1, &range); RecordMemoryBarrier(context, commands, VK_PIPELINE_STAGE_TRANSFER_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_ACCESS_TRANSFER_WRITE_BIT, VK_ACCESS_SHADER_READ_BIT | VK_ACCESS_SHADER_WRITE_BIT | VK_ACCESS_TRANSFER_READ_BIT | VK_ACCESS_TRANSFER_WRITE_BIT); image->MarkDirty(); - if constexpr (StaysWatched) { - SetHostImportBudget(0); - Require(!image->GuestSnapshotValid(), "(s) the pending results had saved guest bytes while their memory was watched"); - AgcDriver::GuestMemory::Unwatch(blockAddress, lead); - Require(!AgcDriver::GuestMemory::Watched(blockAddress, lead) && AgcDriver::GuestMemory::Watched(address, bytes - lead), "(s) the block before the surface did not leave the watch alone"); - } - const auto* import = HostImportFor(base, blockAddress, bytes); + const auto* import = HostImportFor(base, address, bytes); if (import == nullptr) { std::cout << "host import of the stale generation block refused: a range leaving the watch not tested\n"; recorder.Sync(); - return false; + return; } - Require(!AgcDriver::GuestMemory::Watched(blockAddress, bytes), "(s) the imported range stayed watched"); - if constexpr (StaysWatched) Require(image->GuestSnapshotValid(), "(s) the results pending when a walk gave up the watch saved no guest bytes"); + Require(!AgcDriver::GuestMemory::Watched(address, bytes), "(s) the imported range stayed watched"); image->Refresh(); Buffer readback(context, surfaceBytes, VK_BUFFER_USAGE_TRANSFER_DST_BIT); const auto copyCommands = recorder.Commands(); @@ -5479,7 +5461,6 @@ bool staleGenerationTests(const Device& device, Recorder& recorder) { for (std::size_t i = 0; i < surfaceBytes; ++i) Require(texels[i] == expected[i % 4], "(s) results pending when their memory left the watch did not reach it"); } recorder.Sync(); - return true; #endif } @@ -10290,7 +10271,6 @@ int main(int argc, char** argv) { settledBaselineTests(device, recorder); return 0; } - if (argc == 2 && std::string_view(argv[1]) == "--unwalked-import-only") return staleGenerationTests(device, recorder) ? 0 : 77; if (argc == 2 && std::string_view(argv[1]) == "--completion-labels-only") { completionCountTests(device, recorder); afterRecordedWorkTests(device, recorder); @@ -10544,8 +10524,7 @@ int main(int argc, char** argv) { keptUnitSettleTests(device, recorder); settledBaselineTests(device, recorder); importWatchTests(device); - staleGenerationTests(device, recorder); - staleGenerationTests(device, recorder); + staleGenerationTests(device, recorder); importWindowTests(device, recorder); dataWordPositionsTests(); dataRefreshTests(device, recorder); From d84ce7c11ccc4da0c167a3f1d3dc101f990919dc Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Mon, 26 Oct 2026 14:36:49 +1300 Subject: [PATCH 7/7] fix(agc): suspend the tracker lock around the unwatch hook --- core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp index 52bac9f0f78d..e72fabda257e 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp @@ -1826,9 +1826,9 @@ bool ImportWatched(std::uint64_t address, std::size_t bytes, const std::function } } const auto saveBytes = [&](std::uint64_t begin, std::uint64_t end) { - lock.unlock(); + lock.Suspend(); beforeUnwatch(begin, static_cast(end - begin)); - lock.lock(); + lock.Resume(); }; std::vector> leaving; std::erase_if(spans, [&](const auto& span) {