From 2849e970cb1f5d4277d557d48639f6483ec93ee6 Mon Sep 17 00:00:00 2001 From: Lander Gallastegi Date: Sun, 13 Jul 2025 23:27:07 +0200 Subject: [PATCH] Fix segfault with clear mask --- src/common/func_traits.h | 28 ++++++++++-- src/video_core/buffer_cache/buffer_cache.cpp | 13 +++++- .../buffer_cache/region_definitions.h | 7 ++- src/video_core/buffer_cache/region_manager.h | 44 ++++++++++--------- 4 files changed, 62 insertions(+), 30 deletions(-) diff --git a/src/common/func_traits.h b/src/common/func_traits.h index 407b2dbe6..fe938c613 100644 --- a/src/common/func_traits.h +++ b/src/common/func_traits.h @@ -3,23 +3,43 @@ #pragma once +#include #include namespace Common { -template -struct FuncTraits {}; +template +struct FuncTraits; +// Function type template -struct FuncTraits { +struct FuncTraits { using ReturnType = ReturnType_; - static constexpr size_t NUM_ARGS = sizeof...(Args); template using ArgType = std::tuple_element_t>; }; +// Function pointer +template +struct FuncTraits : FuncTraits {}; + +// Member function pointer +template +struct FuncTraits : FuncTraits {}; + +template +struct FuncTraits + : FuncTraits {}; + +// Catch-all for callables +template +struct FuncTraits::operator())>> + : FuncTraits::operator())> {}; + + +// For lambdas: for compat (may be removed) template struct LambdaTraits : LambdaTraits::operator())> {}; diff --git a/src/video_core/buffer_cache/buffer_cache.cpp b/src/video_core/buffer_cache/buffer_cache.cpp index c248ca6cf..4fbf8998b 100644 --- a/src/video_core/buffer_cache/buffer_cache.cpp +++ b/src/video_core/buffer_cache/buffer_cache.cpp @@ -1064,8 +1064,11 @@ void BufferCache::SynchronizeBuffersForDma() { copies.clear(); }; mapped_ranges.ForEach([&](VAddr device_addr, u64 size) { - memory_tracker->ForEachUploadRange(device_addr, size, false, [&](u64 device_addr_out, u64 range_size) { + RENDERER_TRACE; + memory_tracker->ForEachUploadRange(device_addr, size, false, [&](u64 device_addr_out, u64 range_size, RegionBits& clear_mask) { + RENDERER_TRACE; ForEachBufferInRange(device_addr_out, range_size, [&](BufferId buffer_id, Buffer& buffer) { + RENDERER_TRACE; if (last_buffer_id != buffer_id) { upload_pending(); last_buffer_id = buffer_id; @@ -1082,6 +1085,14 @@ void BufferCache::SynchronizeBuffersForDma() { .dstOffset = copy_start - buffer.CpuAddr(), .size = copy_size, }); + + // We need to use tracker page size here, we are marking the clear mask + const u64 page_start = (copy_start & TRACKER_HIGHER_PAGE_MASK) >> TRACKER_PAGE_BITS; + const u64 page_end = + Common::DivCeil((copy_end - 1) & TRACKER_HIGHER_PAGE_MASK, TRACKER_BYTES_PER_PAGE); + ASSERT(page_start < page_end); + LOG_WARNING(Render_Vulkan, "Page start {:#x}, end {:#x}", page_start, page_end); + clear_mask.SetRange(page_start, page_end); }); }, upload_pending); }); diff --git a/src/video_core/buffer_cache/region_definitions.h b/src/video_core/buffer_cache/region_definitions.h index af25226f5..760096df2 100644 --- a/src/video_core/buffer_cache/region_definitions.h +++ b/src/video_core/buffer_cache/region_definitions.h @@ -18,11 +18,10 @@ constexpr u64 TRACKER_HIGHER_PAGE_MASK = TRACKER_HIGHER_PAGE_SIZE - 1ULL; constexpr u64 NUM_PAGES_PER_REGION = TRACKER_HIGHER_PAGE_SIZE / TRACKER_BYTES_PER_PAGE; enum class Type { - None = 0, - CPU = 1 << 0, - GPU = 1 << 1, + None, + CPU, + GPU, }; -DECLARE_ENUM_FLAG_OPERATORS(Type) using RegionBits = Common::BitArray; diff --git a/src/video_core/buffer_cache/region_manager.h b/src/video_core/buffer_cache/region_manager.h index 677811620..619acd802 100644 --- a/src/video_core/buffer_cache/region_manager.h +++ b/src/video_core/buffer_cache/region_manager.h @@ -5,6 +5,7 @@ #include "common/config.h" #include "common/div_ceil.h" +#include "common/func_traits.h" #ifdef __linux__ #include "common/adaptive_mutex.h" @@ -69,20 +70,6 @@ public: } } - template - void PerformDeferredProtections() { - bool was_deferred = True(deferred_protection & type); - if (!was_deferred) { - return; - } - deferred_protection &= ~type; - if constexpr (type == Type::CPU) { - UpdateProtection(); - } else if constexpr (type == Type::GPU) { - UpdateProtection(); - } - } - /** * Change the state of a range of pages * @@ -121,9 +108,11 @@ public: * @param size Size in bytes of the CPU range to loop over * @param func Function to call for each turned off region */ - template - void ForEachModifiedRange(VAddr query_cpu_range, s64 size, auto&& func) { + template + void ForEachModifiedRange(VAddr query_cpu_range, s64 size, F&& func) { RENDERER_TRACE; + using FuncTraits = Common::FuncTraits; + constexpr bool uses_clear_mask = FuncTraits::NUM_ARGS == 3; const size_t offset = query_cpu_range - cpu_addr; const size_t start_page = SanitizeAddress(offset) / TRACKER_BYTES_PER_PAGE; const size_t end_page = @@ -135,18 +124,31 @@ public: RegionBits& bits = GetRegionBits(); RegionBits mask(bits, start_page, end_page); + if constexpr (uses_clear_mask) { + static_assert(clear, "Function must not use clear mask when not clearing"); + RegionBits clear_mask; + for (const auto& [start, end] : mask) { + func(cpu_addr + start * TRACKER_BYTES_PER_PAGE, + (end - start) * TRACKER_BYTES_PER_PAGE, clear_mask); + } + bits &= ~clear_mask; + } else { + for (const auto& [start, end] : mask) { + func(cpu_addr + start * TRACKER_BYTES_PER_PAGE, + (end - start) * TRACKER_BYTES_PER_PAGE); + } + if constexpr (clear) { + bits &= ~mask; + } + } + if constexpr (clear) { - bits.UnsetRange(start_page, end_page); if constexpr (type == Type::CPU) { UpdateProtection(); } else if (Config::readbacks()) { UpdateProtection(); } } - - for (const auto& [start, end] : mask) { - func(cpu_addr + start * TRACKER_BYTES_PER_PAGE, (end - start) * TRACKER_BYTES_PER_PAGE); - } } /**