On Wed, Jul 15, 2020 at 11:46 AM Claire Chang <tientzu@xxxxxxxxxxxx> wrote: > > On Tue, Jul 14, 2020 at 7:01 PM Christoph Hellwig <hch@xxxxxx> wrote: > > > > On Mon, Jul 13, 2020 at 12:55:43PM +0100, Robin Murphy wrote: > > > On 2020-07-13 10:12, Claire Chang wrote: > > >> The bounced DMA ops provide an implementation of DMA ops that bounce > > >> streaming DMA in and out of a specially allocated region. Only the > > >> operations relevant to streaming DMA are supported. > > > > > > I think there are too many implicit assumptions here - apparently that > > > coherent allocations will always be intercepted by > > > dma_*_from_dev_coherent(), and that calling into dma-direct won't actually > > > bounce things a second time beyond where you thought they were going, > > > manage coherency for a different address, and make it all go subtly wrong. > > > Consider "swiotlb=force", for instance... If I understand it correctly, reusing SWIOTLB won't prevent the coherent allocations from always being intercepted by dma_*_from_dev_coherent(), right? Since we can't bounce the coherent memory, we still need to rely on dma_*_from_dev_coherent() and a reserved-memory region for coherent DMA to restrict the device DMA access. As for calling into dma-direct, in this version, I use set_dma_ops to set the dma_bounced_ops, so I just bypass dma-direct and SWIOTLB. "swiotlb=force" won't bounce things a second time and the data will still be bounced to the region set in dts. Besides, I did a quick search and found that only two *-iommu.c directly use dma_direct_map_page. https://elixir.bootlin.com/linux/latest/C/ident/dma_direct_map_page Since bounced DMA is to mitigate the lack of DMA access control on systems without an IOMMU (see patch#4, only call of_dma_set_bounce_buffer for the devices not behind an IOMMU), can we assume no one will use dma-direct? (I understand that if we build bounced DMA on top of SWIOTLB, we don't need to worry about this.) > > > > > > Again, plumbing this straight into dma-direct so that SWIOTLB can simply > > > target a different buffer and always bounce regardless of masks would seem > > > a far better option. > > > > I haven't really had time to read through the details, but I agree that > > any bouncing scheme should reuse the swiotlb code and not invent a > > parallel infrastructure. > Thanks for the feedback. I'll try to reuse SWIOTLB. My current plan is to first change the buffers management logic in SWIOTLB to use gen_pool like this patch (i.e., gen_pool_dma_alloc, gen_pool_free, ect), and then make SWIOTLB use the device's private pool for regular DMA to/from system memory if possible. Does this sound right? Thanks!
