Re: [PATCH v6 16/18] iommufd: Add APIs to preserve/unpreserve a vfio cdev

[Baolu Lu <baolu.lu@linux.intel.com>]

On 10/25/2026 10:19 AM, Samiullah Khawaja wrote:
> Add APIs that can be used to preserve and unpreserve a vfio cdev. Use
> the APIs exported by the IOMMU core to preserve/unpreserve device.
> 
> The LUO token of the preserved iommufd is fetched and returned back to
> the caller as that can be used during restore to get the restored
> iommufd.
> 
> Reviewed-by: Pranjal Shrivastava <praan@google.com>
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>   drivers/iommu/iommufd/device.c          | 156 ++++++++++++++++++++++++
>   drivers/iommu/iommufd/iommufd_private.h |   6 +
>   include/linux/iommufd.h                 |  29 +++++
>   3 files changed, 191 insertions(+)
> 
> diff --git a/drivers/iommu/iommufd/device.c b/drivers/iommu/iommufd/device.c
> index a664c70a6fe7..c8ea2e288240 100644
> --- a/drivers/iommu/iommufd/device.c
> +++ b/drivers/iommu/iommufd/device.c
> @@ -2,6 +2,7 @@
>   /* Copyright (c) 2021-2022, NVIDIA CORPORATION & AFFILIATES
>    */
>   #include <linux/iommu.h>
> +#include <linux/iommu-liveupdate.h>
>   #include <linux/iommufd.h>
>   #include <linux/pci-ats.h>
>   #include <linux/slab.h>
> @@ -686,6 +687,10 @@ int iommufd_hw_pagetable_attach(struct iommufd_hw_pagetable *hwpt,
>   	int rc;
>   
>   	mutex_lock(&igroup->lock);
> +	if (iommufd_device_is_preserved(idev)) {
> +		rc = -EBUSY;
> +		goto err_unlock;
> +	}
>   
>   	attach = xa_cmpxchg(&igroup->pasid_attach, pasid, NULL,
>   			    XA_ZERO_ENTRY, GFP_KERNEL);
> @@ -868,6 +873,10 @@ iommufd_device_do_replace(struct iommufd_device *idev, ioasid_t pasid,
>   	int rc;
>   
>   	mutex_lock(&igroup->lock);
> +	if (iommufd_device_is_preserved(idev)) {
> +		rc = -EBUSY;
> +		goto err_unlock;
> +	}
>   
>   	attach = xa_load(&igroup->pasid_attach, pasid);
>   	if (!attach) {
> @@ -1747,3 +1756,150 @@ int iommufd_get_hw_info(struct iommufd_ucmd *ucmd)
>   	iommufd_put_object(ucmd->ictx, &idev->obj);
>   	return rc;
>   }

Why is there no such check in iommufd_hw_pagetable_detach()?

> +
> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +static bool _iommufd_device_has_pasid_attachments(struct iommufd_device *idev)
> +{
> +	struct iommufd_group *igroup = idev->igroup;
> +	unsigned long start = IOMMU_NO_PASID;
> +
> +	if (xa_find_after(&igroup->pasid_attach, &start, UINT_MAX, XA_PRESENT))
> +		return true;
> +
> +	return false;
> +}
> +
> +/**
> + * iommufd_device_preserve() - Preserve an iommufd device across live update
> + * @s: Live update session
> + * @idev: Target iommufd device
> + * @iommufd_tokenp: Pointer to store outgoing iommufd token
> + *
> + * Return: 0 on success, or negative error code.
> + */
> +int iommufd_device_preserve(struct liveupdate_session *s,
> +			    struct iommufd_device *idev,
> +			    u64 *iommufd_tokenp)
> +{
> +	struct iommufd_hwpt_paging *hwpt_paging;
> +	struct iommufd_hw_pagetable *hwpt;
> +	struct iommufd_attach *attach;
> +	struct iommufd_group *igroup;
> +	int ret;
> +
> +	if (!idev)
> +		return -EINVAL;
> +
> +	if (iommufd_device_is_noiommu(idev))
> +		return -EOPNOTSUPP;
> +
> +	igroup = idev->igroup;
> +	mutex_lock(&igroup->lock);
> +	if (idev->liveupdate_preserved) {
> +		ret = -EBUSY;
> +		goto out;
> +	}
> +
> +	if (_iommufd_device_has_pasid_attachments(idev)) {
> +		ret = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	attach = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
> +	if (!attach) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	if (!xa_load(&attach->device_array, idev->obj.id)) {
> +		ret = -ENOENT;
> +		goto out;
> +	}
> +
> +	hwpt = attach->hwpt;
> +	if (!hwpt_is_paging(hwpt)) {
> +		ret = -EOPNOTSUPP;
> +		goto out;
> +	}
> +
> +	hwpt_paging = to_hwpt_paging(hwpt);
> +	ret = liveupdate_get_token_outgoing(s, idev->ictx->file, iommufd_tokenp);
> +	if (ret)
> +		goto out;
> +
> +	if (!hwpt_paging->liveupdate_preserved) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	ret = iommu_preserve_device(hwpt_paging->common.domain,
> +				    idev->dev,
> +				    *iommufd_tokenp);
> +
> +	if (!ret) {
> +		igroup->num_liveupdate_preserved++;
> +		idev->liveupdate_preserved = true;
> +	}
> +out:
> +	mutex_unlock(&igroup->lock);
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(iommufd_device_preserve, "IOMMUFD");
> +
> +/**
> + * iommufd_device_unpreserve() - Unpreserve an iommufd device
> + * @s: Live update session
> + * @idev: Target iommufd device
> + */
> +void iommufd_device_unpreserve(struct liveupdate_session *s,
> +			       struct iommufd_device *idev)
> +{
> +	struct iommufd_hwpt_paging *hwpt_paging;
> +	struct iommufd_hw_pagetable *hwpt;
> +	struct iommufd_attach *attach;
> +	struct iommufd_group *igroup;
> +
> +	if (!idev)
> +		return;
> +
> +	igroup = idev->igroup;
> +	mutex_lock(&igroup->lock);
> +	if (!idev->liveupdate_preserved)
> +		goto out;
> +
> +	attach = xa_load(&igroup->pasid_attach, IOMMU_NO_PASID);
> +	if (!attach) {
> +		WARN(1, "IOMMU_NO_PASID attachment not found");
> +		goto out;
> +	}
> +
> +	hwpt = attach->hwpt;
> +	if (WARN(!hwpt_is_paging(hwpt), "Attached HWPT is not a paging HWPT"))
> +		goto out;
> +
> +	hwpt_paging = to_hwpt_paging(hwpt);
> +	if (!hwpt_paging->liveupdate_preserved) {
> +		WARN(1, "Attached domain is not preserved");
> +		goto out;
> +	}
> +
> +	iommu_unpreserve_device(hwpt_paging->common.domain, idev->dev);
> +	igroup->num_liveupdate_preserved--;
> +	idev->liveupdate_preserved = false;
> +out:
> +	mutex_unlock(&igroup->lock);
> +}
> +EXPORT_SYMBOL_NS_GPL(iommufd_device_unpreserve, "IOMMUFD");
> +
> +/**
> + * iommufd_device_is_preserved() - Check if an iommufd device is preserved
> + * @idev: Target iommufd device
> + *
> + * Return: true if preserved, false otherwise.
> + */
> +bool iommufd_device_is_preserved(struct iommufd_device *idev)
> +{
> +	return idev && idev->igroup && idev->igroup->num_liveupdate_preserved;
> +}

How is it different from

	return idev && idev->liveupdate_preserved;

?

Device preservation requires a singleton group, so these two are
effectively the same. However, the latter seems more straightforward
given the function's name. Or anything I overlooked?

> +EXPORT_SYMBOL_NS_GPL(iommufd_device_is_preserved, "IOMMUFD");
> +#endif
> diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> index 69d902b69b82..4760fffd2512 100644
> --- a/drivers/iommu/iommufd/iommufd_private.h
> +++ b/drivers/iommu/iommufd/iommufd_private.h
> @@ -507,6 +507,9 @@ struct iommufd_group {
>   	struct xarray pasid_attach;
>   	struct iommufd_sw_msi_maps required_sw_msi;
>   	phys_addr_t sw_msi_start;
> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +	int num_liveupdate_preserved;
> +#endif
>   };
>   
>   /*
> @@ -524,6 +527,9 @@ struct iommufd_device {
>   	bool enforce_cache_coherency;
>   	struct iommufd_vdevice *vdev;
>   	bool destroying;
> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +	bool liveupdate_preserved;
> +#endif
>   };
>   
>   static inline struct iommufd_device *
> diff --git a/include/linux/iommufd.h b/include/linux/iommufd.h
> index 6e7efe83bc5d..80382aceca18 100644
> --- a/include/linux/iommufd.h
> +++ b/include/linux/iommufd.h
> @@ -9,6 +9,7 @@
>   #include <linux/err.h>
>   #include <linux/errno.h>
>   #include <linux/iommu.h>
> +#include <linux/liveupdate.h>
>   #include <linux/refcount.h>
>   #include <linux/types.h>
>   #include <linux/xarray.h>
> @@ -213,6 +214,15 @@ int iommufd_access_rw(struct iommufd_access *access, unsigned long iova,
>   int iommufd_vfio_compat_ioas_get_id(struct iommufd_ctx *ictx, u32 *out_ioas_id);
>   int iommufd_vfio_compat_ioas_create(struct iommufd_ctx *ictx);
>   int iommufd_vfio_compat_set_no_iommu(struct iommufd_ctx *ictx);
> +
> +#ifdef CONFIG_IOMMU_LIVEUPDATE
> +int iommufd_device_preserve(struct liveupdate_session *s,
> +			    struct iommufd_device *idev,
> +			    u64 *iommufd_tokenp);
> +void iommufd_device_unpreserve(struct liveupdate_session *s,
> +			       struct iommufd_device *idev);
> +bool iommufd_device_is_preserved(struct iommufd_device *idev);
> +#endif
>   #else /* !CONFIG_IOMMUFD */
>   static inline struct iommufd_ctx *iommufd_ctx_from_file(struct file *file)
>   {
> @@ -397,4 +407,23 @@ static inline void iommufd_viommu_destroy_mmap(struct iommufd_viommu *viommu,
>   {
>   	_iommufd_destroy_mmap(viommu->ictx, &viommu->obj, offset);
>   }
> +
> +#if !IS_ENABLED(CONFIG_IOMMU_LIVEUPDATE) || !IS_ENABLED(CONFIG_IOMMUFD)
> +static inline int iommufd_device_preserve(struct liveupdate_session *s,
> +					  struct iommufd_device *idev,
> +					  u64 *iommufd_tokenp)
> +{
> +	return 0;

Why does it return success when IOMMU_LIVEUPDATE is disabled?

> +}
> +
> +static inline void iommufd_device_unpreserve(struct liveupdate_session *s,
> +					     struct iommufd_device *idev)
> +{
> +}
> +
> +static inline bool iommufd_device_is_preserved(struct iommufd_device *idev)
> +{
> +	return false;
> +}
> +#endif
>   #endif

Thanks,
baolu