From cd067d1569cfee95097399723f8a011337a512d4 Mon Sep 17 00:00:00 2001 From: Diversao <83223857+diversao@users.noreply.github.com> Date: Mon, 12 Oct 2026 02:00:48 -0300 Subject: [PATCH] fix(agc): release parked host imports a pinned guest range waits for A device replaced at AttachWindow keeps its host imports in HostImports::parked until it is destroyed, and each parked import keeps its guest ranges pinned. A guest munmap, mprotect or free of such a range spun in the pin waiter until APS5_PIN_WAIT_MS and then failed, because the waiter only looked at the live imports. The replaced device was already waited idle by PrepareForReplacement, so the waiter now drops the parked imports over the range. agc_parked_import_unmap imports a range on one device, again on a second one, and unmaps it with a 3 s pin wait. Co-Authored-By: Claude Opus 5.5 --- core/libs/prx/libSceAgcDriver/CMakeLists.txt | 4 +- .../Graphics/src/GuestBufferMemory.cpp | 22 ++++++ .../prx/libSceAgcDriver/tests/Recorder.cpp | 70 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/core/libs/prx/libSceAgcDriver/CMakeLists.txt b/core/libs/prx/libSceAgcDriver/CMakeLists.txt index 6c94ffc4b..26b631e3e 100644 --- a/core/libs/prx/libSceAgcDriver/CMakeLists.txt +++ b/core/libs/prx/libSceAgcDriver/CMakeLists.txt @@ -811,6 +811,8 @@ if(BUILD_TESTING) endif() add_test(NAME agc_host_import_unmap COMMAND agc_driver_recorder_tests --host-import-unmap-only) set_tests_properties(agc_host_import_unmap PROPERTIES SKIP_RETURN_CODE 77) + add_test(NAME agc_parked_import_unmap COMMAND agc_driver_recorder_tests --parked-import-unmap-only) + set_tests_properties(agc_parked_import_unmap PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 30 ENVIRONMENT "APS5_PIN_WAIT_MS=3000") add_test(NAME agc_unit_shadow COMMAND agc_driver_recorder_tests --unit-shadow-only) set_tests_properties(agc_unit_shadow PROPERTIES SKIP_RETURN_CODE 77 TIMEOUT 30 ENVIRONMENT "APS5_PIN_WAIT_MS=3000") add_test(NAME agc_descriptor_pool_recycling COMMAND agc_driver_recorder_tests --descriptor-pool-only) @@ -822,7 +824,7 @@ if(BUILD_TESTING) add_test(NAME agc_draw_inputs_in_place COMMAND agc_driver_recorder_tests --draw-inputs-in-place-only) set_tests_properties(agc_draw_inputs_in_place PROPERTIES SKIP_RETURN_CODE 77) if(WIN32) - set_tests_properties(agc_host_import_unmap agc_unit_shadow agc_descriptor_pool_recycling agc_draw_snapshot_cap agc_multisample_guest_access agc_draw_inputs_in_place PROPERTIES ENVIRONMENT_MODIFICATION + set_tests_properties(agc_host_import_unmap agc_parked_import_unmap agc_unit_shadow agc_descriptor_pool_recycling agc_draw_snapshot_cap agc_multisample_guest_access agc_draw_inputs_in_place PROPERTIES ENVIRONMENT_MODIFICATION "PATH=path_list_prepend:$") endif() endif() diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 73e1015e5..d5a81a071 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -219,6 +219,26 @@ void retireImport(VkDevice device, HostImports& state, std::map> released; + { + std::lock_guard lock(state.mutex); + 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) { + released.push_back(std::move(*it)); + it = list.erase(it); + } else { + ++it; + } + } + device = list.empty() ? state.parked.erase(device) : std::next(device); + } + } + return !released.empty(); +} + const char* bindImport(const Context& context, HostImport& entry, VkExternalMemoryHandleTypeFlagBits handleType, const void* import, std::uint32_t importTypes, bool& allocated, VkResult& failure) { const auto failed = [&](const char* step, VkResult result) -> const char* { if (entry.buffer) context.Function("vkDestroyBuffer")(context.device, entry.buffer, nullptr); @@ -1534,6 +1554,7 @@ bool WaitForLeasesAndImports(std::uintptr_t address, std::size_t bytes) noexcept if (recorder != nullptr) recorder->FlushDeferredReleases(); const auto end = static_cast(address) + bytes; auto& imports = Imports(); + if (releaseParkedImports(imports, address, end)) retiredImport = true; const auto lease = GuestAllocations::GuestAllocationsAcquire_nid_postfix(); { std::lock_guard importsLock(imports.mutex); @@ -1585,6 +1606,7 @@ bool WaitForLeasesAndImports(std::uintptr_t address, std::size_t bytes) noexcept const auto end = static_cast(address) + bytes; auto& imports = Imports(); + if (releaseParkedImports(imports, address, end)) retiredImport = true; bool overlapsImport = false; { std::lock_guard importsLock(imports.mutex); diff --git a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp index 99e4f9a77..e8c9cf883 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Recorder.cpp @@ -2567,6 +2567,71 @@ void hostImportUnmapTests(const Device& device, Recorder& recorder) { std::cout << "host import unmap lifetime: ok\n"; } +void parkedImportUnmapTests(const Device& device) { + const auto& context = device.GetContext(); + if (context.hostImportAlignment == 0) { + std::cout << "host imports unavailable: parked import unmap not tested\n"; + return; + } + const auto alignment = static_cast(std::max(65536u, context.hostImportAlignment)); + const auto bytes = alignment * 4u; +#ifdef _WIN32 + void* block = VirtualAlloc(nullptr, bytes, MEM_RESERVE | MEM_COMMIT, PAGE_READWRITE); +#else + void* block = std::aligned_alloc(alignment, bytes); +#endif + Require(block != nullptr, "cannot allocate the parked import unmap block"); + const auto release = [&] { +#ifdef _WIN32 + VirtualFree(block, 0, MEM_RELEASE); +#else + std::free(block); +#endif + }; + const auto address = reinterpret_cast(block); + { + GuestAllocations::Mutation mutation; + mutation.Add(block, bytes, true, true, true); + } + std::unique_ptr replacement; + bool imported = false; + { + std::lock_guard gpu(GpuMutex()); + imported = HostImportFor(context, address, bytes) != nullptr; + } + if (imported) { + replacement = std::make_unique(); + std::lock_guard gpu(GpuMutex()); + imported = HostImportFor(replacement->GetContext(), address, bytes) != nullptr; + } + if (!imported) { + GuestAllocations::Mutation mutation; + mutation.Remove(block); + if (replacement) ClearHostImports(replacement->GetContext().device); + release(); + std::cout << "host import refused: parked import unmap not tested\n"; + return; + } + bool unmapped = false; + try { + GuestAllocations::Mutation mutation; + mutation.Unmap(block, bytes, [&](const void*, std::size_t, const void*, bool) { unmapped = true; }); + } catch (const std::exception& error) { + ClearHostImports(replacement->GetContext().device); + ClearHostImports(context.device); + GuestAllocations::Mutation mutation; + mutation.Remove(block); + release(); + throw std::runtime_error(std::string("guest unmap after a device replacement: ") + error.what()); + } + ClearHostImports(replacement->GetContext().device); + ClearImageMirrors(replacement->GetContext().device); + replacement.reset(); + release(); + Require(unmapped, "guest unmap of a range a replaced device imported did not run"); + std::cout << "parked import unmap: ok\n"; +} + void importWatchTests(const Device& device) { using namespace AgcDriver::GuestMemory; const auto& context = device.GetContext(); @@ -5400,6 +5465,11 @@ int main(int argc, char** argv) { hostImportUnmapTests(device, recorder); return 0; } + if (argc == 2 && std::string_view(argv[1]) == "--parked-import-unmap-only") { + gpu.unlock(); + parkedImportUnmapTests(device); + return 0; + } if (argc == 2 && std::string_view(argv[1]) == "--unit-shadow-only") { if (!AgcDriver::Graphics::UnitShadowEnabled() || device.GetContext().hostImportAlignment == 0) { std::cout << "skipped, unit shadows or host imports are unavailable\n";