On Tue, Nov 27, 2018 at 11:39:40PM -0800, Christoph Hellwig wrote: > > + /* > > + * dma_sync_sg_*() flush the physical pages, so point > > + * sg->dma_address to the physical ones for the right behavior. > > + */ > > + for_each_sg(msm_obj->sgt->sgl, s, msm_obj->sgt->nents, i) > > + sg_dma_address(s) = sg_phys(s); > > + > > I'm sorry, but this is completely bogus and not acceptable. > > The only place that is allowed to initialize sg_dma_address is > dma_map_sg. If the default dma ops don't work for your setup we have > major a problem and need to fix the dma api / iommu integration instead > of hacking around it. Somewhat related, I recently submitted a patch for the spi-bcm2835.c driver to overcome a limitation of the DMA engine and the spi0 master on the Raspberry Pi: http://lists.infradead.org/pipermail/linux-rpi-kernel/2018-November/008275.html If a buffer needs to be sent or received which was vmalloc'ed (hence is not contiguous in physical memory) and starts in the middle of a page with an offset not a multiple of 4, the buffer could not be transmitted with DMA so far because the spi0 master requires 4 byte writes to the FIFO and the DMA engine is incapable of combining multiple sglist entries of arbitrary length into a continuous stream of 4 byte chunks. The solution I've found is to transfer the first few bytes of the first sglist entry with programmed I/O such that the remainder has a length which is a multiple of 4. For this to work, I have to massage the first sglist entry, i.e. I have to add the number of bytes transmitted with PIO to the DMA address and subtract it from the sglist entry's length. After the DMA engine has done its job, I undo those changes to the sglist. Could you comment on the acceptibility of this approach? If you do not deem it acceptable, could you make a suggestion how the dma-mapping API shall be amended to support such use cases? (I sure hope the approach *is* acceptable, getting this to work at all required endless fiddling. The BCM2835 is in some ways a not so great SoC and could do with better documentation, to put it mildly.) (Note: I've since amended the above-linked patch to sync only the modified cachelines back to memory, not the full first sglist entry. I've also switched to the sg_dma_address() and sg_dma_len() syntax instead of referencing the sglist entry members directly, this appears to be more common in the tree even for l-values. I was going to post these changes as v2 shortly.) Thanks, Lukas
