RE: [PATCH v5 09/19] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

Liu, Yi L
2023-03-07T13:30:06Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, March 7, 2023 8:37 PM
>
> On Tue, Mar 07, 2023 at 02:31:11AM +0000, Tian, Kevin wrote:
> > > From: Jason Gunthorpe <jgg@nvidia.com>
> > > Sent: Monday, March 6, 2023 9:17 PM
> > >
> > > On Fri, Mar 03, 2023 at 09:55:42AM -0700, Alex Williamson wrote:
> > >
> > > > I can't think of a reason DPDK couldn't use hot-reset. If we want to
> > > > make it a policy, it should be enforced by code, but creating that
> > > > policy based on a difficulty in supporting that mode with iommufd isn't
> > > > great.
> > >
> > > On the other hand adding code to allow device FDs in the hot reset
> > > path that is never used and never tested isn't great either..
> > >
> > > hot-reset does work for DPDK, it just doesn't work in the case where
> > > DPDK would have many VFIO devices open and they have overlapping
> > > device sets. Which, again, is something it doesn't do.
> > >
> > > IMHO we should leave it out of the kernel and wait for a no-iommu user
> > > to come forward that wants hot-reset of many devices. Then we can
> add
> > > and test the device FD part. Most likely such a thing will never come
> > > at this point.
> > >
> >
> > I think we don't need to have this tradeoff if following Yi's last proposal
> > which requires every opened device in the set to be covered by the
> > device fd array. with dev_set->lock held in the reset/open path this is
> > a safe measure and fully contained in vfio-pci w/o need of further
> > checking noiommu or iommufd.
>
> I really prefer the 'use the iommufd option' still exist, it is so
> much cleaner and easier for the actual users of this API. We've lost
> the point by worrying about no iommu.

Hmmm, so you are suggesting to have both the device fd approach
and the zero-length array approach, let user to select the best way
based on their wisdom. Is it? how about something like below in the
uapi header.

/**
* VFIO_DEVICE_PCI_HOT_RESET - _IOW(VFIO_TYPE, VFIO_BASE + 13,
* struct vfio_pci_hot_reset)
*
* Userspace requests hot reset for the devices it uses. Due to the
* underlying topology, multiple devices may be affected in the reset.
* The affected devices may have been opened by the user or by other
* users or not opened yet. Only when all the affected devices are
* either opened by the current user or not opened by any user, should
* the reset request be allowed. Otherwise, this request is expected
* to return error. group_fds array can accept either group fds or
* device fds. Users using iommufd (valid fd), could also passing a
* zero-length group_fds array to indicate using the bound iommufd_ctx
* for ownership check to the affected devices that are opened.
*
* Return: 0 on success, -errno on failure.
*/
struct vfio_pci_hot_reset {
__u32 argsz;
__u32 flags;
__u32 count;
__s32 group_fds[];
};

Regards,
Yi Liu