On Thu, Dec 13, 2018 at 11:03 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > On Thu, Dec 13, 2018 at 12:13:29PM +0900, Tomasz Figa wrote: > > Putting aside the problem of memory without struct page, one thing to > > note here that what is a contiguous DMA range for device X, may not be > > mappable contiguously for device Y and it would still need something > > like a scatter list to fully describe the buffer. > > I think we need to define contiguous here. > > If the buffer always is physically contiguous, as it is in the currently > posted series, we can always map it with a single dma_map_single call > (if the hardware can handle that in a single segment is a different > question, but out of scope here). Are you sure the buffer is always physically contiguous? At least the ARM IOMMU dma_ops [1] and the DMA-IOMMU dma_ops [2] will simply allocate pages without any continuity guarantees and remap the pages into a contiguous kernel VA (unless DMA_ATTR_NO_KERNEL_MAPPING is given, which makes them return an opaque cookie instead of the kernel VA). [1] http://git.infradead.org/users/hch/misc.git/blob/2dbb028e4a3017e1b71a6ae3828a3548545eba24:/arch/arm/mm/dma-mapping.c#l1291 [2] http://git.infradead.org/users/hch/misc.git/blob/2dbb028e4a3017e1b71a6ae3828a3548545eba24:/drivers/iommu/dma-iommu.c#l450 > > If on the other hand we have multiple discontiguous physical address > range that are mapped using the iommu and vmap this interface doesn't > work anyway. > > But in that case you should just do multiple allocations and then use > dma_map_sg coalescing on the hardware side, and vmap [1] if really > needed. I guess for this we want to gurantee that dma_alloc_attrs > with the DMA_ATTR_NON_CONSISTENT allows virt_to_page to be used on > the return value, which the currently posted implementation does, > although I'm a it reluctant about the API guarantee. > > > > Do we already have a structure that would work for this purposes? I'd > > assume that we need something like the existing scatterlist but with > > page links replaced with something that doesn't require the memory to > > have struct page, possibly just PFN? > > The problem is that just the PFN / physical address isn't enough for > most use cases as you also need a kernel virtual address. But moving > struct scatterlist to store a pfn instead of a struct page would be > pretty nice for various reasons anyway. > > > > > > > > > It would also be great to use that opportunity to get rid of all the > > > code duplication of almost the same dmabug provides backed by the > > > DMA API. > > > > Could you sched some more light on this? I'm curious what is the code > > duplication you're referring to. > > It seems like a lot of the dmabuf ops are just slight various of > dma_alloc + dma_get_sttable + dma_map_sg / dma_unmap_sg. There must be > a void to not duplicate that over and over. Device/kernel/userspace maps/unmaps shouldn't really be exporter-specific indeed, as long as one provides a uniform way of describing a buffer and then have dma_map_*() work on that. Possibly a part that manages the CPU cache maintenance either. There is still some space for some special device caches (or other synchronization), though. > > [1] and use invalidate_kernel_vmap_range and flush_kernel_vmap_range > to manually take care of cache flushing.
