mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown
@ 2026-08-27 14:58 Yuanhe Shu
  2026-08-27 14:58 ` [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address Yuanhe Shu
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Yuanhe Shu @ 2026-08-27 14:58 UTC (permalink / raw)
  To: joro, will, jgg
  Cc: robin.murphy, baolu.lu, kevin.tian, smostafa, praan, skhawaja,
	iommu, linux-kernel, Yuanhe Shu

Since commit b948a8722848 ("iommu: Fix up map/unmap debugging for
iommupt domains") iommu_map() takes an IOMMU_DEBUG_PAGEALLOC reference
for every page it maps into a domain, and the only path that drops
those references is the IOVA based unmap.  When a generic_pt domain is
freed while mappings are still installed, pt_iommu_deinit() releases
the page table memory without going through that path, so every mapped
page keeps its reference forever and each later allocation or free of
it trips:

    WARNING: drivers/iommu/iommu-debug-pagealloc.c:91 at __iommu_debug_check_unmapped+0x4e/0x70, CPU#0: init/1
    iommu: Detected page leak!

Freeing a domain with mappings still installed is not driver misuse:
the deinit contract in include/linux/generic_pt/iommu.h only requires
the table to be removed from HW access and caches, with no requirement
to unmap first.  iommu_setup_default_domain() frees the old domain with
its IOMMU_RESV_DIRECT mappings still installed (those pages normally
never return to the page allocator, so it does not WARN today), and the
generic_pt kunit suite (CONFIG_IOMMU_PT_KUNIT_TEST) does the same in
pt_kunit_iommu_exit().

Patch 1 adds __iommu_debug_unmap_phys(), the physical address based
counterpart of __iommu_debug_map().  Patch 2 wires it into the deinit
walk: a debug_unmap flag makes __collect_tables() drop the reference of
every OA leaf it destroys, symmetric to how iommu_map() created them.

io-pgtable has the same gap in __arm_lpae_free_pgtable(), but its
cookies cannot reach the struct iommu_domain, so that fix needs an ABI
change or per-driver handling and is left as a follow-up.

The kunit suite doubles as an in-tree reproducer: with
CONFIG_IOMMU_DEBUG_PAGEALLOC=y and CONFIG_IOMMU_PT_KUNIT_TEST=y, boot
with iommu.debug_pagealloc=1
kunit.filter_glob=x86_64_iommu_test.test_pgsize_boundary, then
allocate and free most of memory (the case maps 128K at the hard-coded
OA 0x208b95d000 and never unmaps it, so the machine needs enough RAM
for that address to be online memory - a 150G guest was used here, and
the sweep was a tmpfs filled to 95% of RAM):

    unpatched:          64 page leak WARNINGs
    with this series:   0

The 64 is one WARNING for each of the 32 mapped pages on both its
allocation and free.  On unpatched mainline the suite already fails
test_random_map's NR_SECONDARY_PAGETABLE assertion, which aborts its
cleanup and cascades into the following cases, hence the isolation.  An
out-of-tree module mapping a page into an amdv1 domain and freeing the
domain without unmapping shows the same behaviour, 384 WARNINGs over
64 iterations unpatched and none with the series; each leaked page is
reported again on every later allocation and free, so the count
exceeds the 64 leaked pages.  The control case that unmaps first stays
silent.

The generic_pt format code can be built as a module (e.g.
CONFIG_IOMMU_PT_AMDV1=m), so the new helper and the
iommu_debug_initialized static key are exported GPL.

Yuanhe Shu (2):
  iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
  iommupt: Drop pagealloc references during domain deinit

 drivers/iommu/generic_pt/iommu_pt.h   | 22 +++++++++++++++++---
 drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
 drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
 3 files changed, 70 insertions(+), 6 deletions(-)

base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
-- 
2.43.5

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

* [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
  2026-08-27 14:58 [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Yuanhe Shu
@ 2026-08-27 14:58 ` Yuanhe Shu
  2026-08-27 16:13   ` Mostafa Saleh
  2026-08-27 16:21   ` Jason Gunthorpe
  2026-08-27 14:58 ` [PATCH 2/2] iommupt: Drop pagealloc references during domain deinit Yuanhe Shu
  2026-08-27 16:05 ` [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Mostafa Saleh
  2 siblings, 2 replies; 8+ messages in thread
From: Yuanhe Shu @ 2026-08-27 14:58 UTC (permalink / raw)
  To: joro, will, jgg
  Cc: robin.murphy, baolu.lu, kevin.tian, smostafa, praan, skhawaja,
	iommu, linux-kernel, Yuanhe Shu

The IOMMU_DEBUG_PAGEALLOC sanitizer takes a reference on each page at
iommu_map() time, keyed by physical address, and the only way to drop
them is the IOVA based iommu_debug_unmap_begin()/end().  A path that
tears down a page table with mappings still installed has no IOVA to
unmap with, so add a physical address based counterpart of
__iommu_debug_map() for those paths, sharing the counting loop through
__iommu_debug_update_phys().

Export the iommu_debug_initialized static key, as the generic_pt format
code using these helpers can be built as a module.

Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
Build tested as built-in and as a module (AMD_IOMMU=n,
CONFIG_IOMMU_PT_AMDV1=m).

 drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
 drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
 2 files changed, 51 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
index 80164df5bab1..a2e55164e6a4 100644
--- a/drivers/iommu/iommu-debug-pagealloc.c
+++ b/drivers/iommu/iommu-debug-pagealloc.c
@@ -15,6 +15,8 @@
 
 static bool needed;
 DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
+/* The generic_pt format code using the key can be built as a module */
+EXPORT_SYMBOL_GPL(iommu_debug_initialized);
 
 struct iommu_debug_metadata {
 	atomic_t ref;
@@ -96,7 +98,8 @@ void __iommu_debug_check_unmapped(const struct page *page, int numpages)
 	}
 }
 
