[PATCH v2 0/7] drm: Clean up mmap for TTM-based GEM drivers

[PATCH v2 0/7] drm: Clean up mmap for TTM-based GEM drivers

[Thomas Zimmermann]

Implement mmap via struct drm_gem_object_functions.mmap in amdgpu,
radeon and nouveau. This allows for using common DRM helpers for
the mmap-related callbacks in struct file_operations and struct
drm_driver. The drivers have their own vm_ops, which are now set
automatically by the DRM core functions. The code in each driver's
verify_access becomes part of the driver's new mmap implementation.

With the GEM drivers converted, vmwgfx is the only user of
ttm_bo_mmap() and related infrastructure. So move everything into
vmwgfx and delete the rsp code from TTM.

This touches several drivers. Preferably everything would be merged
at once via drm-misc-next.

v2:
	* removal of amdgpu fbdev mmap already merged (Christian)
	* rebase on top of amdgpu fixes [1] (Felix)
	* replace pr_err() with drm_err() in vmwgfx patch (Zack)
	* several typos

[1] https://patchwork.freedesktop.org/series/88822/

Thomas Zimmermann (7):
  drm/ttm: Don't override vm_ops callbacks, if set
  drm/amdgpu: Implement mmap as GEM object function
  drm/radeon: Implement mmap as GEM object function
  drm/nouveau: Implement mmap as GEM object function
  drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver
  drm/vmwgfx: Inline vmw_verify_access()
  drm/ttm: Remove ttm_bo_mmap() and friends

 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 ---------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c     | 56 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 64 --------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 -
 drivers/gpu/drm/nouveau/nouveau_bo.c        | 10 ----
 drivers/gpu/drm/nouveau/nouveau_drm.c       |  3 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c       | 36 ++++++++++++
 drivers/gpu/drm/nouveau/nouveau_ttm.c       | 49 ----------------
 drivers/gpu/drm/nouveau/nouveau_ttm.h       |  1 -
 drivers/gpu/drm/radeon/radeon_drv.c         |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c         | 52 +++++++++++++++++
 drivers/gpu/drm/radeon/radeon_ttm.c         | 65 ---------------------
 drivers/gpu/drm/radeon/radeon_ttm.h         |  1 -
 drivers/gpu/drm/ttm/ttm_bo_vm.c             | 60 ++-----------------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c  |  9 ---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    | 53 ++++++++++++++++-
 include/drm/ttm/ttm_bo_api.h                | 13 -----
 include/drm/ttm/ttm_device.h                | 15 -----
 20 files changed, 206 insertions(+), 337 deletions(-)

--
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 1/7] drm/ttm: Don't override vm_ops callbacks, if set

[Thomas Zimmermann]

Drivers may want to set their own callbacks for a VM area. Only set
TTM's callbacks if the vm_ops field is clear.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index b31b18058965..bf4a213bc66c 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -534,7 +534,12 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
 
 static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma)
 {
-	vma->vm_ops = &ttm_bo_vm_ops;
+	/*
+	 * Drivers may want to override the vm_ops field. Otherwise we
+	 * use TTM's default callbacks.
+	 */
+	if (!vma->vm_ops)
+		vma->vm_ops = &ttm_bo_vm_ops;
 
 	/*
 	 * Note: We're transferring the bo reference to
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 2/7] drm/amdgpu: Implement mmap as GEM object function

[Thomas Zimmermann]

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change resolves several inconsistencies between regular mmap and
prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
areas. Previously it way only set for regular mmap calls, prime-based
mmap used TTM's default vm_ops. The function amdgpu_verify_access() is
no longer being called and therefore removed by this patch.

As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
implemented in amdgpu's GEM code.

v2:
	* rename amdgpu_ttm_vm_ops and amdgpu_ttm_fault() to
	  amdgpu_gem_vm_ops and amdgpu_gem_fault() (Christian)
	* the check for kfd_bo has meanwhile been removed

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 46 ---------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h |  2 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     |  4 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c     | 56 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 64 ---------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h     |  1 -
 6 files changed, 58 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index e0c4f7c7f1b9..19c5ab08d9ec 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -42,52 +42,6 @@
 #include <linux/pci-p2pdma.h>
 #include <linux/pm_runtime.h>
 
-/**
- * amdgpu_gem_prime_mmap - &drm_driver.gem_prime_mmap implementation
- * @obj: GEM BO
- * @vma: Virtual memory area
- *
- * Sets up a userspace mapping of the BO's memory in the given
- * virtual memory area.
- *
- * Returns:
- * 0 on success or a negative error code on failure.
- */
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
-			  struct vm_area_struct *vma)
-{
-	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
-	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
-	unsigned asize = amdgpu_bo_size(bo);
-	int ret;
-
-	if (!vma->vm_file)
-		return -ENODEV;
-
-	if (adev == NULL)
-		return -ENODEV;
-
-	/* Check for valid size. */
-	if (asize < vma->vm_end - vma->vm_start)
-		return -EINVAL;
-
-	if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
-	    (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
-		return -EPERM;
-	}
-	vma->vm_pgoff += amdgpu_bo_mmap_offset(bo) >> PAGE_SHIFT;
-
-	/* prime mmap does not need to check access, so allow here */
-	ret = drm_vma_node_allow(&obj->vma_node, vma->vm_file->private_data);
-	if (ret)
-		return ret;
-
-	ret = ttm_bo_mmap(vma->vm_file, vma, &adev->mman.bdev);
-	drm_vma_node_revoke(&obj->vma_node, vma->vm_file->private_data);
-
-	return ret;
-}
-
 static int
 __dma_resv_make_exclusive(struct dma_resv *obj)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
index 39b5b9616fd8..3e93b9b407a9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.h
@@ -31,8 +31,6 @@ struct drm_gem_object *amdgpu_gem_prime_import(struct drm_device *dev,
 					    struct dma_buf *dma_buf);
 bool amdgpu_dmabuf_is_xgmi_accessible(struct amdgpu_device *adev,
 				      struct amdgpu_bo *bo);
