panvk: Keep a ref to a pan_kmod_bo in panvk_buffer

We don't need the panfrost_bo object which contains both the BO and
its CPU/GPU mappings. We store the GPU address at bind time. We also
have a hack for index buffers which are currently walked by the CPU
to extract the min/max index. Add a comment to make sure this field
goes away when indirect draw is fixed.

While at it, keep a ref to the buffer object so we don't end up with
a invalid deref (UAF) if the vulkan user does something silly like
freeing the VkDeviceMemory object while the VkBuffer is still active.
Flag this with a TODO to make sure we don't forget about it.

Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Constantine Shablya <constantine.shablya@collabora.com>
Reviewed-by: Erik Faye-Lund <erik.faye-lund@collabora.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/26698>

src/panfrost/vulkan/panvk_device.c

@@ -1304,13 +1304,20 @@ panvk_BindBufferMemory2(VkDevice device, uint32_t bindInfoCount,
    for (uint32_t i = 0; i < bindInfoCount; ++i) {
       VK_FROM_HANDLE(panvk_device_memory, mem, pBindInfos[i].memory);
       VK_FROM_HANDLE(panvk_buffer, buffer, pBindInfos[i].buffer);
+      struct pan_kmod_bo *old_bo = buffer->bo;
 
       if (mem) {
-         buffer->bo = mem->bo;
-         buffer->bo_offset = pBindInfos[i].memoryOffset;
+         buffer->bo = pan_kmod_bo_get(mem->bo->kmod_bo);
+         buffer->dev_addr = mem->bo->ptr.gpu + pBindInfos[i].memoryOffset;
+         if (buffer->vk.usage & VK_BUFFER_USAGE_INDEX_BUFFER_BIT)
+            buffer->host_ptr = mem->bo->ptr.cpu + pBindInfos[i].memoryOffset;
       } else {
          buffer->bo = NULL;
+         buffer->dev_addr = 0;
+         buffer->host_ptr = NULL;
       }
+
+      pan_kmod_bo_put(old_bo);
    }
    return VK_SUCCESS;
 }
@@ -1490,6 +1497,7 @@ panvk_DestroyBuffer(VkDevice _device, VkBuffer _buffer,
    if (!buffer)
       return;
 
+   pan_kmod_bo_put(buffer->bo);
    vk_buffer_destroy(&device->vk, pAllocator, &buffer->vk);
 }
 

src/panfrost/vulkan/panvk_private.h

@@ -526,8 +526,21 @@ struct panvk_descriptor_pool {
 struct panvk_buffer {
    struct vk_buffer vk;
 
-   struct panfrost_bo *bo;
-   VkDeviceSize bo_offset;
+   mali_ptr dev_addr;
+
+   /* TODO: See if we can rework the synchronization logic so we don't need to
+    * pass BOs around.
+    */
+   struct pan_kmod_bo *bo;
+
+   /* FIXME: Only used for index buffers to do the min/max index retrieval on
+    * the CPU side. This is all broken anyway and the min/max search should be
+    * done with a compute shader that also patches the job descriptor
+    * accordingly (basically an indirect draw).
+    *
+    * Make sure this field goes away as soon as we fixed indirect draws.
+    */
+   void *host_ptr;
 };
 
 static inline mali_ptr
@@ -536,7 +549,7 @@ panvk_buffer_gpu_ptr(const struct panvk_buffer *buffer, uint64_t offset)
    if (buffer->bo == NULL)
       return 0;
 
-   return buffer->bo->ptr.gpu + buffer->bo_offset + offset;
+   return buffer->dev_addr + offset;
 }
 
 static inline uint64_t

src/panfrost/vulkan/panvk_vX_cmd_buffer.c

@@ -811,12 +811,11 @@ panvk_index_minmax_search(struct panvk_cmd_buffer *cmdbuf, uint32_t start,
                           uint32_t count, bool restart, uint32_t *min,
                           uint32_t *max)
 {
-   void *ptr = cmdbuf->state.ib.buffer->bo->ptr.cpu +
-               cmdbuf->state.ib.buffer->bo_offset + cmdbuf->state.ib.offset;
+   void *ptr = cmdbuf->state.ib.buffer->host_ptr + cmdbuf->state.ib.offset;
 
    assert(cmdbuf->state.ib.buffer);
    assert(cmdbuf->state.ib.buffer->bo);
-   assert(cmdbuf->state.ib.buffer->bo->ptr.cpu);
+   assert(cmdbuf->state.ib.buffer->host_ptr);
 
    uint32_t debug_flags =
       cmdbuf->device->physical_device->instance->debug_flags;

src/panfrost/vulkan/panvk_vX_meta_copy.c

@@ -1106,7 +1106,7 @@ panvk_meta_copy_buf2img(struct panvk_cmd_buffer *cmdbuf,
       struct panvk_batch *batch = panvk_cmd_open_batch(cmdbuf);
 
       view.first_layer = view.last_layer = l + first_layer;
-      batch->blit.src = buf->bo->kmod_bo;
+      batch->blit.src = buf->bo;
       batch->blit.dst = img->bo;
       panvk_per_arch(cmd_alloc_tls_desc)(cmdbuf, true);
       panvk_per_arch(cmd_alloc_fb_desc)(cmdbuf);
@@ -1523,7 +1523,7 @@ panvk_meta_copy_img2buf(struct panvk_cmd_buffer *cmdbuf,
    struct pan_tls_info tlsinfo = {0};
 
    batch->blit.src = img->bo;
-   batch->blit.dst = buf->bo->kmod_bo;
+   batch->blit.dst = buf->bo;
    batch->tls = pan_pool_alloc_desc(&cmdbuf->desc_pool.base, LOCAL_STORAGE);
    GENX(pan_emit_tls)(&tlsinfo, batch->tls.cpu);
 
@@ -1718,8 +1718,8 @@ panvk_meta_copy_buf2buf(struct panvk_cmd_buffer *cmdbuf,
 
    util_dynarray_append(&batch->jobs, void *, job.cpu);
 
-   batch->blit.src = src->bo->kmod_bo;
-   batch->blit.dst = dst->bo->kmod_bo;
+   batch->blit.src = src->bo;
+   batch->blit.dst = dst->bo;
    panvk_per_arch(cmd_close_batch)(cmdbuf);
 }
 
@@ -1863,7 +1863,7 @@ panvk_meta_fill_buf(struct panvk_cmd_buffer *cmdbuf,
 
    util_dynarray_append(&batch->jobs, void *, job.cpu);
 
-   batch->blit.dst = dst->bo->kmod_bo;
+   batch->blit.dst = dst->bo;
    panvk_per_arch(cmd_close_batch)(cmdbuf);
 }
 
@@ -1913,7 +1913,7 @@ panvk_meta_update_buf(struct panvk_cmd_buffer *cmdbuf,
 
    util_dynarray_append(&batch->jobs, void *, job.cpu);
 
-   batch->blit.dst = dst->bo->kmod_bo;
+   batch->blit.dst = dst->bo;
    panvk_per_arch(cmd_close_batch)(cmdbuf);
 }