On 2021-07-16 07:19, Christoph Hellwig wrote:
On Thu, Jul 15, 2021 at 03:16:08PM +0100, Robin Murphy wrote:
On 2021-07-15 15:07, Christoph Hellwig wrote:
On Thu, Jul 15, 2021 at 02:04:24PM +0100, Robin Murphy wrote:
If people are going to insist on calling iommu_iova_to_phys()
pointlessly and expecting it to work,

Maybe we need to fix that?

Feel free to try, but we didn't have much luck pushing back on it
previously, so playing whack-a-mole against netdev now is a game I'm
personally happy to stay away from ;)

One thing I've done with symbols I want people to not use it to
unexport them.  But what about vfio?

Yeah, it's not like they shouldn't be calling it at all - I see it as primarily intended for use by drivers managing their own domains, but I don't entirely disagree with using it on DMA domains either in niche cases - it's that they blindly grab the default domain without even checking whether DMA mappings are actually translated or not (and thus whether they even need to make that call every time they pull a descriptor back out of a ringbuffer). IIRC the argument was essentially that checking the domain type was an IOMMU API detail that those driver shouldn't have to know about and the abstraction should just take care of it, despite the fact that they're punching through 2 layers of abstraction to even reach that point. And apparently keeping track of their own descriptor addresses would be too much work, but expensive indirect calls to either return the address they already have or go off and do a software table walk with atomic synchronisation and everything are fine :/

While we're talking about iommu_iova_to_phys: __iommu_dma_unmap_swiotlb
calls it unconditionally, despite only needed ing the physical address.
Can we optimize that somehow by splitting out the bounce buffering case
out?

Indeed, as I think I mentioned recently on another thread, all the bounce-buffering stuff is fairly ugly because it's basically the old intel-iommu code dropped in with as few changes as possible for ease of review, since Tom was no longer able to spend time refining it, and nobody else has got round to cleaning it up yet either. In fact the whole flow through iommu_dma_unmap_page() flow might be the worst-hit - reusing the iommu_dma_sync op made perfect sense when it was just cache maintenance, but now means that at worst we do iova_to_phys *twice* plus a pointless swiotlb_sync :(

Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to