Gpu buffer readback to cpu. Fixing visible glitches such as missing collision on some object, but can extend to more.

Assisted-by: Claude Opus 5.5 (debugging & implementation)

8 files changed, 191 insertions, 2 deletions

diff --git a/src/core/device_memory_manager.h b/src/core/device_memory_manager.h
index 9c034f01a1..27ff342213 100644
--- a/src/core/device_memory_manager.h
+++ b/src/core/device_memory_manager.h
@@ -126,6 +126,9 @@ public:
     // New batch API to update multiple ranges with a single lock acquisition.
     void UpdatePagesCachedBatch(std::span<const std::pair<DAddr, size_t>> ranges, s32 delta);
 
+    /// Traps the next guest CPU read of these pages, to learn which GPU writes the CPU reads.
+    void ArmReadTraps(DAddr addr, size_t size);
+
 private:
     struct TranslationEntry {
         DAddr guest_page{};
diff --git a/src/core/device_memory_manager.inc b/src/core/device_memory_manager.inc
index c45bc71656..83faa1b3f3 100644
--- a/src/core/device_memory_manager.inc
+++ b/src/core/device_memory_manager.inc
@@ -692,4 +692,36 @@ void DeviceMemoryManager<Traits>::UpdatePagesCachedBatch(std::span<const std::pa
     }
 }
 
+template <typename Traits>
+void DeviceMemoryManager<Traits>::ArmReadTraps(DAddr addr, size_t size) {
+    const auto MarkRegionReadTrap = &DeviceMemoryManager<Traits>::DeviceMethods::MarkRegionReadTrap;
+    Memory::Memory* run_inter = nullptr;
+    u64 run_begin = 0;
+    u64 run_pages = 0;
+    const auto release_run = [&] {
+        if (run_pages > 0 && run_inter != nullptr) {
+            MarkRegionReadTrap(run_inter, run_begin << Memory::YUZU_PAGEBITS,
+                               run_pages << Memory::YUZU_PAGEBITS);
+        }
+        run_pages = 0;
+    };
+    const size_t page_end = Common::DivCeil(addr + size, Memory::YUZU_PAGESIZE);
+    for (size_t page = addr >> Memory::YUZU_PAGEBITS; page != page_end; ++page) {
+        auto [asid, vaddr] = ExtractCPUBacking(page);
+        const u64 vpage = vaddr >> Memory::YUZU_PAGEBITS;
+        if (vpage == 0) {
+            release_run();
+            continue;
+        }
+        auto* const inter = registered_processes[asid.id];
+        if (inter != run_inter || vpage != run_begin + run_pages) {
+            release_run();
+            run_inter = inter;
+            run_begin = vpage;
+        }
+        ++run_pages;
+    }
+    release_run();
+}
+
 } // namespace Core
diff --git a/src/core/memory.cpp b/src/core/memory.cpp
index 6e0ab0ee6d..cfc39916db 100644
--- a/src/core/memory.cpp
+++ b/src/core/memory.cpp
@@ -8,10 +8,13 @@
 #include <atomic>
 #include <algorithm>
 #include <cstring>
+#include <cstdlib>
 #include <mutex>
+#include <set>
 #include <span>
 #include <thread>
 #include <type_traits>
+#include <utility>
 #include <vector>
 
 #include "common/assert.h"
@@ -468,6 +471,9 @@ struct Memory::Impl {
                 perm |= Common::MemoryPermission::Write;
             }
             host_buffer->Protect(vaddr, size, perm);
+            if (cached && perm != Common::MemoryPermission{}) {
+                ProtectArmedPages(vaddr, size);
+            }
         }
 
         // Iterate over a contiguous CPU address space, which corresponds to the specified GPU
@@ -528,6 +534,84 @@ struct Memory::Impl {
         }
     }
 
