From 8ddb59bd8fc187dc50a06642bd58dd0c4747a21e Mon Sep 17 00:00:00 2001 From: Artem Kharytoniuk Date: Fri, 15 May 2026 16:21:16 +0200 Subject: [PATCH] layers: Update event queue mismatch validation Replace callback with state tracking and add more tests --- layers/core_checks/cc_queue.cpp | 5 +- layers/core_checks/cc_state_tracker.cpp | 36 ++++++---- layers/core_checks/cc_state_tracker.h | 1 + layers/core_checks/cc_synchronization.cpp | 50 ++++++++++--- layers/core_checks/cc_synchronization.h | 12 +++- tests/framework/binding.cpp | 4 +- tests/framework/binding.h | 7 +- tests/unit/event.cpp | 86 ++++++++++++++++++++--- tests/unit/event_positive.cpp | 76 ++++++++++++++++++++ 9 files changed, 235 insertions(+), 42 deletions(-) diff --git a/layers/core_checks/cc_queue.cpp b/layers/core_checks/cc_queue.cpp index d299f0d74c2..cfd6ab49d5c 100644 --- a/layers/core_checks/cc_queue.cpp +++ b/layers/core_checks/cc_queue.cpp @@ -77,7 +77,10 @@ struct CommandBufferSubmitState { auto& cb_sub_state = core::SubState(cb_state); for (const WaitEventSubmitInfo& submit_info : cb_sub_state.wait_event_submit_infos) { - skip |= submit_info.Validate(core, cb_state, local_event_signal_info, loc); + skip |= submit_info.Validate(core, queue_state, cb_state, local_event_signal_info, loc); + } + for (const WaitEvent2SubmitInfo& submit_info : cb_sub_state.wait_event2_submit_infos) { + skip |= submit_info.Validate(core, queue_state, cb_state, local_event_signal_info, loc); } for (auto& function : cb_sub_state.event_updates) { diff --git a/layers/core_checks/cc_state_tracker.cpp b/layers/core_checks/cc_state_tracker.cpp index 4134622a949..041cf27e935 100644 --- a/layers/core_checks/cc_state_tracker.cpp +++ b/layers/core_checks/cc_state_tracker.cpp @@ -651,6 +651,12 @@ void CommandBufferSubState::RecordWaitEvents(vvl::span events, Vk } void CommandBufferSubState::RecordWaitEvent2(VkEvent event, const VkDependencyInfo& dependency_info, const Location& loc) { + const bool submit_validation = !vvl::Contains(event_signaling_states, event); + if (submit_validation) { + WaitEvent2SubmitInfo submit_info; + submit_info.wait_event = event; + wait_event2_submit_infos.emplace_back(std::move(submit_info)); + } // TODO: this will be removed when submit validation callbacks are removed auto first_event_index = base.events.size(); auto event_added_count = 1; @@ -1126,6 +1132,7 @@ void CommandBufferSubState::ResetCBState() { queue_submit_functions.clear(); event_updates.clear(); wait_event_submit_infos.clear(); + wait_event2_submit_infos.clear(); cmd_execute_commands_functions.clear(); query_updates.clear(); @@ -1167,6 +1174,13 @@ void CommandBufferSubState::RecordExecuteCommand(vvl::CommandBuffer& secondary_c wait_event_submit_infos.emplace_back(wait); } } + for (const WaitEvent2SubmitInfo& secondary_wait : secondary_sub_state.wait_event2_submit_infos) { + const bool found_signaling_state = vvl::Contains(event_signaling_states, secondary_wait.wait_event); + if (!found_signaling_state) { + wait_event2_submit_infos.emplace_back(secondary_wait); + } + } + for (auto& [event, event_signaling_state] : secondary_sub_state.event_signaling_states) { event_signaling_states.insert_or_assign(event, event_signaling_state); } @@ -1205,21 +1219,13 @@ void CommandBufferSubState::Submit(vvl::Queue& queue_state, uint32_t perf_submit func(queue_state, base); } - // Update vvl::Event with src_stage from the last recorded SetEvent. - // Ultimately, it tracks the last SetEvent for the entire submission. - { - EventMap local_event_signal_info; - for (const auto& function : event_updates) { - function(base, /*do_validate*/ false, local_event_signal_info, - VK_NULL_HANDLE /* when do_validate is false then wait handler is inactive */, loc); - } - for (const auto& [event, signaling_state] : event_signaling_states) { - if (auto event_state = base.dev_data.Get(event)) { - event_state->signaled = signaling_state.signaled; - event_state->signal_src_stage_mask = signaling_state.src_stage_mask; - event_state->dependency_info = signaling_state.dependency_info; - event_state->signaling_queue = queue_state.VkHandle(); - } + // Update global vvl:Event state with signaling state at the end of the command buffer + for (const auto& [event, signaling_state] : event_signaling_states) { + if (auto event_state = base.dev_data.Get(event)) { + event_state->signaled = signaling_state.signaled; + event_state->signal_src_stage_mask = signaling_state.src_stage_mask; + event_state->dependency_info = signaling_state.dependency_info; + event_state->signaling_queue = queue_state.VkHandle(); } } diff --git a/layers/core_checks/cc_state_tracker.h b/layers/core_checks/cc_state_tracker.h index 1820bb98cd8..f585d9a53b7 100644 --- a/layers/core_checks/cc_state_tracker.h +++ b/layers/core_checks/cc_state_tracker.h @@ -214,6 +214,7 @@ class CommandBufferSubState : public vvl::CommandBufferSubState { EventSignalingStateMap event_signaling_states; std::vector wait_event_submit_infos; + std::vector wait_event2_submit_infos; // Validation functions run when secondary CB is executed in primary std::vector< diff --git a/layers/core_checks/cc_synchronization.cpp b/layers/core_checks/cc_synchronization.cpp index 50e6a05f5e2..37ea97b2667 100644 --- a/layers/core_checks/cc_synchronization.cpp +++ b/layers/core_checks/cc_synchronization.cpp @@ -392,11 +392,37 @@ bool SemaphoreSubmitState::ValidateSignalSemaphore(const Location& signal_semaph return skip; } -bool WaitEventSubmitInfo::Validate(const CoreChecks& core, const vvl::CommandBuffer& cb_state, EventMap& submit_signaling_states, - const Location& loc) const { +static bool ValidateEventQueueMismatch(const CoreChecks& core, const vvl::Queue& queue_state, const vvl::CommandBuffer& cb_state, + const vvl::Event& event_state, const EventMap& submit_signaling_states, + const Location& loc) { bool skip = false; + + // Check if prior command buffers from this submit contain signaling operation. + // If yes, then everything is on the same queue + if (vvl::Contains(submit_signaling_states, event_state.VkHandle())) { + return skip; + } + // Check global event state + if (event_state.signaling_queue != VK_NULL_HANDLE && event_state.signaling_queue != queue_state.VkHandle()) { + const LogObjectList objlist(cb_state.Handle(), event_state.Handle(), event_state.signaling_queue, queue_state.Handle()); + skip |= core.LogError("UNASSIGNED-SubmitValidation-WaitEvents-WrongQueue", objlist, loc, + "waits for event %s on the queue %s but the event was signaled on a different queue %s", + core.FormatHandle(event_state.Handle()).c_str(), core.FormatHandle(queue_state.Handle()).c_str(), + core.FormatHandle(event_state.signaling_queue).c_str()); + } + return skip; +} + +bool WaitEventSubmitInfo::Validate(const CoreChecks& core, const vvl::Queue& queue_state, const vvl::CommandBuffer& cb_state, + EventMap& submit_signaling_states, const Location& loc) const { + bool skip = false; + VkPipelineStageFlags signals_src_stage_mask = 0; for (VkEvent event : wait_events) { + auto event_state = core.Get(event); + if (event_state && !vvl::Contains(signaling_states, event)) { + skip |= ValidateEventQueueMismatch(core, queue_state, cb_state, *event_state, submit_signaling_states, loc); + } // NOTE: if signaling state is "unsignaled" then src stage mask is NONE if (const auto* cb_signaling = vvl::Find(signaling_states, event)) { // a) signal from the same command buffer @@ -404,11 +430,12 @@ bool WaitEventSubmitInfo::Validate(const CoreChecks& core, const vvl::CommandBuf } else if (const auto* submit_signaling = vvl::Find(submit_signaling_states, event)) { // b) signals from the previous command buffers of the same submit command signals_src_stage_mask |= static_cast(submit_signaling->src_stage_mask); - } else if (auto event_state = core.Get(event)) { + } else if (event_state) { // c) global event state signals_src_stage_mask |= static_cast(event_state->signal_src_stage_mask); } } + // Validate src stage mask mismatch if (wait_src_stage_mask != signals_src_stage_mask) { std::ostringstream ss; ss << "(" << core.FormatHandle(cb_state.Handle()) << ") contains a vkCmdWaitEvents call with srcStageMask " @@ -431,6 +458,15 @@ bool WaitEventSubmitInfo::Validate(const CoreChecks& core, const vvl::CommandBuf return skip; } +bool WaitEvent2SubmitInfo::Validate(const CoreChecks& core, const vvl::Queue& queue_state, const vvl::CommandBuffer& cb_state, + EventMap& submit_signaling_states, const Location& loc) const { + bool skip = false; + if (auto event_state = core.Get(wait_event)) { + skip |= ValidateEventQueueMismatch(core, queue_state, cb_state, *event_state, submit_signaling_states, loc); + } + return skip; +} + bool CoreChecks::ValidateStageMaskHost(const LogObjectList& objlist, const Location& stage_mask_loc, VkPipelineStageFlags2KHR stageMask) const { bool skip = false; @@ -1389,14 +1425,6 @@ bool CoreChecks::ValidateWaitEventsAtSubmit(const vvl::CommandBuffer& cb_state, auto event_state = state_data.Get(event); if (!event_state) continue; set_dependency_info = event_state->dependency_info; - - if (event_state->signaling_queue != VK_NULL_HANDLE && event_state->signaling_queue != waiting_queue) { - const LogObjectList objlist(cb_state.Handle(), event, event_state->signaling_queue, waiting_queue); - skip |= state_data.LogError("UNASSIGNED-SubmitValidation-WaitEvents-WrongQueue", objlist, loc, - "waits for event %s on the queue %s but the event was signaled on a different queue %s", - state_data.FormatHandle(event).c_str(), state_data.FormatHandle(waiting_queue).c_str(), - state_data.FormatHandle(event_state->signaling_queue).c_str()); - } } bool set_is_event2 = set_dependency_info.has_value(); diff --git a/layers/core_checks/cc_synchronization.h b/layers/core_checks/cc_synchronization.h index e72208ab870..b7a09637a6c 100644 --- a/layers/core_checks/cc_synchronization.h +++ b/layers/core_checks/cc_synchronization.h @@ -29,6 +29,7 @@ struct Location; namespace vvl { class CommandBuffer; +class Queue; class Semaphore; enum class Func; } // namespace vvl @@ -79,6 +80,13 @@ struct WaitEventSubmitInfo { // Subset of waited events with known signaling state EventSignalingStateMap signaling_states; - bool Validate(const CoreChecks& core, const vvl::CommandBuffer& cb_state, EventMap& submit_signaling_states, - const Location& loc) const; + bool Validate(const CoreChecks& core, const vvl::Queue& queue_state, const vvl::CommandBuffer& cb_state, + EventMap& submit_signaling_states, const Location& loc) const; +}; + +struct WaitEvent2SubmitInfo { + VkEvent wait_event = VK_NULL_HANDLE; + + bool Validate(const CoreChecks& core, const vvl::Queue& queue_state, const vvl::CommandBuffer& cb_state, + EventMap& submit_signaling_states, const Location& loc) const; }; diff --git a/tests/framework/binding.cpp b/tests/framework/binding.cpp index 32708d030fe..49f87870196 100644 --- a/tests/framework/binding.cpp +++ b/tests/framework/binding.cpp @@ -2154,11 +2154,11 @@ void CommandBuffer::EncodeVideo(const VkVideoEncodeInfoKHR& encodeInfo) { vk::Cm void CommandBuffer::EndVideoCoding(const VkVideoEndCodingInfoKHR& endInfo) { vk::CmdEndVideoCodingKHR(handle(), &endInfo); } -void CommandBuffer::SetEvent(Event& event, VkPipelineStageFlags src_stage_mask) { +void CommandBuffer::SetEvent(const Event& event, VkPipelineStageFlags src_stage_mask) { vk::CmdSetEvent(handle(), event, src_stage_mask); } -void CommandBuffer::ResetEvent(Event& event, VkPipelineStageFlags src_stageMask) { +void CommandBuffer::ResetEvent(const Event& event, VkPipelineStageFlags src_stageMask) { vk::CmdResetEvent(handle(), event, src_stageMask); } diff --git a/tests/framework/binding.h b/tests/framework/binding.h index 45dac8bf885..6220800d759 100644 --- a/tests/framework/binding.h +++ b/tests/framework/binding.h @@ -1126,9 +1126,10 @@ class CommandBuffer : public internal::Handle { void EncodeVideo(const VkVideoEncodeInfoKHR &encodeInfo); void EndVideoCoding(const VkVideoEndCodingInfoKHR &endInfo); - void SetEvent(Event& event, VkPipelineStageFlags src_stage_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); - void ResetEvent(Event& event, VkPipelineStageFlags src_stageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); - void WaitEvent(const Event& event, VkPipelineStageFlags src_stage_mask, VkPipelineStageFlags dst_stage_mask); + void SetEvent(const Event& event, VkPipelineStageFlags src_stage_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); + void ResetEvent(const Event& event, VkPipelineStageFlags src_stageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); + void WaitEvent(const Event& event, VkPipelineStageFlags src_stage_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, + VkPipelineStageFlags dst_stage_mask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT); void WaitEvent(const Event& event, VkPipelineStageFlags src_stage_mask, VkPipelineStageFlags dst_stage_mask, const VkMemoryBarrier& memory_barrier); void WaitEvent(const Event& event, VkPipelineStageFlags src_stage_mask, VkPipelineStageFlags dst_stage_mask, diff --git a/tests/unit/event.cpp b/tests/unit/event.cpp index 67f0d0313d9..0dbf6a2383f 100644 --- a/tests/unit/event.cpp +++ b/tests/unit/event.cpp @@ -385,7 +385,7 @@ TEST_F(NegativeEvent, StageMaskTwoSecondariesSameCommand) { m_command_buffer.End(); } -TEST_F(NegativeEvent, DetectInterQueueEventUsage) { +TEST_F(NegativeEvent, InterQueueEventUsage) { TEST_DESCRIPTION("Sets event on one queue and tries to wait on a different queue (CmdSetEvent/CmdWaitEvents)"); all_queue_count_ = true; RETURN_IF_SKIP(Init()); @@ -397,25 +397,23 @@ TEST_F(NegativeEvent, DetectInterQueueEventUsage) { vkt::CommandBuffer cb1(*m_device, m_command_pool); cb1.Begin(); - vk::CmdSetEvent(cb1, event, VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT); + cb1.SetEvent(event); cb1.End(); vkt::CommandPool pool2(*m_device, m_second_queue->family_index); vkt::CommandBuffer cb2(*m_device, pool2); cb2.Begin(); - vk::CmdWaitEvents(cb2, 1, &event.handle(), VK_PIPELINE_STAGE_BOTTOM_OF_PIPE_BIT, VK_PIPELINE_STAGE_TOP_OF_PIPE_BIT, 0, nullptr, - 0, nullptr, 0, nullptr); + cb2.WaitEvent(event); cb2.End(); m_default_queue->Submit(cb1); m_errorMonitor->SetDesiredError("UNASSIGNED-SubmitValidation-WaitEvents-WrongQueue"); m_second_queue->Submit(cb2); m_errorMonitor->VerifyFound(); - m_device->Wait(); } -TEST_F(NegativeEvent, DetectInterQueueEventUsage2) { +TEST_F(NegativeEvent, InterQueueEvent2Usage) { TEST_DESCRIPTION("Sets event on one queue and tries to wait on a different queue (CmdSetEvent2/CmdWaitEvents2)"); SetTargetApiVersion(VK_API_VERSION_1_3); AddRequiredFeature(vkt::Feature::synchronization2); @@ -427,8 +425,6 @@ TEST_F(NegativeEvent, DetectInterQueueEventUsage2) { } VkMemoryBarrier2 barrier = vku::InitStructHelper(); - barrier.srcAccessMask = 0; - barrier.dstAccessMask = 0; barrier.srcStageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; barrier.dstStageMask = VK_PIPELINE_STAGE_NONE; @@ -456,6 +452,80 @@ TEST_F(NegativeEvent, DetectInterQueueEventUsage2) { m_device->Wait(); } +TEST_F(NegativeEvent, InterQueueEventUsageSecondary) { + TEST_DESCRIPTION("Set/Wait event from different queues and use secondary command buffer"); + all_queue_count_ = true; + RETURN_IF_SKIP(Init()); + + if ((m_second_queue_caps & VK_QUEUE_GRAPHICS_BIT) == 0) { + GTEST_SKIP() << "2 graphics queues are needed"; + } + const vkt::Event event(*m_device); + + m_command_buffer.Begin(); + m_command_buffer.SetEvent(event); + m_command_buffer.End(); + + vkt::CommandPool pool(*m_device, m_second_queue->family_index); + vkt::CommandBuffer secondary(*m_device, pool, VK_COMMAND_BUFFER_LEVEL_SECONDARY); + secondary.Begin(); + secondary.WaitEvent(event); + secondary.End(); + + vkt::CommandBuffer cb(*m_device, pool); + cb.Begin(); + cb.ExecuteCommands(secondary); + cb.End(); + + m_default_queue->Submit(m_command_buffer); + m_errorMonitor->SetDesiredError("UNASSIGNED-SubmitValidation-WaitEvents-WrongQueue"); + m_second_queue->Submit(cb); + m_errorMonitor->VerifyFound(); + m_device->Wait(); +} + +TEST_F(NegativeEvent, InterQueueEvent2UsageSecondary) { + TEST_DESCRIPTION("Set2/Wait2 event from different queues and use secondary command buffer"); + SetTargetApiVersion(VK_API_VERSION_1_3); + AddRequiredFeature(vkt::Feature::synchronization2); + all_queue_count_ = true; + RETURN_IF_SKIP(Init()); + + if ((m_second_queue_caps & VK_QUEUE_GRAPHICS_BIT) == 0) { + GTEST_SKIP() << "2 graphics queues are needed"; + } + const vkt::Event event(*m_device); + + VkMemoryBarrier2 barrier = vku::InitStructHelper(); + barrier.srcStageMask = VK_PIPELINE_STAGE_ALL_COMMANDS_BIT; + barrier.dstStageMask = VK_PIPELINE_STAGE_NONE; + + VkDependencyInfo dependency_info = vku::InitStructHelper(); + dependency_info.memoryBarrierCount = 1; + dependency_info.pMemoryBarriers = &barrier; + + m_command_buffer.Begin(); + vk::CmdSetEvent2(m_command_buffer, event, &dependency_info); + m_command_buffer.End(); + + vkt::CommandPool pool(*m_device, m_second_queue->family_index); + vkt::CommandBuffer secondary(*m_device, pool, VK_COMMAND_BUFFER_LEVEL_SECONDARY); + secondary.Begin(); + vk::CmdWaitEvents2(secondary, 1, &event.handle(), &dependency_info); + secondary.End(); + + vkt::CommandBuffer cb(*m_device, pool); + cb.Begin(); + cb.ExecuteCommands(secondary); + cb.End(); + + m_default_queue->Submit(m_command_buffer); + m_errorMonitor->SetDesiredError("UNASSIGNED-SubmitValidation-WaitEvents-WrongQueue"); + m_second_queue->Submit(cb); + m_errorMonitor->VerifyFound(); + m_device->Wait(); +} + TEST_F(NegativeEvent, WaitOnNoEvent) { RETURN_IF_SKIP(Init()); VkEvent bad_event = CastToHandle(0xbaadbeef); diff --git a/tests/unit/event_positive.cpp b/tests/unit/event_positive.cpp index f14b4f8cd2e..c7fea500743 100644 --- a/tests/unit/event_positive.cpp +++ b/tests/unit/event_positive.cpp @@ -315,3 +315,79 @@ TEST_F(PositiveEvent, AsymmetricEventNoMemorySubmit) { // Check that missing memory barrier does not confuse submit validation m_default_queue->SubmitAndWait(m_command_buffer); } + +TEST_F(PositiveEvent, InterQueueEventUsage) { + all_queue_count_ = true; + RETURN_IF_SKIP(Init()); + + if ((m_second_queue_caps & VK_QUEUE_GRAPHICS_BIT) == 0) { + GTEST_SKIP() << "2 graphics queues are needed"; + } + const vkt::Event event(*m_device); + + vkt::CommandBuffer cb1(*m_device, m_command_pool); + cb1.Begin(); + cb1.SetEvent(event); + cb1.End(); + + vkt::CommandPool pool2(*m_device, m_second_queue->family_index); + vkt::CommandBuffer cb2(*m_device, pool2); + cb2.Begin(); + cb2.SetEvent(event); + cb2.End(); + vkt::CommandBuffer cb3(*m_device, pool2); + cb3.Begin(); + cb3.WaitEvent(event); + cb3.End(); + + const VkCommandBuffer queue2_cbs[2] = {cb2, cb3}; + VkSubmitInfo queue2_submit_info = vku::InitStructHelper(); + queue2_submit_info.commandBufferCount = 2; + queue2_submit_info.pCommandBuffers = queue2_cbs; + + m_default_queue->SubmitAndWait(cb1); + + // Check that event wait on the second queue can find the signal on the same queue. + // If due to regression the signal from the first queue is used then it will trigger + // cross queue event usage error + vk::QueueSubmit(*m_second_queue, 1, &queue2_submit_info, VK_NULL_HANDLE); + m_second_queue->Wait(); +} + +TEST_F(PositiveEvent, InterQueueEventUsage2) { + all_queue_count_ = true; + RETURN_IF_SKIP(Init()); + + if ((m_second_queue_caps & VK_QUEUE_GRAPHICS_BIT) == 0) { + GTEST_SKIP() << "2 graphics queues are needed"; + } + const vkt::Event event(*m_device); + const vkt::Event event2(*m_device); + const VkEvent events[2] = {event, event2}; + + vkt::CommandBuffer cb1(*m_device, m_command_pool); + cb1.Begin(); + cb1.SetEvent(event2); + cb1.End(); + + vkt::CommandPool pool2(*m_device, m_second_queue->family_index); + vkt::CommandBuffer cb2(*m_device, pool2); + cb2.Begin(); + cb2.SetEvent(event); + cb2.End(); + vkt::CommandBuffer cb3(*m_device, pool2); + cb3.Begin(); + cb3.SetEvent(event2); + vk::CmdWaitEvents(cb3, 2, events, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, VK_PIPELINE_STAGE_ALL_COMMANDS_BIT, 0, nullptr, 0, + nullptr, 0, nullptr); + cb3.End(); + + const VkCommandBuffer queue2_cbs[2] = {cb2, cb3}; + VkSubmitInfo queue2_submit_info = vku::InitStructHelper(); + queue2_submit_info.commandBufferCount = 2; + queue2_submit_info.pCommandBuffers = queue2_cbs; + + m_default_queue->SubmitAndWait(cb1); + vk::QueueSubmit(*m_second_queue, 1, &queue2_submit_info, VK_NULL_HANDLE); + m_second_queue->Wait(); +}