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>,
        "shameerali.kolothum.thodi@huawei.com" 
        <shameerali.kolothum.thodi@huawei.com>,
        "lulu@redhat.com" <lulu@redhat.com>,
        "suravee.suthikulpanit@amd.com" <suravee.suthikulpanit@amd.com>,
        "intel-gvt-dev@lists.freedesktop.org" 
        <intel-gvt-dev@lists.freedesktop.org>,
        "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>
Subject: RE: [PATCH v2 11/14] vfio: Make vfio_device_open() exclusive between
 group path and device cdev path
Thread-Topic: [PATCH v2 11/14] vfio: Make vfio_device_open() exclusive between
 group path and device cdev path
Thread-Index: AQHZOgpBwImAS0LRl0mKfU0T5wA5rK7DBbMAgAApJqA=
Date:   Tue, 7 Feb 2023 08:54:33 +0000
Message-ID: <DS0PR11MB75299599F5DCFA79D894536AC3DB9@DS0PR11MB7529.namprd11.prod.outlook.com>
References: <20230206090532.95598-1-yi.l.liu@intel.com>
 <20230206090532.95598-12-yi.l.liu@intel.com>
 <BN9PR11MB52764ED1F43F717E04B8E3588CDB9@BN9PR11MB5276.namprd11.prod.outlook.com>
