Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     Matthew Rosato <mjrosato@linux.ibm.com>,
        "jgg@nvidia.com" <jgg@nvidia.com>,
        "alex.williamson@redhat.com" <alex.williamson@redhat.com>
CC:     "jjherne@linux.ibm.com" <jjherne@linux.ibm.com>,
        "akrowiak@linux.ibm.com" <akrowiak@linux.ibm.com>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "hch@infradead.org" <hch@infradead.org>,
        "linux-s390@vger.kernel.org" <linux-s390@vger.kernel.org>,
        "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>,
        "cohuck@redhat.com" <cohuck@redhat.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "zhenyuw@linux.intel.com" <zhenyuw@linux.intel.com>,
        "pasic@linux.ibm.com" <pasic@linux.ibm.com>,
        "borntraeger@linux.ibm.com" <borntraeger@linux.ibm.com>,
        "intel-gvt-dev@lists.freedesktop.org" 
        <intel-gvt-dev@lists.freedesktop.org>,
        "Wang, Zhi A" <zhi.a.wang@intel.com>
Subject: RE: [PATCH v2 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
Thread-Topic: [PATCH v2 1/1] vfio: remove VFIO_GROUP_NOTIFY_SET_KVM
Thread-Index: AQHYav38r+LwcdssakyG6t/K4P4DTq0luTjQ
Date:   Thu, 19 May 2022 06:43:06 +0000
Message-ID: <BN9PR11MB527684F9BD1B906B930E4A468CD09@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20220518212607.467538-1-mjrosato@linux.ibm.com>
 <20220518212607.467538-2-mjrosato@linux.ibm.com>
In-Reply-To: <20220518212607.467538-2-mjrosato@linux.ibm.com>
Accept-Language: en-US
Content-Language: en-US
dlp-version: 11.6.401.20
dlp-product: dlpe-windows
dlp-reaction: no-action
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?oD5w9SZfUYp3UvNLWH2HoXpQWU3WUKdCrquJ9Cmhjj1KJ+/HurHGVJBEnkUr?=
 =?us-ascii?Q?nTbJT5OmEJrNS96oER/gfC7/5FJ6OCVGCjJYx3r9kCuxrDobkHJhpFY9u6jo?=
 =?us-ascii?Q?1/f4kmXjCUS2NRBX3YG7Sdv5o72X8UKJxU9yiKJopEy20+578aP6GMVwFK+s?=
 =?us-ascii?Q?VZcf8EIn3gyd0bYC/5Kw+ZbNDyUN7EU1I4TXw0hTMvdDx495cE20nNOLbJYb?=
 =?us-ascii?Q?RDz4PDFIKNyT3CS8EirNEZhdTY3x+2G958/dXvoqIWgdhatOoYbIW4yju1qK?=
 =?us-ascii?Q?vfjhPACyKx5mAAhJYcPzmsaqz48QTNmWlTRiIu8MSkilKS70JDlQ4h1pBr6k?=
 =?us-ascii?Q?4r/qLEZt3b8RGQUjdWJBho88Kf2qrdinViGcV7fR2W0t7QOpwxykPcz6A7r+?=
 =?us-ascii?Q?Fv2+n9pAleKZIoztFcyqFKTrN0tpzf+Rq/d8zQDgRnWXrfllPNoDxyeWAaK1?=
 =?us-ascii?Q?3eFLpZ4WgAnAJbo1DRv/8UfsmH7FYohVi6ZKxyC1h38XrFTXsRG/Szd3w4uE?=
 =?us-ascii?Q?sdMdQl0Q3KMNS1GrVfyRmPRUofZOmohpAoQSu0F1ZX3uICGqMe11pteBZD9J?=
 =?us-ascii?Q?Di2GDPtxCgBqOtCw8sxyWlBFPSzb5IcIWfr/v7a9A6ta5+9/jvUvNUM2PydX?=
 =?us-ascii?Q?VjdGCcwiJu2OJfrl7iqD/hYW5CPeB/3qHOOyheNKWrCn3d6CHcd8PSN7sBl6?=
 =?us-ascii?Q?Uf9nuS9lYMNzmSuekklB0iTnsQTyWs6PIKGIIv9+9BSG3u6mM6EjrYfIo/zt?=
 =?us-ascii?Q?8orwHo/MlUB/UEoc+hwNyLMabPOdO1Oc9qIrV55JvX6jaZDTozKXrUodTqBB?=
 =?us-ascii?Q?SEMzBPKUJW78CyfZJoWVmsjKq+HaN4Wbrk8gmEqvyg4gRccF7ta94bz0H+8l?=
 =?us-ascii?Q?1TopTPHJeCm1Lis1ScXVqekjLEcWD3nUz6rjmLPYUZA2lLyx9Htlo7tG7HnL?=
 =?us-ascii?Q?TmpWkpw6oTFjz78oMiRSc1PLefXBwxDqxGMSt5bFda7tHxpLvwbmZMlPI5SR?=
 =?us-ascii?Q?6N9uUe6Sb1JiLqu8YYF6cxK0jsY7nqxY+bmbLBOaWzBgR7N2psQWliWyoIdR?=
 =?us-ascii?Q?lzA9z64x+fW31kdIhN5GLBx+hUv8Jsk48HSbrWrf/eNam5OJ3D5OOa6BWw5Z?=
 =?us-ascii?Q?BYbpRS4ddvnz6mX9ONQa6erR54JpuVUOz4Lmr0y0vDpNteQPyZVg9ehivOYg?=
 =?us-ascii?Q?i90opsiqBNGomsRYR9o7jrmk9KRRCPtiktv9lnEozJQqEvEXJBlYgks6mz35?=
 =?us-ascii?Q?+7HgWZqTiGSnaPkTMJMDe2Qrsf3yldDeQaQjAjROWDekuDP9uE074igukURl?=
 =?us-ascii?Q?B1mxmQCcipGX51kwclxyH1hGfdrdxepd7+PnO21l5hQvaM79HEYiZzpPs9BI?=
 =?us-ascii?Q?LnBad9pwgV5e7V4rQU2R+3hdg8OBs8FJ0+6SjzhP1pw0WPsLH7Iw/hU9ZEUr?=
 =?us-ascii?Q?pXLY8z8pukuaz58ZHRU0qux1Hle1fSsalfenTf6zNRamFLCfM3yh7vwG3axD?=
 =?us-ascii?Q?fg3793DL3IQ3T/M1UzUrYqlmZg5utP+Yv5bExQ41YGO/Y2xzymtYYUDbDgF2?=
 =?us-ascii?Q?KXe+2fCwH9giL2CcbtPKLdKXhKGfDnKNaTnyFmclcM+klUdDBd2StQ2VhWI6?=
 =?us-ascii?Q?pnLIZpxxtoHPXP2eAIphI7506R64euJioEvFfGpsSySd2ajrmJDOcvxSW9S9?=
 =?us-ascii?Q?1GLhJNVrEwYSHCP4YpkvB4J6AcMOt8rAf6UJUgPycjPITKTZ9ghZPgH7O/0d?=
 =?us-ascii?Q?LnbZsv1krg=3D=3D?=
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: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 2c6badf4-0a05-4efb-43e4-08da3962d4f6
X-MS-Exchange-CrossTenant-originalarrivaltime: 19 May 2022 06:43:06.8521
 (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: 0i+NI+SvyXMtZpOd+b99HN2s1BkF8LFH3II0Jwga7NzFyLwi9ThgrTfLznMPjVlmxpbymJzycp3Ji6CGxHs6PQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB6077
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-7.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_HI,
        SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Matthew Rosato
> Sent: Thursday, May 19, 2022 5:26 AM
>=20
> Rather than relying on a notifier for associating the KVM with
> the group, let's assume that the association has already been
> made prior to device_open.  The first time a device is opened
> associate the group KVM with the device.
>=20
> This fixes a user-triggerable oops in GVT.

No changelog.

>=20
> Signed-off-by: Jason Gunthorpe <jgg@nvidia.com>
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>

Not sure whether Christoph wants a s-o-b here when he wrote
the snippet to remove the release work of gvt...

> @@ -1083,11 +1083,22 @@ static struct file *vfio_device_open(struct
> vfio_device *device)
>=20
>  	mutex_lock(&device->dev_set->lock);
>  	device->open_count++;
> +	down_read(&device->group->group_rwsem);
> +	if (device->open_count =3D=3D 1 && device->group->kvm) {
> +		/*
> +		 * Here we pass the KVM pointer with the group under the
> read
> +		 * lock.  If the device driver will use it, it must obtain a
> +		 * reference and release it during close_device.
> +		 */
> +		device->kvm =3D device->group->kvm;
> +	}
> +
>  	if (device->open_count =3D=3D 1 && device->ops->open_device) {

Merge the two branches so both are under if (device->open_count =3D=3D 1) {=
}
(and group_rwsem can be also moved inside)

> @@ -1315,9 +1330,13 @@ static int vfio_device_fops_release(struct inode
> *inode, struct file *filep)
>=20
>  	mutex_lock(&device->dev_set->lock);
>  	vfio_assert_device_open(device);
> +	down_read(&device->group->group_rwsem);
>  	if (device->open_count =3D=3D 1 && device->ops->close_device)
>  		device->ops->close_device(device);
>  	device->open_count--;
> +	if (device->open_count =3D=3D 0 && device->kvm)
> +		device->kvm =3D NULL;

This can be moved out of group_rwsem as there is no reference to
vfio_group.

> +	up_read(&device->group->group_rwsem);

otherwise,

Reviewed-by: Kevin Tian <kevin.tian@intel.com>

Thanks
Kevin