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

[Samiullah Khawaja <skhawaja@google.com>]

On Sat, Oct 31, 2026 at 01:56:12PM +0800, Baolu Lu wrote:
>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@google.com>
>>Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>>---
>>  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.

I will fix this.
>
>>  	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?

Nothing special. I will make it the exact reverse of preserve.

>
>Otherwise, this patch looks good to me,
>
>Reviewed-by: Lu Baolu <baolu.lu@linux.intel.com>

Thanks for the review,
Sami