Re: [PATCH 13/14] vfio/pci: Preserve the iommufd state of the vfio cdev

[Pranjal Shrivastava <praan@google.com>]

On Tue, Feb 03, 2026 at 10:09:47PM +0000, Samiullah Khawaja wrote:
> If the vfio cdev is attached to an iommufd, preserve the state of the
> attached iommufd also. Basically preserve the iommu state of the device
> and also the attached domain. The token returned by the preservation API
> will be used to restore/rebind to the iommufd state after liveupdate.
> 
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
> ---
>  drivers/vfio/pci/vfio_pci_liveupdate.c | 28 +++++++++++++++++++++++++-
>  include/linux/kho/abi/vfio_pci.h       | 10 +++++++++
>  2 files changed, 37 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/pci/vfio_pci_liveupdate.c b/drivers/vfio/pci/vfio_pci_liveupdate.c
> index c52d6bdb455f..af6fbfb7a65c 100644
> --- a/drivers/vfio/pci/vfio_pci_liveupdate.c
> +++ b/drivers/vfio/pci/vfio_pci_liveupdate.c
> @@ -15,6 +15,7 @@
>  #include <linux/liveupdate.h>
>  #include <linux/errno.h>
>  #include <linux/vfio.h>
> +#include <linux/iommufd.h>
>  
>  #include "vfio_pci_priv.h"
>  
> @@ -39,6 +40,7 @@ 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;
> +	u64 token = 0;
>  
>  	vdev = container_of(device, struct vfio_pci_core_device, vdev);
>  	pdev = vdev->pdev;
> @@ -49,15 +51,32 @@ static int vfio_pci_liveupdate_preserve(struct liveupdate_file_op_args *args)
>  	if (vfio_pci_is_intel_display(pdev))
>  		return -EINVAL;
>  
> +#if CONFIG_IOMMU_LIVEUPDATE

Did we mean to use #ifdef or #if 

> +	/* If iommufd is attached, preserve the underlying domain */
> +	if (device->iommufd_attached) {
> +		int err = iommufd_device_preserve(args->session,
> +						  device->iommufd_device,
> +						  &token);
> +		if (err < 0)
> +			return err;
> +	}
> +#endif
> +
>  	ser = kho_alloc_preserve(sizeof(*ser));
> -	if (IS_ERR(ser))
> +	if (IS_ERR(ser)) {
> +		if (device->iommufd_attached)
> +			iommufd_device_unpreserve(args->session,
> +						  device->iommufd_device, token);

Few minor things here, we've protected the preserve call above with a
#ifdef but not the unpreserve here in the clean up path. Also, it looks
like in the previous patch we wrap both of these functions in #ifdefs
and define their #else static inline-d versions too?

> +
>  		return PTR_ERR(ser);
> +	}
>  
>  	pci_liveupdate_outgoing_preserve(pdev);
>  
>  	ser->bdf = pci_dev_id(pdev);
>  	ser->domain = pci_domain_nr(pdev->bus);
>  	ser->reset_works = vdev->reset_works;
> +	ser->iommufd_ser.token = token;
>  
>  	args->serialized_data = virt_to_phys(ser);
>  	return 0;
> @@ -66,6 +85,13 @@ static int vfio_pci_liveupdate_preserve(struct liveupdate_file_op_args *args)
>  static void vfio_pci_liveupdate_unpreserve(struct liveupdate_file_op_args *args)
>  {
>  	struct vfio_device *device = vfio_device_from_file(args->file);
> +	struct vfio_pci_core_device_ser *ser;
> +
> +	ser = phys_to_virt(args->serialized_data);
> +	if (device->iommufd_attached)
> +		iommufd_device_unpreserve(args->session,
> +					  device->iommufd_device,
> +					  ser->iommufd_ser.token);
>  
>  	pci_liveupdate_outgoing_unpreserve(to_pci_dev(device->dev));
>  	kho_unpreserve_free(phys_to_virt(args->serialized_data));
> diff --git a/include/linux/kho/abi/vfio_pci.h b/include/linux/kho/abi/vfio_pci.h
> index 6c3d3c6dfc09..d01bd58711c2 100644
> --- a/include/linux/kho/abi/vfio_pci.h
> +++ b/include/linux/kho/abi/vfio_pci.h
> @@ -28,6 +28,15 @@
>  
>  #define VFIO_PCI_LUO_FH_COMPATIBLE "vfio-pci-v1"
>  
> +/**
> + * struct vfio_iommufd_ser - Serialized state of the attached iommufd.
> + *
> + * @token: The token of the bound iommufd state.
> + */
> +struct vfio_iommufd_ser {
> +	u32 token;
> +} __packed;
> +
>  /**
>   * struct vfio_pci_core_device_ser - Serialized state of a single VFIO PCI
>   * device.
> @@ -40,6 +49,7 @@ struct vfio_pci_core_device_ser {
>  	u16 bdf;
>  	u16 domain;
>  	u8 reset_works;
> +	struct vfio_iommufd_ser iommufd_ser;
>  } __packed;
>  

Another struct alignment geekery: when we update vfio_iommufd_ser.token
to __u64 (as pointed out by other comments), putting it right after 
`reset_works` forces an unaligned 8-byte access at offset 5. Ideally, we
should add explicit padding (e.g. __u8 padding[3]) before iommufd_ser to
ensure natural alignment.

Because __packed prevents the compiler from automatically aligning 
fields, placing a 64-bit integer at an odd offset forces the CPU to
perform an unaligned memory read. While some archs handle this with a 
performance penalty, other archs could outright fault on unaligned 64-bit
accesses. Explicit padding guarantees safe, portable access across all
archs while maintaining the strict memory layout required by the KHO ABI.

Thanks,
Praan