Re: [PATCH v2 04/16] iommu: Implement device and IOMMU HW preservation

[Samiullah Khawaja]

On Thu, May 07, 2026 at 10:07:43AM +0800, Baolu Lu wrote:
> On 4/28/26 01:56, Samiullah Khawaja wrote:
> > Add IOMMU ops to preserve/unpreserve a device. These can be implemented
> > by the IOMMU drivers that support preservation of devices that have
> > their IOMMU domains preserved. During device preservation the state of
> > the associated IOMMU is also preserved as dependency.
> >
> > Signed-off-by: Samiullah Khawaja<skhawaja@xxxxxxxxxx>
> > ---
> >  drivers/iommu/liveupdate.c       | 162 +++++++++++++++++++++++++++++++
> >  include/linux/iommu-liveupdate.h |  33 +++++++
> >  include/linux/iommu.h            |  20 ++++
> >  3 files changed, 215 insertions(+)

[snip]
> > +
> > +int iommu_preserve_device(struct iommu_domain *domain,
> > +			  struct device *dev, u64 *preserved_state)
> > +{
> > +	struct iommu_flb_obj *flb_obj;
> > +	struct iommu_device_ser *device_ser;
> > +	struct dev_iommu *iommu;
> > +	struct pci_dev *pdev;
> > +	int ret;
> > +
> > +	if (!dev_is_pci(dev))
> > +		return -EOPNOTSUPP;
> > +
> > +	if (!domain->preserved_state)
> > +		return -EINVAL;
> > +
> > +	if (!iommu_group_dma_owner_claimed(dev->iommu_group))
> > +		return -EINVAL;
> > +
> > +	pdev = to_pci_dev(dev);
> > +	iommu = dev->iommu;
> > +	if (!iommu->iommu_dev->ops->preserve_device ||
> > +	    !iommu->iommu_dev->ops->preserve)
> > +		return -EOPNOTSUPP;

I will check for unpreserve ops here also.
> > +
> > +	ret = liveupdate_flb_get_outgoing(&iommu_flb, (void **)&flb_obj);
> > +	if (ret)
> > +		return ret;
> > +

[snip]
> > +}
> > +
> > +void iommu_unpreserve_device(struct iommu_domain *domain, struct device *dev)
> > +{
> > +	struct iommu_flb_obj *flb_obj;
> > +	struct iommu_device_ser *iommu_device_ser;
> > +	struct dev_iommu *iommu;
> > +	struct pci_dev *pdev;
> > +	int ret;
> > +
> > +	if (!dev_is_pci(dev))
> > +		return;
> > +
> > +	if (!iommu_group_dma_owner_claimed(dev->iommu_group))
> > +		return;
> > +
> > +	pdev = to_pci_dev(dev);
> > +	iommu = dev->iommu;
> > +	if (!iommu->iommu_dev->ops->unpreserve_device ||
> > +	    !iommu->iommu_dev->ops->unpreserve)
> > +		return;
>
> Is it considered a driver bug if it implements the preserve hooks but
> not unpreserve ones? This would at least cause a silent memory leak. How
> about adding a WARN like this?

I think the preservation should not be done if the unpreserve ops are
not implemented by the driver. I will add these checks in the _preserve
functions and return EOPNOTSUPP if it is not implemented by the driver.
>    if (WARN_ON_ONCE(!iommu->iommu_dev->ops->unpreserve_device ||
>                     !iommu->iommu_dev->ops->unpreserve))
>        return;

We can add this warning here also.
> ?
>
> Thanks,
> baolu

Thanks,
Sami