Re: [PATCH v2 2/8] iommu/arm-smmu-v3: Move pgsize out of arm_smmu_inv

[Mostafa Saleh <smostafa@google.com>]

On Mon, Jul 06, 2026 at 01:26:39PM -0300, Jason Gunthorpe wrote:
> pgsize is a constant property of the domain, it is the base translation
> granule of the page table (4k, 16k, 64k) in log2.
> 
> Store it to the struct arm_smmu_domain based on how the page table was created.
> 
> Add smmu_domain to the tlbi and just get tg from the domain.
> 
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |  1 +
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 27 ++++++++-----------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 ++-
>  3 files changed, 14 insertions(+), 17 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 1ed8a6f29dc445..5d4dde3d1cfe87 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
> @@ -334,6 +334,7 @@ struct iommu_domain *arm_smmu_sva_domain_alloc(struct device *dev,
>  	 * ARM_SMMU_FEAT_RANGE_INV is present
>  	 */
>  	smmu_domain->domain.pgsize_bitmap = PAGE_SIZE;
> +	smmu_domain->tgsz_lg2 = PAGE_SHIFT;
>  	smmu_domain->stage = ARM_SMMU_DOMAIN_SVA;
>  	smmu_domain->smmu = smmu;
>  
> 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 0c875771b46d2f..d22012466e3965 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2398,14 +2398,13 @@ static void arm_smmu_tlb_inv_context(void *cookie)
>  static void arm_smmu_cmdq_batch_add_range(struct arm_smmu_device *smmu,
>  					  struct arm_smmu_cmdq_batch *cmds,
>  					  struct arm_smmu_cmd *cmd,
> -					  struct arm_smmu_tlbi *tlbi,
> -					  size_t pgsize)
> +					  struct arm_smmu_tlbi *tlbi)
>  {
>  	size_t inv_range = tlbi->iopte_granule;
>  	unsigned long iova = tlbi->iova;
>  	unsigned long end = iova + tlbi->size;
>  	unsigned long num_pages = 0;
> -	unsigned int tg = pgsize;
> +	unsigned int tg = tlbi->smmu_domain->tgsz_lg2;
>  	u64 orig_data0 = cmd->data[0];
>  	u8 ttl = 0, tg_enc = 0;
>  
> @@ -2513,7 +2512,7 @@ static void arm_smmu_inv_to_cmdq_batch(struct arm_smmu_inv *inv,
>  		return;
>  	}
>  
> -	arm_smmu_cmdq_batch_add_range(inv->smmu, cmds, cmd, tlbi, inv->pgsize);
> +	arm_smmu_cmdq_batch_add_range(inv->smmu, cmds, cmd, tlbi);
>  }
>  
>  static inline bool arm_smmu_invs_end_batch(struct arm_smmu_inv *cur,
> @@ -2615,6 +2614,7 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  			       unsigned int granule, bool leaf)
>  {
>  	struct arm_smmu_tlbi tlbi = {
> +		.smmu_domain = smmu_domain,
>  		.iova = iova,
>  		.size = size,
>  		.iopte_granule = granule,
> @@ -2870,6 +2870,7 @@ static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
>  		return -ENOMEM;
>  
>  	smmu_domain->domain.pgsize_bitmap = pgtbl_cfg.pgsize_bitmap;
> +	smmu_domain->tgsz_lg2 = __ffs(pgtbl_cfg.pgsize_bitmap);
>  	smmu_domain->domain.geometry.aperture_end = (1UL << pgtbl_cfg.ias) - 1;
>  	smmu_domain->domain.geometry.force_aperture = true;
>  	if (enable_dirty && smmu_domain->stage == ARM_SMMU_DOMAIN_S1)
> @@ -3104,15 +3105,13 @@ static void arm_smmu_disable_iopf(struct arm_smmu_master *master,
>  
>  static struct arm_smmu_inv *
>  arm_smmu_master_build_inv(struct arm_smmu_master *master,
> -			  enum arm_smmu_inv_type type, u32 id, ioasid_t ssid,
> -			  size_t pgsize)
> +			  enum arm_smmu_inv_type type, u32 id, ioasid_t ssid)
>  {
>  	struct arm_smmu_invs *build_invs = master->build_invs;
>  	struct arm_smmu_inv *cur, inv = {
>  		.smmu = master->smmu,
>  		.type = type,
>  		.id = id,
> -		.pgsize = pgsize,
>  	};
>  
>  	if (WARN_ON(build_invs->num_invs >= build_invs->max_invs))
> @@ -3164,28 +3163,24 @@ arm_smmu_master_build_invs(struct arm_smmu_master *master, bool ats_enabled,
>  			   ioasid_t ssid, struct arm_smmu_domain *smmu_domain)
>  {
>  	const bool nesting = smmu_domain->nest_parent;
> -	size_t pgsize = 0, i;
> +	size_t i;
>  
>  	iommu_group_mutex_assert(master->dev);
>  
>  	master->build_invs->num_invs = 0;
>  
> -	/* Range-based invalidation requires the leaf pgsize for calculation */
> -	if (master->smmu->features & ARM_SMMU_FEAT_RANGE_INV)
> -		pgsize = __ffs(smmu_domain->domain.pgsize_bitmap);
> -
>  	switch (smmu_domain->stage) {
>  	case ARM_SMMU_DOMAIN_SVA:
>  	case ARM_SMMU_DOMAIN_S1:
>  		if (!arm_smmu_master_build_inv(master, INV_TYPE_S1_ASID,
>  					       smmu_domain->cd.asid,
> -					       IOMMU_NO_PASID, pgsize))
> +					       IOMMU_NO_PASID))
>  			return NULL;
>  		break;
>  	case ARM_SMMU_DOMAIN_S2:
>  		if (!arm_smmu_master_build_inv(master, INV_TYPE_S2_VMID,
>  					       smmu_domain->s2_cfg.vmid,
> -					       IOMMU_NO_PASID, pgsize))
> +					       IOMMU_NO_PASID))
>  			return NULL;
>  		break;
>  	default:
> @@ -3197,7 +3192,7 @@ arm_smmu_master_build_invs(struct arm_smmu_master *master, bool ats_enabled,
>  	if (nesting) {
>  		if (!arm_smmu_master_build_inv(
>  			    master, INV_TYPE_S2_VMID_S1_CLEAR,
> -			    smmu_domain->s2_cfg.vmid, IOMMU_NO_PASID, 0))
> +			    smmu_domain->s2_cfg.vmid, IOMMU_NO_PASID))
>  			return NULL;
>  	}
>  
> @@ -3208,7 +3203,7 @@ arm_smmu_master_build_invs(struct arm_smmu_master *master, bool ats_enabled,
>  		 */
>  		if (!arm_smmu_master_build_inv(
>  			    master, nesting ? INV_TYPE_ATS_FULL : INV_TYPE_ATS,
> -			    master->streams[i].id, ssid, 0))
> +			    master->streams[i].id, ssid))
>  			return NULL;
>  	}
>  
> 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 a364d847c22a92..2fc695817671fe 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -734,7 +734,6 @@ struct arm_smmu_inv {
>  	u8 nsize_opcode;
>  	u32 id; /* ASID or VMID or SID */
>  	union {
> -		size_t pgsize; /* ARM_SMMU_FEAT_RANGE_INV */
>  		u32 ssid; /* INV_TYPE_ATS */
>  	};
>  
> @@ -803,6 +802,7 @@ static inline struct arm_smmu_invs *arm_smmu_invs_alloc(size_t num_invs)
>  }
>  
>  struct arm_smmu_tlbi {
> +	struct arm_smmu_domain *smmu_domain;

The smmu_domain is only used for the tgsz_lg2, can’t we add that
directly instead? Decoupling the invalidation form the domain would
make it easier for KVM to re-use the code.
I see by the end of the series the only difference would be
arm_smmu_domain_tlbi() which need to the domain for the RCU, but that
can be passed instead as all the callers are have the domain already.

Thanks,
Mostafa

>  	unsigned long iova;
>  	size_t size;
>  	unsigned int iopte_granule;
> @@ -1050,6 +1050,7 @@ struct arm_smmu_domain {
>  	spinlock_t			devices_lock;
>  	bool				enforce_cache_coherency : 1;
>  	bool				nest_parent : 1;
> +	u8				tgsz_lg2;
>  
>  	struct mmu_notifier		mmu_notifier;
>  };
> -- 
> 2.43.0
>