From a7730cb9e600b67a303910cc29bf8ae8e3e23cce Mon Sep 17 00:00:00 2001 From: AnyPS5 Contributors Date: Wed, 14 Oct 2026 17:48:14 -0400 Subject: [PATCH] fix(graphics): keep discarded import probes from pinning windows --- .../Graphics/src/GuestBufferMemory.cpp | 23 ++- .../libSceAgcDriver/tests/BdaResources.cpp | 147 ++++++++++++++++++ 2 files changed, 164 insertions(+), 6 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index c04d2bdf93..1674531dd1 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -892,6 +892,7 @@ ImportRun windowRun(const HostImports& state, ImportRun run, std::uint64_t begin return run; } +const HostImport* peekImport(const HostImports& state, std::uint64_t begin, std::uint64_t end); const HostImport* findImport(HostImports& state, std::uint64_t begin, std::uint64_t end); bool importsStale(const Context& context, const HostImports& state); const HostImport* importRunAt(const Context& context, HostImports& state, ImportRun run, bool joined, std::uint64_t address, const GuestAllocations::Lease& lease, std::uint64_t end, bool evict); @@ -910,7 +911,10 @@ const HostImport* importAllocation(const Context& context, HostImports& state, s if (fresh && end != 0) { if (const auto* current = findImport(state, address, end)) return current; } else if (!fresh) { - if (const auto* current = findImport(state, address, address + 1); current != nullptr && current->joined && sameRange(*current, lease)) return current; + if (const auto* current = peekImport(state, address, address + 1); current != nullptr && current->joined && sameRange(*current, lease)) { + current->lastUse = ++state.useCounter; + return current; + } } auto run = importRun(lease, address); // Without an end the caller views the whole run through the import (AcquireRegistered): a window that holds the @@ -941,7 +945,10 @@ const HostImport* importRunAt(const Context& context, HostImports& state, Import failures.insert_or_assign(base, ImportFailure{base, bytes, GuestAllocations::Mapped(run.ranges.begin(), run.ranges.end())}); return nullptr; } - if (const auto found = state.imports.find(base); found != state.imports.end() && sameRun(found->second, run)) return &found->second; + if (const auto found = state.imports.find(base); found != state.imports.end() && sameRun(found->second, run)) { + found->second.lastUse = ++state.useCounter; + return &found->second; + } if (alignment == 0 || base % alignment != 0 || bytes % alignment != 0) return nullptr; if (const auto failed = failures.find(base); failed != failures.end()) { if (sameRun(failed->second, run)) return nullptr; @@ -1153,14 +1160,18 @@ void refreshImports(const Context& context, HostImports& state, const GuestAlloc for (auto it = state.failedJoins.begin(); it != state.failedJoins.end();) it = sameRun(it->second, joinedRun(lease, it->second.base, it->second.base + it->second.bytes)) ? std::next(it) : state.failedJoins.erase(it); } -const HostImport* findImport(HostImports& state, std::uint64_t begin, std::uint64_t end) { +const HostImport* peekImport(const HostImports& state, std::uint64_t begin, std::uint64_t end) { auto found = state.imports.upper_bound(begin); if (found == state.imports.begin()) return nullptr; --found; const auto& entry = found->second; - if (begin < entry.base || end > entry.base + entry.bytes) return nullptr; - entry.lastUse = ++state.useCounter; - return &entry; + return begin < entry.base || end > entry.base + entry.bytes ? nullptr : &entry; +} + +const HostImport* findImport(HostImports& state, std::uint64_t begin, std::uint64_t end) { + const auto* entry = peekImport(state, begin, end); + if (entry != nullptr) entry->lastUse = ++state.useCounter; + return entry; } const HostImport* importAccess(const Context& context, HostImports& state, std::uint64_t begin, std::uint64_t end, const GuestAllocations::Lease& lease) { diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index 2fe20f100c..b79cab3909 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include #include @@ -608,6 +609,151 @@ void importWholeRangeOverWindowTests(const Context& context) { leased.WriteBack(); } +struct ObservedHostImport { + const void* pointer; + VkDeviceSize bytes; +}; + +class ImportAllocationProbe { +public: + explicit ImportAllocationProbe(Context& context) : context(context), original(context.deviceProc) { + Require(active == nullptr && original != nullptr, "invalid import allocation probe scope"); + active = this; + context.deviceProc = Resolve; + } + ~ImportAllocationProbe() { + context.deviceProc = original; + active = nullptr; + } + ImportAllocationProbe(const ImportAllocationProbe&) = delete; + ImportAllocationProbe& operator=(const ImportAllocationProbe&) = delete; + + std::vector calls; + +private: + static PFN_vkVoidFunction VKAPI_CALL Resolve(VkDevice device, const char* name) { + Require(active != nullptr, "import allocation probe escaped its scope"); + if (std::strcmp(name, "vkAllocateMemory") == 0) return reinterpret_cast(Allocate); + return active->original(device, name); + } + static VkResult VKAPI_CALL Allocate(VkDevice device, const VkMemoryAllocateInfo* info, const VkAllocationCallbacks* allocator, VkDeviceMemory* memory) { + Require(active != nullptr, "import allocation callback escaped its scope"); + for (const auto* next = static_cast(info->pNext); next != nullptr; next = next->pNext) { + if (next->sType != VK_STRUCTURE_TYPE_IMPORT_MEMORY_HOST_POINTER_INFO_EXT) continue; + const auto* imported = reinterpret_cast(next); + active->calls.push_back({imported->pHostPointer, info->allocationSize}); + } + const auto allocate = reinterpret_cast(active->original(device, "vkAllocateMemory")); + Require(allocate != nullptr, "import allocation probe has no original allocator"); + return allocate(device, info, allocator, memory); + } + + Context& context; + PFN_vkGetDeviceProcAddr original; + inline static thread_local ImportAllocationProbe* active = nullptr; +}; + +class UnrelatedImportProbeRange { +public: + UnrelatedImportProbeRange() { + GuestAllocations::Mutation().Add(storage.data(), storage.size(), true, false, false); + } + ~UnrelatedImportProbeRange() { + GuestAllocations::Mutation().Remove(storage.data()); + } + UnrelatedImportProbeRange(const UnrelatedImportProbeRange&) = delete; + UnrelatedImportProbeRange& operator=(const UnrelatedImportProbeRange&) = delete; + +private: + std::array storage{}; +}; + +void importStaleProbeTests(const Context& context, const BdaTestAccess& access) { + for (const auto scenario : {0u, 1u, 2u}) { + const bool held = scenario == 1u; + const bool exactWhole = scenario == 2u; + WindowedRange range(context, 2 * WindowedRange::piece, 2); + ImportAllocationProbe probe(range.importing); + const auto first = range.First(); + auto* guest = reinterpret_cast(first); + for (std::size_t offset = 17u; offset < range.bytes; offset += 4096u) guest[offset] = static_cast((offset / 4096u) & 0xffu); + const std::vector originalGuest(guest, guest + range.bytes); + const auto* window = HostImportFor(range.importing, first, exactWhole ? range.bytes : 32u); + Require(window != nullptr && !window->joined && window->base == first && window->bytes == (exactWhole ? range.bytes : range.window), "the stale-probe fixture did not create its ordinary import extent"); + const auto originalBuffer = window->buffer; + const auto originalUse = window->lastUse; + const auto originalSerial = HostImportSerial(range.importing, first, 32u, false); + auto mockContents = access.bytes(window->buffer); + Require(mockContents.size() == window->bytes, "the mock import buffer has an incorrect extent"); + std::copy_n(originalGuest.begin(), window->bytes, mockContents.begin()); + std::uint32_t scratch = 0x12345678u; + { + GuestBufferMemory copied(range.importing); + copied.AddReadable(reinterpret_cast(&scratch), sizeof(scratch)); + copied.Upload(false); + copied.WriteBack(); + } + Require(window->lastUse == originalUse && HostImportSerial(range.importing, first, 32u, false) == originalSerial, "an unrelated copied build touched or replaced the prefix window"); + GuestBufferMemory reader(range.importing); + VkDescriptorBufferInfo heldView{}; + std::uint32_t heldAdjustment = 0u; + if (held) { + reader.AddReadable(first + 16u, 32u); + reader.Upload(false); + heldView = reader.Descriptor(first + 16u, 32u, heldAdjustment); + Require(heldView.buffer == originalBuffer, "the held control did not bind the prefix window"); + } + GuestAllocations::Mapped mapped; + { + const auto lease = GuestAllocations::GuestAllocationsAcquire_nid_postfix(); + for (const auto& owner : lease) if (owner->address == first && owner->bytes == range.bytes) mapped.push_back(owner); + } + Require(mapped.size() == 1u, "the stale-probe fixture lost its registered range"); + const auto originalOwner = mapped.front().lock(); + const auto generation = GuestAllocations::GuestAllocationsGeneration_nid_postfix(); + UnrelatedImportProbeRange unrelated; + Require(GuestAllocations::GuestAllocationsGeneration_nid_postfix() != generation && HostImportsIdentity(range.importing).device == VK_NULL_HANDLE, + "the unrelated registry change did not make imports stale"); + const auto attempts = access.allocationAttempts(); + probe.calls.clear(); + { + std::lock_guard gpu(AgcDriver::GuestMemory::GpuMutex()); + Require(ImportMappedRanges(range.importing, mapped, GuestAllocations::GuestAllocationsGeneration_nid_postfix(), false), "the current device did not admit the map-time import pass"); + } + Require(mapped.front().lock() == originalOwner, "an unrelated registry change replaced the import's memory owner"); + if (held) { + Require(access.allocationAttempts() == attempts && probe.calls.empty(), "a stale probe retired an import held by an actual descriptor"); + const auto* kept = HostImportFor(range.importing, first + 16u, 32u); + Require(kept != nullptr && kept->buffer == originalBuffer && kept->bytes == range.window && HostImportSerial(range.importing, first, 32u, false) == originalSerial, + "the actual held import lost its buffer, extent or serial"); + std::uint32_t adjustment = 0u; + const auto view = reader.Descriptor(first + 16u, 32u, adjustment); + Require(view.buffer == heldView.buffer && view.offset == heldView.offset && view.range == heldView.range && adjustment == heldAdjustment, + "a map-time stale probe invalidated a descriptor that still uses its import"); + const auto contents = access.bytes(view.buffer); + Require(view.offset + adjustment + 32u <= contents.size() && std::memcmp(contents.data() + view.offset + adjustment, reinterpret_cast(first + 16u), 32u) == 0, + "the protected descriptor no longer reads the registered bytes"); + reader.WriteBack(); + } else if (exactWhole) { + Require(access.allocationAttempts() == attempts && probe.calls.empty() && window->lastUse > originalUse, + "accepting the stale exact whole import reallocated it or did not protect its returned pointer"); + const auto* kept = HostImportFor(range.importing, first, range.bytes); + Require(kept != nullptr && kept->buffer == originalBuffer && kept->ranges.size() == 1u && kept->ranges.front() == originalOwner && HostImportSerial(range.importing, first, range.bytes, false) == originalSerial, + "accepting the exact whole import changed its native or registered identity"); + } else { + Require(access.allocationAttempts() == attempts + 1u, "a discarded stale probe made an unheld prefix window block the whole import"); + const auto* whole = HostImportFor(range.importing, first, range.bytes); + Require(whole != nullptr && whole->base == first && whole->bytes == range.bytes && whole->ranges.size() == 1u && whole->ranges.front() == originalOwner, + "the stale-probe repair did not retain the exact registered owner over the whole import"); + Require(HostImportSerial(range.importing, first, range.bytes, false) != originalSerial, "replacing the unheld window reused its old import identity"); + Require(probe.calls.size() == 1u && probe.calls.front().pointer == reinterpret_cast(first) && probe.calls.front().bytes == range.bytes, + "the replacement native import did not reference the exact whole registered memory"); + Require(access.bytes(whole->buffer).size() == range.bytes, "the replacement import buffer does not cover its registered extent"); + } + Require(scratch == 0x12345678u && std::equal(originalGuest.begin(), originalGuest.end(), guest), "the import probe changed guest bytes or unrelated copied data"); + } +} + // An address-based build whose whole-range import does not fit the budget leaves the range's window alone: before, // every such build retired the window and the next access imported it again. void importRefusedWholeRangeTests(const Context& context) { @@ -1787,6 +1933,7 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { spaceReadAcrossImportsWrittenTests(context); importJoinTests(context); importWholeRangeOverWindowTests(context); + importStaleProbeTests(context, access); importRefusedWholeRangeTests(context); importNeighbouringEdgesTests(context); importAtMapOverBudgetTests(context);