panvk: Make panvk_pool_free_mem() error proof

It's pretty easy to pass the wrong pool to panvk_pool_free_mem()
(was the case in panvk_shader_destroy() and
panvk_internal_shader_destroy()), so let's make the existing interface
more robust to this kind of mistake by storing the 'owned-by-pool'
information at the panvk_priv_mem level. We use the lower 3 bits of the
BO pointer for that, since a BO object is guaranteed to be aligned on
8-byte.

Fixes: ce14681ebf ("panvk: Don't leak vertex shader program descriptors")
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Lars-Ivar Hesselberg Simonsen <lars-ivar.simonsen@arm.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/31625>

src/panfrost/vulkan/csf/panvk_vX_event.c

@@ -51,7 +51,7 @@ panvk_per_arch(DestroyEvent)(VkDevice _device, VkEvent _event,
    if (!event)
       return;
 
-   panvk_pool_free_mem(&device->mempools.rw_nc, event->syncobjs);
+   panvk_pool_free_mem(&event->syncobjs);
 
    vk_object_free(&device->vk, pAllocator, event);
 }

src/panfrost/vulkan/csf/panvk_vX_queue.c

@@ -22,7 +22,7 @@ finish_render_desc_ringbuf(struct panvk_queue *queue)
    struct panvk_device *dev = to_panvk_device(queue->vk.base.device);
    struct panvk_desc_ringbuf *ringbuf = &queue->render_desc_ringbuf;
 
-   panvk_pool_free_mem(&dev->mempools.rw, ringbuf->syncobj);
+   panvk_pool_free_mem(&ringbuf->syncobj);
 
    if (dev->debug.decode_ctx && ringbuf->addr.dev) {
       pandecode_inject_free(dev->debug.decode_ctx, ringbuf->addr.dev,
@@ -303,15 +303,13 @@ init_subqueue(struct panvk_queue *queue, enum panvk_subqueue_id subqueue)
 static void
 cleanup_queue(struct panvk_queue *queue)
 {
-   struct panvk_device *dev = to_panvk_device(queue->vk.base.device);
-
    for (uint32_t i = 0; i < PANVK_SUBQUEUE_COUNT; i++)
-      panvk_pool_free_mem(&dev->mempools.rw, queue->subqueues[i].context);
+      panvk_pool_free_mem(&queue->subqueues[i].context);
 
    finish_render_desc_ringbuf(queue);
 
-   panvk_pool_free_mem(&dev->mempools.rw, queue->debug_syncobjs);
-   panvk_pool_free_mem(&dev->mempools.rw, queue->syncobjs);
+   panvk_pool_free_mem(&queue->debug_syncobjs);
+   panvk_pool_free_mem(&queue->syncobjs);
 }
 
 static VkResult
@@ -475,7 +473,7 @@ init_tiler(struct panvk_queue *queue)
    return VK_SUCCESS;
 
 err_free_desc:
-   panvk_pool_free_mem(&dev->mempools.rw, tiler_heap->desc);
+   panvk_pool_free_mem(&tiler_heap->desc);
    return result;
 }
 
@@ -491,7 +489,7 @@ cleanup_tiler(struct panvk_queue *queue)
       drmIoctl(dev->vk.drm_fd, DRM_IOCTL_PANTHOR_TILER_HEAP_DESTROY, &thd);
    assert(!ret);
 
-   panvk_pool_free_mem(&dev->mempools.rw, tiler_heap->desc);
+   panvk_pool_free_mem(&tiler_heap->desc);
 }
 
 static VkResult

src/panfrost/vulkan/panvk_mempool.c

@@ -143,8 +143,16 @@ panvk_pool_alloc_mem(struct panvk_pool *pool, struct panvk_pool_alloc_info info)
          panvk_priv_bo_ref(bo);
    }
 
+   uint32_t flags = 0;
+
+   if (pool->props.owns_bos)
+      flags |= PANVK_PRIV_MEM_OWNED_BY_POOL;
+
+   assert(!((uintptr_t)bo & 7));
+   assert(!(flags & ~7));
+
    struct panvk_priv_mem ret = {
-      .bo = bo,
+      .bo = (uintptr_t)bo | flags,
       .offset = offset,
    };
 

src/panfrost/vulkan/panvk_mempool.h

@@ -119,23 +119,41 @@ panvk_pool_num_bos(struct panvk_pool *pool)
 
 void panvk_pool_get_bo_handles(struct panvk_pool *pool, uint32_t *handles);
 
+enum panvk_priv_mem_flags {
+   PANVK_PRIV_MEM_OWNED_BY_POOL = BITFIELD_BIT(0),
+};
+
 struct panvk_priv_mem {
-   struct panvk_priv_bo *bo;
+   uintptr_t bo;
    unsigned offset;
 };
 
