From ff349a9a50c9c241706d427dedcaaa3d54c61469 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Mon, 12 Oct 2026 06:02:08 +0200 Subject: [PATCH 1/4] test(agc): a host import dropped mid-hold must outlive the hold's later batches Two deterministic, GPU-free checks (mock device) of the import lifetime when refreshImports drops an import whose registered run grew, while the GpuMutex hold that took the import may still record with it: with the recorder idle (the import was destroyed at once) and with a Submit between the keeping batch and the batch that records the use (the import was destroyed when the keeping batch completed, with the later batch still in flight). Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 14 + .../tests/HostImportRetireHold.cpp | 348 ++++++++++++++++++ 2 files changed, 362 insertions(+) create mode 100644 core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 6c94ffc4b..561aa3b03 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -841,6 +841,20 @@ if(BUILD_TESTING) endif() endif() +add_test_executable(agc_host_import_retire_tests EXCLUDE_FROM_ALL tests/HostImportRetireHold.cpp) +target_include_directories(agc_host_import_retire_tests PRIVATE ${LIBS_INCLUDE_DIR} ${CMAKE_SOURCE_DIR}/core/shader/recompiler ${ANYPS5_VULKAN_HEADERS_INCLUDE_DIR}) +target_link_libraries(agc_host_import_retire_tests PRIVATE libSceAgcDriver libc) +set_target_properties(agc_host_import_retire_tests PROPERTIES CXX_EXTENSIONS OFF) +configure_windows_unwind(agc_host_import_retire_tests) +if(BUILD_TESTING) + set_target_properties(agc_host_import_retire_tests PROPERTIES EXCLUDE_FROM_ALL OFF) + add_test(NAME agc_host_import_retire COMMAND agc_host_import_retire_tests) + if(WIN32) + set_tests_properties(agc_host_import_retire PROPERTIES ENVIRONMENT_MODIFICATION + "PATH=path_list_prepend:$") + endif() +endif() + add_test_executable(agc_driver_pm4_tests EXCLUDE_FROM_ALL tests/Pm4.cpp ) diff --git a/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp b/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp new file mode 100644 index 000000000..26ed7d2f6 --- /dev/null +++ b/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp @@ -0,0 +1,348 @@ +#include "prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp" +#include "prx/libSceAgcDriver/Graphics/include/Recorder.hpp" +#include "prx/libSceAgcDriver/Execution/include/GuestMemory.hpp" +#include "prx/libc/include/GuestAllocations.hpp" +#ifdef _WIN32 +#include +#endif +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +using namespace AgcDriver::Graphics; + +namespace { + +int failures = 0; + +void Expect(bool condition, const std::string& what) { + if (condition) return; + std::fprintf(stderr, "FAIL: %s\n", what.c_str()); + ++failures; +} + +constexpr std::size_t Alignment = 65536; +constexpr std::size_t RangeBytes = std::size_t{1} << 20u; + +struct MockDevice { + std::uintptr_t next = 0x1000; + std::map signaled; + std::vector submitted; + std::set buffers; + std::map sizes; + std::set memories; + std::uint64_t nextAddress = 0x500000000ull; +}; + +MockDevice mock; + +bool Live(VkBuffer buffer) { + return mock.buffers.contains(buffer); +} + +VKAPI_ATTR VkResult VKAPI_CALL mockAllocateCommandBuffers(VkDevice, const VkCommandBufferAllocateInfo*, VkCommandBuffer* commands) { + *commands = reinterpret_cast(mock.next++); + return VK_SUCCESS; +} + +VKAPI_ATTR void VKAPI_CALL mockFreeCommandBuffers(VkDevice, VkCommandPool, std::uint32_t, const VkCommandBuffer*) {} + +VKAPI_ATTR VkResult VKAPI_CALL mockCreateFence(VkDevice, const VkFenceCreateInfo*, const VkAllocationCallbacks*, VkFence* fence) { + *fence = reinterpret_cast(mock.next++); + mock.signaled[*fence] = false; + return VK_SUCCESS; +} + +VKAPI_ATTR void VKAPI_CALL mockDestroyFence(VkDevice, VkFence fence, const VkAllocationCallbacks*) { + mock.signaled.erase(fence); +} + +VKAPI_ATTR VkResult VKAPI_CALL mockGetFenceStatus(VkDevice, VkFence fence) { + return mock.signaled.at(fence) ? VK_SUCCESS : VK_NOT_READY; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockWaitForFences(VkDevice, std::uint32_t count, const VkFence* fences, VkBool32, std::uint64_t) { + for (std::uint32_t i = 0; i < count; ++i) mock.signaled.at(fences[i]) = true; + return VK_SUCCESS; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockResetFences(VkDevice, std::uint32_t count, const VkFence* fences) { + for (std::uint32_t i = 0; i < count; ++i) mock.signaled.at(fences[i]) = false; + return VK_SUCCESS; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockBeginCommandBuffer(VkCommandBuffer, const VkCommandBufferBeginInfo*) { + return VK_SUCCESS; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockEndCommandBuffer(VkCommandBuffer) { + return VK_SUCCESS; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockQueueSubmit(VkQueue, std::uint32_t, const VkSubmitInfo*, VkFence fence) { + mock.submitted.push_back(fence); + return VK_SUCCESS; +} + +VKAPI_ATTR void VKAPI_CALL mockCmdPipelineBarrier(VkCommandBuffer, VkPipelineStageFlags, VkPipelineStageFlags, VkDependencyFlags, std::uint32_t, const VkMemoryBarrier*, std::uint32_t, const VkBufferMemoryBarrier*, std::uint32_t, const VkImageMemoryBarrier*) {} +VKAPI_ATTR void VKAPI_CALL mockCmdUpdateBuffer(VkCommandBuffer, VkBuffer, VkDeviceSize, VkDeviceSize, const void*) {} +VKAPI_ATTR void VKAPI_CALL mockCmdBeginQuery(VkCommandBuffer, VkQueryPool, std::uint32_t, VkQueryControlFlags) {} +VKAPI_ATTR void VKAPI_CALL mockCmdEndQuery(VkCommandBuffer, VkQueryPool, std::uint32_t) {} +VKAPI_ATTR void VKAPI_CALL mockCmdResetQueryPool(VkCommandBuffer, VkQueryPool, std::uint32_t, std::uint32_t) {} +VKAPI_ATTR void VKAPI_CALL mockCmdCopyQueryPoolResults(VkCommandBuffer, VkQueryPool, std::uint32_t, std::uint32_t, VkBuffer, VkDeviceSize, VkDeviceSize, VkQueryResultFlags) {} +VKAPI_ATTR void VKAPI_CALL mockCmdBindPipeline(VkCommandBuffer, VkPipelineBindPoint, VkPipeline) {} +VKAPI_ATTR void VKAPI_CALL mockCmdPushConstants(VkCommandBuffer, VkPipelineLayout, VkShaderStageFlags, std::uint32_t, std::uint32_t, const void*) {} +VKAPI_ATTR void VKAPI_CALL mockCmdDispatch(VkCommandBuffer, std::uint32_t, std::uint32_t, std::uint32_t) {} + +VKAPI_ATTR VkResult VKAPI_CALL mockGetMemoryHostPointerProperties(VkDevice, VkExternalMemoryHandleTypeFlagBits, const void*, VkMemoryHostPointerPropertiesEXT* properties) { + properties->memoryTypeBits = 1; + return VK_SUCCESS; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockCreateBuffer(VkDevice, const VkBufferCreateInfo* info, const VkAllocationCallbacks*, VkBuffer* buffer) { + *buffer = reinterpret_cast(mock.next++); + mock.buffers.insert(*buffer); + mock.sizes[*buffer] = info->size; + return VK_SUCCESS; +} + +VKAPI_ATTR void VKAPI_CALL mockDestroyBuffer(VkDevice, VkBuffer buffer, const VkAllocationCallbacks*) { + mock.buffers.erase(buffer); +} + +VKAPI_ATTR void VKAPI_CALL mockGetBufferMemoryRequirements(VkDevice, VkBuffer buffer, VkMemoryRequirements* requirements) { + requirements->size = mock.sizes.at(buffer); + requirements->alignment = Alignment; + requirements->memoryTypeBits = 1; +} + +VKAPI_ATTR VkResult VKAPI_CALL mockAllocateMemory(VkDevice, const VkMemoryAllocateInfo*, const VkAllocationCallbacks*, VkDeviceMemory* memory) { + *memory = reinterpret_cast(mock.next++); + mock.memories.insert(*memory); + return VK_SUCCESS; +} + +VKAPI_ATTR void VKAPI_CALL mockFreeMemory(VkDevice, VkDeviceMemory memory, const VkAllocationCallbacks*) { + mock.memories.erase(memory); +} + +VKAPI_ATTR VkResult VKAPI_CALL mockBindBufferMemory(VkDevice, VkBuffer, VkDeviceMemory, VkDeviceSize) { + return VK_SUCCESS; +} + +VKAPI_ATTR VkDeviceAddress VKAPI_CALL mockGetBufferDeviceAddress(VkDevice, const VkBufferDeviceAddressInfo* info) { + const auto address = mock.nextAddress; + mock.nextAddress += (mock.sizes.at(info->buffer) + Alignment - 1) / Alignment * Alignment; + return address; +} + +PFN_vkVoidFunction VKAPI_CALL mockProc(VkDevice, const char* name) { + static const std::map table{ + {"vkAllocateCommandBuffers", reinterpret_cast(mockAllocateCommandBuffers)}, + {"vkFreeCommandBuffers", reinterpret_cast(mockFreeCommandBuffers)}, + {"vkCreateFence", reinterpret_cast(mockCreateFence)}, + {"vkDestroyFence", reinterpret_cast(mockDestroyFence)}, + {"vkGetFenceStatus", reinterpret_cast(mockGetFenceStatus)}, + {"vkWaitForFences", reinterpret_cast(mockWaitForFences)}, + {"vkResetFences", reinterpret_cast(mockResetFences)}, + {"vkBeginCommandBuffer", reinterpret_cast(mockBeginCommandBuffer)}, + {"vkEndCommandBuffer", reinterpret_cast(mockEndCommandBuffer)}, + {"vkQueueSubmit", reinterpret_cast(mockQueueSubmit)}, + {"vkCmdPipelineBarrier", reinterpret_cast(mockCmdPipelineBarrier)}, + {"vkCmdUpdateBuffer", reinterpret_cast(mockCmdUpdateBuffer)}, + {"vkCmdBeginQuery", reinterpret_cast(mockCmdBeginQuery)}, + {"vkCmdEndQuery", reinterpret_cast(mockCmdEndQuery)}, + {"vkCmdResetQueryPool", reinterpret_cast(mockCmdResetQueryPool)}, + {"vkCmdCopyQueryPoolResults", reinterpret_cast(mockCmdCopyQueryPoolResults)}, + {"vkCmdBindPipeline", reinterpret_cast(mockCmdBindPipeline)}, + {"vkCmdPushConstants", reinterpret_cast(mockCmdPushConstants)}, + {"vkCmdDispatch", reinterpret_cast(mockCmdDispatch)}, + {"vkGetMemoryHostPointerPropertiesEXT", reinterpret_cast(mockGetMemoryHostPointerProperties)}, + {"vkCreateBuffer", reinterpret_cast(mockCreateBuffer)}, + {"vkDestroyBuffer", reinterpret_cast(mockDestroyBuffer)}, + {"vkGetBufferMemoryRequirements", reinterpret_cast(mockGetBufferMemoryRequirements)}, + {"vkAllocateMemory", reinterpret_cast(mockAllocateMemory)}, + {"vkFreeMemory", reinterpret_cast(mockFreeMemory)}, + {"vkBindBufferMemory", reinterpret_cast(mockBindBufferMemory)}, + {"vkGetBufferDeviceAddressKHR", reinterpret_cast(mockGetBufferDeviceAddress)}, + }; + const auto it = table.find(name); + return it == table.end() ? nullptr : it->second; +} + +Context mockContext() { + static const auto device = reinterpret_cast(mock.next++); + static const auto queue = reinterpret_cast(mock.next++); + Context context{}; + context.device = device; + context.queue = queue; + context.deviceProc = mockProc; + context.hostImportAlignment = Alignment; + return context; +} + +struct Heap { + std::byte* base = nullptr; + bool first = false; + bool second = false; + + Heap() { +#ifdef _WIN32 + base = static_cast(VirtualAlloc(nullptr, 2 * RangeBytes, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE)); +#else + base = static_cast(std::aligned_alloc(Alignment, 2 * RangeBytes)); +#endif + if (base == nullptr) throw std::runtime_error("cannot allocate the test heap"); + GuestAllocations::Mutation mutation; + mutation.Add(base, RangeBytes, true, true, true); + first = true; + } + + void Grow() { + GuestAllocations::Mutation mutation; + mutation.Add(base + RangeBytes, RangeBytes, true, true, true); + second = true; + } + + std::uint64_t Address(std::size_t offset = 0) const { + return reinterpret_cast(base) + offset; + } + + ~Heap() { + try { + GuestAllocations::Mutation mutation; + if (second) mutation.Remove(base + RangeBytes); + if (first) mutation.Remove(base); + } catch (...) { + } +#ifdef _WIN32 + VirtualFree(base, 0, MEM_RELEASE); +#else + std::free(base); +#endif + } +}; + +struct ActiveRecorder { + std::unique_ptr recorder; + + ActiveRecorder() { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + recorder = std::make_unique(mockContext()); + recorder->Activate(); + } + + ~ActiveRecorder() { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + recorder.reset(); + } +}; + +void IdleRetireOutlivesTheHold() { + Heap heap; + ActiveRecorder active; + auto& recorder = *active.recorder; + const auto context = mockContext(); + VkBuffer retired = VK_NULL_HANDLE; + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + const auto* taken = HostImportFor(context, heap.Address(), 4096); + Expect(taken != nullptr, "the first range was not imported"); + if (taken == nullptr) return; + retired = taken->buffer; + Expect(recorder.Idle(), "the recorder is not idle before the drop"); + heap.Grow(); + const auto* grown = HostImportFor(context, heap.Address(RangeBytes), 4096); + Expect(grown != nullptr && grown->buffer != retired && grown->base == heap.Address() && grown->bytes == 2 * RangeBytes, "the grown run was not imported as one import replacing the first"); + Expect(Live(retired), "an import dropped with the recorder idle was destroyed at once, while the hold that took it can still record with it"); + if (!Live(retired)) return; + Expect(taken->buffer == retired && taken->base == heap.Address(), "the dropped import's entry changed under the hold that took it"); + recorder.Commands(); + recorder.Submit(); + recorder.Sync(); + Expect(Live(retired), "an import dropped in this hold was destroyed by a Sync inside the same hold"); + recorder.Commands(); + recorder.Submit(); + Expect(Live(retired), "an import dropped in this hold was destroyed while a batch the hold recorded afterwards is in flight"); + } + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + Expect(Live(retired), "an import dropped in a hold was destroyed at the hold's unlock while a batch of that hold is in flight"); + } + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + recorder.Sync(); + } + Expect(!Live(retired), "the dropped import outlived every batch of its hold"); +} + +void BusyRetireOutlivesALaterBatch() { + Heap heap; + ActiveRecorder active; + auto& recorder = *active.recorder; + const auto context = mockContext(); + VkBuffer retired = VK_NULL_HANDLE; + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + recorder.Keep(std::make_shared(0)); + const auto* taken = HostImportFor(context, heap.Address(), 4096); + Expect(taken != nullptr, "the first range was not imported"); + if (taken == nullptr) return; + retired = taken->buffer; + heap.Grow(); + Expect(HostImportFor(context, heap.Address(RangeBytes), 4096) != nullptr, "the grown run was not imported"); + mock.submitted.clear(); + recorder.Submit(); + recorder.Commands(); + recorder.Submit(); + Expect(mock.submitted.size() == 2, "the hold did not submit the keeping batch and the later one apart"); + if (mock.submitted.size() != 2) return; + mock.signaled.at(mock.submitted[0]) = true; + recorder.Reap(); + Expect(Live(retired), "the dropped import was destroyed when the batch open at the drop completed, while a later batch of the hold that may use it is in flight"); + } + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + Expect(Live(retired), "the dropped import was destroyed at the hold's unlock, while a later batch of the hold that may use it is in flight"); + mock.signaled.at(mock.submitted[1]) = true; + recorder.Reap(); + } + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + recorder.Commands(); + recorder.Sync(); + } + Expect(!Live(retired), "the dropped import outlived every batch of its hold"); +} + +} + +int main() { +#ifdef _WIN32 + _putenv_s("APS5_RELEASE_ON_UNLOCK", "1"); +#else + setenv("APS5_RELEASE_ON_UNLOCK", "1", 1); +#endif + try { + IdleRetireOutlivesTheHold(); + BusyRetireOutlivesALaterBatch(); + } catch (const std::exception& error) { + std::fprintf(stderr, "%s\n", error.what()); + return 1; + } + if (failures != 0) { + std::fprintf(stderr, "%d host import retire checks failed\n", failures); + return 1; + } + std::printf("host import retire tests passed\n"); + return 0; +} From e785a902b5b9c0a8b6b5e7207ff8bb985345cdb3 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Mon, 12 Oct 2026 06:02:55 +0200 Subject: [PATCH 2/4] fix(agc): keep a host import dropped mid-hold for every batch that hold opens refreshImports and importAllocation drop an import whose registered run changed (a title growing a heap) while the GpuMutex hold that drops it may still hold a HostImport pointer from before: a build's in-place region (UploadFinish flushes pending images between regions, and a flush may reconcile the imports), an indirect argument or a vertex import. The holder went to the batch open at the drop, or was destroyed at once with the recorder idle, and the map node the pointer names was erased. A Submit or Sync later in the hold put the command using the old buffer or address into a later batch, so the import could be freed before that batch ran and the GPU read an unmapped page (VK_ERROR_DEVICE_LOST; VK_EXT_device_fault reports READ_INVALID inside the dropped import). The dropped import's map node now moves into its holder (std::map::extract), so those pointers stay valid, and the holder is kept by every batch the same hold opens (Recorder::KeepThroughHold), so whatever the hold records with them completes first. Drops for pages the guest releases (the lease waiter) keep the old lifetime, and the waiter lets go of what an ended hold kept so it does not pin the ranges. Co-Authored-By: Claude Opus 5.5 --- .../Execution/include/GuestMemory.hpp | 1 + .../Execution/src/GuestMemory.cpp | 8 ++++- .../Graphics/include/Recorder.hpp | 4 +++ .../Graphics/src/GuestBufferMemory.cpp | 35 ++++++++++--------- .../libSceAgcDriver/Graphics/src/Recorder.cpp | 20 +++++++++++ 5 files changed, 51 insertions(+), 17 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Execution/include/GuestMemory.hpp b/core/libs/prx/libSceAgcDriver/Execution/include/GuestMemory.hpp index 5321424ce..567e7c9f2 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/include/GuestMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Execution/include/GuestMemory.hpp @@ -149,6 +149,7 @@ GpuMutexType& GpuMutex(); // work the hold deferred to run without it (the recorder's release of the objects completed batches // kept). One hook, set once; it must not take GpuMutex itself. void SetGpuUnlockHook(void (*hook)()); +std::uint64_t GpuHoldSerial(); // Debug aid (APS5_ASSERT_GPU_LOCK=1): aborts with a [lock] line naming `where` when the calling // thread does not hold GpuMutex. Recording, submits, command batches and the detiler's pools must // only be reached under it (a resource build's stage A runs without it, see ShaderResources). diff --git a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp index de2fb32eb..d3bf0bc94 100644 --- a/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Execution/src/GuestMemory.cpp @@ -1550,11 +1550,13 @@ const void* ThreadToken() { thread_local char token; return &token; } + +std::atomic gpuHoldSerial{0}; } void GpuMutexType::acquired() { owner.store(ThreadToken(), std::memory_order_relaxed); - ++depth; + if (++depth == 1) gpuHoldSerial.fetch_add(1, std::memory_order_relaxed); } bool GpuMutexType::try_lock() { @@ -1585,6 +1587,10 @@ void SetGpuUnlockHook(void (*hook)()) { gpuUnlockHook.store(hook, std::memory_order_release); } +std::uint64_t GpuHoldSerial() { + return gpuHoldSerial.load(std::memory_order_relaxed); +} + void GpuMutexType::unlock() { const bool outermost = --depth == 0; if (outermost) { diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp index 2954c20f7..7e5defb33 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/Recorder.hpp @@ -86,6 +86,7 @@ public: // Whether recorded work still has completion actions (write-backs the CPU must see) to run. bool HasCompletions() const; void Keep(std::shared_ptr object, std::size_t bytes = 0); + void KeepThroughHold(std::shared_ptr object); void KeepBytes(const void* owner, std::size_t bytes); std::size_t OpenKeptBytes() const { return open != nullptr ? open->keptBytes : 0; } void FlushDeferredReleases(); @@ -715,6 +716,7 @@ private: void recordBarrier(VkCommandBuffer commands, VkPipelineStageFlags sourceStage, VkPipelineStageFlags destinationStage, VkAccessFlags sourceAccess, VkAccessFlags destinationAccess) const; // Opens a batch when none is open, without closing a store run (Commands() does both). void ensureOpen(); + void dropStaleHoldKept(); // Records the run's pending store (vkCmdUpdateBuffer), if any. void flushPendingStore(); // Records the pending store and the run's trailing barrier; the next store starts a new run. @@ -782,6 +784,8 @@ private: // table is empty (most of the time between label groups). std::atomic recordedLabels{0}; std::unique_ptr open; + std::vector> holdKept; + std::uint64_t holdKeptSerial = 0; std::deque> inFlight; // Batches popped from inFlight whose completions have not run yet: their writes stay in the // snapshot (a rebuild from inside a completion must not drop them) until finish returns. diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 73e1015e5..005436362 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -181,14 +181,17 @@ void destroyImport(const Context& context, const HostImport& entry) { #endif } +using ImportNode = std::map::node_type; + // Frees a dropped import's Vulkan objects when it is released. Never copied: a copy would destroy the // same handles twice (and a temporary would destroy them at once). struct RetiredImport { - RetiredImport(VkDevice device, PFN_vkDestroyBuffer destroyBuffer, PFN_vkFreeMemory freeMemory, const HostImport& entry) - : device(device), destroyBuffer(destroyBuffer), freeMemory(freeMemory), entry(entry) {} + RetiredImport(VkDevice device, PFN_vkDestroyBuffer destroyBuffer, PFN_vkFreeMemory freeMemory, ImportNode node) + : device(device), destroyBuffer(destroyBuffer), freeMemory(freeMemory), node(std::move(node)) {} RetiredImport(const RetiredImport&) = delete; RetiredImport& operator=(const RetiredImport&) = delete; ~RetiredImport() { + const auto& entry = node.mapped(); destroyBuffer(device, entry.buffer, nullptr); freeMemory(device, entry.memory, nullptr); #ifdef _WIN32 @@ -198,25 +201,24 @@ struct RetiredImport { VkDevice device; PFN_vkDestroyBuffer destroyBuffer; PFN_vkFreeMemory freeMemory; - HostImport entry; + ImportNode node; }; -// Drops an import. Recorded batches (dispatches, recorded draws, GPU label writes) may still read it, -// so the objects live until the batch open now completed; with no batches in flight all GPU work that -// used it was synchronous and it is destroyed at once. const GuestAllocations::Range* containingRange(const GuestAllocations::Lease& lease, std::uint64_t begin, std::uint64_t end); bool runCovers(const GuestAllocations::Lease& lease, std::uint64_t begin, std::uint64_t end); -void retireImport(VkDevice device, HostImports& state, std::map::iterator it, const GuestAllocations::Lease& lease) { +void retireImport(VkDevice device, HostImports& state, std::map::iterator it, const GuestAllocations::Lease& lease, bool hold = true) { // Results shadowed for the import reach its (old) buffer first where the memory is still a // readable registered range (the import retires because its registration vanished or changed // size); the holder below outlives the batch that copies them. RetireShadow(device, it->second, [&lease](std::uint64_t begin, std::uint64_t end) { return runCovers(lease, begin, end); }); - auto holder = std::make_shared(state.device, state.destroyBuffer, state.freeMemory, it->second); - if (auto* recorder = Recorder::Active(); recorder != nullptr && !recorder->Idle()) recorder->Keep(std::move(holder)); ++state.epoch; state.liveBytes -= it->second.bytes; - state.imports.erase(it); + auto holder = std::make_shared(state.device, state.destroyBuffer, state.freeMemory, state.imports.extract(it)); + auto* recorder = Recorder::Active(); + if (recorder == nullptr) return; + if (hold) recorder->KeepThroughHold(std::move(holder)); + else if (!recorder->Idle()) recorder->Keep(std::move(holder)); } const char* bindImport(const Context& context, HostImport& entry, VkExternalMemoryHandleTypeFlagBits handleType, const void* import, std::uint32_t importTypes, bool& allocated, VkResult& failure) { @@ -764,16 +766,16 @@ const GuestAllocations::Range* containingRange(const GuestAllocations::Lease& le // guest addresses. Walks the imports only when the registry changed since the last walk. void refreshImports(const Context& context, HostImports& state, const GuestAllocations::Lease& lease) { if (state.device != context.device) { - for (const auto& [address, entry] : state.imports) { + while (!state.imports.empty()) { + auto node = state.imports.extract(state.imports.begin()); if (state.device != VK_NULL_HANDLE && state.destroyBuffer != nullptr && state.freeMemory != nullptr) { - state.parked[state.device].push_back(std::make_shared(state.device, state.destroyBuffer, state.freeMemory, entry)); + state.parked[state.device].push_back(std::make_shared(state.device, state.destroyBuffer, state.freeMemory, std::move(node))); continue; } #ifdef _WIN32 - GuestArena::GuestArenaUnmapAlias_nid_postfix(entry.alias); + GuestArena::GuestArenaUnmapAlias_nid_postfix(node.mapped().alias); #endif } - state.imports.clear(); state.liveBytes = 0; state.failed.clear(); state.device = context.device; @@ -1544,7 +1546,7 @@ bool WaitForLeasesAndImports(std::uintptr_t address, std::size_t bytes) noexcept continue; } const auto next = std::next(it); - retireImport(imports.device, imports, it, lease); + retireImport(imports.device, imports, it, lease, false); it = next; retiredImport = true; } @@ -1582,6 +1584,7 @@ bool WaitForLeasesAndImports(std::uintptr_t address, std::size_t bytes) noexcept // yet to record (a test's, or a build whose worker gave up the lock); give it time. std::this_thread::yield(); } + if (recorder != nullptr) recorder->FlushDeferredReleases(); const auto end = static_cast(address) + bytes; auto& imports = Imports(); @@ -1612,7 +1615,7 @@ bool WaitForLeasesAndImports(std::uintptr_t address, std::size_t bytes) noexcept continue; } const auto next = std::next(it); - retireImport(imports.device, imports, it, lease); + retireImport(imports.device, imports, it, lease, false); it = next; retiredImport = true; } diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp index cfcb1c010..ea8d595f5 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/Recorder.cpp @@ -1117,6 +1117,7 @@ Recorder::~Recorder() { } catch (const std::exception& error) { std::fprintf(stderr, "[gpu] recorder teardown: %s\n", error.what()); } + holdKept.clear(); if (activeRecorder == this) { activeRecorder = nullptr; pendingWrites.store(nullptr, std::memory_order_release); @@ -1516,6 +1517,8 @@ void Recorder::ensureOpen() { } batch->queue = GuestMemory::GpuLockThreadTag(); if (countingSamples && !segmentMode()) beginSamples(*batch); + dropStaleHoldKept(); + batch->kept.insert(batch->kept.end(), holdKept.begin(), holdKept.end()); open = std::move(batch); // The whole-batch range: its start stamp waits for the previous batches like any // bottom-of-pipe stamp, so it marks when this batch's execution began. @@ -2452,6 +2455,20 @@ void Recorder::Keep(std::shared_ptr object, std::size_t bytes) { open->keptBytes += bytes; } +void Recorder::KeepThroughHold(std::shared_ptr object) { + GuestMemory::AssertGpuLockHeld("Recorder::KeepThroughHold"); + if (const auto hold = GuestMemory::GpuHoldSerial(); hold != holdKeptSerial) { + holdKept.clear(); + holdKeptSerial = hold; + } + Keep(object); + holdKept.push_back(std::move(object)); +} + +void Recorder::dropStaleHoldKept() { + if (!holdKept.empty() && holdKeptSerial != GuestMemory::GpuHoldSerial() && GuestMemory::GpuMutex().HeldByThisThread()) holdKept.clear(); +} + void Recorder::KeepBytes(const void* owner, std::size_t bytes) { if (owner == nullptr || bytes == 0) return; ensureOpen(); @@ -3116,6 +3133,7 @@ void Recorder::Submit() { // count behind, and the callers that act on it would otherwise take the mutex for nothing at // every packet or submission end until a real batch is next submitted. if (activeRecorder == this) workSinceSubmit.store(0, std::memory_order_relaxed); + dropStaleHoldKept(); if (open == nullptr) return; GuestMemory::AssertGpuLockHeld("Recorder::Submit"); if (!open->keyStores.empty()) recordKeyStores(false); @@ -3270,6 +3288,7 @@ void Recorder::Sync() { } void Recorder::FlushDeferredReleases() { + holdKept.clear(); ReleaseDeferredKeeps(); } @@ -3532,6 +3551,7 @@ bool Recorder::syncThroughUnlocked(std::uint64_t address, std::uint64_t end, int bool Recorder::Reap() { PerformanceTimer frameTiming("Recorder.Reap"); + dropStaleHoldKept(); const bool profile = DrawProfiled(); const auto start = profile ? std::chrono::steady_clock::now() : std::chrono::steady_clock::time_point{}; std::uint64_t retired = 0; From eaf5ca240f8cdbd826f7627d8a80df4106bbed68 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:10:53 +0200 Subject: [PATCH 3/4] fix(agc): read a parked import through its holder's map node RetiredImport now owns the dropped import's map node (std::map::extract), so releaseParkedImports takes the parked import's range from node.mapped(). Co-Authored-By: Claude Opus 5.5 --- .../libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 0230bfbb4..880720c82 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -234,7 +234,7 @@ bool releaseParkedImports(HostImports& state, std::uint64_t begin, std::uint64_t for (auto device = state.parked.begin(); device != state.parked.end();) { auto& list = device->second; for (auto it = list.begin(); it != list.end();) { - if ((*it)->entry.base < end && begin < (*it)->entry.base + (*it)->entry.bytes) { + if ((*it)->node.mapped().base < end && begin < (*it)->node.mapped().base + (*it)->node.mapped().bytes) { released.push_back(std::move(*it)); it = list.erase(it); } else { From 63ddeb09f47b78f3a614fb4451ffe93ada4592c9 Mon Sep 17 00:00:00 2001 From: AlxcBTW <187763517+AlxcBTW@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:00:45 +0200 Subject: [PATCH 4/4] test(agc): keep the retire test's mock device out of the Linux import probe On Linux with write watching (uffd write-protect), the first import asks the device whether a GPU write clears the protection (ProbeImportWriteProtection): it records a vkCmdCopyBuffer and a vkCmdFillBuffer on the import and submits them. The test's mock device records no commands, so agc_host_import_retire failed with "missing Vulkan function: vkCmdCopyBuffer" before it reached the retire (RADV, reported on #4515). The test now sets APS5_WRITE_WATCH_IMPORTS=unwatch, so its imports are compared as on Windows and the probe, which a mock cannot answer, does not run. Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp b/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp index 26ed7d2f6..d1ce27fe5 100644 --- a/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/HostImportRetireHold.cpp @@ -331,6 +331,7 @@ int main() { _putenv_s("APS5_RELEASE_ON_UNLOCK", "1"); #else setenv("APS5_RELEASE_ON_UNLOCK", "1", 1); + setenv("APS5_WRITE_WATCH_IMPORTS", "unwatch", 1); #endif try { IdleRetireOutlivesTheHold();