-int amdgpu_gem_prime_mmap(struct drm_gem_object *obj,
-			  struct vm_area_struct *vma);
 
 extern const struct dma_buf_ops amdgpu_dmabuf_ops;
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 80130c1c0c68..69d2485ec665 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -1686,7 +1686,7 @@ static const struct file_operations amdgpu_driver_kms_fops = {
 	.flush = amdgpu_flush,
 	.release = drm_release,
 	.unlocked_ioctl = amdgpu_drm_ioctl,
-	.mmap = amdgpu_mmap,
+	.mmap = drm_gem_mmap,
 	.poll = drm_poll,
 	.read = drm_read,
 #ifdef CONFIG_COMPAT
@@ -1749,7 +1749,7 @@ static const struct drm_driver amdgpu_kms_driver = {
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import = amdgpu_gem_prime_import,
-	.gem_prime_mmap = amdgpu_gem_prime_mmap,
+	.gem_prime_mmap = drm_gem_prime_mmap,
 
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
index 311bcdc59eda..b85a0018a570 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
@@ -41,6 +41,36 @@
 
 static const struct drm_gem_object_funcs amdgpu_gem_object_funcs;
 
+static vm_fault_t amdgpu_gem_fault(struct vm_fault *vmf)
+{
+	struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+	vm_fault_t ret;
+
+	ret = ttm_bo_vm_reserve(bo, vmf);
+	if (ret)
+		return ret;
+
+	ret = amdgpu_bo_fault_reserve_notify(bo);
+	if (ret)
+		goto unlock;
+
+	ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+				       TTM_BO_VM_NUM_PREFAULT, 1);
+	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+		return ret;
+
+unlock:
+	dma_resv_unlock(bo->base.resv);
+	return ret;
+}
+
+static const struct vm_operations_struct amdgpu_gem_vm_ops = {
+	.fault = amdgpu_gem_fault,
+	.open = ttm_bo_vm_open,
+	.close = ttm_bo_vm_close,
+	.access = ttm_bo_vm_access
+};
+
 static void amdgpu_gem_object_free(struct drm_gem_object *gobj)
 {
 	struct amdgpu_bo *robj = gem_to_amdgpu_bo(gobj);
@@ -205,6 +235,30 @@ static void amdgpu_gem_object_close(struct drm_gem_object *obj,
 	ttm_eu_backoff_reservation(&ticket, &list);
 }
 
+static int amdgpu_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
+{
+	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+	unsigned long asize = amdgpu_bo_size(bo);
+
+	if (!vma->vm_file)
+		return -ENODEV;
+
+	if (!adev)
+		return -ENODEV;
+
+	/* Check for valid size. */
+	if (asize < vma->vm_end - vma->vm_start)
+		return -EINVAL;
+
+	if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
+	    (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
+		return -EPERM;
+	}
+
+	return drm_gem_ttm_mmap(obj, vma);
+}
+
 static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
 	.free = amdgpu_gem_object_free,
 	.open = amdgpu_gem_object_open,
@@ -212,6 +266,8 @@ static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
 	.export = amdgpu_gem_prime_export,
 	.vmap = drm_gem_ttm_vmap,
 	.vunmap = drm_gem_ttm_vunmap,
+	.mmap = amdgpu_gem_prime_mmap,
+	.vm_ops = &amdgpu_gem_vm_ops,
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 1485f33c3cc7..d4083c19402b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -152,25 +152,6 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
 	*placement = abo->placement;
 }
 
-/**
- * amdgpu_verify_access - Verify access for a mmap call
- *
- * @bo:	The buffer object to map
- * @filp: The file pointer from the process performing the mmap
- *
- * This is called by ttm_bo_mmap() to verify whether a process
- * has the right to mmap a BO to their process space.
- */
-static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
-
-	if (amdgpu_ttm_tt_get_usermm(bo->ttm))
-		return -EPERM;
-	return drm_vma_node_verify_access(&abo->tbo.base.vma_node,
-					  filp->private_data);
-}
-
 /**
  * amdgpu_ttm_map_buffer - Map memory into the GART windows
  * @bo: buffer object to map
@@ -1522,7 +1503,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = {
 	.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
 	.evict_flags = &amdgpu_evict_flags,
 	.move = &amdgpu_bo_move,
-	.verify_access = &amdgpu_verify_access,
 	.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
 	.release_notify = &amdgpu_bo_release_notify,
 	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
@@ -1897,50 +1877,6 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
 	adev->mman.buffer_funcs_enabled = enable;
 }
 
-static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
-{
-	struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
-	vm_fault_t ret;
-
-	ret = ttm_bo_vm_reserve(bo, vmf);
-	if (ret)
-		return ret;
-
-	ret = amdgpu_bo_fault_reserve_notify(bo);
-	if (ret)
-		goto unlock;
-
-	ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
-				       TTM_BO_VM_NUM_PREFAULT, 1);
-	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
-		return ret;
-
-unlock:
-	dma_resv_unlock(bo->base.resv);
-	return ret;
-}
-
-static const struct vm_operations_struct amdgpu_ttm_vm_ops = {
-	.fault = amdgpu_ttm_fault,
-	.open = ttm_bo_vm_open,
-	.close = ttm_bo_vm_close,
-	.access = ttm_bo_vm_access
-};
-
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-	struct drm_file *file_priv = filp->private_data;
-	struct amdgpu_device *adev = drm_to_adev(file_priv->minor->dev);
-	int r;
-
-	r = ttm_bo_mmap(filp, vma, &adev->mman.bdev);
-	if (unlikely(r != 0))
-		return r;
-
-	vma->vm_ops = &amdgpu_ttm_vm_ops;
-	return 0;
-}
-
 int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
 		       uint64_t dst_offset, uint32_t byte_count,
 		       struct dma_resv *resv,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
index dec0db8b0b13..6e51faad7371 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
@@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
 			struct dma_resv *resv,
 			struct dma_fence **fence);
 
-int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
 int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
 int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
 uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type);
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 3/7] drm/radeon: Implement mmap as GEM object function

[Thomas Zimmermann]

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

This change also allows to support prime-based mmap via DRM's helper
drm_gem_prime_mmap().

Permission checks are implemented by drm_gem_mmap(), with an additional
check for radeon_ttm_tt_has_userptr() in the GEM object function. The
function radeon_verify_access() is now unused and has thus been removed.

As a side effect, radeon_ttm_vm_ops and radeon_ttm_fault() are now
implemented in amdgpu's GEM code.

v2:
	* rename radeon_ttm_vm_ops and radeon_ttm_fault() to
	  radeon_gem_vm_ops and radeon_gem_fault() (Christian)
	* fix commit description (Alex)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/radeon/radeon_drv.c |  3 +-
 drivers/gpu/drm/radeon/radeon_gem.c | 52 +++++++++++++++++++++++
 drivers/gpu/drm/radeon/radeon_ttm.c | 65 -----------------------------
 drivers/gpu/drm/radeon/radeon_ttm.h |  1 -
 4 files changed, 54 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 8885e849717d..61793ffdfa1b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -558,7 +558,7 @@ static const struct file_operations radeon_driver_kms_fops = {
 	.open = drm_open,
 	.release = drm_release,
 	.unlocked_ioctl = radeon_drm_ioctl,
-	.mmap = radeon_mmap,
+	.mmap = drm_gem_mmap,
 	.poll = drm_poll,
 	.read = drm_read,
 #ifdef CONFIG_COMPAT
@@ -633,6 +633,7 @@ static const struct drm_driver kms_driver = {
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = radeon_gem_prime_import_sg_table,
+	.gem_prime_mmap = drm_gem_prime_mmap,
 
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 05ea2f39f626..e50287649909 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -44,6 +44,42 @@ void radeon_gem_prime_unpin(struct drm_gem_object *obj);
 
 const struct drm_gem_object_funcs radeon_gem_object_funcs;
 
+static vm_fault_t radeon_gem_fault(struct vm_fault *vmf)
+{
+	struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
+	struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
+	vm_fault_t ret;
+
+	down_read(&rdev->pm.mclk_lock);
+
+	ret = ttm_bo_vm_reserve(bo, vmf);
+	if (ret)
+		goto unlock_mclk;
+
+	ret = radeon_bo_fault_reserve_notify(bo);
+	if (ret)
+		goto unlock_resv;
+
+	ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
+				       TTM_BO_VM_NUM_PREFAULT, 1);
+	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+		goto unlock_mclk;
+
+unlock_resv:
+	dma_resv_unlock(bo->base.resv);
+
+unlock_mclk:
+	up_read(&rdev->pm.mclk_lock);
+	return ret;
+}
+
+static const struct vm_operations_struct radeon_gem_vm_ops = {
+	.fault = radeon_gem_fault,
+	.open = ttm_bo_vm_open,
+	.close = ttm_bo_vm_close,
+	.access = ttm_bo_vm_access
+};
+
 static void radeon_gem_object_free(struct drm_gem_object *gobj)
 {
 	struct radeon_bo *robj = gem_to_radeon_bo(gobj);
@@ -226,6 +262,20 @@ static int radeon_gem_handle_lockup(struct radeon_device *rdev, int r)
 	return r;
 }
 
+static int radeon_gem_object_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
+{
+	struct radeon_bo *bo = gem_to_radeon_bo(obj);
+	struct radeon_device *rdev = radeon_get_rdev(bo->tbo.bdev);
+
+	if (!rdev)
+		return -EINVAL;
+
+	if (radeon_ttm_tt_has_userptr(rdev, bo->tbo.ttm))
+		return -EPERM;
+
+	return drm_gem_ttm_mmap(obj, vma);
+}
+
 const struct drm_gem_object_funcs radeon_gem_object_funcs = {
 	.free = radeon_gem_object_free,
 	.open = radeon_gem_object_open,
@@ -236,6 +286,8 @@ const struct drm_gem_object_funcs radeon_gem_object_funcs = {
 	.get_sg_table = radeon_gem_prime_get_sg_table,
 	.vmap = drm_gem_ttm_vmap,
 	.vunmap = drm_gem_ttm_vunmap,
+	.mmap = radeon_gem_object_mmap,
+	.vm_ops = &radeon_gem_vm_ops,
 };
 
 /*
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 380b3007fd0b..8b6efcc4191c 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -136,17 +136,6 @@ static void radeon_evict_flags(struct ttm_buffer_object *bo,
 	*placement = rbo->placement;
 }
 
-static int radeon_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-	struct radeon_bo *rbo = container_of(bo, struct radeon_bo, tbo);
-	struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-
-	if (radeon_ttm_tt_has_userptr(rdev, bo->ttm))
-		return -EPERM;
-	return drm_vma_node_verify_access(&rbo->tbo.base.vma_node,
-					  filp->private_data);
-}
-
 static int radeon_move_blit(struct ttm_buffer_object *bo,
 			bool evict,
 			struct ttm_resource *new_mem,
@@ -705,7 +694,6 @@ static struct ttm_device_funcs radeon_bo_driver = {
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = &radeon_evict_flags,
 	.move = &radeon_bo_move,
-	.verify_access = &radeon_verify_access,
 	.delete_mem_notify = &radeon_bo_delete_mem_notify,
 	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
 };
@@ -802,59 +790,6 @@ void radeon_ttm_set_active_vram_size(struct radeon_device *rdev, u64 size)
 	man->size = size >> PAGE_SHIFT;
 }
 
-static vm_fault_t radeon_ttm_fault(struct vm_fault *vmf)
-{
-	struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
-	struct radeon_device *rdev = radeon_get_rdev(bo->bdev);
-	vm_fault_t ret;
-
-	down_read(&rdev->pm.mclk_lock);
-
-	ret = ttm_bo_vm_reserve(bo, vmf);
-	if (ret)
-		goto unlock_mclk;
-
-	ret = radeon_bo_fault_reserve_notify(bo);
-	if (ret)
-		goto unlock_resv;
-
-	ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
-				       TTM_BO_VM_NUM_PREFAULT, 1);
-	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
-		goto unlock_mclk;
-
-unlock_resv:
-	dma_resv_unlock(bo->base.resv);
-
-unlock_mclk:
-	up_read(&rdev->pm.mclk_lock);
-	return ret;
-}
-
-static const struct vm_operations_struct radeon_ttm_vm_ops = {
-	.fault = radeon_ttm_fault,
-	.open = ttm_bo_vm_open,
-	.close = ttm_bo_vm_close,
-	.access = ttm_bo_vm_access
-};
-
-int radeon_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-	int r;
-	struct drm_file *file_priv = filp->private_data;
-	struct radeon_device *rdev = file_priv->minor->dev->dev_private;
-
-	if (rdev == NULL)
-		return -EINVAL;
-
-	r = ttm_bo_mmap(filp, vma, &rdev->mman.bdev);
-	if (unlikely(r != 0))
-		return r;
-
-	vma->vm_ops = &radeon_ttm_vm_ops;
-	return 0;
-}
-
 #if defined(CONFIG_DEBUG_FS)
 
 static int radeon_mm_vram_dump_table_show(struct seq_file *m, void *unused)
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.h b/drivers/gpu/drm/radeon/radeon_ttm.h
index 4d7b90ee2774..91ea7141bc81 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.h
+++ b/drivers/gpu/drm/radeon/radeon_ttm.h
@@ -32,6 +32,5 @@ struct radeon_device;
 
 int radeon_ttm_init(struct radeon_device *rdev);
 void radeon_ttm_fini(struct radeon_device *rdev);
-int radeon_mmap(struct file *filp, struct vm_area_struct *vma);
 
 #endif				/* __RADEON_TTM_H__ */
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 4/7] drm/nouveau: Implement mmap as GEM object function

