Re: [RFC PATCH 2/4] drivers/iommu: Add calls for iommu debug

["Jörg Rödel" <joro@8bytes.org>]

On Fri, Oct 03, 2025 at 05:32:27PM +0000, 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: Track iommu pages unmapped, using IO virtual
>   address.
> - iommu_debug_remap: Track iommu pages, already mapped using IOVA.
> 
> We have to do the unmap/remap 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.c | 23 +++++++++++++++++++++++
>  drivers/iommu/iommu.c       | 21 +++++++++++++++++++--
>  include/linux/iommu-debug.h |  6 ++++++
>  3 files changed, 48 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/iommu-debug.c b/drivers/iommu/iommu-debug.c
> index 297a35137b38..607f1fcf2235 100644
> --- a/drivers/iommu/iommu-debug.c
> +++ b/drivers/iommu/iommu-debug.c
> @@ -5,11 +5,13 @@
>   * IOMMU API santaizers and debug
>   */
>  #include <linux/atomic.h>
> +#include <linux/iommu.h>
>  #include <linux/iommu-debug.h>
>  #include <linux/kernel.h>
>  #include <linux/page_ext.h>
>  
>  static bool needed;
> +static DEFINE_STATIC_KEY_FALSE(iommu_debug_initialized);
>  
>  struct iommu_debug_metadate {
>  	atomic_t ref;
> @@ -25,6 +27,27 @@ 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(struct iommu_domain *domain, unsigned long iova, size_t size)
> +{
> +}
> +
> +void iommu_debug_remap(struct iommu_domain *domain, unsigned long iova, size_t size)
> +{
> +}
> +
> +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.c b/drivers/iommu/iommu.c
> index 060ebe330ee1..56c89636a33c 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -18,6 +18,7 @@
>  #include <linux/errno.h>
>  #include <linux/host1x_context_bus.h>
>  #include <linux/iommu.h>
> +#include <linux/iommu-debug.h>
>  #include <linux/iommufd.h>
>  #include <linux/idr.h>
>  #include <linux/err.h>
> @@ -231,6 +232,9 @@ static int __init iommu_subsys_init(void)
>  	if (!nb)
>  		return -ENOMEM;
>  
> +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> +	iommu_debug_init();
> +#endif
>  	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]);
> @@ -2518,10 +2522,14 @@ 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);
> +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> +		iommu_debug_map(domain, orig_paddr, orig_size);
> +#endif

These #ifdefs need to be in a header file, not the actual source code.