On Mon, Nov 04, 2024 at 10:58:31AM +0100, Christoph Hellwig wrote: > On Thu, Oct 31, 2024 at 09:17:45PM +0000, Robin Murphy wrote: > > The hilarious amount of work that iommu_dma_map_sg() does is pretty much > > entirely for the benefit of v4l2 and dma-buf importers who *depend* on > > being able to linearise a scatterlist in DMA address space. TBH I doubt > > there are many actual scatter-gather-capable devices with significant > > enough limitations to meaningfully benefit from DMA segment combining these > > days - I've often thought that by now it might be a good idea to turn that > > behaviour off by default and add an attribute for callers to explicitly > > request it. > > Even when devices are not limited they often perform significantly better > when IOVA space is not completely fragmented. While the dma_map_sg code > is a bit gross due to the fact that it has to deal with unaligned segments, > the coalescing itself often is a big win. RDMA is like this too, Almost all the MR HW gets big wins if the entire scatter list is IOVA contiguous. One of the future steps I'd like to see on top of this is to fine tune the IOVA allocation backing MRs to exactly match the HW needs. Having proper alignment and contiguity can be huge reduction in device overhead, like a 100MB MR may need to store 200K of mapping information on-device, but with a properly aligned IOVA this can be reduced to only 16 bytes. Avoiding a double translation tax when the iommu HW is enabled is potentially significant. We have some RDMA workloads with VMs where the NIC is holding ~1GB of memory just for translations, but the iommu is active as the S2. ie we are paying a double tax on translation. It could be a very interesting trade off to reduce the NIC side to nothing and rely on the CPU IOMMU with nested translation instead. > Note that dma_map_sg also has two other very useful features: batching > of the iotlb flushing, and support for P2P, which to be efficient also > requires batching the lookups. This is the main point, and I think, is the uniqueness Leon is talking about. We don't get those properties through any other API and this one series preserves them. In fact I would say that is the entire point of this series: preserve everything special about dma_map_sg() compared to dma_map_page() but don't require a scatterlist. > >> Several approaches have been explored to expand the DMA API with additional > >> scatterlist-like structures (BIO, rlist), instead split up the DMA API > >> to allow callers to bring their own data structure. > > > > And this line of reasoning is still "2 + 2 = Thursday" - what is to say > > those two notions in any way related? We literally already have one generic > > DMA operation which doesn't operate on struct page, yet needed nothing > > "split up" to be possible. > > Yeah, I don't really get the struct page argument. In fact if we look > at the nitty-gritty details of dma_map_page it doesn't really need a > page at all. Today, if you want to map a P2P address you must have a struct page, because page->pgmap is the only source of information on the P2P topology. So the logic is, to get P2P without struct page we need a way to have all the features of dma_map_sg() but without a mandatory scatterlist because we cannot remove struct page from scatterlist. This series gets to the first step - no scatterlist. There will need to be another series to provide an alternative to page->pgmap to get the P2P information. Then we really won't have struct page dependence in the DMA API. I actually once looked at how to enhance dma_map_resource() to support P2P and it was not very nice, the unmap side became quite complex. I think this is a more elgant solution than what I was sketching. > >> for the device. Instead of allocating a scatter list entry per allocated > >> page it can just allocate an array of 'struct page *', saving a large > >> amount of memory. > > > > VFIO already assumes a coherent device with (realistically) an IOMMU which > > it explicitly manages - why is it even pretending to need a generic DMA > > API? > > AFAIK that does isn't really vfio as we know it but the control device > for live migration. But Leon or Jason might fill in more. Yes, this is the control side of the VFIO live migration driver that needs rather a lot of memory to store the migration blob. There is definitely an iommu, and the VF function is definitely translating, but it doesn't mean the PF function is using dma-iommu.c, it is often in iommu passthrough/identity and using DMA direct. It was done as an alternative example on how to use the API. Again there are more improvements possible there, the driver does not take advantage of contiguity or alignment when programming the HW. > Because we only need to preallocate the tiny constant sized dma_iova_state > as part of the request instead of an additional scatterlist that requires > sizeof(struct page *) + sizeof(dma_addr_t) + 3 * sizeof(unsigned int) > per segment, including a memory allocation per I/O for that. Right, eliminating scatterlist entirely on fast paths is a big point. I recall Chuck was keen on the same thing for NFSoRDMA as well. > At least for the block code we have a nice little core wrapper that is > very easy to use, and provides a great reduction of memory use and > allocations. The HMM use case I'll let others talk about. I saw the Intel XE team make a complicated integration with the DMA API that wasn't so good. They were looking at an earlier version of this and I think the feedback was positive. It should make a big difference, but we will need to see what they come up and possibly tweak things. Jason
