v3dv: fix job suspend with command buffer simultaneous use flag

With the simultaneous use flag we can reuse the same command
buffer multiple times. That means, for example, that we can
have an instance of a job running in the GPU while we are
submitting another one for execution to a queue.

This scenario is problematic with dynamic rendering and job
suspension because suspended jobs need to be patched with the
resume address at queue submit time, and thus, if we have another
instance of the same job currently executing in the GPU we could
stomp its resume address, which could be different.

To fix this, at queue submission time, when we detect a suspending
job in a command buffer with the simultaneous use flag, we clone the
job and create its own copy of the BCL so we can patch the resume
address into it safely without conflicting with any other instance
of the job that may be running.

We need to flag these clones  as having their own BCL since
we would have to free it when the job is destroyed, unlike other
clones that don't own any resources of their own. Also, because
this job is created at queue submit time, it won't be in the
execution list of the command buffer, so it won't be automatically
destroyed with it, so we need to add it to the command buffer
as a private object.

Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/28521>

src/broadcom/vulkan/v3dv_cmd_buffer.c

@@ -149,9 +149,21 @@ job_destroy_cloned_gpu_cl_resources(struct v3dv_job *job)
 {
    assert(job->type == V3DV_JOB_TYPE_GPU_CL);
 
-   list_for_each_entry_safe(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) {
-      list_del(&bo->list_link);
-      vk_free(&job->device->vk.alloc, bo);
+   struct v3dv_cmd_buffer *cmd_buffer = job->cmd_buffer;
+   if (job->clone_owns_bcl) {
+      /* For suspending jobs in command buffers with the simultaneous use flag
+       * we allocate a real copy of the BCL.
+       */
+      assert(job->suspending &&
+             cmd_buffer &&
+             (cmd_buffer->usage_flags &
+              VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT));
+      v3dv_cl_destroy(&job->bcl);
+   } else {
+      list_for_each_entry_safe(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) {
+         list_del(&bo->list_link);
+         vk_free(&job->device->vk.alloc, bo);
+      }
    }
 
    list_for_each_entry_safe(struct v3dv_bo, bo, &job->rcl.bo_list, list_link) {
@@ -1923,7 +1935,7 @@ clone_bo_list(struct v3dv_device *device,
 }
 
 struct v3dv_job *
-v3dv_job_clone(struct v3dv_job *job)
+v3dv_job_clone(struct v3dv_job *job, bool skip_bcl)
 {
    struct v3dv_job *clone = vk_alloc(&job->device->vk.alloc,
                                      sizeof(struct v3dv_job), 8,
@@ -1931,7 +1943,16 @@ v3dv_job_clone(struct v3dv_job *job)
    if (!clone)
       return NULL;
 
-   /* Cloned jobs don't duplicate resources! */
+   /* Cloned jobs don't duplicate resources, they share their CLs with the
+    * oringinal job, since they are typically read-only. The exception to this
+    * is dynamic rendering suspension paired with
+    * VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT, since in that case we need
+    * to patch the BCL with the resume address and for that we need to create a
+    * copy of the job so we avoid rewriting the resume address for another copy
+    * of the same job that may be running in the GPU. When we create a job for
+    * this use case skip_bcl is set to True and the caller will be responsible
+    * for creating the BCL.
+    */
    *clone = *job;
    clone->is_clone = true;
    clone->cmd_buffer = NULL;
@@ -1947,8 +1968,10 @@ v3dv_job_clone(struct v3dv_job *job)
       clone->rcl.job = clone;
       clone->indirect.job = clone;
 
-      if (!clone_bo_list(device, &clone->bcl.bo_list, &job->bcl.bo_list))
+      if (!skip_bcl &&
+          !clone_bo_list(device, &clone->bcl.bo_list, &job->bcl.bo_list)) {
          return NULL;
+      }
       if (!clone_bo_list(device, &clone->rcl.bo_list, &job->rcl.bo_list))
          return NULL;
       if (!clone_bo_list(device, &clone->indirect.bo_list, &job->indirect.bo_list))
@@ -1968,7 +1991,7 @@ struct v3dv_job *
 v3dv_job_clone_in_cmd_buffer(struct v3dv_job *job,
                              struct v3dv_cmd_buffer *cmd_buffer)
 {
-   struct v3dv_job *clone = v3dv_job_clone(job);
+   struct v3dv_job *clone = v3dv_job_clone(job, false);
    if (!clone) {
       v3dv_flag_oom(cmd_buffer, NULL);
       return NULL;

src/broadcom/vulkan/v3dv_private.h

@@ -1235,6 +1235,12 @@ struct v3dv_job {
     */
    bool is_clone;
 
+   /* If this is a cloned job, if it has its own BCL resource. This happens
+    * when we suspend jobs with in command buffers with the
+    * VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT flag.
+    */
+   bool clone_owns_bcl;
+
    /* VK_KHR_dynamic_rendering */
    bool suspending;
    bool resuming;
@@ -1398,7 +1404,7 @@ void v3dv_job_start_frame(struct v3dv_job *job,
 bool v3dv_job_type_is_gpu(struct v3dv_job *job);
 
 struct v3dv_job *
-v3dv_job_clone(struct v3dv_job *job);
+v3dv_job_clone(struct v3dv_job *job, bool skip_bcl);
 
 struct v3dv_job *
 v3dv_job_clone_in_cmd_buffer(struct v3dv_job *job,

src/broadcom/vulkan/v3dv_queue.c

@@ -1228,11 +1228,6 @@ v3dv_queue_driver_submit(struct vk_queue *vk_queue,
    for (int i = 0; i < V3DV_QUEUE_COUNT; i++)
       queue->last_job_syncs.first[i] = true;
 
-   /* FIXME: if suspend/resume chains are recorded into command buffers with
-    * usage flag VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT, then this won't
-    * work and we would need to "clone" the jobs instead so we never patch a
-    * job that may still be executing.
-    */
    struct v3dv_job *first_suspend_job = NULL;
    struct v3dv_job *current_suspend_job = NULL;
    for (uint32_t i = 0; i < submit->command_buffer_count; i++) {
@@ -1240,6 +1235,13 @@ v3dv_queue_driver_submit(struct vk_queue *vk_queue,
          container_of(submit->command_buffers[i], struct v3dv_cmd_buffer, vk);
       list_for_each_entry_safe(struct v3dv_job, job,
                                &cmd_buffer->jobs, list_link) {
+         if (job->suspending) {
+            job = v3dv_X(job->device,
+                         cmd_buffer_prepare_suspend_job_for_submit)(job);
+            if (!job)
+               return VK_ERROR_OUT_OF_DEVICE_MEMORY;
+         }
+
          if (job->suspending && !job->resuming) {
             assert(!first_suspend_job);
             assert(!current_suspend_job);

src/broadcom/vulkan/v3dvx_cmd_buffer.c

@@ -2776,3 +2776,111 @@ v3dX(job_patch_resume_address)(struct v3dv_job *first_suspend,
    first_suspend->suspended_bcl_end = resume->bcl.bo->offset +
                                       v3dv_cl_offset(&resume->bcl);
 }
+
+static void
+job_destroy_cb(VkDevice device, uint64_t pobj, VkAllocationCallbacks *allocb)
+{
+   struct v3dv_job *clone = (struct v3dv_job *) (uintptr_t) pobj;
+   v3dv_job_destroy(clone);
+}
+
+/**
+ * This checks if the command buffer has been created with
+ * VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT, in which case we won't be
+ * able to safely patch the resume address into the job (since we could have
+ * another instance of this job running in the GPU, potentially resuming in a
+ * different address). In that case, we clone the job and make the clone have
+ * its own BCL copied from the original job so we can later patch the resume
+ * address into it safely.
+ */
+struct v3dv_job *
+v3dX(cmd_buffer_prepare_suspend_job_for_submit)(struct v3dv_job *job)
+{
+   assert(job->suspending);
+   assert(job->cmd_buffer);
+   assert(job->type == V3DV_JOB_TYPE_GPU_CL);
+
+   if (!(job->cmd_buffer->usage_flags & VK_COMMAND_BUFFER_USAGE_SIMULTANEOUS_USE_BIT))
+      return job;
+
+   /* Create the clone job, but skip the BCL since we are going to create
+    * our own below.
+    */
+   struct v3dv_job *clone = v3dv_job_clone(job, true);
+   if (!clone)
+      return NULL;
+
+   /* Compute total size of BCL we need to copy */
+   uint32_t bcl_size = 0;
+   list_for_each_entry(struct v3dv_bo, bo, &job->bcl.bo_list, list_link)
+      bcl_size += bo->size;
+
+   /* Prepare the BCL for the cloned job. For this we go over the BOs in the
+    * BCL of the original job and we copy their contents into the single BO
+    * in the BCL of the cloned job.
+    */
+   clone->clone_owns_bcl = true;
+   v3dv_cl_init(clone, &clone->bcl);
+   v3dv_cl_ensure_space(&clone->bcl, bcl_size, 4);
+   if (!clone->bcl.bo)
+      return NULL;
+
+   assert(clone->bcl.base);
+   assert(clone->bcl.base == clone->bcl.next);
+
+   /* Unlink this job from the command buffer's execution list */
+   list_inithead(&clone->list_link);
+
+   /* Copy the contents of each BO in the original job's BCL into the single
+    * BO we have in the clone's BCL.
+    *
+    * If the BO is the last in the BCL (which we can tell because it wouldn't
+    * have emitted a BRANCH instruction to link to another BO) we need to copy
+    * up to the current BCL offset, otherwise we need to copy up to the BRANCH
+    * instruction (excluded, since we are putting everything together into a
+    * single BO here).
+    */
+   list_for_each_entry(struct v3dv_bo, bo, &job->bcl.bo_list, list_link) {
+      assert(bo->map);
+      uint32_t copy_size;
+      if (bo->cl_branch_offset == 0xffffffff) { /* Last BO in BCL */
+         assert(bo == list_last_entry(&job->bcl.bo_list, struct v3dv_bo, list_link));
+         copy_size = v3dv_cl_offset(&job->bcl);
+      } else {
+         assert(bo->cl_branch_offset >= cl_packet_length(BRANCH));
+         copy_size = bo->cl_branch_offset - cl_packet_length(BRANCH);
+      }
+
+      assert(v3dv_cl_offset(&job->bcl) + copy_size < bcl_size);
+      memcpy(cl_start(&clone->bcl), bo->map, copy_size);
+      cl_advance_and_end(&clone->bcl, copy_size);
+   }
+
+   /* Now we need to fixup the pointer to the suspend BRANCH instruction at the
+    * end of the BCL so it points to the address in the new BCL. We know that
+    * to suspend a command buffer we always emit a BRANCH+NOP combo, so we just
+    * need to go back that many bytes in to the BCL to find the instruction.
+    */
+   uint32_t suspend_terminator_size =
+      cl_packet_length(BRANCH) + cl_packet_length(NOP);
+   clone->suspend_branch_inst_ptr = (struct v3dv_cl_out *)
+      (((uint8_t *)cl_start(&clone->bcl)) - suspend_terminator_size);
+   assert(*(((uint8_t *)clone->suspend_branch_inst_ptr)) == V3DX(BRANCH_opcode));
+
+   /* This job is not in the execution list of the command buffer so it
+    * won't be destroyed with it; add it as a private object to get it freed.
+    *
+    * FIXME: every time this job is submitted we clone the job and we only
+    * destroy it when the command buffer is destroyed. If the user keeps the
+    * command buffer for the entire lifetime of the application, this command
+    * buffer could grow significantly, so maybe we want to do something smarter
+    * like having a syncobj bound to these jobs and every time we submit the
+    * command buffer again we first check these sncobjs to see if we can free
+    * some of these clones so we avoid blowing up memory.
+    */
+   v3dv_cmd_buffer_add_private_obj(
+      job->cmd_buffer, (uintptr_t)clone,
+      (v3dv_cmd_buffer_private_obj_destroy_cb)job_destroy_cb);
+
+   return clone;
+}

src/broadcom/vulkan/v3dvx_private.h

@@ -130,6 +130,9 @@ v3dX(cmd_buffer_emit_indexed_indirect)(struct v3dv_cmd_buffer *cmd_buffer,
 void
 v3dX(cmd_buffer_suspend)(struct v3dv_cmd_buffer *cmd_buffer);
 
+struct v3dv_job *
+v3dX(cmd_buffer_prepare_suspend_job_for_submit)(struct v3dv_job *job);
+
 void
 v3dX(get_hw_clear_color)(const VkClearColorValue *color,
                          uint32_t internal_type,