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

Tian, Kevin
2023-03-02T09:56:34Z

> From: Liu, Yi L <yi.l.liu@intel.com>
> Sent: Thursday, March 2, 2023 2:07 PM
>
> > - if (!vfio_dev_in_groups(cur_vma, groups)) {
> > + if (cur_vma->vdev.open_count &&
> > + !vfio_dev_in_groups(cur_vma, groups) &&
> > + !vfio_dev_in_iommufd_ctx(cur_vma, iommufd_ctx)) {
>
> Hi Alex, Jason,
>
> There is one concern on this approach which is related to the
> cdev noiommu mode. As patch 16 of this series, cdev path
> supports noiommu mode by passing a negative iommufd to
> kernel. In such case, the vfio_device is not bound to a valid
> iommufd. Then the check in vfio_dev_in_iommufd_ctx() is
> to be broken.
>
> An idea is to add a cdev_noiommu flag in vfio_device, when
> checking the iommufd_ictx, also check this flag. If all the opened
> devices in the dev_set have vfio_device->cdev_noiommu==true,
> then the reset is considered to be doable. But there is a special
> case. If devices in this dev_set are opened by two applications
> that operates in cdev noiommu mode, then this logic is not able
> to differentiate them. In that case, should we allow the reset?
> It seems to ok to allow reset since noiommu mode itself means
> no security between the applications that use it. thoughts?
>

Probably we need still pass in a valid iommufd (instead of using
a negative value) in noiommu case to mark the ownership so the
check in the reset path can correctly catch whether an opened
device belongs to this user.

That implies we may instead use a flag bit to mark NOIOMMU
mode and in the kernel also has a noiommu flag in device
file to differentiate it from normal case.