Linux Documentation
 help / color / mirror / Atom feed
From: Mostafa Saleh <smostafa@google.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
	Jonathan Corbet <corbet@lwn.net>,
	iommu@lists.linux.dev, "Joerg Roedel (AMD)" <joro@8bytes.org>,
	Jean-Philippe Brucker <jpb@kernel.org>,
	linux-arm-kernel@lists.infradead.org, linux-doc@vger.kernel.org,
	Mark Rutland <mark.rutland@arm.com>,
	Randy Dunlap <rdunlap@infradead.org>,
	Robin Murphy <robin.murphy@arm.com>,
	Shuah Khan <skhan@linuxfoundation.org>,
	Will Deacon <will@kernel.org>,
	David Matlack <dmatlack@google.com>,
	Jean-Philippe Brucker <jean-philippe@linaro.org>,
	Jonathan Cameron <Jonathan.Cameron@huawei.com>,
	Nicolin Chen <nicolinc@nvidia.com>,
	Pasha Tatashin <pasha.tatashin@soleen.com>,
	patches@lists.linux.dev, Pranjal Shrivastava <praan@google.com>,
	Samiullah Khawaja <skhawaja@google.com>,
	stable@vger.kernel.org,
	Vijayanand Jitta <vijayanand.jitta@oss.qualcomm.com>
Subject: Re: [PATCH v5 6/9] iommu/arm-smmu-v3: Precompute the invalidation commands
Date: Mon, 7 Sep 2026 14:27:52 +0000	[thread overview]
Message-ID: <ap7J6NKXgWhiwXgL@google.com> (raw)
In-Reply-To: <6-v5-b810cf379bfc+13d738-smmu_tlbi_jgg@nvidia.com>

On Tue, Sep 01, 2026 at 02:49:55PM -0300, Jason Gunthorpe wrote:
> Store the required cmd data in the tlbi and just copy it out when
> processing each item in the invs list. The cmd form only depends on
> if the instance supports RIL or not, otherwise it is always the same.
> 
> This avoids redundant calculations for each invs entry.

As I mentioned on v2, I don’t see a value for this without smmu
sharing over the same domain, as this just adds extra complexity IMHO,
but that's up to Robin and Will.

Thanks,
Mostafa

