fix(agc): hand a recorded submission to the driver before the recorder takes its next call

The recorder released its lock between recording a submission, flip or
suspend point and the driver accepting it. Another thread's delta could
then wait for the accepted work, read its blocks, and record itself
after a call whose work the driver had not accepted yet, so the replay
submitted that work before applying stale bytes over its result. The
entry points now pass the driver call to the recorder, which runs it
under the same lock right after recording the call.

agc_driver_frame_replay_concurrent submits copies from a second thread
during the capture and checks that every memory delta was read after
the submissions recorded before it had completed.

Co-Authored-By: Claude Opus 5.5 <[email protected]>

core/libs/prx/libSceAgcDriver/CMakeLists.txt

@@ -3478,6 +3478,12 @@ if(BUILD_TESTING)
                 TIMEOUT 120)
         set_tests_properties(agc_driver_frame_replay PROPERTIES ENVIRONMENT_MODIFICATION
                 "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>")
+        add_test(NAME agc_driver_frame_replay_concurrent COMMAND agc_driver_frame_replay_tests $<TARGET_FILE:agc_frame_replay> concurrent)
+        set_tests_properties(agc_driver_frame_replay_concurrent PROPERTIES
+                ENVIRONMENT "ANYPS5_NO_SHADER_CACHE=1;APS5_FRAME_CAPTURE=${CMAKE_CURRENT_BINARY_DIR}/frame-replay-concurrent-capture;APS5_FRAME_CAPTURE_AT_FLIP=1;APS5_FRAME_CAPTURE_FRAMES=8"
+                TIMEOUT 120)
+        set_tests_properties(agc_driver_frame_replay_concurrent PROPERTIES ENVIRONMENT_MODIFICATION
+                "PATH=path_list_prepend:$<TARGET_FILE_DIR:libc>")
     endif()
 endif()
 

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

@@ -10,6 +10,7 @@
 #include <cstdint>
 #include <deque>
 #include <filesystem>
+#include <functional>
 #include <map>
 #include <memory>
 #include <mutex>
@@ -35,10 +36,10 @@ public:
     void ResolveGraphicsAbi(const Shader* vertex, const Shader* pixel, std::uint32_t primitiveType);
     void RegisterVideoOutput(std::uint32_t handle);
     void UnregisterVideoOutput(std::uint32_t handle);
-    void Submit(const Packet& packet, std::uint32_t queue);
-    void Flip(const FlipInfo& info);
+    void Submit(const Packet& packet, std::uint32_t queue, const std::function<void()>& accept);
+    void Flip(const FlipInfo& info, const std::function<void()>& accept);
     void ReserveEopFlip(const FlipInfo& info, std::uint32_t token);
-    void Suspend();
+    void Suspend(const std::function<void()>& accept);
     void PresentBuffer(const DisplayBuffer& buffer);
     void FrameBoundary();
 
@@ -62,7 +63,8 @@ private:
     };
 
     FrameCall make(FrameCallKind kind);
-    void record(FrameCall call, bool prologueCall, std::span<const Shader* const> shaders = {});
+    void record(FrameCall call, bool prologueCall, std::span<const Shader* const> shaders = {}, const std::function<void()>& accept = {});
+    void recordLocked(FrameCall call, bool prologueCall, std::span<const Shader* const> shaders);
     void emitDelta();
     void emitShaderDelta(std::span<const Shader* const> shaders);
     void writeDelta(std::span<const Span> spans, std::span<const std::uint64_t> hostPages, std::span<const Range> added = {});

core/libs/prx/libSceAgcDriver/Execution/src/Driver/EntryPoints.cpp

@@ -19,9 +19,8 @@ void Submit(const Packet* packet, std::uint32_t queue) try {
         return;
     }
     GuestMemory::CheckRange(packet, sizeof(Packet), alignof(Packet));
-    recorder->Submit(*packet, queue);
     const auto flips = driver.FlipSubmissions();
