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

Jason Gunthorpe
2025-09-15T12:35:23Z

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