[Intel-gfx] [PATCH v9 06/25] kvm/vfio: Accept vfio device file from userspace

[Liu, Yi L]

Hi Eric,

> From: Eric Auger <eric.auger at redhat.com>
> Sent: Friday, April 7, 2023 4:57 PM
> 
> Hi Yi,
> 
> On 4/7/23 05:42, Liu, Yi L wrote:
> >> From: Alex Williamson <alex.williamson at redhat.com>
> >> Sent: Friday, April 7, 2023 2:58 AM
> >>>> You don't say anything about potential restriction, ie. what if the user calls
> >>>> KVM_DEV_VFIO_FILE with device fds while it has been using legacy
> >> container/group
> >>>> API?
> >>> legacy container/group path cannot do it as the below enhancement.
> >>> User needs to call KVM_DEV_VFIO_FILE before open devices, so this
> >>> should happen before _GET_DEVICE_FD. So the legacy path can never
> >>> pass device fds in KVM_DEV_VFIO_FILE.
> >>>
> >>>
> >>
> https://lore.kernel.org/kvm/20230327102059.333d6976.alex.williamson@redhat.com
> >> /#t
> >>
> >> Wait, are you suggesting that a comment in the documentation suggesting
> >> a usage policy somehow provides enforcement of that ordering??  That's
> >> not how this works.  Thanks,
> > I don't know if there is a good way to enforce this order in the code. The
> > vfio_device->kvm pointer is optional. If it is NULL, vfio just ignores it.
> > So vfio doesn't have a good way to tell if the order requirement is met or
> > not. Perhaps just trigger NULL pointer dereference when kvm pointer is used
> > in the device drivers like kvmgt if this order is not met.
> >
> > So that's why I come up to document it here. The applications uses kvm
> > should know this and follow this otherwise it may encounter error.
> >
> > Do you have other suggestions for it? This order should be a generic
> > requirement. is it? group path also needs to follow it to make the mdev
> > driver that refers kvm pointer to be workable.
> 
> In the same way as kvm_vfio_file_is_valid() called in kvm_vfio_file_add()
> can't you have a kernel API that checks the fd consistence?

I think we are talking about how to check if the order between
KVM_DEV_VFIO_FILE_ADD and the device open (e.g. invoked by
VFIO_GROUP_GET_DEVICE_FD) is met in the code rather than document
it here. Am I missing anything here? Maybe I've misunderstood Alex's
question. ☹

Regards,
Yi Liu

> Thanks
> 
> Eric
> >
> > Thanks,
> > Yi Liu
> >
> >>>>> -The GROUP_ADD operation above should be invoked prior to accessing the
> >>>>> +The FILE/GROUP_ADD operation above should be invoked prior to accessing
> the
> >>>>>  device file descriptor via VFIO_GROUP_GET_DEVICE_FD in order to support
> >>>>>  drivers which require a kvm pointer to be set in their .open_device()
> >>>>> -callback.
> >>>>> +callback.  It is the same for device file descriptor via character device
> >>>>> +open which gets device access via VFIO_DEVICE_BIND_IOMMUFD.  For such
> file
> >>>>> +descriptors, FILE_ADD should be invoked before
> >> VFIO_DEVICE_BIND_IOMMUFD
> >>>>> +to support the drivers mentioned in prior sentence as well.
> >>> just as here. This means device fds can only be passed with KVM_DEV_VFIO_FILE
> >>> in the cdev path.
> >>>
> >>> Regards,
> >>> Yi Liu