Re: [PATCH v3 1/5] dma-iommu: fix sync_sg with swiotlb

[Robin Murphy]

On 2021-08-11 06:57, Christoph Hellwig wrote:
> On Wed, Aug 11, 2021 at 11:42:43AM +0900, David Stevens wrote:
> > From: David Stevens <stevensd@xxxxxxxxxxxx>
> >
> > The is_swiotlb_buffer function takes the physical address of the swiotlb
> > buffer, not the physical address of the original buffer. The sglist
> > contains the physical addresses of the original buffer, so for the
> > sync_sg functions to work properly when a bounce buffer might have been
> > used, we need to use iommu_iova_to_phys to look up the physical address.
> > This is what sync_single does, so call that function on each sglist
> > segment.
> >
> > The previous code mostly worked because swiotlb does the transfer on map
> > and unmap. However, any callers which use DMA_ATTR_SKIP_CPU_SYNC with
> > sglists or which call sync_sg would not have had anything copied to the
> > bounce buffer.
> >
> > Fixes: 82612d66d51d ("iommu: Allow the dma-iommu api to use bounce buffers")
> > Signed-off-by: David Stevens <stevensd@xxxxxxxxxxxx>
> > ---
> >   drivers/iommu/dma-iommu.c | 27 +++++++++++++--------------
> >   1 file changed, 13 insertions(+), 14 deletions(-)
> >
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index 98ba927aee1a..54e103b989d9 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -813,14 +813,13 @@ static void iommu_dma_sync_sg_for_cpu(struct device *dev,
> >   	if (dev_is_dma_coherent(dev) && !dev_is_untrusted(dev))
> >   		return;
> >   
> > +	if (dev_is_untrusted(dev))
> > +		for_each_sg(sgl, sg, nelems, i)
> > +			iommu_dma_sync_single_for_cpu(dev, sg_dma_address(sg),
> > +						      sg->length, dir);
> > +	else
> > +		for_each_sg(sgl, sg, nelems, i)
> >   			arch_sync_dma_for_cpu(sg_phys(sg), sg->length, dir);
> >   }
>
> I'd remove the above check and fold the if (!dev_is_dma_coherent(dev))
> into the else line.  Same for iommu_dma_sync_sg_for_device.

+1

With those also cleaned up,

Reviewed-by: Robin Murphy <robin.murphy@xxxxxxx>