On Mon, Feb 18, 2019 at 05:01:59PM +0000, Robin Murphy wrote:
On 18/02/2019 14:37, Stanislaw Gruszka wrote:
[...]
quoted
Another issue is that dma_map_sg() & dma_map_page() may require some
constraints. I'm not sure about that and I want to clarify that with
CCed mm maintainers. I think DMA drivers may expect sg->offset < PAGE_SIZE
for both dma_map_sg() and dma_map_page(). Additionally dma_map_page()
maight expect that offset & length specify buffer within one page.
Luckily, this came up a while back[1] and we seemed to reach a
consensus that sg->offset >= PAGE_SIZE for dma_map_sg() was weird
but valid. IIRC it was only the Intel IOMMU code which failed to
handle that case appropriately (and which I fixed) - the AMD IOMMU
code always looked like it should be OK, but I'm not sure I've ever
seen definitive test results (and I don't have hardware to do so
myself).
Funny that we have problems on AMD IOMMU and not with Intel IOMMU.
For dma_map_page(), length >= PAGE_SIZE should be perfectly valid
and handled correctly. The offset >= PAGE_SIZE case is a bit harder
to justify, but at the same time has less scope for the DMA API
backend to get it wrong, so either way is likely to be OK in
practice (in particular the AMD IOMMU code looks like it won't have
a problem, since its map_page() implementation converts page and
offset to a plain physical address before doing anything else).
Thanks for clarify this. So my patch which do:
- page = virt_to_head_page(data);
+ page = virt_to_page(data);
offset = data - page_address(page);
sg_set_page(&urb->sg[i], page, sglen, offset);
should not be necessary as IOMMU driver do exactly the same internally.
Are there any alignment requirement for offset for dma_map_{page,sg} ?
It will work with let say sg->offset=113 or we have make sure it is
aligned to some boundary. If so, what boundary ?
Stanislaw