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

[Baolu Lu <baolu.lu@linux.intel.com>]

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?

> 
> 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.

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.

Thanks,
baolu