iris: fix export of GEM handles

We reuse DRM file descriptors internally. Therefore when we export a
GEM handle we must do so in the file descriptor used externally.

This change also fixes a file descriptor leak of the FD given at
screen creation.

v2: Don't bother checking fd equals, they're always different
    Fix dmabuf leak
    Fix GEM handle leaks by tracking exported handles

v3: Check os_same_file_description error (Michel)
    Don't create multiple exports for a given GEM table

v4: Add WARN_ONCE (Ken)
    Rename external_fd to winsys_fd

v5: Remove export lock in favor of bufmgr's

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/2882
Fixes: 7557f16059 ("iris: share buffer managers accross screens")
Tested-by: Eric Engestrom <eric@engestrom.ch>
Tested-by: Tapani Pälli <tapani.palli@intel.com>
Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4861>

src/gallium/drivers/iris/iris_bufmgr.c

@@ -63,6 +63,7 @@
 #include "util/macros.h"
 #include "util/hash_table.h"
 #include "util/list.h"
+#include "util/os_file.h"
 #include "util/u_dynarray.h"
 #include "util/vma.h"
 #include "iris_bufmgr.h"
@@ -91,6 +92,17 @@
 
 #define PAGE_SIZE 4096
 
+#define WARN_ONCE(cond, fmt...) do {                            \
+   if (unlikely(cond)) {                                        \
+      static bool _warned = false;                              \
+      if (!_warned) {                                           \
+         fprintf(stderr, "WARNING: ");                          \
+         fprintf(stderr, fmt);                                  \
+         _warned = true;                                        \
+      }                                                         \
+   }                                                            \
+} while (0)
+
 #define FILE_DEBUG_FLAG DEBUG_BUFMGR
 
 static inline int
@@ -126,6 +138,16 @@ struct bo_cache_bucket {
    uint64_t size;
 };
 
+struct bo_export {
+   /** File descriptor associated with a handle export. */
+   int drm_fd;
+
+   /** GEM handle in drm_fd */
+   uint32_t gem_handle;
+
+   struct list_head link;
+};
+
 struct iris_bufmgr {
    /**
     * List into the list of bufmgr.
@@ -349,9 +371,13 @@ static struct iris_bo *
 bo_calloc(void)
 {
    struct iris_bo *bo = calloc(1, sizeof(*bo));
-   if (bo) {
-      bo->hash = _mesa_hash_pointer(bo);
-   }
+   if (!bo)
+      return NULL;
+
+   list_inithead(&bo->exports);
+
+   bo->hash = _mesa_hash_pointer(bo);
+
    return bo;
 }
 
@@ -705,6 +731,7 @@ iris_bo_gem_create_from_name(struct iris_bufmgr *bufmgr,
       goto err_unref;
 
    bo->tiling_mode = get_tiling.tiling_mode;
+
    /* XXX stride is unknown */
    DBG("bo_create_from_handle: %d (%s)\n", handle, bo->name);
 
@@ -733,6 +760,16 @@ bo_close(struct iris_bo *bo)
 
       entry = _mesa_hash_table_search(bufmgr->handle_table, &bo->gem_handle);
       _mesa_hash_table_remove(bufmgr->handle_table, entry);
+
+      list_for_each_entry_safe(struct bo_export, export, &bo->exports, link) {
+         struct drm_gem_close close = { .handle = export->gem_handle };
+         gen_ioctl(export->drm_fd, DRM_IOCTL_GEM_CLOSE, &close);
+
+         list_del(&export->link);
+         free(export);
+      }
+   } else {
+      assert(list_is_empty(&bo->exports));
    }
 
    /* Close this object */
@@ -1477,6 +1514,69 @@ iris_bo_flink(struct iris_bo *bo, uint32_t *name)
    return 0;
 }
 
