From 86d66243233ae72afaa73d2658cd01cbdd3bff56 Mon Sep 17 00:00:00 2001 From: Chad Date: Sat, 10 Oct 2026 14:56:19 -0500 Subject: [PATCH] fix(agc): copy a sparse descriptor heap only up to its last backed byte Co-authored-by: Scramblrrrr --- .../Graphics/include/GuestBufferMemory.hpp | 4 +- .../Graphics/src/GuestBufferMemory.cpp | 34 ++++++++-- .../libSceAgcDriver/tests/BdaResources.cpp | 66 +++++++++++++++++++ 3 files changed, 98 insertions(+), 6 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index f180e1e9b..f6e5f3e5c 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -247,7 +247,7 @@ public: bool HoldsLease() const { return !lease.empty() || space != nullptr; } std::size_t CopiedBytes() const { std::size_t bytes = 0; - for (const auto& region : regions) if (region.buffer != nullptr) bytes += static_cast(region.end - region.begin); + for (const auto& region : regions) if (region.buffer != nullptr) bytes += static_cast((region.bufferEnd != 0 ? region.bufferEnd : region.end) - region.begin); return bytes; } // Every uploaded region as [begin, end) when all of them are served by host imports, in place or @@ -281,6 +281,8 @@ private: // memory; the rest reads as zeros and is never stored. bool sparse = false; std::vector> backed {}; + std::uint64_t lastView = 0; + std::uint64_t bufferEnd = 0; // Set when the region is served by an image mirror: nothing is copied; writable mirrors are // written back by comparing with the mirror's shadow. Kept alive here for recorded work. std::shared_ptr mirror {}; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 1f6091d71..818b39d65 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -2053,6 +2053,7 @@ void GuestBufferMemory::addDescriptorRegion(std::uint64_t address, std::size_t b Region region{begin, end, true, {}, nullptr}; region.atomic = atomic; region.swept = swept; + region.lastView = begin; auto committed = GuestMemory::DescribeCommitted(begin, static_cast(end - begin)); if (!committed.whole) { // A GPU heap bound whole while the guest commits its pages on demand, or a descriptor left @@ -2376,6 +2377,7 @@ void GuestBufferMemory::UploadPrepare(bool addressable) { previous.hostBacked = true; } mergeBacked(previous, region); + previous.lastView = std::max({previous.lastView, region.lastView, region.begin}); if (region.end > previous.end) previous.direct = nullptr; previous.atomic = previous.atomic || region.atomic; previous.swept = previous.swept || region.swept; @@ -2702,8 +2704,17 @@ void GuestBufferMemory::copyRegion(Region& region, bool addressable) { } } const auto allocateStart = std::chrono::steady_clock::now(); + if (region.sparse && region.buffer == nullptr && (region.hostBacked || region.writable)) { + const auto backedEnd = region.backed.empty() ? region.begin : region.backed.back().second; + const auto viewEnd = std::max(region.lastView, region.begin) + 4u; + constexpr std::uint64_t granule = 256; + const auto extent = std::max(backedEnd, viewEnd) - region.begin; + const auto end = region.begin + std::min((extent + granule - 1) / granule * granule, bytes); + region.bufferEnd = end < region.end ? end : 0; + } + const auto allocated = region.bufferEnd != 0 ? region.bufferEnd - region.begin : bytes; // A buffer made by UploadPrepare for a GPU copy that fell back here (its import went) is kept. - if (region.buffer == nullptr) region.buffer = std::make_shared(context, static_cast(bytes), usage); + if (region.buffer == nullptr) region.buffer = std::make_shared(context, static_cast(allocated), usage); const auto readStart = std::chrono::steady_clock::now(); if (profile) allocateUs.fetch_add(microsecondsSince(allocateStart), std::memory_order_relaxed); // Named for the [hooksync] attribution: the reads below go through the flush hook. @@ -2717,11 +2728,18 @@ void GuestBufferMemory::copyRegion(Region& region, bool addressable) { if (written) region.uploaded.assign(region.buffer->Bytes().begin(), region.buffer->Bytes().end()); } else { // `uploaded` holds the backed pieces back to back. + const auto target = region.buffer->Bytes().first(static_cast(allocated)); + const bool zeroGaps = region.bufferEnd != 0; + std::size_t cursor = 0; for (const auto& [first, last] : region.backed) { - const auto piece = region.buffer->Bytes().subspan(static_cast(first - region.begin), static_cast(last - first)); + const auto at = static_cast(first - region.begin); + if (zeroGaps && at > cursor) std::memset(target.data() + cursor, 0, at - cursor); + const auto piece = target.subspan(at, static_cast(last - first)); GuestMemory::Read(first, piece); if (written) region.uploaded.insert(region.uploaded.end(), piece.begin(), piece.end()); + cursor = at + piece.size(); } + if (zeroGaps && cursor < target.size()) std::memset(target.data() + cursor, 0, target.size() - cursor); } } else std::memcpy(region.buffer->Bytes().data(), region.snapshot.data(), region.snapshot.size()); @@ -2911,8 +2929,13 @@ VkDescriptorBufferInfo GuestBufferMemory::Descriptor(std::uint64_t address, std: Require(context.limits.minStorageBufferOffsetAlignment != 0, "no storage buffer offset alignment"); Require(base % 4 == 0, "a guest buffer view in a GPU owner that does not start at a DWORD boundary is not implemented"); adjustment = static_cast(offset % std::max(context.limits.minStorageBufferOffsetAlignment, 4)); - const auto range = ViewBytes(bytes, adjustment); - const auto end = region.direct != nullptr ? region.direct->base + region.direct->bytes : region.end; + auto range = ViewBytes(bytes, adjustment); + const bool cut = region.direct == nullptr && region.mirror == nullptr && region.bufferEnd != 0; + const auto end = region.direct != nullptr ? region.direct->base + region.direct->bytes : cut ? region.bufferEnd : region.end; + if (cut) { + Require(address - adjustment < end, "guest buffer view starts past its copy"); + range = std::min(range, end - (address - adjustment)); + } if (address - adjustment + range > end) Require(false, describe("rounded up to its view range exceeds")); Require(range <= context.limits.maxStorageBufferRange, "guest buffer view exceeds descriptor range limit"); const auto handle = region.direct != nullptr ? region.direct->buffer : region.mirror != nullptr ? region.mirror->buffer->Handle() : region.buffer->Handle(); @@ -2928,7 +2951,8 @@ ShaderRecompiler::BdaAbi::Range GuestBufferMemory::addressRange(const Region& re const auto address = region.direct != nullptr ? region.direct->address + (region.begin - region.direct->base) : region.mirror != nullptr ? region.mirror->buffer->DeviceAddress() + (region.begin - region.mirror->base) : region.buffer->DeviceAddress(); Require(region.end - region.begin <= std::numeric_limits::max() - address, "GPU address range overflow"); const auto permissions = ShaderRecompiler::BdaAbi::Read | (region.direct != nullptr && region.writable ? ShaderRecompiler::BdaAbi::Write : 0u); - return {region.begin, region.end, address, permissions, 0}; + const bool cut = region.direct == nullptr && region.mirror == nullptr && region.bufferEnd != 0; + return {region.begin, cut ? region.bufferEnd : region.end, address, permissions, 0}; } std::vector GuestBufferMemory::AddressRanges() const { diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index fa8240bd6..5baca0ff9 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -618,6 +618,71 @@ void stridedOverhangTests(const Context& context, const BdaTestAccess& access) { ::operator delete(block, std::align_val_t{allocation}); } +std::byte* reserveSparse(std::size_t reserved, std::size_t committed) { +#ifdef _WIN32 + auto* block = static_cast(VirtualAlloc(nullptr, reserved, MEM_RESERVE, PAGE_NOACCESS)); + Require(block != nullptr && VirtualAlloc(block, committed, MEM_COMMIT, PAGE_READWRITE) != nullptr, "cannot reserve the sparse heap"); +#else + void* mapped = mmap(nullptr, reserved, PROT_NONE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + Require(mapped != MAP_FAILED && mprotect(mapped, committed, PROT_READ | PROT_WRITE) == 0, "cannot reserve the sparse heap"); + auto* block = static_cast(mapped); +#endif + return block; +} + +void releaseSparse(std::byte* block, std::size_t reserved) { +#ifdef _WIN32 + static_cast(reserved); + VirtualFree(block, 0, MEM_RELEASE); +#else + munmap(block, reserved); +#endif +} + +void sparseHeapTests(const Context& context, const BdaTestAccess& access) { + constexpr std::size_t reserved = 64u << 20u; + constexpr std::size_t committed = 65536; + auto* block = reserveSparse(reserved, committed); + std::memset(block, 0x5a, committed); + const auto base = reinterpret_cast(block); + access.limitMemory(4u << 20u); + { + GuestBufferMemory memory(context); + memory.AddWritable(base, reserved); + memory.Upload(false); + std::uint32_t adjustment = 0; + const auto view = memory.Descriptor(base, reserved, adjustment); + Require(adjustment == 0 && view.range == committed, "a sparse heap view is not cut at its last committed byte"); + Require(memory.CopiedBytes() == committed, "a cut sparse heap is charged at its reserved size"); + const auto bytes = access.bytes(view.buffer); + Require(bytes.size() < reserved && bytes[view.offset + committed - 1] == std::byte{0x5a}, "a sparse heap was not copied up to its last committed byte"); + bytes[view.offset + 8] = std::byte{0x77}; + memory.WriteBack(); + Require(block[8] == std::byte{0x77}, "a store to a cut sparse heap was not written back"); + } + { + constexpr std::size_t late = 1u << 20u; + GuestBufferMemory memory(context); + memory.AddWritable(base, reserved); + memory.AddReadable(base + late, 16); + memory.Upload(true); + std::uint32_t adjustment = 0; + const auto view = memory.Descriptor(base + late, 16, adjustment); + Require(view.range == 16 + adjustment, "a view in the uncommitted part of a sparse heap is not bound"); + const auto bytes = access.bytes(view.buffer); + const auto whole = memory.Descriptor(base, reserved, adjustment); + Require(whole.buffer == view.buffer && bytes.size() >= whole.offset + late + 16, "a view in the uncommitted part of a sparse heap has another owner"); + Require(bytes[whole.offset + committed - 1] == std::byte{0x5a} && std::all_of(bytes.begin() + static_cast(whole.offset + committed), bytes.begin() + static_cast(whole.offset + late + 16), [](std::byte byte) { return byte == std::byte{}; }), "the uncommitted part of a cut sparse heap does not read as zeros"); + Require(whole.range == late + 256, "a sparse heap copy does not end past its last view's start"); + Require(memory.CopiedBytes() == late + 256, "a cut sparse heap charge does not include its last view"); + const auto ranges = memory.AddressRanges(); + Require(ranges.size() == 1 && ranges[0].begin == base && ranges[0].end == base + late + 256, "the BDA range of a cut sparse heap is not clamped to its copy"); + memory.WriteBack(); + } + access.limitMemory(std::nullopt); + releaseSparse(block, reserved); +} + void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { alignas(64) std::array guest{}; guest[0] = 123; @@ -781,6 +846,7 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { importedFreshTests(context); gpuMappingTests(context); stridedOverhangTests(context, access); + sparseHeapTests(context, access); Require(AddressCopyOverflow({{0x1000, 0x3000, 0x2000, "uncommitted pages"}}, 0x2000).empty(), "copies within the limit were refused"); const auto copies = AddressCopyOverflow({{0x1000, 0x2000, 0x1000, "not mirrored"}, {0x10000, 0x30000, 0x18000, "uncommitted pages"}}, 0x2000); Require(!copies.empty() && copies.find("0x10000+0x20000 (0.1 MiB committed, uncommitted pages)") < copies.find("0x1000+0x1000"), "the copy limit does not name the largest copy first");