[Thomas Zimmermann]

Moving the driver-specific mmap code into a GEM object function allows
for using DRM helpers for various mmap callbacks.

The GEM object function is provided by GEM TTM helpers. Nouveau's
implementation of verify_access is unused and has been removed. Access
permissions are validated by the DRM helpers.

As a side effect, nouveau_ttm_vm_ops and nouveau_ttm_fault() are now
implemented in nouveau's GEM code.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/gpu/drm/nouveau/nouveau_bo.c  | 10 ------
 drivers/gpu/drm/nouveau/nouveau_drm.c |  3 +-
 drivers/gpu/drm/nouveau/nouveau_gem.c | 36 ++++++++++++++++++++
 drivers/gpu/drm/nouveau/nouveau_ttm.c | 49 ---------------------------
 drivers/gpu/drm/nouveau/nouveau_ttm.h |  1 -
 5 files changed, 38 insertions(+), 61 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 3e09df0472ce..bc67cbccc83b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1051,15 +1051,6 @@ nouveau_bo_move(struct ttm_buffer_object *bo, bool evict,
 	return ret;
 }
 
-static int
-nouveau_bo_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-	struct nouveau_bo *nvbo = nouveau_bo(bo);
-
-	return drm_vma_node_verify_access(&nvbo->bo.base.vma_node,
-					  filp->private_data);
-}
-
 static void
 nouveau_ttm_io_mem_free_locked(struct nouveau_drm *drm,
 			       struct ttm_resource *reg)