+int
+iris_bo_export_gem_handle_for_device(struct iris_bo *bo, int drm_fd,
+                                     uint32_t *out_handle)
+{
+   /* Only add the new GEM handle to the list of export if it belongs to a
+    * different GEM device. Otherwise we might close the same buffer multiple
+    * times.
+    */
+   struct iris_bufmgr *bufmgr = bo->bufmgr;
+   int ret = os_same_file_description(drm_fd, bufmgr->fd);
+   WARN_ONCE(ret < 0,
+             "Kernel has no file descriptor comparison support: %s\n",
+             strerror(errno));
+   if (ret == 0) {
+      *out_handle = iris_bo_export_gem_handle(bo);
+      return 0;
+   }
+
+   struct bo_export *export = calloc(1, sizeof(*export));
+   if (!export)
+      return -ENOMEM;
+
+   export->drm_fd = drm_fd;
+
+   int dmabuf_fd = -1;
+   int err = iris_bo_export_dmabuf(bo, &dmabuf_fd);
+   if (err) {
+      free(export);
+      return err;
+   }
+
+   mtx_lock(&bufmgr->lock);
+   err = drmPrimeFDToHandle(drm_fd, dmabuf_fd, &export->gem_handle);
+   close(dmabuf_fd);
+   if (err) {
+      mtx_unlock(&bufmgr->lock);
+      free(export);
+      return err;
+   }
+
+   bool found = false;
+   list_for_each_entry(struct bo_export, iter, &bo->exports, link) {
+      if (iter->drm_fd != drm_fd)
+         continue;
+      /* Here we assume that for a given DRM fd, we'll always get back the
+       * same GEM handle for a given buffer.
+       */
+      assert(iter->gem_handle == export->gem_handle);
+      free(export);
+      export = iter;
+      found = true;
+      break;
+   }
+   if (!found)
+      list_addtail(&export->link, &bo->exports);
+
+   mtx_unlock(&bufmgr->lock);
+
+   *out_handle = export->gem_handle;
+
+   return 0;
+}
+
 static void
 add_bucket(struct iris_bufmgr *bufmgr, int size)
 {

src/gallium/drivers/iris/iris_bufmgr.h

@@ -28,6 +28,7 @@
 #include <stdint.h>
 #include <stdio.h>
 #include <sys/types.h>
+#include "c11/threads.h"
 #include "util/macros.h"
 #include "util/u_atomic.h"
 #include "util/list.h"
@@ -193,6 +194,9 @@ struct iris_bo {
    /** BO cache list */
    struct list_head head;
 
+   /** List of GEM handle exports of this buffer (bo_export) */
+   struct list_head exports;
+
    /**
     * Synchronization sequence number of most recent access of this BO from
     * each caching domain.
@@ -391,6 +395,18 @@ int iris_bo_export_dmabuf(struct iris_bo *bo, int *prime_fd);
 struct iris_bo *iris_bo_import_dmabuf(struct iris_bufmgr *bufmgr, int prime_fd,
                                       uint32_t tiling, uint32_t stride);
 
+/**
+ * Exports a bo as a GEM handle into a given DRM file descriptor
+ * \param bo Buffer to export
+ * \param drm_fd File descriptor where the new handle is created
+ * \param out_handle Pointer to store the new handle
+ *
+ * Returns 0 if the buffer was successfully exported, a non zero error code
+ * otherwise.
+ */
+int iris_bo_export_gem_handle_for_device(struct iris_bo *bo, int drm_fd,
+                                         uint32_t *out_handle);
+
 uint32_t iris_bo_export_gem_handle(struct iris_bo *bo);
 
 int iris_reg_read(struct iris_bufmgr *bufmgr, uint32_t offset, uint64_t *out);

src/gallium/drivers/iris/iris_resource.c

@@ -1138,7 +1138,7 @@ iris_resource_disable_aux_on_first_query(struct pipe_resource *resource,
 }
 
 static bool
-iris_resource_get_param(struct pipe_screen *screen,
+iris_resource_get_param(struct pipe_screen *pscreen,
                         struct pipe_context *context,
                         struct pipe_resource *resource,
                         unsigned plane,
@@ -1147,6 +1147,7 @@ iris_resource_get_param(struct pipe_screen *screen,
                         unsigned handle_usage,
                         uint64_t *value)
 {
+   struct iris_screen *screen = (struct iris_screen *)pscreen;
    struct iris_resource *res = (struct iris_resource *)resource;
    bool mod_with_aux =
       res->mod_info && res->mod_info->aux_usage != ISL_AUX_USAGE_NONE;
@@ -1155,7 +1156,7 @@ iris_resource_get_param(struct pipe_screen *screen,
    unsigned handle;
 
    if (iris_resource_unfinished_aux_import(res))
-      iris_resource_finish_aux_import(screen, res);
+      iris_resource_finish_aux_import(pscreen, res);
 
    struct iris_bo *bo = wants_aux ? res->aux.bo : res->bo;
 
@@ -1187,9 +1188,19 @@ iris_resource_get_param(struct pipe_screen *screen,
       if (result)
          *value = handle;
       return result;
-   case PIPE_RESOURCE_PARAM_HANDLE_TYPE_KMS:
-      *value = iris_bo_export_gem_handle(bo);
+   case PIPE_RESOURCE_PARAM_HANDLE_TYPE_KMS: {
+      /* Because we share the same drm file across multiple iris_screen, when
+       * we export a GEM handle we must make sure it is valid in the DRM file
+       * descriptor the caller is using (this is the FD given at screen
+       * creation).
+       */
+      uint32_t handle;
+      if (iris_bo_export_gem_handle_for_device(bo, screen->winsys_fd, &handle))
+         return false;
+      *value = handle;
       return true;
+   }
+
    case PIPE_RESOURCE_PARAM_HANDLE_TYPE_FD:
       result = iris_bo_export_dmabuf(bo, (int *) &handle) == 0;
       if (result)
@@ -1207,6 +1218,7 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
                          struct winsys_handle *whandle,
                          unsigned usage)
 {
+   struct iris_screen *screen = (struct iris_screen *) pscreen;
    struct iris_resource *res = (struct iris_resource *)resource;
    bool mod_with_aux =
       res->mod_info && res->mod_info->aux_usage != ISL_AUX_USAGE_NONE;
@@ -1244,9 +1256,18 @@ iris_resource_get_handle(struct pipe_screen *pscreen,
    switch (whandle->type) {
    case WINSYS_HANDLE_TYPE_SHARED:
       return iris_bo_flink(bo, &whandle->handle) == 0;
-   case WINSYS_HANDLE_TYPE_KMS:
-      whandle->handle = iris_bo_export_gem_handle(bo);
+   case WINSYS_HANDLE_TYPE_KMS: {
+      /* Because we share the same drm file across multiple iris_screen, when
+       * we export a GEM handle we must make sure it is valid in the DRM file
+       * descriptor the caller is using (this is the FD given at screen
+       * creation).
+       */
+      uint32_t handle;
+      if (iris_bo_export_gem_handle_for_device(bo, screen->winsys_fd, &handle))
+         return false;
+      whandle->handle = handle;
       return true;
+   }
    case WINSYS_HANDLE_TYPE_FD:
       return iris_bo_export_dmabuf(bo, (int *) &whandle->handle) == 0;
    }

src/gallium/drivers/iris/iris_screen.c

@@ -527,6 +527,7 @@ iris_screen_destroy(struct iris_screen *screen)
    u_transfer_helper_destroy(screen->base.transfer_helper);
    iris_bufmgr_unref(screen->bufmgr);
    disk_cache_destroy(screen->disk_cache);
+   close(screen->winsys_fd);
    ralloc_free(screen);
 }
 
@@ -706,6 +707,7 @@ iris_screen_create(int fd, const struct pipe_screen_config *config)
       return NULL;
 
    screen->fd = iris_bufmgr_get_fd(screen->bufmgr);
+   screen->winsys_fd = fd;
 
    if (getenv("INTEL_NO_HW") != NULL)
       screen->no_hw = true;

src/gallium/drivers/iris/iris_screen.h

@@ -150,9 +150,15 @@ struct iris_screen {
    /** Global slab allocator for iris_transfer_map objects */
    struct slab_parent_pool transfer_pool;
 
-   /** drm device file descriptor, on shared with bufmgr, do not close. */
+   /** drm device file descriptor, shared with bufmgr, do not close. */
    int fd;
 
+   /**
+    * drm device file descriptor to used for window system integration, owned
+    * by iris_screen, can be a different DRM instance than fd.
+    */
+   int winsys_fd;
+
    /** PCI ID for our GPU device */
    int pci_id;