-    driver.Submit(packet, queue);
+    recorder->Submit(*packet, queue, [&] { driver.Submit(packet, queue); });
     if (driver.FlipSubmissions() != flips) recorder->FrameBoundary();
 } catch (const ProcessShutdown&) {
     LibcAwaitExit_nid_postfix();
@@ -41,8 +40,12 @@ void RegisterShader(const Shader* shader) {
 }
 
 void SuspendPoint() {
-    if (auto* const recorder = FrameRecorder::Active()) recorder->Suspend();
-    DriverDetail::Driver::Get().SuspendPoint();
+    auto* const recorder = FrameRecorder::Active();
+    if (recorder == nullptr) {
+        DriverDetail::Driver::Get().SuspendPoint();
+        return;
+    }
+    recorder->Suspend([] { DriverDetail::Driver::Get().SuspendPoint(); });
 }
 
 void RegisterVideoOutput(std::uint32_t handle, const std::shared_ptr<IVideoOutput>& output) {
@@ -146,9 +149,12 @@ extern "C" void AgcDriverSerialProgress_nid_postfix(std::uint64_t* accepted, std
 
 extern "C" void AgcDriverSubmitFlip_nid_postfix(const AgcDriver::FlipInfo& info) try {
     auto* const recorder = AgcDriver::FrameRecorder::Active();
-    if (recorder != nullptr) recorder->Flip(info);
-    AgcDriver::DriverDetail::Driver::Get().SubmitFlip(info);
-    if (recorder != nullptr) recorder->FrameBoundary();
+    if (recorder == nullptr) {
+        AgcDriver::DriverDetail::Driver::Get().SubmitFlip(info);
+        return;
+    }
+    recorder->Flip(info, [&] { AgcDriver::DriverDetail::Driver::Get().SubmitFlip(info); });
+    recorder->FrameBoundary();
 } catch (const ProcessShutdown&) {
     LibcAwaitExit_nid_postfix();
 }

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

@@ -163,20 +163,20 @@ void FrameRecorder::UnregisterVideoOutput(std::uint32_t handle) {
     record(std::move(call), true);
 }
 
-void FrameRecorder::Submit(const Packet& packet, std::uint32_t queue) {
+void FrameRecorder::Submit(const Packet& packet, std::uint32_t queue, const std::function<void()>& accept) {
     auto call = make(FrameCallKind::Submit);
     call.queue = queue;
     call.address = pointer(packet.addr);
     call.size = packet.dw_num;
     call.flags = packet.flags;
-    record(std::move(call), false);
+    record(std::move(call), false, {}, accept);
 }
 
-void FrameRecorder::Flip(const FlipInfo& info) {
+void FrameRecorder::Flip(const FlipInfo& info, const std::function<void()>& accept) {
     auto call = make(FrameCallKind::Flip);
     call.cpuFlip = true;
     call.values = {info.handle, flipValue(info.index), info.mode, flipValue(info.argument)};
-    record(std::move(call), false);
+    record(std::move(call), false, {}, accept);
 }
 
 void FrameRecorder::ReserveEopFlip(const FlipInfo& info, std::uint32_t token) {
@@ -198,8 +198,8 @@ void FrameRecorder::ReserveEopFlip(const FlipInfo& info, std::uint32_t token) {
     ++recordedCalls;
 }
 
-void FrameRecorder::Suspend() {
-    record(make(FrameCallKind::Suspend), false);
+void FrameRecorder::Suspend(const std::function<void()>& accept) {
+    record(make(FrameCallKind::Suspend), false, {}, accept);
 }
 
 void FrameRecorder::PresentBuffer(const DisplayBuffer& buffer) {
@@ -236,8 +236,13 @@ void FrameRecorder::writeReferenceFrames() {
     }
 }
 
-void FrameRecorder::record(FrameCall call, bool prologueCall, std::span<const Shader* const> shaders) {
+void FrameRecorder::record(FrameCall call, bool prologueCall, std::span<const Shader* const> shaders, const std::function<void()>& accept) {
     std::lock_guard lock(mutex);
+    recordLocked(std::move(call), prologueCall, shaders);
+    if (accept) accept();
+}
+
+void FrameRecorder::recordLocked(FrameCall call, bool prologueCall, std::span<const Shader* const> shaders) {
     if (state == State::Waiting) {
         if (!prologueCall) return;
         call.ordinal = ++ordinal;

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

@@ -17,6 +17,7 @@
 #include <memory>
 #include <stdexcept>
 #include <string>
+#include <thread>
 #include <vector>
 #ifdef _WIN32
 #include <windows.h>
@@ -100,14 +101,18 @@ std::vector<std::byte> pattern(std::uint8_t seed) {
     return bytes;
 }
 
-void copyFrame(std::uint32_t* words, const Layout& layout, std::byte* target) {
-    CommandBuffer buffer{words, words + CommandBytes / sizeof(std::uint32_t), words, words + CommandBytes / sizeof(std::uint32_t), nullptr, nullptr, 0};
-    sceAgcDcbDmaData(&buffer, 0, DmaMemory, 0, at(target), DmaMemory, 0, at(layout.source), DataBytes, 0, 1, 0);
-    sceAgcDcbWriteData(&buffer, GraphicsMemory, 0, at(layout.label), &Done, 1, 0, 1);
+void copy(std::uint32_t* words, std::size_t commandBytes, const std::byte* source, std::byte* target, std::uint32_t* label) {
+    CommandBuffer buffer{words, words + commandBytes / sizeof(std::uint32_t), words, words + commandBytes / sizeof(std::uint32_t), nullptr, nullptr, 0};
+    sceAgcDcbDmaData(&buffer, 0, DmaMemory, 0, at(target), DmaMemory, 0, at(source), DataBytes, 0, 1, 0);
+    sceAgcDcbWriteData(&buffer, GraphicsMemory, 0, at(label), &Done, 1, 0, 1);
     Packet packet{words, static_cast<std::uint32_t>(buffer.cursor_up - words), 0, {}};
     check(sceAgcDriverSubmitDcb(&packet) == 0, "submission failed");
 }
 
+void copyFrame(std::uint32_t* words, const Layout& layout, std::byte* target) {
+    copy(words, CommandBytes, layout.source, target, layout.label);
+}
+
 void flip() {
     AgcDriverSubmitFlip_nid_postfix(AgcDriver::FlipInfo{Handle, -1, 1, 0});
 }
@@ -187,15 +192,75 @@ void testCaptureAndReplay(const std::filesystem::path& tool) {
     check(readFile(secondDump) == second, "the replay did not reproduce the second copy");
 }
 
+void testConcurrentCopies(const std::filesystem::path& tool) {
+    constexpr std::size_t Copies = 64;
+    constexpr std::size_t Frames = 8;
+    constexpr std::size_t CopyCommandBytes = 1024;
+    const char* directory = std::getenv("APS5_FRAME_CAPTURE");
+    check(directory != nullptr && *directory != '\0', "APS5_FRAME_CAPTURE is not set");
+    const std::filesystem::path capture(directory);
+    const auto layout = allocate();
+    const auto other = allocate();
+    auto* const otherSource = other.base + 128 * 1024;
+    auto* const targets = otherSource + DataBytes;
+    auto* const otherLabel = reinterpret_cast<std::uint32_t*>(other.base + 512 * 1024);
+    const auto copied = pattern(57);
+    std::memcpy(otherSource, copied.data(), copied.size());
+    auto output = std::make_shared<Output>();
+    AgcDriverRegisterVideoOutput_nid_postfix(Handle, output);
+
+    flip();
+    std::thread copier([&] {
+        for (std::size_t index = 0; index < Copies; ++index) {
+            copy(reinterpret_cast<std::uint32_t*>(other.base + index * CopyCommandBytes), CopyCommandBytes, otherSource, targets + index * DataBytes, otherLabel);
+        }
+    });
+    for (std::size_t frame = 0; frame + 1 < Frames; ++frame) {
+        const auto bytes = pattern(static_cast<std::uint8_t>(frame));
+        std::memcpy(layout.source, bytes.data(), bytes.size());
+        copyFrame(layout.firstCommands, layout, layout.firstTarget);
+        flip();
+    }
+    copier.join();
+    flip();
+    AgcDriverWaitIdle_nid_postfix();
+
+    const FrameCaptureReader reader(capture);
+    check(!reader.Truncated(), "the capture is truncated");
+    std::uint64_t base = 0;
+    std::size_t submits = 0, memory = 0;
+    for (const auto& call : reader.Calls()) {
+        if (call.kind == FrameCallKind::FrameStart) base = call.completed;
+        if (call.kind == FrameCallKind::Memory) {
+            ++memory;
+            check(call.completed >= base + submits, "a memory delta was read with " + std::to_string(base + submits - call.completed) + " of the " + std::to_string(submits) + " submissions recorded before it unfinished");
+        }
+        submits += call.kind == FrameCallKind::Submit;
+    }
+    check(submits == Copies + Frames - 1, "the capture holds " + std::to_string(submits) + " submits, not " + std::to_string(Copies + Frames - 1));
+    check(memory != 0, "the capture holds no memory deltas");
+
+    const auto dump = capture / "targets.bin";
+    const auto command = "\"\"" + tool.string() + "\" \"" + capture.string() + "\" --strict --dump " + std::to_string(at(targets)) + ":" + std::to_string(Copies * DataBytes) + ":\"" + dump.string() + "\"\"";
+    const auto code = std::system(command.c_str());
+    check(code == 0, "agc_frame_replay exited with " + std::to_string(code));
+    const auto replayed = readFile(dump);
+    check(replayed.size() == Copies * DataBytes, "the replay dumped " + std::to_string(replayed.size()) + " bytes of the copy targets");
+    for (std::size_t index = 0; index < Copies; ++index) {
+        check(std::memcmp(replayed.data() + index * DataBytes, copied.data(), DataBytes) == 0, "the replay did not reproduce copy " + std::to_string(index) + " of the second thread");
+    }
+}
+
 }
 
 int main(int argc, char** argv) {
-    if (argc != 2) {
-        std::fprintf(stderr, "usage: agc_driver_frame_replay_tests <agc_frame_replay>\n");
+    if (argc != 2 && !(argc == 3 && std::string(argv[2]) == "concurrent")) {
+        std::fprintf(stderr, "usage: agc_driver_frame_replay_tests <agc_frame_replay> [concurrent]\n");
         return 2;
     }
     try {
-        testCaptureAndReplay(argv[1]);
+        if (argc == 3) testConcurrentCopies(argv[1]);
+        else testCaptureAndReplay(argv[1]);
     } catch (const std::exception& error) {
         std::fprintf(stderr, "FAILED: %s\n", error.what());
         return 1;

docs/dev/BUILD.md

@@ -195,7 +195,7 @@ On Windows use `agc_pm4_replay.exe` and prepend `build/core/libs/libs/unpatched`
 
 ## Frame capture and replay
 
-Set `APS5_FRAME_CAPTURE` to a directory to record a few frames of a title at the AGC driver boundary, then replay them with `agc_frame_replay` through any build of the driver with no title running. From the start of the process the driver keeps every shader registration and ABI resolution, video output registration and EOP flip reservation. At the frame boundary picked by `APS5_FRAME_CAPTURE_AT_FLIP` (default 1), or by the appearance of the file `APS5_FRAME_CAPTURE_TRIGGER` names (a number in it replaces the frame count), it waits for the driver to go idle and writes every committed, readable allocation of the guest allocation registry, the title's image included, as zstd chunks without their all-zero 64 KiB blocks. A frame ends at a CPU flip or at a submission that flips (a flip packet, or the EOP release of a reserved flip token); the reservation itself only records its token. For the next `APS5_FRAME_CAPTURE_FRAMES` (10) frames it records each submission (queue, command buffer address and size), flip and suspend point with the thread that made it and the accepted and completed serials, each preceded by the guest pages the write tracker saw the CPU write since the previous call that handed work to the GPU (`GuestMemory::CollectWrittenBlocks`, per 4 KiB where the tracker kept page stamps), read once the submissions already accepted have completed; an allocation the registry gained after the snapshot is recorded there as a new range with its nonzero 64 KiB blocks. A submission ends a frame only when it flips itself, not when another thread's submission flipped while it was being submitted. Shader registrations and ABI resolutions only collect the blocks of the shader header and the arrays it points to, and the pages of the title's image only when their content changed. At the end it stores each display buffer the title presented, decoded, as a reference image. A capture is `calls.bin`, `memory.bin` and `capture.json` (commit, switches, sizes, warnings), and the recorder stops with a warning when its volume has less than 4 GiB free. A 20-frame capture in the Demon's Souls Nexus is 5.3 GiB of snapshot and 11.5 GiB of deltas before compression, 3.9 GiB on disk.
+Set `APS5_FRAME_CAPTURE` to a directory to record a few frames of a title at the AGC driver boundary, then replay them with `agc_frame_replay` through any build of the driver with no title running. From the start of the process the driver keeps every shader registration and ABI resolution, video output registration and EOP flip reservation. At the frame boundary picked by `APS5_FRAME_CAPTURE_AT_FLIP` (default 1), or by the appearance of the file `APS5_FRAME_CAPTURE_TRIGGER` names (a number in it replaces the frame count), it waits for the driver to go idle and writes every committed, readable allocation of the guest allocation registry, the title's image included, as zstd chunks without their all-zero 64 KiB blocks. A frame ends at a CPU flip or at a submission that flips (a flip packet, or the EOP release of a reserved flip token); the reservation itself only records its token. For the next `APS5_FRAME_CAPTURE_FRAMES` (10) frames it records each submission (queue, command buffer address and size), flip and suspend point with the thread that made it and the accepted and completed serials, each preceded by the guest pages the write tracker saw the CPU write since the previous call that handed work to the GPU (`GuestMemory::CollectWrittenBlocks`, per 4 KiB where the tracker kept page stamps), read once the submissions already accepted have completed (a recorded submission, flip or suspend point reaches the driver before the recorder takes its next call, so that wait covers every call recorded before the delta); an allocation the registry gained after the snapshot is recorded there as a new range with its nonzero 64 KiB blocks. A submission ends a frame only when it flips itself, not when another thread's submission flipped while it was being submitted. Shader registrations and ABI resolutions only collect the blocks of the shader header and the arrays it points to, and the pages of the title's image only when their content changed. At the end it stores each display buffer the title presented, decoded, as a reference image. A capture is `calls.bin`, `memory.bin` and `capture.json` (commit, switches, sizes, warnings), and the recorder stops with a warning when its volume has less than 4 GiB free. A 20-frame capture in the Demon's Souls Nexus is 5.3 GiB of snapshot and 11.5 GiB of deltas before compression, 3.9 GiB on disk.
 
 `agc_frame_replay <capture> [--no-pace] [--no-release] [--release-after-ms <n>] [--no-synthesize] [--strict] [--dump <address>:<bytes>:<file>]... [--frames <dir>]` maps the captured ranges at their addresses and registers them with their access (a range the replaying process already maps writable, such as the driver's fixed memory, is only refilled, and a non-GPU range it uses itself is skipped and counted), repeats the registrations, then replays the calls of each captured thread on a thread of its own, started in captured order. Before a call that hands work to the GPU it waits until no more submissions are in flight than when the title made it (`--no-pace` turns this off). Titles release GPU waits from threads that never call the driver, so when the driver completes nothing for 3 ms the replay reads the waits its queues are blocked on (`AgcDriverAwaitedWaits`) and writes the next recorded CPU value of the awaited dword that satisfies the compare, else the last earlier one, else a value derived from the reference (`--no-synthesize` turns this last step off). It prints one JSON line with the frame count and times, the submissions, the pacing wait, the released waits and how many were derived, the submissions that never completed and, for captures with reference images, the share of identical pixels and the largest channel difference of the replayed display buffers (`--frames` writes both images). Effects that depend on timing (temporal antialiasing, particles) make pixels differ, so expect a visual match rather than identical frames. Replay times follow the GPU clock state: lock the clocks (`nvidia-smi -lgc`, `-lmc`) when comparing two drivers.