RE: [PATCH 10/13] vfio: Make vfio_device_open() exclusive between group path and device cdev path

From: "Liu, Yi L" <yi.l.liu@intel.com>
To: "Tian, Kevin" <kevin.tian@intel.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"jgg@nvidia.com" <jgg@nvidia.com>
Cc: "cohuck@redhat.com" <cohuck@redhat.com>,
	"eric.auger@redhat.com" <eric.auger@redhat.com>,
	"nicolinc@nvidia.com" <nicolinc@nvidia.com>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>,
	"mjrosato@linux.ibm.com" <mjrosato@linux.ibm.com>,
	"chao.p.peng@linux.intel.com" <chao.p.peng@linux.intel.com>,
	"yi.y.sun@linux.intel.com" <yi.y.sun@linux.intel.com>,
	"peterx@redhat.com" <peterx@redhat.com>,
	"jasowang@redhat.com" <jasowang@redhat.com>,
	"suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>
Subject: RE: [PATCH 10/13] vfio: Make vfio_device_open() exclusive between group path and device cdev path
Date: Mon, 30 Jan 2023 11:59:00 +0000	[thread overview]
Message-ID: <DS0PR11MB7529E91906BC8F6FB9A2B1A7C3D39@DS0PR11MB7529.namprd11.prod.outlook.com> (raw)
In-Reply-To: <BN9PR11MB52769CBCA68CD25DAC96B33B8CC49@BN9PR11MB5276.namprd11.prod.outlook.com>

> From: Tian, Kevin <kevin.tian@intel.com>
> Sent: Thursday, January 19, 2023 5:55 PM
> 
> > From: Liu, Yi L <yi.l.liu@intel.com>
> > Sent: Tuesday, January 17, 2023 9:50 PM
> >
> > @@ -17,7 +17,11 @@ struct vfio_device;
> >  struct vfio_container;
> >
> >  struct vfio_device_file {
> > +	/* static fields, init per allocation */
> >  	struct vfio_device *device;
> > +	bool single_open;
> 
> I wonder whether the readability is better by renaming this
> to 'cdev', e.g.:
> 
> 	/*
> 	 * Device cdev path cannot support multiple device open since
> 	 * it doesn't have a secure way for it. So a second device
> 	 * open attempt should be failed if the caller is from a cdev
> 	 * path or the device has already been opened by a cdev path.
> 	 */
> 	if (device->open_count != 0 &&
> 	    (df->cdev || device->single_open))
> 		return -EINVAL;
> 
> 	/*
> 	 * group path supports multiple device open, while cdev doesn't.
> 	 * So use vfio_device_group_close() for !singel_open case.
> 	 */
> 	if (!df->cdev)
> 		vfio_device_group_close(df);
> 
> because from device file p.o.v we just want to differentiate cdev
> vs. group interface. With this change we even don't need the
> comment for the last condition check.
> 
> it's fine to have device->single_open as it's kind of a status bit
> set in the cdev path to prevent more opens on this device.

Ok. 

Regards,
Yi Liu