Re: [PATCH v4 10/10] iommu/arm-smmu-v3: Allow sharing domain across SMMUs

[Jason Gunthorpe]

On Thu, Apr 09, 2026 at 09:32:23PM -0300, Jason Gunthorpe wrote:
> On Thu, Mar 19, 2026 at 12:51:56PM -0700, Nicolin Chen wrote:
> > @@ -987,6 +988,32 @@ struct arm_smmu_nested_domain {
> >  	__le64 ste[2];
> >  };
> >  
> > +static inline bool
> > +arm_smmu_domain_can_share(struct arm_smmu_domain *smmu_domain,
> > +			  struct arm_smmu_device *new_smmu)
> > +{
> 
> Probably a bit big for an inline
> 
> > +	struct io_pgtable *pgtbl =
> > +		io_pgtable_ops_to_pgtable(smmu_domain->pgtbl_ops);
> > +
> > +	if (pgtbl->fmt == ARM_64_LPAE_S1 &&
> > +	    !(new_smmu->features & ARM_SMMU_FEAT_TRANS_S1))
> > +		return false;
> > +	if (pgtbl->fmt == ARM_64_LPAE_S2 &&
> > +	    !(new_smmu->features & ARM_SMMU_FEAT_TRANS_S2))
> > +		return false;
> > +	if (pgtbl->cfg.pgsize_bitmap & ~new_smmu->pgsize_bitmap)
> > +		return false;
> 
> I think this should check the lowest set bit in the
> domain->pgsize_bitmap is set in new_smmu->pgsize_bitmap
> 
> ie that the selected tg is supported.
> 
> The cfg.pgsize_bitmap is something a little different IIRC
> 
> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>

Though something else is missing here, I expected this to be removed too:

struct arm_smmu_domain {
	struct arm_smmu_device		*smmu;

What is left using it?

static int arm_smmu_s1_set_dev_pasid(struct iommu_domain *domain,
				     struct device *dev, ioasid_t id,
				     struct iommu_domain *old)

int arm_smmu_set_pasid(struct arm_smmu_master *master,
		       struct arm_smmu_domain *smmu_domain, ioasid_t pasid,
		       struct arm_smmu_cd *cd, struct iommu_domain *old,
		       arm_smmu_make_cd_fn make_cd_fn)

Thous should use the new helper right? It should work for a S1 domain
too.

static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
{
	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);

	if (smmu_domain->smmu)
		arm_smmu_tlb_inv_context(smmu_domain);
}

I suspect that is just dead code now, it is from before finalize was
part of alloc?

Jason