Re: [PATCH v2 2/3] iommu/vt-d: Clear Present bit before tearing down context entry

Samiullah Khawaja
2026-01-20T18:23:06Z

On Mon, Jan 19, 2026 at 10:20 PM Lu Baolu <baolu.lu@linux.intel.com> wrote:
>
> When tearing down a context entry, the current implementation zeros the
> entire 128-bit entry using multiple 64-bit writes. This creates a window
> where the hardware can fetch a "torn" entry — where some fields are
> already zeroed while the 'Present' bit is still set — leading to
> unpredictable behavior or spurious faults.
>
> While x86 provides strong write ordering, the compiler may reorder writes
> to the two 64-bit halves of the context entry. Even without compiler
> reordering, the hardware fetch is not guaranteed to be atomic with
> respect to multiple CPU writes.
>
> Align with 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 context entry first to
> signal the transition of ownership from hardware to software.
> 2. Use dma_wmb() to ensure the cleared bit is visible to the IOMMU.
> 3. Perform the required cache and context-cache invalidation to ensure
> hardware no longer has cached references to the entry.
> 4. Fully zero out the entry only after the invalidation is complete.
>
> Also, add a dma_wmb() to context_set_present() to ensure the entry
> is fully initialized before the 'Present' bit becomes visible.
>
> Fixes: ba39592764ed2 ("Intel IOMMU: Intel IOMMU driver")
> Reported-by: Dmytro Maluka <dmaluka@chromium.org>
> Closes: https://lore.kernel.org/all/aTG7gc7I5wExai3S@google.com/
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
> drivers/iommu/intel/iommu.h | 21 ++++++++++++++++++++-
> drivers/iommu/intel/iommu.c | 4 +++-
> 2 files changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
> index 25c5e22096d4..599913fb65d5 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -900,7 +900,26 @@ static inline int pfn_level_offset(u64 pfn, int level)
>
> static inline void context_set_present(struct context_entry *context)
> {
> - context->lo |= 1;
> + u64 val;
> +
> + dma_wmb();
> + val = READ_ONCE(context->lo) | 1;
> + WRITE_ONCE(context->lo, val);
> +}
> +
> +/*
> + * Clear the Present (P) bit (bit 0) of a context 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 context_clear_present(struct context_entry *context)
> +{
> + u64 val;
> +
> + val = READ_ONCE(context->lo) & GENMASK_ULL(63, 1);
> + WRITE_ONCE(context->lo, val);
> + dma_wmb();
> }
>
> static inline void context_set_fault_enable(struct context_entry *context)
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 134302fbcd92..c66cc51f9e51 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1240,10 +1240,12 @@ static void domain_context_clear_one(struct device_domain_info *info, u8 bus, u8
> }
>
> did = context_domain_id(context);
> - context_clear_entry(context);
> + context_clear_present(context);
> __iommu_flush_cache(iommu, context, sizeof(*context));
> spin_unlock(&iommu->lock);
> intel_context_flush_no_pasid(info, context, did);
> + context_clear_entry(context);
> + __iommu_flush_cache(iommu, context, sizeof(*context));
> }
>
> int __domain_setup_first_level(struct intel_iommu *iommu, struct device *dev,
> --
> 2.43.0
>

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