@@ -1332,7 +1323,6 @@ struct ttm_device_funcs nouveau_bo_driver = {
 	.evict_flags = nouveau_bo_evict_flags,
 	.delete_mem_notify = nouveau_bo_delete_mem_notify,
 	.move = nouveau_bo_move,
-	.verify_access = nouveau_bo_verify_access,
 	.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
 	.io_mem_free = &nouveau_ttm_io_mem_free,
 };
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 3204fc0a90d2..a616cf4573b8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1179,7 +1179,7 @@ nouveau_driver_fops = {
 	.open = drm_open,
 	.release = drm_release,
 	.unlocked_ioctl = nouveau_drm_ioctl,
-	.mmap = nouveau_ttm_mmap,
+	.mmap = drm_gem_mmap,
 	.poll = drm_poll,
 	.read = drm_read,
 #if defined(CONFIG_COMPAT)
@@ -1212,6 +1212,7 @@ driver_stub = {
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
 	.gem_prime_import_sg_table = nouveau_gem_prime_import_sg_table,
+	.gem_prime_mmap = drm_gem_prime_mmap,
 
 	.dumb_create = nouveau_display_dumb_create,
 	.dumb_map_offset = drm_gem_ttm_dumb_map_offset,
diff --git a/drivers/gpu/drm/nouveau/nouveau_gem.c b/drivers/gpu/drm/nouveau/nouveau_gem.c
index c88cbb85f101..71dfac820c4d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_gem.c
+++ b/drivers/gpu/drm/nouveau/nouveau_gem.c
@@ -39,6 +39,40 @@
 #include <nvif/class.h>
 #include <nvif/push206e.h>
 
+static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
+{
+	struct vm_area_struct *vma = vmf->vma;
+	struct ttm_buffer_object *bo = vma->vm_private_data;
+	pgprot_t prot;
+	vm_fault_t ret;
+
+	ret = ttm_bo_vm_reserve(bo, vmf);
+	if (ret)
+		return ret;
+
+	ret = nouveau_ttm_fault_reserve_notify(bo);
+	if (ret)
+		goto error_unlock;
+
+	nouveau_bo_del_io_reserve_lru(bo);
+	prot = vm_get_page_prot(vma->vm_flags);
+	ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
+	nouveau_bo_add_io_reserve_lru(bo);
+	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
+		return ret;
+
+error_unlock:
+	dma_resv_unlock(bo->base.resv);
+	return ret;
+}
+
+static const struct vm_operations_struct nouveau_ttm_vm_ops = {
+	.fault = nouveau_ttm_fault,
+	.open = ttm_bo_vm_open,
+	.close = ttm_bo_vm_close,
+	.access = ttm_bo_vm_access
+};
+
 void
 nouveau_gem_object_del(struct drm_gem_object *gem)
 {
@@ -180,6 +214,8 @@ const struct drm_gem_object_funcs nouveau_gem_object_funcs = {
 	.get_sg_table = nouveau_gem_prime_get_sg_table,
 	.vmap = drm_gem_ttm_vmap,
 	.vunmap = drm_gem_ttm_vunmap,
+	.mmap = drm_gem_ttm_mmap,
+	.vm_ops = &nouveau_ttm_vm_ops,
 };
 
 int
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index b81ae90b8449..e511a26379da 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -127,55 +127,6 @@ const struct ttm_resource_manager_func nv04_gart_manager = {
 	.free = nouveau_manager_del,
 };
 
-static vm_fault_t nouveau_ttm_fault(struct vm_fault *vmf)
-{
-	struct vm_area_struct *vma = vmf->vma;
-	struct ttm_buffer_object *bo = vma->vm_private_data;
-	pgprot_t prot;
-	vm_fault_t ret;
-
-	ret = ttm_bo_vm_reserve(bo, vmf);
-	if (ret)
-		return ret;
-
-	ret = nouveau_ttm_fault_reserve_notify(bo);
-	if (ret)
-		goto error_unlock;
-
-	nouveau_bo_del_io_reserve_lru(bo);
-	prot = vm_get_page_prot(vma->vm_flags);
-	ret = ttm_bo_vm_fault_reserved(vmf, prot, TTM_BO_VM_NUM_PREFAULT, 1);
-	nouveau_bo_add_io_reserve_lru(bo);
-	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
-		return ret;
-
-error_unlock:
-	dma_resv_unlock(bo->base.resv);
-	return ret;
-}
-
-static const struct vm_operations_struct nouveau_ttm_vm_ops = {
-	.fault = nouveau_ttm_fault,
-	.open = ttm_bo_vm_open,
-	.close = ttm_bo_vm_close,
-	.access = ttm_bo_vm_access
-};
-
-int
-nouveau_ttm_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-	struct drm_file *file_priv = filp->private_data;
-	struct nouveau_drm *drm = nouveau_drm(file_priv->minor->dev);
-	int ret;
-
-	ret = ttm_bo_mmap(filp, vma, &drm->ttm.bdev);
-	if (ret)
-		return ret;
-
-	vma->vm_ops = &nouveau_ttm_vm_ops;
-	return 0;
-}
-
 static int
 nouveau_ttm_init_host(struct nouveau_drm *drm, u8 kind)
 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.h b/drivers/gpu/drm/nouveau/nouveau_ttm.h
index dbf6dc238efd..2f0efda7ccdb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.h
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.h
@@ -17,7 +17,6 @@ struct ttm_tt *nouveau_sgdma_create_ttm(struct ttm_buffer_object *bo,
 
 int  nouveau_ttm_init(struct nouveau_drm *drm);
 void nouveau_ttm_fini(struct nouveau_drm *drm);
-int  nouveau_ttm_mmap(struct file *, struct vm_area_struct *);
 
 int  nouveau_ttm_global_init(struct nouveau_drm *);
 void nouveau_ttm_global_release(struct nouveau_drm *);
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 5/7] drm/vmwgfx: Inline ttm_bo_mmap() into vmwgfx driver

[Thomas Zimmermann]

The vmwgfx driver is the only remaining user of ttm_bo_mmap(). Inline
the code. The internal helper ttm_bo_vm_lookup() is now also part of
vmwgfx as vmw_bo_vm_lookup().

v2:
	* replace pr_err() with drm_err() (Zack)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Zack Rusin <zackr@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 56 ++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index cb9975889e2f..c8b6543b4e39 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -27,6 +27,32 @@
 
 #include "vmwgfx_drv.h"
 
+static struct ttm_buffer_object *vmw_bo_vm_lookup(struct ttm_device *bdev,
+						  unsigned long offset,
+						  unsigned long pages)
+{
+	struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
+	struct drm_device *drm = &dev_priv->drm;
+	struct drm_vma_offset_node *node;
+	struct ttm_buffer_object *bo = NULL;
+
+	drm_vma_offset_lock_lookup(bdev->vma_manager);
+
+	node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
+	if (likely(node)) {
+		bo = container_of(node, struct ttm_buffer_object,
+				  base.vma_node);
+		bo = ttm_bo_get_unless_zero(bo);
+	}
+
+	drm_vma_offset_unlock_lookup(bdev->vma_manager);
+
+	if (!bo)
+		drm_err(drm, "Could not find buffer object to map\n");
+
+	return bo;
+}
+
 int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 {
 	static const struct vm_operations_struct vmw_vm_ops = {
@@ -41,10 +67,28 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 	};
 	struct drm_file *file_priv = filp->private_data;
 	struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
-	int ret = ttm_bo_mmap(filp, vma, &dev_priv->bdev);
+	struct ttm_device *bdev = &dev_priv->bdev;
+	struct ttm_buffer_object *bo;
+	int ret;
+
+	if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
+		return -EINVAL;
+
+	bo = vmw_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
+	if (unlikely(!bo))
+		return -EINVAL;
 
-	if (ret)
-		return ret;
+	if (unlikely(!bo->bdev->funcs->verify_access)) {
+		ret = -EPERM;
+		goto out_unref;
+	}
+	ret = bo->bdev->funcs->verify_access(bo, filp);
+	if (unlikely(ret != 0))
+		goto out_unref;
+
+	ret = ttm_bo_mmap_obj(vma, bo);
+	if (unlikely(ret != 0))
+		goto out_unref;
 
 	vma->vm_ops = &vmw_vm_ops;
 
@@ -52,7 +96,13 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (!is_cow_mapping(vma->vm_flags))
 		vma->vm_flags = (vma->vm_flags & ~VM_MIXEDMAP) | VM_PFNMAP;
 
+	ttm_bo_put(bo); /* release extra ref taken by ttm_bo_mmap_obj() */
+
 	return 0;
+
+out_unref:
+	ttm_bo_put(bo);
+	return ret;
 }
 
 /* struct vmw_validation_mem callback */
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 6/7] drm/vmwgfx: Inline vmw_verify_access()

[Thomas Zimmermann]

Vmwgfx is the only user of the TTM's verify_access callback. Inline
the call and avoid the indirection through the function pointer.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Zack Rusin <zackr@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c | 9 ---------
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c   | 7 ++-----
 2 files changed, 2 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index 2dc031fe4a90..a079734f9d68 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -658,14 +658,6 @@ static void vmw_evict_flags(struct ttm_buffer_object *bo,
 	*placement = vmw_sys_placement;
 }
 
-static int vmw_verify_access(struct ttm_buffer_object *bo, struct file *filp)
-{
-	struct ttm_object_file *tfile =
-		vmw_fpriv((struct drm_file *)filp->private_data)->tfile;
-
-	return vmw_user_bo_verify_access(bo, tfile);
-}
-
 static int vmw_ttm_io_mem_reserve(struct ttm_device *bdev, struct ttm_resource *mem)
 {
 	struct vmw_private *dev_priv = container_of(bdev, struct vmw_private, bdev);
@@ -768,7 +760,6 @@ struct ttm_device_funcs vmw_bo_driver = {
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = vmw_evict_flags,
 	.move = vmw_move,
-	.verify_access = vmw_verify_access,
 	.swap_notify = vmw_swap_notify,
 	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
 };
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index c8b6543b4e39..e6b1f98ec99f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -67,6 +67,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 	};
 	struct drm_file *file_priv = filp->private_data;
 	struct vmw_private *dev_priv = vmw_priv(file_priv->minor->dev);
+	struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
 	struct ttm_device *bdev = &dev_priv->bdev;
 	struct ttm_buffer_object *bo;
 	int ret;
@@ -78,11 +79,7 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
 	if (unlikely(!bo))
 		return -EINVAL;
 
-	if (unlikely(!bo->bdev->funcs->verify_access)) {
-		ret = -EPERM;
-		goto out_unref;
-	}
-	ret = bo->bdev->funcs->verify_access(bo, filp);
+	ret = vmw_user_bo_verify_access(bo, tfile);
 	if (unlikely(ret != 0))
 		goto out_unref;
 
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH v2 7/7] drm/ttm: Remove ttm_bo_mmap() and friends

