From: Jason Gunthorpe <jgg@ziepe.ca>
To: Vasant Hegde <vasant.hegde@amd.com>
Cc: iommu@lists.linux.dev, joro@8bytes.org, will@kernel.org,
robin.murphy@arm.com, suravee.suthikulpanit@amd.com,
Alejandro Jimenez <alejandro.j.jimenez@oracle.com>,
stable@vger.kernel.org, Joao Martins <joao.m.martins@oracle.com>
Subject: Re: [PATCH] iommu/amd/pgtbl: Fix possible race while increase page table level
Date: Thu, 18 Sep 2025 11:17:37 -0300 [thread overview]
Message-ID: <20250918141737.GP1326709@ziepe.ca> (raw)
In-Reply-To: <20250911121416.633216-1-vasant.hegde@amd.com>
On Thu, Sep 11, 2025 at 12:14:15PM +0000, Vasant Hegde wrote:
> The IOMMU IOVA allocator initially starts with 32-bit address and onces its
> exhuasted it switches to 64-bit address (max address is determined based
> on IOMMU and device DMA capability). To support larger IOVA, AMD IOMMU
> driver increases page table level.
Is this the case? I thought I saw something that the allocator is
starting from high addresses?
> But in unmap path (iommu_v1_unmap_pages()), fetch_pte() reads
> pgtable->[root/mode] without lock. So its possible that in exteme corner case,
> when increase_address_space() is updating pgtable->[root/mode], fetch_pte()
> reads wrong page table level (pgtable->mode). It does compare the value with
> level encoded in page table and returns NULL. This will result is
> iommu_unmap ops to fail and upper layer may retry/log WARN_ON.
Yep, definately a bug, I spotted it already and fixed it in iommupt,
you can read about it here:
https://lore.kernel.org/linux-iommu/13-v5-116c4948af3d+68091-iommu_pt_jgg@nvidia.com/
> CPU 0 CPU 1
> ------ ------
> map pages unmap pages
> alloc_pte() -> increase_address_space() iommu_v1_unmap_pages() -> fetch_pte()
> pgtable->root = pte (new root value)
> READ pgtable->[mode/root]
> Reads new root, old mode
> Updates mode (pgtable->mode += 1)
This doesn't solve the whole problem, yes reading the two values
coherently is important but we must also serialize parallel map such
that map only returns if the IOMMU is actually programmed with the new
roots.
I don't see that in this fix.
IMHO unless someone is actually hitting this I'd leave it and focus on
merging iomupt which fully fixes this without adding any locks to the
fast path.
Jason
next prev parent reply other threads:[~2025-09-18 14:17 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-09-11 12:14 [PATCH] iommu/amd/pgtbl: Fix possible race while increase page table level Vasant Hegde
2025-09-13 6:10 ` Jörg Rödel
2025-09-13 6:18 ` Vasant Hegde
2025-09-18 14:17 ` Jason Gunthorpe [this message]
2025-09-22 10:05 ` Vasant Hegde
2025-09-22 13:02 ` Jason Gunthorpe
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=20250918141737.GP1326709@ziepe.ca \
--to=jgg@ziepe.ca \
--cc=alejandro.j.jimenez@oracle.com \
--cc=iommu@lists.linux.dev \
--cc=joao.m.martins@oracle.com \
--cc=joro@8bytes.org \
--cc=robin.murphy@arm.com \
--cc=stable@vger.kernel.org \
--cc=suravee.suthikulpanit@amd.com \
--cc=vasant.hegde@amd.com \
--cc=will@kernel.org \
/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