layers: Remove QueueSubmissionValidator

BUILD.gn

@@ -144,8 +144,6 @@ vvl_sources = [
   "layers/core_checks/cc_spirv.cpp",
   "layers/core_checks/cc_state_tracker.cpp",
   "layers/core_checks/cc_state_tracker.h",
-  "layers/core_checks/cc_submit.cpp",
-  "layers/core_checks/cc_submit.h",
   "layers/core_checks/cc_sync_vuid_maps.h",
   "layers/core_checks/cc_sync_vuid_maps.cpp",
   "layers/core_checks/cc_synchronization.cpp",

layers/CMakeLists.txt

@@ -244,8 +244,6 @@ target_sources(vvl PRIVATE
     core_checks/cc_shader_object.cpp
     core_checks/cc_state_tracker.h
     core_checks/cc_state_tracker.cpp
-    core_checks/cc_submit.h
-    core_checks/cc_submit.cpp
     core_checks/cc_sync_vuid_maps.h
     core_checks/cc_sync_vuid_maps.cpp
     core_checks/cc_synchronization.h

layers/core_checks/cc_queue.cpp

@@ -290,9 +290,11 @@ bool CoreChecks::PreCallValidateQueueSubmit(VkQueue queue, uint32_t submitCount,
                                  chained_device_group_struct->commandBufferCount, submit.commandBufferCount);
             }
         }
-        // Perform submit time validation at the end.
-        // If submit API is used incorrectly, we want those errors to be reported first
-        skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+        // some tests expect that submit state is not updated if regular validation fails
+        if (!skip) {
+            skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        }
     }
 
     return skip;
@@ -452,9 +454,11 @@ bool CoreChecks::ValidateQueueSubmit2(VkQueue queue, uint32_t submitCount, const
             skip |= LogError("VUID-VkSubmitInfo2-commandBuffer-06010", queue, submit_loc,
                              "has a suspended render pass instance that was not resumed.");
         }
-        // Perform submit time validation at the end.
-        // If submit API is used incorrectly, we want those errors to be reported first
-        skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+        // some tests expect that submit state is not updated if regular validation fails
+        if (!skip) {
+            skip |= submit_time_tracker.ProcessSubmitInfo(submit, queue, submit_loc);
+        }
     }
 
     return skip;
@@ -783,8 +787,21 @@ bool CoreChecks::PreCallValidateQueueBindSparse(VkQueue queue, uint32_t bindInfo
     return skip;
 }
 
-bool CoreChecks::ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
-                                        const Location& submit_loc) {
+static Location GetSignaledSemaphoreLocation(const Location& submit_loc, uint32_t index) {
+    vvl::Field field = vvl::Field::Empty;
+    if (submit_loc.function == vvl::Func::vkQueueSubmit || submit_loc.function == vvl::Func::vkQueueBindSparse) {
+        field = vvl::Field::pSignalSemaphores;
+    } else if (submit_loc.function == vvl::Func::vkQueueSubmit2 || submit_loc.function == vvl::Func::vkQueueSubmit2KHR) {
+        field = vvl::Field::pSignalSemaphoreInfos;
+    } else {
+        assert(false && "Unhandled signaling function");
+    }
+    return submit_loc.dot(field, index);
+}
+
+bool CoreChecks::ProcessSubmissionBatch(const vvl::SubmitTimeTracker& tracker,
+                                        const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
+                                        vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores, const Location& submit_loc) {
     bool skip = false;
     // Validate image layouts on the command buffer boundaries
     {
@@ -796,6 +813,27 @@ bool CoreChecks::ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::C
             }
         }
     }
