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

[Samiullah Khawaja <skhawaja@google.com>]

On Fri, Oct 09, 2026 at 08:43:26AM +0800, Baolu Lu wrote:
>On 9/21/26 08:48, Samiullah Khawaja wrote:
>>During normal shutdown the iommu translation is disabled. Since the root
>>table is preserved during live update, it needs to be cleaned up and the
>>context entries of the unpreserved devices and root entries for the
>>unpreserved context tables need to be cleared.
>>
>>This is required because during kexec reboot and shutdown the devices do
>>not go through the release flow, so there might be stale entries in the
>>root table and context tables. Also note that new unpreserved context
>>tables for unpreserved devices might have been added after preservation,
>>so the root table entries for unpreserved context tables also need to
>>removed.
>>
>>Signed-off-by: Samiullah Khawaja<skhawaja@google.com>
>>---
>>  drivers/iommu/intel/iommu.c      |  15 +++-
>>  drivers/iommu/intel/iommu.h      |   5 ++
>>  drivers/iommu/intel/liveupdate.c | 139 +++++++++++++++++++++++++++++++
>>  3 files changed, 157 insertions(+), 2 deletions(-)
>>
>>diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
>>index 4bfc2f173010..474c926172c5 100644
>>--- a/drivers/iommu/intel/iommu.c
>>+++ b/drivers/iommu/intel/iommu.c
>>@@ -1852,6 +1852,14 @@ static int iommu_suspend(void *data)
>>  	iommu_flush_all();
>>+	/*
>>+	 * Note that IOMMU suspend doesn't affect live update. The state
>>+	 * preserved during live update is not released and remains valid during
>>+	 * suspend and reused during IOMMU resume.
>>+	 *
>>+	 * Also note deployment of suspend/resume and live updated use case
>>+	 * should be mostly mutually exclusive.
>>+	 */
>>  	for_each_active_iommu(iommu, drhd) {
>>  		iommu_disable_translation(iommu);
>>@@ -2397,8 +2405,11 @@ void intel_iommu_shutdown(void)
>>  		/* Disable PMRs explicitly here. */
>>  		iommu_disable_protect_mem_regions(iommu);
>>-		/* Make sure the IOMMUs are switched off */
>>-		iommu_disable_translation(iommu);
>>+		/* Make sure the IOMMUs are switched off if not preserved. */
>>+		if (iommu_preserved_state(&iommu->iommu))
>>+			clear_unpreserved_context_entries(iommu);
>>+		else
>>+			iommu_disable_translation(iommu);
>>  	}
>>  }
>>diff --git a/drivers/iommu/intel/iommu.h b/drivers/iommu/intel/iommu.h
>>index 785057236e7c..4feb5bd76b18 100644
>>--- a/drivers/iommu/intel/iommu.h
>>+++ b/drivers/iommu/intel/iommu.h
>>@@ -1307,6 +1307,11 @@ int intel_iommu_preserve(struct iommu_device *iommu,
>>  			 struct iommu_hw_ser *iommu_ser);
>>  void intel_iommu_unpreserve(struct iommu_device *iommu,
>>  			    struct iommu_hw_ser *iommu_ser);
>>+void clear_unpreserved_context_entries(struct intel_iommu *iommu);
>>+#else
>>+static inline void clear_unpreserved_context_entries(struct intel_iommu *iommu)
>>+{
>>+}
>>  #endif
>>  #ifdef CONFIG_INTEL_IOMMU_SVM
>>diff --git a/drivers/iommu/intel/liveupdate.c b/drivers/iommu/intel/liveupdate.c
>>index 0837bb889fed..501dc0e9cc0a 100644
>>--- a/drivers/iommu/intel/liveupdate.c
>>+++ b/drivers/iommu/intel/liveupdate.c
>>@@ -77,6 +77,145 @@ static int preserve_context_table(struct intel_iommu *iommu,
>>  	return 0;
>>  }
>>+static void clear_unpreserved_context_root_entries(struct intel_iommu *iommu,
>>+						   struct iommu_hw_ser *ser)
>>+{
>>+	struct root_entry *root;
>>+	int i;
>>+
>>+	/*
>>+	 * Individual invalidations for each context table removal are not
>>+	 * needed as we issue global invalidations later.
>>+	 */
>>+	for (i = 0; i < ROOT_ENTRY_NR; i++) {
>>+		root = &iommu->root_entry[i];
>>+
>>+		if (!is_context_table_preserved(iommu, ser, i, 0) && (root->lo & 1)) {
>>+			root->lo = 0;
>>+			__iommu_flush_cache(iommu,
>>+					    &root->lo,
>>+					    sizeof(root->lo));
>>+		}
>>+
>>+		if (!sm_supported(iommu))
>>+			continue;
>>+
>>+		if (!is_context_table_preserved(iommu, ser, i, 0x80) && (root->hi & 1)) {
>>+			root->hi = 0;
>>+			__iommu_flush_cache(iommu,
>>+					    &root->hi,
>>+					    sizeof(root->hi));
>>+		}
>>+	}
>>+}
>>+
>>+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.
>
>I don't quite follow why device-TLB flushes could be skipped when the
>device is quiesced. Nothing guarantees that an unpreserved device that
>has ATS enabled doesn't carry stale cache entries. It may keep
>translations to memory that the next kernel reuses.

I agree with your assessment, the device-TLB might still have stale
entries. But this assumption is based on the existing kexec reboot
behaviour. That is, during normal shutdown the iommu translation is
disabled but the device-TLB invalidations are not issued either. The
unpreserved devices go through the same flow in the next kernel as they
do after a normal kexec reboot. I will update the comment to add this
detail.
>
>I would suggest at least a global device-TLB flush for ATS-enabled
>unpreserved devices, or is there anything I've overlooked?

This might be tricky as this late during shutdown some devices might
already be in a low power state, and sending them device-TLB
invalidations would cause invalidation timeouts. Even if that doesn't
happen, it will still add to the shutdown time.
>
>>+	 *
>>+	 * The iommu lock is not needed as the context tables are never removed
>>+	 * and the new ones are not added during shutdown.
>>+	 */
>>+	context = iommu_context_addr(info->iommu, bus, devfn, 0);
>>+	if (context) {
>>+		/*
>>+		 * Individual invalidations are not needed as we issue global
>>+		 * invalidations later once all the cleanups are done.
>>+		 */
>>+		context_clear_present(context);
>>+		__iommu_flush_cache(info->iommu, context, sizeof(*context));
>>+		context_clear_entry(context);
>>+		__iommu_flush_cache(info->iommu, context, sizeof(*context));
>>+	}
>>+}
>
>[-snipped-]
>
>Thanks,
>baolu
>

Thanks,
Sami