On Wed, Nov 29, 2023 at 05:43:03PM +0000, Robin Murphy wrote: > It's somewhat hard to see, but arm64's arch_setup_dma_ops() should only > ever call iommu_setup_dma_ops() after a successful iommu_probe_device(), > which means there should be no harm in instead running it off the back > of iommu_probe_device() itself, as is currently done for x86 and s390 > with .probe_finalize bodges. Pull it all into the main flow properly. > > Signed-off-by: Robin Murphy <robin.murphy@xxxxxxx> > --- > arch/arm64/mm/dma-mapping.c | 2 -- > drivers/iommu/amd/iommu.c | 8 -------- > drivers/iommu/dma-iommu.c | 17 ++++------------- > drivers/iommu/dma-iommu.h | 6 ++++++ > drivers/iommu/intel/iommu.c | 7 ------- > drivers/iommu/iommu.c | 2 ++ > drivers/iommu/s390-iommu.c | 6 ------ > drivers/iommu/virtio-iommu.c | 10 ---------- > include/linux/iommu.h | 7 ------- > 9 files changed, 12 insertions(+), 53 deletions(-) Yes! That probe_finalize() stuff is not nice > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c > index 824989874dee..3a0901165b69 100644 > --- a/drivers/iommu/iommu.c > +++ b/drivers/iommu/iommu.c > @@ -589,6 +589,8 @@ int iommu_probe_device(struct device *dev) > if (ret) > return ret; > > + iommu_setup_dma_ops(dev); > + I'm pretty sure this should be inside the group mutex lock. The setting of dev->dma_ops should exactly follow the setting of group->domain, and all transitions, including from the sysfs override file should update it, right? Thus to avoid races agsinst concurrent sysfs this should be locked. I think you can just put this in iommu_setup_default_domain() and it will take care of all the cases? Once in iommu_setup_default_domain() it is easy to call it with the domain argument and it can know the domain type without using iommu_is_dma_domain() which is one of the very last few places checking for the DMA domain type. Jason
