Re: [PATCH v4 14/27] iommu/arm-smmu-v3: Make changing domains be hitless for ATS

[Jason Gunthorpe <jgg@nvidia.com>]

On Wed, Jan 31, 2024 at 11:20:48AM +0000, Shameerali Kolothum Thodi wrote:

> > +static void arm_smmu_attach_commit(struct arm_smmu_master *master,
> > +				   struct attach_state *state)
> > +{
> > +	lockdep_assert_held(&arm_smmu_asid_lock);
> > +
> > +	if (state->want_ats && !master->ats_enabled) {
> > +		arm_smmu_enable_ats(master);
> > +	} else if (master->ats_enabled) {
> > +		/*
> > +		 * The translation has changed, flush the ATC. At this point the
> > +		 * SMMU is translating for the new domain and both the
> > old&new
> > +		 * domain will issue invalidations.
> > +		 */
> > +		arm_smmu_atc_inv_master(master);
> > +	}
> > +	master->ats_enabled = state->want_ats;
> > +
> > +	arm_smmu_remove_master_domain(master,
> > +				      iommu_get_domain_for_dev(master-
> > >dev));
> 
> I think the core has not yet set the group->domain since we are still in attach().

Any chance you are not running exactly v4? There was an earlier
version on github that had exactly what you said, Nicolin
found it and it was fixed before v4 was posted:

static struct arm_smmu_domain *
to_smmu_domain_devices(struct iommu_domain *domain)
{
        /* The domain can be NULL only when processing the first attach */
        if (!domain)
                return NULL;
      ^^^^^^^^^^^^^^^^^^^^^ This was added
        if (domain->type & __IOMMU_DOMAIN_PAGING)
                return to_smmu_domain(domain);
        return NULL;
}

static void arm_smmu_remove_master_domain(struct arm_smmu_master *master,
                                          struct iommu_domain *domain)
{
        struct arm_smmu_domain *smmu_domain = to_smmu_domain_devices(domain);
        struct arm_smmu_master_domain *master_domain;
        unsigned long flags;

        if (!smmu_domain)  <---- On first attach smmu_domain will be NULL
                return;

(this is what it looks like in my version of v4 at commit
595a8ea0f0ad60e80412dcfbc36b74af7116ba4a)

Otherwise I don't have a guess.. Can you narrow it a bit more?

Thanks,
Jason