> From: Eric Auger <eric.auger@xxxxxxxxxx> > Sent: Wednesday, April 5, 2023 4:28 PM > > Hi Yi, > On 4/1/23 16:44, Yi Liu wrote: > > This prepares vfio core to accept vfio device file from the vfio PCI > > hot reset path. vfio_file_is_group() is still kept for KVM usage. > > > > Reviewed-by: Kevin Tian <kevin.tian@xxxxxxxxx> > > Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx> > > Tested-by: Yanting Jiang <yanting.jiang@xxxxxxxxx> > > Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx> > > --- > > drivers/vfio/group.c | 32 ++++++++++++++------------------ > > drivers/vfio/pci/vfio_pci_core.c | 4 ++-- > > drivers/vfio/vfio.h | 2 ++ > > drivers/vfio/vfio_main.c | 29 +++++++++++++++++++++++++++++ > > include/linux/vfio.h | 1 + > > 5 files changed, 48 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c > > index 27d5ba7cf9dc..d0c95d033605 100644 > > --- a/drivers/vfio/group.c > > +++ b/drivers/vfio/group.c > > @@ -745,6 +745,15 @@ bool vfio_device_has_container(struct vfio_device *device) > > return device->group->container; > > } > > > > +struct vfio_group *vfio_group_from_file(struct file *file) > > +{ > > + struct vfio_group *group = file->private_data; > > + > > + if (file->f_op != &vfio_group_fops) > > + return NULL; > > + return group; > > +} > > + > > /** > > * vfio_file_iommu_group - Return the struct iommu_group for the vfio group file > > * @file: VFIO group file > > @@ -755,13 +764,13 @@ bool vfio_device_has_container(struct vfio_device > *device) > > */ > > struct iommu_group *vfio_file_iommu_group(struct file *file) > > { > > - struct vfio_group *group = file->private_data; > > + struct vfio_group *group = vfio_group_from_file(file); > > struct iommu_group *iommu_group = NULL; > > > > if (!IS_ENABLED(CONFIG_SPAPR_TCE_IOMMU)) > > return NULL; > > > > - if (!vfio_file_is_group(file)) > > + if (!group) > > return NULL; > > > > mutex_lock(&group->group_lock); > > @@ -775,12 +784,12 @@ struct iommu_group *vfio_file_iommu_group(struct file > *file) > > EXPORT_SYMBOL_GPL(vfio_file_iommu_group); > > > > /** > > - * vfio_file_is_group - True if the file is usable with VFIO aPIS > > + * vfio_file_is_group - True if the file is a vfio group file > > * @file: VFIO group file > > */ > > bool vfio_file_is_group(struct file *file) > > { > > - return file->f_op == &vfio_group_fops; > > + return vfio_group_from_file(file); > > } > > EXPORT_SYMBOL_GPL(vfio_file_is_group); > > > > @@ -842,23 +851,10 @@ void vfio_file_set_kvm(struct file *file, struct kvm *kvm) > > } > > EXPORT_SYMBOL_GPL(vfio_file_set_kvm); > > > > -/** > > - * vfio_file_has_dev - True if the VFIO file is a handle for device > > - * @file: VFIO file to check > > - * @device: Device that must be part of the file > > - * > > - * Returns true if given file has permission to manipulate the given device. > > - */ > > -bool vfio_file_has_dev(struct file *file, struct vfio_device *device) > > +bool vfio_group_has_dev(struct vfio_group *group, struct vfio_device *device) > > { > > - struct vfio_group *group = file->private_data; > > - > > - if (!vfio_file_is_group(file)) > > - return false; > > - > > return group == device->group; > > } > > -EXPORT_SYMBOL_GPL(vfio_file_has_dev); > > > > static char *vfio_devnode(const struct device *dev, umode_t *mode) > > { > > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c > > index b68fcba67a4b..2a510b71edcb 100644 > > --- a/drivers/vfio/pci/vfio_pci_core.c > > +++ b/drivers/vfio/pci/vfio_pci_core.c > > @@ -1308,8 +1308,8 @@ vfio_pci_ioctl_pci_hot_reset_groups(struct > vfio_pci_core_device *vdev, > > break; > > } > > > > - /* Ensure the FD is a vfio group FD.*/ > > - if (!vfio_file_is_group(file)) { > > + /* Ensure the FD is a vfio FD. vfio group or vfio device */ > it is a bit strange to update the comment here and in the other places > in this patch whereas file_is_valid still sticks to group file check > By the way I would simply remove the comment which does not bring much ok. yeah, at this moment, it's still group file. may just delete this comment. > > + if (!vfio_file_is_valid(file)) { > > fput(file); > > ret = -EINVAL; > > break; > > diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h > > index 7b19c621e0e6..c0aeea24fbd6 100644 > > --- a/drivers/vfio/vfio.h > > +++ b/drivers/vfio/vfio.h > > @@ -84,6 +84,8 @@ void vfio_device_group_unregister(struct vfio_device *device); > > int vfio_device_group_use_iommu(struct vfio_device *device); > > void vfio_device_group_unuse_iommu(struct vfio_device *device); > > void vfio_device_group_close(struct vfio_device *device); > > +struct vfio_group *vfio_group_from_file(struct file *file); > > +bool vfio_group_has_dev(struct vfio_group *group, struct vfio_device *device); > > bool vfio_device_has_container(struct vfio_device *device); > > int __init vfio_group_init(void); > > void vfio_group_cleanup(void); > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > > index 89497c933490..fe7446805afd 100644 > > --- a/drivers/vfio/vfio_main.c > > +++ b/drivers/vfio/vfio_main.c > > @@ -1154,6 +1154,35 @@ const struct file_operations vfio_device_fops = { > > .mmap = vfio_device_fops_mmap, > > }; > > > > +/** > > + * vfio_file_is_valid - True if the file is valid vfio file > > + * @file: VFIO group file or VFIO device file > I wonder if you shouldn't squash with next patch tbh. yes. this is still group file, no device file yet. Thanks, Yi Liu > > + */ > > +bool vfio_file_is_valid(struct file *file) > > +{ > > + return vfio_group_from_file(file); > > +} > > +EXPORT_SYMBOL_GPL(vfio_file_is_valid); > > + > > +/** > > + * vfio_file_has_dev - True if the VFIO file is a handle for device > > + * @file: VFIO file to check > > + * @device: Device that must be part of the file > > + * > > + * Returns true if given file has permission to manipulate the given device. > > + */ > > +bool vfio_file_has_dev(struct file *file, struct vfio_device *device) > > +{ > > + struct vfio_group *group; > > + > > + group = vfio_group_from_file(file); > > + if (!group) > > + return false; > > + > > + return vfio_group_has_dev(group, device); > > +} > > +EXPORT_SYMBOL_GPL(vfio_file_has_dev); > > + > > /* > > * Sub-module support > > */ > > diff --git a/include/linux/vfio.h b/include/linux/vfio.h > > index 97a1174b922f..f8fb9ab25188 100644 > > --- a/include/linux/vfio.h > > +++ b/include/linux/vfio.h > > @@ -258,6 +258,7 @@ int vfio_mig_get_next_state(struct vfio_device *device, > > */ > > struct iommu_group *vfio_file_iommu_group(struct file *file); > > bool vfio_file_is_group(struct file *file); > > +bool vfio_file_is_valid(struct file *file); > > bool vfio_file_enforced_coherent(struct file *file); > > void vfio_file_set_kvm(struct file *file, struct kvm *kvm); > > bool vfio_file_has_dev(struct file *file, struct vfio_device *device); > Eric