[PATCH] media: staging/ipu7: Synchronize capture buffers at the vb2 boundaries

IPU7 captures into cached videobuf2-dma-sg buffers. On x86,
dma_sync_sgtable_for_device() and dma_sync_sgtable_for_cpu() do not perform
cache maintenance, so neither ownership transition invalidates cached frame
data.

The failure follows the buffer pool rotation. In raw captures, 7.3% of each
frame was bit-identical to the frame four positions earlier. Other lags had
no excess matches. Four was the libcamera buffer count. Last-level cache
eviction removed 97.2% of the excess. Flushing immediately before the
consumer read the buffer removed 98.9%. The stale 64-byte lines appeared as
orange and blue horizontal dashes around moving objects.

The driver already uses ipu7_dma_sync_sgtable(), backed by
clflush_cache_range(), for firmware, boot, and syscom memory. Call it from
buf_prepare and buf_finish, the videobuf2 CPU-to-device and device-to-CPU
ownership boundaries.

The prepare-side flush is required. vb2_dma_sg_alloc_compacted() allocates
with GFP_KERNEL | __GFP_ZERO, leaving dirty cache lines that can later be
written back over captured data. Userspace can also write to MMAP buffers
between captures.

Do not flush in the completion path. isys_isr runs from the hardirq handler
ipu_buttress_isr and holds isys->power_lock while it calls
ipu7_isys_queue_buf_done(). Flushing about 66,000 cache lines for a 4.2 MB
buffer there would run in hardirq context with a spinlock held. buf_finish
moves the work to process context during DQBUF.

ipu7_dma_sync_sg() uses sg_virt(), so every scatterlist entry must have a
permanent kernel mapping. That was true for existing driver-owned callers,
but the capture queue also supports imported DMABUFs. Validate the table in
buf_init before either sync callback can run. Return -EFAULT for unmappable
entries. Check orig_nents because ipu7_dma_sync_sgtable() flushes that set,
rather than the mapped nents set.

This rejects DMABUF imports backed by page-less or highmem scatterlists.
Those imports cannot safely use the required flush. Reviewers may prefer to
remove VB2_DMABUF support until such imports can be synchronized.

The flush averaged 217 us per ownership boundary for a 4.2 MB buffer over
47 passes (145 us minimum, 389 us maximum). At 57.8 fps and two passes per
frame, the calculated cost is 2.5% of one core.

Fixes: a516d36bdc3d ("media: staging/ipu7: add IPU7 input system device driver")
Closes: https://bugzilla.redhat.com/show_bug.cgi?id=2502786
Assisted-by: OpenAI-Codex:gpt-5.6-sol
Assisted-by: Claude-Code:claude-fable-5
Signed-off-by: Christian Murphy <[email protected]>
---

Notes:
    Equivalent modules were tested on a ThinkPad X1 Carbon Gen 14 with Debian
    linux 7.1.8-2. Six 24-frame raw captures, split across a suspend/resume
    cycle, had at most one excess lag-4 block per frame pair. A processed
    1920x1088 qcam preview had no visible corruption.
    
    v4l2-compliance 1.32.0 passed on the connected path: /dev/video0 58/58, the
    IMX471 subdevice 54/54, and /dev/media0 8/8, with no warnings. A full media
    walk had 112 existing failures on unsupported source pads of three
    unconnected CSI2 bridges.
    
    The submitted patch applies to media-committers next at the base commit and
    builds the IPU7 driver with W=1. The DMABUF import path was not tested.
    
    The cost measurement used ktime_get_ns() immediately around the sync call.
    Timer overhead was 36 ns. The 47 steady-state samples exclude the initial
    prepare and teardown finishes. The single initial prepare took 359 us.
    
    IPU6 has the same missing capture-buffer synchronization, but no IPU6
    hardware was available for testing.
    
    No Cc: stable is included because this driver is in staging.

 drivers/staging/media/ipu7/ipu7-isys-queue.c | 45 ++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/drivers/staging/media/ipu7/ipu7-isys-queue.c b/drivers/staging/media/ipu7/ipu7-isys-queue.c
