mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs
@ 2026-10-19  7:14 Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS Karl Mehltretter
                   ` (4 more replies)
  0 siblings, 5 replies; 8+ messages in thread
From: Karl Mehltretter @ 2026-10-19  7:14 UTC (permalink / raw)
  To: linux-media
  Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab,
	Laurent Pinchart, Hans de Goede, Hans Verkuil, Benjamin Gaignard,
	Tomasz Stanislawski, Sumit Semwal, Christian König,
	dri-devel, linaro-mm-sig, linux-kernel, Karl Mehltretter

UVC and vivid access imported DMA-BUFs with the CPU without cache
synchronization. On a Pi 400, captured data remained in the CPU cache
while physical RAM was stale.

Add an opt-in queue flag to bracket CPU access from buf_prepare() through
buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
drivers pass buffers to hardware. Patch 1 balances finish operations in
VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.

This follows the per-queue flag proposed here:
https://lists.openwall.net/linux-kernel/2024/06/20/236

Based on Linux 7.3. Tests cover the same code on these earlier bases:

- Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
  imported CMA DMA-BUFs with a DRM device mapping:

  Path                          Runs   Stale frames
  UVC, vb2-vmalloc               12        0/96
  vivid, vb2-vmalloc             12        0/96
  vivid, vb2-dma-contig          12        0/96

  All six lifecycle cases passed with both vivid allocators. Read-only
  output USERPTR passed. The test added no warnings or failed CPU-access
  callbacks. A control without the DRM mapping found 4/16 stale frames.

- SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
  PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
  cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
  begin/end calls. The baseline was already coherent.

- BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
  six traced lifecycle cases and 60 repeats passed with balanced callbacks
  and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
  captured 96 UVC frames without stale data, with 100/100 begin/end calls.
  Its baseline was already coherent.

Karl Mehltretter (4):
  media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
  media: videobuf2: add opt-in DMA-BUF CPU access
  media: uvcvideo: synchronize imported DMA-BUFs for CPU access
  media: vivid: synchronize imported DMA-BUFs for CPU access

 .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
 .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
 .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
 .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
 drivers/media/test-drivers/vivid/vivid-core.c |  1 +
 drivers/media/usb/uvc/uvc_queue.c             |  1 +
 include/media/videobuf2-core.h                | 12 ++-
 7 files changed, 116 insertions(+), 9 deletions(-)


base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
  2026-10-19  7:14 [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Karl Mehltretter
@ 2026-10-19  7:14 ` Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access Karl Mehltretter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Karl Mehltretter @ 2026-10-19  7:14 UTC (permalink / raw)
  To: linux-media
  Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab,
	Laurent Pinchart, Hans de Goede, Hans Verkuil, Benjamin Gaignard,
	Tomasz Stanislawski, Sumit Semwal, Christian König,
	dri-devel, linaro-mm-sig, linux-kernel, Karl Mehltretter

VIDIOC_PREPARE_BUF leaves a buffer DEQUEUED after calling buf_prepare()
and the prepare() memory operation. VIDIOC_REMOVE_BUFS accepts that state
but frees the buffer without calling the matching finish operations.

With VIDEO_ADV_DEBUG, vivid reports:

  unbalanced counters for queue 000000009fa716f8, buffer 0:
       buf_prepare: 1 buf_finish: 0
       prepare: 1 finish: 0

Finish prepared buffers before freeing them, as queue cancellation does.

Fixes: a3293a85381e ("media: v4l2: Add REMOVE_BUFS ioctl")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/media/common/videobuf2/videobuf2-core.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index b0a6084f17573..460eaf4e82f78 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -1746,6 +1746,19 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
 			goto unlock;
 		}
 	}
