Re: [PATCH v4 11/18] iommu: Restore and reattach preserved domains to devices

On Fri, Aug 14, 2026 at 07:46:01PM +0000, Samiullah Khawaja wrote:
> On Fri, Aug 14, 2026 at 04:59:50PM +0000, Ankit Soni wrote:
> > On Sat, Aug 08, 2026 at 02:27:16AM +0000, Samiullah Khawaja wrote:
> > > During default domain setup, restore the preserved domains by restoring
> > > the page tables using restore() iommupt op. Associated the restored
> > > domain with the iommu group of the preserved device, and reattach the
> > > domain to the device.
> > > 
> > > Signed-off-by: Samiullah Khawaja <[email protected]>
> > > ---
> > >  drivers/iommu/iommu.c            |  76 ++++++++++++++++++
> > >  drivers/iommu/liveupdate.c       | 130 +++++++++++++++++++++++++++++++
> > >  include/linux/iommu-liveupdate.h |  69 ++++++++++++++++
> > >  3 files changed, 275 insertions(+)
> > > 
> > 
> > ../..
> > 
> > > diff --git a/drivers/iommu/liveupdate.c b/drivers/iommu/liveupdate.c
> > > index 20acf123b47a..04c0212cd81b 100644
> > > --- a/drivers/iommu/liveupdate.c
> > > +++ b/drivers/iommu/liveupdate.c
> > > @@ -708,3 +708,133 @@ void iommu_unpreserve_device(struct iommu_domain *domain, struct device *dev)
> > >  	liveupdate_flb_put_outgoing(&iommu_flb);
> > >  }
> > >  EXPORT_SYMBOL_GPL(iommu_unpreserve_device);
> > > +
> > > +static inline bool match_device_ser(struct iommu_device_ser *match,
> > > +				    struct pci_dev *pdev)
> > > +{
> > > +	return match->devid == pci_dev_id(pdev) && match->pci_domain_nr == pci_domain_nr(pdev->bus);
> > > +}
> > > +
> > > +/**
> > > + * iommu_init_device_preserved_data() - Initialize preserved state for device
> > > + * @dev: Target device
> > > + *
> > > + * Looks up incoming Live Update state for @dev and attaches it to the device if
> > > + * found.
> > > + */
> > > +void iommu_init_device_preserved_data(struct device *dev)
> > > +{
> > > +	struct iommu_device_ser *device_ser = NULL;
> > > +	struct iommu_device_array_ser *array;
> > > +	struct iommu_flb_obj *flb_obj;
> > > +	int ret, idx;
> > > +
> > > +	if (!dev_is_pci(dev))
> > > +		return;
> > > +
> > > +	ret = iommu_liveupdate_flb_get_incoming(&flb_obj);
> > > +	if (ret)
> > > +		return;
> > > +
> > > +	mutex_lock(&flb_obj->lock);
> > > +	array = phys_to_virt(flb_obj->ser->device_array_phys);
> > > +	iommu_liveupdate_for_each_arr(array) {
> > > +		iommu_liveupdate_for_each_obj(array, device_ser, idx) {
> > > +			if (match_device_ser(device_ser, to_pci_dev(dev))) {
> > > +				device_ser->hdr.flags |= IOMMU_SER_FLAG_INCOMING;
> > > +				goto out;
> > > +			}
> > > +		}
> > > +	}
> > > +
> > > +	device_ser = NULL;
> > > +out:
> > > +	WRITE_ONCE(dev->iommu->device_ser, device_ser);
> > > +	mutex_unlock(&flb_obj->lock);
> > > +	liveupdate_flb_put_incoming(&iommu_flb);
> > > +}
> > > +EXPORT_SYMBOL(iommu_init_device_preserved_data);
> > > +
> > > +/**
> > > + * iommu_release_restored_device() - Release a restored device
> > > + * @dev: Target device
> > > + */
> > > +void iommu_release_restored_device(struct device *dev)
> > > +{
> > > +	/*
> > > +	 * We do not support releasing the restored devices that are not
> > > +	 * reclaimed by the device drivers as they can fallback to the default
> > > +	 * domain.
> > > +	 */
> > > +	BUG_ON(dev_iommu_restored_state(dev));
> > 
> > Hi,
> 
> Hi,
> 
> Thanks for looking at this.
> 
> > After a successful live update this is one sysfs write away, and nothing in
> > the series disarms it.
> > 
> > At PCI probe, iommu_init_device_preserved_data() matches the incoming FLB on
> > devid + pci_domain_nr and sets IOMMU_SER_FLAG_INCOMING.
> > Nothing clears the flag or device_ser afterwards. The group is meanwhile owned
> > on behalf of iommufd (iommu.c:3229-3231, "will be reclaimed later by the
> > entity (iommufd) that preserved them"), and iommufd_liveupdate_retrieve() is
> > -EOPNOTSUPP, so the reclaim that would end the restored state cannot happen
> > yet. The device is left with the state permanently set.
> 
> Yes, these points are valid and this is intentional. The IOMMU
> persistence support is split into two phases as mentioned in the cover
> letter. The reclaim logic in iommufd will come later as a phase 2. The
> preserved devices go to normal state when these are reclaimed through
> iommufd.
> 
> https://lore.kernel.org/all/[email protected]/

This is pretty much clear, and I wasn't asking for reclaim here -- I described
the parked state because it is where the BUG_ON is reached. It looks
independent of the reclaim design: phase-1 code enforces the phase-1 boundary 
with an unguarded BUG_ON, on a path a plain sysfs remove reaches.

> 
> Regarding the handling of sysfs, my concern is about it coming back and
> going to default domain as mentioned in the comment. But I will evaluate
> if we can allow device tear down here and reattach it to the preserved
> domain.

Agree, that teardown is the right direction. The removal can't be refused there
in any case: it arrives on a void notifier under device_del(), so the BUG_ON is
the whole of "we do not support this" -- taken with pci_rescan_remove_lock held.

It also can't distinguish the unreclaimed case the inline comment describes: the
only caller already gates on dev_iommu_restored_state(dev), so it panics every
time it is reached. Happy to leave the exact shape of the teardown to you.


> 
> Also please see the patchset breakdown here:
> https://docs.google.com/document/d/1enDn-uPE9U77U-xHEnzn6HHGKiePSAtMIP8EDU3NO0M
> 
> Btw since this phase 1 is relatively stable and I don't expect many
> changes in it. I have started reviving the Phase 2 patches and will be
> sending them out as RFC soon if you want to experiment with it.
> 
> Sami
> 

Please do send it, and Cc me. Reclaim is the part I most want to see.

-Ankit