Re: [PATCH] iommu/dma: Respect SWIOTLB force_bounce

[T.J. Mercier]

On Thu, May 2, 2024 at 5:50 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
>
> On 02/05/2024 6:07 am, Christoph Hellwig wrote:
> > On Wed, May 01, 2024 at 08:13:18PM +0000, T.J. Mercier wrote:
> >> iommu_dma_map_page and iommu_dma_map_sg conditionally use SWIOTLB, but
> >> checking if force_bounce is set for the device is not part of that
> >> condition. Check if devices have requested to force SWIOTLB use as part
> >> of deciding to take the existing SWIOTLB paths.
> >
> > This fails to explain why you'd want this somewhat surprising behavior,
> > and why you consider it a bug fix.
>
> Indeed, it's rather intentional that the "swiotlb=force" argument
> doesn't affect iommu-dma, since that's primarily for weeding out drivers
> making dodgy assumptions about DMA addresses, and iommu-dma is
> inherently even better at that already.
>
> Beyond that I think this change also seems likely to interact badly with
> CC_ATTR_GUEST_MEM_ENCRYPT on x86, where we invoke the SWIOTLB_FORCE flag
> for dma-direct, but expect that an IOMMU can provide a decrypted view
> in-place, thus bouncing in that path would be unnecessarily detrimental.
>
> Thanks,
> Robin.

I encountered this while testing a change to DMA direct which makes
sure that sg_dma_mark_swiotlb is called there like it is here. (Right
now the SG_DMA_SWIOTLB flag is set only if dma_map_sgtable takes the
IOMMU path, but not if SWIOTLB is used on the direct path.) While I
agree IOMMU + force_bounce is an unusual config, I found it equally
surprising that swiotlb=force wasn't doing what is advertised, or even
giving a warning/error. Since the iommu-dma code is already set up for
conditionally bouncing through SWIOTLB, it looked straightforward to
give what's asked for in the case of swiotlb=force. If it's
intentional that SWIOTLB options don't affect IOMMU code, then should
we just warn about it here when it's ignored? The presence of a
warning like that would also be a suggestion of, "you probably don't
actually want what you're asking for with this configuration you've
specified".

Thanks,
T.J.