[Thomas Zimmermann]

The function ttm_bo_mmap is unused. Remove it and it's helpers; including
the verify_access callback in struct ttm_device_funcs.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/ttm/ttm_bo_vm.c | 53 ---------------------------------
 include/drm/ttm/ttm_bo_api.h    | 13 --------
 include/drm/ttm/ttm_device.h    | 15 ----------
 3 files changed, 81 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
index bf4a213bc66c..6cd352399941 100644
--- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
+++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
@@ -508,30 +508,6 @@ static const struct vm_operations_struct ttm_bo_vm_ops = {
 	.access = ttm_bo_vm_access,
 };
 
-static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
-						  unsigned long offset,
-						  unsigned long pages)
-{
-	struct drm_vma_offset_node *node;
-	struct ttm_buffer_object *bo = NULL;
-
-	drm_vma_offset_lock_lookup(bdev->vma_manager);
-
-	node = drm_vma_offset_lookup_locked(bdev->vma_manager, offset, pages);
-	if (likely(node)) {
-		bo = container_of(node, struct ttm_buffer_object,
-				  base.vma_node);
-		bo = ttm_bo_get_unless_zero(bo);
-	}
-
-	drm_vma_offset_unlock_lookup(bdev->vma_manager);
-
-	if (!bo)
-		pr_err("Could not find buffer object to map\n");
-
-	return bo;
-}
-
 static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma)
 {
 	/*
@@ -559,35 +535,6 @@ static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_s
 	vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP;
 }
 
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-		struct ttm_device *bdev)
-{
-	struct ttm_buffer_object *bo;
-	int ret;
-
-	if (unlikely(vma->vm_pgoff < DRM_FILE_PAGE_OFFSET_START))
-		return -EINVAL;
-
-	bo = ttm_bo_vm_lookup(bdev, vma->vm_pgoff, vma_pages(vma));
-	if (unlikely(!bo))
-		return -EINVAL;
-
-	if (unlikely(!bo->bdev->funcs->verify_access)) {
-		ret = -EPERM;
-		goto out_unref;
-	}
-	ret = bo->bdev->funcs->verify_access(bo, filp);
-	if (unlikely(ret != 0))
-		goto out_unref;
-
-	ttm_bo_mmap_vma_setup(bo, vma);
-	return 0;
-out_unref:
-	ttm_bo_put(bo);
-	return ret;
-}
-EXPORT_SYMBOL(ttm_bo_mmap);
-
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo)
 {
 	ttm_bo_get(bo);
diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
index 2155e2e38aec..6e35680ac01b 100644
--- a/include/drm/ttm/ttm_bo_api.h
+++ b/include/drm/ttm/ttm_bo_api.h
@@ -522,19 +522,6 @@ void ttm_bo_vunmap(struct ttm_buffer_object *bo, struct dma_buf_map *map);
  */
 int ttm_bo_mmap_obj(struct vm_area_struct *vma, struct ttm_buffer_object *bo);
 
-/**
- * ttm_bo_mmap - mmap out of the ttm device address space.
- *
- * @filp:      filp as input from the mmap method.
- * @vma:       vma as input from the mmap method.
- * @bdev:      Pointer to the ttm_device with the address space manager.
- *
- * This function is intended to be called by the device mmap method.
- * if the device address space is to be backed by the bo manager.
- */
-int ttm_bo_mmap(struct file *filp, struct vm_area_struct *vma,
-		struct ttm_device *bdev);
-
 /**
  * ttm_bo_io
  *
diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
index 7c8f87bd52d3..cd592f8e941b 100644
--- a/include/drm/ttm/ttm_device.h
+++ b/include/drm/ttm/ttm_device.h
@@ -161,21 +161,6 @@ struct ttm_device_funcs {
 		    struct ttm_resource *new_mem,
 		    struct ttm_place *hop);
 
-	/**
-	 * struct ttm_bo_driver_member verify_access
-	 *
-	 * @bo: Pointer to a buffer object.
-	 * @filp: Pointer to a struct file trying to access the object.
-	 *
-	 * Called from the map / write / read methods to verify that the
-	 * caller is permitted to access the buffer object.
-	 * This member may be set to NULL, which will refuse this kind of
-	 * access for all buffer objects.
-	 * This function should return 0 if access is granted, -EPERM otherwise.
-	 */
-	int (*verify_access)(struct ttm_buffer_object *bo,
-			     struct file *filp);
-
 	/**
 	 * Hook to notify driver about a resource delete.
 	 */
-- 
2.31.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/7] drm/amdgpu: Implement mmap as GEM object function

