RE: [PATCH v6 12/24] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET

Tian, Kevin
2023-03-16T06:09:50Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Thursday, March 16, 2023 11:55 AM
>
> > From: Tian, Kevin <kevin.tian@intel.com>
> > Sent: Thursday, March 16, 2023 7:31 AM
> >
> > > From: Alex Williamson <alex.williamson@redhat.com>
> > > Sent: Thursday, March 16, 2023 6:53 AM
> > >
> > > On Wed, 8 Mar 2023 05:28:51 -0800
> > > Yi Liu <yi.l.liu@intel.com> wrote:
> > >
> > > > This is another method to issue PCI hot reset for the users that bounds
> > > > device to a positive iommufd value. In such case, iommufd is a proof of
> > > > device ownership. By passing a zero-length fd array, user indicates
> > kernel
> > > > to do ownership check with the bound iommufd. All the opened devices
> > > within
> > > > the affected dev_set should have been bound to the same iommufd.
> > This is
> > > > simpler and faster as user does not need to pass a set of fds and kernel
> > > > no need to search the device within the given fds.
> > >
> > > Couldn't this same idea apply to containers?
> >
> > User is allowed to create multiple containers. Looks we don't have a way
> > to check whether multiple containers belong to the same user today.
>
> Hi Kevin,
>
> This reminds me. In the compat mode, container fd is actually iommufd.
> If the compat mode passes a zeror-length array to do reset, it is possible
> that the opened devices in this affected dev_set may be set to different
> containers (a.k.a. iommufd_ctx). This would break what we defined in
> uapi. So a better description is users that use cdev can use this zero-length
> approach. And also, in kernel, we need to check if this approach is abused
> by the compat mode.
>

In normal case legacy application uses group fd array and new application
with cdev uses zero-length approach.

In rare case an application which opens /dev/iommu but opts to use it
as a container in compat mode can also use zero-length array to reset
if all devices are attached to a single container (internally to a same
iommufd_ctx). It's still kind of matching uAPI description.

I'm not sure whether we want to add explicit check to prevent it.

Of course if affected devices span multiple compat iommufd's then
it will fail.

The open Alex raised is whether we want to further extend it to
legacy container if all affected devices are in one container. But
I hesitate to do so since iommufd is the future and if an application
can be rewritten to utilize zero-length reset then it probably
should explicitly embrace iommufd instead.

Anyway let's not wait here. Send your v7 and we can have more
focused discussion in your split series about hot reset.

Thanks
Kevin