RE: [PATCH V3 7/8] arm/arm64: dma-mapping: Call iommu's remove_device callback during device detach

["Sricharan" <sricharan@codeaurora.org>]

Hi Robin,

>> dma_deconfigure calls arch_teardown_dma_ops in the device_detach path,
>> which is called when the device gets detached from the driver.
>> When the device was added, iommu's add_device callback was used to
>> add the device in to its iommu_group and setup the device to be ready
>> to use its iommu. Similarly, call remove_device callback to remove the
>> device from the group and reset any other device's iommu configurations.
>>
>> Signed-off-by: Sricharan R <sricharan@codeaurora.org>
>> ---
>>  arch/arm/mm/dma-mapping.c   | 8 ++++++++
>>  arch/arm64/mm/dma-mapping.c | 7 +++++++
>>  2 files changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> index b9191f0..cbe22de 100644
>> --- a/arch/arm/mm/dma-mapping.c
>> +++ b/arch/arm/mm/dma-mapping.c
>> @@ -2289,11 +2289,19 @@ static bool arm_setup_iommu_dma_ops(struct device *dev, u64 dma_base, u64 size,
>>  static void arm_teardown_iommu_dma_ops(struct device *dev)
>>  {
>>  	struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>> +	const struct iommu_ops *ops;
>>
>>  	if (!mapping)
>>  		return;
>>
>>  	__arm_iommu_detach_device(dev);
>> +
>> +	if (dev->iommu_fwspec) {
>> +		ops = dev->iommu_fwspec->ops;
>> +		if (ops->remove_device)
>> +			ops->remove_device(dev);
>> +	}
>> +
>
>Yuck. It's a little unfortunate that we have to do this at all, but I
>see why. Still, it should be done in common code, not duplicated across
>arch code, not least for symmetry with where the matching add_device
>happened (although I think of_dma_deconfigure() would suffice, I'm not
>sure we really need to add an of_iommu_deconfigure() just for this).
>

 So one way is its already called via a the BUS_NOTIFY_REMOVED_DEVICE
 notifier in iommu_bus_notifier. I put it here, one for symmetry and
 another thinking that the remove_device callback should be done
 before the dma_ops is set to NULL. If thats not required then the existing
 iommu_bus_notifier itself can do the cleanup. The corresponding
 add_device in that notifier is dummy now, i will add a patch to remove
 that. If not the whole thing, we can add of_iommu_deconfigure as well.

>It's also broken for IOMMU drivers which rely on the
>of_iommu_configure() mechanism but have not yet been converted to use
>iommu_fwspec (Exynos, MSM, etc.)
>

 I did this, because fwspec was the right way to get ops in the future,
 but yes its getting broken for those which are not converted,
 will have to use dev->bus->iommu_ops in those cases. Will address this
 while changing the above.

Regards,
 Sricharan