[Christian König]

Am 15.04.21 um 12:17 schrieb Thomas Zimmermann:
> Moving the driver-specific mmap code into a GEM object function allows
> for using DRM helpers for various mmap callbacks.
>
> This change resolves several inconsistencies between regular mmap and
> prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
> areas. Previously it way only set for regular mmap calls, prime-based
> mmap used TTM's default vm_ops. The function amdgpu_verify_access() is
> no longer being called and therefore removed by this patch.
>
> As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
> implemented in amdgpu's GEM code.
>
> v2:
> 	* rename amdgpu_ttm_vm_ops and amdgpu_ttm_fault() to
> 	  amdgpu_gem_vm_ops and amdgpu_gem_fault() (Christian)
> 	* the check for kfd_bo has meanwhile been removed
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

[SNIP]
> +static int amdgpu_gem_prime_mmap(struct drm_gem_object *obj, struct vm_area_struct *vma)
> +{

Mhm, just double checking this function is now a core GEM function and 
not prime specific?

If yes maybe drop the _prime part.

> +	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +	unsigned long asize = amdgpu_bo_size(bo);
> +
> +	if (!vma->vm_file)
> +		return -ENODEV;
> +
> +	if (!adev)
> +		return -ENODEV;
> +
> +	/* Check for valid size. */
> +	if (asize < vma->vm_end - vma->vm_start)
> +		return -EINVAL;

Shouldn't we have that check in the common code?

Apart from that looks good to me.

Christian.

> +
> +	if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
> +	    (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
> +		return -EPERM;
> +	}
> +
> +	return drm_gem_ttm_mmap(obj, vma);
> +}
> +
>   static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
>   	.free = amdgpu_gem_object_free,
>   	.open = amdgpu_gem_object_open,
> @@ -212,6 +266,8 @@ static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
>   	.export = amdgpu_gem_prime_export,
>   	.vmap = drm_gem_ttm_vmap,
>   	.vunmap = drm_gem_ttm_vunmap,
> +	.mmap = amdgpu_gem_prime_mmap,
> +	.vm_ops = &amdgpu_gem_vm_ops,
>   };
>   
>   /*
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 1485f33c3cc7..d4083c19402b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -152,25 +152,6 @@ static void amdgpu_evict_flags(struct ttm_buffer_object *bo,
>   	*placement = abo->placement;
>   }
>   
> -/**
> - * amdgpu_verify_access - Verify access for a mmap call
> - *
> - * @bo:	The buffer object to map
> - * @filp: The file pointer from the process performing the mmap
> - *
> - * This is called by ttm_bo_mmap() to verify whether a process
> - * has the right to mmap a BO to their process space.
> - */
> -static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct file *filp)
> -{
> -	struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
> -
> -	if (amdgpu_ttm_tt_get_usermm(bo->ttm))
> -		return -EPERM;
> -	return drm_vma_node_verify_access(&abo->tbo.base.vma_node,
> -					  filp->private_data);
> -}
> -
>   /**
>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>    * @bo: buffer object to map
> @@ -1522,7 +1503,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = {
>   	.eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
>   	.evict_flags = &amdgpu_evict_flags,
>   	.move = &amdgpu_bo_move,
> -	.verify_access = &amdgpu_verify_access,
>   	.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
>   	.release_notify = &amdgpu_bo_release_notify,
>   	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
> @@ -1897,50 +1877,6 @@ void amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool enable)
>   	adev->mman.buffer_funcs_enabled = enable;
>   }
>   
> -static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
> -{
> -	struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
> -	vm_fault_t ret;
> -
> -	ret = ttm_bo_vm_reserve(bo, vmf);
> -	if (ret)
> -		return ret;
> -
> -	ret = amdgpu_bo_fault_reserve_notify(bo);
> -	if (ret)
> -		goto unlock;
> -
> -	ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
> -				       TTM_BO_VM_NUM_PREFAULT, 1);
> -	if (ret == VM_FAULT_RETRY && !(vmf->flags & FAULT_FLAG_RETRY_NOWAIT))
> -		return ret;
> -
> -unlock:
> -	dma_resv_unlock(bo->base.resv);
> -	return ret;
> -}
> -
> -static const struct vm_operations_struct amdgpu_ttm_vm_ops = {
> -	.fault = amdgpu_ttm_fault,
> -	.open = ttm_bo_vm_open,
> -	.close = ttm_bo_vm_close,
> -	.access = ttm_bo_vm_access
> -};
> -
> -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma)
> -{
> -	struct drm_file *file_priv = filp->private_data;
> -	struct amdgpu_device *adev = drm_to_adev(file_priv->minor->dev);
> -	int r;
> -
> -	r = ttm_bo_mmap(filp, vma, &adev->mman.bdev);
> -	if (unlikely(r != 0))
> -		return r;
> -
> -	vma->vm_ops = &amdgpu_ttm_vm_ops;
> -	return 0;
> -}
> -
>   int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
>   		       uint64_t dst_offset, uint32_t byte_count,
>   		       struct dma_resv *resv,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> index dec0db8b0b13..6e51faad7371 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
> @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>   			struct dma_resv *resv,
>   			struct dma_fence **fence);
>   
> -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
>   int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
>   uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, uint32_t type);

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/7] drm/amdgpu: Implement mmap as GEM object function

[Thomas Zimmermann]

[-- Attachment #1.1.1: Type: text/plain, Size: 7457 bytes --]

Hi

Am 15.04.21 um 13:38 schrieb Christian König:
> Am 15.04.21 um 12:17 schrieb Thomas Zimmermann:
>> Moving the driver-specific mmap code into a GEM object function allows
>> for using DRM helpers for various mmap callbacks.
>>
>> This change resolves several inconsistencies between regular mmap and
>> prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
>> areas. Previously it way only set for regular mmap calls, prime-based
>> mmap used TTM's default vm_ops. The function amdgpu_verify_access() is
>> no longer being called and therefore removed by this patch.
>>
>> As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
>> implemented in amdgpu's GEM code.
>>
>> v2:
>>     * rename amdgpu_ttm_vm_ops and amdgpu_ttm_fault() to
>>       amdgpu_gem_vm_ops and amdgpu_gem_fault() (Christian)
>>     * the check for kfd_bo has meanwhile been removed
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> 
> [SNIP]
>> +static int amdgpu_gem_prime_mmap(struct drm_gem_object *obj, struct 
>> vm_area_struct *vma)
>> +{
> 
> Mhm, just double checking this function is now a core GEM function and 
> not prime specific?
> 
> If yes maybe drop the _prime part.

No problem, but other functions in the GEM callbacks are also named 
_prime_. Probably needs a clean-up.

> 
>> +    struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>> +    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>> +    unsigned long asize = amdgpu_bo_size(bo);
>> +
>> +    if (!vma->vm_file)
>> +        return -ENODEV;
>> +
>> +    if (!adev)
>> +        return -ENODEV;
>> +
>> +    /* Check for valid size. */
>> +    if (asize < vma->vm_end - vma->vm_start)
>> +        return -EINVAL;
> 
> Shouldn't we have that check in the common code?

