diff --git a/src/graphics/host_gpu/memoryTracker.h b/src/graphics/host_gpu/memoryTracker.h index f2c2f45d1..ac6169fec 100644 --- a/src/graphics/host_gpu/memoryTracker.h +++ b/src/graphics/host_gpu/memoryTracker.h @@ -62,43 +62,18 @@ public: void ValidateGpuDirtyOwnership(const RangeSet&, uint64_t, uint64_t, const char*) {} #endif - template - void ForEachDownloadRange(uint64_t vaddr, uint64_t size, Preflight&& preflight, Func&& func) { - static_assert(std::is_nothrow_invocable_v); - static_assert(std::is_nothrow_invocable_v); - CheckNotInUploadCallback(); - std::vector managers; - Iterate(vaddr, size, [&](RegionManager* manager, uint64_t, uint64_t) { - managers.push_back(manager); - }); - std::vector> locks; - locks.reserve(managers.size()); - for (auto* manager: managers) { - locks.emplace_back(manager->lock); - } - Iterate(vaddr, size, [&](RegionManager* manager, uint64_t offset, uint64_t bytes) { - const auto address = manager->GetCpuAddr() + offset; - manager->template ForEachModifiedRange(address, bytes, - preflight); - }); - Iterate(vaddr, size, [&](RegionManager* manager, uint64_t offset, uint64_t bytes) { - manager->template ForEachModifiedRange( - manager->GetCpuAddr() + offset, bytes, func); - }); - if constexpr (clear) { - Iterate(vaddr, size, - [&](RegionManager* manager, uint64_t offset, uint64_t bytes) { - const auto address = manager->GetCpuAddr() + offset; - manager->template ForEachModifiedRange( - address, bytes, [](uint64_t, uint64_t) noexcept {}); - }); - } - } - template void ForEachDownloadRange(uint64_t vaddr, uint64_t size, Func&& func) { - ForEachDownloadRange( - vaddr, size, [](uint64_t, uint64_t) noexcept {}, std::forward(func)); + static_assert(std::is_nothrow_invocable_v); + CheckNotInUploadCallback(); + Iterate(vaddr, size, [&](RegionManager* manager, uint64_t offset, uint64_t bytes) { + std::scoped_lock lock(manager->lock); + const auto address = manager->GetCpuAddr() + offset; + manager->template ForEachModifiedRange(address, bytes, func); + if constexpr (clear) { + manager->template ChangeState(address, bytes); + } + }); } template diff --git a/src/graphics/host_gpu/renderer/cache/bufferCache.cpp b/src/graphics/host_gpu/renderer/cache/bufferCache.cpp index 7558390cc..9c94062da 100644 --- a/src/graphics/host_gpu/renderer/cache/bufferCache.cpp +++ b/src/graphics/host_gpu/renderer/cache/bufferCache.cpp @@ -43,13 +43,6 @@ void BufferCache::WriteDataBuffer(Buffer& buffer, uint64_t address, const void* } } -struct BufferCache::DownloadCopy { - Buffer* buffer = nullptr; - uint64_t source_offset = 0; - uint64_t address = 0; - uint64_t size = 0; -}; - void BufferCache::Register(BufferId id) { ChangeRegister(id); } @@ -115,78 +108,70 @@ void BufferCache::DeleteBuffer(BufferId id) { } } -std::pair BufferCache::DownloadEnvelope(const DownloadCopy& copy) { - if (copy.buffer == nullptr || copy.size == 0 || copy.source_offset > copy.buffer->Size() || - copy.size > copy.buffer->Size() - copy.source_offset) { - EXIT("BufferCache: invalid download copy\n"); +bool BufferCache::DownloadBufferMemory(Buffer& buffer, uint64_t vaddr, uint64_t size) { + std::vector copies; + uint64_t total_size = 0; + const auto buffer_address = buffer.CpuAddress(); + m_memory_tracker.ForEachDownloadRange( + vaddr, size, [&](uint64_t address, uint64_t bytes) noexcept { + m_memory_tracker.ValidateGpuDirtyPages(m_gpu_modified_ranges, address, bytes, + "buffer download"); + m_gpu_modified_ranges.ForEachInRange(address, bytes, [&](uint64_t start, uint64_t end) { + copies.emplace_back(start - buffer_address, total_size, end - start); + // Keep packed ranges on separate cache lines, as in shadPS4. + total_size += Common::AlignUp(end - start, 64); + }); + m_gpu_modified_ranges.Subtract(address, bytes); + }); + if (copies.empty()) { + return false; } - const auto begin = Common::AlignDown(copy.source_offset, 4); - if (copy.source_offset + copy.size > UINT64_MAX - 3) { - EXIT("BufferCache: download copy alignment overflow\n"); - } - const auto end = Common::AlignUp(copy.source_offset + copy.size, 4); - if (end > copy.buffer->Size()) { - EXIT("BufferCache: aligned download copy exceeds its owner\n"); - } - return {begin, end - begin}; -} -void BufferCache::DownloadBufferMemory(std::span copies) { - std::vector batch; - batch.reserve(copies.size()); - uint64_t packed_size = 0; - auto& download = m_download_buffer; - const auto flush = [&] { - const auto [mapped, base_offset] = download.Map(packed_size, DOWNLOAD_ALIGNMENT); - EXIT_IF(mapped == nullptr); - uint64_t cursor = 0; - for (const auto& copy: batch) { - const auto [source_begin, envelope_size] = DownloadEnvelope(copy); - download.CopyFrom(m_scheduler.Current(), *copy.buffer, source_begin, base_offset + cursor, - envelope_size, vk::AccessFlagBits::eMemoryWrite, vk::AccessFlags {}, - vk::AccessFlagBits::eMemoryRead | vk::AccessFlagBits::eMemoryWrite, - vk::AccessFlagBits::eHostRead); - cursor += Common::AlignUp(envelope_size, DOWNLOAD_ALIGNMENT); - } - download.Commit(); - const auto completion_tick = m_scheduler.CurrentTick(); - m_scheduler.Finish(); - m_scheduler.WaitPriorityOperations(completion_tick); - cursor = 0; - for (const auto& copy: batch) { - const auto [source_begin, envelope_size] = DownloadEnvelope(copy); - const auto offset = cursor + copy.source_offset - source_begin; - download.Invalidate(base_offset + offset, copy.size); - Libs::LibKernel::Memory::WriteBacking(copy.address, mapped + offset, copy.size); - cursor += Common::AlignUp(envelope_size, DOWNLOAD_ALIGNMENT); - } - batch.clear(); - packed_size = 0; - }; - for (auto copy: copies) { - while (copy.size != 0) { - const auto available = download.Size() - packed_size; - const auto prefix = copy.source_offset & 3u; - const auto bytes = std::min(copy.size, available - prefix); - DownloadCopy part {copy.buffer, copy.source_offset, copy.address, bytes}; - const auto [source_begin, envelope_size] = DownloadEnvelope(part); - (void)source_begin; - packed_size += Common::AlignUp(envelope_size, DOWNLOAD_ALIGNMENT); - batch.push_back(part); - copy.source_offset += bytes; - copy.address += bytes; - copy.size -= bytes; - if (packed_size == download.Size()) { - flush(); - } - } + const auto [mapped, offset] = m_download_buffer.Map(total_size, 64); + if (mapped == nullptr) { + EXIT("BufferCache: download exceeds 32 MiB staging buffer capacity\n"); } - if (!batch.empty()) { - flush(); - } - for (const auto& copy: copies) { - m_gpu_modified_ranges.Subtract(copy.address, copy.size); + m_download_buffer.Commit(); + for (auto& copy: copies) { + copy.dstOffset += offset; } + + auto& command = m_scheduler.Current(); + command.EndRendering(); + const auto native = command.Handle(); + vk::BufferMemoryBarrier before {}; + before.srcAccessMask = vk::AccessFlagBits::eMemoryRead | vk::AccessFlagBits::eMemoryWrite; + before.dstAccessMask = vk::AccessFlagBits::eTransferRead; + before.srcQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + before.dstQueueFamilyIndex = VK_QUEUE_FAMILY_IGNORED; + before.buffer = buffer.Handle(); + before.offset = 0; + before.size = buffer.Size(); + native.pipelineBarrier(vk::PipelineStageFlagBits::eAllCommands, + vk::PipelineStageFlagBits::eTransfer, {}, 0, nullptr, 1, &before, 0, + nullptr); + native.copyBuffer(buffer.Handle(), m_download_buffer.Handle(), + static_cast(copies.size()), copies.data()); + + auto after = before; + after.srcAccessMask = vk::AccessFlagBits::eTransferWrite; + after.dstAccessMask = vk::AccessFlagBits::eHostRead; + after.buffer = m_download_buffer.Handle(); + after.offset = offset; + after.size = total_size; + native.pipelineBarrier(vk::PipelineStageFlagBits::eTransfer, + vk::PipelineStageFlagBits::eAllCommands | + vk::PipelineStageFlagBits::eHost, + {}, 0, nullptr, 1, &after, 0, nullptr); + m_scheduler.DeferPriorityOperation([this, mapped, offset, total_size, buffer_address, + copies = std::move(copies)] { + m_download_buffer.Invalidate(offset, total_size); + for (const auto& copy: copies) { + Libs::LibKernel::Memory::WriteBacking(buffer_address + copy.srcOffset, + mapped + (copy.dstOffset - offset), copy.size); + } + }); + return true; } BufferCache::BufferCache(GraphicContext& graphics, CommandScheduler& scheduler, @@ -264,21 +249,10 @@ void BufferCache::ReadMemory(uint64_t vaddr, uint64_t size, bool is_write) { const auto window_begin = std::max(Common::AlignDown(vaddr, WindowSize), buffer_begin); const auto window_end = std::min(std::max(window_begin + WindowSize, vaddr + size), buffer_end); - std::vector copies; - m_memory_tracker.ForEachDownloadRange( - window_begin, window_end - window_begin, - [&](uint64_t address, uint64_t bytes) noexcept { - m_memory_tracker.ValidateGpuDirtyPages(m_gpu_modified_ranges, address, bytes, - "memory invalidation"); - }, - [&](uint64_t address, uint64_t bytes) noexcept { - m_gpu_modified_ranges.ForEachInRange(address, bytes, [&](uint64_t start, uint64_t end) { - copies.push_back({&buffer, buffer.Offset(start), start, end - start}); - }); - }); - if (!copies.empty()) { - DownloadBufferMemory(copies); - // The enumeration covered whole dirty pages and every exact interval on them. + if (DownloadBufferMemory(buffer, window_begin, window_end - window_begin)) { + const auto tick = m_scheduler.CurrentTick(); + m_scheduler.Wait(tick); + m_scheduler.WaitPriorityOperations(tick); m_memory_tracker.UnmarkRegionAsGpuModified(window_begin, window_end - window_begin); } if (is_write) { @@ -618,8 +592,7 @@ void BufferCache::RunGarbageCollector() { const size_t limit = aggressive ? 64 : 32; std::vector dirty_buffers; - std::vector copies; - size_t retire_count = 0; + size_t retire_count = 0; m_lru_cache.ForEachItemBelow(tick - age, [&](BufferId id) { auto& buffer = m_slot_buffers[id]; EXIT_IF(buffer.is_deleted); @@ -630,19 +603,7 @@ void BufferCache::RunGarbageCollector() { return false; } if (dirty) { - m_memory_tracker.ForEachDownloadRange( - buffer.CpuAddress(), buffer.Size(), - [&](uint64_t dirty_address, uint64_t dirty_size) noexcept { - m_memory_tracker.ValidateGpuDirtyPages(m_gpu_modified_ranges, dirty_address, - dirty_size, "garbage collection"); - }, - [&](uint64_t dirty_address, uint64_t dirty_size) noexcept { - m_gpu_modified_ranges.ForEachInRange( - dirty_address, dirty_size, [&](uint64_t start, uint64_t end) { - copies.push_back({&buffer, start - buffer.CpuAddress(), start, - end - start}); - }); - }); + EXIT_IF(!DownloadBufferMemory(buffer, buffer.CpuAddress(), buffer.Size())); dirty_buffers.push_back(id); } else { m_memory_tracker.UntrackMemory(buffer.CpuAddress(), buffer.Size()); @@ -654,8 +615,10 @@ void BufferCache::RunGarbageCollector() { return; } - EXIT_IF(copies.empty()); - DownloadBufferMemory(copies); + // Publish all queued downloads before releasing their tracked pages and owners. + const auto completion_tick = m_scheduler.CurrentTick(); + m_scheduler.Wait(completion_tick); + m_scheduler.WaitPriorityOperations(completion_tick); for (const auto id: dirty_buffers) { auto& buffer = m_slot_buffers[id]; m_memory_tracker.UnmarkRegionAsGpuModified(buffer.CpuAddress(), buffer.Size()); diff --git a/src/graphics/host_gpu/renderer/cache/bufferCache.h b/src/graphics/host_gpu/renderer/cache/bufferCache.h index 924f9edf2..1ca6cae9f 100644 --- a/src/graphics/host_gpu/renderer/cache/bufferCache.h +++ b/src/graphics/host_gpu/renderer/cache/bufferCache.h @@ -88,11 +88,8 @@ private: bool has_stream_leap; }; - struct DownloadCopy; using PageTable = MultiLevelPageTable; static_assert(CACHING_PAGESIZE == (uint64_t {1} << PageTable::kPageBits)); - static constexpr uint64_t DOWNLOAD_ALIGNMENT = 64; - [[nodiscard]] static std::pair DownloadEnvelope(const DownloadCopy& copy); void WriteDataBuffer(Buffer& buffer, uint64_t address, const void* source, uint64_t size); void TouchBuffer(const Buffer& buffer); [[nodiscard]] OverlapResult ResolveOverlaps(uint64_t vaddr, uint64_t size); @@ -108,7 +105,8 @@ private: [[nodiscard]] vk::Buffer UploadCopies(Buffer& buffer, std::span copies, uint64_t total_size); [[nodiscard]] bool SynchronizeBufferFromImage(Buffer& buffer, uint64_t vaddr, uint64_t size); - void DownloadBufferMemory(std::span copies); + // Queues backing publication; callers wait before clearing dirty pages or reusing their data. + [[nodiscard]] bool DownloadBufferMemory(Buffer& buffer, uint64_t vaddr, uint64_t size); GraphicContext& m_graphics; CommandScheduler& m_scheduler; diff --git a/tests/MemoryTrackerTests.cpp b/tests/MemoryTrackerTests.cpp index 477211080..fb48d1d50 100644 --- a/tests/MemoryTrackerTests.cpp +++ b/tests/MemoryTrackerTests.cpp @@ -493,12 +493,15 @@ void TestGpuDownloadProtectionMirrors() { "non-clearing download changed protection or lost sparse ranges"); visited.clear(); + bool protected_during_download = false; tracker.ForEachDownloadRange( address + 16, 32, [&](uint64_t range_address, uint64_t range_size) noexcept { + protected_during_download = Protection(memory) == PAGE_NOACCESS; visited.push_back({range_address, range_size}); }); - Check(visited.size() == 1 && visited[0].first == address && + Check(protected_during_download && visited.size() == 1 && + visited[0].first == address && visited[0].second == page_size && g_protection_log.size() == 1 && g_protection_log[0].address == address && g_protection_log[0].size == page_size && @@ -641,11 +644,14 @@ void TestDownloadDoesNotSerializeDisjointRegion() { std::binary_semaphore finish_download{0}; std::binary_semaphore mutation_finished{0}; std::jthread downloader([&] { - tracker.ForEachDownloadRange(allocation_base, page_size, - [&](uint64_t, uint64_t) noexcept { - download_entered.release(); - finish_download.acquire(); - }); + tracker.ForEachDownloadRange( + allocation_base, second_region + page_size - allocation_base, + [&](uint64_t address, uint64_t) noexcept { + if (address == allocation_base) { + download_entered.release(); + finish_download.acquire(); + } + }); }); download_entered.acquire(); std::jthread mutation([&] { diff --git a/tests/ShaderRecompilerComputeTests.cpp b/tests/ShaderRecompilerComputeTests.cpp index a3b3e0068..d6aeab3e1 100644 --- a/tests/ShaderRecompilerComputeTests.cpp +++ b/tests/ShaderRecompilerComputeTests.cpp @@ -4054,7 +4054,7 @@ public: "partial invalidation lost disjoint native bytes"); constexpr uint64_t large_offset = 0x10000; - constexpr uint64_t large_size = 33ull * 1024 * 1024; + constexpr uint64_t large_size = 32ull << 20; constexpr uint32_t large_value = 0x5aa55aa5u; constexpr uint32_t large_stale = 0x12345678u; std::memcpy(memory + large_offset, &large_stale, sizeof(large_stale)); @@ -4066,12 +4066,17 @@ public: large_allocation.first != nullptr, "failed to allocate the near-capacity dirty native buffer"); cache.FillBuffer(base + large_offset, large_size, large_value, false); + const auto large_submission_tick = scheduler.CurrentTick(); for (uint32_t tick = 0; tick <= 160; tick++) { cache.RunGarbageCollector(); } + // Earlier dirty owners in this GC pass may require one ring-wrap drain + // before the full-capacity download can reserve the stream. Require(name, "near-capacity synchronized retirement", - !cache.IsRegionRegistered(base + large_offset, large_size), - "near-capacity dirty Buffer survived pressured collection"); + !cache.IsRegionRegistered(base + large_offset, large_size) && + scheduler.CurrentTick() > large_submission_tick && + scheduler.CurrentTick() <= large_submission_tick + 2, + "capacity-sized dirty Buffer exceeded one ring-wrap drain"); uint32_t large_before_completion = 0; Libs::LibKernel::Memory::TryReadBacking(base + large_offset, &large_before_completion, @@ -4088,15 +4093,13 @@ public: fixed_download->Handle() == fixed_download_handle && fixed_download->Size() == (32ull << 20), "image acquisition replaced the shared Buffer download stream"); - uint32_t large_first = 0; - uint32_t large_last = 0; - Libs::LibKernel::Memory::TryReadBacking(base + large_offset, &large_first, - sizeof(large_first)); - Libs::LibKernel::Memory::TryReadBacking(base + large_offset + large_size - - sizeof(large_last), - &large_last, sizeof(large_last)); + std::vector large_published(large_size / sizeof(uint32_t)); Require(name, "near-capacity Buffer publication contents", - large_first == large_value && large_last == large_value, + Libs::LibKernel::Memory::TryReadBacking( + base + large_offset, large_published.data(), large_size) && + std::ranges::all_of(large_published, [](uint32_t value) { + return value == large_value; + }), "near-capacity Buffer GC did not publish its complete transfer"); constexpr uint64_t grouped_first_offset = 0x10000; @@ -4149,6 +4152,94 @@ public: grouped_second_backing == grouped_second_value, "per-owner GC transfers lost data while wrapping the fixed ring"); + constexpr uint64_t sparse_offset = 0x2180000; + constexpr uint64_t sparse_owner_stride = 2 * BufferCache::CACHING_PAGESIZE; + constexpr std::array sparse_starts{1, 7, 13}; + constexpr std::array sparse_sizes{3, 2, 1}; + constexpr std::array sparse_values{0x41372b19u, 0x957f6953u}; + std::array sparse_clean{}; + sparse_clean.fill(0xcdu); + const auto FillSparseOwner = [&](uint32_t owner_index) { + const auto address = base + sparse_offset + + owner_index * sparse_owner_stride; + Libs::LibKernel::Memory::WriteBacking(address, sparse_clean.data(), + sparse_clean.size()); + Libs::Graphics::Buffer *owner = nullptr; + for (size_t index = 0; index < sparse_starts.size(); ++index) { + const auto allocation = cache.ObtainBuffer( + address + sparse_starts[index], sparse_sizes[index], true, false); + Require(name, "sparse byte-range owner", + allocation.first != nullptr && + (owner == nullptr || owner == allocation.first), + "disjoint byte ranges did not resolve to one cached owner"); + owner = allocation.first; + } + // Deliberately change native clean neighbors too: only the exact dirty + // bytes may be published to guest memory. + owner->Fill(owner->Offset(address), sparse_clean.size(), + sparse_values[owner_index]); + }; + const auto CheckSparseOwner = [&](uint32_t owner_index) { + const auto address = base + sparse_offset + + owner_index * sparse_owner_stride; + auto expected = sparse_clean; + std::array value_bytes{}; + std::memcpy(value_bytes.data(), &sparse_values[owner_index], + value_bytes.size()); + for (size_t index = 0; index < sparse_starts.size(); ++index) { + for (uint64_t byte = sparse_starts[index]; + byte < sparse_starts[index] + sparse_sizes[index]; ++byte) { + expected[byte] = value_bytes[byte % value_bytes.size()]; + } + } + std::array published{}; + Require(name, "sparse byte-range publication", + Libs::LibKernel::Memory::TryReadBacking( + address, published.data(), published.size()) && + published == expected && + !cache.HasGpuDirtyBytes(address, published.size()) && + !cache.IsRegionGpuModified(address, published.size()), + "odd-byte readback lost dirty bytes or overwrote clean neighbors"); + }; + FillSparseOwner(0); + bool sparse_callback_ran = false; + bool sparse_callback_clean = false; + scheduler.DeferOperation([&] { + sparse_callback_clean = + !cache.HasGpuDirtyBytes(base + sparse_offset, sparse_clean.size()) && + !cache.IsRegionGpuModified(base + sparse_offset, sparse_clean.size()); + sparse_callback_ran = true; + }); + const auto sparse_read_tick = scheduler.CurrentTick(); + cache.ReadMemory(base + sparse_offset + sparse_starts[0], sparse_sizes[0]); + Require(name, "sparse byte-range submission", + scheduler.CurrentTick() == sparse_read_tick + 1, + "sparse source ranges were downloaded in separate submissions"); + scheduler.PopPendingOperations(); + Require(name, "readback callback ownership consistency", + sparse_callback_ran && sparse_callback_clean, + "readback drained a deferred callback before clearing page ownership"); + CheckSparseOwner(0); + + FillSparseOwner(0); + FillSparseOwner(1); + Require(name, "sparse disjoint GC owners", + BufferCacheTestAccess::PageOwner(cache, base + sparse_offset) != + BufferCacheTestAccess::PageOwner( + cache, base + sparse_offset + sparse_owner_stride), + "sparse GC fixtures merged into one source owner"); + const auto sparse_gc_tick = scheduler.CurrentTick(); + for (uint32_t tick = 0; tick <= 160; ++tick) { + cache.RunGarbageCollector(); + } + Require(name, "sparse multi-owner GC submission", + scheduler.CurrentTick() == sparse_gc_tick + 1 && + !cache.IsRegionRegistered(base + sparse_offset, + sparse_owner_stride + 16), + "sparse GC owners did not share one completion drain"); + CheckSparseOwner(0); + CheckSparseOwner(1); + constexpr uint64_t disjoint_owner_offset = 0x2140000; constexpr uint64_t disjoint_owner_size = 0x8000; constexpr uint64_t disjoint_dirty_offset = disjoint_owner_offset + 0x100;