-void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t size)
+static void __iommu_debug_update_phys(struct iommu_domain *domain,
+				      phys_addr_t phys, size_t size, bool inc)
 {
 	size_t off, end;
 	size_t page_size = iommu_debug_page_size(domain);
@@ -104,9 +107,30 @@ void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t siz
 	if (WARN_ON(!phys || check_add_overflow(phys, size, &end)))
 		return;
 
-	for (off = 0 ; off < size ; off += page_size)
-		iommu_debug_inc_page(phys + off);
+	for (off = 0 ; off < size ; off += page_size) {
+		if (inc)
+			iommu_debug_inc_page(phys + off);
+		else
+			iommu_debug_dec_page(phys + off);
+	}
+}
+
+void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t size)
+{
+	__iommu_debug_update_phys(domain, phys, size, true);
+}
+
+/*
+ * Physical address counterpart of __iommu_debug_map(), for teardown paths
+ * that destroy mapped entries without an IOVA.  The OAs must have been
+ * accounted by a prior iommu_map().
+ */
+void __iommu_debug_unmap_phys(struct iommu_domain *domain, phys_addr_t phys,
+			      size_t size)
+{
+	__iommu_debug_update_phys(domain, phys, size, false);
 }
+EXPORT_SYMBOL_GPL(__iommu_debug_unmap_phys);
 
 static void __iommu_debug_update_iova(struct iommu_domain *domain,
 				      unsigned long iova, size_t size, bool inc)
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index aaffad5854fc..12528a40bcd8 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -71,11 +71,18 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
 
 void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys,
 		       size_t size);
+void __iommu_debug_unmap_phys(struct iommu_domain *domain, phys_addr_t phys,
+			      size_t size);
 void __iommu_debug_unmap_begin(struct iommu_domain *domain,
 			       unsigned long iova, size_t size);
 void __iommu_debug_unmap_end(struct iommu_domain *domain,
 			     unsigned long iova, size_t size, size_t unmapped);
 
