[PATCH 14/27] iommu/arm-smmu-v3: Make changing domains be hitless for ATS
Jason Gunthorpe
jgg at nvidia.com
Tue Oct 24 16:56:49 PDT 2023
On Tue, Oct 24, 2023 at 04:09:06PM +0800, Michael Shavit wrote:
> > + /*
> > + * During prepare we want the current smmu_domain and new smmu_domain to
> > + * be in the devices list before we change any HW. This ensures that
> > + * both domains will send ATS invalidations to the master until we are
> > + * done.
> > + *
> > + * It is tempting to make this list only track masters that are using
> > + * ATS, but arm_smmu_share_asid() also uses this to change the ASID of a
> > + * domain, unrelated to ATS.
> > + */
> > + spin_lock_irqsave(&smmu_domain->devices_lock, flags);
> > + cur_master_domain = arm_smmu_find_master_domain(smmu_domain, master);
> > + if (cur_master_domain) {
> > + kfree(master_domain);
> > + state->existing_master_domain = true;
>
> I'm confused, why would we expect to find that the domain is already
> attached to the master???
Yes, it can happen, I agree it is
surprising. __iommu_group_set_domain_internal()'s error handling is
designed to support the wonky drivers (like smmu before this series)
which would destroy the domain attachment and then fail
attach_dev().
To accommodate this the error unwind for
__iommu_group_set_domain_internal() deliberately resets the correct
domain again, expecting good drivers to make it into a NOP and wonky
drivers to restore the attachment they threw away.
> If the iommu core really allows such attach() calls, can we check
> for that earlier and simply return early from attach_dev()?
We could, but that looked more complex.. We trade an extra if
under arm_smmu_attach_commit() for an extra if in two callers.
What do you think?
Jason
More information about the linux-arm-kernel
mailing list