Re: [PATCH v4 08/18] iommu/vt-d: Clear unpreserved context entries during shutdown

[Samiullah Khawaja <skhawaja@google.com>]

On Thu, Aug 27, 2026 at 02:09:26PM +0800, Baolu Lu wrote:
>On 8/27/26 04:30, Samiullah Khawaja wrote:
>>
>>>>+static void clear_unpreserved_context(struct device_domain_info 
>>>>*info, u8 bus, u8 devfn)
>>>>+{
>>>>+    struct context_entry *context;
>>>>+
>>>>+    /*
>>>>+     * This cleanup is done during shutdown, so it should be 
>>>>fine to only
>>>>+     * clear the entries here and issue one global invalidation 
>>>>later to
>>>>+     * invalidate all cleared entries.
>>>>+     *
>>>>+     * Note that the device IOTLB invalidation for unpreserved 
>>>>devices is
>>>>+     * skipped this way, but that should not be needed as the 
>>>>devices are
>>>>+     * quiesced at this point. This should improve the 
>>>>performance of the
>>>>+     * cleanup process and avoids any invalidation timeouts 
>>>>because drivers
>>>>+     * might have moved devices to D3 state.
>>>>+     */
>>>>+    context = iommu_context_addr(info->iommu, bus, devfn, 0);
>>>>+    if (context) {
>>>>+        context_clear_entry(context);
>>>>+        __iommu_flush_cache(info->iommu, context, sizeof(*context));
>>>
>>>For tearing down a present context entry, please follow the VT-d
>>>recommended sequence:
>>>
>>>- clear only the Present bit,
>>>- flush the updated entry to memory if necessary,
>>>- issue the required cache invalidations,
>>
>>Do we still need the individual cache invalidations if we issue global
>>invalidations (context, pasid-cache and iotlb), after clearing all
>>entries, as they would be done if a new root table was being setup?
>>
>>Looking at the VT-d specs (Invalidation of Translation Caches), each
>>invalidation type (cache, pasid and iotlb) defines granularity in both
>>register and queue based interface. And the granularity indicates that
>>Global invalidations clear the cached entries for that specific type.
>>For example following text is used for each cache type (in queue
>>interface):
>>
>>   Context-cache:
>>   Global Invalidation (01b): All context-cache entries cached at the
>>   remapping hardware are invalidated.
>>
>>   Pasid-cache:
>>   Global Invalidation (11b): All PASID-cache entries are invalidated.
>>
>>   Iotlb:
>>   Global Invalidation (01b):
>>   - All IOTLB entries are invalidated.
>>   - All paging-structure-cache entries are invalidated.
>>
>>A similar note about using Global invalidation is suggested in the specs
>>when setting up root table (Set Root Table Pointer Operation).
>>
>>   ... software must perform a global invalidate of the contextcache,
>>   PASID-cache (if applicable), and IOTLB, in that order. This is
>>   required to ensure hardware references only the remapping structures
>>   referenced by the new root table pointer and not stale cached entries.
>>
>>Also please note that this is happening during dmar unit teardown and
>>system shutdown, and while the context table entries in root table are
>>being cleared, the memory is not freed until the global invalidation is
>>issued.
>>
>>Since this is during shutdown, issuing global invalidations instead of
>>multiple individual invalidations for devices and aliases is simpler and
>>would likely also have shutdown time improvements and reduce the
>>blackout time during liveupdate.
>>
>>Please let me know if my global invalidations and granularity
>>understanding is not correct.
>
>The global-invalidation approach (similar to the “set new root table”
>flow) looks correct to me.
>
>On the device side: if ATS is enabled, some translations may still be
>cached in the device. My understanding is that unpreserved devices are
>already DMA-quiesced and then go through reset + reprobe (as in a normal
>reboot), which should flush those device-side caches. Are we aligned on
>that assumption?

Yes, we are aligned on it. I already added a note about unpreserved
devices being quiesced at this point, in a comment above in this
function.
>
>>
>>I added a comment at the top of this function to explain this, let me
>>know if you want me to expand it with more details.
>>
>>>- then clear the remaining fields of the entry.
>
>One important point remains: even if cache invalidation is deferred and
>done globally, we should still clear the Present bit before clearing the
>rest of a context entry. While Present is set, hardware may fetch the
>256-bit entry in multiple chunks. Rewriting the full entry is not atomic
>(it becomes multiple CPU writes), so hardware could observe a torn value
>(a mix of old and new fields), which may lead to undefined behavior or
>spurious faults.

Ah yes, I wanted to write that in my previous reply but it seems I
missed that.
>
>So the recommended sequence is:
>- clear only the Present bit,
>- flush the updated entry to memory if necessary,
>- [add a comment about the delayed global invalidation approach,]
>- then clear the remaining fields of the entry.

This is exactly my plan for the next revision. We are aligned on it.
>
>Thanks,
>baolu
>

Thanks,
Sami