diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp index e1b80ff503db..2bcdbbbca07c 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/GuestBufferMemory.hpp @@ -351,7 +351,7 @@ class GuestBufferMemory { // The region of a descriptor-bound range (AddWritable/AddReadable), committed pages only. A // range inside a base region of the space adds nothing: the region serves it, as today's merge // of the two did. - void addDescriptorRegion(std::uint64_t address, std::size_t bytes, bool atomic, bool swept); + void addDescriptorRegion(std::uint64_t address, std::size_t bytes, bool atomic, bool swept, bool readOnly); static bool servesView(const Region& region, std::uint64_t view); static bool sparseHeapCandidate(const Region& region); bool bindSparseMirror(Region& region, const GuestAllocations::Lease& lease); @@ -390,6 +390,7 @@ class GuestBufferMemory { // regions follow the registry's order), so AddSnapshot can search instead of scanning. bool regionsSorted = false; std::vector> writes; + std::vector> readsOverBase; std::vector>> heapReferences; // UploadPrepare ran (regions are frozen); `uploaded` once UploadFinish ran. bool prepared = false; diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp index 8f0be0a68e4d..7c254e6b92de 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp @@ -2605,7 +2605,21 @@ const GuestBufferMemory::Region* GuestBufferMemory::owner(std::uint64_t address) return own; } -void GuestBufferMemory::addDescriptorRegion(std::uint64_t address, std::size_t bytes, bool atomic, bool swept) { +namespace { + +template +bool mirrorOverlaps(const Regions& base, std::uint64_t begin, std::uint64_t end) { + auto found = std::upper_bound(base.begin(), base.end(), begin, [](std::uint64_t value, const auto& region) { return value < region.begin; }); + if (found != base.begin()) --found; + for (; found != base.end() && found->begin < end; ++found) { + if (found->end > begin && found->mirror != nullptr) return true; + } + return false; +} + +} + +void GuestBufferMemory::addDescriptorRegion(std::uint64_t address, std::size_t bytes, bool atomic, bool swept, bool readOnly) { validate(address, bytes); const auto begin = address & ~std::uint64_t{3}; const auto end = begin == address ? address + bytes : (address + bytes + 3) & ~std::uint64_t{3}; @@ -2618,6 +2632,10 @@ void GuestBufferMemory::addDescriptorRegion(std::uint64_t address, std::size_t b } return; case BaseOverlap::Partial: + if (readOnly && !mirrorOverlaps(space->base, begin, end) && !WritesOverlap(begin, static_cast(end - begin))) { + readsOverBase.emplace_back(begin, end); + break; + } Spaces().dissolvedOverlap.fetch_add(1, std::memory_order_relaxed); dissolveSpace(false); break; @@ -2683,7 +2701,11 @@ bool GuestBufferMemory::bindSparseMirror(Region& region, const GuestAllocations: } void GuestBufferMemory::AddWritable(std::uint64_t address, std::size_t bytes, bool atomic, bool swept) { - addDescriptorRegion(address, bytes, atomic, swept); + if (space != nullptr && std::any_of(readsOverBase.begin(), readsOverBase.end(), [&](const auto& range) { return address < range.second && range.first < address + bytes; })) { + Spaces().dissolvedOverlap.fetch_add(1, std::memory_order_relaxed); + dissolveSpace(false); + } + addDescriptorRegion(address, bytes, atomic, swept, false); writes.emplace_back(address, address + bytes); } @@ -2691,7 +2713,7 @@ void GuestBufferMemory::AddReadable(std::uint64_t address, std::size_t bytes) { // Not in `writes`: no reference copy (copyRegion), no write-back, no pending-write note, no // direct-write mark, and UploadPrepare copies it without the device lock. A written descriptor // overlapping the range still covers it through its own Writes() entry. - addDescriptorRegion(address, bytes, false, false); + addDescriptorRegion(address, bytes, false, false, true); } void GuestBufferMemory::AddSnapshot(const GuestMemorySnapshot& snapshot) { diff --git a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp index 08fe07ffa11e..356fe0d4ff8d 100644 --- a/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp @@ -474,6 +474,89 @@ void importAtMapOverBudgetTests(const Context& context) { for (auto* block : {kept, used, mapped}) ::operator delete(block, std::align_val_t{bytes}); } +void spaceReadAcrossImportsTests(const Context& context, const BdaTestAccess& access) { + constexpr std::size_t piece = 65536; + constexpr std::size_t window = 2 * piece; + void* block = ::operator new(3 * piece, std::align_val_t{window}); + auto* guest = static_cast(block); + const auto first = reinterpret_cast(block); + const auto boundary = first + window; + std::memset(guest, 0x5a, window); + std::memset(guest + window, 0x6b, piece); + auto importing = context; + importing.hostImportAlignment = piece; + SetHostImportWindow(window); + const auto change = [&](std::uint8_t* range, bool add) { + auto* mutation = GuestAllocations::GuestAllocationsBegin_nid_postfix(); + if (add) GuestAllocations::GuestAllocationsAdd_nid_postfix(mutation, range, piece, true, false, true); + else GuestAllocations::GuestAllocationsRemove_nid_postfix(mutation, range); + GuestAllocations::GuestAllocationsEnd_nid_postfix(mutation); + }; + for (std::size_t index = 0; index < 3; ++index) change(guest + index * piece, true); + for (int build = 0; build < 2; ++build) { + const auto before = AddressSpaceCounters(); + GuestBufferMemory leased(importing); + leased.AcquireRegistered(); + leased.AddReadable(boundary - 256, 512); + leased.Upload(true); + std::uint32_t adjustment = 0; + const auto view = leased.Descriptor(boundary - 256, 512, adjustment); + Require(view.buffer != VK_NULL_HANDLE && view.range == 512 + adjustment, "a read-only V# across two imports has no descriptor"); + const auto bytes = access.bytes(view.buffer); + Require(view.offset + adjustment + 512 <= bytes.size() && bytes[view.offset + adjustment] == std::byte{0x5a} && bytes[view.offset + adjustment + 256] == std::byte{0x6b}, "a read-only V# across two imports misses the bytes of either range"); + const auto ranges = leased.AddressRanges(); + Require(std::any_of(ranges.begin(), ranges.end(), [&](const auto& range) { return range.begin <= first && boundary <= range.end; }) && std::any_of(ranges.begin(), ranges.end(), [&](const auto& range) { return range.begin <= boundary && boundary + piece <= range.end; }), "an import is missing from the BDA table of a build with a read-only V# across it"); + leased.WriteBack(); + const auto after = AddressSpaceCounters(); + if (after.enabled && build == 1) Require(after.hits == before.hits + 1 && after.dissolvedOverlap == before.dissolvedOverlap, "a read-only V# across two imports dissolved the cached address space"); + } + for (std::size_t index = 0; index < 3; ++index) change(guest + index * piece, false); + SetHostImportWindow(0); + ::operator delete(block, std::align_val_t{window}); +} + +void spaceReadAcrossImportsWrittenTests(const Context& context) { + constexpr std::size_t piece = 65536; + constexpr std::size_t window = 2 * piece; + void* block = ::operator new(3 * piece, std::align_val_t{window}); + auto* guest = static_cast(block); + const auto boundary = reinterpret_cast(block) + window; + std::memset(guest, 0x5a, window); + std::memset(guest + window, 0x6b, piece); + auto importing = context; + importing.hostImportAlignment = piece; + SetHostImportWindow(window); + const auto change = [&](std::uint8_t* range, bool add) { + auto* mutation = GuestAllocations::GuestAllocationsBegin_nid_postfix(); + if (add) GuestAllocations::GuestAllocationsAdd_nid_postfix(mutation, range, piece, true, false, true); + else GuestAllocations::GuestAllocationsRemove_nid_postfix(mutation, range); + GuestAllocations::GuestAllocationsEnd_nid_postfix(mutation); + }; + for (const bool writtenFirst : {false, true}) { + for (std::size_t index = 0; index < 3; ++index) change(guest + index * piece, true); + for (int build = 0; build < 2; ++build) { + const auto before = AddressSpaceCounters(); + GuestBufferMemory leased(importing); + leased.AcquireRegistered(); + if (writtenFirst) leased.AddWritable(boundary - 384, 256); + leased.AddReadable(boundary - 256, 512); + if (!writtenFirst) leased.AddWritable(boundary - 384, 256); + leased.Upload(true); + std::uint32_t readAdjustment = 0; + std::uint32_t writeAdjustment = 0; + const auto read = leased.Descriptor(boundary - 256, 512, readAdjustment); + const auto written = leased.Descriptor(boundary - 384, 256, writeAdjustment); + Require(read.buffer != VK_NULL_HANDLE && read.buffer == written.buffer && written.offset + writeAdjustment + 128 == read.offset + readAdjustment, "a written V# inside a read-only V# across two imports maps other bytes than it"); + leased.WriteBack(); + const auto after = AddressSpaceCounters(); + if (after.enabled && build == 0) Require(after.dissolvedOverlap == before.dissolvedOverlap + 1, "a written V# inside a read-only V# across two imports kept the cached address space"); + } + for (std::size_t index = 0; index < 3; ++index) change(guest + index * piece, false); + } + SetHostImportWindow(0); + ::operator delete(block, std::align_val_t{window}); +} + ShaderRecompiler::DescriptorBinding binding(Role role, std::uint32_t slot) { return {ShaderRecompiler::DescriptorKind::StorageBuffer, role, 0, slot, 1, {}, false}; } @@ -1471,6 +1554,8 @@ void RunBdaResourceTests(const Context& context, const BdaTestAccess& access) { failedImportRemapTests(context, access); spaceImportRunTests(context); importWindowTests(context); + spaceReadAcrossImportsTests(context, access); + spaceReadAcrossImportsWrittenTests(context); importJoinTests(context); importWholeRangeOverWindowTests(context); importRefusedWholeRangeTests(context);