[PATCH v4 2/4] iommu: Add calls for IOMMU_DEBUG_PAGEALLOC

Mostafa Saleh posted 4 patches 4 days, 16 hours ago
[PATCH v4 2/4] iommu: Add calls for IOMMU_DEBUG_PAGEALLOC
Posted by Mostafa Saleh 4 days, 16 hours ago
Add calls for the new iommu debug config IOMMU_DEBUG_PAGEALLOC:
- iommu_debug_init: Enable the debug mode if configured by the user.
- iommu_debug_map: Track iommu pages mapped, using physical address.
- iommu_debug_unmap_begin: Track start of iommu unmap operation, with
  IOVA and size.
- iommu_debug_unmap_end: Track the end of unmap operation, passing the
  actual unmapped size versus the tracked one at unmap_begin.

We have to do the unmap_begin/end as once pages are unmapped we lose
the information of the physical address.
This is racy, but the API is racy by construction as it uses refcounts
and doesn't attempt to lock/synchronize with the IOMMU API as that will
be costly, meaning that possibility of false negative exists.

Signed-off-by: Mostafa Saleh <smostafa@google.com>
---
 drivers/iommu/iommu-debug-pagealloc.c | 28 +++++++++++++
 drivers/iommu/iommu-priv.h            | 58 +++++++++++++++++++++++++++
 drivers/iommu/iommu.c                 | 11 ++++-
 include/linux/iommu-debug-pagealloc.h |  1 +
 4 files changed, 96 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
index 4022e9af7f27..1d343421da98 100644
--- a/drivers/iommu/iommu-debug-pagealloc.c
+++ b/drivers/iommu/iommu-debug-pagealloc.c
@@ -5,11 +5,15 @@
  * IOMMU API debug page alloc sanitizer
  */
 #include <linux/atomic.h>
+#include <linux/iommu.h>
 #include <linux/iommu-debug-pagealloc.h>
 #include <linux/kernel.h>
 #include <linux/page_ext.h>
 
+#include "iommu-priv.h"
+
 static bool needed;
+DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
 
 struct iommu_debug_metadata {
 	atomic_t ref;
@@ -25,6 +29,30 @@ struct page_ext_operations page_iommu_debug_ops = {
 	.need = need_iommu_debug,
 };
 
+void __iommu_debug_map(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)
+{
+}
+
+void iommu_debug_init(void)
+{
+	if (!needed)
+		return;
+
+	pr_info("iommu: Debugging page allocations, expect overhead or disable iommu.debug_pagealloc");
+	static_branch_enable(&iommu_debug_initialized);
+}
+
 static int __init iommu_debug_pagealloc(char *str)
 {
 	return kstrtobool(str, &needed);
diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
index c95394cd03a7..aaffad5854fc 100644
--- a/drivers/iommu/iommu-priv.h
+++ b/drivers/iommu/iommu-priv.h
@@ -5,6 +5,7 @@
 #define __LINUX_IOMMU_PRIV_H
 
 #include <linux/iommu.h>
+#include <linux/iommu-debug-pagealloc.h>
 #include <linux/msi.h>
 
 static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
@@ -65,4 +66,61 @@ static inline int iommufd_sw_msi(struct iommu_domain *domain,
 int iommu_replace_device_pasid(struct iommu_domain *domain,
 			       struct device *dev, ioasid_t pasid,
 			       struct iommu_attach_handle *handle);
+
+#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
+
+void __iommu_debug_map(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 void iommu_debug_map(struct iommu_domain *domain,
+				   phys_addr_t phys, size_t size)
+{
+	if (static_branch_unlikely(&iommu_debug_initialized))
+		__iommu_debug_map(domain, phys, size);
+}
+
+static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
+					   unsigned long iova, size_t size)
+{
+	if (static_branch_unlikely(&iommu_debug_initialized))
+		__iommu_debug_unmap_begin(domain, iova, size);
+}
+
+static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
+					 unsigned long iova, size_t size,
+					 size_t unmapped)
+{
+	if (static_branch_unlikely(&iommu_debug_initialized))
+		__iommu_debug_unmap_end(domain, iova, size, unmapped);
+}
+
+void iommu_debug_init(void);
+
+#else
+static inline void iommu_debug_map(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)
+{
+}
+
+static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
+					 unsigned long iova, size_t size,
+					 size_t unmapped)
+{
+}
+
+static inline void iommu_debug_init(void)
+{
+}
+
+#endif /* CONFIG_IOMMU_DEBUG_PAGEALLOC */
+
 #endif /* __LINUX_IOMMU_PRIV_H */
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2ca990dfbb88..01b062575519 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -232,6 +232,8 @@ static int __init iommu_subsys_init(void)
 	if (!nb)
 		return -ENOMEM;
 
+	iommu_debug_init();
+
 	for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
 		nb[i].notifier_call = iommu_bus_notifier;
 		bus_register_notifier(iommu_buses[i], &nb[i]);
@@ -2562,10 +2564,12 @@ int iommu_map_nosync(struct iommu_domain *domain, unsigned long iova,
 	}
 
 	/* unroll mapping in case something went wrong */
-	if (ret)
+	if (ret) {
 		iommu_unmap(domain, orig_iova, orig_size - size);
-	else
+	} else {
 		trace_map(orig_iova, orig_paddr, orig_size);
+		iommu_debug_map(domain, orig_paddr, orig_size);
+	}
 
 	return ret;
 }
