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

Pranjal Shrivastava
2026-03-25T20:55:12Z

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