From a3046f449ce09fabc19f7564816272ea115a4dad Mon Sep 17 00:00:00 2001 From: Nicolas Gonzalez Date: Fri, 30 Oct 2026 20:14:51 -0300 Subject: [PATCH] perf(agc): trap label reads of small images inside pending large ones The label read trap skipped every image sharing a page with another pending image. Large tiled images are never stored at a label, so an image inside one no longer counts as sharing. A store of an image over a trapped one now stores the trapped one in the same flush, and an eviction or explicit write-back stores the trapped images inside it first. Co-Authored-By: Claude Opus 5.5 --- .../Graphics/include/Texture.hpp | 1 + .../libSceAgcDriver/Graphics/src/Texture.cpp | 39 ++++++++++++++- .../prx/libSceAgcDriver/tests/Recorder.cpp | 47 +++++++++++++++++++ docs/dev/debt/libSceAgcDriver.md | 2 +- 4 files changed, 87 insertions(+), 2 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp index cd022bcad1..ac4b597ec8 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Texture.hpp @@ -495,6 +495,7 @@ private: void writeBack(std::uint64_t address, std::size_t bytes); void writeBackLayers(const std::vector& layers); void releaseReadTraps(bool sharingPages); + void storeTrappedOverlaps(const char* reason); bool unchangedSinceBaseline(std::uint64_t from, std::uint64_t to) const; void saveBeforeUnwatch(); void forgetKeptAtUnwatch(std::uint32_t unit); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp index bee5003107..59a21c0cec 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp @@ -4228,6 +4228,7 @@ std::uint64_t StorageTexture::storeDrawnRect(const DrawnArea& area, std::span& stored) { return stored->overlaps(texture->descriptor.baseAddress, static_cast(texture->guestBytes)); }; + if (texture == except || texture == exempt || !texture->readTrapped.load(std::memory_order_acquire) || std::none_of(flush.begin(), flush.begin() + static_cast(selected), covered)) { + ++it; + continue; + } + texture->dirty = false; + if (auto alive = texture->weak_from_this().lock()) flush.push_back(std::move(alive)); + it = pending.textures.erase(it); + } + } +#endif // Still pending for the adjacency rule of writeBack until each one is stored. for (const auto& texture : flush) pending.flushing.push_back(texture.get()); pending.flushingCount.store(pending.flushing.size(), std::memory_order_release); @@ -4764,7 +4781,8 @@ void StorageTexture::StoreReadbackBeforeLabel(const char* reason) { const auto first = texture->descriptor.baseAddress & ~(page - 1); const auto bytes = static_cast(((texture->descriptor.baseAddress + texture->guestBytes + page - 1) & ~(page - 1)) - first); const auto shares = [&](const StorageTexture* other) { return other != texture && other->overlaps(first, bytes); }; - return std::any_of(pending.textures.begin(), pending.textures.end(), shares) || std::any_of(pending.flushing.begin(), pending.flushing.end(), shares); + const auto sharesStored = [&](const StorageTexture* other) { return shares(other) && (other->descriptor.tileMode == TextureTileMode::kLinear || other->guestBytes <= SmallStoreBytes); }; + return std::any_of(pending.textures.begin(), pending.textures.end(), sharesStored) || std::any_of(pending.flushing.begin(), pending.flushing.end(), shares); }; #endif for (const auto* texture : pending.textures) { @@ -5619,6 +5637,7 @@ bool StorageTexture::FillClear(std::span pattern, std::u } void StorageTexture::WriteBack() { + storeTrappedOverlaps("explicit"); const auto previous = std::exchange(flushReason, "explicit"); writeBack(descriptor.baseAddress, static_cast(guestBytes)); flushReason = previous; @@ -6069,6 +6088,24 @@ void StorageTexture::releaseReadTraps(bool sharingPages) { #endif } +void StorageTexture::storeTrappedOverlaps(const char* reason) { +#if defined(__APPLE__) + if (!labelTrapsSet.load(std::memory_order_relaxed)) return; + std::vector> ranges; + { + auto& pending = Pending(); + std::lock_guard lock(pending.mutex); + if (!dirty) return; + for (const auto* texture : pending.textures) { + if (texture != this && texture->readTrapped.load(std::memory_order_acquire) && texture->overlaps(descriptor.baseAddress, static_cast(guestBytes))) ranges.emplace_back(texture->descriptor.baseAddress, static_cast(texture->guestBytes)); + } + } + for (const auto& [address, bytes] : ranges) static_cast(FlushPending(address, bytes, nullptr, reason)); +#else + static_cast(reason); +#endif +} + void StorageTexture::writeBack(std::uint64_t address, std::size_t bytes) { if (readTrapped.load(std::memory_order_acquire)) { address = descriptor.baseAddress; diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 7a414c41f3..9f9ca656a6 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -6366,6 +6366,53 @@ bool smallTiledShadowLabelTests(const Device& device, Recorder& recorder) { StorageTexture::StoreReadbackBeforeLabel("readback before label"); Require(HostPageProtection(pageBytes) == VM_PROT_NONE, "a label did not trap the image a neighbouring CPU store left pending"); Require(readUnlocked([&] { return holdsIn(linearA, static_cast(layoutA.guestBytes), 0x63); }), "a read of the re-trapped image did not see its pending results"); + + auto outer = resource; + outer.baseAddress = address; + outer.width = 256; + outer.height = 256; + outer.tileMode = TextureTileMode::kR64KBX; + const auto outerLayout = DescribeSurface(outer); + Require(outerLayout.guestBytes > StorageTexture::SmallStoreBytes && outer.baseAddress + outerLayout.guestBytes <= resource.baseAddress, "the large tiled surface does not fit below the small one"); + auto inner = linearA; + inner.baseAddress = address + 0x20000; + const auto innerLayout = DescribeSurface(inner); + auto outerImage = StorageTexture::Create(context, detiler, outer, 0); + auto innerImage = StorageTexture::Create(context, detiler, inner, 0); + outerImage->Refresh(); + innerImage->Refresh(); + settle(); + auto* innerBytes = memory + (inner.baseAddress - address); + const auto innerHolds = [&](std::uint8_t value) { return holdsIn(inner, static_cast(innerLayout.guestBytes), value); }; + const auto storeOuterBlock = [&] { + Require(StorageTexture::FlushPending(address + 0x28000, 256, nullptr, "memory access", PublishScope::None), "a store of the large image's block stored nothing"); + settle(); + }; + rewriteImage(outerImage, 256u, 0x71); + rewriteImage(innerImage, 8u, 0x72); + StorageTexture::StoreReadbackBeforeLabel("readback before label"); + Require(HostPageProtection(innerBytes) == VM_PROT_NONE, "a label stored a small image inside a pending large tiled one instead of trapping its reads"); + storeOuterBlock(); + Require(HostPageProtection(innerBytes) != VM_PROT_NONE, "a store of a large image's block left a trapped image inside it trapped"); + Require(innerHolds(0x72), "a store of a large image's block over a trapped image did not leave the bytes a label store leaves"); + rewriteImage(outerImage, 256u, 0x73); + rewriteImage(innerImage, 8u, 0x75); + rewriteImage(outerImage, 256u, 0x74); + StorageTexture::StoreReadbackBeforeLabel("readback before label"); + Require(HostPageProtection(innerBytes) == VM_PROT_NONE, "a label did not trap the small image again"); + storeOuterBlock(); + Require(HostPageProtection(innerBytes) != VM_PROT_NONE && innerHolds(0x75), "a large image written again after a trapped one was stored before it"); + rewriteImage(outerImage, 256u, 0x76); + rewriteImage(innerImage, 8u, 0x77); + StorageTexture::StoreReadbackBeforeLabel("readback before label"); + Require(HostPageProtection(innerBytes) == VM_PROT_NONE, "a label did not trap the small image a third time"); + outerImage->Flush(); + settle(); + Require(HostPageProtection(innerBytes) != VM_PROT_NONE && innerHolds(0x77), "evicting a large image did not store the trapped image inside it first"); + rewriteImage(innerImage, 8u, 0x78); + StorageTexture::StoreReadbackBeforeLabel("readback before label"); + Require(HostPageProtection(innerBytes) == VM_PROT_NONE, "a label did not trap the small image after the large one was evicted"); + Require(readUnlocked([&] { return innerHolds(0x78); }), "a read of a trapped image inside an evicted large one did not see its results"); } #endif auto large = resource; diff --git a/docs/dev/debt/libSceAgcDriver.md b/docs/dev/debt/libSceAgcDriver.md index 7d31c9d16c..1f9b4950d6 100644 --- a/docs/dev/debt/libSceAgcDriver.md +++ b/docs/dev/debt/libSceAgcDriver.md @@ -170,7 +170,7 @@ - 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), except drawn resident linear targets (next entry), 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. Explicit-size file writes (`write`, `pwrite`, `writev`, `pwritev`, AIO and `fwrite`) synchronize their source ranges through the [host-read hook](../../../core/libs/prx/libc/src/GuestArena.cpp); other host readers are not covered. `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. -- [Label read traps](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver, macOS): before a label, a pending linear or small storage image that shares no page with another pending or storing image, and is not direct memory mapped twice, gets a [read trap](../../../core/libs/prx/libc/src/GuestWriteWatch.cpp) instead of its store; the first access to its pages stores it whole through the host-read hook. Any other store releases the trap once its GPU copy into the import is recorded, so a read before that copy lands sees the old bytes; a store through the CPU releases it and the traps of other images on its pages first, leaving those pending untrapped until the next label. A thread that holds `GpuMutex` and touches a trapped image waits for the resolver forever, a system call that reads one without the host-read hook fails with `EFAULT`, and other memory mapped twice is not detected. `APS5_NO_LABEL_READ_TRAP=1` stores at the label instead. +- [Label read traps](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver, macOS): at a label, a pending linear or small storage image that shares no page with another image a label stores or one being stored, and is not direct memory mapped twice, gets a [read trap](../../../core/libs/prx/libc/src/GuestWriteWatch.cpp) instead of its store. The first access to its pages stores it whole through the host-read hook; a store of another image over it stores it too, and an eviction or explicit write-back stores it first. Other stores release the trap once their GPU copy is recorded, so a read before that copy lands sees the old bytes; a CPU store releases the traps on its pages first, leaving those images untrapped until the next label. A thread holding `GpuMutex` touching a trapped image waits forever, a system call that reads one without the hook fails with `EFAULT`, and other double mappings go undetected. `APS5_NO_LABEL_READ_TRAP=1` stores at the label. - [Resident linear color targets](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver): a CPU store inside a drawn scissor rectangle, on a pixel no primitive wrote, made before the target's store is overwritten by it, where the console keeps it. Recorded draws' results are stored before the graphics queue's next flip, label, end-of-pipe interrupt, drain or device wait, as the union of their scissor rectangles; a draw that brings a 17th rectangle that does not merge stores the target at once. A target with an attachment proxy, several mips or layers, a 3D or DCC surface, or results of a dispatch, fill or copy is stored after each draw. One that a fill or copy writes after its draws is stored whole at the next of those points, and GPU results in its CPU-written 64 KiB blocks are dropped. A store the draw pipeline's committer makes (label, end-of-pipe interrupt, device wait) skips a target whose range a queued draw still reads (`DrawPipeline::HeldReads`). - [Direct writes over a drawn linear target](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Texture.cpp) (libSceAgcDriver): a CPU store into a 64 KiB block of a drawn resident linear target that lands while the completion of a direct write recorded before the draws checks and stamps that block (`StampKeepingNewerResults`) drops the draws' results in that block, where the console keeps them: the stamp's own collect records the store per page only, so the store reads the block as written by the driver. - [Linear color targets](../../../core/libs/prx/libSceAgcDriver/Graphics/src/Draw.cpp) (libSceAgcDriver) with no host import are drawn synchronously (copied in, drawn, waited for, copied back), where the console renders into memory in stream order, unless the draw can make an import that keeps them write-watched. It makes one for a target in registered, writable memory only when the whole import run is write-watched and the import stays watched: a separate alias of shared memory, a direct import of 1 MiB or more of private memory that settles on Windows (when the import probe keeps the watch), or one the probe keeps watched elsewhere. Smaller private ranges on Windows, runs with memory outside the write watch and devices whose imports are compared stay synchronous: a compared import makes every refresh of the resident image compare the whole target. A target over an existing import is resident, compared or not. `APS5_NO_RESIDENT_LINEAR=1` keeps every linear target synchronous.