From 9982cff98b4db38565715cc515ea496b6195725b Mon Sep 17 00:00:00 2001 From: VonChenPlus Date: Mon, 27 Jun 2022 12:39:57 +0800 Subject: [PATCH] Core: Fix get nvmap object random crash --- src/core/hle/service/nvdrv/core/nvmap.cpp | 15 ++++++++++++++- src/core/hle/service/nvdrv/core/nvmap.h | 5 +++++ src/core/hle/service/nvdrv/devices/nvmap.cpp | 12 ------------ src/core/hle/service/nvdrv/devices/nvmap.h | 5 ----- src/core/hle/service/nvdrv/nvdrv.h | 3 +-- .../service/nvflinger/buffer_queue_consumer.cpp | 9 +++++++-- .../service/nvflinger/buffer_queue_consumer.h | 8 +++++++- .../service/nvflinger/buffer_queue_producer.cpp | 10 +++++++--- .../service/nvflinger/buffer_queue_producer.h | 9 ++++++++- src/core/hle/service/nvflinger/nvflinger.cpp | 2 +- src/core/hle/service/vi/display/vi_display.cpp | 16 ++++++++++------ src/core/hle/service/vi/display/vi_display.h | 7 ++++++- 12 files changed, 66 insertions(+), 35 deletions(-) diff --git a/src/core/hle/service/nvdrv/core/nvmap.cpp b/src/core/hle/service/nvdrv/core/nvmap.cpp index b02dbb9c9..dd30e156e 100644 --- a/src/core/hle/service/nvdrv/core/nvmap.cpp +++ b/src/core/hle/service/nvdrv/core/nvmap.cpp @@ -207,6 +207,19 @@ void NvMap::UnpinHandle(Handle::Id handle) { } } +void NvMap::DuplicateHandle(Handle::Id handle) { + auto handle_description{GetHandle(handle)}; + if (!handle_description) { + LOG_CRITICAL(Service_NVDRV, "Unregistered handle!"); + return; + } + + auto result = handle_description->Duplicate(false); + if (result != NvResult::Success) { + LOG_CRITICAL(Service_NVDRV, "Could not duplicate handle!"); + } +} + std::optional NvMap::FreeHandle(Handle::Id handle, bool internal_session) { std::weak_ptr hWeak{GetHandle(handle)}; FreeInfo freeInfo; @@ -254,7 +267,7 @@ std::optional NvMap::FreeHandle(Handle::Id handle, bool interna // Handle hasn't been freed from memory, set address to 0 to mark that the handle wasn't freed if (!hWeak.expired()) { - LOG_ERROR(Service_NVDRV, "nvmap handle: {} wasn't freed as it is still in use", handle); + LOG_DEBUG(Service_NVDRV, "nvmap handle: {} wasn't freed as it is still in use", handle); freeInfo.address = 0; } diff --git a/src/core/hle/service/nvdrv/core/nvmap.h b/src/core/hle/service/nvdrv/core/nvmap.h index 1082bb58d..b6613a521 100644 --- a/src/core/hle/service/nvdrv/core/nvmap.h +++ b/src/core/hle/service/nvdrv/core/nvmap.h @@ -162,6 +162,11 @@ public: */ void UnpinHandle(Handle::Id handle); + /** + * @brief Tries to duplicate a handle + */ + void DuplicateHandle(Handle::Id handle); + /** * @brief Tries to free a handle and remove a single dupe * @note If a handle has no dupes left and has no other users a FreeInfo struct will be returned diff --git a/src/core/hle/service/nvdrv/devices/nvmap.cpp b/src/core/hle/service/nvdrv/devices/nvmap.cpp index f84fc8c37..ddf273b5e 100644 --- a/src/core/hle/service/nvdrv/devices/nvmap.cpp +++ b/src/core/hle/service/nvdrv/devices/nvmap.cpp @@ -69,18 +69,6 @@ NvResult nvmap::Ioctl3(DeviceFD fd, Ioctl command, const std::vector& input, void nvmap::OnOpen(DeviceFD fd) {} void nvmap::OnClose(DeviceFD fd) {} -VAddr nvmap::GetObjectAddress(u32 handle) const { - auto obj = file.GetHandle(handle); - if (obj) { - return obj->address; - } - return 0; -} - -std::shared_ptr nvmap::GetObject(u32 handle) const { - return file.GetHandle(handle); -} - NvResult nvmap::IocCreate(const std::vector& input, std::vector& output) { IocCreateParams params; std::memcpy(¶ms, input.data(), sizeof(params)); diff --git a/src/core/hle/service/nvdrv/devices/nvmap.h b/src/core/hle/service/nvdrv/devices/nvmap.h index c22eb57a4..52e1d7cff 100644 --- a/src/core/hle/service/nvdrv/devices/nvmap.h +++ b/src/core/hle/service/nvdrv/devices/nvmap.h @@ -36,11 +36,6 @@ public: void OnOpen(DeviceFD fd) override; void OnClose(DeviceFD fd) override; - /// Returns the allocated address of an nvmap object given its handle. - VAddr GetObjectAddress(u32 handle) const; - - std::shared_ptr GetObject(u32 handle) const; - enum class HandleParameterType : u32_le { Size = 1, Alignment = 2, diff --git a/src/core/hle/service/nvdrv/nvdrv.h b/src/core/hle/service/nvdrv/nvdrv.h index b26254753..22836529d 100644 --- a/src/core/hle/service/nvdrv/nvdrv.h +++ b/src/core/hle/service/nvdrv/nvdrv.h @@ -96,6 +96,7 @@ public: private: friend class EventInterface; + friend class Service::NVFlinger::NVFlinger; /// Id to use for the next open file descriptor. DeviceFD next_fd = 1; @@ -111,8 +112,6 @@ private: /// Manages syncpoints on the host NvCore::Container container; - void CreateEvent(u32 event_id); - void FreeEvent(u32 event_id); std::unordered_map> builders; }; diff --git a/src/core/hle/service/nvflinger/buffer_queue_consumer.cpp b/src/core/hle/service/nvflinger/buffer_queue_consumer.cpp index 4b3d5efd6..a0330ab4a 100644 --- a/src/core/hle/service/nvflinger/buffer_queue_consumer.cpp +++ b/src/core/hle/service/nvflinger/buffer_queue_consumer.cpp @@ -5,15 +5,18 @@ // https://cs.android.com/android/platform/superproject/+/android-5.1.1_r38:frameworks/native/libs/gui/BufferQueueConsumer.cpp #include "common/logging/log.h" +#include "core/hle/service/nvdrv/core/nvmap.h" #include "core/hle/service/nvflinger/buffer_item.h" #include "core/hle/service/nvflinger/buffer_queue_consumer.h" #include "core/hle/service/nvflinger/buffer_queue_core.h" #include "core/hle/service/nvflinger/producer_listener.h" +#include "core/hle/service/nvflinger/ui/graphic_buffer.h" namespace Service::android { -BufferQueueConsumer::BufferQueueConsumer(std::shared_ptr core_) - : core{std::move(core_)}, slots{core->slots} {} +BufferQueueConsumer::BufferQueueConsumer(std::shared_ptr core_, + Service::Nvidia::NvCore::NvMap& nvmap_) + : core{std::move(core_)}, slots{core->slots}, nvmap(nvmap_) {} BufferQueueConsumer::~BufferQueueConsumer() = default; @@ -133,6 +136,8 @@ Status BufferQueueConsumer::ReleaseBuffer(s32 slot, u64 frame_number, const Fenc slots[slot].buffer_state = BufferState::Free; + nvmap.FreeHandle(slots[slot].graphic_buffer->BufferId(), false); + listener = core->connected_producer_listener; LOG_DEBUG(Service_NVFlinger, "releasing slot {}", slot); diff --git a/src/core/hle/service/nvflinger/buffer_queue_consumer.h b/src/core/hle/service/nvflinger/buffer_queue_consumer.h index b598c314f..4ec06ca13 100644 --- a/src/core/hle/service/nvflinger/buffer_queue_consumer.h +++ b/src/core/hle/service/nvflinger/buffer_queue_consumer.h @@ -13,6 +13,10 @@ #include "core/hle/service/nvflinger/buffer_queue_defs.h" #include "core/hle/service/nvflinger/status.h" +namespace Service::Nvidia::NvCore { +class NvMap; +} // namespace Service::Nvidia::NvCore + namespace Service::android { class BufferItem; @@ -21,7 +25,8 @@ class IConsumerListener; class BufferQueueConsumer final { public: - explicit BufferQueueConsumer(std::shared_ptr core_); + explicit BufferQueueConsumer(std::shared_ptr core_, + Service::Nvidia::NvCore::NvMap& nvmap_); ~BufferQueueConsumer(); Status AcquireBuffer(BufferItem* out_buffer, std::chrono::nanoseconds expected_present); @@ -32,6 +37,7 @@ public: private: std::shared_ptr core; BufferQueueDefs::SlotsType& slots; + Service::Nvidia::NvCore::NvMap& nvmap; }; } // namespace Service::android diff --git a/src/core/hle/service/nvflinger/buffer_queue_producer.cpp b/src/core/hle/service/nvflinger/buffer_queue_producer.cpp index 337431488..a4e46964c 100644 --- a/src/core/hle/service/nvflinger/buffer_queue_producer.cpp +++ b/src/core/hle/service/nvflinger/buffer_queue_producer.cpp @@ -14,7 +14,7 @@ #include "core/hle/kernel/k_writable_event.h" #include "core/hle/kernel/kernel.h" #include "core/hle/service/kernel_helpers.h" -#include "core/hle/service/nvdrv/nvdrv.h" +#include "core/hle/service/nvdrv/core/nvmap.h" #include "core/hle/service/nvflinger/buffer_queue_core.h" #include "core/hle/service/nvflinger/buffer_queue_producer.h" #include "core/hle/service/nvflinger/consumer_listener.h" @@ -26,8 +26,10 @@ namespace Service::android { BufferQueueProducer::BufferQueueProducer(Service::KernelHelpers::ServiceContext& service_context_, - std::shared_ptr buffer_queue_core_) - : service_context{service_context_}, core{std::move(buffer_queue_core_)}, slots(core->slots) { + std::shared_ptr buffer_queue_core_, + Service::Nvidia::NvCore::NvMap& nvmap_) + : service_context{service_context_}, core{std::move(buffer_queue_core_)}, slots(core->slots), + nvmap(nvmap_) { buffer_wait_event = service_context.CreateEvent("BufferQueue:WaitEvent"); } @@ -530,6 +532,8 @@ Status BufferQueueProducer::QueueBuffer(s32 slot, const QueueBufferInput& input, item.is_droppable = core->dequeue_buffer_cannot_block || async; item.swap_interval = swap_interval; + nvmap.DuplicateHandle(item.graphic_buffer->BufferId()); + sticky_transform = sticky_transform_; if (core->queue.empty()) { diff --git a/src/core/hle/service/nvflinger/buffer_queue_producer.h b/src/core/hle/service/nvflinger/buffer_queue_producer.h index 42d4722dc..0ba03a568 100644 --- a/src/core/hle/service/nvflinger/buffer_queue_producer.h +++ b/src/core/hle/service/nvflinger/buffer_queue_producer.h @@ -31,6 +31,10 @@ namespace Service::KernelHelpers { class ServiceContext; } // namespace Service::KernelHelpers +namespace Service::Nvidia::NvCore { +class NvMap; +} // namespace Service::Nvidia::NvCore + namespace Service::android { class BufferQueueCore; @@ -39,7 +43,8 @@ class IProducerListener; class BufferQueueProducer final : public IBinder { public: explicit BufferQueueProducer(Service::KernelHelpers::ServiceContext& service_context_, - std::shared_ptr buffer_queue_core_); + std::shared_ptr buffer_queue_core_, + Service::Nvidia::NvCore::NvMap& nvmap_); ~BufferQueueProducer(); void Transact(Kernel::HLERequestContext& ctx, android::TransactionId code, u32 flags) override; @@ -78,6 +83,8 @@ private: s32 next_callback_ticket{}; s32 current_callback_ticket{}; std::condition_variable_any callback_condition; + + Service::Nvidia::NvCore::NvMap& nvmap; }; } // namespace Service::android diff --git a/src/core/hle/service/nvflinger/nvflinger.cpp b/src/core/hle/service/nvflinger/nvflinger.cpp index 4658f1e8b..aa14d2cbc 100644 --- a/src/core/hle/service/nvflinger/nvflinger.cpp +++ b/src/core/hle/service/nvflinger/nvflinger.cpp @@ -149,7 +149,7 @@ std::optional NVFlinger::CreateLayer(u64 display_id) { void NVFlinger::CreateLayerAtId(VI::Display& display, u64 layer_id) { const auto buffer_id = next_buffer_queue_id++; - display.CreateLayer(layer_id, buffer_id); + display.CreateLayer(layer_id, buffer_id, nvdrv->container); } void NVFlinger::CloseLayer(u64 layer_id) { diff --git a/src/core/hle/service/vi/display/vi_display.cpp b/src/core/hle/service/vi/display/vi_display.cpp index aa49aa775..288aafaaf 100644 --- a/src/core/hle/service/vi/display/vi_display.cpp +++ b/src/core/hle/service/vi/display/vi_display.cpp @@ -12,6 +12,7 @@ #include "core/hle/kernel/k_readable_event.h" #include "core/hle/kernel/k_writable_event.h" #include "core/hle/service/kernel_helpers.h" +#include "core/hle/service/nvdrv/core/container.h" #include "core/hle/service/nvflinger/buffer_item_consumer.h" #include "core/hle/service/nvflinger/buffer_queue_consumer.h" #include "core/hle/service/nvflinger/buffer_queue_core.h" @@ -29,11 +30,13 @@ struct BufferQueue { std::unique_ptr consumer; }; -static BufferQueue CreateBufferQueue(KernelHelpers::ServiceContext& service_context) { +static BufferQueue CreateBufferQueue(KernelHelpers::ServiceContext& service_context, + Service::Nvidia::NvCore::NvMap& nvmap) { auto buffer_queue_core = std::make_shared(); - return {buffer_queue_core, - std::make_unique(service_context, buffer_queue_core), - std::make_unique(buffer_queue_core)}; + return { + buffer_queue_core, + std::make_unique(service_context, buffer_queue_core, nvmap), + std::make_unique(buffer_queue_core, nvmap)}; } Display::Display(u64 id, std::string name_, @@ -74,10 +77,11 @@ void Display::SignalVSyncEvent() { vsync_event->GetWritableEvent().Signal(); } -void Display::CreateLayer(u64 layer_id, u32 binder_id) { +void Display::CreateLayer(u64 layer_id, u32 binder_id, + Service::Nvidia::NvCore::Container& nv_core) { ASSERT_MSG(layers.empty(), "Only one layer is supported per display at the moment"); - auto [core, producer, consumer] = CreateBufferQueue(service_context); + auto [core, producer, consumer] = CreateBufferQueue(service_context, nv_core.GetNvMapFile()); auto buffer_item_consumer = std::make_shared(std::move(consumer)); buffer_item_consumer->Connect(false); diff --git a/src/core/hle/service/vi/display/vi_display.h b/src/core/hle/service/vi/display/vi_display.h index 8dbb0ef80..33d5f398c 100644 --- a/src/core/hle/service/vi/display/vi_display.h +++ b/src/core/hle/service/vi/display/vi_display.h @@ -27,6 +27,11 @@ namespace Service::NVFlinger { class HosBinderDriverServer; } +namespace Service::Nvidia::NvCore { +class Container; +class NvMap; +} // namespace Service::Nvidia::NvCore + namespace Service::VI { class Layer; @@ -93,7 +98,7 @@ public: /// @param layer_id The ID to assign to the created layer. /// @param binder_id The ID assigned to the buffer queue. /// - void CreateLayer(u64 layer_id, u32 binder_id); + void CreateLayer(u64 layer_id, u32 binder_id, Service::Nvidia::NvCore::Container& core); /// Closes and removes a layer from this display with the given ID. ///