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

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

On 10/25/26 10:19, 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 | 134 +++++++++++++++++++++++++++++++
>   3 files changed, 152 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iommu/intel/iommu.c b/drivers/iommu/intel/iommu.c
> index 14bd1ca4fbd5..23a1da351ddf 100644
> --- a/drivers/iommu/intel/iommu.c
> +++ b/drivers/iommu/intel/iommu.c
> @@ -1967,6 +1967,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);
>   
> @@ -2512,8 +2520,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 a6c0ed55db30..16ab103d308a 100644
> --- a/drivers/iommu/intel/iommu.h
> +++ b/drivers/iommu/intel/iommu.h
> @@ -1316,6 +1316,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 0047f9d92cbc..f92cb774cc99 100644
> --- a/drivers/iommu/intel/liveupdate.c
> +++ b/drivers/iommu/intel/liveupdate.c
> @@ -82,6 +82,140 @@ 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. This matches the normal kexec reboot, where the
> +	 * IOMMU translation is disabled without invalidating the device IOTLBs,
> +	 * and the unpreserved devices go through the same flow in the next
> +	 * kernel as they do after a normal kexec reboot. Issuing device IOTLB
> +	 * invalidations this late during shutdown could also cause invalidation
> +	 * timeouts, as drivers might have moved devices to a low power state,
> +	 * and would 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));
> +	}
> +}
> +
> +static int clear_unpreserved_alias_cb(struct pci_dev *pdev, u16 alias, void *data)
> +{
> +	struct device_domain_info *info = data;
> +
> +	clear_unpreserved_context(info, PCI_BUS_NUM(alias), alias & 0xff);
> +	return 0;
> +}
> +
> +static int clear_unpreserve_context_entry_fn(struct device *dev,
> +					     struct iommu_device *iommu_dev)
> +{
> +	struct device_domain_info *info;
> +
> +	info = dev_iommu_priv_get(dev);
> +	if (!info)
> +		return 0;
> +
> +	if (dev_iommu_preserved_state(dev))
> +		return 0;
> +
> +	if (dev_is_pci(dev))
> +		pci_for_each_dma_alias(to_pci_dev(dev),
> +				       clear_unpreserved_alias_cb, info);
> +	else
> +		clear_unpreserved_context(info, info->bus, info->devfn);
> +
> +	return 0;
> +}
> +
> +/**
> + * clear_unpreserved_context_entries() - Clear context entries for unpreserved devices
> + * @iommu: Target IOMMU
> + *
> + * Clear the context entries of unpreserved devices during shutdown before kexec.
> + */
> +void clear_unpreserved_context_entries(struct intel_iommu *iommu)
> +{
> +	/*
> +	 * Clear context entries for unpreserved devices 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.
> +	 *
> +	 * Note that the error can be ignored as the iterator function does not
> +	 * fail.
> +	 */
> +	iommu_for_each_dev(&iommu->iommu, clear_unpreserve_context_entry_fn);
> +
> +	/*
> +	 * New unpreserved context tables for unpreserved devices might have
> +	 * been added after preservation, so the root table entries for
> +	 * unpreserved context tables need to be removed.
> +	 */
> +	clear_unpreserved_context_root_entries(iommu,
> +					       iommu_preserved_state(&iommu->iommu));
> +
> +	/*
> +	 * Some devices might not have teardown/detached properly depending on
> +	 * whether a proper device remove is done before kexec is triggered.
> +	 * Also unpreserved context tables and entries are removed during
> +	 * shutdown. So issue global invalidations to remove references to
> +	 * unpreserved tables and entries.
> +	 */
> +	iommu->flush.flush_context(iommu, 0, 0, 0, DMA_CCMD_GLOBAL_INVL);
> +	if (sm_supported(iommu))
> +		qi_flush_pasid_cache(iommu, 0, QI_PC_GLOBAL, 0);
> +	iommu->flush.flush_iotlb(iommu, 0, 0, 0, DMA_TLB_GLOBAL_FLUSH);
> +}
> +
>   static void unpreserve_iommu_context_tables(struct intel_iommu *iommu,
>   					    struct iommu_hw_ser *ser)
>   {

This patch looks good to me,

Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>