| Message ID | 20230426145419.450922-6-yi.l.liu@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Enhance vfio PCI hot reset for vfio cdev device | expand |
> From: Liu, Yi L <yi.l.liu@intel.com> > Sent: Wednesday, April 26, 2023 10:54 PM > > Use it to differentiate whether to report group_id or dev_id in revised > VFIO_DEVICE_GET_PCI_HOT_RESET_INFO ioctl. Though it is not set at this > moment introducing it now allows us to get hot reset ready for cdev. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> Reviewed-by: Kevin Tian <kevin.tian@intel.com>
On Wed, 26 Apr 2023 07:54:15 -0700 Yi Liu <yi.l.liu@intel.com> wrote: > Use it to differentiate whether to report group_id or dev_id in revised > VFIO_DEVICE_GET_PCI_HOT_RESET_INFO ioctl. Though it is not set at this > moment introducing it now allows us to get hot reset ready for cdev. > > Signed-off-by: Yi Liu <yi.l.liu@intel.com> > --- > include/linux/vfio.h | 7 +++++++ > 1 file changed, 7 insertions(+) > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > index 4ee613924435..298f4ef16be7 100644 > --- a/include/linux/vfio.h > +++ b/include/linux/vfio.h > @@ -63,6 +63,7 @@ struct vfio_device { > bool iommufd_attached; > #endif > bool noiommu; > + bool cdev_opened; > }; > > /** > @@ -140,6 +141,12 @@ int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); > ((int (*)(struct vfio_device *vdev, u32 *pt_id)) NULL) > #endif > > +static inline bool vfio_device_cdev_opened(struct vfio_device *device) > +{ > + lockdep_assert_held(&device->dev_set->lock); > + return device->cdev_opened; > +} The lockdep test doesn't make much sense here, the method of opening the device can't change from an ioctl called on the device, which is the only path using this. If there needs to be a placeholder for future code, it should probably statically return false here and we can save adding the structure field and locking checks based on the semantics of how the field is actually used later. Thanks, Alex
On 2023/4/28 02:43, Alex Williamson wrote: > On Wed, 26 Apr 2023 07:54:15 -0700 > Yi Liu <yi.l.liu@intel.com> wrote: > >> Use it to differentiate whether to report group_id or dev_id in revised >> VFIO_DEVICE_GET_PCI_HOT_RESET_INFO ioctl. Though it is not set at this >> moment introducing it now allows us to get hot reset ready for cdev. >> >> Signed-off-by: Yi Liu <yi.l.liu@intel.com> >> --- >> include/linux/vfio.h | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/include/linux/vfio.h b/include/linux/vfio.h >> index 4ee613924435..298f4ef16be7 100644 >> --- a/include/linux/vfio.h >> +++ b/include/linux/vfio.h >> @@ -63,6 +63,7 @@ struct vfio_device { >> bool iommufd_attached; >> #endif >> bool noiommu; >> + bool cdev_opened; >> }; >> >> /** >> @@ -140,6 +141,12 @@ int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); >> ((int (*)(struct vfio_device *vdev, u32 *pt_id)) NULL) >> #endif >> >> +static inline bool vfio_device_cdev_opened(struct vfio_device *device) >> +{ >> + lockdep_assert_held(&device->dev_set->lock); >> + return device->cdev_opened; >> +} > > The lockdep test doesn't make much sense here, the method of opening > the device can't change from an ioctl called on the device, which is > the only path using this. yes, it is only used by the ioctl paths so far. And this flag should be static in other user-space trigger-able paths with device fd, like the read/write/mmap. Do you think a comment would help to avoid abuse in other paths or the name of it implies it should be used after a device is opened? > If there needs to be a placeholder for > future code, it should probably statically return false here and we can > save adding the structure field and locking checks based on the > semantics of how the field is actually used later. Thanks, Thanks for this suggestion. I was also wondering how to handle it when Eric questioned better to add cdev_opened when it is really used.[1] [1] https://lore.kernel.org/kvm/DS0PR11MB752914607E1DC9CFBFC2EF7AC3609@DS0PR11MB7529.namprd11.prod.outlook.com/
diff --git a/include/linux/vfio.h b/include/linux/vfio.h index 4ee613924435..298f4ef16be7 100644 --- a/include/linux/vfio.h +++ b/include/linux/vfio.h @@ -63,6 +63,7 @@ struct vfio_device { bool iommufd_attached; #endif bool noiommu; + bool cdev_opened; }; /** @@ -140,6 +141,12 @@ int vfio_iommufd_emulated_attach_ioas(struct vfio_device *vdev, u32 *pt_id); ((int (*)(struct vfio_device *vdev, u32 *pt_id)) NULL) #endif +static inline bool vfio_device_cdev_opened(struct vfio_device *device) +{ + lockdep_assert_held(&device->dev_set->lock); + return device->cdev_opened; +} + /** * struct vfio_migration_ops - VFIO bus device driver migration callbacks *
Use it to differentiate whether to report group_id or dev_id in revised VFIO_DEVICE_GET_PCI_HOT_RESET_INFO ioctl. Though it is not set at this moment introducing it now allows us to get hot reset ready for cdev. Signed-off-by: Yi Liu <yi.l.liu@intel.com> --- include/linux/vfio.h | 7 +++++++ 1 file changed, 7 insertions(+)