From nobody Sat Sep 13 13:20:37 2025 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id EF396C63797 for ; Thu, 2 Feb 2023 06:44:55 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231761AbjBBGov (ORCPT ); Thu, 2 Feb 2023 01:44:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:39856 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230156AbjBBGoo (ORCPT ); Thu, 2 Feb 2023 01:44:44 -0500 Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2C5AE81B10; Wed, 1 Feb 2023 22:44:40 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1675320280; x=1706856280; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=OHstDGFwDe+/ayhWanHdrJ5agJvf+TjXsiqBM6wvEjw=; b=FII9Ccn+UmyvsVLslpqWOsnxqSENE6UZlbJ0DNVKpU8FDF9tWUb6PyoI aN36qVQDaADvsE0ho/1FhtZJR4b/NZ3XwdFBlM9EaCBkUYQk0YhH8up8A fWnIsS6YUbMSbzX6NhUm6F15G6460R7MCQ5rXQtf/pbVoAX+aZ0OtFVgV f8JGrCi+3ewQ0IP1PqwldjXJK20JNt9Rnkg588NenXPUeZKRt2wh81BxN YtFiiNXAFHsceVWIx3H451Dd1U8k5tQ6yJ7sRudc5BLW/T1hAoZpEwkzF /uNca9zjUGkexShZLXBy/bx7ciNo09rNU00YO4hLQ5XlgkoZHE1pQY4bX g==; X-IronPort-AV: E=McAfee;i="6500,9779,10608"; a="330503950" X-IronPort-AV: E=Sophos;i="5.97,266,1669104000"; d="scan'208";a="330503950" Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 Feb 2023 22:44:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6500,9779,10608"; a="993959536" X-IronPort-AV: E=Sophos;i="5.97,266,1669104000"; d="scan'208";a="993959536" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by fmsmga005.fm.intel.com with ESMTP; 01 Feb 2023 22:44:38 -0800 Received: from fmsmsx612.amr.corp.intel.com (10.18.126.92) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Wed, 1 Feb 2023 22:44:38 -0800 Received: from fmsmsx610.amr.corp.intel.com (10.18.126.90) by fmsmsx612.amr.corp.intel.com (10.18.126.92) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16; Wed, 1 Feb 2023 22:44:37 -0800 Received: from FMSEDG603.ED.cps.intel.com (10.1.192.133) by fmsmsx610.amr.corp.intel.com (10.18.126.90) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.16 via Frontend Transport; Wed, 1 Feb 2023 22:44:37 -0800 Received: from NAM10-DM6-obe.outbound.protection.outlook.com (104.47.58.103) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2507.16; Wed, 1 Feb 2023 22:44:37 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=iLOiEPLXRd23Rc+orWxQ8zD2f50jaYyW/2mrgA+Ufd/gbuNohZaeDtEFZ6zQePDmNa70bJYVJcMSXZkNJeRVX3Owa0w7Rx0b8zQLGewM7Xm/tRw2g4DKpPykTMCMcxhL/e4NWqNL5RPxMmgOv3QqQmx9CsNfZ+RJxc9N54YEuTOi/EJzg/GEw6olEmCAJXAF+U5hZa+h1s0a+h1TxkpRiIHgr2R5McDFvKt93nRXkjhYrpcXR//ZkMKzyt2WKX1T1RW1w7a/6auXFzLynCFh+5Dfh45uVV5YivzrEDsg/cd5HY5gQvKY/YHgmu2kqHEd+Yt/zabL0LW5SmnUHDUh2Q== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=OHstDGFwDe+/ayhWanHdrJ5agJvf+TjXsiqBM6wvEjw=; b=TdDhNwnDwTKN45GOuEPeAL0nvGCRJxK8r3uvI3VVvykJi7VTBjvB//eibDSX9aXhgMx9fT5iWqGM4wtCklhQa18pBeb/kvzC9k3O8yPG0BHJvGRjfSLJQIHiO9y5E+Co6h7muMk7m/sUuF87OgcGS+HcWMdMR2jIACvnS3unDDZft9ZDdJ2HFG5wJBp0JdBAgEcZN1o8cMbniBkG6u03mpUPhqYrcbhn8sTI9qXrFXEMjTXHiQtHHsT4fmvJw5hS5y3t39Ob2Oh01nXH9YO3mBoRWBgzSCfbKWVYy/4NjHbObjcX7wMT3PkgArDihtzXdFX0NbgS7N0VK6QMlP7bvw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none Received: from DS0PR11MB7529.namprd11.prod.outlook.com (2603:10b6:8:141::20) by PH0PR11MB5807.namprd11.prod.outlook.com (2603:10b6:510:140::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.6043.38; Thu, 2 Feb 2023 06:44:29 +0000 Received: from DS0PR11MB7529.namprd11.prod.outlook.com ([fe80::e1fa:abbe:2009:b0a3]) by DS0PR11MB7529.namprd11.prod.outlook.com ([fe80::e1fa:abbe:2009:b0a3%4]) with mapi id 15.20.6064.024; Thu, 2 Feb 2023 06:44:29 +0000 From: "Liu, Yi L" To: Alex Williamson CC: Matthew Rosato , "pbonzini@redhat.com" , "jgg@nvidia.com" , "cohuck@redhat.com" , "farman@linux.ibm.com" , "pmorel@linux.ibm.com" , "borntraeger@linux.ibm.com" , "frankja@linux.ibm.com" , "imbrenda@linux.ibm.com" , "david@redhat.com" , "akrowiak@linux.ibm.com" , "jjherne@linux.ibm.com" , "pasic@linux.ibm.com" , "zhenyuw@linux.intel.com" , "Wang, Zhi A" , "Christopherson,, Sean" , "Tian, Kevin" , "linux-s390@vger.kernel.org" , "kvm@vger.kernel.org" , "intel-gvt-dev@lists.freedesktop.org" , "intel-gfx@lists.freedesktop.org" , "linux-kernel@vger.kernel.org" Subject: RE: [PATCH v2] vfio: fix deadlock between group lock and kvm lock Thread-Topic: [PATCH v2] vfio: fix deadlock between group lock and kvm lock Thread-Index: AQHZNnJFDhMzPRK6pEGcr6itqpvSQ666vJwAgAA/7dCAABBdAIAAKCuw Date: Thu, 2 Feb 2023 06:44:29 +0000 Message-ID: References: <20230201192010.42748-1-mjrosato@linux.ibm.com> <20230201162730.685b5332.alex.williamson@redhat.com> <20230201211452.429c8e34.alex.williamson@redhat.com> In-Reply-To: <20230201211452.429c8e34.alex.williamson@redhat.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-traffictypediagnostic: DS0PR11MB7529:EE_|PH0PR11MB5807:EE_ x-ms-office365-filtering-correlation-id: a3da16cd-c622-4584-1658-08db04e8eef7 x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: sjhqUSLvLBhQczXREwPtH6Qn17ayM8H7bR5bD6Y+yB+rbUlXdEZix7p92Ix/Fpu9eTX4R/T9hmb9nQjqneV4zJkAr1jeRFYSIRqWOQ2b47jaxec6Zn5SKDS2nYCyhnwWcpRS4GRTDg7GpNWoVWNm12tdNZaBiSvr/TjcYayk+cMsvquvayheh2Zeeo7Y7Ji88a5EUa+P1mKDK+AKA4FeeW51il0fzLXLhNbrXONxqXLPv0B/wehNP2Edsdu5vTMCG7rsU3OSO6e4Y6JjfDqnZNlRjQBrp9k/H90VnYTAQDtkwlm7/5FMBAJ122Bg4YqLChVKSe4rgPOIByMHwMl08FQkKUkQEmAGnu2FHzbgm9qPBTj6+6rW61QP/kjX4GLSbReNW8ULYH3h6V+fqMhBJzm3sC+sMm5SHtZw3ck9pj1G48AY0DvJI/qkqeB+hw7rW84qtbAxWYX/dDfNB6BHPfEYQ8CrwRxh7Usm2QEPN98OLUrWLrH5ilf0NBq6vRB9vlUxS+VUoUobV8A1+WxTlWlKxYgy7fubP4NoyQVYSrJwpBY7NrOd5Sz8qxvwz0fB2ZwQdClvPfhsiDyJBQs2QcUUCJZgr6QkaTzV+iveNsRyT/K8y04dNF/YCyN1X+2CfY9kbVqnPl/6W1NZMZuLC7BaWUxO0rbvtqjG7exFms2WIfR8H70Uz/V2DchUeF6bsnIu6Npl0uKBXs50wmgZk3Ij5hP0cGZE+bHkSZT5i/Q= x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DS0PR11MB7529.namprd11.prod.outlook.com;PTR:;CAT:NONE;SFS:(13230025)(376002)(346002)(136003)(366004)(39860400002)(396003)(451199018)(33656002)(2906002)(7696005)(54906003)(478600001)(316002)(71200400001)(83380400001)(66446008)(26005)(38070700005)(66476007)(186003)(9686003)(6916009)(76116006)(4326008)(6506007)(66556008)(53546011)(7416002)(8936002)(5660300002)(52536014)(55016003)(64756008)(86362001)(8676002)(41300700001)(66946007)(82960400001)(38100700002)(122000001)(13296009);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?utf-8?B?WUtHQkljRGUzQWU4a3VzMGhLeVIxZ095RWVTQUxRVXBNKzZDbW1uanhZdmQ2?= =?utf-8?B?dGsxS3dpN2swczlRcmlVYWZzaHJQYzZuVkZwTkNYWGlNWnBCK0pqeHNTY2M1?= =?utf-8?B?aFllc09sd3k0OCswU3VaTXR2VFhzVXM5UzUrVTNPRFdhRmZXR3dsczVUK0NK?= =?utf-8?B?akNSM3huaDhCS0xnRTRDM3c4R0JYZnVibVpFRDNWSXRrMEVPcUdnZmJLOFBk?= =?utf-8?B?Z2dNMUdSZ29SRXdOWkNDVmhLcmVSd0x2R1R1QzVoQTFPRHFPVzhyajJ5dGxG?= =?utf-8?B?TjdRN0lEbGpiMm94TWhBOWlJZnNpWGFKQytWK0g1MVRNbjhGV0IxdHJRL3V5?= =?utf-8?B?dDNUSGc3ajFsZC90dUVhTGFaWURiTlI5cEEyZ3hkQUlWbGQvVldGUUJGcmN3?= =?utf-8?B?YWY3M3FlSGtTaWQ3eXV6Y1hSZlhSNGVmWmhWeVZxaXpNUlN3a25wWXlNNGFl?= =?utf-8?B?ZkJhcVJpK1pJWnRpUER4Z2NFUzNDSXZPT2JUeHRIdnF3L2ZEVVNtd0U1Yjdp?= =?utf-8?B?anpSVHhlZ3UxaHJTNmJ3M0p5SnNVOEZMMW5RRUpQbDZYc09xaEVyWkVwV1hR?= =?utf-8?B?Z0hxVDVWSXE3VGJGMFR0SXluWlQ0QjE4dDh3WFd1eGYxS1dZMFM0SEMxMEps?= =?utf-8?B?eXpJNmIyOW1qN0crVHpNSUNHMDUwT2pSeFAvd2FSVWNobXdRcVNGcXA3cEtl?= =?utf-8?B?eEdmczFIenpuZGkyclNKS1JDemhYa2pzWFlTR2dMUWVhYU5nMmJXS2ptUFJz?= =?utf-8?B?VzMzRHdRVUJaeTNKeHRhL3NCeHhwYjZvZzRycWFSY2FucklLekh6UTFDZ0Zs?= =?utf-8?B?TWJDZm5QSXoxZWlTL0dJK3cxSVowZ2U3Nk9GUzZOa1JRS2VVODZYQnVIZzlZ?= =?utf-8?B?Vlc5TjR5RWc3ME5WM2RpZlF0WThNeldyZTNtcHQrazZmekhONWp3STdIQmk1?= =?utf-8?B?UmlDclRReEM0TnNySXNHendjNWhSdzFTdFl6Mmw2WEpDVCtnUHZYcXd3V3hM?= =?utf-8?B?TjFSSWZ1NHBZcC9jZEt5YkVQcy9RUjhBWFh2d01MU3E0ZzF1WGdiZmpNSmJO?= =?utf-8?B?UytwQ1B4NExnRjF0MmhLTVU4QnY0UXpFbExmS1IzeXVqT3JJcllCeWdMRGsv?= =?utf-8?B?YVRFV0pFZjhtWEJueHkrZE5PZy9yTFIvQkhmTERocm4zTTQ3S0szcE1kTGZk?= =?utf-8?B?SnEzVDY1dXljeXlhQnI3SlUvNUpFT0VxckRiaVpJeENtUDh2UThGZW9ISmY3?= =?utf-8?B?Q3djRFRLTVQ4WmkyWHJFR1VmUktxWlIzSWJ4Um9VU3dtSGVOQzZPd01MQlZM?= =?utf-8?B?MExxUmU0aU9xaVpLcjM0WGJqaGtldjhMajNnT1VGejduc1ZKN01jQnBReVZT?= =?utf-8?B?M1B3SzE1OEFoZ2NYZmxPUmFtellqV2dLNzl5L21NeWQwOG4zTEJhWnR5ays5?= =?utf-8?B?bEg0Qm93T2c0TkJMd2YxRitpWjdLNDljVXZFY0J2WWtSelJUU2VFT3Z6cHUz?= =?utf-8?B?WE1aeXRXUmZ4Ynk4U0pxUmFVT3JYM3U3ZWV4UkIvcHgvdnVPdjYrTVpxNDZs?= =?utf-8?B?YkdjTGRYRi9vNmdXN0dhN3JPWmlnaUJhNldLbllUMmdFVlFTRlI3QlB6WjNn?= =?utf-8?B?TzlxaUIrZUF2V1d3dHo3ZGxnQlFZYU0vM3dUNE5heFYvRHhsSW9obFprdlRj?= =?utf-8?B?WGJKSGU1UkswUG55OXBkZ0NZYkNhRnZEL2ZhVHorUVU4YXBYZkc5dWpDVG5O?= =?utf-8?B?SndYazB5NHVlOS9wTVFZNzFCZFY0bUsvQkRGRUNLM28yZkE0OTFkaHk3QmJV?= =?utf-8?B?eUlHOS9mUUFjNGdhWndqUUZvRjJiUE0xdWtpUHNPYjIrWWVaaE0xREtpT3du?= =?utf-8?B?dFJ5RlFTTGU1OTE3RkFXMkxnNkZpZGF5NVlnMWR6Wm9TMnFpVUNUVlZwcHpR?= =?utf-8?B?emo4Szc3TlNIazgzL2RLMElFZzFuenpGVVY5M0FwRGxPZ2tmdkhYU25VVFpS?= =?utf-8?B?WGNZWS83V2NEcHlnTFduNG9SOUlBZUU2c01vbE1JbWJBQlBKQi9GcXppNUxi?= =?utf-8?B?RDZwUVZDYzhtS1UxUXQxOGNrbVhjU1RyTk9ZNXJvd0lHY0pDelQ1bHlWeEZs?= =?utf-8?Q?0Zf3BbeM+0ytE+s4tevFKMXyo?= Content-Type: text/plain; charset="utf-8" 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: a3da16cd-c622-4584-1658-08db04e8eef7 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Feb 2023 06:44:29.0526 (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: 6eijlNf5EWoWg/6sEvHiUW4vOSUPdIqglcDgBT3Crz4aXBL2NXvqEJEiArMmzj+TQCehFSV+JOnZ8xJJJcHHsQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH0PR11MB5807 X-OriginatorOrg: intel.com Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org > -----Original Message----- > From: Alex Williamson > Sent: Thursday, February 2, 2023 12:15 PM > To: Liu, Yi L > Cc: Matthew Rosato ; pbonzini@redhat.com; > jgg@nvidia.com; cohuck@redhat.com; farman@linux.ibm.com; > pmorel@linux.ibm.com; borntraeger@linux.ibm.com; > frankja@linux.ibm.com; imbrenda@linux.ibm.com; david@redhat.com; > akrowiak@linux.ibm.com; jjherne@linux.ibm.com; pasic@linux.ibm.com; > zhenyuw@linux.intel.com; Wang, Zhi A ; > Christopherson,, Sean ; Tian, Kevin > ; linux-s390@vger.kernel.org; kvm@vger.kernel.org; > intel-gvt-dev@lists.freedesktop.org; intel-gfx@lists.freedesktop.org; lin= ux- > kernel@vger.kernel.org > Subject: Re: [PATCH v2] vfio: fix deadlock between group lock and kvm lock >=20 > On Thu, 2 Feb 2023 03:46:59 +0000 > "Liu, Yi L" wrote: >=20 > > > From: Alex Williamson > > > Sent: Thursday, February 2, 2023 7:28 AM > > > > > > On Wed, 1 Feb 2023 14:20:10 -0500 > > > Matthew Rosato wrote: > > > > > > > After 51cdc8bc120e, we have another deadlock scenario between the > > > > kvm->lock and the vfio group_lock with two different codepaths > acquiring > > > > the locks in different order. Specifically in vfio_open_device, vf= io > > > > holds the vfio group_lock when issuing device->ops->open_device but > > > some > > > > drivers (like vfio-ap) need to acquire kvm->lock during their > open_device > > > > routine; Meanwhile, kvm_vfio_release will acquire the kvm->lock fi= rst > > > > before calling vfio_file_set_kvm which will acquire the vfio group_= lock. > > > > > > > > To resolve this, let's remove the need for the vfio group_lock from= the > > > > kvm_vfio_release codepath. This is done by introducing a new > spinlock to > > > > protect modifications to the vfio group kvm pointer, and acquiring a > kvm > > > > ref from within vfio while holding this spinlock, with the referenc= e held > > > > until the last close for the device in question. > > > > > > > > Fixes: 51cdc8bc120e ("kvm/vfio: Fix potential deadlock on vfio > group_lock") > > > > Reported-by: Anthony Krowiak > > > > Suggested-by: Jason Gunthorpe > > > > Signed-off-by: Matthew Rosato > > > > --- > > > > Changes from v1: > > > > * use spin_lock instead of spin_lock_irqsave (Jason) > > > > * clear device->kvm_put as part of vfio_kvm_put_kvm (Yi) > > > > * Re-arrange code to avoid referencing the group contents from > within > > > > vfio_main (Kevin) which meant moving most of the code in this pat= ch > > > > to group.c along with getting/dropping of the dev_set lock > > > > --- > > > > drivers/vfio/group.c | 90 > > > +++++++++++++++++++++++++++++++++++++--- > > > > drivers/vfio/vfio.h | 1 + > > > > drivers/vfio/vfio_main.c | 11 ++--- > > > > include/linux/vfio.h | 2 +- > > > > 4 files changed, 91 insertions(+), 13 deletions(-) > > > > > > > > diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c > > > > index bb24b2f0271e..52f434861294 100644 > > > > --- a/drivers/vfio/group.c > > > > +++ b/drivers/vfio/group.c > > > > @@ -13,6 +13,9 @@ > > > > #include > > > > #include > > > > #include > > > > +#ifdef CONFIG_HAVE_KVM > > > > +#include > > > > +#endif > > > > #include "vfio.h" > > > > > > > > static struct vfio { > > > > @@ -154,6 +157,55 @@ static int > vfio_group_ioctl_set_container(struct > > > vfio_group *group, > > > > return ret; > > > > } > > > > > > > > +#ifdef CONFIG_HAVE_KVM > > > > +static bool vfio_kvm_get_kvm_safe(struct vfio_device *device, > struct > > > kvm *kvm) > > > > > > I'm tempted to name these vfio_device_get_kvm_safe() and only pass > the > > > vfio_device, where of course we can get the kvm pointer from the > group > > > internally. > > > > > > > +{ > > > > + void (*pfn)(struct kvm *kvm); > > > > + bool (*fn)(struct kvm *kvm); > > > > + bool ret; > > > > + > > > > > > We should assert_lockdep_held(&device->dev_set->lock) in both of > these > > > since that seems to be what's protecting device->kvm and > > > device->put_kvm. > > > > > > If we change as above to get the kvm pointer from the group within th= is > > > function, we can also move the kvm_ref_lock here, which seems to > > > simplify the caller quite a bit. > > > > > > > + pfn =3D symbol_get(kvm_put_kvm); > > > > + if (WARN_ON(!pfn)) > > > > + return false; > > > > + > > > > + fn =3D symbol_get(kvm_get_kvm_safe); > > > > + if (WARN_ON(!fn)) { > > > > + symbol_put(kvm_put_kvm); > > > > + return false; > > > > + } > > > > + > > > > + ret =3D fn(kvm); > > > > + if (ret) > > > > + device->put_kvm =3D pfn; > > > > + else > > > > + symbol_put(kvm_put_kvm); > > > > + > > > > + symbol_put(kvm_get_kvm_safe); > > > > + > > > > + return ret; > > > > +} > > > > + > > > > +static void vfio_kvm_put_kvm(struct vfio_device *device) > > > > +{ > > > > + if (WARN_ON(!device->kvm || !device->put_kvm)) > > > > + return; > > > > > > It simplifies the caller if we can use this even in the !device->kvm > > > case. > > > > > > > + > > > > + device->put_kvm(device->kvm); > > > > + device->put_kvm =3D NULL; > > > > + symbol_put(kvm_put_kvm); > > > > +} > > > > + > > > > +#else > > > > +static bool vfio_kvm_get_kvm_safe(struct vfio_device *device, > struct > > > kvm *kvm) > > > > +{ > > > > + return false; > > > > +} > > > > + > > > > +static void vfio_kvm_put_kvm(struct vfio_device *device) > > > > +{ > > > > +} > > > > +#endif > > > > + > > > > static int vfio_device_group_open(struct vfio_device *device) > > > > { > > > > int ret; > > > > @@ -164,14 +216,32 @@ static int vfio_device_group_open(struct > > > vfio_device *device) > > > > goto out_unlock; > > > > } > > > > > > > > + mutex_lock(&device->dev_set->lock); > > > > + > > > > /* > > > > - * Here we pass the KVM pointer with the group under the lock. If > > > the > > > > - * device driver will use it, it must obtain a reference and rele= ase it > > > > - * during close_device. > > > > + * Before the first device open, get the KVM pointer currently > > > > + * associated with the group (if there is one) and obtain a refer= ence > > > > + * now that will be held until the open_count reaches 0 again. S= ave > > > > + * the pointer in the device for use by drivers. > > > > */ > > > > + if (device->open_count =3D=3D 0) { > > > > + spin_lock(&device->group->kvm_ref_lock); > > > > + if (device->group->kvm && > > > > + vfio_kvm_get_kvm_safe(device, device->group->kvm)) > > > > + device->kvm =3D device->group->kvm; > > > > + spin_unlock(&device->group->kvm_ref_lock); > > > > + } > > > > + > > > > ret =3D vfio_device_open(device, device->group->iommufd, > > > > device->group->kvm); > > > > > > We're using device->group->kvm outside of kvm_ref_lock here, it > should > > > be using device->kvm. > > > > Existing code set device->kvm in the vfio_device_first_open() which is > > called by vfio_device_open(). After above change, seems not necessary > > to pass kvm pointer into the call chain. Isn't it? >=20 > Yes, we can get it from the device. I didn't check how much this > bloats the patch though. As a fix, it might make sense to save that > refactoring for a follow-on patch. Thanks, 65 lines diff file. =F0=9F=98=8A follow-on path works well. diff --git a/drivers/vfio/group.c b/drivers/vfio/group.c index bb24b2f0271e..9e04e55c838f 100644 --- a/drivers/vfio/group.c +++ b/drivers/vfio/group.c @@ -169,8 +169,7 @@ static int vfio_device_group_open(struct vfio_device *d= evice) * device driver will use it, it must obtain a reference and release it * during close_device. */ - ret =3D vfio_device_open(device, device->group->iommufd, - device->group->kvm); + ret =3D vfio_device_open(device, device->group->iommufd); =20 out_unlock: mutex_unlock(&device->group->group_lock); diff --git a/drivers/vfio/vfio.h b/drivers/vfio/vfio.h index f8219a438bfb..4ece6cb4cf2e 100644 --- a/drivers/vfio/vfio.h +++ b/drivers/vfio/vfio.h @@ -19,7 +19,7 @@ struct vfio_container; void vfio_device_put_registration(struct vfio_device *device); bool vfio_device_try_get_registration(struct vfio_device *device); int vfio_device_open(struct vfio_device *device, - struct iommufd_ctx *iommufd, struct kvm *kvm); + struct iommufd_ctx *iommufd); void vfio_device_close(struct vfio_device *device, struct iommufd_ctx *iommufd); =20 diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c index 5177bb061b17..45a7d6d38e2e 100644 --- a/drivers/vfio/vfio_main.c +++ b/drivers/vfio/vfio_main.c @@ -345,7 +345,7 @@ static bool vfio_assert_device_open(struct vfio_device = *device) } =20 static int vfio_device_first_open(struct vfio_device *device, - struct iommufd_ctx *iommufd, struct kvm *kvm) + struct iommufd_ctx *iommufd) { int ret; =20 @@ -361,7 +361,6 @@ static int vfio_device_first_open(struct vfio_device *d= evice, if (ret) goto err_module_put; =20 - device->kvm =3D kvm; if (device->ops->open_device) { ret =3D device->ops->open_device(device); if (ret) @@ -396,14 +395,14 @@ static void vfio_device_last_close(struct vfio_device= *device, } =20 int vfio_device_open(struct vfio_device *device, - struct iommufd_ctx *iommufd, struct kvm *kvm) + struct iommufd_ctx *iommufd) { int ret =3D 0; =20 mutex_lock(&device->dev_set->lock); device->open_count++; if (device->open_count =3D=3D 1) { - ret =3D vfio_device_first_open(device, iommufd, kvm); + ret =3D vfio_device_first_open(device, iommufd); if (ret) device->open_count--; }