All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] iommupt: Do not allow PTEs to be split when dirty logging
@ 2026-10-15 17:20 Jason Gunthorpe
  2026-10-16  1:33 ` Samiullah Khawaja
  0 siblings, 1 reply; 2+ messages in thread
From: Jason Gunthorpe @ 2026-10-15 17:20 UTC (permalink / raw)
  To: iommu, Joerg Roedel (AMD), linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon
  Cc: Alejandro Jimenez, Joerg Roedel, Kevin Tian, Pasha Tatashin,
	patches, Samarth Narang, Samiullah Khawaja

If a single PTE cannot be stored in the current bitmap that should be a
failure and reported back to userspace. Allowing a PTE to be split creates
a possibility that dirty memory will be reported as clean.

The gather must also cover the entire PTE that is being cleaned, not just
the part that fits within the bitmap.

Fix both issues by computing the full IOVA range of every PTE and using
that to update the bitmap and the gather. The bitmap code now rejects
ranges that don't fit in it, and the gather always combined ranges.

The selftest quietly relied on a bug where the CONT was split, that is why
the half page size for the mock domain saw alternating dirties. Increase
the dirty test page size to PAGE_SIZE so it still works as intended.

Cc: stable@vger.kernel.org
Fixes: 4a00f9434891 ("iommupt: Add read_and_clear_dirty op")
Reported-by: Samarth Narang <samarth.colleges@gmail.com>
Closes: https://lore.kernel.org/all/20261013165814.309255
Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
---
 drivers/iommu/generic_pt/iommu_pt.h     | 30 +++++++------------------
 tools/testing/selftests/iommu/iommufd.c | 10 ++++-----
 2 files changed, 13 insertions(+), 27 deletions(-)

This relies on other changes in the iommufd tree so I will take it

diff --git a/drivers/iommu/generic_pt/iommu_pt.h b/drivers/iommu/generic_pt/iommu_pt.h
index 07ec2b3ab986a6..837d835e4fad8e 100644
--- a/drivers/iommu/generic_pt/iommu_pt.h
+++ b/drivers/iommu/generic_pt/iommu_pt.h
@@ -240,27 +240,14 @@ struct pt_iommu_dirty_args {
 };
 
 static void record_dirty(struct pt_state *pts,
-			 struct pt_iommu_dirty_args *dirty,
-			 unsigned int num_contig_lg2)
+			 struct pt_iommu_dirty_args *dirty)
 {
-	pt_vaddr_t dirty_len;
-
-	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);
-
-		/* Adjust for being contained inside a contiguous page */
-		end_index = min(end_index, pts->end_index);
-		dirty_len = (end_index - index) *
-				log2_to_int(pt_table_item_lg2sz(pts));
-	} else {
-		dirty_len = log2_to_int(pt_table_item_lg2sz(pts));
-	}
+	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, pts->range->va,
-				dirty_len);
+		iova_bitmap_set(dirty->dirty->bitmap, start_va, dirty_len);
 
 	if (!(dirty->flags & IOMMU_DIRTY_NO_CLEAR)) {
 		/*
@@ -268,8 +255,8 @@ static void record_dirty(struct pt_state *pts,
 		 * dirty tracking bits can't work together
 		 */
 		pt_entry_make_write_clean(pts);
-		iommu_iotlb_gather_add_range(dirty->dirty->gather,
-					     pts->range->va, dirty_len);
+		iommu_iotlb_gather_add_range(dirty->dirty->gather, start_va,
+					     dirty_len);
 	}
 }
 
@@ -289,8 +276,7 @@ static inline int __read_and_clear_dirty(struct pt_range *range, void *arg,
 			continue;
 		}
 		if (pts.type == PT_ENTRY_OA && pt_entry_is_write_dirty(&pts))
-			record_dirty(&pts, dirty,
-				     pt_entry_num_contig_lg2(&pts));
+			record_dirty(&pts, dirty);
 	}
 	return 0;
 }
diff --git a/tools/testing/selftests/iommu/iommufd.c b/tools/testing/selftests/iommu/iommufd.c
index 44193d171ba98d..ca2b35e9d443a1 100644
--- a/tools/testing/selftests/iommu/iommufd.c
+++ b/tools/testing/selftests/iommu/iommufd.c
@@ -2175,9 +2175,9 @@ FIXTURE_SETUP(iommufd_dirty_tracking)
 	test_ioctl_ioas_alloc(&self->ioas_id);
 
 	/*
-	 * For dirty testing it is important that the page size fed into
-	 * the iommu page tables matches the size the dirty logic
-	 * expects, or set_dirty can touch too much stuff.
+	 * Disabling huge pages maps in PAGE_SIZE units, which can span
+	 * multiple contiguous MOCK_PAGE_SIZE entries. The dirty tests must
+	 * expect the entire mapping to be reported dirty.
 	 */
 	cmd.object_id = self->ioas_id;
 	if (!variant->hugepages)
@@ -2335,7 +2335,7 @@ TEST_F(iommufd_dirty_tracking, device_dirty_capability)
 
 TEST_F(iommufd_dirty_tracking, get_dirty_bitmap)
 {
-	uint32_t page_size = MOCK_PAGE_SIZE;
+	uint32_t page_size = PAGE_SIZE;
 	uint32_t ioas_id = self->ioas_id;
 	uint32_t hwpt_id;
 
@@ -2377,7 +2377,7 @@ TEST_F(iommufd_dirty_tracking, get_dirty_bitmap)
 
 TEST_F(iommufd_dirty_tracking, get_dirty_bitmap_no_clear)
 {
-	uint32_t page_size = MOCK_PAGE_SIZE;
+	uint32_t page_size = PAGE_SIZE;
 	uint32_t ioas_id = self->ioas_id;
 	uint32_t hwpt_id;
 

base-commit: e21952d72b4740f08a272d694cca223b6b0099ca
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] iommupt: Do not allow PTEs to be split when dirty logging
  2026-10-15 17:20 [PATCH] iommupt: Do not allow PTEs to be split when dirty logging Jason Gunthorpe
@ 2026-10-16  1:33 ` Samiullah Khawaja
  0 siblings, 0 replies; 2+ messages in thread
