[PATCH v2 1/3] iommu/vt-d: Clear Present bit before tearing down PASID entry

Lu Baolu posted 3 patches 2 weeks, 5 days ago
[PATCH v2 1/3] iommu/vt-d: Clear Present bit before tearing down PASID entry
Posted by Lu Baolu 2 weeks, 5 days ago
The Intel VT-d Scalable Mode PASID table entry consists of 512 bits (64
bytes). When tearing down an entry, the current implementation zeros the
entire 64-byte structure immediately using multiple 64-bit writes.

Since the IOMMU hardware may fetch these 64 bytes using multiple
internal transactions (e.g., four 128-bit bursts), updating or zeroing
the entire entry while it is active (P=1) risks a "torn" read. If a
hardware fetch occurs simultaneously with the CPU zeroing the entry, the
hardware could observe an inconsistent state, leading to unpredictable
behavior or spurious faults.

Follow the "Guidance to Software for Invalidations" in the VT-d spec
(Section 6.5.3.3) by implementing the recommended ownership handshake:

1. Clear only the 'Present' (P) bit of the PASID entry.
2. Use a dma_wmb() to ensure the cleared bit is visible to hardware
   before proceeding.
3. Execute the required invalidation sequence (PASID cache, IOTLB, and
   Device-TLB flush) to ensure the hardware has released all cached
   references.
4. Only after the flushes are complete, zero out the remaining fields
   of the PASID entry.

Also, add a dma_wmb() in pasid_set_present() to ensure that all other
fields of the PASID entry are visible to the hardware before the Present
bit is set.

Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables")
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
---
 drivers/iommu/intel/pasid.h | 14 ++++++++++++++
 drivers/iommu/intel/pasid.c |  6 +++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
index b4c85242dc79..0b303bd0b0c1 100644
--- a/drivers/iommu/intel/pasid.h
+++ b/drivers/iommu/intel/pasid.h
@@ -234,9 +234,23 @@ static inline void pasid_set_wpe(struct pasid_entry *pe)
  */
 static inline void pasid_set_present(struct pasid_entry *pe)
 {
+	dma_wmb();
 	pasid_set_bits(&pe->val[0], 1 << 0, 1);
 }
 
+/*
+ * Clear the Present (P) bit (bit 0) of a scalable-mode PASID table entry.
+ * This initiates the transition of the entry's ownership from hardware
+ * to software. The caller is responsible for fulfilling the invalidation
+ * handshake recommended by the VT-d spec, Section 6.5.3.3 (Guidance to
+ * Software for Invalidations).
+ */
+static inline void pasid_clear_present(struct pasid_entry *pe)
+{
+	pasid_set_bits(&pe->val[0], 1 << 0, 0);
+	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 3e2255057079..eb069aefa4fa 100644
--- a/drivers/iommu/intel/pasid.c
+++ b/drivers/iommu/intel/pasid.c
@@ -272,7 +272,7 @@ 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);
-	intel_pasid_clear_entry(dev, pasid, fault_ignore);
+	pasid_clear_present(pte);
 	spin_unlock(&iommu->lock);
 
 	if (!ecap_coherent(iommu->ecap))
@@ -286,6 +286,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 (!ecap_coherent(iommu->ecap))
+		clflush_cache_range(pte, sizeof(*pte));
+
 	if (!fault_ignore)
 		intel_iommu_drain_pasid_prq(dev, pasid);
 }
