Re: [PATCH v6 3/4] iommu: debug-pagealloc: Track IOMMU pages
From: Jason Gunthorpe
Date: Mon Jan 12 2026 - 13:27:24 EST
On Mon, Jan 12, 2026 at 02:58:47PM +0000, Mostafa Saleh wrote:
> On Mon, Jan 12, 2026 at 1:52 PM Jason Gunthorpe <jgg@xxxxxxxx> wrote:
> >
> > On Mon, Jan 12, 2026 at 01:43:41PM +0000, Mostafa Saleh wrote:
> > > But I don’t see why not. from the documentation:
> > > /**
> > > * pfn_valid - check if there is a valid memory map entry for a PFN
> > > * @pfn: the page frame number to check
> > > *
> > > * Check if there is a valid memory map entry aka struct page for the @pfn.
> > > * Note, that availability of the memory map entry does not imply that
> > > * there is actual usable memory at that @pfn. The struct page may
> > > * represent a hole or an unusable page frame.
> > > …
> > >
> > > That means that struct page exists, which is all what we need here.
> >
> > A struct page that has never been initialize shouldn't ever be read. I
> > don't know how that relates to page_ext, but are you really sure that
> > is all you need?
> >
>
> AFAIU, if pfn_valid() returns true, it means the struct page is valid,
> and lookup_page_ext() will check that a valid page_ext exists for this
> entry.
> So, what is missing is the NULL check for the page_ext returned, as it
> can be NULL even if pfn_valid() was true.
>
> But I can't see why we shouldn't use pfn_valid() at all in that path.
> I don't like the approach of using the prot to check that, as the
> driver can be buggy which is what the santizer is defending against.
> If we find some CONFIGs conflicting with it, we can just express that
> in Kconfig and disable the santaizer in that case.
That's a fair point.
How about adding a page_ext_from_phys() kind of function that could
use pfn_valid internally and has documented semantics for what it even
does for "holes" in the page map?
I'd be happier to see such a well defined API than randomly adding
pfn_valid() and phys_to_page() when we are trying hard to not have
those things in these paths.
> > That's sure looks sketchy to me.. Eg if CONFIG_WANT_PAGE_VIRTUAL is
> > set and you try to feed a MMIO through through that kmap() it will
> > explode.
> >
> > KVM can argue that it doesn't work with CONFIG_WANT_PAGE_VIRTUAL but
> > iommu cannot.
>
> WANT_PAGE_VIRTUAL seems possible in loongarch which supports KVM.
Yikes, maybe that configuration doesn't run KVM?
Jason