@@ -2627,6 +2631,8 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
 
 	pr_debug("unmap this: iova 0x%lx size 0x%zx\n", iova, size);
 
+	iommu_debug_unmap_begin(domain, iova, size);
+
 	/*
 	 * Keep iterating until we either unmap 'size' bytes (or more)
 	 * or we hit an area that isn't mapped.
@@ -2647,6 +2653,7 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
 	}
 
 	trace_unmap(orig_iova, size, unmapped);
+	iommu_debug_unmap_end(domain, orig_iova, size, unmapped);
 	return unmapped;
 }
 
diff --git a/include/linux/iommu-debug-pagealloc.h b/include/linux/iommu-debug-pagealloc.h
index 83e64d70bf6c..a439d6815ca1 100644
--- a/include/linux/iommu-debug-pagealloc.h
+++ b/include/linux/iommu-debug-pagealloc.h
@@ -9,6 +9,7 @@
 #define __LINUX_IOMMU_DEBUG_PAGEALLOC_H
 
 #ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
+DECLARE_STATIC_KEY_FALSE(iommu_debug_initialized);
 
 extern struct page_ext_operations page_iommu_debug_ops;
 
-- 
2.52.0.223.gf5cc29aaa4-goog
Re: [PATCH v4 2/4] iommu: Add calls for IOMMU_DEBUG_PAGEALLOC
Posted by Baolu Lu 4 days, 3 hours ago
On 12/11/25 20:59, Mostafa Saleh wrote:
> Add calls for the new iommu debug config IOMMU_DEBUG_PAGEALLOC:
> - iommu_debug_init: Enable the debug mode if configured by the user.
> - iommu_debug_map: Track iommu pages mapped, using physical address.
> - iommu_debug_unmap_begin: Track start of iommu unmap operation, with
>    IOVA and size.
> - iommu_debug_unmap_end: Track the end of unmap operation, passing the
>    actual unmapped size versus the tracked one at unmap_begin.
> 
> We have to do the unmap_begin/end as once pages are unmapped we lose
> the information of the physical address.
> This is racy, but the API is racy by construction as it uses refcounts
> and doesn't attempt to lock/synchronize with the IOMMU API as that will
> be costly, meaning that possibility of false negative exists.
> 
> Signed-off-by: Mostafa Saleh <smostafa@google.com>
> ---
>   drivers/iommu/iommu-debug-pagealloc.c | 28 +++++++++++++
>   drivers/iommu/iommu-priv.h            | 58 +++++++++++++++++++++++++++
>   drivers/iommu/iommu.c                 | 11 ++++-
>   include/linux/iommu-debug-pagealloc.h |  1 +
>   4 files changed, 96 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
> index 4022e9af7f27..1d343421da98 100644
> --- a/drivers/iommu/iommu-debug-pagealloc.c
> +++ b/drivers/iommu/iommu-debug-pagealloc.c
> @@ -5,11 +5,15 @@
>    * IOMMU API debug page alloc sanitizer
>    */
>   #include <linux/atomic.h>
> +#include <linux/iommu.h>
>   #include <linux/iommu-debug-pagealloc.h>
>   #include <linux/kernel.h>
>   #include <linux/page_ext.h>
>   
> +#include "iommu-priv.h"
> +
>   static bool needed;
> +DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
>   
>   struct iommu_debug_metadata {
>   	atomic_t ref;
> @@ -25,6 +29,30 @@ struct page_ext_operations page_iommu_debug_ops = {
>   	.need = need_iommu_debug,
>   };
>   
> +void __iommu_debug_map(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)
> +{
> +}
> +
> +void iommu_debug_init(void)
> +{
> +	if (!needed)
> +		return;
> +
> +	pr_info("iommu: Debugging page allocations, expect overhead or disable iommu.debug_pagealloc");
> +	static_branch_enable(&iommu_debug_initialized);
> +}
> +
>   static int __init iommu_debug_pagealloc(char *str)
>   {
>   	return kstrtobool(str, &needed);
> diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
> index c95394cd03a7..aaffad5854fc 100644
> --- a/drivers/iommu/iommu-priv.h
> +++ b/drivers/iommu/iommu-priv.h
> @@ -5,6 +5,7 @@
>   #define __LINUX_IOMMU_PRIV_H
>   
>   #include <linux/iommu.h>
> +#include <linux/iommu-debug-pagealloc.h>
>   #include <linux/msi.h>
>   
>   static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
> @@ -65,4 +66,61 @@ static inline int iommufd_sw_msi(struct iommu_domain *domain,
>   int iommu_replace_device_pasid(struct iommu_domain *domain,
>   			       struct device *dev, ioasid_t pasid,
>   			       struct iommu_attach_handle *handle);
> +
> +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> +
> +void __iommu_debug_map(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 void iommu_debug_map(struct iommu_domain *domain,
> +				   phys_addr_t phys, size_t size)
> +{
> +	if (static_branch_unlikely(&iommu_debug_initialized))
> +		__iommu_debug_map(domain, phys, size);
> +}
> +
> +static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
> +					   unsigned long iova, size_t size)
> +{
> +	if (static_branch_unlikely(&iommu_debug_initialized))
> +		__iommu_debug_unmap_begin(domain, iova, size);
> +}
> +
> +static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
> +					 unsigned long iova, size_t size,
> +					 size_t unmapped)
> +{
> +	if (static_branch_unlikely(&iommu_debug_initialized))
> +		__iommu_debug_unmap_end(domain, iova, size, unmapped);
> +}

I am wondering whether it would be better if we move iommu_debug_map()
to iommu-debug-pagealloc.c,

void iommu_debug_map(struct iommu_domain *domain,
		     phys_addr_t phys, size_t size)
{
	if (static_branch_likely(&iommu_debug_initialized))
		__iommu_debug_map(domain, phys, size);
}

(Does it make sense to use static_branch_likely() here? Normally, people
  who enable CONFIG_IOMMU_DEBUG_PAGEALLOC would want to use this
  debugging feature. Or not?)

So that ...

> +
> +void iommu_debug_init(void);
> +
> +#else
> +static inline void iommu_debug_map(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)
> +{
> +}
> +
> +static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
> +					 unsigned long iova, size_t size,
> +					 size_t unmapped)
> +{
> +}
> +
> +static inline void iommu_debug_init(void)
> +{
> +}
> +
> +#endif /* CONFIG_IOMMU_DEBUG_PAGEALLOC */
> +
>   #endif /* __LINUX_IOMMU_PRIV_H */
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 2ca990dfbb88..01b062575519 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -232,6 +232,8 @@ static int __init iommu_subsys_init(void)
>   	if (!nb)
>   		return -ENOMEM;
>   
> +	iommu_debug_init();
> +
>   	for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
>   		nb[i].notifier_call = iommu_bus_notifier;
>   		bus_register_notifier(iommu_buses[i], &nb[i]);
> @@ -2562,10 +2564,12 @@ int iommu_map_nosync(struct iommu_domain *domain, unsigned long iova,
>   	}
>   
>   	/* unroll mapping in case something went wrong */
> -	if (ret)
> +	if (ret) {
>   		iommu_unmap(domain, orig_iova, orig_size - size);
> -	else
> +	} else {
>   		trace_map(orig_iova, orig_paddr, orig_size);
> +		iommu_debug_map(domain, orig_paddr, orig_size);
> +	}
>   
>   	return ret;
>   }
> @@ -2627,6 +2631,8 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
>   
>   	pr_debug("unmap this: iova 0x%lx size 0x%zx\n", iova, size);
>   
> +	iommu_debug_unmap_begin(domain, iova, size);
> +
>   	/*
>   	 * Keep iterating until we either unmap 'size' bytes (or more)
>   	 * or we hit an area that isn't mapped.
> @@ -2647,6 +2653,7 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
>   	}
>   
>   	trace_unmap(orig_iova, size, unmapped);
> +	iommu_debug_unmap_end(domain, orig_iova, size, unmapped);
>   	return unmapped;
>   }
>   
> diff --git a/include/linux/iommu-debug-pagealloc.h b/include/linux/iommu-debug-pagealloc.h
> index 83e64d70bf6c..a439d6815ca1 100644
> --- a/include/linux/iommu-debug-pagealloc.h
> +++ b/include/linux/iommu-debug-pagealloc.h
> @@ -9,6 +9,7 @@
>   #define __LINUX_IOMMU_DEBUG_PAGEALLOC_H
>   
>   #ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> +DECLARE_STATIC_KEY_FALSE(iommu_debug_initialized);

... we could make this static?

>   
>   extern struct page_ext_operations page_iommu_debug_ops;
>   

Thanks,
baolu
Re: [PATCH v4 2/4] iommu: Add calls for IOMMU_DEBUG_PAGEALLOC
Posted by Mostafa Saleh 3 days, 11 hours ago
On Fri, Dec 12, 2025 at 10:33:20AM +0800, Baolu Lu wrote:
> On 12/11/25 20:59, Mostafa Saleh wrote:
> > Add calls for the new iommu debug config IOMMU_DEBUG_PAGEALLOC:
> > - iommu_debug_init: Enable the debug mode if configured by the user.
> > - iommu_debug_map: Track iommu pages mapped, using physical address.
> > - iommu_debug_unmap_begin: Track start of iommu unmap operation, with
> >    IOVA and size.
> > - iommu_debug_unmap_end: Track the end of unmap operation, passing the
> >    actual unmapped size versus the tracked one at unmap_begin.
> > 
> > We have to do the unmap_begin/end as once pages are unmapped we lose
> > the information of the physical address.
> > This is racy, but the API is racy by construction as it uses refcounts
> > and doesn't attempt to lock/synchronize with the IOMMU API as that will
> > be costly, meaning that possibility of false negative exists.
> > 
> > Signed-off-by: Mostafa Saleh <smostafa@google.com>
> > ---
> >   drivers/iommu/iommu-debug-pagealloc.c | 28 +++++++++++++
> >   drivers/iommu/iommu-priv.h            | 58 +++++++++++++++++++++++++++
> >   drivers/iommu/iommu.c                 | 11 ++++-
> >   include/linux/iommu-debug-pagealloc.h |  1 +
> >   4 files changed, 96 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
> > index 4022e9af7f27..1d343421da98 100644
> > --- a/drivers/iommu/iommu-debug-pagealloc.c
> > +++ b/drivers/iommu/iommu-debug-pagealloc.c
> > @@ -5,11 +5,15 @@
> >    * IOMMU API debug page alloc sanitizer
> >    */
> >   #include <linux/atomic.h>
> > +#include <linux/iommu.h>
> >   #include <linux/iommu-debug-pagealloc.h>
> >   #include <linux/kernel.h>
> >   #include <linux/page_ext.h>
> > +#include "iommu-priv.h"
> > +
> >   static bool needed;
> > +DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
> >   struct iommu_debug_metadata {
> >   	atomic_t ref;
> > @@ -25,6 +29,30 @@ struct page_ext_operations page_iommu_debug_ops = {
> >   	.need = need_iommu_debug,
> >   };
> > +void __iommu_debug_map(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)
> > +{
> > +}
> > +
> > +void iommu_debug_init(void)
> > +{
> > +	if (!needed)
> > +		return;
> > +
> > +	pr_info("iommu: Debugging page allocations, expect overhead or disable iommu.debug_pagealloc");
> > +	static_branch_enable(&iommu_debug_initialized);
> > +}
> > +
> >   static int __init iommu_debug_pagealloc(char *str)
> >   {
> >   	return kstrtobool(str, &needed);
> > diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
> > index c95394cd03a7..aaffad5854fc 100644
> > --- a/drivers/iommu/iommu-priv.h
> > +++ b/drivers/iommu/iommu-priv.h
> > @@ -5,6 +5,7 @@
> >   #define __LINUX_IOMMU_PRIV_H
> >   #include <linux/iommu.h>
> > +#include <linux/iommu-debug-pagealloc.h>
> >   #include <linux/msi.h>
> >   static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
> > @@ -65,4 +66,61 @@ static inline int iommufd_sw_msi(struct iommu_domain *domain,
> >   int iommu_replace_device_pasid(struct iommu_domain *domain,
> >   			       struct device *dev, ioasid_t pasid,
> >   			       struct iommu_attach_handle *handle);
> > +
> > +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> > +
> > +void __iommu_debug_map(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 void iommu_debug_map(struct iommu_domain *domain,
> > +				   phys_addr_t phys, size_t size)
> > +{
> > +	if (static_branch_unlikely(&iommu_debug_initialized))
> > +		__iommu_debug_map(domain, phys, size);
> > +}
> > +
> > +static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
> > +					   unsigned long iova, size_t size)
> > +{
> > +	if (static_branch_unlikely(&iommu_debug_initialized))
> > +		__iommu_debug_unmap_begin(domain, iova, size);
> > +}
> > +
> > +static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
> > +					 unsigned long iova, size_t size,
> > +					 size_t unmapped)
> > +{
> > +	if (static_branch_unlikely(&iommu_debug_initialized))
> > +		__iommu_debug_unmap_end(domain, iova, size, unmapped);
> > +}
> 
> I am wondering whether it would be better if we move iommu_debug_map()
> to iommu-debug-pagealloc.c,
> 
> void iommu_debug_map(struct iommu_domain *domain,
> 		     phys_addr_t phys, size_t size)
> {
> 	if (static_branch_likely(&iommu_debug_initialized))
> 		__iommu_debug_map(domain, phys, size);
> }
> 
> (Does it make sense to use static_branch_likely() here? Normally, people
>  who enable CONFIG_IOMMU_DEBUG_PAGEALLOC would want to use this
>  debugging feature. Or not?)
> 
> So that ...

This actually was the v1 implementation [1], but Jörg suggested to move
it to a header file as a function call would have an overhead if this
feautre is disabled.

I believe the priority would be to keep the performance overhead minimal
with CONFIG_IOMMU_DEBUG_PAGEALLOC and the commandline disabled, so people
can run with the config in production and only enable the commandline
it to debug problems, without having overhead on the typical case.

> 
> > +
> > +void iommu_debug_init(void);
> > +
> > +#else
> > +static inline void iommu_debug_map(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)
> > +{
> > +}
> > +
> > +static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
> > +					 unsigned long iova, size_t size,
> > +					 size_t unmapped)
> > +{
> > +}
> > +
> > +static inline void iommu_debug_init(void)
> > +{
> > +}
> > +
> > +#endif /* CONFIG_IOMMU_DEBUG_PAGEALLOC */
> > +
> >   #endif /* __LINUX_IOMMU_PRIV_H */
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2ca990dfbb88..01b062575519 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -232,6 +232,8 @@ static int __init iommu_subsys_init(void)
> >   	if (!nb)
> >   		return -ENOMEM;
> > +	iommu_debug_init();
> > +
> >   	for (int i = 0; i < ARRAY_SIZE(iommu_buses); i++) {
> >   		nb[i].notifier_call = iommu_bus_notifier;
> >   		bus_register_notifier(iommu_buses[i], &nb[i]);
> > @@ -2562,10 +2564,12 @@ int iommu_map_nosync(struct iommu_domain *domain, unsigned long iova,
> >   	}
> >   	/* unroll mapping in case something went wrong */
> > -	if (ret)
> > +	if (ret) {
> >   		iommu_unmap(domain, orig_iova, orig_size - size);
> > -	else
> > +	} else {
> >   		trace_map(orig_iova, orig_paddr, orig_size);
> > +		iommu_debug_map(domain, orig_paddr, orig_size);
> > +	}
> >   	return ret;
> >   }
> > @@ -2627,6 +2631,8 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
> >   	pr_debug("unmap this: iova 0x%lx size 0x%zx\n", iova, size);
> > +	iommu_debug_unmap_begin(domain, iova, size);
> > +
> >   	/*
> >   	 * Keep iterating until we either unmap 'size' bytes (or more)
> >   	 * or we hit an area that isn't mapped.
> > @@ -2647,6 +2653,7 @@ static size_t __iommu_unmap(struct iommu_domain *domain,
> >   	}
> >   	trace_unmap(orig_iova, size, unmapped);
> > +	iommu_debug_unmap_end(domain, orig_iova, size, unmapped);
> >   	return unmapped;
> >   }
> > diff --git a/include/linux/iommu-debug-pagealloc.h b/include/linux/iommu-debug-pagealloc.h
> > index 83e64d70bf6c..a439d6815ca1 100644
> > --- a/include/linux/iommu-debug-pagealloc.h
> > +++ b/include/linux/iommu-debug-pagealloc.h
> > @@ -9,6 +9,7 @@
> >   #define __LINUX_IOMMU_DEBUG_PAGEALLOC_H
> >   #ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> > +DECLARE_STATIC_KEY_FALSE(iommu_debug_initialized);
> 
> ... we could make this static?
>

This is not static because of the header usage as mentioned above.

Thanks,
Mostafa

> >   extern struct page_ext_operations page_iommu_debug_ops;

[1] https://lore.kernel.org/linux-iommu/20251003173229.1533640-2-smostafa@google.com/
> 
> Thanks,
> baolu
> 
Re: [PATCH v4 2/4] iommu: Add calls for IOMMU_DEBUG_PAGEALLOC
Posted by Baolu Lu 1 day, 2 hours ago
On 12/13/25 02:44, Mostafa Saleh wrote:
> On Fri, Dec 12, 2025 at 10:33:20AM +0800, Baolu Lu wrote:
>> On 12/11/25 20:59, Mostafa Saleh wrote:
>>> Add calls for the new iommu debug config IOMMU_DEBUG_PAGEALLOC:
>>> - iommu_debug_init: Enable the debug mode if configured by the user.
>>> - iommu_debug_map: Track iommu pages mapped, using physical address.
>>> - iommu_debug_unmap_begin: Track start of iommu unmap operation, with
>>>     IOVA and size.
>>> - iommu_debug_unmap_end: Track the end of unmap operation, passing the
>>>     actual unmapped size versus the tracked one at unmap_begin.
>>>
>>> We have to do the unmap_begin/end as once pages are unmapped we lose
>>> the information of the physical address.
>>> This is racy, but the API is racy by construction as it uses refcounts
>>> and doesn't attempt to lock/synchronize with the IOMMU API as that will
>>> be costly, meaning that possibility of false negative exists.
>>>
>>> Signed-off-by: Mostafa Saleh<smostafa@google.com>
>>> ---
>>>    drivers/iommu/iommu-debug-pagealloc.c | 28 +++++++++++++
>>>    drivers/iommu/iommu-priv.h            | 58 +++++++++++++++++++++++++++
>>>    drivers/iommu/iommu.c                 | 11 ++++-
>>>    include/linux/iommu-debug-pagealloc.h |  1 +
>>>    4 files changed, 96 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/iommu/iommu-debug-pagealloc.c b/drivers/iommu/iommu-debug-pagealloc.c
>>> index 4022e9af7f27..1d343421da98 100644
>>> --- a/drivers/iommu/iommu-debug-pagealloc.c
>>> +++ b/drivers/iommu/iommu-debug-pagealloc.c
>>> @@ -5,11 +5,15 @@
>>>     * IOMMU API debug page alloc sanitizer
>>>     */
>>>    #include <linux/atomic.h>
>>> +#include <linux/iommu.h>
>>>    #include <linux/iommu-debug-pagealloc.h>
>>>    #include <linux/kernel.h>
>>>    #include <linux/page_ext.h>
>>> +#include "iommu-priv.h"
>>> +
>>>    static bool needed;
>>> +DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
>>>    struct iommu_debug_metadata {
>>>    	atomic_t ref;
>>> @@ -25,6 +29,30 @@ struct page_ext_operations page_iommu_debug_ops = {
>>>    	.need = need_iommu_debug,
>>>    };
>>> +void __iommu_debug_map(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)
>>> +{
>>> +}
>>> +
>>> +void iommu_debug_init(void)
>>> +{
>>> +	if (!needed)
>>> +		return;
>>> +
>>> +	pr_info("iommu: Debugging page allocations, expect overhead or disable iommu.debug_pagealloc");
>>> +	static_branch_enable(&iommu_debug_initialized);
>>> +}
>>> +
>>>    static int __init iommu_debug_pagealloc(char *str)
>>>    {
>>>    	return kstrtobool(str, &needed);
>>> diff --git a/drivers/iommu/iommu-priv.h b/drivers/iommu/iommu-priv.h
>>> index c95394cd03a7..aaffad5854fc 100644
>>> --- a/drivers/iommu/iommu-priv.h
>>> +++ b/drivers/iommu/iommu-priv.h
>>> @@ -5,6 +5,7 @@
>>>    #define __LINUX_IOMMU_PRIV_H
>>>    #include <linux/iommu.h>
>>> +#include <linux/iommu-debug-pagealloc.h>
>>>    #include <linux/msi.h>
>>>    static inline const struct iommu_ops *dev_iommu_ops(struct device *dev)
>>> @@ -65,4 +66,61 @@ static inline int iommufd_sw_msi(struct iommu_domain *domain,
>>>    int iommu_replace_device_pasid(struct iommu_domain *domain,
>>>    			       struct device *dev, ioasid_t pasid,
>>>    			       struct iommu_attach_handle *handle);
>>> +
>>> +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
>>> +
>>> +void __iommu_debug_map(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 void iommu_debug_map(struct iommu_domain *domain,
>>> +				   phys_addr_t phys, size_t size)
>>> +{
>>> +	if (static_branch_unlikely(&iommu_debug_initialized))
>>> +		__iommu_debug_map(domain, phys, size);
>>> +}
>>> +
>>> +static inline void iommu_debug_unmap_begin(struct iommu_domain *domain,
>>> +					   unsigned long iova, size_t size)
>>> +{
>>> +	if (static_branch_unlikely(&iommu_debug_initialized))
>>> +		__iommu_debug_unmap_begin(domain, iova, size);
>>> +}
>>> +
>>> +static inline void iommu_debug_unmap_end(struct iommu_domain *domain,
>>> +					 unsigned long iova, size_t size,
>>> +					 size_t unmapped)
>>> +{
>>> +	if (static_branch_unlikely(&iommu_debug_initialized))
>>> +		__iommu_debug_unmap_end(domain, iova, size, unmapped);
>>> +}
>> I am wondering whether it would be better if we move iommu_debug_map()
>> to iommu-debug-pagealloc.c,
>>
>> void iommu_debug_map(struct iommu_domain *domain,
>> 		     phys_addr_t phys, size_t size)
>> {
>> 	if (static_branch_likely(&iommu_debug_initialized))
>> 		__iommu_debug_map(domain, phys, size);
>> }
>>
>> (Does it make sense to use static_branch_likely() here? Normally, people
>>   who enable CONFIG_IOMMU_DEBUG_PAGEALLOC would want to use this
>>   debugging feature. Or not?)
>>
>> So that ...
> This actually was the v1 implementation [1], but Jörg suggested to move
> it to a header file as a function call would have an overhead if this
> feautre is disabled.
> 
> I believe the priority would be to keep the performance overhead minimal
> with CONFIG_IOMMU_DEBUG_PAGEALLOC and the commandline disabled, so people
> can run with the config in production and only enable the commandline
> it to debug problems, without having overhead on the typical case.

Okay, fair enough.

Thanks,
baolu