Re: [PATCH v6 17/18] vfio/pci: Preserve the iommufd state of the vfio cdev

[Baolu Lu]

On 10/25/2026 10:19 AM, Samiullah Khawaja wrote:
> If the vfio cdev is attached to an iommufd, preserve the state of the
> attached iommufd also. Basically preserve the iommu specific state of
> the device and also the attach iommu HW unit.
>
> Once the device and its iommufd attachment is preserved, it cannot be
> detached or attached to another IOAS until it is unpreserved.
>
> Reviewed-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> Signed-off-by: Samiullah Khawaja <skhawaja@xxxxxxxxxx>
> ---
>   drivers/vfio/device_cdev.c             | 10 +++++++++
>   drivers/vfio/pci/vfio_pci_liveupdate.c | 28 ++++++++++++++++++++++++--
>   2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/device_cdev.c b/drivers/vfio/device_cdev.c
> index b818aab92f76..4eb7ffd489bd 100644
> --- a/drivers/vfio/device_cdev.c
> +++ b/drivers/vfio/device_cdev.c
> @@ -284,6 +284,11 @@ int vfio_df_ioctl_attach_pt(struct vfio_device_file *df,
>   	}
>   
>   	mutex_lock(&device->dev_set->lock);
> +	if (iommufd_device_is_preserved(device->iommufd_device)) {
> +		ret = -EBUSY;
> +		goto out_unlock;
> +	}
> +
>   	if (attach.flags & VFIO_DEVICE_ATTACH_PASID)
>   		ret = device->ops->pasid_attach_ioas(device,
>   						     attach.pasid,
> @@ -342,6 +347,11 @@ int vfio_df_ioctl_detach_pt(struct vfio_device_file *df,
>   	}
>   
>   	mutex_lock(&device->dev_set->lock);
> +	if (iommufd_device_is_preserved(device->iommufd_device)) {
> +		mutex_unlock(&device->dev_set->lock);
> +		return -EBUSY;
> +	}
> +
>   	if (detach.flags & VFIO_DEVICE_DETACH_PASID)
>   		device->ops->pasid_detach_ioas(device, detach.pasid);
>   	else
> diff --git a/drivers/vfio/pci/vfio_pci_liveupdate.c b/drivers/vfio/pci/vfio_pci_liveupdate.c
> index f0ea37d98696..fb2141ee8942 100644
> --- a/drivers/vfio/pci/vfio_pci_liveupdate.c
> +++ b/drivers/vfio/pci/vfio_pci_liveupdate.c
> @@ -106,6 +106,7 @@
>   
>   #include <linux/errno.h>
>   #include <linux/file.h>
> +#include <linux/iommufd.h>
>   #include <linux/kexec_handover.h>
>   #include <linux/kho/abi/vfio_pci.h>
>   #include <linux/liveupdate.h>
> @@ -114,6 +115,8 @@
>   
>   #include "vfio_pci_priv.h"
>   
> +MODULE_IMPORT_NS("IOMMUFD");
> +
>   static bool vfio_pci_liveupdate_can_preserve(struct liveupdate_file_handler *handler,
>   					     struct file *file)
>   {
> @@ -154,15 +157,25 @@ static int vfio_pci_liveupdate_preserve(struct liveupdate_file_op_args *args)
>   	struct vfio_pci_core_device_ser *ser;
>   	struct vfio_pci_core_device *vdev;
>   	struct pci_dev *pdev;
> -	int ret;
> +	u64 iommufd_token;
> +	int ret = 0;

nit: no need to initialize ret.

>   
>   	vdev = container_of(device, struct vfio_pci_core_device, vdev);
>   	pdev = vdev->pdev;
>   
> -	ret = pci_liveupdate_preserve(pdev);
> +	mutex_lock(&device->dev_set->lock);
> +	ret = iommufd_device_preserve(args->session,
> +				      device->iommufd_device,
> +				      &iommufd_token);
> +	mutex_unlock(&device->dev_set->lock);
> +
>   	if (ret)
>   		return ret;
>   
> +	ret = pci_liveupdate_preserve(pdev);
> +	if (ret)
> +		goto err_iommufd_unpreserve;
> +
>   	ser = kho_alloc_preserve(sizeof(*ser));
>   	if (IS_ERR(ser)) {
>   		ret = PTR_ERR(ser);
> @@ -177,6 +190,12 @@ static int vfio_pci_liveupdate_preserve(struct liveupdate_file_op_args *args)
>   
>   err_unpreserve:
>   	pci_liveupdate_unpreserve(pdev);
> +
> +err_iommufd_unpreserve:
> +	mutex_lock(&device->dev_set->lock);
> +	iommufd_device_unpreserve(args->session,
> +				  device->iommufd_device);
> +	mutex_unlock(&device->dev_set->lock);
>   	return ret;
>   }
>   
> @@ -184,6 +203,11 @@ static void vfio_pci_liveupdate_unpreserve(struct liveupdate_file_op_args *args)
>   {
>   	struct vfio_device *device = vfio_device_from_file(args->file);
>   
> +	mutex_lock(&device->dev_set->lock);
> +	iommufd_device_unpreserve(args->session,
> +				  device->iommufd_device);
> +	mutex_unlock(&device->dev_set->lock);
> +
>   	pci_liveupdate_unpreserve(to_pci_dev(device->dev));
>   	kho_unpreserve_free(phys_to_virt(args->serialized_data));
>   }

Unpreservation isn't the exact reverse of preservation. Anything special
here?

Otherwise, this patch looks good to me,

Reviewed-by: Lu Baolu <baolu.lu@xxxxxxxxxxxxxxx>