Re: [PATCH 2/3] media: camss: video: check that a buffer is DMA contiguous and add a prepare hook

[Bryan O'Donoghue <bryan.odonoghue@linaro.org>]

On 29/10/2026 12:31, Qianhui Xu wrote:
> The write master of the newer VFE generations can only be programmed with
> a buffer whose payload is physically contiguous

The first problem is that is _not_ what your code checks - you should 
use sg_phys() for that - which gets the physical address of a CPU 
scatterlist entry.

What your code currently does is check that the IOVA addresses are 
contiguous.

But what happens is an IOVA allocation is made which is linearly 
contiguous in the IOVA address space and then SMMU entries map the IOVA 
back to physical entries.

So all we need to do is give the IOVA base to CAMSS and it can write 
IOVA buffers - its the SMMU's job to ensure the resulting upstream 
writes land at the right physical address.

> increment registers describe one linear address range.  Check that the DMA
> mapping of every plane is contiguous for at least sizeimage bytes and
> reject the buffer otherwise, instead of silently writing past the end of
> the first scatterlist entry.

> Also let the SoC code add its own alignment requirements to a queued
> buffer through a new prepare_buffer() video operation, which runs after
> the generic checks.
> 
> Signed-off-by: Qianhui Xu <xiaqinghuan@mail.ustc.edu.cn>

And I can prove it too.

buildroot# uname -a
Linux buildroot 7.3.0-rc1+ #19 SMP PREEMPT Thu Oct 29 13:26:34 GMT 2026 
aarch64 GNU/Linux
buildroot# dmesg | grep -i crd
[    0.000000] Machine model: Qualcomm Technologies, Inc. X1E80100 CRD
buildroot# dmesg | grep "plane 0"
qcom-camss acb7000.isp: plane 0 chunk 0: phys 0x000000089a800000 len 8388608
qcom-camss acb7000.isp: plane 0 chunk 1: phys 0x000000089a600000 len 2097152
qcom-camss acb7000.isp: plane 0 chunk 2: phys 0x0000000899900000 len 1048576
qcom-camss acb7000.isp: plane 0 chunk 3: phys 0x0000000899b00000 len 131072
qcom-camss acb7000.isp: plane 0: 11677696 bytes, 6 cpu entries, 5 
physical breaks, largest physical run 8388608, 1 dma segments, 0 iova 
breaks, iova 0x00000000ff000000

Plane 0 == 11677696 bytes
Largest physical run = 8388608

Interestingly we see some physically adjacent allocations here and then 
also non-contiguous allocations, which is the negation of the thesis, 
constraining IOVA to physically contiguous maps is provably incorrect.

NAK

diff --git a/drivers/media/platform/qcom/camss/camss-video.c 
b/drivers/media/platform/qcom/camss/camss-video.c
index 0852eb6f13..d2df368d5f 100644
--- a/drivers/media/platform/qcom/camss/camss-video.c
+++ b/drivers/media/platform/qcom/camss/camss-video.c
@@ -146,6 +146,44 @@ static int video_queue_setup(struct vb2_queue *q,
  	return 0;
  }

+/* DEBUG: how a plane is laid out in physical and IOVA space */
+static void video_buf_dump_sg(struct camss_video *video, unsigned int 
plane,
+			      struct sg_table *sgt)
+{
+	dma_addr_t iova = sg_dma_address(sgt->sgl);
+	unsigned int j, pbreaks = 0, dbreaks = 0;
+	size_t total = 0, run = 0, maxrun = 0;
+	struct scatterlist *sg;
+	phys_addr_t pend = 0;
+	dma_addr_t dend = 0;
+
+	for_each_sgtable_sg(sgt, sg, j) {
+		if (j && sg_phys(sg) != pend) {
+			pbreaks++;
+			run = 0;
+		}
+		if (j < 4)
+			dev_info(video->camss->dev, "plane %u chunk %u: phys %pa len %u\n",
+				 plane, j, &(phys_addr_t){ sg_phys(sg) }, sg->length);
+		run += sg->length;
+		if (run > maxrun)
+			maxrun = run;
+		total += sg->length;
+		pend = sg_phys(sg) + sg->length;
+	}
+
+	for_each_sgtable_dma_sg(sgt, sg, j) {
+		if (j && sg_dma_address(sg) != dend)
+			dbreaks++;
+		dend = sg_dma_address(sg) + sg_dma_len(sg);
+	}
+
+	dev_info(video->camss->dev,
+		 "plane %u: %zu bytes, %u cpu entries, %u physical breaks, largest 
physical run %zu, %u dma segments, %u iova breaks, iova %pad\n",
+		 plane, total, sgt->orig_nents, pbreaks, maxrun, sgt->nents,
+		 dbreaks, &iova);
+}
+
  static int video_buf_init(struct vb2_buffer *vb)
  {
  	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -163,6 +201,7 @@ static int video_buf_init(struct vb2_buffer *vb)
  			return -EFAULT;

  		buffer->addr[i] = sg_dma_address(sgt->sgl);
+		video_buf_dump_sg(video, i, sgt);
  	}

  	if (format->pixelformat == V4L2_PIX_FMT_NV12 ||