From 60f12c888d85e6bac7ec3cf6d09a3e4eb1977cae Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Sun, 11 Oct 2026 14:15:27 +1300 Subject: [PATCH] fix(agc): compare host imports when device writes reach the write watch The import probe only checked whether a GPU read of an imported range reports its pages as written. lavapipe runs command buffers on CPU threads, so its writes into an import go through the page tables and are collected as CPU stores. In agc_unit_shadow the whole publish's copy into the import is then collected as a CPU store over unit 3, and the next partial publish drops the slab's newer bytes as stale: "the partial publish did not copy exactly the partly covered unit". The probe now also fills the scratch range on the GPU and collects again. When the read or the fill reports pages as written, host and dma-buf imports are compared. lavapipe reports 0 of 4 pages after the read and 4 of 4 after the fill; NVIDIA reports 0 after the fill. The probe change is the first commit of #1046. --- .../Graphics/include/GuestBufferMemory.hpp | 1 + .../Graphics/src/GuestBufferMemory.cpp | 46 +++++++++++-------- .../prx/libSceAgcDriver/tests/Recorder.cpp | 2 +- 3 files changed, 30 insertions(+), 19 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index f180e1e9b7..b260da581d 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -44,6 +44,7 @@ struct ImportProbe { std::uint32_t writtenAtImport = 0; std::uint32_t writtenAfterSubmit = 0; std::uint32_t writtenByCpu = 0; + std::uint32_t writtenAfterFill = 0; }; ImportProbe ProbeImportWriteProtection(const Context& context); diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 1f6091d71c..9ffe299e6e 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -499,8 +499,8 @@ void decideImportWatch(const Context& context, HostImports& state) { state.unwatchImports = true; std::fprintf(stderr, "[write-watch] host imports resolve write protection: unknown (probe failed at %s, %d); imported ranges are compared\n", probe.failure, static_cast(probe.result)); } else { - state.unwatchImports = probe.writtenAfterSubmit != 0; - std::fprintf(stderr, "[write-watch] host imports resolve write protection: %s (%u of %u scratch pages written after a GPU read, %u after the import); imported ranges %s\n", state.unwatchImports ? "yes" : "no", probe.writtenAfterSubmit, probe.pages, probe.writtenAtImport, state.unwatchImports ? "are compared" : "stay watched"); + state.unwatchImports = probe.writtenAfterSubmit != 0 || probe.writtenAfterFill != 0; + std::fprintf(stderr, "[write-watch] host imports resolve write protection: %s (%u of %u scratch pages written after a GPU read, %u after a GPU write, %u after the import); imported ranges %s\n", state.unwatchImports ? "yes" : "no", probe.writtenAfterSubmit, probe.pages, probe.writtenAfterFill, probe.writtenAtImport, state.unwatchImports ? "are compared" : "stay watched"); } state.unwatchDmaBufImports = state.unwatchImports; if (!context.dmaBufImport) return; @@ -510,8 +510,8 @@ void decideImportWatch(const Context& context, HostImports& state) { std::fprintf(stderr, "[write-watch] dma-buf imports keep write protection: unknown (probe failed at %s, %d); imported ranges are compared\n", dmaBuf.failure, static_cast(dmaBuf.result)); return; } - state.unwatchDmaBufImports = dmaBuf.writtenAfterSubmit != 0 || dmaBuf.writtenByCpu == 0; - std::fprintf(stderr, "[write-watch] dma-buf imports keep write protection: %s (%u of %u scratch pages written after a GPU read, %u after the import, %u seen after a CPU store); imported ranges %s\n", state.unwatchDmaBufImports ? "no" : "yes", dmaBuf.writtenAfterSubmit, dmaBuf.pages, dmaBuf.writtenAtImport, dmaBuf.writtenByCpu, state.unwatchDmaBufImports ? "are compared" : "stay watched"); + state.unwatchDmaBufImports = dmaBuf.writtenAfterSubmit != 0 || dmaBuf.writtenAfterFill != 0 || dmaBuf.writtenByCpu == 0; + std::fprintf(stderr, "[write-watch] dma-buf imports keep write protection: %s (%u of %u scratch pages written after a GPU read, %u after a GPU write, %u after the import, %u seen after a CPU store); imported ranges %s\n", state.unwatchDmaBufImports ? "no" : "yes", dmaBuf.writtenAfterSubmit, dmaBuf.pages, dmaBuf.writtenAfterFill, dmaBuf.writtenAtImport, dmaBuf.writtenByCpu, state.unwatchDmaBufImports ? "are compared" : "stay watched"); #endif } @@ -1510,7 +1510,7 @@ void runImportProbe(const Context& context, std::uint64_t base, std::uint64_t by HostImport import{base, bytes, VK_NULL_HANDLE, VK_NULL_HANDLE, 0}; VkBuffer destination = VK_NULL_HANDLE; VkDeviceMemory destinationMemory = VK_NULL_HANDLE; - VkCommandBuffer commands = VK_NULL_HANDLE; + VkCommandBuffer commands[2]{}; VkFence fence = VK_NULL_HANDLE; bool submitted = false; const auto collect = [&](std::uint32_t& pages) { @@ -1540,24 +1540,34 @@ void runImportProbe(const Context& context, std::uint64_t base, std::uint64_t by VkCommandBufferAllocateInfo allocate{VK_STRUCTURE_TYPE_COMMAND_BUFFER_ALLOCATE_INFO}; allocate.commandPool = context.pool; allocate.level = VK_COMMAND_BUFFER_LEVEL_PRIMARY; - allocate.commandBufferCount = 1; - if ((probe.result = context.Function("vkAllocateCommandBuffers")(context.device, &allocate, &commands)) != VK_SUCCESS) return "vkAllocateCommandBuffers"; + allocate.commandBufferCount = 2; + if ((probe.result = context.Function("vkAllocateCommandBuffers")(context.device, &allocate, commands)) != VK_SUCCESS) return "vkAllocateCommandBuffers"; VkCommandBufferBeginInfo begin{VK_STRUCTURE_TYPE_COMMAND_BUFFER_BEGIN_INFO}; begin.flags = VK_COMMAND_BUFFER_USAGE_ONE_TIME_SUBMIT_BIT; - if ((probe.result = context.Function("vkBeginCommandBuffer")(commands, &begin)) != VK_SUCCESS) return "vkBeginCommandBuffer"; + if ((probe.result = context.Function("vkBeginCommandBuffer")(commands[0], &begin)) != VK_SUCCESS) return "vkBeginCommandBuffer"; const VkBufferCopy region{0, 0, bytes}; - context.Function("vkCmdCopyBuffer")(commands, import.buffer, destination, 1, ®ion); - if ((probe.result = context.Function("vkEndCommandBuffer")(commands)) != VK_SUCCESS) return "vkEndCommandBuffer"; + context.Function("vkCmdCopyBuffer")(commands[0], import.buffer, destination, 1, ®ion); + if ((probe.result = context.Function("vkEndCommandBuffer")(commands[0])) != VK_SUCCESS) return "vkEndCommandBuffer"; + if ((probe.result = context.Function("vkBeginCommandBuffer")(commands[1], &begin)) != VK_SUCCESS) return "vkBeginCommandBuffer"; + context.Function("vkCmdFillBuffer")(commands[1], import.buffer, 0, bytes, 0x02020202u); + if ((probe.result = context.Function("vkEndCommandBuffer")(commands[1])) != VK_SUCCESS) return "vkEndCommandBuffer"; const VkFenceCreateInfo fenceInfo{VK_STRUCTURE_TYPE_FENCE_CREATE_INFO}; if ((probe.result = context.Function("vkCreateFence")(context.device, &fenceInfo, nullptr, &fence)) != VK_SUCCESS) return "vkCreateFence"; - VkSubmitInfo submit{VK_STRUCTURE_TYPE_SUBMIT_INFO}; - submit.commandBufferCount = 1; - submit.pCommandBuffers = &commands; - if ((probe.result = context.Function("vkQueueSubmit")(context.queue, 1, &submit, fence)) != VK_SUCCESS) return "vkQueueSubmit"; - submitted = true; - if ((probe.result = context.Function("vkWaitForFences")(context.device, 1, &fence, VK_TRUE, 10'000'000'000ull)) != VK_SUCCESS) return "vkWaitForFences"; - submitted = false; + const auto execute = [&](VkCommandBuffer buffer) -> const char* { + if ((probe.result = context.Function("vkResetFences")(context.device, 1, &fence)) != VK_SUCCESS) return "vkResetFences"; + VkSubmitInfo submit{VK_STRUCTURE_TYPE_SUBMIT_INFO}; + submit.commandBufferCount = 1; + submit.pCommandBuffers = &buffer; + if ((probe.result = context.Function("vkQueueSubmit")(context.queue, 1, &submit, fence)) != VK_SUCCESS) return "vkQueueSubmit"; + submitted = true; + if ((probe.result = context.Function("vkWaitForFences")(context.device, 1, &fence, VK_TRUE, 10'000'000'000ull)) != VK_SUCCESS) return "vkWaitForFences"; + submitted = false; + return nullptr; + }; + if (const char* step = execute(commands[0])) return step; if (!collect(probe.writtenAfterSubmit)) return "the collect after the submission"; + if (const char* step = execute(commands[1])) return step; + if (!collect(probe.writtenAfterFill)) return "the collect after the fill"; *reinterpret_cast(base) = 2; if (!collect(probe.writtenByCpu)) return "the collect after a CPU store"; return nullptr; @@ -1565,7 +1575,7 @@ void runImportProbe(const Context& context, std::uint64_t base, std::uint64_t by probe.failure = run(); if (submitted) context.Function("vkQueueWaitIdle")(context.queue); if (fence != VK_NULL_HANDLE) context.Function("vkDestroyFence")(context.device, fence, nullptr); - if (commands != VK_NULL_HANDLE) context.Function("vkFreeCommandBuffers")(context.device, context.pool, 1, &commands); + if (commands[0] != VK_NULL_HANDLE) context.Function("vkFreeCommandBuffers")(context.device, context.pool, 2, commands); if (destination != VK_NULL_HANDLE) context.Function("vkDestroyBuffer")(context.device, destination, nullptr); if (destinationMemory != VK_NULL_HANDLE) context.Function("vkFreeMemory")(context.device, destinationMemory, nullptr); if (import.buffer != VK_NULL_HANDLE) destroyImport(context, import); diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index cc3c4bff40..adb35a6d7d 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -2294,7 +2294,7 @@ void importWatchTests(const Device& device) { } const auto probe = ProbeImportWriteProtection(context); Require(probe.failure == nullptr, std::string("(u) the import probe failed at ") + (probe.failure != nullptr ? probe.failure : "") + " (" + std::to_string(static_cast(probe.result)) + ")"); - std::cout << "import probe: " << probe.writtenAfterSubmit << " of " << probe.pages << " scratch pages written after a GPU read, " << probe.writtenAtImport << " after the import\n"; + std::cout << "import probe: " << probe.writtenAfterSubmit << " of " << probe.pages << " scratch pages written after a GPU read, " << probe.writtenAfterFill << " after a GPU write, " << probe.writtenAtImport << " after the import\n"; Require(probe.writtenByCpu != 0, "(u) a CPU store after the import probe's GPU read was not collected"); if (context.dmaBufImport) { const auto dmaBuf = ProbeDmaBufImportWriteProtection(context);