fix(agc): save pending storage results in every 64 KiB block that leaves the write watch

On Windows the write watch is dropped in whole 64 KiB blocks
(WriteWatchCoverage::Exclude), but the unwatch hook was given the page
range of the import. An image with results pending in the same block as
an imported page, outside that page, lost the watch without its guest
bytes saved, and storing its results threw.

The hook now gets the span of blocks Exclude drops (ExcludedSpan, the same
block math), so every image with results pending in those blocks saves its
bytes first.

core/libs/prx/libSceAgcDriver/Execution/include/WriteWatchCoverage.hpp

@@ -26,18 +26,18 @@ public:
         return std::none_of(excluded.begin() + first, excluded.begin() + last + 1, [](auto generation) { return generation != 0; });
     }
 
+    bool ExcludedSpan(std::uint64_t address, std::size_t bytes, std::uint64_t* begin, std::uint64_t* end) const {
+        std::size_t first = 0;
+        std::size_t last = 0;
+        if (!excludedBlocks(address, bytes, first, last)) return false;
+        *begin = base + first * BlockBytes;
+        *end = std::min<std::uint64_t>(base + (last + 1) * BlockBytes, base + size);
+        return true;
+    }
     bool Exclude(std::uint64_t address, std::size_t bytes, std::uint64_t generation = 1) {
-        if (bytes == 0 || size == 0) return false;
-        if (address < base) {
-            if (base - address >= bytes) return false;
-            bytes -= base - address;
-            address = base;
-        }
-        const auto offset = address - base;
-        if (offset >= size) return false;
-        bytes = static_cast<std::size_t>(std::min<std::uint64_t>(bytes, size - offset));
-        const auto first = offset / BlockBytes;
-        const auto last = (offset + bytes - 1) / BlockBytes;
+        std::size_t first = 0;
+        std::size_t last = 0;
+        if (!excludedBlocks(address, bytes, first, last)) return false;
         bool changed = false;
         for (auto block = first; block <= last; ++block) {
             changed = changed || excluded[block] == 0;
@@ -73,6 +73,20 @@ public:
     }
 
 private:
+    bool excludedBlocks(std::uint64_t address, std::size_t bytes, std::size_t& first, std::size_t& last) const {
+        if (bytes == 0 || size == 0) return false;
+        if (address < base) {
+            if (base - address >= bytes) return false;
+            bytes -= base - address;
+            address = base;
+        }
+        const auto offset = address - base;
+        if (offset >= size) return false;
+        bytes = static_cast<std::size_t>(std::min<std::uint64_t>(bytes, size - offset));
+        first = offset / BlockBytes;
+        last = (offset + bytes - 1) / BlockBytes;
+        return true;
+    }
     static constexpr std::size_t BlockBytes = 65536;
     std::uint64_t base = 0;
     std::size_t size = 0;

core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp

@@ -1267,7 +1267,21 @@ namespace {
 std::atomic<void (*)(std::uint64_t, std::size_t)> unwatchHook{nullptr};
 
 void beforeUnwatch(std::uint64_t address, std::size_t bytes) {
-    if (const auto hook = unwatchHook.load(std::memory_order_acquire)) hook(address, bytes);
+    const auto hook = unwatchHook.load(std::memory_order_acquire);
+    if (hook == nullptr) return;
+#ifdef _WIN32
+    std::uint64_t begin = 0;
+    std::uint64_t end = 0;
+    {
+        auto& tracker = Tracker();
+        const auto lock = lockTracker(tracker);
+        tracker.initialize();
+        if (!tracker.watched || !tracker.coverage.ExcludedSpan(address, bytes, &begin, &end)) return;
+    }
+    hook(begin, static_cast<std::size_t>(end - begin));
+#else
+    hook(address, bytes);
+#endif
 }
 
 void unwatchLocked(WriteTracker& tracker, std::uint64_t address, std::size_t bytes) {

core/libs/prx/libSceAgcDriver/tests/WriteWatchCoverage.cpp

@@ -18,6 +18,13 @@ int main() {
         require(coverage.Covers(base, 4 * block + 1), "the initialized arena is not watched");
         require(!coverage.Covers(base - 1, 1) && !coverage.Covers(base + 4 * block + 1, 1), "outside memory is watched");
         require(!coverage.Covers(base, 0) && !coverage.Covers(base, std::numeric_limits<std::size_t>::max()), "an empty or overflowing range is watched");
+        std::uint64_t spanBegin = 0;
+        std::uint64_t spanEnd = 0;
+        require(coverage.ExcludedSpan(base + block + 4096, 4096, &spanBegin, &spanEnd) && spanBegin == base + block && spanEnd == base + 2 * block, "the span of an imported page is not its whole block");
+        require(coverage.ExcludedSpan(base + 3 * block - 1, 2, &spanBegin, &spanEnd) && spanBegin == base + 2 * block && spanEnd == base + 4 * block, "the span of a boundary import is not both blocks");
+        require(coverage.ExcludedSpan(base + 4 * block, std::numeric_limits<std::size_t>::max(), &spanBegin, &spanEnd) && spanBegin == base + 4 * block && spanEnd == base + 4 * block + 1, "the span of the last partial block is not clipped to the arena");
+        require(coverage.ExcludedSpan(base - 1, 2, &spanBegin, &spanEnd) && spanBegin == base && spanEnd == base + block, "the span of an import crossing the arena start is not clipped");
+        require(!coverage.ExcludedSpan(base - block, block, &spanBegin, &spanEnd) && !coverage.ExcludedSpan(base + 5 * block, block, &spanBegin, &spanEnd) && !coverage.ExcludedSpan(base, 0, &spanBegin, &spanEnd), "an outside or empty import has a span");
         require(coverage.Exclude(base + block + 4096, 1), "an imported page was not excluded");
         require(!coverage.Covers(base + block, block), "the imported block is still watched");
         require(!coverage.Covers(base + block - 1, 2), "a range crossing into the import is watched");

core/libs/prx/libSceAgcDriver/tests/execution/StoreOverUntracked.cpp

@@ -37,6 +37,8 @@ using AgcDriver::Graphics::StorageTexture;
 constexpr std::uint32_t Side = 256;
 constexpr std::uint32_t Format32UInt = 20;
 constexpr std::uint32_t TileR64KBX = 0x1b;
+constexpr std::uint32_t TileLinear = 0;
+std::uint32_t surfaceTile = TileR64KBX;
 constexpr std::uint32_t Type2D = 9;
 constexpr std::size_t Page = 0x1000;
 constexpr std::size_t Block = 0x10000;
@@ -68,7 +70,7 @@ std::array<std::uint32_t, 8> TextureDescriptor(const std::uint32_t* texels, std:
         static_cast<std::uint32_t>(address >> 8u),
         static_cast<std::uint32_t>((address >> 40u) & 0xffu) | (Format32UInt << 20u) | (((side - 1u) & 3u) << 30u),
         ((side - 1u) >> 2u) | ((side - 1u) << 14u),
-        0xfacu | (TileR64KBX << 20u) | (Type2D << 28u),
+        0xfacu | (surfaceTile << 20u) | (Type2D << 28u),
         0u,
         0u,
         0u,
@@ -120,40 +122,42 @@ void Fill(const std::uint32_t* texels) {
 
 class Surface {
 public:
-    explicit Surface(bool watched = false) : watched(watched) {
+    explicit Surface(bool watched = false, std::size_t lead = 0) : watched(watched), regionBytes(lead == 0 ? SurfaceBytes : SurfaceBytes + Block) {
         if (watched) {
 #ifdef _WIN32
-            texels = static_cast<std::uint32_t*>(GuestArena::GuestArenaAllocate_nid_postfix(SurfaceBytes, Block));
-            if (texels != nullptr) GuestArena::GuestArenaCommit_nid_postfix(texels, SurfaceBytes, PAGE_READWRITE, SurfaceBytes);
+            region = static_cast<std::uint8_t*>(GuestArena::GuestArenaAllocate_nid_postfix(regionBytes, Block));
+            if (region != nullptr) GuestArena::GuestArenaCommit_nid_postfix(region, regionBytes, PAGE_READWRITE, regionBytes);
 #else
-            void* mapped = mmap(nullptr, SurfaceBytes + Block, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+            void* mapped = mmap(nullptr, regionBytes + Block, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
             if (mapped != MAP_FAILED) {
                 const auto begin = reinterpret_cast<std::uintptr_t>(mapped);
                 const auto aligned = (begin + Block - 1) & ~std::uintptr_t{Block - 1};
                 if (aligned != begin) munmap(mapped, aligned - begin);
-                if (aligned != begin + Block) munmap(reinterpret_cast<void*>(aligned + SurfaceBytes), begin + Block - aligned);
-                texels = reinterpret_cast<std::uint32_t*>(aligned);
-                GuestWriteWatch::GuestWriteWatchRegister_nid_postfix(texels, SurfaceBytes);
+                if (aligned != begin + Block) munmap(reinterpret_cast<void*>(aligned + regionBytes), begin + Block - aligned);
+                region = reinterpret_cast<std::uint8_t*>(aligned);
+                GuestWriteWatch::GuestWriteWatchRegister_nid_postfix(region, regionBytes);
             }
 #endif
+            if (region != nullptr) texels = reinterpret_cast<std::uint32_t*>(region + lead);
         } else {
             storage.resize((SurfaceBytes + Block) / 4u);
             texels = reinterpret_cast<std::uint32_t*>((reinterpret_cast<std::uintptr_t>(storage.data()) + Block - 1) & ~std::uintptr_t{Block - 1});
         }
         Require(texels != nullptr, "store over untracked: cannot allocate the guest surface");
         std::fill(texels, texels + SurfaceBytes / 4u, Untouched);
-        GuestAllocations::Mutation().Add(texels, SurfaceBytes, true, true, true);
+        allocation = region != nullptr ? static_cast<void*>(region) : static_cast<void*>(texels);
+        GuestAllocations::Mutation().Add(allocation, region != nullptr ? regionBytes : SurfaceBytes, true, true, true);
     }
 
     ~Surface() {
-        GuestAllocations::Mutation().Remove(texels);
+        GuestAllocations::Mutation().Remove(allocation);
         if (!watched) return;
 #ifdef _WIN32
-        GuestArena::GuestArenaReset_nid_postfix(texels, SurfaceBytes);
-        GuestArena::GuestArenaRelease_nid_postfix(texels, SurfaceBytes);
+        GuestArena::GuestArenaReset_nid_postfix(region, regionBytes);
+        GuestArena::GuestArenaRelease_nid_postfix(region, regionBytes);
 #else
-        munmap(texels, SurfaceBytes);
-        GuestWriteWatch::GuestWriteWatchUnregister_nid_postfix(texels, SurfaceBytes);
+        munmap(region, regionBytes);
+        GuestWriteWatch::GuestWriteWatchUnregister_nid_postfix(region, regionBytes);
 #endif
     }
 
@@ -166,7 +170,10 @@ public:
 
 private:
     bool watched;
+    std::size_t regionBytes;
     std::vector<std::uint32_t> storage;
+    std::uint8_t* region = nullptr;
+    void* allocation = nullptr;
     std::uint32_t* texels = nullptr;
 };
 
@@ -210,7 +217,7 @@ void Settle(THold& hold, const Surface& surface, std::uint32_t result) {
     hold.unlock();
 }
 
-enum class Kind { ReadAfter, ReadBefore, Filled, Copied, LeftWatch, WrittenThenLeftWatch, ImportLeftWatch, NeighbourCopied };
+enum class Kind { ReadAfter, ReadBefore, Filled, Copied, LeftWatch, WrittenThenLeftWatch, ImportLeftWatch, NeighbourCopied, NeighbourLeftWatch };
 
 // One 64 KiB block of the surface: the image of its last block, and the source copied into it.
 constexpr std::uint32_t BlockSide = 128;
@@ -289,8 +296,10 @@ Outcome RunSecondStore(const char* name) {
 Outcome Run(const Case& test) {
     std::optional<Surface> source;
     if (test.kind == Kind::Copied || test.kind == Kind::NeighbourCopied) source.emplace();
-    const bool leaves = test.kind == Kind::LeftWatch || test.kind == Kind::WrittenThenLeftWatch || test.kind == Kind::ImportLeftWatch;
-    Surface surface(leaves);
+    const bool neighbour = test.kind == Kind::NeighbourLeftWatch;
+    const bool leaves = test.kind == Kind::LeftWatch || test.kind == Kind::WrittenThenLeftWatch || test.kind == Kind::ImportLeftWatch || neighbour;
+    surfaceTile = neighbour ? TileLinear : TileR64KBX;
+    Surface surface(leaves, neighbour ? Page : 0);
     struct WaitForWork {
         ~WaitForWork() { AgcDriverWaitIdle_nid_postfix(); }
     } waitForWork;
@@ -385,8 +394,9 @@ Outcome Run(const Case& test) {
         Require(image != nullptr, "store over untracked: the surface has no storage image");
         Require(!image->GuestSnapshotValid(), "store over untracked: the pending result had saved guest bytes while its memory was write-watched");
         if (test.kind == Kind::ImportLeftWatch) Require(AgcDriver::GuestMemory::ImportWatched(address, SurfaceBytes, [] { return true; }, false), "store over untracked: the import that leaves the write watch failed");
+        else if (neighbour) Require(AgcDriver::GuestMemory::ImportWatched(address - Page, Page, [] { return true; }, false), "store over untracked: the import of the page before the surface failed");
         else AgcDriver::GuestMemory::Unwatch(address, SurfaceBytes);
-        Require(!AgcDriver::GuestMemory::Watched(address, SurfaceBytes), "store over untracked: the surface is still write-watched");
+        if (!neighbour) Require(!AgcDriver::GuestMemory::Watched(address, SurfaceBytes), "store over untracked: the surface is still write-watched");
     }
     std::array<std::byte, Page> read{};
     if (test.kind == Kind::ReadBefore) AgcDriver::GuestMemory::Read(address + Block + Page, read);
@@ -418,7 +428,7 @@ int main() {
             Surface probe;
             Require(AgcDriver::Graphics::DescribeSurface(AgcDriver::Graphics::DecodeTextureResource(TextureDescriptor(probe.Texels()))).guestBytes == SurfaceBytes, "store over untracked: unexpected surface size");
         }
-        constexpr std::array<Case, 8> cases{{
+        constexpr std::array<Case, 9> cases{{
             {"another block read after the CPU write", Kind::ReadAfter, Block + Page},
             {"another block read before the CPU write", Kind::ReadBefore, 0xe000},
             {"a fill of the surface", Kind::Filled, 0xe000},
@@ -427,6 +437,7 @@ int main() {
             {"a surface written before it left the write watch", Kind::WrittenThenLeftWatch, 0xe000},
             {"a surface whose import left the write watch", Kind::ImportLeftWatch, 0xe000},
             {"an image copy into another block of the surface", Kind::NeighbourCopied, 0xe000},
+            {"a surface whose first block left the write watch with the page before it", Kind::NeighbourLeftWatch, 0xe000},
         }};
         bool kept = true;
         for (const auto& test : cases) {