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

[Samiullah Khawaja]

On Wed, Oct 28, 2026 at 09:51:18AM +0800, Baolu Lu wrote:
> On 10/25/26 10:19, Samiullah Khawaja wrote:
> > Add implementation of the device and iommu preservation in a separate
> > file. Also set the device and iommu preserve/unpreserve ops in the
> > struct iommu_ops.
> >
> > Preservation is refused for devices where ATS is supported. Carrying an
> > active ATS configuration across kexec would require adopting the ATS
> > state in the PCI core and in the context entries, and handling the
> > disable-to-enable, enable-to-disable and STU mismatch cases for a live
> > device.
> >
> > Note that the preserved iommu root table, context table and device pasid
> > table will need cleanup as devices do not go through the release flow
> > during kexec. So the stale entries need to be removed during shutdown.
> > This is done in the next commit.
> >
> > Signed-off-by: Samiullah Khawaja <skhawaja@xxxxxxxxxx>
> > ---
> >  MAINTAINERS                      |   8 +
> >  drivers/iommu/intel/Makefile     |   1 +
> >  drivers/iommu/intel/iommu.c      |   9 +-
> >  drivers/iommu/intel/iommu.h      |  13 ++
> >  drivers/iommu/intel/liveupdate.c | 246 +++++++++++++++++++++++++++++++
> >  include/linux/kho/abi/iommu.h    |  27 ++++
> >  6 files changed, 302 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/iommu/intel/liveupdate.c

[snip]

> > +
> > +static int preserve_iommu_context_tables(struct device_domain_info *info)
> > +{
> > +	struct iommu_hw_ser *iommu_ser;
> > +	struct intel_iommu *iommu;
> > +	int ret;
> > +	int i;
> > +
> > +	/* IOMMU for this device should already preserved.*/
>
> ... should already be preserved ...

I will fix this in the next revision.
> > +	iommu = info->iommu;
> > +	iommu_ser = iommu_preserved_state(&iommu->iommu);
> > +	if (!iommu_ser)
> > +		return -EINVAL;
> > +
> > +	/*
> > +	 * We could do preservation of context tables only for the bus of this
> > +	 * device, but these devices can have PCI aliases, so context tables for
> > +	 * those will also require preservation. Also unpreserve would require
> > +	 * some kind of refcounting where the context table will only be
> > +	 * unpreserved when the last device associated with it is unpreserved.
> > +	 *
> > +	 * This introduces unnecessary complication with minimum benefits as the
> > +	 * unpreserved context tables will probably be recreated by the next
> > +	 * kernel as these are all active devices. We follow simpler approach by
> > +	 * just preserving the currently active context tables.
> > +	 */
> > +	for (i = 0; i < ROOT_ENTRY_NR; i++) {
> > +		ret = preserve_context_table(iommu, iommu_ser, i, 0);
> > +		if (ret)
> > +			return ret;
> > +
> > +		if (!sm_supported(iommu))
> > +			continue;
> > +
> > +		ret = preserve_context_table(iommu, iommu_ser, i, 0x80);
> > +		if (ret)
> > +			return ret;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +

[snip]

> > +/**
> > + * struct iommu_intel_ser - Serialized state of an Intel IOMMU instance
> > + * @features: Bitmask of Intel IOMMU specific features
> > + * @sm_enabled: Whether the IOMMU was used in scalable mode
> > + * @phys_addr: Physical address of the IOMMU register base
> > + * @root_table: Physical address of the root entry table
> > + * @context_tables_bitmap: Bitmap representing the context tables that are
> > + * preserved.
> > + */
> > +struct iommu_intel_ser {
> > +	u32 features;
> > +	u8 sm_enabled;
> > +	u8 padding[3];
> > +	u64 phys_addr;
> > +	u64 root_table;
> > +	u64 context_tables_bitmap[VTD_PRESERVED_BITMAP_LONGS];
> > +} __packed;
> > +
> >  /**
> >   * struct iommu_hw_ser - Serialized state of an IOMMU instance
> >   * @hdr: Common object header
> >   * @token: Unique token to identify the IOMMU instance
> >   * @type: IOMMU type serialized state belongs to
> >   * @reserved: Reserved for future common fields.
> > + * @intel: Intel specific serialization data
> >   */
> >  struct iommu_hw_ser {
> >  	struct iommu_hdr_ser hdr;
> >  	u64 token;
> >  	u64 type;
> >  	u64 reserved[2];
> > +	union {
> > +		struct iommu_intel_ser intel;
> > +	};
> >  } __packed;
> >  /**
>
> This patch looks good to me,
>
> Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>

Thanks for the review,
Sami