+    /// Read traps replace reactive flushing's blanket read protection of every cached page: only
+    /// pages the GPU writes are protected, and each one only until its first read
+    /// (EDEN_NX2_READ_TRAPS=0 turns them off). Under NCE every fault is taken as a write, so they
+    /// are not used there.
+    bool ReadTrapsEnabled() const {
+        static const bool disabled = [] {
+            const char* value = std::getenv("EDEN_NX2_READ_TRAPS");
+            return value != nullptr && value[0] == '0';
+        }();
+        return !disabled && current_page_table->fastmem_arena &&
+               !Settings::values.use_reactive_flushing.GetValue() &&
+               Settings::values.cpu_backend.GetValue() != Settings::CpuBackend::Nce;
+    }
+
+    void RasterizerArmReadTraps(u64 vaddr, u64 size) {
+        if (vaddr == 0 || size == 0 || !AddressSpaceContains(*current_page_table, vaddr, size) ||
+            !ReadTrapsEnabled()) {
+            return;
+        }
+        const u64 page_end = ((vaddr + size - 1) >> YUZU_PAGEBITS) + 1;
+        std::scoped_lock lk{read_trap_mutex};
+        u64 run_begin = 0;
+        u64 run_pages = 0;
+        const auto protect_run = [&] {
+            if (run_pages > 0) {
+                // Reads that hit a cached read area must not skip the flush of a new trap.
+                read_trap_generation.fetch_add(1, std::memory_order_release);
+                host_buffer->Protect(run_begin << YUZU_PAGEBITS, run_pages << YUZU_PAGEBITS,
+                                     Common::MemoryPermission{});
+                run_pages = 0;
+            }
+        };
+        for (u64 page = vaddr >> YUZU_PAGEBITS; page != page_end; ++page) {
+            const bool cached = std::as_const(current_page_table->entries)[page].Type() ==
+                                Common::PageType::RasterizerCachedMemory;
+            if (!cached || !armed_read_traps.insert(page).second) {
+                protect_run();
+                continue;
+            }
+            armed_read_trap_count.fetch_add(1, std::memory_order_relaxed);
+            if (run_pages == 0) {
+                run_begin = page;
+            }
+            ++run_pages;
+        }
+        protect_run();
+    }
+
+    /// Takes read permission away again from armed pages that were just cached again.
+    void ProtectArmedPages(u64 vaddr, u64 size) {
+        if (armed_read_trap_count.load(std::memory_order_relaxed) == 0) {
+            return;
+        }
+        std::scoped_lock lk{read_trap_mutex};
+        const u64 page_end = ((vaddr + size - 1) >> YUZU_PAGEBITS) + 1;
+        for (auto it = armed_read_traps.lower_bound(vaddr >> YUZU_PAGEBITS);
+             it != armed_read_traps.end() && *it < page_end; ++it) {
+            host_buffer->Protect(*it << YUZU_PAGEBITS, YUZU_PAGESIZE, Common::MemoryPermission{});
+        }
+    }
+
+    /// A slow-path read of a cached page: its trap, if any, has done its job.
+    void DisarmReadTraps(u64 vaddr, size_t size) {
+        if (armed_read_trap_count.load(std::memory_order_relaxed) == 0) {
+            return;
+        }
+        std::scoped_lock lk{read_trap_mutex};
+        const u64 page_end = ((vaddr + size - 1) >> YUZU_PAGEBITS) + 1;
+        auto it = armed_read_traps.lower_bound(vaddr >> YUZU_PAGEBITS);
+        while (it != armed_read_traps.end() && *it < page_end) {
+            // Still cached (this read came through the rasterizer path): writes stay trapped.
+            host_buffer->Protect(*it << YUZU_PAGEBITS, YUZU_PAGESIZE,
+                                 Common::MemoryPermission::Read);
+            it = armed_read_traps.erase(it);
+            armed_read_trap_count.fetch_sub(1, std::memory_order_relaxed);
+        }
+    }
+
     /**
      * Maps a region of pages as a specific type.
      *
@@ -698,12 +782,18 @@ struct Memory::Impl {
         }
         const size_t core = system.GetCurrentHostThreadID();
         auto& current_area = rasterizer_read_areas[core];
+        if (const u64 generation = read_trap_generation.load(std::memory_order_acquire);
+            read_area_generations[core] != generation) {
+            current_area = {};
+            read_area_generations[core] = generation;
+        }
         gpu_device_memory->ApplyOpOnPointer(p, scratch_buffers[core], [&](DAddr address) {
             const DAddr end_address = address + size;
             if (current_area.start_address <= address && end_address <= current_area.end_address)
                 [[likely]] {
                 return;
             }
+            DisarmReadTraps(v_address, size);
             current_area = system.GPU().OnCPURead(address, size);
         });
     }
@@ -777,6 +867,11 @@ struct Memory::Impl {
     Common::PageTable* current_page_table = nullptr;
 
     std::array<VideoCore::RasterizerDownloadArea, Core::Hardware::NUM_CPU_CORES> rasterizer_read_areas{};
+    std::array<u64, Core::Hardware::NUM_CPU_CORES> read_area_generations{};
+    std::atomic<u64> read_trap_generation{};
+    std::set<u64> armed_read_traps;
+    std::atomic<size_t> armed_read_trap_count{};
+    std::mutex read_trap_mutex;
     std::array<GPUDirtyState, Core::Hardware::NUM_CPU_CORES> rasterizer_write_areas{};
     std::array<Common::ScratchBuffer<u32>, Core::Hardware::NUM_CPU_CORES> scratch_buffers{};
     std::span<Core::GPUDirtyMemoryManager> gpu_dirty_managers;
@@ -969,6 +1064,10 @@ void Memory::RasterizerMarkRegionCached(Common::ProcessAddress vaddr, u64 size,
     impl->RasterizerMarkRegionCached(GetInteger(vaddr), size, cached);
 }
 
+void Memory::RasterizerArmReadTraps(Common::ProcessAddress vaddr, u64 size) {
+    impl->RasterizerArmReadTraps(GetInteger(vaddr), size);
+}
+
 void Memory::MarkRegionDebug(Common::ProcessAddress vaddr, u64 size, bool debug) {
     impl->MarkRegionDebug(GetInteger(vaddr), size, debug);
 }
diff --git a/src/core/memory.h b/src/core/memory.h
index 204b7514f6..2b418b8864 100644
--- a/src/core/memory.h
+++ b/src/core/memory.h
@@ -484,6 +484,12 @@ public:
     void RasterizerMarkRegionCached(Common::ProcessAddress vaddr, u64 size, bool cached);
 
     /**
+     * Makes the next guest read of each rasterizer cached page in the range take the slow path
+     * (fastmem only), so that the read reaches the GPU's reactive flush once.
+     */
+    void RasterizerArmReadTraps(Common::ProcessAddress vaddr, u64 size);
+
+    /**
      * Marks each page within the specified address range as debug or non-debug.
      * Debug addresses are not accessible from fastmem pointers.
      *
diff --git a/src/video_core/buffer_cache/buffer_cache.h b/src/video_core/buffer_cache/buffer_cache.h
index 42d7ba066d..e90af3214d 100644
--- a/src/video_core/buffer_cache/buffer_cache.h
+++ b/src/video_core/buffer_cache/buffer_cache.h
@@ -279,8 +279,10 @@ bool BufferCache<P>::DMACopy(GPUVAddr src_address, GPUVAddr dest_address, u64 am
         const DAddr diff = base_address - *cpu_src_address;
         const DAddr new_base_address = *cpu_dest_address + diff;
         tmp_intervals.push_back({new_base_address, size});
-        if (AsyncBufferDownloads()) {
+        if (DownloadsAtFence(new_base_address, size)) {
             uncommitted_gpu_modified_ranges.Add(new_base_address, size);
+        } else {
+            ArmReadTraps(new_base_address, size);
         }
     };
     gpu_modified_ranges.ForEachInRange(*cpu_src_address, amount, mirror);
@@ -1603,8 +1605,10 @@ void BufferCache<P>::MarkWrittenBuffer(BufferId buffer_id, DAddr device_addr, u3
     }
     memory_tracker.MarkRegionAsGpuModified(device_addr, size);
     gpu_modified_ranges.Add(device_addr, size);
-    if (AsyncBufferDownloads()) {
+    if (DownloadsAtFence(device_addr, size)) {
         uncommitted_gpu_modified_ranges.Add(device_addr, size);
+    } else {
+        ArmReadTraps(device_addr, size);
     }
 }
 
@@ -2090,6 +2094,8 @@ void BufferCache<P>::DeleteBuffer(BufferId buffer_id, bool do_not_mark) {
     if constexpr (requires { runtime.OnBufferDeleted(slot_buffers[buffer_id]); }) {
         runtime.OnBufferDeleted(slot_buffers[buffer_id]);
     }
+    read_trapped_ranges.Subtract(slot_buffers[buffer_id].CpuAddr(),
+                                 slot_buffers[buffer_id].SizeBytes());
     bool dirty_index{false};
     boost::container::small_vector<u64, NUM_VERTEX_BUFFERS> dirty_vertex_buffers;
     const auto scalar_replace = [buffer_id](Binding& binding) {
diff --git a/src/video_core/buffer_cache/buffer_cache_base.h b/src/video_core/buffer_cache/buffer_cache_base.h
index 45a1836341..d8a61b7913 100644
--- a/src/video_core/buffer_cache/buffer_cache_base.h
+++ b/src/video_core/buffer_cache/buffer_cache_base.h
@@ -245,6 +245,12 @@ public:
 
     std::optional<VideoCore::RasterizerDownloadArea> GetFlushArea(DAddr device_addr, u64 size);
 
+    /// Without downloads at every fence, a guest CPU read is the only way GPU-written buffer
+    /// data reaches guest memory, so reads must ask the buffer cache (GetFlushArea).
+    bool FlushesOnCpuRead() const {
+        return !AsyncBufferDownloads();
+    }
+
     bool InlineMemory(DAddr dest_address, size_t copy_size, std::span<const u8> inlined_buffer);
 
     void BindGraphicsUniformBuffer(size_t stage, u32 index, GPUVAddr gpu_addr, u32 size);
@@ -536,6 +542,31 @@ private:
         }
         return gpu_memory == nullptr || !gpu_memory->UsesNativeTiling();
     }
+
+    /// Whether a GPU write to this range is copied back at the next fence. Ranges the guest CPU
+    /// has read (Preflushable, set by the first reactive flush) always are: later reads skip the
+    /// flush and rely on these downloads, e.g. collision meshes skinned by compute shaders.
+    bool DownloadsAtFence(DAddr device_addr, u64 size) noexcept {
+        return AsyncBufferDownloads() || memory_tracker.IsRegionPreflushable(device_addr, size);
+    }
+
+    /// GPU writes that are not downloaded at fences: trap the guest CPU's next read of their
+    /// pages, which flushes them once and marks them Preflushable (then DownloadsAtFence).
+    void ArmReadTraps(DAddr device_addr, u64 size) {
+        const DAddr begin = Common::AlignDown(device_addr, Core::DEVICE_PAGESIZE);
+        const DAddr end = Common::AlignUp(device_addr + size, Core::DEVICE_PAGESIZE);
+        u64 armed_bytes = 0;
+        read_trapped_ranges.ForEachInRange(begin, end - begin, [&](DAddr lower, DAddr upper) {
+            armed_bytes += upper - lower;
+        });
+        if (armed_bytes == end - begin) {
+            return;
+        }
+        read_trapped_ranges.Add(begin, end - begin);
+        device_memory.ArmReadTraps(begin, end - begin);
+    }
+
+    Common::RangeSet<DAddr> read_trapped_ranges;
     Common::RangeSet<DAddr> gpu_modified_ranges;
     std::deque<Common::RangeSet<DAddr>> committed_gpu_modified_ranges;
 
diff --git a/src/video_core/host1x/gpu_device_memory_manager.cpp b/src/video_core/host1x/gpu_device_memory_manager.cpp
index 668c2f08b8..3d7d88b92d 100644
--- a/src/video_core/host1x/gpu_device_memory_manager.cpp
+++ b/src/video_core/host1x/gpu_device_memory_manager.cpp
@@ -12,6 +12,11 @@ struct MaxwellDeviceMethods {
                                          size_t size, bool caching) {
         interface->RasterizerMarkRegionCached(address, size, caching);
     }
+
+    static inline void MarkRegionReadTrap(Core::Memory::Memory* interface, VAddr address,
+                                          size_t size) {
+        interface->RasterizerArmReadTraps(address, size);
+    }
 };
 
 } // namespace Tegra
diff --git a/src/video_core/renderer_vulkan/vk_rasterizer.cpp b/src/video_core/renderer_vulkan/vk_rasterizer.cpp
index 0826820714..cb31a0e7e4 100644
--- a/src/video_core/renderer_vulkan/vk_rasterizer.cpp
+++ b/src/video_core/renderer_vulkan/vk_rasterizer.cpp
@@ -1006,6 +1006,13 @@ VideoCore::RasterizerDownloadArea RasterizerVulkan::GetFlushArea(DAddr addr, u64
             return *area;
         }
     }
+    if (buffer_cache.FlushesOnCpuRead()) {
+        std::scoped_lock lock{buffer_cache.mutex};
+        auto area = buffer_cache.GetFlushArea(addr, size);
+        if (area) {
+            return *area;
+        }
+    }
     VideoCore::RasterizerDownloadArea new_area{
         .start_address = Common::AlignDown(addr, Core::DEVICE_PAGESIZE),
         .end_address = Common::AlignUp(addr + size, Core::DEVICE_PAGESIZE),