-- 
2.43.0
RE: [PATCH v2 1/3] iommu/vt-d: Clear Present bit before tearing down PASID entry
Posted by Tian, Kevin 2 weeks, 4 days ago
> From: Lu Baolu <baolu.lu@linux.intel.com>
> Sent: Tuesday, January 20, 2026 2:18 PM
> 
> The Intel VT-d Scalable Mode PASID table entry consists of 512 bits (64
> bytes). When tearing down an entry, the current implementation zeros the
> entire 64-byte structure immediately using multiple 64-bit writes.
> 
> Since the IOMMU hardware may fetch these 64 bytes using multiple
> internal transactions (e.g., four 128-bit bursts), updating or zeroing
> the entire entry while it is active (P=1) risks a "torn" read. If a
> hardware fetch occurs simultaneously with the CPU zeroing the entry, the
> hardware could observe an inconsistent state, leading to unpredictable
> behavior or spurious faults.
> 
> Follow the "Guidance to Software for Invalidations" in the VT-d spec
> (Section 6.5.3.3) by implementing the recommended ownership handshake:
> 
> 1. Clear only the 'Present' (P) bit of the PASID entry.
> 2. Use a dma_wmb() to ensure the cleared bit is visible to hardware
>    before proceeding.
> 3. Execute the required invalidation sequence (PASID cache, IOTLB, and
>    Device-TLB flush) to ensure the hardware has released all cached
>    references.
> 4. Only after the flushes are complete, zero out the remaining fields
>    of the PASID entry.
> 
> Also, add a dma_wmb() in pasid_set_present() to ensure that all other
> fields of the PASID entry are visible to the hardware before the Present
> bit is set.
> 
> Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Reviewed-by: Kevin Tian <kevin.tian@intel.com>
Re: [PATCH v2 1/3] iommu/vt-d: Clear Present bit before tearing down PASID entry
Posted by Dmytro Maluka 2 weeks, 4 days ago
On Tue, Jan 20, 2026 at 02:18:12PM +0800, Lu Baolu wrote:
> The Intel VT-d Scalable Mode PASID table entry consists of 512 bits (64
> bytes). When tearing down an entry, the current implementation zeros the
> entire 64-byte structure immediately using multiple 64-bit writes.
> 
> Since the IOMMU hardware may fetch these 64 bytes using multiple
> internal transactions (e.g., four 128-bit bursts), updating or zeroing
> the entire entry while it is active (P=1) risks a "torn" read. If a
> hardware fetch occurs simultaneously with the CPU zeroing the entry, the
> hardware could observe an inconsistent state, leading to unpredictable
> behavior or spurious faults.
> 
> Follow the "Guidance to Software for Invalidations" in the VT-d spec
> (Section 6.5.3.3) by implementing the recommended ownership handshake:
> 
> 1. Clear only the 'Present' (P) bit of the PASID entry.
> 2. Use a dma_wmb() to ensure the cleared bit is visible to hardware
>    before proceeding.
> 3. Execute the required invalidation sequence (PASID cache, IOTLB, and
>    Device-TLB flush) to ensure the hardware has released all cached
>    references.
> 4. Only after the flushes are complete, zero out the remaining fields
>    of the PASID entry.
> 
> Also, add a dma_wmb() in pasid_set_present() to ensure that all other
> fields of the PASID entry are visible to the hardware before the Present
> bit is set.
> 
> Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables")
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>

Reviewed-by: Dmytro Maluka <dmaluka@chromium.org>

