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

From: "Liu, Yi L" <yi.l.liu@intel.com>
To: Jason Gunthorpe <jgg@nvidia.com>
Cc: Alex Williamson <alex.williamson@redhat.com>,
	"Tian, Kevin" <kevin.tian@intel.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"Hao, Xudong" <xudong.hao@intel.com>,
	"peterx@redhat.com" <peterx@redhat.com>,
	"Xu, Terrence" <terrence.xu@intel.com>,
	"chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
	"linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
	"mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
	"lulu@redhat.com" <lulu@redhat.com>,
	"joro@8bytes.org" <joro@8bytes.org>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"Zhao, Yan Y" <yan.y.zhao@intel.com>,
	"intel-gfx@lists.freedesktop.org"
	<intel-gfx@lists.freedesktop.org>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"intel-gvt-dev@lists.freedesktop.org" 
	<intel-gvt-dev@lists.freedesktop.org>,
	"yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
	"cohuck@redhat.com" <cohuck@redhat.com>,
	"shameerali.kolothum.thodi@huawei.com" 
	<shameerali.kolothum.thodi@huawei.com>,
	"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>
Subject: RE: [PATCH v6 12/24] vfio/pci: Allow passing zero-length fd array in VFIO_DEVICE_PCI_HOT_RESET
Date: Thu, 23 Mar 2023 03:15:20 +0000	[thread overview]
Message-ID: <DS0PR11MB7529E2DEEB1CBBC9413A0480C3879@DS0PR11MB7529.namprd11.prod.outlook.com> (raw)
In-Reply-To: <ZBsF950laMs2ldFc@nvidia.com>

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Wednesday, March 22, 2023 9:43 PM
> 
> On Wed, Mar 22, 2023 at 01:33:09PM +0000, Liu, Yi L wrote:
> 
> > Thanks. So this new _INFO only reports a limited scope instead of
> > the full list of affected devices. Also, it is not static scope since device
> > may be opened just after the _INFO returns.
> 
> Yes, it would be simplest for qemu to do the query after it gains a
> new dev_id and then it can add the new dev_id with the correct reset
> group.

I see. QEMU can decide. For now, it seems like QEMU doesn't store
such the info return by the existing _INFO ioctl. It is used in the hot
reset helper and freed before it returns. Though, I'm not sure whether
QEMU will store the dev_id info returned by the new _INFO. Perhaps
Alex can give some guidance.

> > > I'd probably use similar logic as the reset path, if one of reset
> > > group devices is open and on a different iommufd_ctx then fail the
> > > IOCTL with EPERM.
> >
> > Say there are three devices in the dev_set. When the first device is
> > opened by the current qemu, this new _INFO would return one dev_id
> > to user. When the second device is opened, this new _INFO will return
> > two dev_ids to user.
> 
> Yes
> 
> > If the third device is opened by another qemu, then
> > the new _INFO would fail since the former two devices were opened and
> > have different iommufd_ctx with the third device.
> 
> Yes
> 
> qemu should refuse to use the device at this moment.

Yes. it is.

btw.  Another question about this new _INFO ioctl. If there are affected
devices that have not been bound to vfio driver yet, should this new _INFO
ioctl fail all the same with EPERM? Or it still just returns the dev_ids
of the devices that are already bound and opened. This seems to make sense
with two reasons:
 - for one, the new _INFO is not designed to give userspace an exact
   affected device list;
 - for two, reset shall fail when checking vfio_pci_dev_set_resettable();
Please feel free to correct me if this is wrong.

Regards,
Yi Liu