It's at [1]. I didn't really bother about tidying up the checks 
themselves. We can drop all these except for the usermm branch below. 
Same for the radeon patch.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/v5.12-rc7/source/drivers/gpu/drm/drm_gem.c#L1059

> 
> Apart from that looks good to me.
> 
> Christian.
> 
>> +
>> +    if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
>> +        (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
>> +        return -EPERM;
>> +    }
>> +
>> +    return drm_gem_ttm_mmap(obj, vma);
>> +}
>> +
>>   static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
>>       .free = amdgpu_gem_object_free,
>>       .open = amdgpu_gem_object_open,
>> @@ -212,6 +266,8 @@ static const struct drm_gem_object_funcs 
>> amdgpu_gem_object_funcs = {
>>       .export = amdgpu_gem_prime_export,
>>       .vmap = drm_gem_ttm_vmap,
>>       .vunmap = drm_gem_ttm_vunmap,
>> +    .mmap = amdgpu_gem_prime_mmap,
>> +    .vm_ops = &amdgpu_gem_vm_ops,
>>   };
>>   /*
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> index 1485f33c3cc7..d4083c19402b 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>> @@ -152,25 +152,6 @@ static void amdgpu_evict_flags(struct 
>> ttm_buffer_object *bo,
>>       *placement = abo->placement;
>>   }
>> -/**
>> - * amdgpu_verify_access - Verify access for a mmap call
>> - *
>> - * @bo:    The buffer object to map
>> - * @filp: The file pointer from the process performing the mmap
>> - *
>> - * This is called by ttm_bo_mmap() to verify whether a process
>> - * has the right to mmap a BO to their process space.
>> - */
>> -static int amdgpu_verify_access(struct ttm_buffer_object *bo, struct 
>> file *filp)
>> -{
>> -    struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>> -
>> -    if (amdgpu_ttm_tt_get_usermm(bo->ttm))
>> -        return -EPERM;
>> -    return drm_vma_node_verify_access(&abo->tbo.base.vma_node,
>> -                      filp->private_data);
>> -}
>> -
>>   /**
>>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>>    * @bo: buffer object to map
>> @@ -1522,7 +1503,6 @@ static struct ttm_device_funcs amdgpu_bo_driver = {
>>       .eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
>>       .evict_flags = &amdgpu_evict_flags,
>>       .move = &amdgpu_bo_move,
>> -    .verify_access = &amdgpu_verify_access,
>>       .delete_mem_notify = &amdgpu_bo_delete_mem_notify,
>>       .release_notify = &amdgpu_bo_release_notify,
>>       .io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
>> @@ -1897,50 +1877,6 @@ void amdgpu_ttm_set_buffer_funcs_status(struct 
>> amdgpu_device *adev, bool enable)
>>       adev->mman.buffer_funcs_enabled = enable;
>>   }
>> -static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
>> -{
>> -    struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
>> -    vm_fault_t ret;
>> -
>> -    ret = ttm_bo_vm_reserve(bo, vmf);
>> -    if (ret)
>> -        return ret;
>> -
>> -    ret = amdgpu_bo_fault_reserve_notify(bo);
>> -    if (ret)
>> -        goto unlock;
>> -
>> -    ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
>> -                       TTM_BO_VM_NUM_PREFAULT, 1);
>> -    if (ret == VM_FAULT_RETRY && !(vmf->flags & 
>> FAULT_FLAG_RETRY_NOWAIT))
>> -        return ret;
>> -
>> -unlock:
>> -    dma_resv_unlock(bo->base.resv);
>> -    return ret;
>> -}
>> -
>> -static const struct vm_operations_struct amdgpu_ttm_vm_ops = {
>> -    .fault = amdgpu_ttm_fault,
>> -    .open = ttm_bo_vm_open,
>> -    .close = ttm_bo_vm_close,
>> -    .access = ttm_bo_vm_access
>> -};
>> -
>> -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma)
>> -{
>> -    struct drm_file *file_priv = filp->private_data;
>> -    struct amdgpu_device *adev = drm_to_adev(file_priv->minor->dev);
>> -    int r;
>> -
>> -    r = ttm_bo_mmap(filp, vma, &adev->mman.bdev);
>> -    if (unlikely(r != 0))
>> -        return r;
>> -
>> -    vma->vm_ops = &amdgpu_ttm_vm_ops;
>> -    return 0;
>> -}
>> -
>>   int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
>>                  uint64_t dst_offset, uint32_t byte_count,
>>                  struct dma_resv *resv,
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> index dec0db8b0b13..6e51faad7371 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>> @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>               struct dma_resv *resv,
>>               struct dma_fence **fence);
>> -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>>   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
>>   int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
>>   uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, 
>> uint32_t type);
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 2/7] drm/amdgpu: Implement mmap as GEM object function

[Christian König]

Am 15.04.21 um 14:11 schrieb Thomas Zimmermann:
> Hi
>
> Am 15.04.21 um 13:38 schrieb Christian König:
>> Am 15.04.21 um 12:17 schrieb Thomas Zimmermann:
>>> Moving the driver-specific mmap code into a GEM object function allows
>>> for using DRM helpers for various mmap callbacks.
>>>
>>> This change resolves several inconsistencies between regular mmap and
>>> prime-based mmap. The vm_ops field in vma is now set for all mmap'ed
>>> areas. Previously it way only set for regular mmap calls, prime-based
>>> mmap used TTM's default vm_ops. The function amdgpu_verify_access() is
>>> no longer being called and therefore removed by this patch.
>>>
>>> As a side effect, amdgpu_ttm_vm_ops and amdgpu_ttm_fault() are now
>>> implemented in amdgpu's GEM code.
>>>
>>> v2:
>>>     * rename amdgpu_ttm_vm_ops and amdgpu_ttm_fault() to
>>>       amdgpu_gem_vm_ops and amdgpu_gem_fault() (Christian)
>>>     * the check for kfd_bo has meanwhile been removed
>>>
>>> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
>>
>> [SNIP]
>>> +static int amdgpu_gem_prime_mmap(struct drm_gem_object *obj, struct 
>>> vm_area_struct *vma)
>>> +{
>>
>> Mhm, just double checking this function is now a core GEM function 
>> and not prime specific?
>>
>> If yes maybe drop the _prime part.
>
> No problem, but other functions in the GEM callbacks are also named 
> _prime_. Probably needs a clean-up.

Yeah, some still needs to be renamed. But there shouldn't be any left in 
amdgpu_gem.c

>
>>
>>> +    struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
>>> +    struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
>>> +    unsigned long asize = amdgpu_bo_size(bo);
>>> +
>>> +    if (!vma->vm_file)
>>> +        return -ENODEV;
>>> +
>>> +    if (!adev)
>>> +        return -ENODEV;
>>> +
>>> +    /* Check for valid size. */
>>> +    if (asize < vma->vm_end - vma->vm_start)
>>> +        return -EINVAL;
>>
>> Shouldn't we have that check in the common code?
>
> It's at [1]. I didn't really bother about tidying up the checks 
> themselves. We can drop all these except for the usermm branch below. 
> Same for the radeon patch.