> ---
>  drivers/iommu/intel/pasid.h | 14 ++++++++++++++
>  drivers/iommu/intel/pasid.c |  6 +++++-
>  2 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
> index b4c85242dc79..0b303bd0b0c1 100644
> --- a/drivers/iommu/intel/pasid.h
> +++ b/drivers/iommu/intel/pasid.h
> @@ -234,9 +234,23 @@ static inline void pasid_set_wpe(struct pasid_entry *pe)
>   */
>  static inline void pasid_set_present(struct pasid_entry *pe)
>  {
> +	dma_wmb();
>  	pasid_set_bits(&pe->val[0], 1 << 0, 1);
>  }
>  
> +/*
> + * Clear the Present (P) bit (bit 0) of a scalable-mode PASID table entry.
> + * This initiates the transition of the entry's ownership from hardware
> + * to software. The caller is responsible for fulfilling the invalidation
> + * handshake recommended by the VT-d spec, Section 6.5.3.3 (Guidance to
> + * Software for Invalidations).
> + */
> +static inline void pasid_clear_present(struct pasid_entry *pe)
> +{
> +	pasid_set_bits(&pe->val[0], 1 << 0, 0);
> +	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 3e2255057079..eb069aefa4fa 100644
> --- a/drivers/iommu/intel/pasid.c
> +++ b/drivers/iommu/intel/pasid.c
> @@ -272,7 +272,7 @@ 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);
> -	intel_pasid_clear_entry(dev, pasid, fault_ignore);
> +	pasid_clear_present(pte);
>  	spin_unlock(&iommu->lock);
>  
>  	if (!ecap_coherent(iommu->ecap))
> @@ -286,6 +286,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 (!ecap_coherent(iommu->ecap))
> +		clflush_cache_range(pte, sizeof(*pte));
> +
>  	if (!fault_ignore)
>  		intel_iommu_drain_pasid_prq(dev, pasid);
>  }
> -- 
> 2.43.0
Re: [PATCH v2 1/3] iommu/vt-d: Clear Present bit before tearing down PASID entry
Posted by Samiullah Khawaja 2 weeks, 4 days ago
On Tue, Jan 20, 2026 at 5:56 AM Dmytro Maluka <dmaluka@chromium.org> wrote:
>
> On Tue, Jan 20, 2026 at 02:18:12PM +0800, Lu Baolu wrote:
> > The Intel VT-d Scalable Mode PASID table entry consists of 512 bits (64
> > bytes). When tearing down an entry, the current implementation zeros the
> > entire 64-byte structure immediately using multiple 64-bit writes.
> >
> > Since the IOMMU hardware may fetch these 64 bytes using multiple
> > internal transactions (e.g., four 128-bit bursts), updating or zeroing
> > the entire entry while it is active (P=1) risks a "torn" read. If a
> > hardware fetch occurs simultaneously with the CPU zeroing the entry, the
> > hardware could observe an inconsistent state, leading to unpredictable
> > behavior or spurious faults.
> >
> > Follow the "Guidance to Software for Invalidations" in the VT-d spec
> > (Section 6.5.3.3) by implementing the recommended ownership handshake:
> >
> > 1. Clear only the 'Present' (P) bit of the PASID entry.
> > 2. Use a dma_wmb() to ensure the cleared bit is visible to hardware
> >    before proceeding.
> > 3. Execute the required invalidation sequence (PASID cache, IOTLB, and
> >    Device-TLB flush) to ensure the hardware has released all cached
> >    references.
> > 4. Only after the flushes are complete, zero out the remaining fields
> >    of the PASID entry.
> >
> > Also, add a dma_wmb() in pasid_set_present() to ensure that all other
> > fields of the PASID entry are visible to the hardware before the Present
> > bit is set.
> >
> > Fixes: 0bbeb01a4faf ("iommu/vt-d: Manage scalalble mode PASID tables")
> > Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
>
> Reviewed-by: Dmytro Maluka <dmaluka@chromium.org>
>
> > ---
> >  drivers/iommu/intel/pasid.h | 14 ++++++++++++++
> >  drivers/iommu/intel/pasid.c |  6 +++++-
> >  2 files changed, 19 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/intel/pasid.h b/drivers/iommu/intel/pasid.h
> > index b4c85242dc79..0b303bd0b0c1 100644
> > --- a/drivers/iommu/intel/pasid.h
> > +++ b/drivers/iommu/intel/pasid.h
> > @@ -234,9 +234,23 @@ static inline void pasid_set_wpe(struct pasid_entry *pe)
> >   */
> >  static inline void pasid_set_present(struct pasid_entry *pe)
> >  {
> > +     dma_wmb();
> >       pasid_set_bits(&pe->val[0], 1 << 0, 1);
> >  }
> >
> > +/*
> > + * Clear the Present (P) bit (bit 0) of a scalable-mode PASID table entry.
> > + * This initiates the transition of the entry's ownership from hardware
> > + * to software. The caller is responsible for fulfilling the invalidation
> > + * handshake recommended by the VT-d spec, Section 6.5.3.3 (Guidance to
> > + * Software for Invalidations).
> > + */
> > +static inline void pasid_clear_present(struct pasid_entry *pe)
> > +{
> > +     pasid_set_bits(&pe->val[0], 1 << 0, 0);
> > +     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 3e2255057079..eb069aefa4fa 100644
> > --- a/drivers/iommu/intel/pasid.c
> > +++ b/drivers/iommu/intel/pasid.c
> > @@ -272,7 +272,7 @@ 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);
> > -     intel_pasid_clear_entry(dev, pasid, fault_ignore);
> > +     pasid_clear_present(pte);
> >       spin_unlock(&iommu->lock);
> >
> >       if (!ecap_coherent(iommu->ecap))
> > @@ -286,6 +286,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 (!ecap_coherent(iommu->ecap))
> > +             clflush_cache_range(pte, sizeof(*pte));
> > +
> >       if (!fault_ignore)
> >               intel_iommu_drain_pasid_prq(dev, pasid);
> >  }
> > --
> > 2.43.0

Reviewed-by: Samiullah Khawaja <skhawaja@google.com>