On 2026-02-25 03:41 PM, Alex Williamson wrote:
> On Thu, 29 Jan 2026 21:24:52 +0000 David Matlack <[email protected]> wrote:

> >  static bool vfio_pci_liveupdate_can_preserve(struct 
> > liveupdate_file_handler *handler,
> >                                          struct file *file)
> >  {
> > -   return false;
> > +   struct vfio_device_file *df = to_vfio_device_file(file);
> > +
> > +   if (!df)
> > +           return false;
> > +
> > +   /* Live Update support is limited to cdev files. */
> > +   if (df->group)
> > +           return false;
> > +
> > +   return df->device->ops == &vfio_pci_ops;
> >  }
> 
> Why can't we use vfio_device_cdev_opened() here and avoid all the new
> exposure in public headers?

I thought I explored using vfio_device_cdev_opened() but I can't
remember now why I went with df->group. Maybe there wasn't a good
reason. I'll switch to vfio_device_cdev_opened() in the next version.

> >  
> >  static int vfio_pci_liveupdate_preserve(struct liveupdate_file_op_args 
> > *args)
> >  {
> > -   return -EOPNOTSUPP;
> > +   struct vfio_device *device = vfio_device_from_file(args->file);
> > +   struct vfio_pci_core_device_ser *ser;
> > +   struct vfio_pci_core_device *vdev;
> > +   struct pci_dev *pdev;
> > +
> > +   vdev = container_of(device, struct vfio_pci_core_device, vdev);
> > +   pdev = vdev->pdev;
> > +
> > +   if (IS_ENABLED(CONFIG_VFIO_PCI_ZDEV_KVM))
> > +           return -EINVAL;
> > +
> > +   if (vfio_pci_is_intel_display(pdev))
> > +           return -EINVAL;
> 
> Some comments describing what's missing, if these are TODO or DONTCARE
> would be useful.

Will do.

> > +static int vfio_pci_liveupdate_freeze(struct liveupdate_file_op_args *args)
> > +{
> > +   struct vfio_device *device = vfio_device_from_file(args->file);
> > +   struct vfio_pci_core_device *vdev;
> > +   struct pci_dev *pdev;
> > +   int ret;
> > +
> > +   vdev = container_of(device, struct vfio_pci_core_device, vdev);
> > +   pdev = vdev->pdev;
> > +
> > +   guard(mutex)(&device->dev_set->lock);
> > +
> > +   /*
> > +    * Userspace must disable interrupts on the device prior to freeze so
> > +    * that the device does not send any interrupts until new interrupt
> > +    * handlers have been established by the next kernel.
> > +    */
> > +   if (vdev->irq_type != VFIO_PCI_NUM_IRQS) {
> > +           pci_err(pdev, "Freeze failed! Interrupts are still enabled.\n");
> > +           return -EINVAL;
> > +   }
> > +
> > +   pci_dev_lock(pdev);
> 
> device_lock() is a dangerous source of deadlocks, for instance how can
> we know the freeze isn't occurring with an outstanding driver unbind?

I can change this to a try-lock and return an error if taking the lock
fails. The freeze() callbacks are triggered by liveupdate_reboot() which
is called from kernel_kexec(). So returning an error to userspace is
possible.

My only concern is whether using try-lock would make kexec flaky, or if
it would only fail if userspace is misbehavior (e.g. unbinding drivers
while kexecing).

> > -static struct vfio_device *vfio_device_from_file(struct file *file)
> > -{
> > -   struct vfio_device_file *df = file->private_data;
> > -
> > -   if (file->f_op != &vfio_device_fops)
> > -           return NULL;
> > -   return df->device;
> > -}
> > +EXPORT_SYMBOL_GPL(vfio_device_fops);
> 
> Seems we just need to export vfio_device_from_file().  Thanks,

Will do.

Reply via email to