In this case I would just drop that from both amdgpu and radeon.

Thanks,
Christian.

>
> Best regards
> Thomas
>
> [1] 
> https://elixir.bootlin.com/linux/v5.12-rc7/source/drivers/gpu/drm/drm_gem.c#L1059
>
>>
>> Apart from that looks good to me.
>>
>> Christian.
>>
>>> +
>>> +    if (amdgpu_ttm_tt_get_usermm(bo->tbo.ttm) ||
>>> +        (bo->flags & AMDGPU_GEM_CREATE_NO_CPU_ACCESS)) {
>>> +        return -EPERM;
>>> +    }
>>> +
>>> +    return drm_gem_ttm_mmap(obj, vma);
>>> +}
>>> +
>>>   static const struct drm_gem_object_funcs amdgpu_gem_object_funcs = {
>>>       .free = amdgpu_gem_object_free,
>>>       .open = amdgpu_gem_object_open,
>>> @@ -212,6 +266,8 @@ static const struct drm_gem_object_funcs 
>>> amdgpu_gem_object_funcs = {
>>>       .export = amdgpu_gem_prime_export,
>>>       .vmap = drm_gem_ttm_vmap,
>>>       .vunmap = drm_gem_ttm_vunmap,
>>> +    .mmap = amdgpu_gem_prime_mmap,
>>> +    .vm_ops = &amdgpu_gem_vm_ops,
>>>   };
>>>   /*
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 1485f33c3cc7..d4083c19402b 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -152,25 +152,6 @@ static void amdgpu_evict_flags(struct 
>>> ttm_buffer_object *bo,
>>>       *placement = abo->placement;
>>>   }
>>> -/**
>>> - * amdgpu_verify_access - Verify access for a mmap call
>>> - *
>>> - * @bo:    The buffer object to map
>>> - * @filp: The file pointer from the process performing the mmap
>>> - *
>>> - * This is called by ttm_bo_mmap() to verify whether a process
>>> - * has the right to mmap a BO to their process space.
>>> - */
>>> -static int amdgpu_verify_access(struct ttm_buffer_object *bo, 
>>> struct file *filp)
>>> -{
>>> -    struct amdgpu_bo *abo = ttm_to_amdgpu_bo(bo);
>>> -
>>> -    if (amdgpu_ttm_tt_get_usermm(bo->ttm))
>>> -        return -EPERM;
>>> -    return drm_vma_node_verify_access(&abo->tbo.base.vma_node,
>>> -                      filp->private_data);
>>> -}
>>> -
>>>   /**
>>>    * amdgpu_ttm_map_buffer - Map memory into the GART windows
>>>    * @bo: buffer object to map
>>> @@ -1522,7 +1503,6 @@ static struct ttm_device_funcs 
>>> amdgpu_bo_driver = {
>>>       .eviction_valuable = amdgpu_ttm_bo_eviction_valuable,
>>>       .evict_flags = &amdgpu_evict_flags,
>>>       .move = &amdgpu_bo_move,
>>> -    .verify_access = &amdgpu_verify_access,
>>>       .delete_mem_notify = &amdgpu_bo_delete_mem_notify,
>>>       .release_notify = &amdgpu_bo_release_notify,
>>>       .io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
>>> @@ -1897,50 +1877,6 @@ void 
>>> amdgpu_ttm_set_buffer_funcs_status(struct amdgpu_device *adev, bool 
>>> enable)
>>>       adev->mman.buffer_funcs_enabled = enable;
>>>   }
>>> -static vm_fault_t amdgpu_ttm_fault(struct vm_fault *vmf)
>>> -{
>>> -    struct ttm_buffer_object *bo = vmf->vma->vm_private_data;
>>> -    vm_fault_t ret;
>>> -
>>> -    ret = ttm_bo_vm_reserve(bo, vmf);
>>> -    if (ret)
>>> -        return ret;
>>> -
>>> -    ret = amdgpu_bo_fault_reserve_notify(bo);
>>> -    if (ret)
>>> -        goto unlock;
>>> -
>>> -    ret = ttm_bo_vm_fault_reserved(vmf, vmf->vma->vm_page_prot,
>>> -                       TTM_BO_VM_NUM_PREFAULT, 1);
>>> -    if (ret == VM_FAULT_RETRY && !(vmf->flags & 
>>> FAULT_FLAG_RETRY_NOWAIT))
>>> -        return ret;
>>> -
>>> -unlock:
>>> -    dma_resv_unlock(bo->base.resv);
>>> -    return ret;
>>> -}
>>> -
>>> -static const struct vm_operations_struct amdgpu_ttm_vm_ops = {
>>> -    .fault = amdgpu_ttm_fault,
>>> -    .open = ttm_bo_vm_open,
>>> -    .close = ttm_bo_vm_close,
>>> -    .access = ttm_bo_vm_access
>>> -};
>>> -
>>> -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma)
>>> -{
>>> -    struct drm_file *file_priv = filp->private_data;
>>> -    struct amdgpu_device *adev = drm_to_adev(file_priv->minor->dev);
>>> -    int r;
>>> -
>>> -    r = ttm_bo_mmap(filp, vma, &adev->mman.bdev);
>>> -    if (unlikely(r != 0))
>>> -        return r;
>>> -
>>> -    vma->vm_ops = &amdgpu_ttm_vm_ops;
>>> -    return 0;
>>> -}
>>> -
>>>   int amdgpu_copy_buffer(struct amdgpu_ring *ring, uint64_t src_offset,
>>>                  uint64_t dst_offset, uint32_t byte_count,
>>>                  struct dma_resv *resv,
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h 
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> index dec0db8b0b13..6e51faad7371 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h
>>> @@ -146,7 +146,6 @@ int amdgpu_fill_buffer(struct amdgpu_bo *bo,
>>>               struct dma_resv *resv,
>>>               struct dma_fence **fence);
>>> -int amdgpu_mmap(struct file *filp, struct vm_area_struct *vma);
>>>   int amdgpu_ttm_alloc_gart(struct ttm_buffer_object *bo);
>>>   int amdgpu_ttm_recover_gart(struct ttm_buffer_object *tbo);
>>>   uint64_t amdgpu_ttm_domain_start(struct amdgpu_device *adev, 
>>> uint32_t type);
>>
>

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH v2 1/7] drm/ttm: Don't override vm_ops callbacks, if set

[Daniel Vetter]

On Thu, Apr 15, 2021 at 12:17:34PM +0200, Thomas Zimmermann wrote:
> Drivers may want to set their own callbacks for a VM area. Only set
> TTM's callbacks if the vm_ops field is clear.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/ttm/ttm_bo_vm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> index b31b18058965..bf4a213bc66c 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c
> @@ -534,7 +534,12 @@ static struct ttm_buffer_object *ttm_bo_vm_lookup(struct ttm_device *bdev,
>  
>  static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_struct *vma)
>  {
> -	vma->vm_ops = &ttm_bo_vm_ops;
> +	/*
> +	 * Drivers may want to override the vm_ops field. Otherwise we
> +	 * use TTM's default callbacks.
> +	 */
> +	if (!vma->vm_ops)
> +		vma->vm_ops = &ttm_bo_vm_ops;
>  
>  	/*
>  	 * Note: We're transferring the bo reference to
> -- 
> 2.31.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx