Re: [PATCH v6 3/3] vfio/pci: Check the device set open count on reset

From: Alex Williamson <alex.williamson@redhat.com>
To: Yi Liu <yi.l.liu@intel.com>
Cc: Anthony DeRossi <ajderossi@gmail.com>, <kvm@vger.kernel.org>,
	<cohuck@redhat.com>, <jgg@nvidia.com>, <kevin.tian@intel.com>,
	<abhsahu@nvidia.com>, <yishaih@nvidia.com>
Subject: Re: [PATCH v6 3/3] vfio/pci: Check the device set open count on reset
Date: Wed, 9 Nov 2022 21:17:15 -0700	[thread overview]
Message-ID: <20221109211715.7cdacf3d.alex.williamson@redhat.com> (raw)
In-Reply-To: <49b64e4b-43b9-ec7b-23d2-2fa1bf921046@intel.com>

On Thu, 10 Nov 2022 11:03:29 +0800
Yi Liu <yi.l.liu@intel.com> wrote:

> Hi DeRossi,
> 
> On 2022/11/10 09:40, Anthony DeRossi wrote:
> > vfio_pci_dev_set_needs_reset() inspects the open_count of every device
> > in the set to determine whether a reset is allowed. The current device
> > always has open_count == 1 within vfio_pci_core_disable(), effectively
> > disabling the reset logic. This field is also documented as private in
> > vfio_device, so it should not be used to determine whether other devices
> > in the set are open.  
> 
> haven't went through the prior version. maybe may question has been already
> answered. My question is:
> 
> the major reason is the order problem in vfio_main.c. close_device() is
> always called before decreasing open_count to be 0. So even other device
> has no open fd, the current vfio_device still have one open count. So why
> can't we just switch the order of open_count-- and close_device()?

This is what was originally proposed and Jason shot it down:

https://lore.kernel.org/all/Y1kY0I4lr7KntbWp@ziepe.ca/

> > Checking for vfio_device_set_open_count() > 1 on the device set fixes
> > both issues  
> tbh. it's weird to me that a driver needs to know the internal logic of
> vfio core before knowing it needs to check the vfio_device_set_open_count()
> in this way. Is vfio-pci the only driver that needs to do this check or
> there are other drivers? If there are other drivers, maybe fixing the order
> in core is better.

Please see the evolution of reflck into device sets.  Both PCI and FSL
can have multiple devices in a set, AIUI.  The driver defines the set.
This ability to test for the last close among devices in the set is a
fundamental feature of the original reflck.  Thanks,

Alex