> 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
