Re: [PATCH 1/3] iommu: Add Visconti5 IOMMU driver

[Jason Gunthorpe <jgg@nvidia.com>]

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