In-Reply-To: <BN9PR11MB52764ED1F43F717E04B8E3588CDB9@BN9PR11MB5276.namprd11.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?t1s2RA9vrTw3h1eFDmlM3OieoA0ahBSS1IVo6Av+52c5ofwqbJLO7K4eSPA8?=
 =?us-ascii?Q?G6bFgJcGQoW8MJYNWiU/oMIaIZGepIXrCRn4nEHftg5ZZ69k7is1rH7mXhtE?=
 =?us-ascii?Q?ofddUsE1H6AaJ1mg/s/uMM7X/sOlrdlB9poPQq4UrfKn/nSVUcpV80Z/4ih5?=
 =?us-ascii?Q?5D0Lvk2RQ3Aua3dI8bMHUaaSuSGkzQ3sQ/RLyN9kMIQLUOLZvune+nUh1EgA?=
 =?us-ascii?Q?XWF36Qlqrj2O0QZLUwXhyyH9OTHHRYmLU3AwGKc/kSDBIQ2DNyZd9aD9SVQF?=
 =?us-ascii?Q?GksZoPWETJZczZU1exCYNNzfq7D68eBoOQOOewoyaxeESKSZwhzMjaOOj5if?=
 =?us-ascii?Q?IwVbe8DEI1+NTR+jlbm3TwjqtTmLyqMFDCmprfv20sv7cKxLGJVXPWeuehwi?=
 =?us-ascii?Q?/CIuZQ2trQPkD6CS/hjM6PEPwO2Xb3oheOLVSPoFxUwMq2DtOv3i2gEhxbu4?=
 =?us-ascii?Q?LQekq+hwjto74yKBWCwGMUz1/VBG2cj3XxSAvG6AxhDLyK2RQSj4C6QTEnrc?=
 =?us-ascii?Q?sf+7kcCQTPDSaWpcMw3Ko+IuL49tPRW/vhJXMOdI6+nu2SSBiXIZXiiEyKF8?=
 =?us-ascii?Q?6A4JflpC1/6gtIlM+1PUlLITAyGGrSRHEFEVtaf4WU5LlTJx3V5UqqigLx5j?=
 =?us-ascii?Q?lqWEgpbWU7AE56MNc6UQA739b2MU/YzuSO5XihfsNaGXyVSHYvw+j4eJDx/O?=
 =?us-ascii?Q?wLBaK6YEQVQcqe5DscC34QCEwm4KIL3Te/74janbUH6ZS8kp9bqEmpSt8vu8?=
 =?us-ascii?Q?tMIA+rsKbh5hLpw+WZuteng1P0OXrsP2b5qQ8ieXkNlvRSuP7W7Mk7jerS5o?=
 =?us-ascii?Q?8nhPRghYgMSSWcnSOE6fQNNPIrecx5sSIzKwZ10pO0RL2vBDaMKhiyZEfQ97?=
 =?us-ascii?Q?Xx2C3RbUfOqi/3ZTgjXkzwJW9saVRgbz4Uoz3PpuJVFkCbQyjv4VO/UdCqXm?=
 =?us-ascii?Q?gKlwjWZNrjHz9l20KxyI375yeqw9VzLfF1PVXHg5557vtndbQ9H6Qumji6Kf?=
 =?us-ascii?Q?LXJSz/JYllZg3CJ3WaXijFOrWwHCg54XmU9aD52eLn5W5WBWJGCdZ5Y/HX+0?=
 =?us-ascii?Q?U9NzE58d0yi8vw+R/fh+WclSVKvSRhIPuslTKY2Z+sjNI3pxG0z/Tzuig8P1?=
 =?us-ascii?Q?opEeEtSvA5X7pumLS6UxukXmZhSm70pZcnczsortilHOcKL1qAxJNamhrvwa?=
 =?us-ascii?Q?6Dx6BvEtatm7WIncFr3dNbEnTeMVPNJGYPba0k9R8tqxgyk6TqXj6IMfPtmo?=
 =?us-ascii?Q?GSxB/kyklzsStYdA40brPCEvdh8jOcaudz4G7+DIfI6EO6M37ri3nEJeCz5f?=
 =?us-ascii?Q?V/rJRCFWJFqTU3vI5uEa0nWeEYKOcJzO35uFPmjtfrVw0TG3zgxAOuoYag6h?=
 =?us-ascii?Q?LItjWxdcQpXXTgRo8Lsc56YT/3LNWKPPg8AbYVudO/aN5iJlvZw/UTmBsdR6?=
 =?us-ascii?Q?1OYvmcy4oZPxd55wyI2Msgdpt8lQNklK6X7mXuKUg6lgNb+Q9k8aVuKC2CjJ?=
 =?us-ascii?Q?+lqlVFu2mhf6EPQ141KtZ3LDYbENt74U6nWtxc5PBm/nvJEiFnU5GOlDyKEI?=
 =?us-ascii?Q?sRN4KeqM9AyQAtd/umSIRY7ynrZ5hAoXbQaiSayd?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB7529.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 142be129-1b52-40da-cd40-08db08e8eec4
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Feb 2023 08:54:33.3741
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: fRpKoSmpQ/54FOgMDC6jtnllLoQGtQ2xWIbJerYL04XSRtp1nCRNI0P5nlwQYJP7vQtOCPfSdZmycEMMkQ0Myw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB6868
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Tian, Kevin <kevin.tian@intel.com>
> Sent: Tuesday, February 7, 2023 2:25 PM
>=20
> > From: Liu, Yi L <yi.l.liu@intel.com>
> > Sent: Monday, February 6, 2023 5:05 PM
> >
> >  struct vfio_device_file *
> > -vfio_allocate_device_file(struct vfio_device *device)
> > +vfio_allocate_device_file(struct vfio_device *device, bool
> is_cdev_device)
> >  {
> >  	struct vfio_device_file *df;
> >
> > @@ -407,6 +407,7 @@ vfio_allocate_device_file(struct vfio_device
> *device)
> >  		return ERR_PTR(-ENOMEM);
> >
> >  	df->device =3D device;
> > +	df->is_cdev_device =3D is_cdev_device;
>=20
> You missed Alex's comment that the one caller can open code the
> assignment instead of introducing an unmemorable Boolean arg here.

Oh, yes. will open code to set it in cdev path.

> >
> > +	/*
> > +	 * 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 !=3D 0 &&
> > +	    (df->is_cdev_device || device->single_open))
> > +		return -EINVAL;
> > +
>=20
> If we are gonna handle the exclusive open via dma ownership, then
> here we don't need a new single_open flag inside vfio_device since
> only one interface (cdev or group) could be used to open this device.
>=20
> Just preventing multi-open in cdev is sufficient here.

I see. Per below discussion, just need to make group path always
use vfio_group pointer as DMA marker.

https://lore.kernel.org/kvm/BN9PR11MB5276FA68E8CAD6394A8887848CDB9@BN9PR11M=
B5276.namprd11.prod.outlook.com/

Regards,
Yi Liu