From da464d84bcd6dfd94a14c3d79e7be838520b8e93 Mon Sep 17 00:00:00 2001 From: PixelyIon Date: Fri, 29 Jul 2022 02:29:18 +0530 Subject: [PATCH] Consolidate `NCE::TrapRegions` functionality into `CreateTrap` `NCE::TrapRegions` was a bit too overloaded as a method as it implicitly trapped which was unnecessary in all current usage cases, this has now been made more explicit by consolidating the functionality into `NCE::CreateTrap` which handles just creation of the trap and nothing past that, `RetrapRegions` has been renamed to `TrapRegions` and handles all trapping now. Co-authored-by: Billy Laws --- app/src/main/cpp/skyline/gpu/buffer.cpp | 8 ++++---- app/src/main/cpp/skyline/gpu/texture/texture.cpp | 12 ++++++------ app/src/main/cpp/skyline/nce.cpp | 12 +++++------- app/src/main/cpp/skyline/nce.h | 9 ++++----- 4 files changed, 19 insertions(+), 22 deletions(-) diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp index f5b6a3f6..2a21b38b 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.cpp +++ b/app/src/main/cpp/skyline/gpu/buffer.cpp @@ -17,7 +17,7 @@ namespace skyline::gpu { // We can't just capture this in the lambda since the lambda could exceed the lifetime of the buffer std::weak_ptr weakThis{shared_from_this()}; - trapHandle = gpu.state.nce->TrapRegions(*guest, true, [weakThis] { + trapHandle = gpu.state.nce->CreateTrap(*guest, [weakThis] { auto buffer{weakThis.lock()}; if (!buffer) return; @@ -107,7 +107,7 @@ namespace skyline::gpu { BlockAllCpuBackingWrites(); AdvanceSequence(); // The GPU will modify buffer contents so advance to the next sequence - gpu.state.nce->RetrapRegions(*trapHandle, false); + gpu.state.nce->TrapRegions(*trapHandle, false); } void Buffer::WaitOnFence() { @@ -163,7 +163,7 @@ namespace skyline::gpu { AdvanceSequence(); // We are modifying GPU backing contents so advance to the next sequence if (!skipTrap) - gpu.state.nce->RetrapRegions(*trapHandle, true); // Trap any future CPU writes to this buffer, must be done before the memcpy so that any modifications during the copy are tracked + gpu.state.nce->TrapRegions(*trapHandle, true); // Trap any future CPU writes to this buffer, must be done before the memcpy so that any modifications during the copy are tracked std::memcpy(backing.data(), mirror.data(), mirror.size()); } @@ -189,7 +189,7 @@ namespace skyline::gpu { } if (!skipTrap) - gpu.state.nce->RetrapRegions(*trapHandle, true); + gpu.state.nce->TrapRegions(*trapHandle, true); return true; } diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.cpp b/app/src/main/cpp/skyline/gpu/texture/texture.cpp index 35e1fce6..dc971bf3 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.cpp +++ b/app/src/main/cpp/skyline/gpu/texture/texture.cpp @@ -145,7 +145,7 @@ namespace skyline::gpu { // We can't just capture `this` in the lambda since the lambda could exceed the lifetime of the buffer std::weak_ptr weakThis{weak_from_this()}; - trapHandle = gpu.state.nce->TrapRegions(mappings, true, [weakThis] { + trapHandle = gpu.state.nce->CreateTrap(mappings, [weakThis] { auto texture{weakThis.lock()}; if (!texture) return; @@ -690,7 +690,7 @@ namespace skyline::gpu { if (gpuDirty && dirtyState == DirtyState::Clean) { // If a texture is Clean then we can just transition it to being GPU dirty and retrap it dirtyState = DirtyState::GpuDirty; - gpu.state.nce->RetrapRegions(*trapHandle, false); + gpu.state.nce->TrapRegions(*trapHandle, false); return; } else if (dirtyState != DirtyState::CpuDirty) { return; // If the texture has not been modified on the CPU, there is no need to synchronize it @@ -709,7 +709,7 @@ namespace skyline::gpu { cycle = lCycle; } - gpu.state.nce->RetrapRegions(*trapHandle, !gpuDirty); // Trap any future CPU reads (optionally) + writes to this texture + gpu.state.nce->TrapRegions(*trapHandle, !gpuDirty); // Trap any future CPU reads (optionally) + writes to this texture } void Texture::SynchronizeHostInline(const vk::raii::CommandBuffer &commandBuffer, const std::shared_ptr &pCycle, bool gpuDirty) { @@ -722,7 +722,7 @@ namespace skyline::gpu { std::scoped_lock lock{stateMutex}; if (gpuDirty && dirtyState == DirtyState::Clean) { dirtyState = DirtyState::GpuDirty; - gpu.state.nce->RetrapRegions(*trapHandle, false); + gpu.state.nce->TrapRegions(*trapHandle, false); return; } else if (dirtyState != DirtyState::CpuDirty) { return; @@ -739,7 +739,7 @@ namespace skyline::gpu { cycle = pCycle; } - gpu.state.nce->RetrapRegions(*trapHandle, !gpuDirty); // Trap any future CPU reads (optionally) + writes to this texture + gpu.state.nce->TrapRegions(*trapHandle, !gpuDirty); // Trap any future CPU reads (optionally) + writes to this texture } void Texture::SynchronizeGuest(bool cpuDirty, bool skipTrap) { @@ -790,7 +790,7 @@ namespace skyline::gpu { if (cpuDirty) gpu.state.nce->DeleteTrap(*trapHandle); else - gpu.state.nce->RetrapRegions(*trapHandle, true); // Trap any future CPU writes to this texture + gpu.state.nce->TrapRegions(*trapHandle, true); // Trap any future CPU writes to this texture } std::shared_ptr Texture::GetView(vk::ImageViewType type, vk::ImageSubresourceRange range, texture::Format pFormat, vk::ComponentMapping mapping) { diff --git a/app/src/main/cpp/skyline/nce.cpp b/app/src/main/cpp/skyline/nce.cpp index aa284715..894cc35e 100644 --- a/app/src/main/cpp/skyline/nce.cpp +++ b/app/src/main/cpp/skyline/nce.cpp @@ -535,17 +535,15 @@ namespace skyline::nce { constexpr NCE::TrapHandle::TrapHandle(const TrapMap::GroupHandle &handle) : TrapMap::GroupHandle(handle) {} - NCE::TrapHandle NCE::TrapRegions(span> regions, bool writeOnly, const LockCallback &lockCallback, const TrapCallback &readCallback, const TrapCallback &writeCallback) { - TRACE_EVENT("host", "NCE::TrapRegions"); + NCE::TrapHandle NCE::CreateTrap(span> regions, const LockCallback &lockCallback, const TrapCallback &readCallback, const TrapCallback &writeCallback) { + TRACE_EVENT("host", "NCE::CreateTrap"); std::scoped_lock lock(trapMutex); - auto protection{writeOnly ? TrapProtection::WriteOnly : TrapProtection::ReadWrite}; - TrapHandle handle{trapMap.Insert(regions, CallbackEntry{protection, lockCallback, readCallback, writeCallback})}; - ReprotectIntervals(handle->intervals, protection); + TrapHandle handle{trapMap.Insert(regions, CallbackEntry{TrapProtection::None, lockCallback, readCallback, writeCallback})}; return handle; } - void NCE::RetrapRegions(TrapHandle handle, bool writeOnly) { - TRACE_EVENT("host", "NCE::RetrapRegions"); + void NCE::TrapRegions(TrapHandle handle, bool writeOnly) { + TRACE_EVENT("host", "NCE::TrapRegions"); std::scoped_lock lock(trapMutex); auto protection{writeOnly ? TrapProtection::WriteOnly : TrapProtection::ReadWrite}; handle->value.protection = protection; diff --git a/app/src/main/cpp/skyline/nce.h b/app/src/main/cpp/skyline/nce.h index dbeeb57a..b3840b20 100644 --- a/app/src/main/cpp/skyline/nce.h +++ b/app/src/main/cpp/skyline/nce.h @@ -102,23 +102,22 @@ namespace skyline::nce { }; /** - * @brief Traps a region of guest memory with a callback for when an access to it has been made - * @param writeOnly If the trap is optimally for write-only accesses initially, this is not guarenteed + * @brief Creates a region of guest memory that can be trapped with a callback for when an access to it has been made * @param lockCallback A callback to lock the resource that is being trapped, it must block until the resource is locked but unlock it prior to returning * @param readCallback A callback for read accesses to the trapped region, it must not block and return a boolean if it would block * @param writeCallback A callback for write accesses to the trapped region, it must not block and return a boolean if it would block * @note The handle **must** be deleted using DeleteTrap before the NCE instance is destroyed * @note It is UB to supply a region of host memory rather than guest memory - * @note Any regions trapped without writeOnly may have their data (except border pages) paged out and it needs to be paged back in inside the callbacks + * @note This doesn't trap the region in itself, any trapping must be done via TrapRegions(...) */ - TrapHandle TrapRegions(span> regions, bool writeOnly, const LockCallback& lockCallback, const TrapCallback& readCallback, const TrapCallback& writeCallback); + TrapHandle CreateTrap(span> regions, const LockCallback& lockCallback, const TrapCallback& readCallback, const TrapCallback& writeCallback); /** * @brief Re-traps a region of memory after protections were removed * @param writeOnly If the trap is optimally for write-only accesses, this is not guarenteed * @note Any regions trapped without writeOnly may have their data (except border pages) paged out and it needs to be paged back in inside the callbacks */ - void RetrapRegions(TrapHandle handle, bool writeOnly); + void TrapRegions(TrapHandle handle, bool writeOnly); /** * @brief Removes protections from a region of memory