+    // Ensure that timeline signals are monotonically increasing values
+    for (uint32_t i = 0; i < (uint32_t)signal_semaphores.size(); ++i) {
+        const VkSemaphore semaphore = signal_semaphores[i].semaphore;
+        const std::optional<uint64_t> current_value = tracker.GetTimelineValue(semaphore);
+        if (!current_value.has_value()) {
+            continue;
+        }
+        // Validate the case where the signal value is less than the current payload.
+        // Equality, which is also invalid, is handled earlier by SemaphoreSubmitState::ValidateTimelineSignal.
+        // Equality can be checked early because it is invalid regardless submit order.
+        // For not equal values, the comparison depends on submit ordering and is handled here
+        const uint64_t signal_value = signal_semaphores[i].value;
+        const bool invlid_signal_value = signal_value < *current_value;
+        if (invlid_signal_value) {
+            const Location signal_semaphore_loc = GetSignaledSemaphoreLocation(submit_loc, i);
+            const auto& vuid = GetQueueSubmitVUID(signal_semaphore_loc, vvl::SubmitError::kTimelineSemSmallValue);
+            LogError(vuid, semaphore, signal_semaphore_loc,
+                     "(%s) signaled with value %" PRIu64 " which is smaller than the current value %" PRIu64,
+                     FormatHandle(semaphore).c_str(), signal_value, *current_value);
+        }
+    }
     if (!skip) {
         for (const auto& cb : command_buffers) {
             if (cb) {

layers/core_checks/cc_state_tracker.cpp

@@ -41,7 +41,7 @@ void CoreChecks::Created(vvl::CommandBuffer& cb) {
 }
 
 void CoreChecks::Created(vvl::Queue& queue) {
-    queue.SetSubState(container_type, std::make_unique<core::QueueSubState>(*this, queue));
+    queue.SetSubState(container_type, std::make_unique<core::QueueSubState>(queue));
 }
 
 namespace core {
@@ -1214,9 +1214,6 @@ void CommandBufferSubState::SubmitTimeValidate() {
     }
 }
 
-QueueSubState::QueueSubState(CoreChecks& core_checks, vvl::Queue& q)
-    : vvl::QueueSubState(q), queue_submission_validator_(core_checks) {}
-
 void QueueSubState::PreSubmit(std::vector<vvl::QueueSubmission>& submissions) {
     for (const auto& submission : submissions) {
         for (auto& cb : submission.cb_submissions) {
@@ -1228,8 +1225,6 @@ void QueueSubState::PreSubmit(std::vector<vvl::QueueSubmission>& submissions) {
 }
 
 void QueueSubState::Retire(vvl::QueueSubmission& submission) {
-    queue_submission_validator_.Validate(submission);
-
     auto is_query_updated_after = [this](const QueryObject& query_object) {
         auto guard = base.Lock();
         bool first_queue_submission = true;

layers/core_checks/cc_state_tracker.h

@@ -18,7 +18,6 @@
  * limitations under the License.
  */
 #pragma once
-#include "cc_submit.h"
 #include "state_tracker/state_tracker.h"
 #include "state_tracker/cmd_buffer_state.h"
 #include "state_tracker/queue_state.h"
@@ -245,15 +244,12 @@ static inline const CommandBufferSubState &SubState(const vvl::CommandBuffer &cb
 
 class QueueSubState : public vvl::QueueSubState {
   public:
-    QueueSubState(CoreChecks &core_checks, vvl::Queue& q);
+    QueueSubState(vvl::Queue& q) : vvl::QueueSubState(q) {}
 
     void PreSubmit(std::vector<vvl::QueueSubmission> &submissions) override;
 
     // Override Retire to validate submissions in the order defined by synchronization
     void Retire(vvl::QueueSubmission&) override;
-
-  private:
-    QueueSubmissionValidator queue_submission_validator_;
 };
 
 }  // namespace core

layers/core_checks/cc_synchronization.cpp

@@ -1648,9 +1648,11 @@ bool CoreChecks::PreCallValidateSignalSemaphore(VkDevice device, const VkSemapho
                          FormatHandle(*semaphore_state).c_str(), payload_type, *far_away_payload);
     }
 
-    // Perform submit time validation at the end.
-    // If signal semaphore API is used incorrectly, we want those errors to be reported first
-    skip |= submit_time_tracker.ProcessSignalSemaphore(*pSignalInfo);
+    // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+    // some tests expect that submit state is not updated if regular validation fails
+    if (!skip) {
+        skip |= submit_time_tracker.ProcessSignalSemaphore(*pSignalInfo);
+    }
     return skip;
 }
 

layers/core_checks/cc_wsi.cpp

@@ -1263,9 +1263,11 @@ bool CoreChecks::PreCallValidateQueuePresentKHR(VkQueue queue, const VkPresentIn
         skip |= ValidateSwapchainPresentFenceInfo(queue, *pPresentInfo, *swapchain_present_fence_info, present_info_loc);
     }
 
-    // Perform submit time validation at the end.
-    // If present API is used incorrectly, we want those errors to be reported first
-    skip |= submit_time_tracker.ProcessPresent(*pPresentInfo, present_info_loc);
+    // Run submit time validation only if skip is not set. This is mostly for vvl testing,
+    // some tests expect that submit state is not updated if regular validation fails
+    if (!skip) {
+        skip |= submit_time_tracker.ProcessPresent(*pPresentInfo, present_info_loc);
+    }
 
     return skip;
 }

layers/core_checks/core_validation.h

@@ -1587,8 +1587,9 @@ class CoreChecks : public vvl::DeviceProxy {
                                        const RecordObject& record_obj) override;
     void PostCallRecordQueueSubmit2(VkQueue queue, uint32_t submitCount, const VkSubmitInfo2* pSubmits, VkFence fence,
                                     const RecordObject& record_obj) override;
-    bool ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
-                                const Location& submit_loc) override;
+    bool ProcessSubmissionBatch(const vvl::SubmitTimeTracker& tracker,
+                                const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
+                                vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores, const Location& submit_loc) override;
     bool ProcessPresentBatch(const vvl::Image& swapchain_image, const Location& present_info_loc) override;
     bool IgnoreAllocationSize(const VkMemoryAllocateInfo& allocate_info) const;
     bool HasExternalMemoryImportSupport(const vvl::Buffer& buffer, VkExternalMemoryHandleTypeFlagBits handle_type) const;

layers/state_tracker/queue_state.h

@@ -20,22 +20,20 @@
 #include "state_tracker/state_object.h"
 #include "state_tracker/fence_state.h"
 #include "state_tracker/semaphore_state.h"
+#include "error_message/error_location.h"
+#include "chassis/dispatch_object.h"
+#include "vk_layer_config.h"
 #include <condition_variable>
 #include <deque>
 #include <future>
+#include <string>
 #include <thread>
 #include <vector>
-#include <string>
-#include "error_message/error_location.h"
-#include "chassis/dispatch_object.h"
-#include "vk_layer_config.h"
 
 namespace vvl {
 
 class CommandBuffer;
 class DeviceState;
-class Image;
-class Queue;
 class QueueSubState;
 
 struct CommandBufferSubmission {
@@ -72,7 +70,6 @@ struct QueueSubmission {
 
     // Swapchain is not null if this submission represents QueuePresent request
     std::shared_ptr<Swapchain> swapchain;
-    std::shared_ptr<const vvl::Image> swapchain_image;
 
     LocationCapture loc;
     uint64_t seq{0};

layers/state_tracker/semaphore_state.cpp

@@ -407,7 +407,7 @@ bool vvl::Semaphore::CanRetireTimelineWait(const vvl::Queue* current_queue, uint
             continue;
         }
         // If the next signal is on the waiting (current) queue, it can't be a resolving signal (blocked by wait).
-        // QueueSubmissionValidator will also report an error about non-increasing signal values
+        // Submit time validation will also report an error about non-increasing signal values
         if (t.signal_submit->queue != nullptr && t.signal_submit->queue == current_queue) {
             continue;
         }

layers/state_tracker/state_tracker.cpp

@@ -4398,9 +4398,7 @@ void DeviceState::PostCallRecordQueuePresentKHR(VkQueue queue, const VkPresentIn
         if (present_fence_info) {
             present_submission.AddFence(Get<Fence>(present_fence_info->pFences[i]));
         }
-        auto swapchain = Get<Swapchain>(pPresentInfo->pSwapchains[i]);
-        present_submission.swapchain = swapchain;
-        present_submission.swapchain_image = swapchain->GetSwapChainImageShared(pPresentInfo->pImageIndices[i]);
+        present_submission.swapchain = Get<Swapchain>(pPresentInfo->pSwapchains[i]);
     }
 
     vvl::Semaphore::SwapchainWaitInfo semaphore_swapchain_info;

layers/state_tracker/state_tracker.h

@@ -83,6 +83,7 @@ struct ShaderObject;
 class VideoSession;
 class VideoSessionParameters;
 class DataGraphPipelineSession;
+class SubmitTimeTracker;
 }  // namespace vvl
 
 namespace chassis {
@@ -2358,8 +2359,9 @@ class DeviceProxy : public vvl::BaseDevice {
     // especially when a timeline signal resolves pending work on another queue.
     // This became even more important after the spec allowed internally synchronized queues,
     // which means the same queue can be used from multiple threads
-    virtual bool ProcessSubmissionBatch(const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
-                                        const Location& submit_loc) {
+    virtual bool ProcessSubmissionBatch(const SubmitTimeTracker& tracker,
+                                        const std::vector<std::shared_ptr<vvl::CommandBuffer>>& command_buffers,
+                                        vvl::span<const VkSemaphoreSubmitInfo> signal_semaphores, const Location& submit_loc) {
         return false;
     }
 

layers/state_tracker/submit_time_tracker.cpp

@@ -100,7 +100,7 @@ bool SubmitTimeTracker::ProcessBatch(std::vector<std::shared_ptr<CommandBuffer>>
         return skip;
     }
 
-    skip |= validator_.ProcessSubmissionBatch(command_buffers, submit_loc);
+    skip |= validator_.ProcessSubmissionBatch(*this, command_buffers, signal_semaphores, submit_loc);
 
     const bool new_timeline_signals = RegisterTimelineSignals(signal_semaphores);
     if (new_timeline_signals) {
@@ -159,7 +159,8 @@ bool SubmitTimeTracker::PropagateTimelineSignals() {
                 if (!CanBeResolved(batch)) {
                     break;
                 }
-                skip |= validator_.ProcessSubmissionBatch(batch.command_buffers, batch.submit_loc_capture.Get());
+                const auto signals = vvl::span<const VkSemaphoreSubmitInfo>(batch.signals.data(), batch.signals.size());
+                skip |= validator_.ProcessSubmissionBatch(*this, batch.command_buffers, signals, batch.submit_loc_capture.Get());
                 new_timeline_signals |= RegisterTimelineSignals(batch.signals);
                 batches.erase(batches.begin());
             }