[RFC PATCH] mesa: Export BOs in RW mode

Boris Brezillon boris.brezillon at collabora.com
Wed Jul 3 13:33:57 UTC 2019


Exported BOs might be imported back, then mmap()-ed to be written
too. Most drivers handle that by mmap()-ing the GEM handle after it's
been imported, but, according to [1], this is illegal. The panfrost
driver has recently switched to this generic helper (which was renamed
into drm_gem_map_offset() in the meantime) [2], and mmap()-ing
of imported BOs now fails.

Now I'm wondering how this should be solved. I guess the first question
is, is mmap()-ing of imported BOs really forbidden for all BOs? I guess
calling mmap() on a buffer that's been exported by the DRM driver itself
then re-imported shouldn't hurt, so maybe we can check that before
failing.

Now, if we really want to forbid mmap() on imported BOs, that means we
need a solution to mmap() the dmabuf object directly, and sometimes this
mapping will request RW permissions. The problem is, all function
exporting BOs in mesa are exporting them in RO-mode (resulting FD is
O_READ), thus preventing mmap()s in RW mode.

This patch modifies all
drmPrimeHandleToFD()/ioctl(DRM_IOCTL_PRIME_HANDLE_TO_FD) call sites
to pass the DRM_RDWR flag so that what's described above becomes
possible.

I'm not saying this is what we should do, it's more a way to start the
discussion. Feel free to propose alternives to this solution.

[1]https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/drm_gem.c?h=v5.2-rc7#n318
[2]https://cgit.freedesktop.org/drm/drm-misc/commit/drivers/gpu/drm/panfrost?id=583bbf46133c726bae277e8f4e32bfba2a528c7f

Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
Cc: <dri-devel at lists.freedesktop.org>
Cc: <mesa-dev at lists.freedesktop.org>
Cc: Steven Price <steven.price at arm.com>
Cc: Rob Herring <robh at kernel.org>
---
Cc-ing dri-devel is not a mistake, I really to have feedback from the DRM
maintainers, since this started with a kernel-side change.
---
 src/etnaviv/drm/etnaviv_bo.c                      | 4 ++--
 src/freedreno/drm/freedreno_bo.c                  | 4 ++--
 src/freedreno/vulkan/tu_drm.c                     | 2 +-
 src/gallium/auxiliary/renderonly/renderonly.c     | 2 +-
 src/gallium/drivers/iris/iris_bufmgr.c            | 2 +-
 src/gallium/drivers/lima/lima_bo.c                | 2 +-
 src/gallium/drivers/panfrost/pan_drm.c            | 2 +-
 src/gallium/drivers/v3d/v3d_bufmgr.c              | 2 +-
 src/gallium/drivers/vc4/vc4_bufmgr.c              | 2 +-
 src/gallium/winsys/radeon/drm/radeon_drm_bo.c     | 3 ++-
 src/gallium/winsys/svga/drm/vmw_screen_dri.c      | 5 +++--
 src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c | 2 +-
 src/gallium/winsys/virgl/drm/virgl_drm_winsys.c   | 3 ++-
 src/intel/vulkan/anv_gem.c                        | 2 +-
 src/mesa/drivers/dri/i965/brw_bufmgr.c            | 2 +-
 15 files changed, 21 insertions(+), 18 deletions(-)

