1. Patchew
  2. linux
  3. View series

[PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()

Marek Szyprowski posted 1 patch 1 week ago 
drivers/iommu/dma-iommu.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Marek Szyprowski 1 week ago 
Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
dma_iova_link() code path, but missed that the CPU cache is being also
touched in the dma_iova_unlink() path. Fix this.

Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/iommu/dma-iommu.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
index e52d19d2e833..c92088855450 100644
--- a/drivers/iommu/dma-iommu.c
+++ b/drivers/iommu/dma-iommu.c
@@ -2008,7 +2008,7 @@ static void iommu_dma_iova_unlink_range_slow(struct device *dev,
 			end - addr, iovad->granule - iova_start_pad);
 
 		if (!dev_is_dma_coherent(dev) &&
-		    !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
+		    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
 			arch_sync_dma_for_cpu(phys, len, dir);
 
 		swiotlb_tbl_unmap_single(dev, phys, len, dir, attrs);
@@ -2032,7 +2032,8 @@ static void __iommu_dma_iova_unlink(struct device *dev,
 	size_t unmapped;
 
 	if ((state->__size & DMA_IOVA_USE_SWIOTLB) ||
-	    (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)))
+	    (!dev_is_dma_coherent(dev) &&
+	     !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO))))
 		iommu_dma_iova_unlink_range_slow(dev, addr, size, dir, attrs);
 
 	iommu_iotlb_gather_init(&iotlb_gather);
-- 
2.34.1
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Jason Gunthorpe 1 week ago 
On Mon, Nov 24, 2025 at 06:09:55PM +0100, Marek Szyprowski wrote:
> Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
> dma_iova_link() code path, but missed that the CPU cache is being also
> touched in the dma_iova_unlink() path. Fix this.
> 
> Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/iommu/dma-iommu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)

Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>

Did you find it by code inspection?

Jason
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Marek Szyprowski 1 week ago 
On 24.11.2025 19:17, Jason Gunthorpe wrote:
> On Mon, Nov 24, 2025 at 06:09:55PM +0100, Marek Szyprowski wrote:
>> Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
>> dma_iova_link() code path, but missed that the CPU cache is being also
>> touched in the dma_iova_unlink() path. Fix this.
>>
>> Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/iommu/dma-iommu.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
> Reviewed-by: Jason Gunthorpe <jgg@nvidia.com>
>
> Did you find it by code inspection?

Yes, I checked how the batched CPu sync will fit there and noticed the 
unlink path, see 
https://lore.kernel.org/all/f8a0b873-b93c-4a91-b47d-2d11bfcf36b4@samsung.com/

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Leon Romanovsky 1 week ago 
On Mon, Nov 24, 2025 at 06:09:55PM +0100, Marek Szyprowski wrote:
> Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
> dma_iova_link() code path, but missed that the CPU cache is being also
> touched in the dma_iova_unlink() path. Fix this.
> 
> Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/iommu/dma-iommu.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 

Thanks,
Reviewed-by: Leon Romanovsky <leon@kernel.org>
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Marek Szyprowski 6 days, 11 hours ago 
On 24.11.2025 18:26, Leon Romanovsky wrote:
> On Mon, Nov 24, 2025 at 06:09:55PM +0100, Marek Szyprowski wrote:
>> Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
>> dma_iova_link() code path, but missed that the CPU cache is being also
>> touched in the dma_iova_unlink() path. Fix this.
>>
>> Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>   drivers/iommu/dma-iommu.c | 5 +++--
>>   1 file changed, 3 insertions(+), 2 deletions(-)
> Reviewed-by: Leon Romanovsky <leon@kernel.org>

Applied to dma-mapping-fixes.

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Robin Murphy 1 week ago 
On 2025-11-24 5:09 pm, Marek Szyprowski wrote:
> Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
> dma_iova_link() code path, but missed that the CPU cache is being also
> touched in the dma_iova_unlink() path. Fix this.
> 
> Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>   drivers/iommu/dma-iommu.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> index e52d19d2e833..c92088855450 100644
> --- a/drivers/iommu/dma-iommu.c
> +++ b/drivers/iommu/dma-iommu.c
> @@ -2008,7 +2008,7 @@ static void iommu_dma_iova_unlink_range_slow(struct device *dev,
>   			end - addr, iovad->granule - iova_start_pad);
>   
>   		if (!dev_is_dma_coherent(dev) &&
> -		    !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> +		    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
>   			arch_sync_dma_for_cpu(phys, len, dir);
>   
>   		swiotlb_tbl_unmap_single(dev, phys, len, dir, attrs);
> @@ -2032,7 +2032,8 @@ static void __iommu_dma_iova_unlink(struct device *dev,
>   	size_t unmapped;
>   
>   	if ((state->__size & DMA_IOVA_USE_SWIOTLB) ||
> -	    (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)))
> +	    (!dev_is_dma_coherent(dev) &&
> +	     !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO))))

