Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Tue, Jan 15, 2019 at 03:24:55PM +0100, Christian König wrote: > Yeah, indeed. Bounce buffers are an absolute no-go for GPUs. > > If the DMA API finds that a piece of memory is not directly accessible by > the GPU we need to return an error and not try to use bounce buffers behind > the surface. > > That is something which always annoyed me with the DMA API, which is > otherwise rather cleanly defined. That is exactly what I want to fix with my series to make DMA_ATTR_NON_CONSISTENT more useful and always available: https://lists.linuxfoundation.org/pipermail/iommu/2018-December/031985.html With that you allocate the memory using dma_alloc_attrs with DMA_ATTR_NON_CONSISTENT, and use dma_sync_single_* to transfer ownership to the cpu and back to the device, with a gurantee that there won't be any bouncing. So far the interest by the parties that requested the feature has been rather lacklustre, though.
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Tue, Jan 15, 2019 at 06:03:39PM +, Thomas Hellstrom wrote: > In the graphics case, it's probably because it doesn't fit the graphics > use-cases: > > 1) Memory typically needs to be mappable by another device. (the "dma- > buf" interface) And there is nothing preventing dma-buf sharing of these buffers. Unlike the get_sgtable mess it can actually work reliably on architectures that have virtually tagged caches and/or don't guarantee cache coherency with mixed attribute mappings. > 2) DMA buffers are exported to user-space and is sub-allocated by it. > Mostly there are no GPU user-space kernel interfaces to sync / flush > subregions and these syncs may happen on a smaller-than-cache-line > granularity. I know of no architectures that can do cache maintainance on a less than cache line basis. Either the instructions require you to specifcy cache lines, or they do sometimes more, sometimes less intelligent rounding up. Note that as long dma non-coherent buffers are devices owned it is up to the device and the user space driver to take care of coherency, the kernel very much is out of the picture.
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Tue, Jan 15, 2019 at 07:13:11PM +, Koenig, Christian wrote: > Thomas is correct that the interface you propose here doesn't work at > all for GPUs. > > The kernel driver is not informed of flush/sync, but rather just setups > coherent mappings between system memory and devices. > > In other words you have an array of struct pages and need to map that to > a specific device and so create dma_addresses for the mappings. If you want a coherent mapping you need to use dma_alloc_coherent and dma_mmap_coherent and you are done, that is not the problem. That actually is one of the vmgfx modes, so I don't understand what problem we are trying to solve if you don't actually want a non-coherent mapping. Although last time I had that discussion with Daniel Vetter I was under the impressions that GPUs really wanted non-coherent mappings. But if you want a coherent mapping you can't go to a struct page, because on many systems you can't just map arbitrary memory as uncachable. It might either come from very special limited pools, or might need other magic applied to it so that it is not visible in the normal direct mapping, or at least not access through it.
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Wed, Jan 16, 2019 at 07:28:13AM +, Koenig, Christian wrote: > To summarize once more: We have an array of struct pages and want to > coherently map that to a device. And the answer to that is very simple: you can't. What is so hard to understand about? If you want to map arbitrary memory it simply can't be done in a coherent way on about half of our platforms. > If that is not possible because of whatever reason we want to get an > error code or even not load the driver from the beginning. That is a bullshit attitude. Just like everyone else makes their drivers work you should not be lazy. > > bool dma_streaming_is_coherent(const struct device *) > > > > API to help us decide when to load or not. > > Yes, please. Hell no.
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Tue, Jan 15, 2019 at 02:25:01PM -0700, Jason Gunthorpe wrote: > RDMA needs something similar as well, in this case drivers take a > struct page * from get_user_pages() and need to have the DMA map fail > if the platform can't DMA map in a way that does not require any > additional DMA API calls to ensure coherence. (think Userspace RDMA > MR's) Any time you dma map pages you need to do further DMA API calls to ensure coherent, that is the way it is implemented. These calls just happen to be no-ops sometimes. > Today we just do the normal DMA map and when it randomly doesn't work > and corrupts data tell those people their platforms don't support RDMA > - it would be nice to have a safer API base solution.. Now that all these drivers are consolidated in rdma-core you can fix the code to actually do the right thing. It isn't that userspace DMA coherent is any harder than in-kernel DMA coherenence. It just is that no one bothered to do it properly.
Re: [PATCH] lib/scatterlist: Provide a DMA page iterator
On Wed, Jan 16, 2019 at 10:24:36AM -0700, Jason Gunthorpe wrote: > The fact is there is 0 industry interest in using RDMA on platforms > that can't do HW DMA cache coherency - the kernel syscalls required to > do the cache flushing on the IO path would just destroy performance to > the point of making RDMA pointless. Better to use netdev on those > platforms. In general there is no syscall required for doing cache flushing, you just issue the proper instructions directly from userspace. > The reality is that *all* the subsytems doing DMA kernel bypass are > ignoring the DMA mapping rules, I think we should support this better, > and just accept that user space DMA will not be using syncing. Block > access in cases when this is required, otherwise let it work as is > today. In that case we just need to block userspace DMA access entirely. Which given the amount of problems it creates sounds like a pretty good idea anyway.
