Re: [PATCH v4 1/7] iommu/arm-smmu-v3: Add release_domain to attach prior to release_dev()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Sep 12, 2025 at 09:33:06AM +0000, Tian, Kevin wrote:
> > From: Nicolin Chen <nicolinc@nvidia.com>
> > Sent: Monday, September 1, 2025 7:32 AM
> > 
> > +static int arm_smmu_attach_dev_release(struct iommu_domain *domain,
> > +				       struct device *dev)
> > +{
> > +	struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> > +
> > +	WARN_ON(master->iopf_refcount);

This doesn't look right anymore..

Now that iopf is managed automatically it technically doesn't go to
zero until the attaches below:

> > +
> > +	/* Put the STE back to what arm_smmu_init_strtab() sets */
> > +	if (dev->iommu->require_direct)
> > +
> > 	arm_smmu_attach_dev_identity(&arm_smmu_identity_domain,
> > dev);
> > +	else
> > +
> > 	arm_smmu_attach_dev_blocked(&arm_smmu_blocked_domain,
> > dev);

And I'd argue the attaches internally should have the assertion. If no
pasids and blocked/identity the iopf == 0.

> it's a bit confusing that a BLOCKED domain type could turn to the
> identity mode, though this movement doesn't change the original
> behavior.

That isn't what is happening here..

If dev->iommu->require_direct is set we prevent attaching BLOCKING
domains entirely:

	if (dev->iommu->require_direct &&
	    (new_domain->type == IOMMU_DOMAIN_BLOCKED ||
	     new_domain == group->blocking_domain)) {
		dev_warn(dev,
			 "Firmware has requested this device have a 1:1 IOMMU mapping, rejecting configuring the device without a 1:1 mapping. Contact your platform vendor.\n");
		return -EINVAL;
	}

So in most sane cases the above will never convert BLOCKING to
IDENTITY. What it is doing is preserving the RMRs...

Also, I don't think this should be in the smmu driver, every driver
should have this same logic, it is part of the definition of RMR
Let's put it in the core code:

	if (!dev->iommu->attach_deferred && ops->release_domain) {
		struct iommu_domain *release_domain = ops->release_domain;

		/*
		 * If the device requires direct mappings then it should not 
		 * be parked on a BLOCKED domain during release as that would
		 * break the direct mappings.
		 */
		if (dev->iommu->require_direct && ops->identity_domain &&
		    release_domain == ops->blocked_domain)
			release_domain = ops->identity_domain;

		release_domain->ops->attach_dev(release_domain, dev);
	}

Jason