Re: [PATCH] iommupt: report the whole contiguous group in read_and_clear_dirty

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Oct 13, 2026 at 09:58:14AM -0700, samarth.colleges@gmail.com wrote:
> From: Samarth Narang <samarth.colleges@gmail.com>
> 
> The record_dirty() function computes the reported dirty length of a contiguous
> (multi-item) leaf group as:
> 
> 	end_index = log2_set_mod_max_t(unsigned int, pts->index, num_contig_lg2);
> 	end_index = min(end_index, pts->end_index);
> 	dirty_len = (end_index - index) * log2_to_int(pt_table_item_lg2sz(pts));
> 
> Here, log2_set_mod_max_t() sets the low num_contig_lg2 bits of the index to all
> ones, i.e. it returns the *inclusive* last index of the contiguous group.
> However, pts->end_index is an *exclusive* bound, this can be seen from the
> definition of pt_range_to_end_index() which returns of the form "index + 1".
> Also, the iterator stops on index >= end_index. Therefore, subtracting an
> inclusive end from an exclusive bound will yield one less item than the actual
> length of the contiguous group.
> 
> For IOMMU dirty tracking used by VFIO/iommufd live migration this silently
> drops the last page of a contiguous dirty mapping from the migration
> bitmap.
> 
> This patch makes end_index exclusive to match pts->end_index and the span cleared by
> pt_entry_make_write_clean(). Also, the same length is passed to
> iommu_iotlb_gather_add_range() so this fix would correct that as well.
> 
> Fixes: 4a00f9434891 ("iommupt: Add read_and_clear_dirty op")
> Signed-off-by: Samarth Narang <samarth.colleges@gmail.com>
> ---
>  drivers/iommu/generic_pt/iommu_pt.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/generic_pt/iommu_pt.h b/drivers/iommu/generic_pt/iommu_pt.h
> --- a/drivers/iommu/generic_pt/iommu_pt.h
> +++ b/drivers/iommu/generic_pt/iommu_pt.h
> @@ -248,7 +248,7 @@ static void record_dirty(struct pt_state *pts,
>  	if (num_contig_lg2 != ilog2(1)) {
>  		unsigned int index = pts->index;
>  		unsigned int end_index = log2_set_mod_max_t(
> -			unsigned int, pts->index, num_contig_lg2);
> +			unsigned int, pts->index, num_contig_lg2) + 1;

This is a bug, but there is a bigger bug here.. The function is
supposed to record the entire entry that is marked as dirty because it
can only clean the entire entry.

Taken with the patch I just commented on this means the system call
should be failing if the entry does not fit within the bitmap.

So this function should look like this:

static void record_dirty(struct pt_state *pts,
			 struct pt_iommu_dirty_args *dirty)
{
	unsigned int lg2sz = pt_entry_oa_lg2sz(pts);
	pt_vaddr_t start_va = log2_set_mod(pts->range->va, 0, lg2sz);
	pt_vaddr_t dirty_len = log2_to_int(lg2sz);

	if (dirty->dirty->bitmap)
		iova_bitmap_set(dirty->dirty->bitmap, start_va, dirty_len);

	if (!(dirty->flags & IOMMU_DIRTY_NO_CLEAR)) {
		/*
		 * No write log required because DMA incoherence and atomic
		 * dirty tracking bits can't work together
		 */
		pt_entry_make_write_clean(pts);
		iommu_iotlb_gather_add_range(dirty->dirty->gather, start_va,
					     dirty_len);
	}
}

Along with the patch I just commented on to make iova_bitmap_set
generate a failure if the iova range is not recordable.

I will update this patch in this way

Jason