On Wed, May 25, 2022 at 02:26:37PM +0800, Baolu Lu wrote: > On 2022/5/25 09:31, Nobuhiro Iwamatsu wrote: > > +static const struct iommu_ops visconti_atu_ops = { > > + .domain_alloc = visconti_atu_domain_alloc, > > + .probe_device = visconti_atu_probe_device, > > + .release_device = visconti_atu_release_device, > > + .device_group = generic_device_group, > > + .of_xlate = visconti_atu_of_xlate, > > + .pgsize_bitmap = ATU_IOMMU_PGSIZE_BITMAP, > > + .default_domain_ops = &(const struct iommu_domain_ops) { > > + .attach_dev = visconti_atu_attach_device, > > + .detach_dev = visconti_atu_detach_device, > > The detach_dev callback is about to be deprecated. The new drivers > should implement the default domain and blocking domain instead. Yes please, new drivers need to use default_domains. It is very strange that visconti_atu_detach_device() does nothing. It is not required that a domain is fully unmapped before being destructed, I think detach should set ATU_AT_EN to 0. What behavior does the HW have when ATU_AT_ENTRY_EN == 0? If DMA is rejected then this driver should have a IOMMU_DOMAIN_BLOCKING and return that from ops->def_domain_type(). Attaching a the blocking domain should set ATU_AT_ENTRY_EN = 0 Also, if I surmise how this works properly, it is not following the iommu API to halt all DMA during map/unmap operations. Should at least document this and explain why it is OK.. I'm feeling like these "special" drivers need some kind of handshake with their only users because they don't work with things like VFIO.. Jason