linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Mostafa Saleh <smostafa@google.com>
To: Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-mm@kvack.org, iommu@lists.linux.dev,
	linux-kernel@vger.kernel.org,  linux-doc@vger.kernel.org,
	corbet@lwn.net, joro@8bytes.org, will@kernel.org,
	 robin.murphy@arm.com, akpm@linux-foundation.org, vbabka@suse.cz,
	 surenb@google.com, mhocko@suse.com, jackmanb@google.com,
	hannes@cmpxchg.org,  ziy@nvidia.com, david@redhat.com,
	lorenzo.stoakes@oracle.com,  Liam.Howlett@oracle.com,
	rppt@kernel.org, xiaqinxin@huawei.com,  baolu.lu@linux.intel.com,
	rdunlap@infradead.org,  Samiullah Khawaja <skhawaja@google.com>
Subject: Re: [PATCH v6 3/4] iommu: debug-pagealloc: Track IOMMU pages
Date: Mon, 12 Jan 2026 14:58:47 +0000	[thread overview]
Message-ID: <CAFgf54q+9Y5TtGJDB=8q_BW-0F=TM7zBbCcMzvtvr_N2WMnd-w@mail.gmail.com> (raw)
In-Reply-To: <20260112135208.GD745888@ziepe.ca>

On Mon, Jan 12, 2026 at 1:52 PM Jason Gunthorpe <jgg@ziepe.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


  reply	other threads:[~2026-01-12 14:59 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-09 17:18 [PATCH v6 0/4] iommu: Add page_ext for IOMMU_DEBUG_PAGEALLOC Mostafa Saleh
2026-01-09 17:18 ` [PATCH v6 1/4] " Mostafa Saleh
2026-01-09 17:18 ` [PATCH v6 2/4] iommu: Add calls " Mostafa Saleh
2026-01-09 17:18 ` [PATCH v6 3/4] iommu: debug-pagealloc: Track IOMMU pages Mostafa Saleh
2026-01-09 19:15   ` Pranjal Shrivastava
2026-01-09 19:51   ` Jason Gunthorpe
2026-01-12 10:20     ` Mostafa Saleh
2026-01-12 13:32       ` Jason Gunthorpe
2026-01-12 13:43         ` Mostafa Saleh
2026-01-12 13:52           ` Jason Gunthorpe
2026-01-12 14:58             ` Mostafa Saleh [this message]
2026-01-12 18:27               ` Jason Gunthorpe
2026-01-12 19:11               ` David Hildenbrand (Red Hat)
2026-01-12 19:17                 ` Jason Gunthorpe
2026-01-09 17:18 ` [PATCH v6 4/4] iommu: debug-pagealloc: Check mapped/unmapped kernel memory Mostafa Saleh
2026-01-09 19:16   ` Pranjal Shrivastava
2026-01-10  9:53 ` [PATCH v6 0/4] iommu: Add page_ext for IOMMU_DEBUG_PAGEALLOC Jörg Rödel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAFgf54q+9Y5TtGJDB=8q_BW-0F=TM7zBbCcMzvtvr_N2WMnd-w@mail.gmail.com' \
    --to=smostafa@google.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=baolu.lu@linux.intel.com \
    --cc=corbet@lwn.net \
    --cc=david@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=iommu@lists.linux.dev \
    --cc=jackmanb@google.com \
    --cc=jgg@ziepe.ca \
    --cc=joro@8bytes.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lorenzo.stoakes@oracle.com \
    --cc=mhocko@suse.com \
    --cc=rdunlap@infradead.org \
    --cc=robin.murphy@arm.com \
    --cc=rppt@kernel.org \
    --cc=skhawaja@google.com \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=will@kernel.org \
    --cc=xiaqinxin@huawei.com \
    --cc=ziy@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox