Re: [PATCH rc 5/5] iommupt: Fix the end_index calculation in __map_range_leaf()

[Samiullah Khawaja <skhawaja@google.com>]

On Tue, May 12, 2026 at 01:46:17PM -0300, Jason Gunthorpe wrote:
>Sashiko noticed a mismatch of units in this math: num_leaves is
>actually the number of leaf *entries* (so a 16-item contiguous leaf
>is one num_leaves), while index is in items. The mismatch in maths
>causes __map_range_leaf() to exit early instead of efficiently
>filling a larger range of contiguous PTEs.
>
>The early exit is caught by the functions above and then
>__map_range_leaf() is re-invoked, so there is no functional issue.
>
>Correct the misuse of units by adjusting num_leaves with the leaf
>size and avoid the performance cost of looping externally.
>
>There are also some mismatched types for num_leaves; simplify
>things to remove the duplicated calculations.
>
>Fixes: d6c65b0fd621 ("iommupt: Avoid rewalking during map")
>Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>---
> drivers/iommu/generic_pt/iommu_pt.h | 20 +++++++++++++-------
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
>diff --git a/drivers/iommu/generic_pt/iommu_pt.h b/drivers/iommu/generic_pt/iommu_pt.h
>index 4877b05291c9d4..dc91fb4e2f61cb 100644
>--- a/drivers/iommu/generic_pt/iommu_pt.h
>+++ b/drivers/iommu/generic_pt/iommu_pt.h
>@@ -534,10 +534,12 @@ static int __map_range_leaf(struct pt_range *range, void *arg,
> 	struct pt_state pts = pt_init(range, level, table);
> 	struct pt_iommu_map_args *map = arg;
> 	unsigned int leaf_pgsize_lg2 = map->leaf_pgsize_lg2;
>+	unsigned int leaves_avail;
> 	unsigned int start_index;
> 	pt_oaddr_t oa = map->oa;
>-	unsigned int num_leaves;
>+	pt_vaddr_t num_leaves;
> 	unsigned int orig_end;
>+	unsigned int step_lg2;
> 	pt_vaddr_t last_va;
> 	unsigned int step;
> 	bool need_contig;
>@@ -546,21 +548,25 @@ static int __map_range_leaf(struct pt_range *range, void *arg,
> 	PT_WARN_ON(map->leaf_level != level);
> 	PT_WARN_ON(!pt_can_have_leaf(&pts));
>
>-	step = log2_to_int_t(unsigned int,
>-			     leaf_pgsize_lg2 - pt_table_item_lg2sz(&pts));
>-	need_contig = leaf_pgsize_lg2 != pt_table_item_lg2sz(&pts);
>+	step_lg2 = leaf_pgsize_lg2 - pt_table_item_lg2sz(&pts);
>+	step = log2_to_int_t(unsigned int, step_lg2);
>+	need_contig = step_lg2 != 0;
>
> 	_pt_iter_first(&pts);
> 	start_index = pts.index;
> 	orig_end = pts.end_index;
>-	if (pts.index + map->num_leaves < pts.end_index) {
>+	leaves_avail =
>+		log2_div_t(unsigned int, pts.end_index - pts.index, step_lg2);
>+	if (map->num_leaves <= leaves_avail) {
> 		/* Need to stop in the middle of the table to change sizes */
>-		pts.end_index = pts.index + map->num_leaves;
>+		pts.end_index = pts.index + log2_mul(map->num_leaves, step_lg2);
> 		num_leaves = 0;
> 	} else {
>-		num_leaves = map->num_leaves - (pts.end_index - pts.index);
>+		num_leaves = map->num_leaves - leaves_avail;
> 	}
>
>+	PT_WARN_ON(
>+		log2_mod_t(unsigned int, pts.end_index - pts.index, step_lg2));
> 	do {
> 		pts.type = pt_load_entry_raw(&pts);
> 		if (pts.type != PT_ENTRY_EMPTY || need_contig) {
>-- 
>2.43.0
>

Reviewed-by: Samiullah Khawaja <skhawaja@google.com>

Thanks,
Sami