From 2441a4b0d9f472fe38bc44634a9aa620f2f29663 Mon Sep 17 00:00:00 2001 From: TibixDev Date: Mon, 12 Oct 2026 07:34:15 +0300 Subject: [PATCH] fix(agc): retry failed host imports after range replacement Retain weak identities and the full failed span so a replacement of either owner or a shorter remaining run retries the import. Keep unchanged failures cached across unrelated mapping changes without pinning their allocations. Existing upstream import lifetime, span windows and Windows fallback remain in place. AI-assisted implementation. --- .../Graphics/src/GuestBufferMemory.cpp | 27 ++++++--- .../libSceAgcDriver/tests/BdaResources.cpp | 57 +++++++++++++++++++ 2 files changed, 76 insertions(+), 8 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 73e1015e5..832d44eeb 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -142,6 +142,12 @@ bool addressSpaceCacheEnabled() { return !disabled; } +struct ImportFailure { + std::uint64_t base; + std::uint64_t bytes; + GuestAllocations::Mapped ranges; +}; + // Imports persist across draws, keyed by allocation base, and are dropped when their allocation leaves // the registered set; a dropped import is destroyed once the recorded work that may read it completed. struct HostImports { @@ -152,7 +158,7 @@ struct HostImports { std::map imports; std::map>> parked; std::uint64_t liveBytes = 0; - std::set failed; + std::map failed; std::set unaliasedSpans; // Registry generation the imports were last reconciled with. std::uint64_t refreshedGeneration = 0; @@ -595,7 +601,8 @@ bool runCovers(const GuestAllocations::Lease& lease, std::uint64_t begin, std::u return !run.ranges.empty() && end <= run.base + run.bytes; } -bool sameRun(const HostImport& entry, const ImportRun& run) { +template +bool sameRun(const Entry& entry, const ImportRun& run) { if (run.base != entry.base || run.bytes != entry.bytes || run.ranges.size() != entry.ranges.size()) return false; for (std::size_t index = 0; index < run.ranges.size(); ++index) { if (entry.ranges[index].owner_before(run.ranges[index]) || run.ranges[index].owner_before(entry.ranges[index])) return false; @@ -629,7 +636,11 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s } if (auto it = state.imports.lower_bound(base); it != state.imports.begin() && std::prev(it)->second.base + std::prev(it)->second.bytes > base) retireImport(context.device, state, std::prev(it), lease); const auto alignment = context.hostImportAlignment; - if (alignment == 0 || base % alignment != 0 || bytes % alignment != 0 || state.failed.contains(base)) return nullptr; + if (alignment == 0 || base % alignment != 0 || bytes % alignment != 0) return nullptr; + if (const auto failed = state.failed.find(base); failed != state.failed.end()) { + if (sameRun(failed->second, run)) return nullptr; + state.failed.erase(failed); + } // Pinned imports count against the driver's system memory budget; past it ordinary host // allocations fail, so imports stop at APS5_HOST_IMPORT_MIB (default 6 GiB, which covers the // registered memory of address-based shaders; past it they copy gigabytes per dispatch). @@ -661,7 +672,7 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s for (std::uint64_t cursor = base; cursor < base + bytes;) { MEMORY_BASIC_INFORMATION info{}; if (VirtualQuery(reinterpret_cast(cursor), &info, sizeof(info)) == 0 || info.State != MEM_COMMIT || (info.Protect & (PAGE_NOACCESS | PAGE_GUARD)) != 0) { - state.failed.insert(base); + state.failed.insert_or_assign(base, ImportFailure{base, bytes, GuestAllocations::Mapped(entry.ranges.begin(), entry.ranges.end())}); return nullptr; } const auto protection = info.Protect & 0xffu; @@ -673,7 +684,7 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s cursor = reinterpret_cast(info.BaseAddress) + info.RegionSize; } if (!writable && readOnly) { - state.failed.insert(base); + state.failed.insert_or_assign(base, ImportFailure{base, bytes, GuestAllocations::Mapped(entry.ranges.begin(), entry.ranges.end())}); return nullptr; } if (!writable) { @@ -692,7 +703,7 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s } #else if (!GuestMemory::Accessible(reinterpret_cast(base), static_cast(bytes), true)) { - state.failed.insert(base); + state.failed.insert_or_assign(base, ImportFailure{base, bytes, GuestAllocations::Mapped(entry.ranges.begin(), entry.ranges.end())}); return nullptr; } #endif @@ -712,7 +723,7 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s #ifdef _WIN32 GuestArena::GuestArenaUnmapAlias_nid_postfix(entry.alias); #endif - state.failed.insert(base); + state.failed.insert_or_assign(base, ImportFailure{base, bytes, GuestAllocations::Mapped(entry.ranges.begin(), entry.ranges.end())}); std::fprintf(stderr, "[gpu] host import of 0x%llx+0x%llx failed at %s (%d); falling back to copies\n", static_cast(base), static_cast(bytes), step, static_cast(result)); #ifdef _WIN32 static const bool trace = std::getenv("APS5_TRACE_HOST_IMPORT") != nullptr; @@ -800,7 +811,7 @@ void refreshImports(const Context& context, HostImports& state, const GuestAlloc retireImport(context.device, state, it, lease); it = next; } - for (auto it = state.failed.begin(); it != state.failed.end();) it = leasedRangeAt(lease, *it) != nullptr ? std::next(it) : state.failed.erase(it); + for (auto it = state.failed.begin(); it != state.failed.end();) it = sameRun(it->second, importRun(lease, it->first)) ? std::next(it) : state.failed.erase(it); } const HostImport* findImport(HostImports& state, std::uint64_t begin, std::uint64_t end) { diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index 777c3fa77..392eab435 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -119,6 +119,62 @@ void importSpanTests(const Context& context) { ::operator delete(block, std::align_val_t{piece}); } +void failedImportRemapTests(const Context& context, const BdaTestAccess& access) { + constexpr std::size_t piece = 65536; + auto* block = static_cast(::operator new(4 * piece, std::align_val_t{piece})); + auto importing = context; + importing.hostImportAlignment = piece; + const auto address = reinterpret_cast(block); + const auto add = [&](std::size_t index) { + GuestAllocations::Mutation mutation; + mutation.Add(block + index * piece, piece, true, true, true); + }; + const auto remove = [&](std::size_t index) { + GuestAllocations::Mutation mutation; + mutation.Remove(block + index * piece); + }; + SetHostImportWindow(0); + for (std::size_t replaced = 0; replaced < 2; ++replaced) { + add(0); + add(1); + std::weak_ptr owner; + { + const auto lease = GuestAllocations::GuestAllocationsAcquire_nid_postfix(); + const auto found = std::find_if(lease.begin(), lease.end(), [&](const auto& range) { return range->address == address + replaced * piece; }); + Require(found != lease.end(), "failed import fixture has no registered owner"); + owner = *found; + } + access.limitMemory(0); + const auto attempts = access.allocationAttempts(); + Require(HostImportFor(importing, address, 2 * piece) == nullptr && access.allocationAttempts() == attempts + 1, "host import failure was not exercised"); + Require(owner.use_count() == 1, "a failed import pins its owner"); + Require(HostImportFor(importing, address, 2 * piece) == nullptr && access.allocationAttempts() == attempts + 1, "an unchanged failed import was retried"); + add(3); + Require(HostImportFor(importing, address, 2 * piece) == nullptr && access.allocationAttempts() == attempts + 1, "an unrelated mapping retried a failed import"); + remove(replaced); + Require(owner.expired(), "a failed import kept the replaced owner alive"); + add(replaced); + access.limitMemory(std::nullopt); + const auto* imported = HostImportFor(importing, address, 2 * piece); + Require(imported != nullptr && imported->base == address && imported->bytes == 2 * piece && access.allocationAttempts() == attempts + 2, "same-address remap retained a failed import decision"); + remove(0); + remove(1); + remove(3); + ClearHostImports(importing.device); + } + add(0); + add(1); + access.limitMemory(0); + Require(HostImportFor(importing, address, 2 * piece) == nullptr, "span failure was not exercised"); + remove(1); + access.limitMemory(std::nullopt); + const auto* shortened = HostImportFor(importing, address, piece); + Require(shortened != nullptr && shortened->bytes == piece, "a failed span suppressed an import of its remaining range"); + remove(0); + ClearHostImports(importing.device); + ::operator delete(block, std::align_val_t{piece}); +} + void importWindowTests(const Context& context) { constexpr std::size_t piece = 65536; constexpr std::size_t window = 2 * piece; @@ -1105,6 +1161,7 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { } importCrossingTests(context, access); importSpanTests(context); + failedImportRemapTests(context, access); importWindowTests(context); heapMirrorTests(context, access); importedHeapMirrorTests(context, access);