diff --git a/app/src/main/cpp/skyline/gpu/buffer.cpp b/app/src/main/cpp/skyline/gpu/buffer.cpp index 6dbab898..b569d908 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.cpp +++ b/app/src/main/cpp/skyline/gpu/buffer.cpp @@ -15,23 +15,37 @@ namespace skyline::gpu { alignedMirror = gpu.state.process->memory.CreateMirror(span{alignedData, alignedSize}); mirror = alignedMirror.subspan(static_cast(guest->data() - alignedData), guest->size()); - trapHandle = gpu.state.nce->TrapRegions(*guest, true, [this] { - std::scoped_lock lock{*this}; - }, [this] { - std::unique_lock lock{*this, std::try_to_lock}; + // 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(*guest, true, [weakThis] { + auto buffer{weakThis.lock()}; + if (!buffer) + return; + std::scoped_lock lock{*buffer}; + }, [weakThis] { + auto buffer{weakThis.lock()}; + if (!buffer) + return true; + + std::unique_lock lock{*buffer, std::try_to_lock}; if (!lock) return false; SynchronizeGuest(true); // We can skip trapping since the caller will do it + return true; - }, [this] { + }, [weakThis] { + auto buffer{weakThis.lock()}; + if (!buffer) + return true; + DirtyState expectedState{DirtyState::Clean}; - if (dirtyState.compare_exchange_strong(expectedState, DirtyState::CpuDirty, std::memory_order_relaxed) || expectedState == DirtyState::CpuDirty) + if (buffer->dirtyState.compare_exchange_strong(expectedState, DirtyState::CpuDirty, std::memory_order_relaxed) || expectedState == DirtyState::CpuDirty) return true; // If we can transition the buffer to CPU dirty (from Clean) or if it already is CPU dirty then we can just return, we only need to do the lock and corresponding sync if the buffer is GPU dirty - std::unique_lock lock{*this, std::try_to_lock}; + std::unique_lock lock{*buffer, std::try_to_lock}; if (!lock) return false; - SynchronizeGuest(true, false, true); // We need to assume the buffer is dirty since we don't know what the guest is writing + buffer->SynchronizeGuest(true, false, true); // We need to assume the buffer is dirty since we don't know what the guest is writing return true; }); } diff --git a/app/src/main/cpp/skyline/gpu/buffer.h b/app/src/main/cpp/skyline/gpu/buffer.h index 6cc28993..44ea64db 100644 --- a/app/src/main/cpp/skyline/gpu/buffer.h +++ b/app/src/main/cpp/skyline/gpu/buffer.h @@ -117,7 +117,7 @@ namespace skyline::gpu { friend BufferManager; /** - * @brief Sets up mirror mappings for the guest mappings + * @brief Sets up mirror mappings for the guest mappings, this must be called after construction for the mirror to be valid */ void SetupGuestMappings(); @@ -138,6 +138,10 @@ namespace skyline::gpu { return span(backing); } + /** + * @brief Creates a buffer object wrapping the guest buffer with a backing that can represent the guest buffer data + * @note The guest mappings will not be setup until SetupGuestMappings() is called + */ Buffer(GPU &gpu, GuestBuffer guest); /** diff --git a/app/src/main/cpp/skyline/gpu/buffer_manager.cpp b/app/src/main/cpp/skyline/gpu/buffer_manager.cpp index ff73a6df..e8bfe71e 100644 --- a/app/src/main/cpp/skyline/gpu/buffer_manager.cpp +++ b/app/src/main/cpp/skyline/gpu/buffer_manager.cpp @@ -69,6 +69,7 @@ namespace skyline::gpu { LockedBuffer newBuffer{std::make_shared(gpu, span{lowestAddress, highestAddress}), tag}; // If we don't lock the buffer prior to trapping it during synchronization, a race could occur with a guest trap acquiring the lock before we do and mutating the buffer prior to it being ready + newBuffer->SetupGuestMappings(); newBuffer->SynchronizeHost(false); // Overlaps don't necessarily fully cover the buffer so we have to perform a sync here to prevent any gaps auto copyBuffer{[](auto dstGuest, auto srcGuest, auto dstPtr, auto srcPtr) { @@ -164,6 +165,7 @@ namespace skyline::gpu { if (overlaps.empty()) { // If we couldn't find any overlapping buffers, create a new buffer without coalescing LockedBuffer buffer{std::make_shared(gpu, guestMapping), tag}; + buffer->SetupGuestMappings(); buffer->SynchronizeHost(); InsertBuffer(*buffer); return buffer->GetView(offset, size); diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.cpp b/app/src/main/cpp/skyline/gpu/texture/texture.cpp index 3f92b8cd..0a6b99cd 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.cpp +++ b/app/src/main/cpp/skyline/gpu/texture/texture.cpp @@ -143,25 +143,40 @@ namespace skyline::gpu { mirror = alignedMirror.subspan(static_cast(frontMapping.data() - alignedData), totalSize); } - trapHandle = gpu.state.nce->TrapRegions(mappings, true, [this] { - std::scoped_lock lock{*this}; - }, [this] { - std::unique_lock lock{*this, std::try_to_lock}; + // 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] { + auto texture{weakThis.lock()}; + if (!texture) + return; + + std::scoped_lock lock{*texture}; + }, [weakThis] { + auto texture{weakThis.lock()}; + if (!texture) + return true; + + std::unique_lock lock{*texture, std::try_to_lock}; if (!lock) return false; - SynchronizeGuest(true); // We can skip trapping since the caller will do it - WaitOnFence(); + + texture->SynchronizeGuest(true); // We can skip trapping since the caller will do it + texture->WaitOnFence(); return true; - }, [this] { + }, [weakThis] { + auto texture{weakThis.lock()}; + if (!texture) + return true; + DirtyState expectedState{DirtyState::Clean}; - if (dirtyState.compare_exchange_strong(expectedState, DirtyState::CpuDirty, std::memory_order_relaxed) || expectedState == DirtyState::CpuDirty) + if (texture->dirtyState.compare_exchange_strong(expectedState, DirtyState::CpuDirty, std::memory_order_relaxed) || expectedState == DirtyState::CpuDirty) return true; // If we can transition the texture to CPU dirty (from Clean) or if it already is CPU dirty then we can just return, we only need to do the lock and corresponding sync if the texture is GPU dirty - std::unique_lock lock{*this, std::try_to_lock}; + std::unique_lock lock{*texture, std::try_to_lock}; if (!lock) return false; - SynchronizeGuest(true, true); // We need to assume the texture is dirty since we don't know what the guest is writing - WaitOnFence(); + texture->SynchronizeGuest(true, true); // We need to assume the texture is dirty since we don't know what the guest is writing + texture->WaitOnFence(); return true; }); } diff --git a/app/src/main/cpp/skyline/gpu/texture/texture.h b/app/src/main/cpp/skyline/gpu/texture/texture.h index 7a213375..2749f9fb 100644 --- a/app/src/main/cpp/skyline/gpu/texture/texture.h +++ b/app/src/main/cpp/skyline/gpu/texture/texture.h @@ -390,7 +390,7 @@ namespace skyline::gpu { friend TextureView; /** - * @brief Sets up mirror mappings for the guest mappings + * @brief Sets up mirror mappings for the guest mappings, this must be called after construction for the mirror to be valid */ void SetupGuestMappings(); @@ -460,6 +460,7 @@ namespace skyline::gpu { /** * @brief Creates a texture object wrapping the guest texture with a backing that can represent the guest texture data + * @note The guest mappings will not be setup until SetupGuestMappings() is called */ Texture(GPU &gpu, GuestTexture guest); diff --git a/app/src/main/cpp/skyline/gpu/texture_manager.cpp b/app/src/main/cpp/skyline/gpu/texture_manager.cpp index b17cd27f..457d4c04 100644 --- a/app/src/main/cpp/skyline/gpu/texture_manager.cpp +++ b/app/src/main/cpp/skyline/gpu/texture_manager.cpp @@ -87,6 +87,7 @@ namespace skyline::gpu { // Create a texture as we cannot find one that matches auto texture{std::make_shared(gpu, guestTexture)}; + texture->SetupGuestMappings(); texture->TransitionLayout(vk::ImageLayout::eGeneral); auto it{texture->guest->mappings.begin()}; textures.emplace(mappingEnd, TextureMapping{texture, it, guestMapping});