Re: [PATCH 06/14] iommu/vt-d: Implement device and iommu preserve/unpreserve ops

Vipin Sharma
2026-03-19T16:04:37Z

On Tue, Feb 03, 2026 at 10:09:40PM +0000, Samiullah Khawaja wrote:
> /*
> * Take a root_entry and return the Lower Context Table Pointer (LCTP)
> * if marked present.
> @@ -2378,8 +2379,10 @@ 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);
> + if (!__maybe_clean_unpreserved_context_entries(iommu)) {
> + /* Make sure the IOMMUs are switched off */
> + iommu_disable_translation(iommu);
> + }

I think instead of having a "maybe" function where it might or might not
clear the context entry, it will be simpler to just make if-else
explicit here.

@@ -2375,8 +2375,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);
+ if (iommu->iommu.outgoing_preserved_state)
+ clear_unpreserved_context_entries();
+ else
+ /* Make sure the IOMMUs are switched off */
+ iommu_disable_translation(iommu);
}
}

clear_unpreserved_context_entries() will explicitly do what
__maybe_clean_unpreserved_context_entries() is doing.

> }
> }
>
> @@ -2902,6 +2905,38 @@ static const struct iommu_dirty_ops intel_second_stage_dirty_ops = {
> .set_dirty_tracking = intel_iommu_set_dirty_tracking,
> };
>
> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +static bool __maybe_clean_unpreserved_context_entries(struct intel_iommu *iommu)

Nit: s/clean/clear, this matches domain_context_clear().

> +{
> + struct device_domain_info *info;
> + struct pci_dev *pdev = NULL;
> +
> + if (!iommu->iommu.outgoing_preserved_state)
> + return false;
> +
> + for_each_pci_dev(pdev) {
> + info = dev_iommu_priv_get(&pdev->dev);
> + if (!info)
> + continue;
> +
> + if (info->iommu != iommu)
> + continue;
> +
> + if (dev_iommu_preserved_state(&pdev->dev))
> + continue;
> +
> + domain_context_clear(info);
> + }
> +
> + return true;
> +}
> +++ b/drivers/iommu/intel/liveupdate.c
> @@ -0,0 +1,134 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +/*
> + * Copyright (C) 2025, Google LLC
> + * Author: Samiullah Khawaja <skhawaja@google.com>
> + */
> +
> +#define pr_fmt(fmt) "iommu: liveupdate: " fmt

Should this be 'DMAR: liveupdate' to be consistent with other files in
intel iommu?

> +
> +#include <linux/kexec_handover.h>
> +#include <linux/liveupdate.h>
> +#include <linux/iommu-lu.h>
> +#include <linux/module.h>
> +#include <linux/pci.h>
> +
> +#include "iommu.h"
> +#include "../iommu-pages.h"
> +
> +static void unpreserve_iommu_context(struct intel_iommu *iommu, int end)

Nit: Can this be unpreserve_iommu_context_table(), similar to existing
free_context_table()? Same for corresponding preserve function

> +{
> + struct context_entry *context;
> + int i;
> +
> + if (end < 0)
> + end = ROOT_ENTRY_NR;

Can't we just pass ROOT_ENTRY_NR as end value instead of -1?