[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAFgf54q+9Y5TtGJDB=8q_BW-0F=TM7zBbCcMzvtvr_N2WMnd-w@mail.gmail.com>
Date: Mon, 12 Jan 2026 14:58:47 +0000
From: Mostafa Saleh <smostafa@...gle.com>
To: Jason Gunthorpe <jgg@...pe.ca>
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,
baolu.lu@...ux.intel.com, rdunlap@...radead.org,
Samiullah Khawaja <skhawaja@...gle.com>
Subject: Re: [PATCH v6 3/4] iommu: debug-pagealloc: Track IOMMU pages
On Mon, Jan 12, 2026 at 1:52 PM Jason Gunthorpe <jgg@...pe.ca> 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.
> > I can see many places have the same pattern in the kernel already, for example:
> > - vfio_iommu_type1.c, is_invalid_reserved_pfn() which does the same
> > check which can include MMIO and then get the page struct.
>
> This whole flow is nonsensical and wrong though, I wouldn't point to
> it as something reliable.
>
> > - kvm_main.c: in __kvm_vcpu_map(), it distinguishes MMIO from memory
> > and then accesses the page struct.
>
> 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.
Thanks,
Mostafa
> So, again, IDK, we are trying not to use pfn_valid() in the DMA code.
>
> Jason
Powered by blists - more mailing lists