From: Samiullah Khawaja @ 2026-10-16  1:33 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, Joerg Roedel (AMD), linux-kselftest, Robin Murphy,
	Shuah Khan, Will Deacon, Alejandro Jimenez, Joerg Roedel,
	Kevin Tian, Pasha Tatashin, patches, Samarth Narang

On Thu, Oct 15, 2026 at 02:20:10PM -0300, Jason Gunthorpe wrote:
>If a single PTE cannot be stored in the current bitmap that should be a
>failure and reported back to userspace. Allowing a PTE to be split creates
>a possibility that dirty memory will be reported as clean.
>
>The gather must also cover the entire PTE that is being cleaned, not just
>the part that fits within the bitmap.
>
>Fix both issues by computing the full IOVA range of every PTE and using
>that to update the bitmap and the gather. The bitmap code now rejects
>ranges that don't fit in it, and the gather always combined ranges.
>
>The selftest quietly relied on a bug where the CONT was split, that is why
>the half page size for the mock domain saw alternating dirties. Increase
>the dirty test page size to PAGE_SIZE so it still works as intended.
>
>Cc: stable@vger.kernel.org
>Fixes: 4a00f9434891 ("iommupt: Add read_and_clear_dirty op")
>Reported-by: Samarth Narang <samarth.colleges@gmail.com>
>Closes: https://lore.kernel.org/all/20261013165814.309255
>Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
>---
> drivers/iommu/generic_pt/iommu_pt.h     | 30 +++++++------------------
> tools/testing/selftests/iommu/iommufd.c | 10 ++++-----
> 2 files changed, 13 insertions(+), 27 deletions(-)
>
>This relies on other changes in the iommufd tree so I will take it
>

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-16  1:34 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-15 17:20 [PATCH] iommupt: Do not allow PTEs to be split when dirty logging Jason Gunthorpe
2026-10-16  1:33 ` Samiullah Khawaja

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.