+
+	/* A buffer can be prepared without being queued. Finish it first. */
+	for (i = start; i < start + count; i++) {
+		struct vb2_buffer *vb = vb2_get_buffer(q, i);
+
+		if (!vb->prepared)
+			continue;
+
+		__vb2_buf_mem_finish(vb);
+		call_void_vb_qop(vb, buf_finish, vb);
+		vb->prepared = 0;
+	}
+
 	__vb2_queue_free(q, start, count);
 	dprintk(q, 2, "%u buffers removed\n", count);
 
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access
  2026-10-19  7:14 [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS Karl Mehltretter
@ 2026-10-19  7:14 ` Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 3/4] media: uvcvideo: synchronize imported DMA-BUFs for " Karl Mehltretter
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 8+ messages in thread
From: Karl Mehltretter @ 2026-10-19  7:14 UTC (permalink / raw)
  To: linux-media
  Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab,
	Laurent Pinchart, Hans de Goede, Hans Verkuil, Benjamin Gaignard,
	Tomasz Stanislawski, Sumit Semwal, Christian König,
	dri-devel, linaro-mm-sig, linux-kernel, Karl Mehltretter

Drivers must bracket CPU access to imported DMA-BUFs with
dma_buf_begin_cpu_access() and dma_buf_end_cpu_access().

Add a queue flag to do so from buf_prepare() through buf_finish(). Use
read access for output and write access for capture. Map imports
bidirectionally because CPU and device access directions are opposite.
Pass the mapping direction to attach_dmabuf() so USERPTR and MMAP keep
the queue's normal direction.

Keep this opt-in because some vb2-vmalloc drivers pass buffers to
hardware. End access in process context because DMA-BUF callbacks may
sleep.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 83 ++++++++++++++++++-
 .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
 .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
 .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
 include/media/videobuf2-core.h                | 12 ++-
 5 files changed, 101 insertions(+), 9 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index 460eaf4e82f78..d2a78cd0a86bb 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -371,6 +371,65 @@ static void __vb2_buf_mem_finish(struct vb2_buffer *vb)
 		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 }
 
+/* Output buffers are read by the driver; capture buffers are written. */
+static enum dma_data_direction vb2_cpu_access_dir(const struct vb2_queue *q)
+{
+	return q->is_output ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+}
+
+static bool __vb2_dmabuf_is_duplicate(const struct vb2_buffer *vb,
+				      unsigned int plane)
+{
+	unsigned int i;
+
+	for (i = 0; i < plane; i++)
+		if (vb->planes[i].dbuf == vb->planes[plane].dbuf)
+			return true;
+
+	return false;
+}
+
+static void __vb2_buf_end_cpu_access(struct vb2_buffer *vb,
+				     unsigned int num_planes)
+{
+	struct vb2_queue *q = vb->vb2_queue;
+	enum dma_data_direction dir = vb2_cpu_access_dir(q);
+	unsigned int plane;
+
+	for (plane = 0; plane < num_planes; plane++) {
+		int ret;
+
+		if (__vb2_dmabuf_is_duplicate(vb, plane))
+			continue;
+
+		ret = dma_buf_end_cpu_access(vb->planes[plane].dbuf, dir);
+		if (ret)
+			pr_warn_ratelimited("[%s] failed to end CPU access to dmabuf plane %u: %d\n",
+					    q->name, plane, ret);
+	}
+}
+
+static int __vb2_buf_begin_cpu_access(struct vb2_buffer *vb)
+{
+	struct vb2_queue *q = vb->vb2_queue;
+	enum dma_data_direction dir = vb2_cpu_access_dir(q);
+	unsigned int plane;
+	int ret;
+
+	for (plane = 0; plane < vb->num_planes; plane++) {
+		if (__vb2_dmabuf_is_duplicate(vb, plane))
+			continue;
+
+		ret = dma_buf_begin_cpu_access(vb->planes[plane].dbuf, dir);
+		if (ret) {
+			__vb2_buf_end_cpu_access(vb, plane);
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
 /*
  * __setup_offsets() - setup unique offsets ("cookies") for every plane in
  * the buffer.
@@ -1423,6 +1482,9 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 	}
 
 	if (reacquired) {
+		enum dma_data_direction attach_dir = q->uses_cpu_access ?
+			DMA_BIDIRECTIONAL : q->dma_dir;
+
 		if (vb->planes[0].mem_priv) {
 			vb->copied_timestamp = 0;
 			call_void_vb_qop(vb, buf_cleanup, vb);
@@ -1453,7 +1515,8 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 						  vb,
 						  q->alloc_devs[plane] ? : q->dev,
 						  planes[plane].dbuf,
-						  planes[plane].length);
+						  planes[plane].length,
+						  attach_dir);
 			if (IS_ERR(mem_priv)) {
 				dprintk(q, 1, "failed to attach dmabuf\n");
 				ret = PTR_ERR(mem_priv);
@@ -1504,9 +1567,21 @@ static int __prepare_dmabuf(struct vb2_buffer *vb)
 		}
 	}
 
+	if (q->uses_cpu_access) {
+		ret = __vb2_buf_begin_cpu_access(vb);
+		if (ret) {
+			dprintk(q, 1, "failed to begin CPU access to dmabuf: %d\n",
+				ret);
+			call_void_vb_qop(vb, buf_cleanup, vb);
+			goto err_put_vb2_buf;
+		}
+	}
+
 	ret = call_vb_qop(vb, buf_prepare, vb);
 	if (ret) {
 		dprintk(q, 1, "buffer preparation failed\n");
+		if (q->uses_cpu_access)
+			__vb2_buf_end_cpu_access(vb, vb->num_planes);
 		call_void_vb_qop(vb, buf_cleanup, vb);
 		goto err_put_vb2_buf;
 	}
@@ -1756,6 +1831,8 @@ int vb2_core_remove_bufs(struct vb2_queue *q, unsigned int start, unsigned int c
 
 		__vb2_buf_mem_finish(vb);
 		call_void_vb_qop(vb, buf_finish, vb);
+		if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
+			__vb2_buf_end_cpu_access(vb, vb->num_planes);
 		vb->prepared = 0;
 	}
 
@@ -2160,6 +2237,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	}
 
 	call_void_vb_qop(vb, buf_finish, vb);
+	if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
+		__vb2_buf_end_cpu_access(vb, vb->num_planes);
 	vb->prepared = 0;
 
 	if (pindex)
@@ -2296,6 +2375,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 
 		if (vb->prepared) {
 			call_void_vb_qop(vb, buf_finish, vb);
+			if (q->memory == VB2_MEMORY_DMABUF && q->uses_cpu_access)
+				__vb2_buf_end_cpu_access(vb, vb->num_planes);
 			vb->prepared = 0;
 		}
 		__vb2_dqbuf(vb);
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-contig.c b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
index 9ce6284cd5f26..68c146c043fb1 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-contig.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-contig.c
@@ -768,7 +768,8 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
-				  struct dma_buf *dbuf, unsigned long size)
+				  struct dma_buf *dbuf, unsigned long size,
+				  enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_buf *buf;
 	struct dma_buf_attachment *dba;
@@ -794,7 +795,7 @@ static void *vb2_dc_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
 		return dba;
 	}
 
-	buf->dma_dir = vb->vb2_queue->dma_dir;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 
diff --git a/drivers/media/common/videobuf2/videobuf2-dma-sg.c b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
index b1d0695cda260..ab03f9d31391a 100644
--- a/drivers/media/common/videobuf2/videobuf2-dma-sg.c
+++ b/drivers/media/common/videobuf2/videobuf2-dma-sg.c
@@ -616,7 +616,8 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
-				      struct dma_buf *dbuf, unsigned long size)
+				      struct dma_buf *dbuf, unsigned long size,
+				      enum dma_data_direction dma_dir)
 {
 	struct vb2_dma_sg_buf *buf;
 	struct dma_buf_attachment *dba;
@@ -640,7 +641,7 @@ static void *vb2_dma_sg_attach_dmabuf(struct vb2_buffer *vb, struct device *dev,
 		return dba;
 	}
 
-	buf->dma_dir = vb->vb2_queue->dma_dir;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 	buf->vb = vb;
diff --git a/drivers/media/common/videobuf2/videobuf2-vmalloc.c b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
index 63f023696f3ee..c38faa0188bc9 100644
--- a/drivers/media/common/videobuf2/videobuf2-vmalloc.c
+++ b/drivers/media/common/videobuf2/videobuf2-vmalloc.c
@@ -402,7 +402,8 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
 static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
 				       struct device *dev,
 				       struct dma_buf *dbuf,
-				       unsigned long size)
+				       unsigned long size,
+				       enum dma_data_direction dma_dir)
 {
 	struct vb2_vmalloc_buf *buf;
 
@@ -414,7 +415,7 @@ static void *vb2_vmalloc_attach_dmabuf(struct vb2_buffer *vb,
 		return ERR_PTR(-ENOMEM);
 
 	buf->dbuf = dbuf;
-	buf->dma_dir = vb->vb2_queue->dma_dir;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 
 	return buf;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 4b4f4c15c53a7..8dfe22fa4423c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -78,7 +78,8 @@ struct vb2_buffer;
  *		to the userspace, also optional.
  * @attach_dmabuf: attach a shared &struct dma_buf for a hardware operation;
  *		   used for DMABUF memory types; dev is the alloc device
- *		   dbuf is the shared dma_buf; returns ERR_PTR() on failure;
+ *		   dbuf is the shared dma_buf; dma_dir is the DMA mapping
+ *		   direction; returns ERR_PTR() on failure;
  *		   allocator private per-buffer structure on success;
  *		   this needs to be used for further accesses to the buffer.
  * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF
@@ -136,7 +137,8 @@ struct vb2_mem_ops {
 	void		*(*attach_dmabuf)(struct vb2_buffer *vb,
 					  struct device *dev,
 					  struct dma_buf *dbuf,
-					  unsigned long size);
+					  unsigned long size,
+					  enum dma_data_direction dma_dir);
 	void		(*detach_dmabuf)(void *buf_priv);
 	int		(*map_dmabuf)(void *buf_priv);
 	void		(*unmap_dmabuf)(void *buf_priv);
@@ -510,6 +512,11 @@ struct vb2_buf_ops {
  *		->finish().
  * @non_coherent_mem: when set queue will attempt to allocate buffers using
  *		non-coherent memory.
+ * @uses_cpu_access: the driver transfers buffer data with the CPU instead of
+ *		DMA. For imported dma-bufs, videobuf2 brackets the interval from
+ *		buf_prepare through buf_finish with dma_buf_begin_cpu_access()
+ *		and dma_buf_end_cpu_access(). Imported attachments use
+ *		bidirectional DMA mappings.
  * @lock:	pointer to a mutex that protects the &struct vb2_queue. The
  *		driver must set this to a mutex to let the v4l2 core serialize
  *		the queuing ioctls. This lock is used when waiting for a new
@@ -614,6 +621,7 @@ struct vb2_queue {
 	unsigned int			uses_requests:1;
 	unsigned int			allow_cache_hints:1;
 	unsigned int			non_coherent_mem:1;
+	unsigned int			uses_cpu_access:1;
 
 	struct mutex			*lock;
 	void				*owner;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/4] media: uvcvideo: synchronize imported DMA-BUFs for CPU access
  2026-10-19  7:14 [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access Karl Mehltretter
@ 2026-10-19  7:14 ` Karl Mehltretter
  2026-10-19  7:14 ` [PATCH 4/4] media: vivid: " Karl Mehltretter
  2026-10-19 12:44 ` [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Laurent Pinchart
  4 siblings, 0 replies; 8+ messages in thread
From: Karl Mehltretter @ 2026-10-19  7:14 UTC (permalink / raw)
  To: linux-media
  Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab,
	Laurent Pinchart, Hans de Goede, Hans Verkuil, Benjamin Gaignard,
	Tomasz Stanislawski, Sumit Semwal, Christian König,
	dri-devel, linaro-mm-sig, linux-kernel, Karl Mehltretter

uvcvideo writes USB payloads into imported DMA-BUFs with the CPU without
cache synchronization, leaving physical RAM stale.

Set uses_cpu_access to bracket the driver's CPU access.

On a Pi 400, 12 eight-frame captures each had stale data before this
change. All 96 frames matched RAM with it.

Fixes: ccc4c539c05f ("[media] v4l: uvc: add support for DMABUF importing")
Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/media/usb/uvc/uvc_queue.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 3c002c8f442fb..1f92ee96e0710 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -247,6 +247,7 @@ int uvc_queue_init(struct uvc_streaming *stream, struct uvc_video_queue *queue,
 	queue->queue.drv_priv = queue;
 	queue->queue.buf_struct_size = sizeof(struct uvc_buffer);
 	queue->queue.mem_ops = &vb2_vmalloc_memops;
+	queue->queue.uses_cpu_access = true;
 	queue->queue.timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC
 		| V4L2_BUF_FLAG_TSTAMP_SRC_SOE;
 	queue->queue.lock = &queue->mutex;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/4] media: vivid: synchronize imported DMA-BUFs for CPU access
  2026-10-19  7:14 [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Karl Mehltretter
                   ` (2 preceding siblings ...)
  2026-10-19  7:14 ` [PATCH 3/4] media: uvcvideo: synchronize imported DMA-BUFs for " Karl Mehltretter
@ 2026-10-19  7:14 ` Karl Mehltretter
  2026-10-19 12:44 ` [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Laurent Pinchart
  4 siblings, 0 replies; 8+ messages in thread
From: Karl Mehltretter @ 2026-10-19  7:14 UTC (permalink / raw)
  To: linux-media
  Cc: Tomasz Figa, Marek Szyprowski, Mauro Carvalho Chehab,
	Laurent Pinchart, Hans de Goede, Hans Verkuil, Benjamin Gaignard,
	Tomasz Stanislawski, Sumit Semwal, Christian König,
	dri-devel, linaro-mm-sig, linux-kernel, Karl Mehltretter

vivid accesses imported DMA-BUFs with the CPU without cache
synchronization, leaving physical RAM stale after capture.

Set uses_cpu_access to bracket the driver's CPU access.

On a Pi 400, stale frames fell from 25/96 to 0/96 with this change.

Assisted-by: LLM
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
---
 drivers/media/test-drivers/vivid/vivid-core.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/test-drivers/vivid/vivid-core.c b/drivers/media/test-drivers/vivid/vivid-core.c
index 62cfb5feb2cf5..e3aa1fc3bcf75 100644
--- a/drivers/media/test-drivers/vivid/vivid-core.c
+++ b/drivers/media/test-drivers/vivid/vivid-core.c
@@ -925,6 +925,7 @@ static int vivid_create_queue(struct vivid_dev *dev,
 	q->ops = ops;
 	q->mem_ops = allocators[dev->inst] == 1 ? &vb2_dma_contig_memops :
 						  &vb2_vmalloc_memops;
+	q->uses_cpu_access = true;
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
 	q->min_reqbufs_allocation = min_reqbufs_allocation;
 	q->lock = &dev->mutex;
-- 
2.47.3


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs
  2026-10-19  7:14 [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Karl Mehltretter
                   ` (3 preceding siblings ...)
  2026-10-19  7:14 ` [PATCH 4/4] media: vivid: " Karl Mehltretter
@ 2026-10-19 12:44 ` Laurent Pinchart
  2026-10-19 14:48   ` Christian König
  4 siblings, 1 reply; 8+ messages in thread
From: Laurent Pinchart @ 2026-10-19 12:44 UTC (permalink / raw)
  To: Karl Mehltretter
  Cc: linux-media, Tomasz Figa, Marek Szyprowski,
	Mauro Carvalho Chehab, Hans de Goede, Hans Verkuil,
	Benjamin Gaignard, Tomasz Stanislawski, Sumit Semwal,
	Christian König, dri-devel, linaro-mm-sig, linux-kernel

On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
> UVC and vivid access imported DMA-BUFs with the CPU without cache
> synchronization. On a Pi 400, captured data remained in the CPU cache
> while physical RAM was stale.
> 
> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
> drivers pass buffers to hardware. Patch 1 balances finish operations in
> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.

This seems completely wrong.

> This follows the per-queue flag proposed here:
> https://lists.openwall.net/linux-kernel/2024/06/20/236
> 
> Based on Linux 7.3. Tests cover the same code on these earlier bases:
> 
> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
>   imported CMA DMA-BUFs with a DRM device mapping:
> 
>   Path                          Runs   Stale frames
>   UVC, vb2-vmalloc               12        0/96
>   vivid, vb2-vmalloc             12        0/96
>   vivid, vb2-dma-contig          12        0/96
> 
>   All six lifecycle cases passed with both vivid allocators. Read-only
>   output USERPTR passed. The test added no warnings or failed CPU-access
>   callbacks. A control without the DRM mapping found 4/16 stale frames.
> 
> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
>   begin/end calls. The baseline was already coherent.
> 
> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
>   Its baseline was already coherent.
> 
> Karl Mehltretter (4):
>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
>   media: videobuf2: add opt-in DMA-BUF CPU access
>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
>   media: vivid: synchronize imported DMA-BUFs for CPU access
> 
>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
>  include/media/videobuf2-core.h                | 12 ++-
>  7 files changed, 116 insertions(+), 9 deletions(-)
> 
> 
> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs
  2026-10-19 12:44 ` [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Laurent Pinchart
@ 2026-10-19 14:48   ` Christian König
  2026-10-19 15:01     ` Laurent Pinchart
  0 siblings, 1 reply; 8+ messages in thread
From: Christian König @ 2026-10-19 14:48 UTC (permalink / raw)
  To: Laurent Pinchart, Karl Mehltretter
  Cc: linux-media, Tomasz Figa, Marek Szyprowski,
	Mauro Carvalho Chehab, Hans de Goede, Hans Verkuil,
	Benjamin Gaignard, Tomasz Stanislawski, Sumit Semwal, dri-devel,
	linaro-mm-sig, linux-kernel

On 10/19/26 14:44, Laurent Pinchart wrote:
> On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
>> UVC and vivid access imported DMA-BUFs with the CPU without cache
>> synchronization. On a Pi 400, captured data remained in the CPU cache
>> while physical RAM was stale.
>>
>> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
>> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
>> drivers pass buffers to hardware. Patch 1 balances finish operations in
>> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.
> 
> This seems completely wrong.

It looks a bit questionable from 1000 miles above, but I might be wrong.

Let me iterate the rules here:

Accessing imported DMA-buf with the CPU from userspace should only be possible by using the dma_buf_mmap() interface and that includes the DMA_BUF_IOCTL_SYNC IOCTL to flush buffers dirtied by the CPU. If that is not done this way then that would be a *major* security issue, but I guess that this isn't the issue here.

What the drivers do more likely is that they use the vmap interface to access the DMA-buf with the CPU inside the kernel. Correct?

In this case dma_buf_begin_cpu_access() and dma_buf_end_cpu_access() should be called to properly frame that access.

That seems to be added in patch #2, but I absolutely can't judge if that is correct or not since at the same time you speak about userptr access and the code really looks a bit like spaghetti. 

Regards,
Christian.

> 
>> This follows the per-queue flag proposed here:
>> https://lists.openwall.net/linux-kernel/2024/06/20/236
>>
>> Based on Linux 7.3. Tests cover the same code on these earlier bases:
>>
>> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
>>   imported CMA DMA-BUFs with a DRM device mapping:
>>
>>   Path                          Runs   Stale frames
>>   UVC, vb2-vmalloc               12        0/96
>>   vivid, vb2-vmalloc             12        0/96
>>   vivid, vb2-dma-contig          12        0/96
>>
>>   All six lifecycle cases passed with both vivid allocators. Read-only
>>   output USERPTR passed. The test added no warnings or failed CPU-access
>>   callbacks. A control without the DRM mapping found 4/16 stale frames.
>>
>> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
>>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
>>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
>>   begin/end calls. The baseline was already coherent.
>>
>> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
>>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
>>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
>>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
>>   Its baseline was already coherent.
>>
>> Karl Mehltretter (4):
>>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
>>   media: videobuf2: add opt-in DMA-BUF CPU access
>>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
>>   media: vivid: synchronize imported DMA-BUFs for CPU access
>>
>>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
>>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
>>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
>>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
>>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
>>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
>>  include/media/videobuf2-core.h                | 12 ++-
>>  7 files changed, 116 insertions(+), 9 deletions(-)
>>
>>
>> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs
  2026-10-19 14:48   ` Christian König
@ 2026-10-19 15:01     ` Laurent Pinchart
  0 siblings, 0 replies; 8+ messages in thread
From: Laurent Pinchart @ 2026-10-19 15:01 UTC (permalink / raw)
  To: Christian König
  Cc: Karl Mehltretter, linux-media, Tomasz Figa, Marek Szyprowski,
	Mauro Carvalho Chehab, Hans de Goede, Hans Verkuil,
	Benjamin Gaignard, Tomasz Stanislawski, Sumit Semwal, dri-devel,
	linaro-mm-sig, linux-kernel

On Mon, Oct 19, 2026 at 04:48:13PM +0200, Christian König wrote:
> On 10/19/26 14:44, Laurent Pinchart wrote:
> > On Mon, Oct 19, 2026 at 09:14:07AM +0200, Karl Mehltretter wrote:
> >> UVC and vivid access imported DMA-BUFs with the CPU without cache
> >> synchronization. On a Pi 400, captured data remained in the CPU cache
> >> while physical RAM was stale.
> >>
> >> Add an opt-in queue flag to bracket CPU access from buf_prepare() through
> >> buf_finish(). The allocator cannot imply CPU use because some vb2-vmalloc
> >> drivers pass buffers to hardware. Patch 1 balances finish operations in
> >> VIDIOC_REMOVE_BUFS so removing prepared buffers closes this interval.
> > 
> > This seems completely wrong.
> 
> It looks a bit questionable from 1000 miles above, but I might be wrong.
> 
> Let me iterate the rules here:
> 
> Accessing imported DMA-buf with the CPU from userspace should only be
> possible by using the dma_buf_mmap() interface and that includes the
> DMA_BUF_IOCTL_SYNC IOCTL to flush buffers dirtied by the CPU. If that
> is not done this way then that would be a *major* security issue, but
> I guess that this isn't the issue here.
> 
> What the drivers do more likely is that they use the vmap interface to
> access the DMA-buf with the CPU inside the kernel. Correct?

This patch series introduces a big penalty in the common case where a
userspace application consumes the frame with the CPU. There should be
no cache invalidation in that case.

> In this case dma_buf_begin_cpu_access() and dma_buf_end_cpu_access()
> should be called to properly frame that access.
> 
> That seems to be added in patch #2, but I absolutely can't judge if
> that is correct or not since at the same time you speak about userptr
> access and the code really looks a bit like spaghetti. 
> 
> >> This follows the per-queue flag proposed here:
> >> https://lists.openwall.net/linux-kernel/2024/06/20/236
> >>
> >> Based on Linux 7.3. Tests cover the same code on these earlier bases:
> >>
> >> - Pi 400, 7.3-rc4, all four patches: eight 640x480 YUYV frames per run,
> >>   imported CMA DMA-BUFs with a DRM device mapping:
> >>
> >>   Path                          Runs   Stale frames
> >>   UVC, vb2-vmalloc               12        0/96
> >>   vivid, vb2-vmalloc             12        0/96
> >>   vivid, vb2-dma-contig          12        0/96
> >>
> >>   All six lifecycle cases passed with both vivid allocators. Read-only
> >>   output USERPTR passed. The test added no warnings or failed CPU-access
> >>   callbacks. A control without the DRM mapping found 4/16 stale frames.
> >>
> >> - SAM9X75 (ARMv5), 6.18 backport of all four patches: the baseline
> >>   PREPARE_BUF/REMOVE_BUFS imbalance was fixed. USERPTR and six lifecycle
> >>   cases passed. All 128 vivid and 96 UVC frames matched RAM, with 100/100
> >>   begin/end calls. The baseline was already coherent.
> >>
> >> - BeagleV Ahead (RISC-V), 7.3-rc7, patches 1, 2 and 4: read-only USERPTR,
> >>   six traced lifecycle cases and 60 repeats passed with balanced callbacks
> >>   and no kernel diagnostics. A vendor 5.10 backport of patches 2 and 3
> >>   captured 96 UVC frames without stale data, with 100/100 begin/end calls.
> >>   Its baseline was already coherent.
> >>
> >> Karl Mehltretter (4):
> >>   media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS
> >>   media: videobuf2: add opt-in DMA-BUF CPU access
> >>   media: uvcvideo: synchronize imported DMA-BUFs for CPU access
> >>   media: vivid: synchronize imported DMA-BUFs for CPU access
> >>
> >>  .../media/common/videobuf2/videobuf2-core.c   | 96 ++++++++++++++++++-
> >>  .../common/videobuf2/videobuf2-dma-contig.c   |  5 +-
> >>  .../media/common/videobuf2/videobuf2-dma-sg.c |  5 +-
> >>  .../common/videobuf2/videobuf2-vmalloc.c      |  5 +-
> >>  drivers/media/test-drivers/vivid/vivid-core.c |  1 +
> >>  drivers/media/usb/uvc/uvc_queue.c             |  1 +
> >>  include/media/videobuf2-core.h                | 12 ++-
> >>  7 files changed, 116 insertions(+), 9 deletions(-)
> >>
> >>
> >> base-commit: c03c1a14507dfaf8b856c2608f9ac216021fc222

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-10-19 15:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-19  7:14 [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Karl Mehltretter
2026-10-19  7:14 ` [PATCH 1/4] media: videobuf2: finish prepared buffers in VIDIOC_REMOVE_BUFS Karl Mehltretter
2026-10-19  7:14 ` [PATCH 2/4] media: videobuf2: add opt-in DMA-BUF CPU access Karl Mehltretter
2026-10-19  7:14 ` [PATCH 3/4] media: uvcvideo: synchronize imported DMA-BUFs for " Karl Mehltretter
2026-10-19  7:14 ` [PATCH 4/4] media: vivid: " Karl Mehltretter
2026-10-19 12:44 ` [PATCH 0/4] media: synchronize CPU access to imported DMA-BUFs Laurent Pinchart
2026-10-19 14:48   ` Christian König
2026-10-19 15:01     ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®