[PATCH RFC 00/12] drm/v3d: Add support for write-back cacheable BOs

[PATCH RFC 00/12] drm/v3d: Add support for write-back cacheable BOs

[Maíra Canal]

This series aims to implement cached maps and explicit flushing for the
V3D driver, which is mostly motivated by Vulkan's host cached memory.

V3D's implementation of cached maps is quite similar to Panfrost's and
therefore, I decided to revamp Panthor/Panfrost's efforts [1] to implement
common shmem helpers for CPU cache maintenance. The other option would be
copying the same code in V3D, so I preferred to retake the discussion
and see if we can reach some consensus.

The most controversial part of this series are the first three patches,
which were extensively discussed in Boris' series.

Patch 1 lets the PRIME helpers work with custom dma_buf_ops, following
Thomas' feedback on [1]: drm_gem_prime_export_with_ops() exports with
driver-provided ops, and gem_prime_is_exporter() lets a driver recognize
its own DMA-BUFs on import, without adding buffer callbacks to struct
drm_driver, as requested by Thomas.

Patches 2-3 are taken from [1], rebased on top of drm-misc-next with some
minor kernel-doc fixes.

Patches 4-8 convert Panfrost, Panthor and amdgpu to the new helpers. Note
that I didn't use the new helpers from patches 2-3 in Panthor, as they
recently parted ways with DRM GEM shmem.

Finally, patches 9-12 add v3d support to cacheable BOs and the new ioctl
DRM_IOCTL_V3D_SYNC_BO.

About the userspace implementation, I tested this implementation using
a hack implementation (based in PanVK), as I preferred to leave the
userspace implementation to be finished by JaeHoon Lee, which is already
working on it on [2].

[1] https://lore.kernel.org/dri-devel/20251126124455.3656651-1-boris.brezillon@collabora.com/
[2] https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/42974

Best regards,
- Maíra

---
Boris Brezillon (1):
      drm/shmem: Provide a generic {begin, end}_cpu_access() implementation

Faith Ekstrand (1):
      drm/shmem: Add a drm_gem_shmem_sync() helper

Maíra Canal (10):
      drm/prime: Allow custom dma_buf_ops in PRIME helpers
      drm/panfrost: Use PRIME helpers with custom dma_buf_ops
      drm/panfrost: Use common shmem helpers for dma_buf CPU access
      drm/panfrost: Use drm_gem_shmem_sync()
      drm/panthor: Use PRIME helpers with custom dma_buf_ops
      drm/amdgpu: Use drm_gem_prime_export_with_ops()
      drm/v3d: Provide a custom dma_buf implementation
      drm/v3d: Add flag to map BOs write-back cacheable
      drm/v3d: Add a DRM_IOCTL_V3D_SYNC_BO ioctl
      drm/v3d: Advertise support for write-back cacheable BOs

 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c |   9 +-
 drivers/gpu/drm/drm_gem_shmem_helper.c      | 208 ++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_prime.c                 |  47 +++++--
 drivers/gpu/drm/panfrost/panfrost_drv.c     |   2 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c     | 185 ++-----------------------
 drivers/gpu/drm/panfrost/panfrost_gem.h     |   5 +-
 drivers/gpu/drm/panthor/panthor_drv.c       |   2 +-
 drivers/gpu/drm/panthor/panthor_gem.c       |  28 +---
 drivers/gpu/drm/panthor/panthor_gem.h       |   5 +-
 drivers/gpu/drm/v3d/v3d_bo.c                | 101 +++++++++++++-
 drivers/gpu/drm/v3d/v3d_drv.c               |   5 +
 drivers/gpu/drm/v3d/v3d_drv.h               |   6 +-
 drivers/gpu/drm/v3d/v3d_irq.c               |   2 +-
 include/drm/drm_drv.h                       |  11 ++
 include/drm/drm_gem_shmem_helper.h          |  27 ++++
 include/drm/drm_prime.h                     |   4 +
 include/uapi/drm/v3d_drm.h                  |  58 +++++++-
 17 files changed, 474 insertions(+), 231 deletions(-)
---
base-commit: a1e63ab695ad76850ab9787b9eb6aab695dc2965
change-id: 20261028-v3d-cached-maps-faae3ba95e11

[PATCH RFC 01/12] drm/prime: Allow custom dma_buf_ops in PRIME helpers

[Maíra Canal]

drm_gem_prime_export() and drm_gem_is_prime_exported_dma_buf() hardcode
drm_gem_prime_dmabuf_ops. Drivers with custom dma_buf_ops can't use them
and end up duplicating the export and self-import logic instead.

Add drm_gem_prime_export_with_ops() to export with driver-provided ops,
and an optional drm_driver.gem_prime_is_exporter() callback to let drivers
identify their own DMA-BUFs. Note that the callback must receive DMA-BUF,
so that the driver can check its ops before dereferencing dma_buf->priv.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_prime.c | 47 +++++++++++++++++++++++++++++++++++----------
 include/drm/drm_drv.h       | 11 +++++++++++
 include/drm/drm_prime.h     |  4 ++++
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index fe3436d1235d..4183def83cdd 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -905,22 +905,26 @@ unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt)
 EXPORT_SYMBOL(drm_prime_get_contiguous_size);
 
 /**
- * drm_gem_prime_export - helper library implementation of the export callback
+ * drm_gem_prime_export_with_ops - export a GEM object with custom dma_buf_ops
  * @obj: GEM object to export
+ * @dma_buf_ops: &dma_buf_ops of the exported DMA-BUF
  * @flags: flags like DRM_CLOEXEC and DRM_RDWR
  *
- * This is the implementation of the &drm_gem_object_funcs.export functions for GEM drivers
- * using the PRIME helpers. It is used as the default in
- * drm_gem_prime_handle_to_fd().
+ * Same as drm_gem_prime_export(), but with driver-provided &dma_buf_ops.
+ * Drivers using this can implement &drm_driver.gem_prime_is_exporter for
+ * the PRIME import helpers to recognize their own buffers.
+ *
+ * Return: the exported DMA-BUF on success, or an ERR_PTR() on failure.
  */
-struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
-				     int flags)
+struct dma_buf *drm_gem_prime_export_with_ops(struct drm_gem_object *obj,
+					      const struct dma_buf_ops *dma_buf_ops,
+					      int flags)
 {
 	struct drm_device *dev = obj->dev;
 	struct dma_buf_export_info exp_info = {
 		.exp_name = KBUILD_MODNAME, /* white lie for debug */
 		.owner = dev->driver->fops->owner,
-		.ops = &drm_gem_prime_dmabuf_ops,
+		.ops = dma_buf_ops,
 		.size = obj->size,
 		.flags = flags,
 		.priv = obj,
@@ -929,24 +933,47 @@ struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
 
 	return drm_gem_dmabuf_export(dev, &exp_info);
 }
+EXPORT_SYMBOL(drm_gem_prime_export_with_ops);
+
+/**
+ * drm_gem_prime_export - helper library implementation of the export callback
+ * @obj: GEM object to export
+ * @flags: flags like DRM_CLOEXEC and DRM_RDWR
+ *
+ * This is the implementation of the &drm_gem_object_funcs.export function for
+ * GEM drivers using the PRIME helpers. It is used as the default in
+ * drm_gem_prime_handle_to_fd().
+ *
+ * Return: the exported DMA-BUF on success, or an ERR_PTR() on failure.
+ */
+struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
+				     int flags)
+{
+	return drm_gem_prime_export_with_ops(obj, &drm_gem_prime_dmabuf_ops, flags);
+}
 EXPORT_SYMBOL(drm_gem_prime_export);
 
 
 /**
- * drm_gem_is_prime_exported_dma_buf -
- * checks if the DMA-BUF was exported from a GEM object belonging to @dev.
+ * drm_gem_is_prime_exported_dma_buf - check if a DMA-BUF was exported by @dev
  * @dev: drm_device to check against
  * @dma_buf: dma-buf object to import
  *
+ * DMA-BUFs exported with custom &dma_buf_ops are recognized through
+ * &drm_driver.gem_prime_is_exporter.
+ *
  * Return: true if the DMA-BUF was exported from a GEM object belonging
  * to @dev, false otherwise.
  */
-
 bool drm_gem_is_prime_exported_dma_buf(struct drm_device *dev,
 				       struct dma_buf *dma_buf)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
 
+	if (dev->driver->gem_prime_is_exporter &&
+	    dev->driver->gem_prime_is_exporter(dev, dma_buf))
+		return true;
+
 	return (dma_buf->ops == &drm_gem_prime_dmabuf_ops) && (obj->dev == dev);
 }
 EXPORT_SYMBOL(drm_gem_is_prime_exported_dma_buf);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index b23830494ed4..d613740a06d4 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -310,6 +310,17 @@ struct drm_driver {
 	 */
 	struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,
 				struct dma_buf *dma_buf);