+static struct panvk_priv_bo *
+panvk_priv_mem_bo(struct panvk_priv_mem mem)
+{
+   return (void *)(mem.bo & ~7ull);
+}
+
+static uint32_t
+panvk_priv_mem_flags(struct panvk_priv_mem mem)
+{
+   return mem.bo & 7ull;
+}
+
 static inline uint64_t
 panvk_priv_mem_dev_addr(struct panvk_priv_mem mem)
 {
-   return mem.bo ? mem.bo->addr.dev + mem.offset : 0;
+   struct panvk_priv_bo *bo = panvk_priv_mem_bo(mem);
+
+   return bo ? bo->addr.dev + mem.offset : 0;
 }
 
 static inline void *
 panvk_priv_mem_host_addr(struct panvk_priv_mem mem)
 {
-   return mem.bo && mem.bo->addr.host
-             ? (uint8_t *)mem.bo->addr.host + mem.offset
-             : NULL;
+   struct panvk_priv_bo *bo = panvk_priv_mem_bo(mem);
+
+   return bo && bo->addr.host ? (uint8_t *)bo->addr.host + mem.offset : NULL;
 }
 
 struct panvk_pool_alloc_info {
@@ -160,10 +178,17 @@ struct panvk_priv_mem panvk_pool_alloc_mem(struct panvk_pool *pool,
                                            struct panvk_pool_alloc_info info);
 
 static inline void
-panvk_pool_free_mem(struct panvk_pool *pool, struct panvk_priv_mem mem)
+panvk_pool_free_mem(struct panvk_priv_mem *mem)
 {
-   if (!pool->props.owns_bos)
-      panvk_priv_bo_unref(mem.bo);
+   struct panvk_priv_bo *bo = panvk_priv_mem_bo(*mem);
+   uint32_t flags = panvk_priv_mem_flags(*mem);
+
+   if (bo) {
+      if (likely(!(flags & PANVK_PRIV_MEM_OWNED_BY_POOL)))
+         panvk_priv_bo_unref(bo);
+
+      memset(mem, 0, sizeof(*mem));
+   }
 }
 
 static inline struct panvk_priv_mem

src/panfrost/vulkan/panvk_shader.h

@@ -186,11 +186,10 @@ VkResult panvk_per_arch(link_shaders)(struct panvk_pool *desc_pool,
                                       struct panvk_shader_link *link);
 
 static inline void
-panvk_shader_link_cleanup(struct panvk_pool *desc_pool,
-                          struct panvk_shader_link *link)
+panvk_shader_link_cleanup(struct panvk_shader_link *link)
 {
-   panvk_pool_free_mem(desc_pool, link->vs.attribs);
-   panvk_pool_free_mem(desc_pool, link->fs.attribs);
+   panvk_pool_free_mem(&link->vs.attribs);
+   panvk_pool_free_mem(&link->fs.attribs);
 }
 
 bool panvk_per_arch(nir_lower_descriptors)(

src/panfrost/vulkan/panvk_vX_buffer_view.c

@@ -153,6 +153,6 @@ panvk_per_arch(DestroyBufferView)(VkDevice _device, VkBufferView bufferView,
    if (!view)
       return;
 
-   panvk_pool_free_mem(&device->mempools.rw, view->mem);
+   panvk_pool_free_mem(&view->mem);
    vk_buffer_view_destroy(&device->vk, pAllocator, &view->vk);
 }

src/panfrost/vulkan/panvk_vX_image_view.c

@@ -279,6 +279,6 @@ panvk_per_arch(DestroyImageView)(VkDevice _device, VkImageView _view,
    if (!view)
       return;
 
-   panvk_pool_free_mem(&device->mempools.rw, view->mem);
+   panvk_pool_free_mem(&view->mem);
    vk_image_view_destroy(&device->vk, pAllocator, &view->vk);
 }

src/panfrost/vulkan/panvk_vX_shader.c

@@ -769,18 +769,18 @@ panvk_shader_destroy(struct vk_device *vk_dev, struct vk_shader *vk_shader,
    free((void *)shader->asm_str);
    ralloc_free((void *)shader->nir_str);
 
-   panvk_pool_free_mem(&dev->mempools.exec, shader->code_mem);
+   panvk_pool_free_mem(&shader->code_mem);
 
 #if PAN_ARCH <= 7
-   panvk_pool_free_mem(&dev->mempools.exec, shader->rsd);
-   panvk_pool_free_mem(&dev->mempools.exec, shader->desc_info.others.map);
+   panvk_pool_free_mem(&shader->rsd);
+   panvk_pool_free_mem(&shader->desc_info.others.map);
 #else
    if (shader->info.stage != MESA_SHADER_VERTEX) {
-      panvk_pool_free_mem(&dev->mempools.exec, shader->spd);
+      panvk_pool_free_mem(&shader->spd);
    } else {
-      panvk_pool_free_mem(&dev->mempools.exec, shader->spds.var);
-      panvk_pool_free_mem(&dev->mempools.exec, shader->spds.pos_points);
-      panvk_pool_free_mem(&dev->mempools.exec, shader->spds.pos_triangles);
+      panvk_pool_free_mem(&shader->spds.var);
+      panvk_pool_free_mem(&shader->spds.pos_points);
+      panvk_pool_free_mem(&shader->spds.pos_triangles);
    }
 #endif
 
@@ -1455,12 +1455,12 @@ panvk_internal_shader_destroy(struct vk_device *vk_dev,
    struct panvk_internal_shader *shader =
       container_of(vk_shader, struct panvk_internal_shader, vk);
 
-   panvk_pool_free_mem(&dev->mempools.exec, shader->code_mem);
+   panvk_pool_free_mem(&shader->code_mem);
 
 #if PAN_ARCH <= 7
-   panvk_pool_free_mem(&dev->mempools.exec, shader->rsd);
+   panvk_pool_free_mem(&shader->rsd);
 #else
-   panvk_pool_free_mem(&dev->mempools.exec, shader->spd);
+   panvk_pool_free_mem(&shader->spd);
 #endif
 
    vk_shader_free(&dev->vk, pAllocator, &shader->vk);