* [PATCH 1/2] iommu/vt-d: Set FPD when tearing down a PASID entry with fault_ignore [not found] <20261104051059.1345010-1-baolu.lu@linux.intel.com> @ 2026-11-04 5:10 ` Lu Baolu 2026-11-05 22:01 ` Vinicius Costa Gomes 0 siblings, 1 reply; 4+ messages in thread From: Lu Baolu @ 2026-11-04 5:10 UTC (permalink / raw) To: Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe, Kevin Tian Cc: iommu, linux-kernel, Lu Baolu, stable When an SVA process exits, intel_mm_release() tears down its PASID entry with fault_ignore=true. It does not drain the PRQ at that point. Instead, it is supposed to leave the FPD bit set. Later, when the device is unbound, the driver checks for this bit and drains the PRQ. Commit 75ed00055c05 ("iommu/vt-d: Clear Present bit before tearing down PASID entry") changed the teardown order. It clears the Present bit before calling intel_pasid_clear_entry(), which sets the FPD bit only if the entry is still present. As a result, FPD is not set and the entire entry is cleared, so the PRQ is not drained for that PASID during unbind: intel_mm_release() intel_pasid_tear_down_entry(fault_ignore=true) pasid_clear_present() intel_pasid_clear_entry() <- P=0, so FPD is not set ... iommu_sva_unbind_device() iommu_detach_device_pasid() blocking_domain_set_dev_pasid() intel_pasid_tear_down_entry(fault_ignore=false) !present && !FPD <- return, no PRQ drain intel_iommu_disable_iopf() iopf_queue_remove_device() <- groups still queued kfree(handle) <- handle still used by iopf work Page requests queued before process exit may later be handled by iommu_sva_handle_iopf() after the SVA attach handle has been freed, causing a use-after-free. Fix this by setting FPD in the same write that clears the Present bit when fault_ignore is requested. Fixes: 75ed00055c05 ("iommu/vt-d: Clear Present bit before tearing down PASID entry") Cc: stable@vger.kernel.org Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com> --- drivers/iommu/intel/pasid.h | 11 +++++++++++ drivers/iommu/intel/pasid.c | 28 ++++++++++------------------ 2 files changed, 21 insertions(+), 18 deletions(-) diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h index 48d3bb6b68de..af798a6ee121 100644 --- a/drivers/iommu/intel/pasid.h +++ b/drivers/iommu/intel/pasid.h @@ -251,6 +251,17 @@ static inline void pasid_clear_present(struct pasid_entry *pe) dma_wmb(); } +/* + * Clear the Present bit and set the FPD bit in a single write, so that + * hardware never sees the entry non-present without FPD. + */ +static inline void pasid_clear_present_set_fpd(struct pasid_entry *pe) +{ + pasid_set_bits(&pe->val[0], PASID_PTE_PRESENT | PASID_PTE_FPD, + PASID_PTE_FPD); + dma_wmb(); +} + /* * Setup Page Walk Snoop bit (Bit 87) of a scalable mode PASID * entry. diff --git a/drivers/iommu/intel/pasid.c b/drivers/iommu/intel/pasid.c index e4f24d3f19a6..d1c08d180788 100644 --- a/drivers/iommu/intel/pasid.c +++ b/drivers/iommu/intel/pasid.c @@ -178,21 +178,6 @@ static struct pasid_entry *intel_pasid_get_entry(struct device *dev, u32 pasid) /* * Interfaces for PASID table entry manipulation: */ -static void -intel_pasid_clear_entry(struct device *dev, u32 pasid, bool fault_ignore) -{ - struct pasid_entry *pe; - - pe = intel_pasid_get_entry(dev, pasid); - if (WARN_ON(!pe)) - return; - - if (fault_ignore && pasid_pte_is_present(pe)) - pasid_clear_entry_with_fpd(pe); - else - pasid_clear_entry(pe); -} - static void pasid_cache_invalidation_with_pasid(struct intel_iommu *iommu, u16 did, u32 pasid) @@ -253,7 +238,6 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, if (!pasid_pte_is_present(pte)) { if (!pasid_pte_is_fault_disabled(pte)) { - WARN_ON(READ_ONCE(pte->val[0]) != 0); spin_unlock(&iommu->lock); return; } @@ -266,6 +250,8 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, */ pasid_clear_entry(pte); spin_unlock(&iommu->lock); + if (!ecap_coherent(iommu->ecap)) + clflush_cache_range(pte, sizeof(*pte)); intel_iommu_drain_pasid_prq(dev, pasid); return; @@ -273,7 +259,10 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, did = pasid_get_domain_id(pte); pgtt = pasid_pte_get_pgtt(pte); - pasid_clear_present(pte); + if (fault_ignore) + pasid_clear_present_set_fpd(pte); + else + pasid_clear_present(pte); spin_unlock(&iommu->lock); if (!ecap_coherent(iommu->ecap)) @@ -287,7 +276,10 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, iommu->flush.flush_iotlb(iommu, did, 0, 0, DMA_TLB_DSI_FLUSH); devtlb_invalidation_with_pasid(iommu, dev, pasid); - intel_pasid_clear_entry(dev, pasid, fault_ignore); + if (fault_ignore) + pasid_clear_entry_with_fpd(pte); + else + pasid_clear_entry(pte); if (!ecap_coherent(iommu->ecap)) clflush_cache_range(pte, sizeof(*pte)); -- 2.43.0 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Set FPD when tearing down a PASID entry with fault_ignore 2026-11-04 5:10 ` [PATCH 1/2] iommu/vt-d: Set FPD when tearing down a PASID entry with fault_ignore Lu Baolu @ 2026-11-05 22:01 ` Vinicius Costa Gomes 2026-11-09 7:34 ` Baolu Lu 0 siblings, 1 reply; 4+ messages in thread From: Vinicius Costa Gomes @ 2026-11-05 22:01 UTC (permalink / raw) To: Lu Baolu, Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe, Kevin Tian Cc: iommu, linux-kernel, Lu Baolu, stable Lu Baolu <baolu.lu@linux.intel.com> writes: > static void > pasid_cache_invalidation_with_pasid(struct intel_iommu *iommu, > u16 did, u32 pasid) > @@ -253,7 +238,6 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, > > if (!pasid_pte_is_present(pte)) { > if (!pasid_pte_is_fault_disabled(pte)) { > - WARN_ON(READ_ONCE(pte->val[0]) != 0); > spin_unlock(&iommu->lock); > return; > } > @@ -266,6 +250,8 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, > */ > pasid_clear_entry(pte); > spin_unlock(&iommu->lock); > + if (!ecap_coherent(iommu->ecap)) > + clflush_cache_range(pte, sizeof(*pte)); > intel_iommu_drain_pasid_prq(dev, pasid); > I think it's worth documenting in the commit message why both of these are needed, removing the warning and adding the flush. Cheers, -- Vinicius ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/2] iommu/vt-d: Set FPD when tearing down a PASID entry with fault_ignore 2026-11-05 22:01 ` Vinicius Costa Gomes @ 2026-11-09 7:34 ` Baolu Lu 2026-11-12 5:58 ` Tian, Kevin 0 siblings, 1 reply; 4+ messages in thread From: Baolu Lu @ 2026-11-09 7:34 UTC (permalink / raw) To: Vinicius Costa Gomes, Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe, Kevin Tian Cc: baolu.lu, iommu, linux-kernel, stable On 11/6/2026 6:01 AM, Vinicius Costa Gomes wrote: > Lu Baolu<baolu.lu@linux.intel.com> writes: > >> static void >> pasid_cache_invalidation_with_pasid(struct intel_iommu *iommu, >> u16 did, u32 pasid) >> @@ -253,7 +238,6 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, >> >> if (!pasid_pte_is_present(pte)) { >> if (!pasid_pte_is_fault_disabled(pte)) { >> - WARN_ON(READ_ONCE(pte->val[0]) != 0); >> spin_unlock(&iommu->lock); >> return; >> } >> @@ -266,6 +250,8 @@ void intel_pasid_tear_down_entry(struct intel_iommu *iommu, struct device *dev, >> */ >> pasid_clear_entry(pte); >> spin_unlock(&iommu->lock); >> + if (!ecap_coherent(iommu->ecap)) >> + clflush_cache_range(pte, sizeof(*pte)); >> intel_iommu_drain_pasid_prq(dev, pasid); >> > I think it's worth documenting in the commit message why both of these > are needed, removing the warning and adding the flush. Yes. I will add the following to the commit message: Also drop the WARN_ON() for a non-present entry without FPD. Since commit 75ed00055c05 a concurrent teardown can see such an entry before it is zeroed. Flush the cache after clearing an FPD-only entry, as other PASID entry updates do on non-coherent hardware. Thanks, baolu ^ permalink raw reply [flat|nested] 4+ messages in thread
* RE: [PATCH 1/2] iommu/vt-d: Set FPD when tearing down a PASID entry with fault_ignore 2026-11-09 7:34 ` Baolu Lu @ 2026-11-12 5:58 ` Tian, Kevin 0 siblings, 0 replies; 4+ messages in thread From: Tian, Kevin @ 2026-11-12 5:58 UTC (permalink / raw) To: Baolu Lu, Gomes, Vinicius, Joerg Roedel, Will Deacon, Robin Murphy, Jason Gunthorpe Cc: iommu@lists.linux.dev, linux-kernel@vger.kernel.org, stable@vger.kernel.org > From: Baolu Lu <baolu.lu@linux.intel.com> > Sent: Monday, November 9, 2026 3:35 PM > > On 11/6/2026 6:01 AM, Vinicius Costa Gomes wrote: > > Lu Baolu<baolu.lu@linux.intel.com> writes: > > > >> static void > >> pasid_cache_invalidation_with_pasid(struct intel_iommu *iommu, > >> u16 did, u32 pasid) > >> @@ -253,7 +238,6 @@ void intel_pasid_tear_down_entry(struct > intel_iommu *iommu, struct device *dev, > >> > >> if (!pasid_pte_is_present(pte)) { > >> if (!pasid_pte_is_fault_disabled(pte)) { > >> - WARN_ON(READ_ONCE(pte->val[0]) != 0); > >> spin_unlock(&iommu->lock); > >> return; > >> } > >> @@ -266,6 +250,8 @@ void intel_pasid_tear_down_entry(struct > intel_iommu *iommu, struct device *dev, > >> */ > >> pasid_clear_entry(pte); > >> spin_unlock(&iommu->lock); > >> + if (!ecap_coherent(iommu->ecap)) > >> + clflush_cache_range(pte, sizeof(*pte)); > >> intel_iommu_drain_pasid_prq(dev, pasid); > >> > > I think it's worth documenting in the commit message why both of these > > are needed, removing the warning and adding the flush. > > Yes. I will add the following to the commit message: > > Also drop the WARN_ON() for a non-present entry without FPD. Since > commit 75ed00055c05 a concurrent teardown can see such an entry before > it is zeroed. Flush the cache after clearing an FPD-only entry, as > other PASID entry updates do on non-coherent hardware. > hmm is it a separate fix? ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-11-12 5:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261104051059.1345010-1-baolu.lu@linux.intel.com>
2026-11-04 5:10 ` [PATCH 1/2] iommu/vt-d: Set FPD when tearing down a PASID entry with fault_ignore Lu Baolu
2026-11-05 22:01 ` Vinicius Costa Gomes
2026-11-09 7:34 ` Baolu Lu
2026-11-12 5:58 ` Tian, Kevin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox