[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aSV85G0F4bQNYwHE@google.com>
Date: Tue, 25 Nov 2025 09:54:44 +0000
From: Mostafa Saleh <smostafa@...gle.com>
To: Baolu Lu <baolu.lu@...ux.intel.com>
Cc: linux-mm@...ck.org, iommu@...ts.linux.dev, linux-kernel@...r.kernel.org,
linux-doc@...r.kernel.org, corbet@....net, joro@...tes.org,
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, xiaqinxin@...wei.com
Subject: Re: [PATCH v3 1/4] drivers/iommu: Add page_ext for
IOMMU_DEBUG_PAGEALLOC
On Tue, Nov 25, 2025 at 03:17:47PM +0800, Baolu Lu wrote:
> On 11/25/25 04:08, Mostafa Saleh wrote:
> > Add a new config IOMMU_DEBUG_PAGEALLOC, which registers new data to
> > page_ext.
> >
> > This config will be used by the IOMMU API to track pages mapped in
> > the IOMMU to catch drivers trying to free kernel memory that they
> > still map in their domains, causing all types of memory corruption.
> >
> > This behaviour is disabled by default and can be enabled using
> > kernel cmdline iommu.debug_pagealloc.
> >
> > Signed-off-by: Mostafa Saleh <smostafa@...gle.com>
> > ---
> > .../admin-guide/kernel-parameters.txt | 6 ++++
> > drivers/iommu/Kconfig | 19 +++++++++++
> > drivers/iommu/Makefile | 1 +
> > drivers/iommu/iommu-debug-pagealloc.c | 32 +++++++++++++++++++
> > include/linux/iommu-debug-pagealloc.h | 17 ++++++++++
> > mm/page_ext.c | 4 +++
> > 6 files changed, 79 insertions(+)
> > create mode 100644 drivers/iommu/iommu-debug-pagealloc.c
> > create mode 100644 include/linux/iommu-debug-pagealloc.h
> >
>
> [..]
>
> > diff --git a/include/linux/iommu-debug-pagealloc.h b/include/linux/iommu-debug-pagealloc.h
> > new file mode 100644
> > index 000000000000..83e64d70bf6c
> > --- /dev/null
> > +++ b/include/linux/iommu-debug-pagealloc.h
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2025 - Google Inc
> > + * Author: Mostafa Saleh <smostafa@...gle.com>
> > + * IOMMU API debug page alloc sanitizer
> > + */
> > +
> > +#ifndef __LINUX_IOMMU_DEBUG_PAGEALLOC_H
> > +#define __LINUX_IOMMU_DEBUG_PAGEALLOC_H
> > +
> > +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> > +
> > +extern struct page_ext_operations page_iommu_debug_ops;
>
> How about moving above to below mm/page_ext.c and remove iommu-debug-
> pagealloc.h header file? ...
>
> > +
> > +#endif /* CONFIG_IOMMU_DEBUG_PAGEALLOC */
> > +
> > +#endif /* __LINUX_IOMMU_DEBUG_PAGEALLOC_H */
> > diff --git a/mm/page_ext.c b/mm/page_ext.c
> > index d7396a8970e5..297e4cd8ce90 100644
> > --- a/mm/page_ext.c
> > +++ b/mm/page_ext.c
> > @@ -11,6 +11,7 @@
> > #include <linux/page_table_check.h>
> > #include <linux/rcupdate.h>
> > #include <linux/pgalloc_tag.h>
> > +#include <linux/iommu-debug-pagealloc.h>
>
> ... remove this include line and put the "extern" line here,
>
> extern struct page_ext_operations page_iommu_debug_ops;
This file is needed for other functions added later, which is then
included by iommu.c mm.h
Also, the pattern in page_ext looks that users have their own headers:
include/linux/page_owner.h:extern struct page_ext_operations page_owner_ops;
include/linux/page_table_check.h:extern struct page_ext_operations page_table_check_ops;
include/linux/pgalloc_tag.h:extern struct page_ext_operations page_alloc_tagging_ops;
>
> > /*
> > * struct page extension
> > @@ -89,6 +90,9 @@ static struct page_ext_operations *page_ext_ops[] __initdata = {
> > #ifdef CONFIG_PAGE_TABLE_CHECK
> > &page_table_check_ops,
> > #endif
> > +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> > + &page_iommu_debug_ops,
> > +#endif
> > };
> > unsigned long page_ext_size;
>
> Or, put it directly in linux/iommu.h?
>
I tried that, but in the last patch we need to include that in linux/mm.h
which didn't compile and required including other files which seemed
unnecessary and that it would be better to isolate this feature.
Thanks,
Mostafa
> diff --git a/include/linux/iommu-debug-pagealloc.h
> b/include/linux/iommu-debug-pagealloc.h
> index 87f593305465..b2b82cf032e6 100644
> --- a/include/linux/iommu-debug-pagealloc.h
> +++ b/include/linux/iommu-debug-pagealloc.h
> @@ -14,8 +14,6 @@ struct iommu_domain;
>
> DECLARE_STATIC_KEY_FALSE(iommu_debug_initialized);
>
> -extern struct page_ext_operations page_iommu_debug_ops;
> -
> void __iommu_debug_map(struct iommu_domain *domain, phys_addr_t phys,
> size_t size);
> void __iommu_debug_unmap_begin(struct iommu_domain *domain,
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 801b2bd9e8d4..40133031985a 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -1185,6 +1185,10 @@ void iommu_detach_device_pasid(struct iommu_domain
> *domain,
> struct device *dev, ioasid_t pasid);
> ioasid_t iommu_alloc_global_pasid(struct device *dev);
> void iommu_free_global_pasid(ioasid_t pasid);
> +
> +#ifdef CONFIG_IOMMU_DEBUG_PAGEALLOC
> +extern struct page_ext_operations page_iommu_debug_ops;
> +#endif /* CONFIG_IOMMU_DEBUG_PAGEALLOC */
> #else /* CONFIG_IOMMU_API */
>
> struct iommu_ops {};
> diff --git a/mm/page_ext.c b/mm/page_ext.c
> index 297e4cd8ce90..345af8c9fcce 100644
> --- a/mm/page_ext.c
> +++ b/mm/page_ext.c
> @@ -11,7 +11,7 @@
> #include <linux/page_table_check.h>
> #include <linux/rcupdate.h>
> #include <linux/pgalloc_tag.h>
> -#include <linux/iommu-debug-pagealloc.h>
> +#include <linux/iommu.h>
>
> /*
> * struct page extension
>
> Thanks,
> baolu
Powered by blists - more mailing lists