diff --git a/src/etnaviv/drm/etnaviv_bo.c b/src/etnaviv/drm/etnaviv_bo.c
index 6e952fa47858..92634141b580 100644
--- a/src/etnaviv/drm/etnaviv_bo.c
+++ b/src/etnaviv/drm/etnaviv_bo.c
@@ -294,8 +294,8 @@ int etna_bo_dmabuf(struct etna_bo *bo)
 {
 	int ret, prime_fd;
 
-	ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
-				&prime_fd);
+	ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
+				 DRM_CLOEXEC | DRM_RDWR, &prime_fd);
 	if (ret) {
 		ERROR_MSG("failed to get dmabuf fd: %d", ret);
 		return ret;
diff --git a/src/freedreno/drm/freedreno_bo.c b/src/freedreno/drm/freedreno_bo.c
index 7449160f1371..ba19b08d7c54 100644
--- a/src/freedreno/drm/freedreno_bo.c
+++ b/src/freedreno/drm/freedreno_bo.c
@@ -318,8 +318,8 @@ int fd_bo_dmabuf(struct fd_bo *bo)
 {
 	int ret, prime_fd;
 
-	ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle, DRM_CLOEXEC,
-			&prime_fd);
+	ret = drmPrimeHandleToFD(bo->dev->fd, bo->handle,
+				 DRM_CLOEXEC | DRM_RDWR, &prime_fd);
 	if (ret) {
 		ERROR_MSG("failed to get dmabuf fd: %d", ret);
 		return ret;
diff --git a/src/freedreno/vulkan/tu_drm.c b/src/freedreno/vulkan/tu_drm.c
index 9b2e6f78879e..6bef3012ddb5 100644
--- a/src/freedreno/vulkan/tu_drm.c
+++ b/src/freedreno/vulkan/tu_drm.c
@@ -147,7 +147,7 @@ tu_gem_export_dmabuf(const struct tu_device *dev, uint32_t gem_handle)
 {
    int prime_fd;
    int ret = drmPrimeHandleToFD(dev->physical_device->local_fd, gem_handle,
-                                DRM_CLOEXEC, &prime_fd);
+                                DRM_CLOEXEC | DRM_RDWR, &prime_fd);
 
    return ret == 0 ? prime_fd : -1;
 }
diff --git a/src/gallium/auxiliary/renderonly/renderonly.c b/src/gallium/auxiliary/renderonly/renderonly.c
index d6a344009378..c1cc31115105 100644
--- a/src/gallium/auxiliary/renderonly/renderonly.c
+++ b/src/gallium/auxiliary/renderonly/renderonly.c
@@ -101,7 +101,7 @@ renderonly_create_kms_dumb_buffer_for_resource(struct pipe_resource *rsc,
    out_handle->type = WINSYS_HANDLE_TYPE_FD;
    out_handle->stride = create_dumb.pitch;
 
-   err = drmPrimeHandleToFD(ro->kms_fd, create_dumb.handle, O_CLOEXEC,
+   err = drmPrimeHandleToFD(ro->kms_fd, create_dumb.handle, O_CLOEXEC | O_RDWR,
          (int *)&out_handle->handle);
    if (err < 0) {
       fprintf(stderr, "failed to export dumb buffer: %s\n", strerror(errno));
diff --git a/src/gallium/drivers/iris/iris_bufmgr.c b/src/gallium/drivers/iris/iris_bufmgr.c
index d80945824098..68e5d8a8bb13 100644
--- a/src/gallium/drivers/iris/iris_bufmgr.c
+++ b/src/gallium/drivers/iris/iris_bufmgr.c
@@ -1369,7 +1369,7 @@ iris_bo_export_dmabuf(struct iris_bo *bo, int *prime_fd)
    iris_bo_make_external(bo);
 
    if (drmPrimeHandleToFD(bufmgr->fd, bo->gem_handle,
-                          DRM_CLOEXEC, prime_fd) != 0)
+                          DRM_CLOEXEC | DRM_RDWR, prime_fd) != 0)
       return -errno;
 
    bo->reusable = false;
diff --git a/src/gallium/drivers/lima/lima_bo.c b/src/gallium/drivers/lima/lima_bo.c
index 1d6dd720602a..df0114fc67ed 100644
--- a/src/gallium/drivers/lima/lima_bo.c
+++ b/src/gallium/drivers/lima/lima_bo.c
@@ -205,7 +205,7 @@ bool lima_bo_export(struct lima_bo *bo, struct winsys_handle *handle)
       return true;
 
    case WINSYS_HANDLE_TYPE_FD:
-      if (drmPrimeHandleToFD(screen->fd, bo->handle, DRM_CLOEXEC,
+      if (drmPrimeHandleToFD(screen->fd, bo->handle, DRM_CLOEXEC | DRM_RDWR,
                              (int*)&handle->handle))
          return false;
 
diff --git a/src/gallium/drivers/panfrost/pan_drm.c b/src/gallium/drivers/panfrost/pan_drm.c
index 8de4f483435c..a086575c453a 100644
--- a/src/gallium/drivers/panfrost/pan_drm.c
+++ b/src/gallium/drivers/panfrost/pan_drm.c
@@ -181,7 +181,7 @@ panfrost_drm_export_bo(struct panfrost_screen *screen, const struct panfrost_bo
 {
         struct drm_prime_handle args = {
                 .handle = bo->gem_handle,
-                .flags = DRM_CLOEXEC,
+                .flags = DRM_CLOEXEC | DRM_RDWR,
         };
 
         int ret = drmIoctl(screen->fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
diff --git a/src/gallium/drivers/v3d/v3d_bufmgr.c b/src/gallium/drivers/v3d/v3d_bufmgr.c
index f0df1b983737..0c78994d899f 100644
--- a/src/gallium/drivers/v3d/v3d_bufmgr.c
+++ b/src/gallium/drivers/v3d/v3d_bufmgr.c
@@ -424,7 +424,7 @@ v3d_bo_get_dmabuf(struct v3d_bo *bo)
 {
         int fd;
         int ret = drmPrimeHandleToFD(bo->screen->fd, bo->handle,
-                                     O_CLOEXEC, &fd);
+                                     DRM_CLOEXEC | DRM_RDWR, &fd);
         if (ret != 0) {
                 fprintf(stderr, "Failed to export gem bo %d to dmabuf\n",
                         bo->handle);
diff --git a/src/gallium/drivers/vc4/vc4_bufmgr.c b/src/gallium/drivers/vc4/vc4_bufmgr.c
index 716ca50ea069..c7e89b6a517e 100644
--- a/src/gallium/drivers/vc4/vc4_bufmgr.c
+++ b/src/gallium/drivers/vc4/vc4_bufmgr.c
@@ -462,7 +462,7 @@ vc4_bo_get_dmabuf(struct vc4_bo *bo)
 {
         int fd;
         int ret = drmPrimeHandleToFD(bo->screen->fd, bo->handle,
-                                     O_CLOEXEC, &fd);
+                                     DRM_CLOEXEC | DRM_RDWR, &fd);
         if (ret != 0) {
                 fprintf(stderr, "Failed to export gem bo %d to dmabuf\n",
                         bo->handle);
diff --git a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
index d1e2a8685ba9..ae7958a8b892 100644
--- a/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
+++ b/src/gallium/winsys/radeon/drm/radeon_drm_bo.c
@@ -1320,7 +1320,8 @@ static bool radeon_winsys_bo_get_handle(struct pb_buffer *buffer,
     } else if (whandle->type == WINSYS_HANDLE_TYPE_KMS) {
         whandle->handle = bo->handle;
     } else if (whandle->type == WINSYS_HANDLE_TYPE_FD) {
-        if (drmPrimeHandleToFD(ws->fd, bo->handle, DRM_CLOEXEC, (int*)&whandle->handle))
+        if (drmPrimeHandleToFD(ws->fd, bo->handle, DRM_CLOEXEC | DRM_RDWR,
+                               (int*)&whandle->handle))
             return false;
     }
 
diff --git a/src/gallium/winsys/svga/drm/vmw_screen_dri.c b/src/gallium/winsys/svga/drm/vmw_screen_dri.c
index a85ee18e45b9..24f6ddac0890 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen_dri.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen_dri.c
@@ -345,8 +345,9 @@ vmw_drm_surface_get_handle(struct svga_winsys_screen *sws,
        whandle->handle = vsrf->sid;
        break;
     case WINSYS_HANDLE_TYPE_FD:
-       ret = drmPrimeHandleToFD(vws->ioctl.drm_fd, vsrf->sid, DRM_CLOEXEC,
-				(int *)&whandle->handle);
+       ret = drmPrimeHandleToFD(vws->ioctl.drm_fd, vsrf->sid,
+                                DRM_CLOEXEC | DRM_RDWR,
+                                (int *)&whandle->handle);
        if (ret) {
 	  vmw_error("Failed to get file descriptor from prime.\n");
 	  return FALSE;
diff --git a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
index d9b417dc4dad..ce47e66ab1e5 100644
--- a/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
+++ b/src/gallium/winsys/sw/kms-dri/kms_dri_sw_winsys.c
@@ -446,7 +446,7 @@ kms_sw_displaytarget_get_handle(struct sw_winsys *winsys,
       return TRUE;
    case WINSYS_HANDLE_TYPE_FD:
       if (!drmPrimeHandleToFD(kms_sw->fd, kms_sw_dt->handle,
-                             DRM_CLOEXEC, (int*)&whandle->handle)) {
+                             DRM_CLOEXEC | DRM_RDWR, (int*)&whandle->handle)) {
          whandle->stride = plane->stride;
          whandle->offset = plane->offset;
          return TRUE;
diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
index 9eec92f57363..07fab2735e25 100644
--- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
+++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
@@ -418,7 +418,8 @@ static boolean virgl_drm_winsys_resource_get_handle(struct virgl_winsys *qws,
    } else if (whandle->type == WINSYS_HANDLE_TYPE_KMS) {
       whandle->handle = res->bo_handle;
    } else if (whandle->type == WINSYS_HANDLE_TYPE_FD) {
-      if (drmPrimeHandleToFD(qdws->fd, res->bo_handle, DRM_CLOEXEC, (int*)&whandle->handle))
+      if (drmPrimeHandleToFD(qdws->fd, res->bo_handle, DRM_CLOEXEC | DRM_RDWR,
+                             (int*)&whandle->handle))
             return FALSE;
       mtx_lock(&qdws->bo_handles_mutex);
       util_hash_table_set(qdws->bo_handles, (void *)(uintptr_t)res->bo_handle, res);
diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c
index 1bdf040c1a37..8f07928638ce 100644
--- a/src/intel/vulkan/anv_gem.c
+++ b/src/intel/vulkan/anv_gem.c
@@ -399,7 +399,7 @@ anv_gem_handle_to_fd(struct anv_device *device, uint32_t gem_handle)
 {
    struct drm_prime_handle args = {
       .handle = gem_handle,
-      .flags = DRM_CLOEXEC,
+      .flags = DRM_CLOEXEC | DRM_RDWR,
    };
 
    int ret = anv_ioctl(device->fd, DRM_IOCTL_PRIME_HANDLE_TO_FD, &args);
diff --git a/src/mesa/drivers/dri/i965/brw_bufmgr.c b/src/mesa/drivers/dri/i965/brw_bufmgr.c
index a7c684063158..ee639d2cf23a 100644
--- a/src/mesa/drivers/dri/i965/brw_bufmgr.c
+++ b/src/mesa/drivers/dri/i965/brw_bufmgr.c
@@ -1489,7 +1489,7 @@ brw_bo_gem_export_to_prime(struct brw_bo *bo, int *prime_fd)
    brw_bo_make_external(bo);
 
    if (drmPrimeHandleToFD(bufmgr->fd, bo->gem_handle,
-                          DRM_CLOEXEC, prime_fd) != 0)
+                          DRM_CLOEXEC | DRM_RDWR, prime_fd) != 0)
       return -errno;
 
    bo->reusable = false;
-- 
2.21.0



More information about the dri-devel mailing list