This isn't needed, since there's no way an MMIO mapping could have used 
SWIOTLB.

Thanks,
Robin.

>   		iommu_dma_iova_unlink_range_slow(dev, addr, size, dir, attrs);
>   
>   	iommu_iotlb_gather_init(&iotlb_gather);
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Leon Romanovsky 1 week ago 
On Mon, Nov 24, 2025 at 05:15:48PM +0000, Robin Murphy wrote:
> On 2025-11-24 5:09 pm, Marek Szyprowski wrote:
> > Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
> > dma_iova_link() code path, but missed that the CPU cache is being also
> > touched in the dma_iova_unlink() path. Fix this.
> > 
> > Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > ---
> >   drivers/iommu/dma-iommu.c | 5 +++--
> >   1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
> > index e52d19d2e833..c92088855450 100644
> > --- a/drivers/iommu/dma-iommu.c
> > +++ b/drivers/iommu/dma-iommu.c
> > @@ -2008,7 +2008,7 @@ static void iommu_dma_iova_unlink_range_slow(struct device *dev,
> >   			end - addr, iovad->granule - iova_start_pad);
> >   		if (!dev_is_dma_coherent(dev) &&
> > -		    !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
> > +		    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
> >   			arch_sync_dma_for_cpu(phys, len, dir);
> >   		swiotlb_tbl_unmap_single(dev, phys, len, dir, attrs);
> > @@ -2032,7 +2032,8 @@ static void __iommu_dma_iova_unlink(struct device *dev,
> >   	size_t unmapped;
> >   	if ((state->__size & DMA_IOVA_USE_SWIOTLB) ||
> > -	    (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)))
> > +	    (!dev_is_dma_coherent(dev) &&
> > +	     !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO))))
> 
> This isn't needed, since there's no way an MMIO mapping could have used
> SWIOTLB.

This "if" is taken even for non-SWIOTLB path, because of "||" operator
in line above.

Thanks

> 
> Thanks,
> Robin.
> 
> >   		iommu_dma_iova_unlink_range_slow(dev, addr, size, dir, attrs);
> >   	iommu_iotlb_gather_init(&iotlb_gather);
>
Re: [PATCH] iommu/dma: add missing support for DMA_ATTR_MMIO for dma_iova_unlink()
Posted by Robin Murphy 1 week ago 
On 2025-11-24 5:20 pm, Leon Romanovsky wrote:
> On Mon, Nov 24, 2025 at 05:15:48PM +0000, Robin Murphy wrote:
>> On 2025-11-24 5:09 pm, Marek Szyprowski wrote:
>>> Commit c288d657dd51 added support for DMA_ATTR_MMIO attribute in the
>>> dma_iova_link() code path, but missed that the CPU cache is being also
>>> touched in the dma_iova_unlink() path. Fix this.
>>>
>>> Fixes: c288d657dd51 ("iommu/dma: implement DMA_ATTR_MMIO for dma_iova_link().")
>>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>>> ---
>>>    drivers/iommu/dma-iommu.c | 5 +++--
>>>    1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>>> index e52d19d2e833..c92088855450 100644
>>> --- a/drivers/iommu/dma-iommu.c
>>> +++ b/drivers/iommu/dma-iommu.c
>>> @@ -2008,7 +2008,7 @@ static void iommu_dma_iova_unlink_range_slow(struct device *dev,
>>>    			end - addr, iovad->granule - iova_start_pad);
>>>    		if (!dev_is_dma_coherent(dev) &&
>>> -		    !(attrs & DMA_ATTR_SKIP_CPU_SYNC))
>>> +		    !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO)))
>>>    			arch_sync_dma_for_cpu(phys, len, dir);
>>>    		swiotlb_tbl_unmap_single(dev, phys, len, dir, attrs);
>>> @@ -2032,7 +2032,8 @@ static void __iommu_dma_iova_unlink(struct device *dev,
>>>    	size_t unmapped;
>>>    	if ((state->__size & DMA_IOVA_USE_SWIOTLB) ||
>>> -	    (!dev_is_dma_coherent(dev) && !(attrs & DMA_ATTR_SKIP_CPU_SYNC)))
>>> +	    (!dev_is_dma_coherent(dev) &&
>>> +	     !(attrs & (DMA_ATTR_SKIP_CPU_SYNC | DMA_ATTR_MMIO))))
>>
>> This isn't needed, since there's no way an MMIO mapping could have used
>> SWIOTLB.
> 
> This "if" is taken even for non-SWIOTLB path, because of "||" operator
> in line above.

Oof, clearly it's already time to stop trying to read code and go home... :)

Thanks,
Robin.

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.