[Intel-gfx] [PATCH v6 12/24] 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: Thursday, March 16, 2023 2:10 PM
> 
> > From: Liu, Yi L <yi.l.liu at intel.com>
> > Sent: Thursday, March 16, 2023 11:55 AM
> >
> > > From: Tian, Kevin <kevin.tian at intel.com>
> > > Sent: Thursday, March 16, 2023 7:31 AM
> > >
> > > > From: Alex Williamson <alex.williamson at redhat.com>
> > > > Sent: Thursday, March 16, 2023 6:53 AM
> > > >
> > > > On Wed,  8 Mar 2023 05:28:51 -0800
> > > > Yi Liu <yi.l.liu at 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.

Yes. this failure matches the uapi description. And it is rare case,
mostly likely applications should be explicitly updated to use cdev
and then use this zero-length approach. Before that, the legacy
applications do not know it at all. Even it uses this approach
by mistake, it will fail in the multiple compat iommufd case.

So maybe no need to limit it.

> 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.

For this, I agree with you.

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

Sure. Once Nicolin's patch is updated, I can send v7 with the hot
reset series as well.

Regards,
Yi Liu