Hi,
On 17. 03. 23 18:19, Lizhi Hou wrote:
Hi Martin,
On 3/17/23 04:18, Martin Tůma wrote:
Hi
On 16. 03. 23 19:25, Lizhi Hou wrote:
Hi Martin,
Based on the dmaengine document
https://kernel.org/doc/html/v6.3-rc2/driver-api/dmaengine/client.html
The sg_len used for dmaengine_prep_slave_sg should be returned by
dma_map_sg.
This implies there should not be zero segment for the first sg_len
segments.
I do get the mapping from vb2_dma_sg_plane_desc() which internally
uses dma_map_sg_attrs() and yet in some special cases sg_dma_len()
returns zero. So this is a "real-life" scenario. I'm still investigating
This sounds odd to me. dma_map_sg() is supposed to return number of
Non-zero sg because the zero length sg will be merged.
what the root cause is for getting such empty mappings from v4l2, but
the DMA driver should IMHO not crash the kernel in such case anyway.
I checked some dma driver and I did not see obvious code to check the sg
len. e.g.
https://elixir.bootlin.com/linux/v6.3-rc2/source/drivers/dma/k3dma.c#L531
Didn't examinate this one in detail, but I don't think this one would
crash either. In your case, the problem is, that you do not pre-allocate
the descriptor in case sg_dma_len(sg) is zero and then access it in:
desc->bytes = cpu_to_le32(len);
which is the exact line where xdma crashes.
M.
I have looked into some randomly chosen kernel code that uses
sg_dma_len() and all usages that I have seen are prepared for the case
the length is zero. All but one driver simply stop the iteration while
the remaining one reported this case as an error.
Are those dma drivers?
Adding a check sounds fine to me. And I am going to create a patch
anyway. :)
Thanks,
Lizhi
M.
I think that is why we need to provide 'sg_len' for
dmaengine_prep_slave_sg().
With 'sg_len', we do not need to worry about zero length sg in the
driver callback.
Thanks,
Lizhi
On 3/16/23 10:03, Martin Tůma wrote:
Hi,
The Xilinx XDMA driver crashes when the scatterlist provided to
xdma_prep_device_sg() contains an empty entry, i.e. sg_dma_len()
returns zero. As I do get such sgl from v4l2 I suppose that this
is a valid scenario and not a bug in our parent mgb4 driver. Also
the documentation for sg_dma_len() suggests that there may be
zero-length entries.
The following trivial patch fixes the crash:
diff --git a/drivers/dma/xilinx/xdma.c b/drivers/dma/xilinx/xdma.c
index 462109c61653..cd5fcd911c50 100644
--- a/drivers/dma/xilinx/xdma.c
+++ b/drivers/dma/xilinx/xdma.c
@@ -487,6 +487,8 @@ xdma_prep_device_sg(struct dma_chan *chan,
struct scatterlist *sgl,
for_each_sg(sgl, sg, sg_len, i) {
addr = sg_dma_address(sg);
rest = sg_dma_len(sg);
+ if (!rest)
+ break;
do {
len = min_t(u32, rest, XDMA_DESC_BLEN_MAX);
M.