index 434d9d9c7158..9a829e1705bb 100644
--- a/drivers/staging/media/ipu7/ipu7-isys-queue.c
+++ b/drivers/staging/media/ipu7/ipu7-isys-queue.c
@@ -29,6 +29,26 @@
 
 #define IPU_MAX_FRAME_COUNTER	(U8_MAX + 1)
 
+static int ipu7_isys_check_sgtable(struct ipu7_isys *isys, struct sg_table *sgt)
+{
+	struct device *dev = &isys->adev->auxdev.dev;
+	struct scatterlist *sg;
+	unsigned int i;
+
+	/* Validate every entry ipu7_dma_sync_sgtable() will later flush. */
+	for_each_sg(sgt->sgl, sg, sgt->orig_nents, i) {
+		struct page *page = sg_page(sg);
+
+		if (!page || PageHighMem(page)) {
+			dev_err_ratelimited(dev,
+					    "sg[%u] has no permanent kernel mapping\n", i);
+			return -EFAULT;
+		}
+	}
+
+	return 0;
+}
+
 static int ipu7_isys_buf_init(struct vb2_buffer *vb)
 {
 	struct ipu7_isys *isys = vb2_get_drv_priv(vb->vb2_queue);
@@ -38,6 +58,10 @@ static int ipu7_isys_buf_init(struct vb2_buffer *vb)
 		vb2_buffer_to_ipu7_isys_video_buffer(vvb);
 	int ret;
 
+	ret = ipu7_isys_check_sgtable(isys, sg);
+	if (ret)
+		return ret;
+
 	ret = ipu7_dma_map_sgtable(isys->adev, sg, DMA_TO_DEVICE, 0);
 	if (ret)
 		return ret;
@@ -82,6 +106,20 @@ static int ipu7_isys_queue_setup(struct vb2_queue *q, unsigned int *num_buffers,
 	return 0;
 }
 
+static void ipu7_isys_buf_sync(struct vb2_buffer *vb)
+{
+	struct ipu7_isys *isys = vb2_get_drv_priv(vb->vb2_queue);
+	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
+
+	/*
+	 * Device writes do not invalidate the CPU cache and the DMA API
+	 * sync helpers do no cache maintenance on x86. Flush dirty
+	 * lines before the device writes the buffer and stale lines
+	 * before userspace reads it.
+	 */
+	ipu7_dma_sync_sgtable(isys->adev, sgt);
+}
+
 static int ipu7_isys_buf_prepare(struct vb2_buffer *vb)
 {
 	struct ipu7_isys_queue *aq = vb2_queue_to_isys_queue(vb->vb2_queue);
@@ -99,10 +137,16 @@ static int ipu7_isys_buf_prepare(struct vb2_buffer *vb)
 	dev_dbg(dev, "buffer: %s: bytesperline %u, height %u\n",
 		av->vdev.name, bytesperline, height);
 	vb2_set_plane_payload(vb, 0, bytesperline * height);
+	ipu7_isys_buf_sync(vb);
 
 	return 0;
 }
 
+static void ipu7_isys_buf_finish(struct vb2_buffer *vb)
+{
+	ipu7_isys_buf_sync(vb);
+}
+
 /*
  * Queue a buffer list back to incoming or active queues. The buffers
  * are removed from the buffer list.
@@ -790,6 +834,7 @@ static const struct vb2_ops ipu7_isys_queue_ops = {
 	.queue_setup = ipu7_isys_queue_setup,
 	.buf_init = ipu7_isys_buf_init,
 	.buf_prepare = ipu7_isys_buf_prepare,
+	.buf_finish = ipu7_isys_buf_finish,
 	.buf_cleanup = ipu7_isys_buf_cleanup,
 	.start_streaming = start_streaming,
 	.stop_streaming = stop_streaming,

base-commit: 4900cad020c0580dfb1be27776ff10a4ef110cfa
-- 
2.53.0