Re: [PATCH v5 8/9] iommu/arm-smmu-v3: Change how the tlbi describes the invalidation

On Mon, Sep 07, 2026 at 02:42:57PM +0000, Mostafa Saleh wrote:

On Tue, Sep 01, 2026 at 02:49:57PM -0300, Jason Gunthorpe wrote:

quoted

The RIL logic has long had a FIXME that there is not enough
information to properly compute the RIL. There is also subtly not
enough information to properly compute the single stride either.

Change tlbi to use the information format that iommupt is going to
use for ARM. This prepares the invalidation code to support iommupt
and fixes two small limitations with the current code.

iommupt is designed to accumulate all invalidation into a single
gather, then the iommu driver should issue a small number of commands
to execute the gather to control invalidation latency. This is in
contrast to io-pgtable-arm.c which generates many gather flushes and
direct walk cache flushes as it progresses.

To accommodate this the gather will accumulate "damage" in bitmaps,
one for leaf changes and one for table changes. This is enough
information for SMMUv3 to compute the proper stride for single
invalidation and to generate ideal hints for range invalidation.

I am not sure I understand that, in what situation the leaf_bitmap
would be used instead of a single page size?=
Would iommupt combine different page sizes in a single invalidation?

Yes

And then I see in this patch it has:
if (!is_power_of_2(leaf_bitmap))
	return 0;

Right, ARM doesn't support mixed leaves in a RIL so we can't use TTL
if iommupt has constructed something like that.

Would that actually be better for performance than using 2 sets of
RILs, one for each page size?
As I'd imagine the HW will spend more effort on the TTL=0 case
otherwise it wouldn't require it.

I have no idea, it is a hint. Since SW has no knowledge I think it
should just issue as few commands as possible. There is no way to know
what will work better on any particular HW.

quoted

@@ -140,17 +140,34 @@ 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);
+	u8 tgsz_lg2 = smmu_domain->tgsz_lg2;
 	struct arm_smmu_tlbi tlbi = {
 		.tgsz_lg2 = smmu_domain->tgsz_lg2,
-		.iova = start,
+		.start = start,
+		.last = end - 1,
 		/*
-		 * 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.
+		 * No information comes from the mm, assume the worst case that
+		 * it changed every table level. The way this is hooked into the
+		 * mm is tricky, the range won't be expanded to include an
+		 * entire table level if one was removed like the iommu gather
+		 * does. Thus even if this is a 4k invalidation it may be
+		 * including any table level too.
 		 */
-		.size = end - start,
-		.iopte_size = PAGE_SIZE,
+		.table_levels_bitmap = 0xfe,

What does that mean, won't arm have a max of 4 levels?

It is really ~1, the extra leading 1s don't matter. Just can't have
the leaf bit set.

quoted

+	/*
+	 * If the size is small then we can infer the invalidation is PTE only
+	 * and set the PTE level only. Otherwise it could be some other
+	 * combination so just set them all. This allows RIL to use TTL=3 in
+	 * cases of PTE only changes. The mm must not try to partially
+	 * invalidate pmd/etc.
+	 */

How does that work with splitting blocks?  I imagine that might be
ossible with userspace.

If mm splits anything then the invalidation will not be PAGE_SIZE
big. A split requires invalidating the original larger size.

quoted

+static u8 arm_smmu_tlbi_calc_stride(struct arm_smmu_tlbi *tlbi)
+{
+	u8 combined = tlbi->table_levels_bitmap | tlbi->leaf_levels_bitmap;
+	u8 tg_szlg2 = tlbi->tgsz_lg2;
+
+	if (WARN_ON(!combined))
+		return U8_MAX;

When can that happen?

It can't, thats why it is a WARN_ON :)

quoted

@@ -4152,21 +4237,28 @@ static void arm_smmu_flush_iotlb_all(struct iommu_domain *domain)
 		arm_smmu_tlb_inv_context(smmu_domain);
 }
 
+/*
+ * Called by io-pgtable-arm.c for each run of same pgsize leaf only

I believe that it is called from dma-iommu.c, io-pgtable-arm.c will
call the tlb_add_page which builds the gather though.

Sort of, for the purposes of this comment the important flush is
initiated by io-pgtable-arm.c under tlb_add_page() when it calls
arm_smmu_tlb_inv_page_nosync(), which calls
iommu_iotlb_gather_add_page(), which calls iommu_iotlb_sync()

That's done in a way that guarentees the same-pgsize property:

	if ((gather->pgsize && gather->pgsize != size) ||

Yes it is also called from dma-iommu.c, but only for the "trailing"
gather and that doesn't do anything to change what is in the gather..

I'll add a few more words here

quoted

+ * invalidation. If it has to change to a different leaf level then it flushes
+ * the gather and starts a fresh one. Thus this always targets only a single
+ * leaf level.
+ */
 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);
+	unsigned int tg = smmu_domain->tgsz_lg2;
 	struct arm_smmu_tlbi tlbi = {
 		.tgsz_lg2 = smmu_domain->tgsz_lg2,
-		.iova = gather->start,
-		.size = gather->end - gather->start + 1,
-		.iopte_size = gather->pgsize,
-		.leaf_only = true,
+		.start = gather->start,
+		.last = gather->end,
 	};
 
-	if (!gather->pgsize)
+	if (WARN_ON(gather->pgsize < BIT(tg)))
 		return;
 
+	tlbi.leaf_levels_bitmap = BIT((ilog2(gather->pgsize) - tg) / (tg - 3));

Having some page table macros would be helpful (and in other places in
this patch)

At least this one gets deleted in the next series, so I left it like
this deliberately. Was there something else you saw that had
duplication?

Thanks,
Jason