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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aQIjP3jVigyz8t38@google.com>
Date: Wed, 29 Oct 2025 14:22:55 +0000
From: Mostafa Saleh <smostafa@...gle.com>
To: Jörg Rödel <joro@...tes.org>
Cc: linux-mm@...ck.org, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, corbet@....net, will@...nel.org,
	robin.murphy@....com, akpm@...ux-foundation.org, vbabka@...e.cz,
	surenb@...gle.com, mhocko@...e.com, jackmanb@...gle.com,
	hannes@...xchg.org, ziy@...dia.com, david@...hat.com,
	lorenzo.stoakes@...cle.com, Liam.Howlett@...cle.com,
	rppt@...nel.org
Subject: Re: [RFC PATCH 2/4] drivers/iommu: Add calls for iommu debug

On Mon, Oct 27, 2025 at 12:43:06PM +0100, Jörg Rödel wrote:
> 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@...gle.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.

I will fix it in v2.

Thanks,
Mostafa

>