Re: [PATCH v2 6/8] iommu/arm-smmu-v3: Populate the tlbi at the top of the call chain

[Mostafa Saleh <smostafa@google.com>]

On Mon, Jul 06, 2026 at 01:26:43PM -0300, Jason Gunthorpe wrote:
> Each of these has their own unique situation, populate the tlbi right
> at the top and pass it into arm_smmu_domain_inv_range(). They will
> diverge further when the iommupt invalidation scheme is introduced.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   | 21 ++++----
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 51 +++++++++----------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   | 13 +++--
>  3 files changed, 45 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> index 5d4dde3d1cfe87..e6001913e2b043 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
> @@ -139,16 +139,19 @@ static void arm_smmu_mm_arch_invalidate_secondary_tlbs(struct mmu_notifier *mn,
>  {
>  	struct arm_smmu_domain *smmu_domain =
>  		container_of(mn, struct arm_smmu_domain, mmu_notifier);
> -	size_t size;
> +	struct arm_smmu_tlbi tlbi = {
> +		.smmu_domain = smmu_domain,
> +		.iova = start,
> +		/*
> +		 * The mm_types defines vm_end as the first byte after the end
> +		 * address, different from IOMMU subsystem using the last
> +		 * address of an address range.
> +		 */
> +		.size = end - start,
> +		.iopte_granule = PAGE_SIZE,
> +	};
>  
> -	/*
> -	 * The mm_types defines vm_end as the first byte after the end address,
> -	 * different from IOMMU subsystem using the last address of an address
> -	 * range. So do a simple translation here by calculating size correctly.
> -	 */
> -	size = end - start;
> -
> -	arm_smmu_domain_inv_range(smmu_domain, start, size, PAGE_SIZE, false);
> +	arm_smmu_domain_tlbi(&tlbi);
>  }
>  
>  static void arm_smmu_mm_release(struct mmu_notifier *mn, struct mm_struct *mm)
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index fbe3e5dc42f964..2e477f15080148 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2661,25 +2661,12 @@ static void arm_smmu_domain_tlbi_inv(struct arm_smmu_tlbi *tlbi,
>  	}
>  }
>  
> -void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
> -			       unsigned long iova, size_t size,
> -			       unsigned int granule, bool leaf)
> +void arm_smmu_domain_tlbi(struct arm_smmu_tlbi *tlbi)
>  {
> -	struct arm_smmu_tlbi tlbi = {
> -		.smmu_domain = smmu_domain,
> -		.iova = iova,
> -		.size = size,
> -		.iopte_granule = granule,
> -		.leaf_only = leaf,
> -	};
>  	struct arm_smmu_invs *invs;
>  
> -	if (!size || size == SIZE_MAX) {
> -		tlbi.single.use_full_inv = true;
> -		tlbi.range.use_full_inv = true;
> -	} else {
> -		arm_smmu_tlbi_calc_single(&tlbi);
> -	}
> +	if (!tlbi->single.use_full_inv)
> +		arm_smmu_tlbi_calc_single(tlbi);
>  
>  	/*
>  	 * An invalidation request must follow some IOPTE change and then load

The rest of the comment still refers to the old name
arm_smmu_domain_inv_range()

> @@ -2709,14 +2696,14 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  	smp_mb();
>  
>  	rcu_read_lock();
> -	invs = rcu_dereference(smmu_domain->invs);
> +	invs = rcu_dereference(tlbi->smmu_domain->invs);
>  
>  	/* Only precaculate RIL if it will be used. */
>  	if (invs->has_range_inv) {
> -		if (!tlbi.range.use_full_inv)
> -			arm_smmu_tlbi_calc_range(&tlbi);
> +		if (!tlbi->range.use_full_inv)
> +			arm_smmu_tlbi_calc_range(tlbi);
>  	} else {
> -		tlbi.range.use_full_inv = true;
> +		tlbi->range.use_full_inv = true;
>  	}
>  
>  	/*
> @@ -2727,10 +2714,10 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  		unsigned long flags;
>  
>  		read_lock_irqsave(&invs->rwlock, flags);
> -		arm_smmu_domain_tlbi_inv(&tlbi, invs);
> +		arm_smmu_domain_tlbi_inv(tlbi, invs);
>  		read_unlock_irqrestore(&invs->rwlock, flags);
>  	} else {
> -		arm_smmu_domain_tlbi_inv(&tlbi, invs);
> +		arm_smmu_domain_tlbi_inv(tlbi, invs);
>  	}
>  
>  	rcu_read_unlock();
> @@ -2750,8 +2737,14 @@ static void arm_smmu_tlb_inv_walk(unsigned long iova, size_t size,
>  				  size_t granule, void *cookie)
>  {
>  	struct arm_smmu_domain *smmu_domain = cookie;
> +	struct arm_smmu_tlbi tlbi = {
> +		.smmu_domain = smmu_domain,
> +		.iova = iova,
> +		.size = size,
> +		.iopte_granule = granule,
> +	};
>  
> -	arm_smmu_domain_inv_range(smmu_domain, iova, size, granule, false);
> +	arm_smmu_domain_tlbi(&tlbi);
>  }
>  
>  static const struct iommu_flush_ops arm_smmu_flush_ops = {
> @@ -4018,14 +4011,18 @@ static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
>  static void arm_smmu_iotlb_sync(struct iommu_domain *domain,
>  				struct iommu_iotlb_gather *gather)
>  {
> -	struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> +	struct arm_smmu_tlbi tlbi = {
> +		.smmu_domain = to_smmu_domain(domain),
> +		.iova = gather->start,
> +		.size = gather->end - gather->start + 1,
> +		.iopte_granule = gather->pgsize,
> +		.leaf_only = true,
> +	};
>  
>  	if (!gather->pgsize)
>  		return;
>  
> -	arm_smmu_domain_inv_range(smmu_domain, gather->start,
> -				  gather->end - gather->start + 1,
> -				  gather->pgsize, true);
> +	arm_smmu_domain_tlbi(&tlbi);
>  }
>  
>  static phys_addr_t
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 9d262ef6076225..5f97d1a63ebbfd 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -1169,13 +1169,18 @@ 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);
>  
> -void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
> -			       unsigned long iova, size_t size,
> -			       unsigned int granule, bool leaf);
> +void arm_smmu_domain_tlbi(struct arm_smmu_tlbi *tlbi);
>  
>  static inline void arm_smmu_domain_inv(struct arm_smmu_domain *smmu_domain)
>  {
> -	arm_smmu_domain_inv_range(smmu_domain, 0, 0, 0, false);
> +	/* Prefilled for invalidate all */
> +	struct arm_smmu_tlbi tlbi = {
> +		.smmu_domain = smmu_domain,
> +		.single.use_full_inv = true,
> +		.range.use_full_inv = true,

Those were introduced last patch, but I am wondering if use_full_inv
should be a common field instead of having it in both single and range.

Thanks,
Mostafa

> +	};
> +
> +	arm_smmu_domain_tlbi(&tlbi);
>  }
>  
>  void __arm_smmu_cmdq_skip_err(struct arm_smmu_device *smmu,
> -- 
> 2.43.0
>