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

[Liu, Yi L]

> From: Tian, Kevin <kevin.tian at intel.com>
> Sent: Wednesday, March 8, 2023 4:14 PM
> 
> > From: Liu, Yi L <yi.l.liu at intel.com>
> > Sent: Wednesday, March 8, 2023 4:01 PM
> >
> > > From: Tian, Kevin <kevin.tian at intel.com>
> > > Sent: Wednesday, March 8, 2023 3:55 PM
> > >
> > > > From: Liu, Yi L <yi.l.liu at intel.com>
> > > > Sent: Wednesday, March 8, 2023 3:47 PM
> > > >
> > > > > From: Tian, Kevin <kevin.tian at intel.com>
> > > > > Sent: Wednesday, March 8, 2023 3:26 PM
> > > > >
> > > > > > From: Liu, Yi L <yi.l.liu at intel.com>
> > > > > > Sent: Tuesday, March 7, 2023 9:29 PM
> > > > > >
> > > > > > >
> > > > > > > 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[];
> > > > > > };
> > > > > >
> > > > >
> > > > >  * Userspace requests hot reset for the devices it uses.  Due to the
> > > > >  * underlying topology, multiple devices can be affected in the reset
> > > > >  * while some might be opened by another user. To avoid
> interference
> > > > >  * the calling user must ensure all affected devices, if opened, are
> > > > >  * owned by itself.
> > > > >  *
> > > > >  * The ownership can be proved in three ways:
> > > > >  *   - An array of group fds
> > > > >  *   - An array of device fds
> > > > >  *   - A zero-length array
> > > > >  *
> > > > Thanks.
> > > > >  * In the last case all affected devices which are opened by this user
> > > must
> > > > >  * have been bound to a same iommufd_ctx.
> > > >
> > > > I think we only allow it when this iommufd_ctx is valid. Is it? To
> > > > user, it means device should be bound to a positive iommufd.
> > >
> > > I didn't get it. Do we have a iommufd_ctx created but marked as
> > > invalid?
> >
> > I mean iommufd_ctx==NULL. If a negative iommufd is provided,
> > then kernel side only has a NULL iommufd_ctx. If so, the ownership
> > check just fail if it uses iommufd_ctx for ownership proof.
> 
> it's fine. iommufd_ctx check doesn't work with noiommu.
> 
> User should use device fd if involving noiommu.

Yes, this is my point. This zero-length array approach is only
available for devices that are bound to positive iommufd.