From patchwork Wed Jul 3 13:33:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Boris Brezillon X-Patchwork-Id: 11029643 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 38F7C14F6 for ; Wed, 3 Jul 2019 13:34:08 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 29D3226255 for ; Wed, 3 Jul 2019 13:34:08 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1DB4B2893D; Wed, 3 Jul 2019 13:34:08 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 3D72426255 for ; Wed, 3 Jul 2019 13:34:07 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id C7ED96E13A; Wed, 3 Jul 2019 13:34:04 +0000 (UTC) X-Original-To: dri-devel@lists.freedesktop.org Delivered-To: dri-devel@lists.freedesktop.org Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 5A2BD6E13A; Wed, 3 Jul 2019 13:34:03 +0000 (UTC) Received: from localhost.localdomain (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 7331728A8CE; Wed, 3 Jul 2019 14:34:01 +0100 (BST) From: Boris Brezillon To: Alyssa Rosenzweig , Tomeu Vizoso , Rob Herring Subject: [RFC PATCH] mesa: Export BOs in RW mode Date: Wed, 3 Jul 2019 15:33:57 +0200 Message-Id: <20190703133357.10217-1-boris.brezillon@collabora.com> X-Mailer: git-send-email 2.21.0 MIME-Version: 1.0 X-BeenThere: dri-devel@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Direct Rendering Infrastructure - Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: mesa-dev@lists.freedesktop.org, Boris Brezillon , dri-devel@lists.freedesktop.org, Steven Price Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" X-Virus-Scanned: ClamAV using ClamSMTP 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 Cc: Cc: Cc: Steven Price Cc: Rob Herring --- 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;