+	/**
+	 * @gem_prime_is_exporter:
+	 *
+	 * Returns true if @dma_buf was exported from a GEM object of @dev.
+	 * Drivers exporting with a custom &dma_buf_ops can implement this
+	 * hook to let the PRIME import helpers recognize their own buffers.
+	 *
+	 * This callback is optional.
+	 */
+	bool (*gem_prime_is_exporter)(struct drm_device *dev,
+				      struct dma_buf *dma_buf);
 	/**
 	 * @gem_prime_import_sg_table:
 	 *
diff --git a/include/drm/drm_prime.h b/include/drm/drm_prime.h
index f50f862f0d8b..9b704e3ca919 100644
--- a/include/drm/drm_prime.h
+++ b/include/drm/drm_prime.h
@@ -53,6 +53,7 @@ struct device;
 
 struct dma_buf_export_info;
 struct dma_buf;
+struct dma_buf_ops;
 struct dma_buf_attachment;
 struct iosys_map;
 
@@ -96,6 +97,9 @@ struct sg_table *drm_prime_pages_to_sg(struct drm_device *dev,
 				       struct page **pages, unsigned int nr_pages);
 struct dma_buf *drm_gem_prime_export(struct drm_gem_object *obj,
 				     int flags);
+struct dma_buf *drm_gem_prime_export_with_ops(struct drm_gem_object *obj,
+					      const struct dma_buf_ops *dma_buf_ops,
+					      int flags);
 
 unsigned long drm_prime_get_contiguous_size(struct sg_table *sgt);
 

-- 
2.56.0

[PATCH RFC 02/12] drm/shmem: Provide a generic {begin, end}_cpu_access() implementation

[Maíra Canal]

From: Boris Brezillon <boris.brezillon@collabora.com>

The default implementation simply takes care of invalidating/flushing
caches around CPU accesses. It takes care of both the exporter and
the importers, which forces us to overload the default
::[un]map_dma_buf() implementation provided by drm_gem.c to store the
sgt.

Reviewed-by: Steven Price <steven.price@arm.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
[mcanal: rebased on top of drm-misc-next and fixed kernel-doc typo]
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 115 +++++++++++++++++++++++++++++++++
 include/drm/drm_gem_shmem_helper.h     |  10 +++
 2 files changed, 125 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index d654a22a30de..13c4a68fdde0 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -993,6 +993,121 @@ struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
 }
 EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_import_no_map);
 
+/**
+ * drm_gem_shmem_prime_map_dma_buf - Default map_dma_buf() for exported buffers
+ * @attach: attachment
+ * @dir: type of access done by this attachment
+ *
+ * Default implementation for dma_buf_ops::map_dma_buf(). This is just a wrapper
+ * around drm_gem_map_dma_buf() that lets us set the dma_buf_attachment::priv
+ * to the sgt so that drm_gem_shmem_prime_{begin,end}_cpu_access() can sync
+ * around CPU accesses.
+ *
+ * Return: the mapped SG table on success, or an ERR_PTR() on failure.
+ */
+struct sg_table *
+drm_gem_shmem_prime_map_dma_buf(struct dma_buf_attachment *attach,
+				enum dma_data_direction dir)
+{
+	struct sg_table *sgt = drm_gem_map_dma_buf(attach, dir);
+
+	if (!IS_ERR(sgt))
+		attach->priv = sgt;
+
+	return sgt;
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_map_dma_buf);
+
+/**
+ * drm_gem_shmem_prime_unmap_dma_buf - Default unmap_dma_buf() for exported buffers
+ * @attach: attachment
+ * @sgt: SG table to unmap
+ * @dir: type of access done by this attachment
+ *
+ * Default implementation for dma_buf_ops::unmap_dma_buf(). This is just a wrapper
+ * around drm_gem_unmap_dma_buf() that lets us reset the dma_buf_attachment::priv
+ * field so that drm_gem_shmem_prime_{begin,end}_cpu_access() don't consider it
+ * as a mapped attachment to sync against.
+ */
+void drm_gem_shmem_prime_unmap_dma_buf(struct dma_buf_attachment *attach,
+				       struct sg_table *sgt,
+				       enum dma_data_direction dir)
+{
+	attach->priv = NULL;
+	drm_gem_unmap_dma_buf(attach, sgt, dir);
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_unmap_dma_buf);
+
+/**
+ * drm_gem_shmem_prime_begin_cpu_access - Default begin_cpu_access() for exported buffers
+ * @dma_buf: The exported DMA buffer this acts on
+ * @dir: direction of the access
+ *
+ * Default implementation for dma_buf_ops::begin_cpu_access(). This only takes care of
+ * cache maintenance.
+ */
+int drm_gem_shmem_prime_begin_cpu_access(struct dma_buf *dma_buf,
+					 enum dma_data_direction dir)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+	struct dma_buf_attachment *attach;
+
+	dma_resv_lock(obj->resv, NULL);
+	if (shmem->sgt)
+		dma_sync_sgtable_for_cpu(dev->dev, shmem->sgt, dir);
+
+	if (shmem->vaddr)
+		invalidate_kernel_vmap_range(shmem->vaddr, shmem->base.size);
+
+	list_for_each_entry(attach, &dma_buf->attachments, node) {
+		struct sg_table *sgt = attach->priv;
+
+		if (sgt)
+			dma_sync_sgtable_for_cpu(attach->dev, sgt, dir);
+	}
+	dma_resv_unlock(obj->resv);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_begin_cpu_access);
+
+/**
+ * drm_gem_shmem_prime_end_cpu_access - Default end_cpu_access() for exported buffers
+ * @dma_buf: The exported DMA buffer this acts on
+ * @dir: direction of the access
+ *
+ * Default implementation for dma_buf_ops::end_cpu_access(). This only takes care of
+ * cache maintenance.
+ */
+int drm_gem_shmem_prime_end_cpu_access(struct dma_buf *dma_buf,
+				       enum dma_data_direction dir)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+	struct drm_device *dev = obj->dev;
+	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
+	struct dma_buf_attachment *attach;
+
+	dma_resv_lock(obj->resv, NULL);
+	list_for_each_entry(attach, &dma_buf->attachments, node) {
+		struct sg_table *sgt = attach->priv;
+
+		if (sgt)
+			dma_sync_sgtable_for_device(attach->dev, sgt, dir);
+	}
+
+	if (shmem->vaddr)
+		flush_kernel_vmap_range(shmem->vaddr, shmem->base.size);
+
+	if (shmem->sgt)
+		dma_sync_sgtable_for_device(dev->dev, shmem->sgt, dir);
+
+	dma_resv_unlock(obj->resv);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_prime_end_cpu_access);
+
 /*
  * Kunit helpers
  */
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index bcbb92f3fe9b..358cd247f803 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -293,6 +293,16 @@ int drm_gem_shmem_dumb_create(struct drm_file *file, struct drm_device *dev,
 			      struct drm_mode_create_dumb *args);
 struct drm_gem_object *drm_gem_shmem_prime_import_no_map(struct drm_device *dev,
 							 struct dma_buf *buf);