> 
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> Tested-by: Nicolin Chen <nicolinc@nvidia.com>
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 141 +++++++++++---------
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  12 +-
>  2 files changed, 91 insertions(+), 62 deletions(-)
> 
> 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 68dd7b69392737..883dfc584ed6c6 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2506,14 +2506,12 @@ static struct arm_smmu_ril_range arm_smmu_ril_init_end(u64 last_tg, u64 num_tg)
>  	return ril;
>  }
>  
> -static void arm_smmu_cmdq_batch_add_ril(struct arm_smmu_device *smmu,
> -					struct arm_smmu_cmdq_batch *cmds,
> -					struct arm_smmu_cmd *ref_cmd,
> -					bool leaf_only,
> +static void arm_smmu_tlbi_add_range_cmd(struct arm_smmu_tlbi *tlbi,
>  					const struct arm_smmu_ril_range *ril,
>  					u8 ttl, u8 tg_enc)
>  {
> -	struct arm_smmu_cmd cmd;
> +	struct arm_smmu_cmd *cmd =
> +		&tlbi->range.cmds[tlbi->range.num_cmds++];
>  	unsigned int tgsz_lg2 = tg_enc * 2 + 10;
>  	u64 iova = ril->start_tg << tgsz_lg2;
>  	unsigned int num = ril->num - 1;
> @@ -2542,18 +2540,16 @@ static void arm_smmu_cmdq_batch_add_ril(struct arm_smmu_device *smmu,
>  	if (!num && !ril->scale && !ttl)
>  		tg_enc = 0;
>  
> -	cmd.data[0] = ref_cmd->data[0] | FIELD_PREP(CMDQ_TLBI_0_NUM, num) |
> -		      FIELD_PREP(CMDQ_TLBI_0_SCALE, ril->scale);
> -	cmd.data[1] = ref_cmd->data[1] |
> -		      FIELD_PREP(CMDQ_TLBI_1_LEAF, leaf_only) |
> -		      FIELD_PREP(CMDQ_TLBI_1_TTL, ttl) |
> -		      FIELD_PREP(CMDQ_TLBI_1_TG, tg_enc) | iova;
> -	arm_smmu_cmdq_batch_add_cmd_p(smmu, cmds, &cmd);
> +	cmd->data[0] = FIELD_PREP(CMDQ_TLBI_0_NUM, num) |
> +		       FIELD_PREP(CMDQ_TLBI_0_SCALE, ril->scale);
> +	cmd->data[1] = FIELD_PREP(CMDQ_TLBI_1_LEAF, tlbi->leaf_only) |
> +		       FIELD_PREP(CMDQ_TLBI_1_TTL, ttl) |
> +		       FIELD_PREP(CMDQ_TLBI_1_TG, tg_enc) | iova;
>  }
>  
>  /*
> - * Issue up to two range TLBI commands covering [iova, iova+size). Returns true
> - * if successful, false if the range is too large to fit into RIL commands.
> + * Generate up to two range TLBI command payloads covering [iova, iova+size).
> + * Sets use_full_inv if the range is too large to represent.
>   *
>   * Normally the first RIL is the largest representable span which does not
>   * exceed the requested range. If necessary, the second RIL is the smallest
> @@ -2561,14 +2557,12 @@ static void arm_smmu_cmdq_batch_add_ril(struct arm_smmu_device *smmu,
>   * excess coverage from the second RIL overlaps the first instead of exceeding
>   * the requested range.
>   *
> - * If a SVA is being invalidated and the SMMU has the ARM_SMMU_OPT_FULL_CONT_RIL
> - * errata this produces only a single RIL and overinvalidates to ensure any
> - * potential CONT is covered with a single RIL.
> + * For SVA on an invs containing an SMMU with ARM_SMMU_OPT_FULL_CONT_RIL,
> + * produce only a single RIL and overinvalidate so any potential CONT is
> + * covered by one command.
>   */
> -static bool 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)
> +static void arm_smmu_tlbi_calc_range(struct arm_smmu_tlbi *tlbi,
> +				     bool single_ril)
>  {
>  	u8 tgsz_lg2 = tlbi->tgsz_lg2;
>  	struct arm_smmu_ril_range first = { .start_tg = tlbi->iova >>
> @@ -2579,9 +2573,6 @@ static bool arm_smmu_cmdq_batch_add_range(struct arm_smmu_device *smmu,
>  	struct arm_smmu_ril_range trail;
>  	u8 ttl = 0;
>  
> -	if (!tlbi->size)
> -		return false;
> -
>  	/*
>  	 * Determine what level the granule is at. For non-leaf, both
>  	 * io-pgtable and SVA pass a nominal last-level granule because they
> @@ -2602,10 +2593,11 @@ static bool arm_smmu_cmdq_batch_add_range(struct arm_smmu_device *smmu,
>  	first.scale = fls64((num_tg - 1) / 32);
>  	if (first.scale > 31) {
>  		/* Range too large for a single command do full invalidation */
> -		return false;
> +		tlbi->range.use_full_inv = true;
> +		return;
>  	}
>  
> -	if (tlbi->has_cont && (smmu->options & ARM_SMMU_OPT_FULL_CONT_RIL)) {
> +	if (single_ril) {
>  		/*
>  		 * Produce a single invalidation by rounding up and disabling
>  		 * the trailer.
> @@ -2621,40 +2613,27 @@ static bool arm_smmu_cmdq_batch_add_range(struct arm_smmu_device *smmu,
>  		trail = arm_smmu_ril_init_end(
>  			last_tg, num_tg - ((u64)first.num << first.scale));
>  	}
> -	arm_smmu_cmdq_batch_add_ril(smmu, cmds, cmd, tlbi->leaf_only, &first,
> -				    ttl, tg_enc);
> +	arm_smmu_tlbi_add_range_cmd(tlbi, &first, ttl, tg_enc);
>  
>  	if (trail.num)
> -		arm_smmu_cmdq_batch_add_ril(smmu, cmds, cmd, tlbi->leaf_only,
> -					    &trail, ttl, tg_enc);
> -	return true;
> +		arm_smmu_tlbi_add_range_cmd(tlbi, &trail, ttl, tg_enc);
>  }
>  
>  /*
>   * One TLBI command per IOTLB entry, assuming the entries are all at least
> - * iopte_granule sized. Returns false if too many commands would be needed which
> - * indicates too high a latency. The threshold is similar to MAX_DVM_OPS in
> - * arch/arm64/include/asm/tlbflush.h for the 4k PAGE_SIZE.
> + * iopte_granule sized. Sets use_full_inv if too many commands would be needed
> + * which indicates too high a latency. The threshold is similar to MAX_DVM_OPS
> + * in arch/arm64/include/asm/tlbflush.h for the 4k PAGE_SIZE.
>   */
> -static bool arm_smmu_cmdq_batch_add_single(struct arm_smmu_device *smmu,
> -					   struct arm_smmu_cmdq_batch *cmds,
> -					   struct arm_smmu_cmd *cmd,
> -					   struct arm_smmu_tlbi *tlbi)
> +static void arm_smmu_tlbi_calc_single(struct arm_smmu_tlbi *tlbi)
>  {
>  	unsigned long num_ops = tlbi->size / tlbi->iopte_size;
> -	unsigned long iova = tlbi->iova;
> -	unsigned long i;
>  
> -	if (!num_ops || num_ops > 512)
> -		return false;
> -
> -	for (i = 0; i < num_ops; i++) {
> -		cmd->data[1] = FIELD_PREP(CMDQ_TLBI_1_LEAF, tlbi->leaf_only) |
> -			       (iova & ~GENMASK_U64(11, 0));
> -		arm_smmu_cmdq_batch_add_cmd_p(smmu, cmds, cmd);
> -		iova += tlbi->iopte_size;
> +	if (!num_ops || num_ops > 512) {
> +		tlbi->single.use_full_inv = true;
> +		return;
>  	}
> -	return true;
> +	tlbi->single.num = num_ops;
>  }
>  
>  static void arm_smmu_inv_all_cmd(struct arm_smmu_inv *inv,
> @@ -2674,16 +2653,37 @@ static bool arm_smmu_inv_to_cmdq_batch(struct arm_smmu_inv *inv,
>  				       struct arm_smmu_cmd *cmd,
>  				       struct arm_smmu_tlbi *tlbi)
>  {
> +	u64 iova = tlbi->iova;
> +	unsigned int i;
> +
>  	if (inv->smmu->features & ARM_SMMU_FEAT_RANGE_INV) {
> -		if (arm_smmu_cmdq_batch_add_range(inv->smmu, cmds, cmd, tlbi))
> -			return false;
> -	} else {
> -		if (arm_smmu_cmdq_batch_add_single(inv->smmu, cmds, cmd, tlbi))
> -			return false;
> +		if (tlbi->range.use_full_inv) {
> +			arm_smmu_inv_all_cmd(inv, cmds, cmd);
> +			return true;
> +		}
> +		for (i = 0; i < tlbi->range.num_cmds; i++) {
> +			struct arm_smmu_cmd range_cmd = tlbi->range.cmds[i];
> +
> +			range_cmd.data[0] |= cmd->data[0];
> +			range_cmd.data[1] |= cmd->data[1];
> +			arm_smmu_cmdq_batch_add_cmd_p(inv->smmu, cmds,
> +						      &range_cmd);
> +		}
> +		return false;
>  	}
>  
> -	arm_smmu_inv_all_cmd(inv, cmds, cmd);
> -	return true;
> +	if (tlbi->single.use_full_inv) {
> +		arm_smmu_inv_all_cmd(inv, cmds, cmd);
> +		return true;
> +	}
> +
> +	for (i = 0; i < tlbi->single.num; i++) {
> +		cmd->data[1] = FIELD_PREP(CMDQ_TLBI_1_LEAF, tlbi->leaf_only) |
> +			       (iova & ~GENMASK_U64(11, 0));
> +		iova += tlbi->iopte_size;
> +		arm_smmu_cmdq_batch_add_cmd_p(inv->smmu, cmds, cmd);
> +	}
> +	return false;
>  }
>  
>  static inline bool arm_smmu_invs_end_batch(struct arm_smmu_inv *cur,
> @@ -2702,8 +2702,8 @@ static inline bool arm_smmu_invs_end_batch(struct arm_smmu_inv *cur,
>  	return false;
>  }
>  
> -static void __arm_smmu_domain_inv_range(struct arm_smmu_tlbi *tlbi,
> -					struct arm_smmu_invs *invs)
> +static void arm_smmu_domain_tlbi_inv(struct arm_smmu_tlbi *tlbi,
> +				     struct arm_smmu_invs *invs)
>  {
>  	struct arm_smmu_inv *used_s12_vmall = NULL;
>  	struct arm_smmu_cmdq_batch cmds = {};
> @@ -2798,11 +2798,17 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  		.iova = iova,
>  		.size = size,
>  		.iopte_size = granule,
> -		.has_cont = smmu_domain->stage == ARM_SMMU_DOMAIN_SVA,
>  		.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);
> +	}
> +
>  	/*
>  	 * An invalidation request must follow some IOPTE change and then load
>  	 * an invalidation array. In the meantime, a domain attachment mutates
> @@ -2833,6 +2839,19 @@ void arm_smmu_domain_inv_range(struct arm_smmu_domain *smmu_domain,
>  	rcu_read_lock();
>  	invs = rcu_dereference(smmu_domain->invs);
>  
> +	/*
> +	 * Only precalculate RIL if it will be used, invs generation ensures
> +	 * this matches the instances used for invalidation.
> +	 */
> +	if (invs->has_range_inv) {
> +		if (!tlbi.range.use_full_inv) {
> +			arm_smmu_tlbi_calc_range(
> +				&tlbi,
> +				smmu_domain->stage == ARM_SMMU_DOMAIN_SVA &&
> +					invs->has_full_cont_ril);
> +		}
> +	}
> +
>  	/*
>  	 * Avoid locking unless ATS is being used. No ATC invalidation can be
>  	 * going on after a domain is detached.
> @@ -2841,10 +2860,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_inv_range(&tlbi, invs);
> +		arm_smmu_domain_tlbi_inv(&tlbi, invs);
>  		read_unlock_irqrestore(&invs->rwlock, flags);
>  	} else {
> -		__arm_smmu_domain_inv_range(&tlbi, invs);
> +		arm_smmu_domain_tlbi_inv(&tlbi, invs);
>  	}
>  
>  	rcu_read_unlock();
> 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 4b9f04825eefa2..33ef99775aea87 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -815,8 +815,18 @@ struct arm_smmu_tlbi {
>  	unsigned int iopte_size;
>  	/* Base Translation Granule of the page table */
>  	u8 tgsz_lg2;
> -	bool has_cont;
>  	bool leaf_only;
> +
> +	struct {
> +		bool use_full_inv;
> +		u16 num;
> +	} single;
> +
> +	struct {
> +		bool use_full_inv;
> +		u8 num_cmds;
> +		struct arm_smmu_cmd cmds[2];
> +	} range;
>  };
>  
>  struct arm_smmu_evtq {
> -- 
> 2.43.0
> 

  reply	other threads:[~2026-09-07 14:27 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-01 17:49 [PATCH v5 0/9] Organize the SMMUv3 invalidation flow so iommupt can use it Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 1/9] iommu/arm-smmu-v3: Handle ARM erratum for CONT under invalidation with SVA Jason Gunthorpe
2026-09-07 14:21   ` Mostafa Saleh
2026-09-07 15:11     ` Jason Gunthorpe
2026-09-07 15:17       ` Mostafa Saleh
2026-09-07 15:18         ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 2/9] iommu/arm-smmu-v3: Pass the parameters for the invalidation in a struct Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 3/9] iommu/arm-smmu-v3: Move pgsize out of arm_smmu_inv Jason Gunthorpe
2026-09-07 14:22   ` Mostafa Saleh
2026-09-01 17:49 ` [PATCH v5 4/9] iommu/arm-smmu-v3: Optimize range invalidation for latency Jason Gunthorpe
2026-09-07 14:26   ` Mostafa Saleh
2026-09-07 15:23     ` Jason Gunthorpe
2026-09-07 19:34       ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 5/9] iommu/arm-smmu-v3: Keep track in the arm_smmu_invs if RIL is used Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 6/9] iommu/arm-smmu-v3: Precompute the invalidation commands Jason Gunthorpe
2026-09-07 14:27   ` Mostafa Saleh [this message]
2026-09-07 15:19     ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 7/9] iommu/arm-smmu-v3: Populate the tlbi at the top of the call chain Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 8/9] iommu/arm-smmu-v3: Change how the tlbi describes the invalidation Jason Gunthorpe
2026-09-07 14:42   ` Mostafa Saleh
2026-09-07 15:34     ` Jason Gunthorpe
2026-09-09 15:57       ` Jason Gunthorpe
2026-09-01 17:49 ` [PATCH v5 9/9] iommu/arm-smmu-v3: Support the DS expansion of RIL's SCALE Jason Gunthorpe
2026-09-07 14:44   ` Mostafa Saleh
2026-09-07 19:39     ` Jason Gunthorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ap7J6NKXgWhiwXgL@google.com \
    --to=smostafa@google.com \
    --cc=Jonathan.Cameron@huawei.com \
    --cc=catalin.marinas@arm.com \
    --cc=corbet@lwn.net \
    --cc=dmatlack@google.com \
    --cc=iommu@lists.linux.dev \
    --cc=jean-philippe@linaro.org \
    --cc=jgg@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=jpb@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=nicolinc@nvidia.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=patches@lists.linux.dev \
    --cc=praan@google.com \
    --cc=rdunlap@infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=skhan@linuxfoundation.org \
    --cc=skhawaja@google.com \
    --cc=stable@vger.kernel.org \
    --cc=vijayanand.jitta@oss.qualcomm.com \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox