[PATCH] iommu/vt-d: Remove an unnecessary call set_dma_ops()

[PATCH] iommu/vt-d: Remove an unnecessary call set_dma_ops()

[Petr Tesarik]

Do not touch per-device DMA ops when the driver has been converted to use
the dma-iommu API.

Fixes: c588072bba6b ("iommu/vt-d: Convert intel iommu driver to the iommu ops")
Signed-off-by: Petr Tesarik <ptesarik@suse.com>
---
 drivers/iommu/intel/iommu.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
index bf1f0c814348..b7e500451d4f 100644
--- a/drivers/iommu/intel/iommu.c
+++ b/drivers/iommu/intel/iommu.c
@@ -3789,7 +3789,6 @@ static void intel_iommu_release_device(struct device *dev)
 	intel_pasid_free_table(dev);
 	intel_iommu_debugfs_remove_dev(info);
 	kfree(info);
-	set_dma_ops(dev, NULL);
 }
 
 static void intel_iommu_get_resv_regions(struct device *device,
-- 
2.48.1

Re: [PATCH] iommu/vt-d: Remove an unnecessary call set_dma_ops()

[Petr Tesařík]

On Thu,  3 Apr 2025 18:56:05 +0200
Petr Tesarik <ptesarik@suse.com> wrote:

> Do not touch per-device DMA ops when the driver has been converted to use
> the dma-iommu API.
> 
> Fixes: c588072bba6b ("iommu/vt-d: Convert intel iommu driver to the iommu ops")
> Signed-off-by: Petr Tesarik <ptesarik@suse.com>

Does anyone agree? Or see any issues?

Petr T

> ---
>  drivers/iommu/intel/iommu.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index bf1f0c814348..b7e500451d4f 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -3789,7 +3789,6 @@ static void intel_iommu_release_device(struct device *dev)
>  	intel_pasid_free_table(dev);
>  	intel_iommu_debugfs_remove_dev(info);
>  	kfree(info);
> -	set_dma_ops(dev, NULL);
>  }
>  
>  static void intel_iommu_get_resv_regions(struct device *device,

Re: [PATCH] iommu/vt-d: Remove an unnecessary call set_dma_ops()

[Baolu Lu]

On 4/9/25 23:44, Petr Tesařík wrote:
> On Thu,  3 Apr 2025 18:56:05 +0200
> Petr Tesarik<ptesarik@suse.com> wrote:
> 
>> Do not touch per-device DMA ops when the driver has been converted to use
>> the dma-iommu API.
>>
>> Fixes: c588072bba6b ("iommu/vt-d: Convert intel iommu driver to the iommu ops")
>> Signed-off-by: Petr Tesarik<ptesarik@suse.com>
> Does anyone agree? Or see any issues?

This change looks good to me. The iommu driver should not call
set_dma_ops() anyway. I will queue this for v6.15-rc.

I ever wondered whether we could clean up dev->dma_iommu when the device
is released by the iommu core. But, iommu_release_device() is only
called in the device removal path, so keeping dev->dma_iommu set doesn't
impose any functional impact AFAICS.

Thanks,
baolu

Re: [PATCH] iommu/vt-d: Remove an unnecessary call set_dma_ops()

[Robin Murphy]

On 10/04/2025 3:59 am, Baolu Lu wrote:
> On 4/9/25 23:44, Petr Tesařík wrote:
>> On Thu,  3 Apr 2025 18:56:05 +0200
>> Petr Tesarik<ptesarik@suse.com> wrote:
>>
>>> Do not touch per-device DMA ops when the driver has been converted to 
>>> use
>>> the dma-iommu API.
>>>
>>> Fixes: c588072bba6b ("iommu/vt-d: Convert intel iommu driver to the 
>>> iommu ops")
>>> Signed-off-by: Petr Tesarik<ptesarik@suse.com>
>> Does anyone agree? Or see any issues?

At least it definitely serves no purpose since b5c58b2fdc42 
("dma-mapping: direct calls for dma-iommu"). FWIW,

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

> This change looks good to me. The iommu driver should not call
> set_dma_ops() anyway. I will queue this for v6.15-rc.
> 
> I ever wondered whether we could clean up dev->dma_iommu when the device
> is released by the iommu core. But, iommu_release_device() is only
> called in the device removal path, so keeping dev->dma_iommu set doesn't
> impose any functional impact AFAICS.

Funnily enough I've just been looking at that - I'd long held the same 
assumption, but I'd been forgetting the edge case that's always been 
there if bus_set_iommu(), and now iommu_device_register(), failed and 
groups and default domains were torn down for existing devices. So in 
fact it seems Intel was one of the few drivers/architectures managing to 
do the right thing before :)

Cheers,
Robin.