+struct sg_table *
+drm_gem_shmem_prime_map_dma_buf(struct dma_buf_attachment *attach,
+				enum dma_data_direction dir);
+void drm_gem_shmem_prime_unmap_dma_buf(struct dma_buf_attachment *attach,
+				       struct sg_table *sgt,
+				       enum dma_data_direction dir);
+int drm_gem_shmem_prime_begin_cpu_access(struct dma_buf *dma_buf,
+					 enum dma_data_direction dir);
+int drm_gem_shmem_prime_end_cpu_access(struct dma_buf *dma_buf,
+				       enum dma_data_direction dir);
 
 /**
  * DRM_GEM_SHMEM_DRIVER_OPS - Default shmem GEM operations

-- 
2.56.0

[PATCH RFC 03/12] drm/shmem: Add a drm_gem_shmem_sync() helper

[Maíra Canal]

From: Faith Ekstrand <faith.ekstrand@collabora.com>

This enables syncing mapped GEM objects between the CPU and GPU via calls
to dma_sync_*().  It's a bit annoying as it requires walking the sg_table
so it's best if every driver doesn't hand-roll it.

Signed-off-by: Faith Ekstrand <faith.ekstrand@collabora.com>
Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
Reviewed-by: Steven Price <steven.price@arm.com>
[mcanal: rebased on top of drm-misc-next and fixed a kernel-doc nit]
Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/drm_gem_shmem_helper.c | 93 ++++++++++++++++++++++++++++++++++
 include/drm/drm_gem_shmem_helper.h     | 17 +++++++
 2 files changed, 110 insertions(+)

diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
index 13c4a68fdde0..9bf727fc1c19 100644
--- a/drivers/gpu/drm/drm_gem_shmem_helper.c
+++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
@@ -790,6 +790,99 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
 }
 EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
 
+/**
+ * drm_gem_shmem_sync - Sync CPU-mapped data
+ * @shmem: shmem GEM object
+ * @offset: Offset into the GEM object
+ * @size: Size of the area to sync
+ * @type: Type of synchronization
+ *
+ * Returns:
+ * 0 on success or a negative error code on failure.
+ */
+int drm_gem_shmem_sync(struct drm_gem_shmem_object *shmem, size_t offset,
+		       size_t size, enum drm_gem_shmem_sync_type type)
+{
+	const struct drm_device *dev = shmem->base.dev;
+	struct sg_table *sgt;
+	struct scatterlist *sgl;
+	unsigned int count;
+
+	/* Make sure the range is in bounds. */
+	if (offset + size < offset || offset + size > shmem->base.size)
+		return -EINVAL;
+
+	/* Disallow CPU-cache maintenance on imported buffers. */
+	if (drm_gem_is_imported(&shmem->base))
+		return -EINVAL;
+
+	switch (type) {
+	case DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH:
+	case DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE:
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	/* Don't bother if it's WC-mapped */
+	if (shmem->map_wc)
+		return 0;
+
+	/* Nothing to do if the size is zero. */
+	if (size == 0)
+		return 0;
+
+	sgt = drm_gem_shmem_get_pages_sgt(shmem);
+	if (IS_ERR(sgt))
+		return PTR_ERR(sgt);
+
+	for_each_sgtable_dma_sg(sgt, sgl, count) {
+		if (size == 0)
+			break;
+
+		dma_addr_t paddr = sg_dma_address(sgl);
+		size_t len = sg_dma_len(sgl);
+
+		if (len <= offset) {
+			offset -= len;
+			continue;
+		}
+
+		paddr += offset;
+		len -= offset;
+		len = min_t(size_t, len, size);
+		size -= len;
+		offset = 0;
+
+		/* It's unclear whether dma_sync_xxx() is the right API to do CPU
+		 * cache maintenance given an IOMMU can register their own
+		 * implementation doing more than just CPU cache flushes/invalidation,
+		 * and what we really care about here is CPU caches only, but that's
+		 * the best we have that is both arch-agnostic and does at least the
+		 * CPU cache maintenance on a <page,offset,size> tuple.
+		 *
+		 * Also, I wish we could do a single
+		 *
+		 *	dma_sync_single_for_device(BIDIR)
+		 *
+		 * and get a flush+invalidate, but that's not how it's implemented
+		 * in practice (at least on arm64), so we have to make it
+		 *
+		 *	dma_sync_single_for_device(TO_DEVICE)
+		 *	dma_sync_single_for_cpu(FROM_DEVICE)
+		 *
+		 * for the flush+invalidate case.
+		 */
+		dma_sync_single_for_device(dev->dev, paddr, len, DMA_TO_DEVICE);
+		if (type == DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE)
+			dma_sync_single_for_cpu(dev->dev, paddr, len, DMA_FROM_DEVICE);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_shmem_sync);
+
 /**
  * drm_gem_shmem_print_info() - Print &drm_gem_shmem_object info for debugfs
  * @shmem: shmem GEM object
diff --git a/include/drm/drm_gem_shmem_helper.h b/include/drm/drm_gem_shmem_helper.h
index 358cd247f803..3c6ed3a5f0ac 100644
--- a/include/drm/drm_gem_shmem_helper.h
+++ b/include/drm/drm_gem_shmem_helper.h
@@ -122,6 +122,23 @@ void drm_gem_shmem_vunmap_locked(struct drm_gem_shmem_object *shmem,
 				 struct iosys_map *map);
 int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct *vma);
 
+/**
+ * enum drm_gem_shmem_sync_type - Type of synchronization
+ */
+enum drm_gem_shmem_sync_type {
+	/** @DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH: Flush CPU caches */
+	DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH = 0,
+
+	/**
+	 * @DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE: Flush and
+	 * invalidate CPU caches
+	 */
+	DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE,
+};
+
+int drm_gem_shmem_sync(struct drm_gem_shmem_object *shmem, size_t offset,
+		       size_t size, enum drm_gem_shmem_sync_type type);
+
 int drm_gem_shmem_pin_locked(struct drm_gem_shmem_object *shmem);
 void drm_gem_shmem_unpin_locked(struct drm_gem_shmem_object *shmem);
 

-- 
2.56.0

[PATCH RFC 04/12] drm/panfrost: Use PRIME helpers with custom dma_buf_ops

[Maíra Canal]

Panfrost duplicates the PRIME export and self-import logic only because
the core helpers hardcoded drm_gem_prime_dmabuf_ops. Use the new
drm_gem_prime_export_with_ops() and gem_prime_is_exporter() functions
instead.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/panfrost/panfrost_drv.c |  2 +-
 drivers/gpu/drm/panfrost/panfrost_gem.c | 28 ++++------------------------
 drivers/gpu/drm/panfrost/panfrost_gem.h |  5 ++---
 3 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_drv.c b/drivers/gpu/drm/panfrost/panfrost_drv.c
index 7c348a0bf1ee..d26f8ca52906 100644
--- a/drivers/gpu/drm/panfrost/panfrost_drv.c
+++ b/drivers/gpu/drm/panfrost/panfrost_drv.c
@@ -824,7 +824,7 @@ static const struct drm_driver panfrost_drm_driver = {
 	.minor			= 7,
 
 	.gem_create_object	= panfrost_gem_create_object,
-	.gem_prime_import	= panfrost_gem_prime_import,
+	.gem_prime_is_exporter	= panfrost_gem_prime_is_exporter,
 	.gem_prime_import_sg_table = panfrost_gem_prime_import_sg_table,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = panfrost_device_debugfs_init,
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 518f24549b9d..207110f33700 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -380,35 +380,15 @@ static const struct dma_buf_ops panfrost_dma_buf_ops = {
 static struct dma_buf *
 panfrost_gem_prime_export(struct drm_gem_object *obj, int flags)
 {
-	struct drm_device *dev = obj->dev;
-	struct dma_buf_export_info exp_info = {
-		.exp_name = KBUILD_MODNAME,
-		.owner = THIS_MODULE,
-		.ops = &panfrost_dma_buf_ops,
-		.size = obj->size,
-		.flags = flags,
-		.priv = obj,
-		.resv = obj->resv,
-	};
-
-	return drm_gem_dmabuf_export(dev, &exp_info);
+	return drm_gem_prime_export_with_ops(obj, &panfrost_dma_buf_ops, flags);
 }
 
-struct drm_gem_object *
-panfrost_gem_prime_import(struct drm_device *dev,
-			  struct dma_buf *dma_buf)
+bool panfrost_gem_prime_is_exporter(struct drm_device *dev,
+				    struct dma_buf *dma_buf)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
 
-	if (dma_buf->ops == &panfrost_dma_buf_ops && obj->dev == dev) {
-		/* Importing dmabuf exported from our own gem increases
-		 * refcount on gem itself instead of f_count of dmabuf.
-		 */
-		drm_gem_object_get(obj);
-		return obj;
-	}
-
-	return drm_gem_prime_import(dev, dma_buf);
+	return dma_buf->ops == &panfrost_dma_buf_ops && obj->dev == dev;
 }
 
 static const struct drm_gem_object_funcs panfrost_gem_funcs = {
diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.h b/drivers/gpu/drm/panfrost/panfrost_gem.h
index 3008afcf4bff..9d30c2b768d7 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.h
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.h
@@ -138,9 +138,8 @@ struct drm_gem_object *
 panfrost_gem_prime_import_sg_table(struct drm_device *dev,
 				   struct dma_buf_attachment *attach,
 				   struct sg_table *sgt);
-struct drm_gem_object *
-panfrost_gem_prime_import(struct drm_device *dev,
-			  struct dma_buf *dma_buf);
+bool panfrost_gem_prime_is_exporter(struct drm_device *dev,
+				    struct dma_buf *dma_buf);
 
 struct panfrost_gem_object *
 panfrost_gem_create(struct drm_device *dev, size_t size, u32 flags);

-- 
2.56.0

[PATCH RFC 05/12] drm/panfrost: Use common shmem helpers for dma_buf CPU access

[Maíra Canal]

Panfrost's dma_buf map/unmap and CPU access hooks are identical to the
generic shmem implementation. Use the common shmem helpers instead.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/panfrost/panfrost_gem.c | 83 ++-------------------------------
 1 file changed, 4 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 207110f33700..5f099c478edd 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -289,92 +289,17 @@ static size_t panfrost_gem_rss(struct drm_gem_object *obj)
 	return 0;
 }
 
-static struct sg_table *
-panfrost_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
-			       enum dma_data_direction dir)
-{
-	struct sg_table *sgt = drm_gem_map_dma_buf(attach, dir);
-
-	if (!IS_ERR(sgt))
-		attach->priv = sgt;
-
-	return sgt;
-}
-
-static void
-panfrost_gem_prime_unmap_dma_buf(struct dma_buf_attachment *attach,
-				 struct sg_table *sgt,
-				 enum dma_data_direction dir)
-{
-	attach->priv = NULL;
-	drm_gem_unmap_dma_buf(attach, sgt, dir);
-}
-
-static int
-panfrost_gem_prime_begin_cpu_access(struct dma_buf *dma_buf,
-				    enum dma_data_direction dir)
-{
-	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
-	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
-	struct dma_buf_attachment *attach;
-
-	dma_resv_lock(obj->resv, NULL);
-	if (shmem->sgt)
-		dma_sync_sgtable_for_cpu(dev->dev, shmem->sgt, dir);
-
-	if (shmem->vaddr)
-		invalidate_kernel_vmap_range(shmem->vaddr, shmem->base.size);
-
-	list_for_each_entry(attach, &dma_buf->attachments, node) {
-		struct sg_table *sgt = attach->priv;
-
-		if (sgt)
-			dma_sync_sgtable_for_cpu(attach->dev, sgt, dir);
-	}
-	dma_resv_unlock(obj->resv);
-
-	return 0;
-}
-
-static int
-panfrost_gem_prime_end_cpu_access(struct dma_buf *dma_buf,
-				  enum dma_data_direction dir)
-{
-	struct drm_gem_object *obj = dma_buf->priv;
-	struct drm_device *dev = obj->dev;
-	struct drm_gem_shmem_object *shmem = to_drm_gem_shmem_obj(obj);
-	struct dma_buf_attachment *attach;
-
-	dma_resv_lock(obj->resv, NULL);
-	list_for_each_entry(attach, &dma_buf->attachments, node) {
-		struct sg_table *sgt = attach->priv;
-
-		if (sgt)
-			dma_sync_sgtable_for_device(attach->dev, sgt, dir);
-	}
-
-	if (shmem->vaddr)
-		flush_kernel_vmap_range(shmem->vaddr, shmem->base.size);
-
-	if (shmem->sgt)
-		dma_sync_sgtable_for_device(dev->dev, shmem->sgt, dir);
-
-	dma_resv_unlock(obj->resv);
-	return 0;
-}
-
 static const struct dma_buf_ops panfrost_dma_buf_ops = {
 	.attach = drm_gem_map_attach,
 	.detach = drm_gem_map_detach,
-	.map_dma_buf = panfrost_gem_prime_map_dma_buf,
-	.unmap_dma_buf = panfrost_gem_prime_unmap_dma_buf,
+	.map_dma_buf = drm_gem_shmem_prime_map_dma_buf,
+	.unmap_dma_buf = drm_gem_shmem_prime_unmap_dma_buf,
 	.release = drm_gem_dmabuf_release,
 	.mmap = drm_gem_dmabuf_mmap,
 	.vmap = drm_gem_dmabuf_vmap,
 	.vunmap = drm_gem_dmabuf_vunmap,
-	.begin_cpu_access = panfrost_gem_prime_begin_cpu_access,
-	.end_cpu_access = panfrost_gem_prime_end_cpu_access,
+	.begin_cpu_access = drm_gem_shmem_prime_begin_cpu_access,
+	.end_cpu_access = drm_gem_shmem_prime_end_cpu_access,
 };
 
 static struct dma_buf *

-- 
2.56.0

[PATCH RFC 06/12] drm/panfrost: Use drm_gem_shmem_sync()

[Maíra Canal]

panfrost_gem_sync() has the same code as the function drm_gem_shmem_sync().
Translate the UAPI sync type to the common shmem type and use the shmem
helper instead.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/panfrost/panfrost_gem.c | 74 +++------------------------------
 1 file changed, 5 insertions(+), 69 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_gem.c b/drivers/gpu/drm/panfrost/panfrost_gem.c
index 5f099c478edd..80a4a1771ce7 100644
--- a/drivers/gpu/drm/panfrost/panfrost_gem.c
+++ b/drivers/gpu/drm/panfrost/panfrost_gem.c
@@ -460,84 +460,20 @@ int
 panfrost_gem_sync(struct drm_gem_object *obj, u32 type, u32 offset, u32 size)
 {
 	struct panfrost_gem_object *bo = to_panfrost_bo(obj);
-	struct drm_gem_shmem_object *shmem = &bo->base;
-	const struct drm_device *dev = shmem->base.dev;
-	struct sg_table *sgt;
-	struct scatterlist *sgl;
-	unsigned int count;
-
-	/* Make sure the range is in bounds. */
-	if (offset + size < offset || offset + size > shmem->base.size)
-		return -EINVAL;
-
-	/* Disallow CPU-cache maintenance on imported buffers. */
-	if (drm_gem_is_imported(&shmem->base))
-		return -EINVAL;
+	enum drm_gem_shmem_sync_type shmem_type;
 
 	switch (type) {
 	case PANFROST_BO_SYNC_CPU_CACHE_FLUSH:
+		shmem_type = DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH;
+		break;
 	case PANFROST_BO_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE:
+		shmem_type = DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE;
 		break;
-
 	default:
 		return -EINVAL;
 	}
 
-	/* Don't bother if it's WC-mapped */
-	if (shmem->map_wc)
-		return 0;
-
-	/* Nothing to do if the size is zero. */
-	if (size == 0)
-		return 0;
-
-	sgt = drm_gem_shmem_get_pages_sgt(shmem);
-	if (IS_ERR(sgt))
-		return PTR_ERR(sgt);
-
-	for_each_sgtable_dma_sg(sgt, sgl, count) {
-		if (size == 0)
-			break;
-
-		dma_addr_t paddr = sg_dma_address(sgl);
-		size_t len = sg_dma_len(sgl);
-
-		if (len <= offset) {
-			offset -= len;
-			continue;
-		}
-
-		paddr += offset;
-		len -= offset;
-		len = min_t(size_t, len, size);
-		size -= len;
-		offset = 0;
-
-		/* It's unclear whether dma_sync_xxx() is the right API to do CPU
-		 * cache maintenance given an IOMMU can register their own
-		 * implementation doing more than just CPU cache flushes/invalidation,
-		 * and what we really care about here is CPU caches only, but that's
-		 * the best we have that is both arch-agnostic and does at least the
-		 * CPU cache maintenance on a <page,offset,size> tuple.
-		 *
-		 * Also, I wish we could do a single
-		 *
-		 *      dma_sync_single_for_device(BIDIR)
-		 *
-		 * and get a flush+invalidate, but that's not how it's implemented
-		 * in practice (at least on arm64), so we have to make it
-		 *
-		 *      dma_sync_single_for_device(TO_DEVICE)
-		 *      dma_sync_single_for_cpu(FROM_DEVICE)
-		 *
-		 * for the flush+invalidate case.
-		 */
-		dma_sync_single_for_device(dev->dev, paddr, len, DMA_TO_DEVICE);
-		if (type == PANFROST_BO_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE)
-			dma_sync_single_for_cpu(dev->dev, paddr, len, DMA_FROM_DEVICE);
-	}
-
-	return 0;
+	return drm_gem_shmem_sync(&bo->base, offset, size, shmem_type);
 }
 
 void

-- 
2.56.0

[PATCH RFC 07/12] drm/panthor: Use PRIME helpers with custom dma_buf_ops

[Maíra Canal]

Panthor duplicates the PRIME export and self-import logic only because
the core helpers hardcoded drm_gem_prime_dmabuf_ops. Use the new
drm_gem_prime_export_with_ops() and gem_prime_is_exporter() functions
instead.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/panthor/panthor_drv.c |  2 +-
 drivers/gpu/drm/panthor/panthor_gem.c | 28 ++++------------------------
 drivers/gpu/drm/panthor/panthor_gem.h |  5 ++---
 3 files changed, 7 insertions(+), 28 deletions(-)

diff --git a/drivers/gpu/drm/panthor/panthor_drv.c b/drivers/gpu/drm/panthor/panthor_drv.c
index dc8d0191aef4..37048030afcb 100644
--- a/drivers/gpu/drm/panthor/panthor_drv.c
+++ b/drivers/gpu/drm/panthor/panthor_drv.c
@@ -1801,7 +1801,7 @@ static const struct drm_driver panthor_drm_driver = {
 	.minor = 9,
 
 	.gem_prime_import_sg_table = panthor_gem_prime_import_sg_table,
-	.gem_prime_import = panthor_gem_prime_import,
+	.gem_prime_is_exporter = panthor_gem_prime_is_exporter,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = panthor_debugfs_init,
 #endif
diff --git a/drivers/gpu/drm/panthor/panthor_gem.c b/drivers/gpu/drm/panthor/panthor_gem.c
index 22b57ab6f929..2b7e4d5f5f5c 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.c
+++ b/drivers/gpu/drm/panthor/panthor_gem.c
@@ -562,39 +562,19 @@ static const struct dma_buf_ops panthor_dma_buf_ops = {
 static struct dma_buf *
 panthor_gem_prime_export(struct drm_gem_object *obj, int flags)
 {
-	struct drm_device *dev = obj->dev;
-	struct dma_buf_export_info exp_info = {
-		.exp_name = KBUILD_MODNAME,
-		.owner = THIS_MODULE,
-		.ops = &panthor_dma_buf_ops,
-		.size = obj->size,
-		.flags = flags,
-		.priv = obj,
-		.resv = obj->resv,
-	};
-
 	/* We can't export GEMs that have an exclusive VM. */
 	if (to_panthor_bo(obj)->exclusive_vm_root_gem)
 		return ERR_PTR(-EINVAL);
 
-	return drm_gem_dmabuf_export(dev, &exp_info);
+	return drm_gem_prime_export_with_ops(obj, &panthor_dma_buf_ops, flags);
 }
 
-struct drm_gem_object *
-panthor_gem_prime_import(struct drm_device *dev,
-			 struct dma_buf *dma_buf)
+bool panthor_gem_prime_is_exporter(struct drm_device *dev,
+				   struct dma_buf *dma_buf)
 {
 	struct drm_gem_object *obj = dma_buf->priv;
 
-	if (dma_buf->ops == &panthor_dma_buf_ops && obj->dev == dev) {
-		/* Importing dmabuf exported from our own gem increases
-		 * refcount on gem itself instead of f_count of dmabuf.
-		 */
-		drm_gem_object_get(obj);
-		return obj;
-	}
-
-	return drm_gem_prime_import(dev, dma_buf);
+	return dma_buf->ops == &panthor_dma_buf_ops && obj->dev == dev;
 }
 
 static void panthor_gem_print_info(struct drm_printer *p, unsigned int indent,
diff --git a/drivers/gpu/drm/panthor/panthor_gem.h b/drivers/gpu/drm/panthor/panthor_gem.h
index 5ae37d0d3646..a68cd3c280ed 100644
--- a/drivers/gpu/drm/panthor/panthor_gem.h
+++ b/drivers/gpu/drm/panthor/panthor_gem.h
@@ -274,9 +274,8 @@ void panthor_gem_kernel_bo_set_label(struct panthor_kernel_bo *bo, const char *l
 int panthor_gem_sync(struct drm_gem_object *obj,
 		     u32 type, u64 offset, u64 size);
 
-struct drm_gem_object *
-panthor_gem_prime_import(struct drm_device *dev,
-			 struct dma_buf *dma_buf);
+bool panthor_gem_prime_is_exporter(struct drm_device *dev,
+				   struct dma_buf *dma_buf);
 
 static inline u64
 panthor_kernel_bo_gpuva(struct panthor_kernel_bo *bo)

-- 
2.56.0

[PATCH RFC 08/12] drm/amdgpu: Use drm_gem_prime_export_with_ops()

[Maíra Canal]

amdgpu exports with the default PRIME dma_buf_ops and replaces them
afterwards. Instead of open-code the replacement of the DMA-BUF
operations, use the new helper drm_gem_prime_export_with_ops() to export
amdgpu_dmabuf_ops directly.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index 4031ebe075d8..2e4a1d1b6322 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -407,7 +407,7 @@ const struct dma_buf_ops amdgpu_dmabuf_ops = {
  * @gobj: GEM BO
  * @flags: Flags such as DRM_CLOEXEC and DRM_RDWR.
  *
- * The main work is done by the &drm_gem_prime_export helper.
+ * The main work is done by drm_gem_prime_export_with_ops().
  *
  * Returns:
  * Shared DMA buffer representing the GEM BO from the given device.
@@ -416,7 +416,6 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj,
 					int flags)
 {
 	struct amdgpu_bo *bo = gem_to_amdgpu_bo(gobj);
-	struct dma_buf *buf;
 	struct ttm_operation_ctx ctx = {
 		.interruptible = true,
 		.no_wait_gpu = true,
@@ -434,11 +433,7 @@ struct dma_buf *amdgpu_gem_prime_export(struct drm_gem_object *gobj,
 	if (ret)
 		return ERR_PTR(ret);
 
-	buf = drm_gem_prime_export(gobj, flags);
-	if (!IS_ERR(buf))
-		buf->ops = &amdgpu_dmabuf_ops;
-
-	return buf;
+	return drm_gem_prime_export_with_ops(gobj, &amdgpu_dmabuf_ops, flags);
 }
 
 /**

-- 
2.56.0

[PATCH RFC 09/12] drm/v3d: Provide a custom dma_buf implementation

[Maíra Canal]

Before introducing cached CPU mappings, V3D needs a dma_buf implementation
that satisfies synchronization requests around CPU accesses coming from
a DMA-BUF exported by the driver. Use the generic shmem PRIME helpers
for that.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_bo.c  | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_drv.c |  1 +
 drivers/gpu/drm/v3d/v3d_drv.h |  1 +
 3 files changed, 28 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index db95a2fac5d0..803f3bbb7144 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -63,12 +63,38 @@ void v3d_free_object(struct drm_gem_object *obj)
 	drm_gem_shmem_free(&bo->base);
 }
 
+static const struct dma_buf_ops v3d_dma_buf_ops = {
+	.attach = drm_gem_map_attach,
+	.detach = drm_gem_map_detach,
+	.map_dma_buf = drm_gem_shmem_prime_map_dma_buf,
+	.unmap_dma_buf = drm_gem_shmem_prime_unmap_dma_buf,
+	.release = drm_gem_dmabuf_release,
+	.mmap = drm_gem_dmabuf_mmap,
+	.vmap = drm_gem_dmabuf_vmap,
+	.vunmap = drm_gem_dmabuf_vunmap,
+	.begin_cpu_access = drm_gem_shmem_prime_begin_cpu_access,
+	.end_cpu_access = drm_gem_shmem_prime_end_cpu_access,
+};
+
+static struct dma_buf *v3d_prime_export(struct drm_gem_object *obj, int flags)
+{
+	return drm_gem_prime_export_with_ops(obj, &v3d_dma_buf_ops, flags);
+}
+
+bool v3d_prime_is_exporter(struct drm_device *dev, struct dma_buf *dma_buf)
+{
+	struct drm_gem_object *obj = dma_buf->priv;
+
+	return dma_buf->ops == &v3d_dma_buf_ops && obj->dev == dev;
+}
+
 static const struct drm_gem_object_funcs v3d_gem_funcs = {
 	.free = v3d_free_object,
 	.print_info = drm_gem_shmem_object_print_info,
 	.pin = drm_gem_shmem_object_pin,
 	.unpin = drm_gem_shmem_object_unpin,
 	.get_sg_table = drm_gem_shmem_object_get_sg_table,
+	.export = v3d_prime_export,
 	.vmap = drm_gem_shmem_object_vmap,
 	.vunmap = drm_gem_shmem_object_vunmap,
 	.mmap = drm_gem_shmem_object_mmap,
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index b790be8a7fac..7d91c528c1e9 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -283,6 +283,7 @@ static const struct drm_driver v3d_drm_driver = {
 
 	.gem_create_object = v3d_create_object,
 	.gem_prime_import_sg_table = v3d_prime_import_sg_table,
+	.gem_prime_is_exporter = v3d_prime_is_exporter,
 
 	.ioctls = v3d_drm_ioctls,
 	.num_ioctls = ARRAY_SIZE(v3d_drm_ioctls),
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 7c7bdf6e51a5..e0eed4cfef61 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -606,6 +606,7 @@ int v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
 struct drm_gem_object *v3d_prime_import_sg_table(struct drm_device *dev,
 						 struct dma_buf_attachment *attach,
 						 struct sg_table *sgt);
+bool v3d_prime_is_exporter(struct drm_device *dev, struct dma_buf *dma_buf);
 
 /* v3d_debugfs.c */
 void v3d_debugfs_init(struct drm_minor *minor);

-- 
2.56.0

[PATCH RFC 10/12] drm/v3d: Add flag to map BOs write-back cacheable

[Maíra Canal]

All V3D BOs are mapped write-combined, which makes CPU reads slow.
Buffers frequently read by the CPU, or with access patterns that make
non-cacheable mappings inefficient, can benefit from cached mappings.

Add DRM_V3D_BO_WB_MMAP to let userspace request a cached mapping. As
V3D is not I/O coherent, userspace becomes responsible for CPU cache
maintenance around the job execution. Note that this also includes CPU
jobs, as the kernel accesses the BOs through write-combined mappings.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_bo.c  | 11 ++++++-----
 drivers/gpu/drm/v3d/v3d_drv.h |  3 +--
 drivers/gpu/drm/v3d/v3d_irq.c |  2 +-
 include/uapi/drm/v3d_drm.h    |  9 +++++++--
 4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 803f3bbb7144..0a5a056d4b26 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -173,18 +173,19 @@ v3d_bo_create_finish(struct drm_gem_object *obj)
 	return 0;
 }
 
-struct v3d_bo *v3d_bo_create(struct drm_device *dev, struct drm_file *file_priv,
-			     size_t unaligned_size)
+struct v3d_bo *v3d_bo_create(struct drm_device *dev, size_t size, u32 flags)
 {
 	struct drm_gem_shmem_object *shmem_obj;
 	struct v3d_bo *bo;
 	int ret;
 
-	shmem_obj = drm_gem_shmem_create(dev, unaligned_size);
+	shmem_obj = drm_gem_shmem_create(dev, size);
 	if (IS_ERR(shmem_obj))
 		return ERR_CAST(shmem_obj);
+
 	bo = to_v3d_bo(&shmem_obj->base);
 	bo->vaddr = NULL;
+	bo->base.map_wc = !(flags & DRM_V3D_BO_WB_MMAP);
 
 	ret = v3d_bo_create_finish(&shmem_obj->base);
 	if (ret)
@@ -239,12 +240,12 @@ int v3d_create_bo_ioctl(struct drm_device *dev, void *data,
 	struct v3d_bo *bo = NULL;
 	int ret;
 
-	if (args->flags != 0) {
+	if (args->flags & ~DRM_V3D_BO_WB_MMAP) {
 		drm_dbg(dev, "unknown create_bo flags: %d\n", args->flags);
 		return -EINVAL;
 	}
 
-	bo = v3d_bo_create(dev, file_priv, PAGE_ALIGN(args->size));
+	bo = v3d_bo_create(dev, PAGE_ALIGN(args->size), args->flags);
 	if (IS_ERR(bo))
 		return PTR_ERR(bo);
 
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index e0eed4cfef61..4f32fc3ab1de 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -591,8 +591,7 @@ struct v3d_submit_ext {
 /* v3d_bo.c */
 struct drm_gem_object *v3d_create_object(struct drm_device *dev, size_t size);
 void v3d_free_object(struct drm_gem_object *gem_obj);
-struct v3d_bo *v3d_bo_create(struct drm_device *dev, struct drm_file *file_priv,
-			     size_t size);
+struct v3d_bo *v3d_bo_create(struct drm_device *dev, size_t size, u32 flags);
 void v3d_get_bo_vaddr(struct v3d_bo *bo);
 void v3d_put_bo_vaddr(struct v3d_bo *bo);
 int v3d_create_bo_ioctl(struct drm_device *dev, void *data,
diff --git a/drivers/gpu/drm/v3d/v3d_irq.c b/drivers/gpu/drm/v3d/v3d_irq.c
index 963d711dc16b..19315acf567a 100644
--- a/drivers/gpu/drm/v3d/v3d_irq.c
+++ b/drivers/gpu/drm/v3d/v3d_irq.c
@@ -43,7 +43,7 @@ v3d_overflow_mem_work(struct work_struct *work)
 	struct v3d_dev *v3d =
 		container_of(work, struct v3d_dev, overflow_mem_work);
 	struct drm_device *dev = &v3d->drm;
-	struct v3d_bo *bo = v3d_bo_create(dev, NULL /* XXX: GMP */, 256 * 1024);
+	struct v3d_bo *bo = v3d_bo_create(dev, SZ_256K, 0);
 	struct v3d_queue_state *queue = &v3d->queue[V3D_BIN];
 	struct v3d_bin_job *bin_job;
 	struct drm_gem_object *obj;
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index 2750a6fbb6b8..4221c7544d08 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -237,11 +237,16 @@ struct drm_v3d_wait_bo {
 	__u64 timeout_ns;
 };
 
+/*
+ * Map the BO CPU-cached instead of write-combined. Userspace is responsible
+ * for CPU cache maintenance.
+ */
+#define DRM_V3D_BO_WB_MMAP			  0x01
+
 /**
  * struct drm_v3d_create_bo - ioctl argument for creating V3D BOs.
  *
- * There are currently no values for the flags argument, but it may be
- * used in a future extension.
+ * @flags is a mask of DRM_V3D_BO_* flags.
  */
 struct drm_v3d_create_bo {
 	__u32 size;

-- 
2.56.0

[PATCH RFC 11/12] drm/v3d: Add a DRM_IOCTL_V3D_SYNC_BO ioctl

[Maíra Canal]

BOs mapped with DRM_V3D_BO_WB_MMAP require CPU cache maintenance around
job execution. ARM64 userspace can do it through userspace cache
operations, but AArch32 cache maintenance is privileged, even on an ARM64
kernel, which means that 32-bit userspace can't do cache mainteinance.
Without a kernel interface, a 32-bit userspace wouldn't be able to use
cached BOs.

Add an ioctl taking an array of sync operations, so userspace can flush or
invalidate multiple BO ranges in a single call.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_bo.c  | 64 +++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/v3d/v3d_drv.c |  1 +
 drivers/gpu/drm/v3d/v3d_drv.h |  2 ++
 include/uapi/drm/v3d_drm.h    | 48 ++++++++++++++++++++++++++++++++
 4 files changed, 115 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_bo.c b/drivers/gpu/drm/v3d/v3d_bo.c
index 0a5a056d4b26..78eb3a1cc794 100644
--- a/drivers/gpu/drm/v3d/v3d_bo.c
+++ b/drivers/gpu/drm/v3d/v3d_bo.c
@@ -280,6 +280,70 @@ int v3d_mmap_bo_ioctl(struct drm_device *dev, void *data,
 	return 0;
 }
 
+static int v3d_bo_sync(struct drm_gem_object *obj,
+		       const struct drm_v3d_bo_sync_op *op)
+{
+	struct v3d_bo *bo = to_v3d_bo(obj);
+	enum drm_gem_shmem_sync_type type;
+
+	switch (op->type) {
+	case DRM_V3D_BO_SYNC_CPU_CACHE_FLUSH:
+		type = DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH;
+		break;
+	case DRM_V3D_BO_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE:
+		type = DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	return drm_gem_shmem_sync(&bo->base, op->offset, op->size, type);
+}
+
+int v3d_sync_bo_ioctl(struct drm_device *dev, void *data,
+		      struct drm_file *file_priv)
+{
+	struct drm_v3d_sync_bo *args = data;
+	struct drm_v3d_bo_sync_op *ops;
+	struct drm_gem_object *obj;
+	int ret = 0;
+
+	if (args->pad)
+		return -EINVAL;
+
+	if (!args->op_count)
+		return 0;
+
+	/* Limits the number of sync operations requested by userspace. */
+	if (args->op_count > SZ_64K) {
+		drm_dbg(dev, "Too many sync ops: %u\n", args->op_count);
+		return -EINVAL;
+	}
+
+	ops = vmemdup_array_user(u64_to_user_ptr(args->ops), args->op_count,
+				 sizeof(*ops));
+	if (IS_ERR(ops))
+		return PTR_ERR(ops);
+
+	for (u32 i = 0; i < args->op_count; i++) {
+		obj = drm_gem_object_lookup(file_priv, ops[i].handle);
+		if (!obj) {
+			drm_dbg(dev, "Failed to look up GEM BO %d\n",
+				ops[i].handle);
+			ret = -ENOENT;
+			break;
+		}
+
+		ret = v3d_bo_sync(obj, &ops[i]);
+		drm_gem_object_put(obj);
+		if (ret)
+			break;
+	}
+
+	kvfree(ops);
+	return ret;
+}
+
 int v3d_get_bo_offset_ioctl(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv)
 {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 7d91c528c1e9..9e6e4f832eee 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -267,6 +267,7 @@ static const struct drm_ioctl_desc v3d_drm_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(V3D_SUBMIT_CPU, v3d_submit_cpu_ioctl, DRM_RENDER_ALLOW | DRM_AUTH),
 	DRM_IOCTL_DEF_DRV(V3D_PERFMON_GET_COUNTER, v3d_perfmon_get_counter_ioctl, DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(V3D_PERFMON_SET_GLOBAL, v3d_perfmon_set_global_ioctl, DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(V3D_SYNC_BO, v3d_sync_bo_ioctl, DRM_RENDER_ALLOW),
 };
 
 static const struct drm_driver v3d_drm_driver = {
diff --git a/drivers/gpu/drm/v3d/v3d_drv.h b/drivers/gpu/drm/v3d/v3d_drv.h
index 4f32fc3ab1de..e5fb3a7b8d96 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.h
+++ b/drivers/gpu/drm/v3d/v3d_drv.h
@@ -602,6 +602,8 @@ int v3d_get_bo_offset_ioctl(struct drm_device *dev, void *data,
 			    struct drm_file *file_priv);
 int v3d_wait_bo_ioctl(struct drm_device *dev, void *data,
 		      struct drm_file *file_priv);
+int v3d_sync_bo_ioctl(struct drm_device *dev, void *data,
+		      struct drm_file *file_priv);
 struct drm_gem_object *v3d_prime_import_sg_table(struct drm_device *dev,
 						 struct dma_buf_attachment *attach,
 						 struct sg_table *sgt);
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index 4221c7544d08..c2f7f1409ae4 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -44,6 +44,7 @@ extern "C" {
 #define DRM_V3D_SUBMIT_CPU                        0x0b
 #define DRM_V3D_PERFMON_GET_COUNTER               0x0c
 #define DRM_V3D_PERFMON_SET_GLOBAL                0x0d
+#define DRM_V3D_SYNC_BO                           0x0e
 
 #define DRM_IOCTL_V3D_SUBMIT_CL           DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_SUBMIT_CL, struct drm_v3d_submit_cl)
 #define DRM_IOCTL_V3D_WAIT_BO             DRM_IOWR(DRM_COMMAND_BASE + DRM_V3D_WAIT_BO, struct drm_v3d_wait_bo)
@@ -64,6 +65,7 @@ extern "C" {
 						   struct drm_v3d_perfmon_get_counter)
 #define DRM_IOCTL_V3D_PERFMON_SET_GLOBAL  DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_PERFMON_SET_GLOBAL, \
 						   struct drm_v3d_perfmon_set_global)
+#define DRM_IOCTL_V3D_SYNC_BO             DRM_IOW(DRM_COMMAND_BASE + DRM_V3D_SYNC_BO, struct drm_v3d_sync_bo)
 
 #define DRM_V3D_SUBMIT_CL_FLUSH_CACHE             0x01
 #define DRM_V3D_SUBMIT_EXTENSION		  0x02
@@ -796,6 +798,52 @@ struct drm_v3d_perfmon_set_global {
 	__u32 id;
 };
 
+/* Valid types to pass to drm_v3d_bo_sync_op */
+#define DRM_V3D_BO_SYNC_CPU_CACHE_FLUSH                0
+#define DRM_V3D_BO_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE 1
+
+/**
+ * struct drm_v3d_bo_sync_op - BO synchronization operation
+ */
+struct drm_v3d_bo_sync_op {
+	/** @handle: Handle of the buffer object to sync. */
+	__u32 handle;
+
+	/** @type: Type of sync operation, DRM_V3D_BO_SYNC_*. */
+	__u32 type;
+
+	/**
+	 * @offset: Offset into the BO at which the sync range starts.
+	 *
+	 * This will be rounded down to the nearest cache line as needed.
+	 */
+	__u32 offset;
+
+	/**
+	 * @size: Size of the range to sync.
+	 *
+	 * @size + @offset will be rounded up to the nearest cache line as
+	 * needed.
+	 */
+	__u32 size;
+};
+
+/**
+ * struct drm_v3d_sync_bo - ioctl argument for CPU cache maintenance on BOs
+ *
+ * Operations on write-combined BOs are no-ops, and imported BOs are rejected.
+ */
+struct drm_v3d_sync_bo {
+	/** @ops: Pointer to an array of struct drm_v3d_bo_sync_op. */
+	__u64 ops;
+
+	/** @op_count: Number of elements in @ops. */
+	__u32 op_count;
+
+	/** @pad: MBZ. */
+	__u32 pad;
+};
+
 #if defined(__cplusplus)
 }
 #endif

-- 
2.56.0

[PATCH RFC 12/12] drm/v3d: Advertise support for write-back cacheable BOs

[Maíra Canal]

Advertise support for DRM_V3D_BO_WB_MMAP and DRM_IOCTL_V3D_SYNC_BO by
adding a new V3D parameter.

Signed-off-by: Maíra Canal <mcanal@igalia.com>
---
 drivers/gpu/drm/v3d/v3d_drv.c | 3 +++
 include/uapi/drm/v3d_drm.h    | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/v3d/v3d_drv.c b/drivers/gpu/drm/v3d/v3d_drv.c
index 9e6e4f832eee..e613af3ac81f 100644
--- a/drivers/gpu/drm/v3d/v3d_drv.c
+++ b/drivers/gpu/drm/v3d/v3d_drv.c
@@ -132,6 +132,9 @@ static int v3d_get_param_ioctl(struct drm_device *dev, void *data,
 	case DRM_V3D_PARAM_SUPPORTS_TFU_CCOL:
 		args->value = (v3d->ver >= V3D_GEN_71);
 		return 0;
+	case DRM_V3D_PARAM_SUPPORTS_WB_MMAP:
+		args->value = 1;
+		return 0;
 	default:
 		drm_dbg(dev, "Unknown parameter %d\n", args->param);
 		return -EINVAL;
diff --git a/include/uapi/drm/v3d_drm.h b/include/uapi/drm/v3d_drm.h
index c2f7f1409ae4..7035319766e8 100644
--- a/include/uapi/drm/v3d_drm.h
+++ b/include/uapi/drm/v3d_drm.h
@@ -304,6 +304,7 @@ enum drm_v3d_param {
 	DRM_V3D_PARAM_GLOBAL_RESET_COUNTER,
 	DRM_V3D_PARAM_CONTEXT_RESET_COUNTER,
 	DRM_V3D_PARAM_SUPPORTS_TFU_CCOL,
+	DRM_V3D_PARAM_SUPPORTS_WB_MMAP,
 };
 
 struct drm_v3d_get_param {

-- 
2.56.0

Re: [PATCH RFC 02/12] drm/shmem: Provide a generic {begin, end}_cpu_access() implementation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/shmem: wrong vmap flush order in drm_gem_shmem_prime_end_cpu_access()
--

commit 4543a541900e1a7c8880d3f04fbf76393b153371
Author: Boris Brezillon <boris.brezillon@collabora.com>

drm/shmem: Provide a generic {begin, end}_cpu_access() implementation

This commit adds default dma_buf_ops implementations for map_dma_buf,
unmap_dma_buf, begin_cpu_access, and end_cpu_access to drm_gem_shmem_helper
so that CPU caches and vmap ranges are synchronized across the exporter and
all mapped attachments.

[Severity: Medium]
In drivers/gpu/drm/drm_gem_shmem_helper.c, inside
drm_gem_shmem_prime_end_cpu_access() (around line 1093), should
flush_kernel_vmap_range(shmem->vaddr, shmem->base.size) be called before
iterating over dma_buf->attachments and calling
dma_sync_sgtable_for_device(attach->dev, sgt, dir)?

Currently, drm_gem_shmem_prime_end_cpu_access() syncs each attachment's
sg_table for the device first, then calls flush_kernel_vmap_range() on
shmem->vaddr, and finally calls dma_sync_sgtable_for_device() on shmem->sgt.

When a cacheable shmem GEM object (with map_wc set to false) is exported,
mapped by an importing device via drm_gem_shmem_prime_map_dma_buf(), and
also mapped into kernel virtual address space via drm_gem_shmem_vmap_locked(),
modifications made through shmem->vaddr may sit in a virtually indexed or
aliasing cache line (such as on 32-bit arm with vivt or vipt-aliasing caches
and an outer l2 cache, or mips with primary d-cache aliasing). Calling
dma_sync_sgtable_for_device() on the importer attachments before
flush_kernel_vmap_range() means the virtual alias in shmem->vaddr has not yet
been written back when dma_sync_sgtable_for_device() cleans the physical
mapping, cleans the outer cache, or copies to swiotlb bounce buffers, which
can leave importing devices reading stale data.

As documented in Documentation/core-api/cachetlb.rst and done in other
dma-buf exporters such as system_heap_dma_buf_end_cpu_access() and
cma_heap_dma_buf_end_cpu_access(), flush_kernel_vmap_range() should happen
before all dma_sync_sgtable_for_device() calls.

Similarly, in drm_gem_shmem_prime_begin_cpu_access() (around line 1058),
invalidate_kernel_vmap_range(shmem->vaddr, shmem->base.size) is placed
between syncing shmem->sgt and syncing dma_buf->attachments. Should
invalidate_kernel_vmap_range() be called after all
dma_sync_sgtable_for_cpu() calls have completed so that both exporter and
importer sg_table syncs are ordered consistently with respect to the kernel
vmap range invalidation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-v3d-cached-maps-v1-0-0a6a6b363382@igalia.com?part=2

Re: [PATCH RFC 03/12] drm/shmem: Add a drm_gem_shmem_sync() helper

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] drm/shmem: use-after-free in drm_gem_shmem_sync() due to unlocked sgt walk
- [Critical] drm/shmem: drm_gem_shmem_sync() calls dma_sync_single_* on coalesced IOVA sgl
- [High] drm/shmem: drm_gem_shmem_sync() allocates pages on unbacked or purged BOs
--

commit d4e8dcdfcd9df9ada386f6de7bfd50169a8e2266
Author: Faith Ekstrand <faith.ekstrand@collabora.com>

drm/shmem: Add a drm_gem_shmem_sync() helper

This commit adds a drm_gem_shmem_sync() helper to synchronize CPU-mapped
shmem GEM objects between the CPU and GPU via dma_sync_single_for_device()
and dma_sync_single_for_cpu().

> diff --git a/drivers/gpu/drm/drm_gem_shmem_helper.c b/drivers/gpu/drm/drm_gem_shmem_helper.c
> index 13c4a68fdde0e..9bf727fc1c197 100644
> --- a/drivers/gpu/drm/drm_gem_shmem_helper.c
> +++ b/drivers/gpu/drm/drm_gem_shmem_helper.c
> @@ -790,6 +790,99 @@ int drm_gem_shmem_mmap(struct drm_gem_shmem_object *shmem, struct vm_area_struct
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_shmem_mmap);
>  
> +/**
> + * drm_gem_shmem_sync - Sync CPU-mapped data
> + * @shmem: shmem GEM object
> + * @offset: Offset into the GEM object
> + * @size: Size of the area to sync
> + * @type: Type of synchronization
> + *
> + * Returns:
> + * 0 on success or a negative error code on failure.
> + */
> +int drm_gem_shmem_sync(struct drm_gem_shmem_object *shmem, size_t offset,
> +		       size_t size, enum drm_gem_shmem_sync_type type)
> +{
> +	const struct drm_device *dev = shmem->base.dev;
> +	struct sg_table *sgt;
> +	struct scatterlist *sgl;
> +	unsigned int count;
> +
> +	/* Make sure the range is in bounds. */
> +	if (offset + size < offset || offset + size > shmem->base.size)
> +		return -EINVAL;
> +
> +	/* Disallow CPU-cache maintenance on imported buffers. */
> +	if (drm_gem_is_imported(&shmem->base))
> +		return -EINVAL;
> +
> +	switch (type) {
> +	case DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH:
> +	case DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE:
> +		break;
> +
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	/* Don't bother if it's WC-mapped */
> +	if (shmem->map_wc)
> +		return 0;
> +
> +	/* Nothing to do if the size is zero. */
> +	if (size == 0)
> +		return 0;
> +
> +	sgt = drm_gem_shmem_get_pages_sgt(shmem);
> +	if (IS_ERR(sgt))
> +		return PTR_ERR(sgt);

[Severity: High]
Won't calling drm_gem_shmem_get_pages_sgt() here allocate backing pages and
create a DMA mapping even when the object has no backing pages yet
(!shmem->pages) or has already been purged (shmem->madv < 0)?

In drivers/gpu/drm/drm_gem_shmem_helper.c, drm_gem_shmem_get_pages_sgt()
calls drm_gem_shmem_get_pages_sgt_locked():

	if (shmem->sgt)
		return shmem->sgt;

	drm_WARN_ON(obj->dev, drm_gem_is_imported(obj));

	ret = drm_gem_shmem_get_pages_locked(shmem);

If drm_gem_shmem_sync() is called on an unbacked object, this populates
shmem->pages and shmem->sgt even though no CPU-mapped pages exist to sync.

More importantly, if the object was already purged via
drm_gem_shmem_purge_locked(), shmem->madv is -1 and shmem->sgt is NULL.
Calling drm_gem_shmem_get_pages_sgt() allocates fresh backing pages (or
increments pages_use_count on truncated pages if a VMA is still open) and
creates a new shmem->sgt while shmem->madv remains -1. Because
drm_gem_shmem_is_purgeable() in include/drm/drm_gem_shmem_helper.h checks
for shmem->madv > 0:

	return (shmem->madv > 0) &&
		!refcount_read(&shmem->pages_pin_count) && shmem->sgt &&
		!shmem->base.dma_buf && !drm_gem_is_imported(&shmem->base);

and drm_gem_shmem_madvise_locked() ignores updates when shmem->madv < 0,
those newly allocated pages and sg_table cannot be faulted in by userspace
and can never be reclaimed by the shrinker until the GEM object is
destroyed.

Should drm_gem_shmem_sync() check under shmem->base.resv whether the object
is purged or has no pages before attempting to sync?

> +
> +	for_each_sgtable_dma_sg(sgt, sgl, count) {

[Severity: Critical]
Can traversing sgt here race with drm_gem_shmem_purge_locked() and cause a
use-after-free?

In drivers/gpu/drm/drm_gem_shmem_helper.c, drm_gem_shmem_get_pages_sgt()
unlocks shmem->base.resv before returning sgt, and only increments
shmem->pages_use_count while leaving shmem->pages_pin_count at zero:

	ret = dma_resv_lock_interruptible(shmem->base.resv, NULL);
	if (ret)
		return ERR_PTR(ret);
	sgt = drm_gem_shmem_get_pages_sgt_locked(shmem);
	dma_resv_unlock(shmem->base.resv);

	return sgt;

For non-imported shmem objects, shmem->sgt and shmem->pages are protected by
shmem->base.resv. If userspace marks the buffer purgeable (shmem->madv > 0)
while shmem->pages_pin_count is zero, a concurrent shrinker thread calling
drm_gem_shmem_purge_locked() (or a caller invoking
__drm_gem_shmem_free_sgt_locked()) can acquire shmem->base.resv right after
drm_gem_shmem_get_pages_sgt() unlocks it:

	dma_unmap_sgtable(dev->dev, shmem->sgt, DMA_BIDIRECTIONAL, 0);
	sg_free_table(shmem->sgt);
	kfree(shmem->sgt);
	shmem->sgt = NULL;

	drm_gem_shmem_put_pages_locked(shmem);

That unmaps and frees shmem->sgt and releases the backing pages while
drm_gem_shmem_sync() is walking sgt and running DMA cache maintenance in
for_each_sgtable_dma_sg().

Should drm_gem_shmem_sync() hold shmem->base.resv (using
drm_gem_shmem_get_pages_sgt_locked()) across the sg_table walk?

> +		if (size == 0)
> +			break;
> +
> +		dma_addr_t paddr = sg_dma_address(sgl);
> +		size_t len = sg_dma_len(sgl);
> +
> +		if (len <= offset) {
> +			offset -= len;
> +			continue;
> +		}
> +
> +		paddr += offset;
> +		len -= offset;
> +		len = min_t(size_t, len, size);
> +		size -= len;
> +		offset = 0;
> +
> +		/* It's unclear whether dma_sync_xxx() is the right API to do CPU
> +		 * cache maintenance given an IOMMU can register their own
> +		 * implementation doing more than just CPU cache flushes/invalidation,
> +		 * and what we really care about here is CPU caches only, but that's
> +		 * the best we have that is both arch-agnostic and does at least the
> +		 * CPU cache maintenance on a <page,offset,size> tuple.
> +		 *
> +		 * Also, I wish we could do a single
> +		 *
> +		 *	dma_sync_single_for_device(BIDIR)
> +		 *
> +		 * and get a flush+invalidate, but that's not how it's implemented
> +		 * in practice (at least on arm64), so we have to make it
> +		 *
> +		 *	dma_sync_single_for_device(TO_DEVICE)
> +		 *	dma_sync_single_for_cpu(FROM_DEVICE)
> +		 *
> +		 * for the flush+invalidate case.
> +		 */
> +		dma_sync_single_for_device(dev->dev, paddr, len, DMA_TO_DEVICE);
> +		if (type == DRM_GEM_SHMEM_SYNC_CPU_CACHE_FLUSH_AND_INVALIDATE)
> +			dma_sync_single_for_cpu(dev->dev, paddr, len, DMA_FROM_DEVICE);

[Severity: Critical]
Is it safe to pass coalesced scatterlist DMA segments (sg_dma_address(sgl)
and sg_dma_len(sgl)) to dma_sync_single_for_device() and
dma_sync_single_for_cpu() when the device is behind an IOMMU?

In drm_gem_shmem_get_pages_sgt_locked(), shmem->sgt is mapped with
dma_map_sgtable(). When dev->dev uses an IOMMU DMA domain, iommu_dma_map_sg()
coalesces physically discontiguous scatterlist entries (sgt->orig_nents) into
virtually contiguous IOVA segments (sgt->nents), so a single sg_dma_len(sgl)
entry can span multiple discontiguous physical pages.

However, in drivers/iommu/dma-iommu.c, iommu_dma_sync_single_for_cpu() and
iommu_dma_sync_single_for_device() expect a physically contiguous buffer (as
mapped by dma_map_single() or dma_map_page()) and only translate the starting
IOVA:

	phys = iommu_iova_to_phys(iommu_get_dma_domain(dev), dma_handle);
	if (!dev_is_dma_coherent(dev)) {
		arch_sync_dma_for_cpu(phys, size, dir);
		arch_sync_dma_flush();
	}

On a non-coherent device with an IOMMU, calling dma_sync_single_for_device()
and dma_sync_single_for_cpu() across a coalesced sg_dma_len(sgl) range will
perform CPU cache maintenance on [phys, phys + len) starting from the first
page's physical address, leaving the subsequent pages of the GEM object
unsynced while flushing or invalidating unrelated physical memory adjacent
to the first page.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030-v3d-cached-maps-v1-0-0a6a6b363382@igalia.com?part=3