Re: [PATCH v4 10/18] iommu/vt-d: Restore IOMMU state and reclaimed domain ids

[Samiullah Khawaja]

On Thu, Aug 27, 2026 at 03:22:26PM +0800, Baolu Lu wrote:
> On 8/8/26 10:27, Samiullah Khawaja wrote:
> > During boot fetch the preserved state of IOMMU unit and if found then
> > restore the state.
> >
> > - Reuse the root_table that was preserved in the previous kernel.
> > - Reclaim the domain ids of the preserved domains for each preserved
> >   devices so these are not acquired by another domain.
> >
> > Signed-off-by: Samiullah Khawaja <skhawaja@xxxxxxxxxx>
> > ---
> >  drivers/iommu/intel/iommu.c      | 111 +++++++++++++++++++------------
> >  drivers/iommu/intel/iommu.h      |   7 ++
> >  drivers/iommu/intel/liveupdate.c |  69 +++++++++++++++++++
> >  3 files changed, 144 insertions(+), 43 deletions(-)

[snip]

> > +
> > +static int _restore_used_domain_ids(struct iommu_device_ser *ser, void *arg)
> > +{
> > +	int id = ser->domain_iommu_ser.attachment_id;
> > +	struct iommu_hw_ser *iommu_hw_ser;
> > +	struct intel_iommu *iommu = arg;
> > +
> > +	if (WARN_ON(!ser->domain_iommu_ser.iommu_phys))
> > +		return 0;
> > +
> > +	iommu_hw_ser = phys_to_virt(ser->domain_iommu_ser.iommu_phys);
> > +	if (iommu_hw_ser->type != IOMMU_INTEL)
> > +		return 0;
> > +
> > +	/* Only allocate domain ID from associated IOMMU HW unit */
> > +	if (iommu_hw_ser->intel.phys_addr != iommu->reg_phys)
> > +		return 0;
> > +
> > +	/*
> > +	 * This can fail as multiple preserved devices can share the same domain
> > +	 * ID. Since this is done during DMAR init so these failures can be
> > +	 * ignored.
> > +	 */
> > +	ida_alloc_range(&iommu->domain_ida, id, id, GFP_ATOMIC);
>
> This mixes two different cases:
>
> - another preserved device has already reserved the same DID.
> - the DID is not reserved because ida_alloc_range() failed (for example,
>  memory allocation failure).
>
> Case #1 is expected. Case #2 must not be ignored, because it means
> restore failed. So this should probably be something like (not tested):
>
> 	mutex_lock(&iommu->did_lock);
> 	if (ida_find_first_range(&iommu->domain_ida, id, id) >= 0)
> 		BUG_ON(ida_alloc_range(&iommu->domain_ida, id, id, GFP_KERNEL) < 0)
> 	mutex_unlock(&iommu->did_lock);

This is a good point, I will update this. Actually, I was thinking of
adding a new struct in the preserved state that represents the
association between domain-iommu-device. All the drivers have this, so
it is better to have a representation of this. Let me evaluate that.
> ?
>
> By the way, why GFP_ATOMIC here at all?

Will update as you suggested.
> > +	return 0;
> > +}
> > +
> > +/**
> > + * intel_iommu_liveupdate_restore_root_table() - Restore root table and reclaim domain IDs
> > + * @iommu: Target IOMMU
> > + * @iommu_ser: Serialized IOMMU hardware state from previous kernel
> > + *
> > + * Restores the preserved root table and context tables for the IOMMU hardware
> > + * instance across Live Update, and reclaims all domain IDs previously allocated
> > + * to preserved devices so they are not reused.
> > + */
> > +void intel_iommu_liveupdate_restore_root_table(struct intel_iommu *iommu,
> > +					       struct iommu_hw_ser *iommu_ser)
> > +{
> > +	if (!iommu_ser->intel.restored)
> > +		iommu_restore_pages(iommu_ser->intel.root_table);
> > +
> > +	iommu->root_entry = __va(iommu_ser->intel.root_table);
> > +
> > +	if (!iommu_ser->intel.restored)
> > +		restore_iommu_context(iommu);
> > +
> > +	iommu_ser->intel.restored = 1;
>
> This uses iommu_ser->intel.restored to prevent restoring the root and
> context tables multiple times. For this to work safely, iommu_ser-
> > intel.restored must be 0 the first time restore runs after kexec.
>
> The problem is: this field is inside struct iommu_hw_ser, and that
> struct is allocated in the old kernel. How to ensure that the previous
> kernel has zeroed this out? Maybe I’m overthinking this.

This is a valid point. However, the old kernel allocates this struct
with GFP_ZERO and it never touches the restored field prior to kexec.
So it should be zero in the new kernel.
> > +	BUG_ON(iommu_for_each_preserved_device(_restore_used_domain_ids, iommu));
> > +}
> > +
> >  /**
> >   * intel_iommu_preserve_device() - Intel IOMMU callback to preserve device state
> >   * @dev: Target device
>
> Thanks,
> baolu

Thanks,
Sami