+static inline bool iommu_debug_pagealloc_enabled(void)
+{
+	return static_branch_unlikely(&iommu_debug_initialized);
+}
+
 static inline void iommu_debug_map(struct iommu_domain *domain,
 				   phys_addr_t phys, size_t size)
 {
@@ -83,6 +90,13 @@ static inline void iommu_debug_map(struct iommu_domain *domain,
 		__iommu_debug_map(domain, phys, size);
 }
 
+static inline void iommu_debug_unmap_phys(struct iommu_domain *domain,
+					  phys_addr_t phys, size_t size)
+{
+	if (static_branch_unlikely(&iommu_debug_initialized))
+		__iommu_debug_unmap_phys(domain, phys, size);
+}
+
 static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
 					   unsigned long iova, size_t size)
 {
@@ -101,11 +115,21 @@ static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
 void iommu_debug_init(void);
 
 #else
+static inline bool iommu_debug_pagealloc_enabled(void)
+{
+	return false;
+}
+
 static inline void iommu_debug_map(struct iommu_domain *domain,
 				   phys_addr_t phys, size_t size)
 {
 }
 
+static inline void iommu_debug_unmap_phys(struct iommu_domain *domain,
+					  phys_addr_t phys, size_t size)
+{
+}
+
 static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
 					   unsigned long iova, size_t size)
 {
-- 
2.43.5


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

* [PATCH 2/2] iommupt: Drop pagealloc references during domain deinit
  2026-08-27 14:58 [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Yuanhe Shu
  2026-08-27 14:58 ` [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address Yuanhe Shu
@ 2026-08-27 14:58 ` Yuanhe Shu
  2026-08-27 16:18   ` Jason Gunthorpe
  2026-08-27 16:05 ` [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Mostafa Saleh
  2 siblings, 1 reply; 8+ messages in thread
From: Yuanhe Shu @ 2026-08-27 14:58 UTC (permalink / raw)
  To: joro, will, jgg
  Cc: robin.murphy, baolu.lu, kevin.tian, smostafa, praan, skhawaja,
	iommu, linux-kernel, Yuanhe Shu

Since commit b948a8722848 ("iommu: Fix up map/unmap debugging for
iommupt domains") iommu_map() takes an IOMMU_DEBUG_PAGEALLOC reference
for every page it maps into a generic_pt domain, and those references
are only dropped by the IOVA based unmap path.  When a domain is freed
while mappings are still installed, pt_iommu_deinit() releases the page
table memory without dropping them, so every mapped page stays counted
as IOMMU-mapped after it returns to the buddy allocator and each later
allocation or free of it reports:

    WARNING: drivers/iommu/iommu-debug-pagealloc.c:91 at __iommu_debug_check_unmapped+0x4e/0x70, CPU#0: init/1
    iommu: Detected page leak!

Freeing a domain with mappings still installed is not driver misuse:
the deinit contract in include/linux/generic_pt/iommu.h only requires
the table to be removed from HW access and caches, with no requirement
to unmap first, and the kunit suite itself frees domains with live
mappings in pt_kunit_iommu_exit().

Set a debug_unmap flag in the deinit collect args and have
__collect_tables() drop the reference of every OA leaf it destroys,
symmetric to how iommu_map() created them.  Since the flag is only set
when the sanitizer is enabled, the !pt_can_have_table() fast path is
unaffected when it is off.  This uses the __iommu_debug_unmap_phys()
helper added by the previous patch, which any backport of this fix
needs as well.

Verified with the generic_pt kunit suite as an in-tree reproducer:
running the x86_64 format's test_pgsize_boundary() with
iommu.debug_pagealloc=1 on a machine with enough RAM for its
hard-coded OA 0x208b95d000 to be online memory, then allocating and
freeing most of memory, produces 64 page leak WARNINGs unpatched - one
for each of the 32 mapped pages on both its allocation and free - and
none with this patch.

Fixes: b948a8722848 ("iommu: Fix up map/unmap debugging for iommupt domains")
Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
---
 drivers/iommu/generic_pt/iommu_pt.h | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/iommu/generic_pt/iommu_pt.h b/drivers/iommu/generic_pt/iommu_pt.h
index 07ec2b3ab986..f4b02894f1c4 100644
--- a/drivers/iommu/generic_pt/iommu_pt.h
+++ b/drivers/iommu/generic_pt/iommu_pt.h
@@ -14,6 +14,7 @@
 #include <linux/export.h>
 #include <linux/iommu.h>
 #include "../iommu-pages.h"
+#include "../iommu-priv.h"
 #include <linux/cleanup.h>
 #include <linux/dma-mapping.h>
 
@@ -379,6 +380,8 @@ struct pt_iommu_collect_args {
 	struct iommupt_pending_gather pending;
 	/* Fail if any OAs are within the range */
 	u8 check_mapped : 1;
+	/* Drop the IOMMU_DEBUG_PAGEALLOC references of any OAs in the range */
+	u8 debug_unmap : 1;
 };
 
 static int __collect_tables(struct pt_range *range, void *arg,
@@ -388,7 +391,8 @@ static int __collect_tables(struct pt_range *range, void *arg,
 	struct pt_iommu_collect_args *collect = arg;
 	int ret;
 
-	if (!collect->check_mapped && !pt_can_have_table(&pts))
+	if (!collect->check_mapped && !collect->debug_unmap &&
+	    !pt_can_have_table(&pts))
 		return 0;
 
 	for_each_pt_level_entry(&pts) {
@@ -400,8 +404,19 @@ static int __collect_tables(struct pt_range *range, void *arg,
 				return ret;
 			continue;
 		}
-		if (pts.type == PT_ENTRY_OA && collect->check_mapped)
-			return -EADDRINUSE;
+		if (pts.type == PT_ENTRY_OA) {
+			if (collect->check_mapped)
+				return -EADDRINUSE;
+			if (collect->debug_unmap) {
+				struct pt_iommu *iommu_table =
+					iommu_from_common(range->common);
+				size_t oasz = log2_to_int_t(size_t,
+						pt_entry_oa_lg2sz(&pts));
+
+				iommu_debug_unmap_phys(&iommu_table->domain,
+						       pt_entry_oa(&pts), oasz);
+			}
+		}
 	}
 	return 0;
 }
@@ -1162,6 +1177,7 @@ static void NS(deinit)(struct pt_iommu *iommu_table)
 	struct pt_common *common = common_from_iommu(iommu_table);
 	struct pt_range range = pt_all_range(common);
 	struct pt_iommu_collect_args collect = {
+		.debug_unmap = iommu_debug_pagealloc_enabled(),
 		.pending.free_list = IOMMU_PAGES_LIST_INIT(
 			collect.pending.free_list),
 	};
-- 
2.43.5


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

* Re: [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown
  2026-08-27 14:58 [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Yuanhe Shu
  2026-08-27 14:58 ` [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address Yuanhe Shu
  2026-08-27 14:58 ` [PATCH 2/2] iommupt: Drop pagealloc references during domain deinit Yuanhe Shu
@ 2026-08-27 16:05 ` Mostafa Saleh
  2 siblings, 0 replies; 8+ messages in thread
From: Mostafa Saleh @ 2026-08-27 16:05 UTC (permalink / raw)
  To: Yuanhe Shu
  Cc: joro, will, jgg, robin.murphy, baolu.lu, kevin.tian, praan,
	skhawaja, iommu, linux-kernel

Hi Yuanhe,

On Thu, Aug 27, 2026 at 10:58:53PM +0800, Yuanhe Shu wrote:
> Since commit b948a8722848 ("iommu: Fix up map/unmap debugging for
> iommupt domains") iommu_map() takes an IOMMU_DEBUG_PAGEALLOC reference
> for every page it maps into a domain, and the only path that drops
> those references is the IOVA based unmap.  When a generic_pt domain is
> freed while mappings are still installed, pt_iommu_deinit() releases
> the page table memory without going through that path, so every mapped
> page keeps its reference forever and each later allocation or free of
> it trips:
> 
>     WARNING: drivers/iommu/iommu-debug-pagealloc.c:91 at __iommu_debug_check_unmapped+0x4e/0x70, CPU#0: init/1
>     iommu: Detected page leak!
> 
> Freeing a domain with mappings still installed is not driver misuse:
> the deinit contract in include/linux/generic_pt/iommu.h only requires

Which drivers cause this?

AFAICT, users of the DMA-API must unmap the pages, otherwise they
run into bigger issue (as leaking IOVA).
This is stated in Documentation/core-api/dma-api-howto.rst
  Every dma_map_{single,sg}() call should have its dma_unmap_{single,sg}()
  counterpart, because the DMA address space is a shared resource and
  you could render the machine unusable by consuming all DMA addresses.

On the other side, there are very few driver that use the IOMMU API
directly, and from what I can see they do iommu_unmap().

Some page table implementations might tolerate it (because it is simpler
and more efficient to implement instead of descending to last level
tables) but I don't think that makes it right.

Thanks,
Mostafa


> the table to be removed from HW access and caches, with no requirement
> to unmap first.  iommu_setup_default_domain() frees the old domain with
> its IOMMU_RESV_DIRECT mappings still installed (those pages normally
> never return to the page allocator, so it does not WARN today), and the
> generic_pt kunit suite (CONFIG_IOMMU_PT_KUNIT_TEST) does the same in
> pt_kunit_iommu_exit().
> 
> Patch 1 adds __iommu_debug_unmap_phys(), the physical address based
> counterpart of __iommu_debug_map().  Patch 2 wires it into the deinit
> walk: a debug_unmap flag makes __collect_tables() drop the reference of
> every OA leaf it destroys, symmetric to how iommu_map() created them.
> 
> io-pgtable has the same gap in __arm_lpae_free_pgtable(), but its
> cookies cannot reach the struct iommu_domain, so that fix needs an ABI
> change or per-driver handling and is left as a follow-up.
> 
> The kunit suite doubles as an in-tree reproducer: with
> CONFIG_IOMMU_DEBUG_PAGEALLOC=y and CONFIG_IOMMU_PT_KUNIT_TEST=y, boot
> with iommu.debug_pagealloc=1
> kunit.filter_glob=x86_64_iommu_test.test_pgsize_boundary, then
> allocate and free most of memory (the case maps 128K at the hard-coded
> OA 0x208b95d000 and never unmaps it, so the machine needs enough RAM
> for that address to be online memory - a 150G guest was used here, and
> the sweep was a tmpfs filled to 95% of RAM):
> 
>     unpatched:          64 page leak WARNINGs
>     with this series:   0
> 
> The 64 is one WARNING for each of the 32 mapped pages on both its
> allocation and free.  On unpatched mainline the suite already fails
> test_random_map's NR_SECONDARY_PAGETABLE assertion, which aborts its
> cleanup and cascades into the following cases, hence the isolation.  An
> out-of-tree module mapping a page into an amdv1 domain and freeing the
> domain without unmapping shows the same behaviour, 384 WARNINGs over
> 64 iterations unpatched and none with the series; each leaked page is
> reported again on every later allocation and free, so the count
> exceeds the 64 leaked pages.  The control case that unmaps first stays
> silent.
> 
> The generic_pt format code can be built as a module (e.g.
> CONFIG_IOMMU_PT_AMDV1=m), so the new helper and the
> iommu_debug_initialized static key are exported GPL.
> 
> Yuanhe Shu (2):
>   iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
>   iommupt: Drop pagealloc references during domain deinit
> 
>  drivers/iommu/generic_pt/iommu_pt.h   | 22 +++++++++++++++++---
>  drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
>  drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
>  3 files changed, 70 insertions(+), 6 deletions(-)
> 
> base-commit: 45c13f3f9e3bb15fd89ff2864c6f627a3b4b4229
> -- 
> 2.43.5

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

* Re: [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
  2026-08-27 14:58 ` [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address Yuanhe Shu
@ 2026-08-27 16:13   ` Mostafa Saleh
  2026-08-27 16:17     ` Mostafa Saleh
  2026-08-27 16:21   ` Jason Gunthorpe
  1 sibling, 1 reply; 8+ messages in thread
From: Mostafa Saleh @ 2026-08-27 16:13 UTC (permalink / raw)
  To: Yuanhe Shu
  Cc: joro, will, jgg, robin.murphy, baolu.lu, kevin.tian, praan,
	skhawaja, iommu, linux-kernel

On Thu, Aug 27, 2026 at 10:58:54PM +0800, Yuanhe Shu wrote:
> The IOMMU_DEBUG_PAGEALLOC sanitizer takes a reference on each page at
> iommu_map() time, keyed by physical address, and the only way to drop
> them is the IOVA based iommu_debug_unmap_begin()/end().  A path that
> tears down a page table with mappings still installed has no IOVA to
> unmap with, so add a physical address based counterpart of
> __iommu_debug_map() for those paths, sharing the counting loop through
> __iommu_debug_update_phys().

But the IOVA can be calculated when walking the table from freeing
context or am I missing something?

Thanks,
Mostafa

> 
> Export the iommu_debug_initialized static key, as the generic_pt format
> code using these helpers can be built as a module.
> 
> Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
> ---
> Build tested as built-in and as a module (AMD_IOMMU=n,
> CONFIG_IOMMU_PT_AMDV1=m).
> 
>  drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
>  drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
>  2 files changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
> index 80164df5bab1..a2e55164e6a4 100644
> --- a/drivers/iommu/iommu-debug-pagealloc.c
> +++ b/drivers/iommu/iommu-debug-pagealloc.c
> @@ -15,6 +15,8 @@
>  
>  static bool needed;
>  DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
> +/* The generic_pt format code using the key can be built as a module */
> +EXPORT_SYMBOL_GPL(iommu_debug_initialized);
>  
>  struct iommu_debug_metadata {
>  	atomic_t ref;
> @@ -96,7 +98,8 @@ void __iommu_debug_check_unmapped(const struct page *page, int numpages)
>  	}
>  }
>  
> -void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t size)
> +static void __iommu_debug_update_phys(struct iommu_domain *domain,
> +				      phys_addr_t phys, size_t size, bool inc)
>  {
>  	size_t off, end;
>  	size_t page_size = iommu_debug_page_size(domain);
> @@ -104,9 +107,30 @@ void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t siz
>  	if (WARN_ON(!phys || check_add_overflow(phys, size, &end)))
>  		return;
>  
> -	for (off = 0 ; off < size ; off += page_size)
> -		iommu_debug_inc_page(phys + off);
> +	for (off = 0 ; off < size ; off += page_size) {
> +		if (inc)
> +			iommu_debug_inc_page(phys + off);
> +		else
> +			iommu_debug_dec_page(phys + off);
> +	}
> +}
> +
> +void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t size)
> +{
> +	__iommu_debug_update_phys(domain, phys, size, true);
> +}
> +
> +/*
> + * Physical address counterpart of __iommu_debug_map(), for teardown paths
> + * that destroy mapped entries without an IOVA.  The OAs must have been
> + * accounted by a prior iommu_map().
> + */
> +void __iommu_debug_unmap_phys(struct iommu_domain *domain, phys_addr_t phys,
> +			      size_t size)
> +{
> +	__iommu_debug_update_phys(domain, phys, size, false);
>  }
> +EXPORT_SYMBOL_GPL(__iommu_debug_unmap_phys);
>  
>  static void __iommu_debug_update_iova(struct iommu_domain *domain,
>  				      unsigned long iova, size_t size, bool inc)
> diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
> index aaffad5854fc..12528a40bcd8 100644
> --- a/drivers/iommu/iommu-priv.h
> +++ b/drivers/iommu/iommu-priv.h
> @@ -71,11 +71,18 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
>  
>  void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys,
>  		       size_t size);
> +void __iommu_debug_unmap_phys(struct iommu_domain *domain, phys_addr_t phys,
> +			      size_t size);
>  void __iommu_debug_unmap_begin(struct iommu_domain *domain,
>  			       unsigned long iova, size_t size);
>  void __iommu_debug_unmap_end(struct iommu_domain *domain,
>  			     unsigned long iova, size_t size, size_t unmapped);
>  
> +static inline bool iommu_debug_pagealloc_enabled(void)
> +{
> +	return static_branch_unlikely(&iommu_debug_initialized);
> +}
> +
>  static inline void iommu_debug_map(struct iommu_domain *domain,
>  				   phys_addr_t phys, size_t size)
>  {
> @@ -83,6 +90,13 @@ static inline void iommu_debug_map(struct iommu_domain *domain,
>  		__iommu_debug_map(domain, phys, size);
>  }
>  
> +static inline void iommu_debug_unmap_phys(struct iommu_domain *domain,
> +					  phys_addr_t phys, size_t size)
> +{
> +	if (static_branch_unlikely(&iommu_debug_initialized))
> +		__iommu_debug_unmap_phys(domain, phys, size);
> +}
> +
>  static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
>  					   unsigned long iova, size_t size)
>  {
> @@ -101,11 +115,21 @@ static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
>  void iommu_debug_init(void);
>  
>  #else
> +static inline bool iommu_debug_pagealloc_enabled(void)
> +{
> +	return false;
> +}
> +
>  static inline void iommu_debug_map(struct iommu_domain *domain,
>  				   phys_addr_t phys, size_t size)
>  {
>  }
>  
> +static inline void iommu_debug_unmap_phys(struct iommu_domain *domain,
> +					  phys_addr_t phys, size_t size)
> +{
> +}
> +
>  static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
>  					   unsigned long iova, size_t size)
>  {
> -- 
> 2.43.5
> 

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

* Re: [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
  2026-08-27 16:13   ` Mostafa Saleh
@ 2026-08-27 16:17     ` Mostafa Saleh
  0 siblings, 0 replies; 8+ messages in thread
From: Mostafa Saleh @ 2026-08-27 16:17 UTC (permalink / raw)
  To: Yuanhe Shu
  Cc: joro, will, jgg, robin.murphy, baolu.lu, kevin.tian, praan,
	skhawaja, iommu, linux-kernel

On Thu, Aug 27, 2026 at 5:13 PM Mostafa Saleh <smostafa@google.com> wrote:
>
> On Thu, Aug 27, 2026 at 10:58:54PM +0800, Yuanhe Shu wrote:
> > The IOMMU_DEBUG_PAGEALLOC sanitizer takes a reference on each page at
> > iommu_map() time, keyed by physical address, and the only way to drop
> > them is the IOVA based iommu_debug_unmap_begin()/end().  A path that
> > tears down a page table with mappings still installed has no IOVA to
> > unmap with, so add a physical address based counterpart of
> > __iommu_debug_map() for those paths, sharing the counting loop through
> > __iommu_debug_update_phys().
>
> But the IOVA can be calculated when walking the table from freeing
> context or am I missing something?
>

Ok I am missing that it might not be possible to call in the table
(iova_to_phys) while it is being freed.

> Thanks,
> Mostafa
>
> >
> > Export the iommu_debug_initialized static key, as the generic_pt format
> > code using these helpers can be built as a module.
> >
> > Signed-off-by: Yuanhe Shu <xiangzao@linux.alibaba.com>
> > ---
> > Build tested as built-in and as a module (AMD_IOMMU=n,
> > CONFIG_IOMMU_PT_AMDV1=m).
> >
> >  drivers/iommu/iommu-debug-pagealloc.c | 30 ++++++++++++++++++++++++---
> >  drivers/iommu/iommu-priv.h            | 24 +++++++++++++++++++++
> >  2 files changed, 51 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
> > index 80164df5bab1..a2e55164e6a4 100644
> > --- a/drivers/iommu/iommu-debug-pagealloc.c
> > +++ b/drivers/iommu/iommu-debug-pagealloc.c
> > @@ -15,6 +15,8 @@
> >
> >  static bool needed;
> >  DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
> > +/* The generic_pt format code using the key can be built as a module */
> > +EXPORT_SYMBOL_GPL(iommu_debug_initialized);
> >
> >  struct iommu_debug_metadata {
> >       atomic_t ref;
> > @@ -96,7 +98,8 @@ void __iommu_debug_check_unmapped(const struct page *page, int numpages)
> >       }
> >  }
> >
> > -void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t size)
> > +static void __iommu_debug_update_phys(struct iommu_domain *domain,
> > +                                   phys_addr_t phys, size_t size, bool inc)
> >  {
> >       size_t off, end;
> >       size_t page_size = iommu_debug_page_size(domain);
> > @@ -104,9 +107,30 @@ void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t siz
> >       if (WARN_ON(!phys || check_add_overflow(phys, size, &end)))
> >               return;
> >
> > -     for (off = 0 ; off < size ; off += page_size)
> > -             iommu_debug_inc_page(phys + off);
> > +     for (off = 0 ; off < size ; off += page_size) {
> > +             if (inc)
> > +                     iommu_debug_inc_page(phys + off);
> > +             else
> > +                     iommu_debug_dec_page(phys + off);
> > +     }
> > +}
> > +
> > +void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys, size_t size)
> > +{
> > +     __iommu_debug_update_phys(domain, phys, size, true);
> > +}
> > +
> > +/*
> > + * Physical address counterpart of __iommu_debug_map(), for teardown paths
> > + * that destroy mapped entries without an IOVA.  The OAs must have been
> > + * accounted by a prior iommu_map().
> > + */
> > +void __iommu_debug_unmap_phys(struct iommu_domain *domain, phys_addr_t phys,
> > +                           size_t size)
> > +{
> > +     __iommu_debug_update_phys(domain, phys, size, false);
> >  }
> > +EXPORT_SYMBOL_GPL(__iommu_debug_unmap_phys);
> >
> >  static void __iommu_debug_update_iova(struct iommu_domain *domain,
> >                                     unsigned long iova, size_t size, bool inc)
> > diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
> > index aaffad5854fc..12528a40bcd8 100644
> > --- a/drivers/iommu/iommu-priv.h
> > +++ b/drivers/iommu/iommu-priv.h
> > @@ -71,11 +71,18 @@ int iommu_replace_device_pasid(struct iommu_domain *domain,
> >
> >  void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys,
> >                      size_t size);
> > +void __iommu_debug_unmap_phys(struct iommu_domain *domain, phys_addr_t phys,
> > +                           size_t size);
> >  void __iommu_debug_unmap_begin(struct iommu_domain *domain,
> >                              unsigned long iova, size_t size);
> >  void __iommu_debug_unmap_end(struct iommu_domain *domain,
> >                            unsigned long iova, size_t size, size_t unmapped);
> >
> > +static inline bool iommu_debug_pagealloc_enabled(void)
> > +{
> > +     return static_branch_unlikely(&iommu_debug_initialized);
> > +}
> > +
> >  static inline void iommu_debug_map(struct iommu_domain *domain,
> >                                  phys_addr_t phys, size_t size)
> >  {
> > @@ -83,6 +90,13 @@ static inline void iommu_debug_map(struct iommu_domain *domain,
> >               __iommu_debug_map(domain, phys, size);
> >  }
> >
> > +static inline void iommu_debug_unmap_phys(struct iommu_domain *domain,
> > +                                       phys_addr_t phys, size_t size)
> > +{
> > +     if (static_branch_unlikely(&iommu_debug_initialized))
> > +             __iommu_debug_unmap_phys(domain, phys, size);
> > +}
> > +
> >  static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
> >                                          unsigned long iova, size_t size)
> >  {
> > @@ -101,11 +115,21 @@ static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
> >  void iommu_debug_init(void);
> >
> >  #else
> > +static inline bool iommu_debug_pagealloc_enabled(void)
> > +{
> > +     return false;
> > +}
> > +
> >  static inline void iommu_debug_map(struct iommu_domain *domain,
> >                                  phys_addr_t phys, size_t size)
> >  {
> >  }
> >
> > +static inline void iommu_debug_unmap_phys(struct iommu_domain *domain,
> > +                                       phys_addr_t phys, size_t size)
> > +{
> > +}
> > +
> >  static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
> >                                          unsigned long iova, size_t size)
> >  {
> > --
> > 2.43.5
> >

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

* Re: [PATCH 2/2] iommupt: Drop pagealloc references during domain deinit
  2026-08-27 14:58 ` [PATCH 2/2] iommupt: Drop pagealloc references during domain deinit Yuanhe Shu
@ 2026-08-27 16:18   ` Jason Gunthorpe
  0 siblings, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2026-08-27 16:18 UTC (permalink / raw)
  To: Yuanhe Shu
  Cc: joro, will, robin.murphy, baolu.lu, kevin.tian, smostafa, praan,
	skhawaja, iommu, linux-kernel

On Thu, Aug 27, 2026 at 10:58:55PM +0800, Yuanhe Shu wrote:
> Since commit b948a8722848 ("iommu: Fix up map/unmap debugging for
> iommupt domains") iommu_map() takes an IOMMU_DEBUG_PAGEALLOC reference
> for every page it maps into a generic_pt domain, and those references
> are only dropped by the IOVA based unmap path.  When a domain is freed
> while mappings are still installed, pt_iommu_deinit() releases the page
> table memory without dropping them, so every mapped page stays counted
> as IOMMU-mapped after it returns to the buddy allocator and each later
> allocation or free of it reports:
> 
>     WARNING: drivers/iommu/iommu-debug-pagealloc.c:91 at __iommu_debug_check_unmapped+0x4e/0x70, CPU#0: init/1
>     iommu: Detected page leak!
> 
> Freeing a domain with mappings still installed is not driver misuse:
> the deinit contract in include/linux/generic_pt/iommu.h only requires
> the table to be removed from HW access and caches, with no requirement
> to unmap first, and the kunit suite itself frees domains with live
> mappings in pt_kunit_iommu_exit().

It is not misusing iommupt, but it is definitely misusing any iommu
API built on top of it. How did you trigger this?

> Verified with the generic_pt kunit suite as an in-tree reproducer:
> running the x86_64 format's test_pgsize_boundary() with

If this is the only trigger, then it is different testing things
interacting badly and I don't think it should be fixed like this.

Probably have the kunits directly unmap as they were being lazy

Jason

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

* Re: [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address
  2026-08-27 14:58 ` [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address Yuanhe Shu
  2026-08-27 16:13   ` Mostafa Saleh
@ 2026-08-27 16:21   ` Jason Gunthorpe
  1 sibling, 0 replies; 8+ messages in thread
From: Jason Gunthorpe @ 2026-08-27 16:21 UTC (permalink / raw)
  To: Yuanhe Shu
  Cc: joro, will, robin.murphy, baolu.lu, kevin.tian, smostafa, praan,
	skhawaja, iommu, linux-kernel

On Thu, Aug 27, 2026 at 10:58:54PM +0800, Yuanhe Shu wrote:
> The IOMMU_DEBUG_PAGEALLOC sanitizer takes a reference on each page at
> iommu_map() time, keyed by physical address, and the only way to drop
> them is the IOVA based iommu_debug_unmap_begin()/end().  A path that
> tears down a page table with mappings still installed has no IOVA to
> unmap with, so add a physical address based counterpart of
> __iommu_debug_map() for those paths, sharing the counting loop through
> __iommu_debug_update_phys().

This is not right. The following patch has the iova:

+                               iommu_debug_unmap_phys(&iommu_table->domain,
+                                                      pt_entry_oa(&pts), oasz);

Every pts has the iova of its current iteration location. Call
pt_index_to_va() and then pts->range->va is the current spot.

Jason

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

end of thread, other threads:[~2026-08-27 16:21 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-27 14:58 [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Yuanhe Shu
2026-08-27 14:58 ` [PATCH 1/2] iommu: Add __iommu_debug_unmap_phys() to drop refs by physical address Yuanhe Shu
2026-08-27 16:13   ` Mostafa Saleh
2026-08-27 16:17     ` Mostafa Saleh
2026-08-27 16:21   ` Jason Gunthorpe
2026-08-27 14:58 ` [PATCH 2/2] iommupt: Drop pagealloc references during domain deinit Yuanhe Shu
2026-08-27 16:18   ` Jason Gunthorpe
2026-08-27 16:05 ` [PATCH 0/2] iommu: Drop IOMMU_DEBUG_PAGEALLOC refs on iommupt domain teardown Mostafa Saleh

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®