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

[Samiullah Khawaja <skhawaja@google.com>]

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@google.com>
>>---
>>  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