Date:   Tue, 31 Jan 2023 11:12:47 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     Matthew Rosato <mjrosato@linux.ibm.com>
Cc:     Anthony Krowiak <akrowiak@linux.ibm.com>,
        Yi Liu <yi.l.liu@intel.com>, alex.williamson@redhat.com,
        pbonzini@redhat.com, kevin.tian@intel.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, jjherne@linux.ibm.com,
        pasic@linux.ibm.com, zhenyuw@linux.intel.com, zhi.a.wang@intel.com,
        seanjc@google.com, 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] kvm/vfio: Fix potential deadlock on vfio group_lock
Message-ID: <Y9kv70l4rES+9mg+@nvidia.com>
References: <20230120150528.471752-1-yi.l.liu@intel.com>
 <67ec09bf-cb24-34e3-6ec4-1ae87b0738bd@linux.ibm.com>
 <Y9km+xezgD4ovjDX@nvidia.com>
 <b0ed98f6-3586-3151-47e7-9ec86d6c716d@linux.ibm.com>
 <Y9kqNnJ5ayCQSW9z@nvidia.com>
 <03ff0663-da37-8907-53ec-15234fd7dcad@linux.ibm.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <03ff0663-da37-8907-53ec-15234fd7dcad@linux.ibm.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?5OEEPUBoIV5KR/m/fSiLxtUio/VhAOajZSwtx8a+KpThLab5cvobm0fL+cd6?=
 =?us-ascii?Q?p2BcUiESWNv6QKyCLpO89qfR5cx6CdmvBgo3M68GblvThxkuAiiKiOiYjF5h?=
 =?us-ascii?Q?oLmA0SSOk1h7YoV6oSpGPn6/1kYapcGJImqVikfiEPg/eU8hXf29qFSHTAcT?=
 =?us-ascii?Q?fg506myM5rMDG3zqwZoUOjXZLICvw84MOqDMpRMAvaHM3WKvcYpE2X1sDkTI?=
 =?us-ascii?Q?cBccbF3Qu3YMVWtSkCPbNWf6MazLKutG1nu1rQgYAK1Xc9/nh409xXFuWMG1?=
 =?us-ascii?Q?bM3TJb7fCAIPTtiZSAOd7/lFyv1ZKZlqFHtUOySblvGIx8HYe6UItzqGIVQc?=
 =?us-ascii?Q?skg3cOgWmXHV75mZLPjmqtVeT6qG4u1BeGWIRSF9VM7vHO77c5SEk/rwvEoO?=
 =?us-ascii?Q?3dQfxy+pAFR53iybsvdmrYVl91sq92s0iXFtd5X3yBMUz+fraPNiJIJ0ZEEn?=
 =?us-ascii?Q?i9hXH0DMiiviyLCo5Roz9pgu0Ca4GjiXG9FekZxxTiJanxmNobWX3V8IKcTX?=
 =?us-ascii?Q?PHw9d2KepxPBDP/x2jRe4OyA3UcZrjzRTUzW6BiymekDZ0E0fpG/sg+k/YT7?=
 =?us-ascii?Q?qJgTzNCefHb+oEnOzgLjgY4kie1VyDcl3ctS24EP+ds2sLw8GnvrIm61h1yy?=
 =?us-ascii?Q?6eMX6glYp2N3GhJaJwHxHD8haB0ljQ0iuGDAOCdTlfp9LtPFqIvqXnBcUxGH?=
 =?us-ascii?Q?YB4U5VD3ooNaVsaIruEeJaHkvX8R4J+T4zQdWWY9EAEZTIt815RxhoPF8F8C?=
 =?us-ascii?Q?fstp8F/fSwQSk5qqNuGbb1UO1TuHhOsEyeQO4ul7vZ5Hf99SQLuPwOQvDrbY?=
 =?us-ascii?Q?EzbZxNTOpXulyWaAmyl/NGMnAGo3+3rSENRI1IRiZNIdDK9qei5ZwbkHRUi1?=
 =?us-ascii?Q?p/rCkDDdJ2lZsZ4pDc2lrDdZPsShgsGshAXkUehFKQyACvgWFEfziFzH/LRp?=
 =?us-ascii?Q?6KrujjbwOOjuYzsJ3V25DCC0ixvV6A5mvgJmMCPB4AdRnN4l8PEsN8Qi58Tm?=
 =?us-ascii?Q?WLRi1Osn6WmNHj9DjeR+k3+W177Ft0CejBz0NhaN5ARBpWt6QLbh6qZ118+u?=
 =?us-ascii?Q?tKZgALBp/2avHqOIySCTuN9tpA2XCkNM6cP7Pc6ejqs3xsYRidx4/gnWx1DW?=
 =?us-ascii?Q?/G+n//XmNOrnBpMMfAZHBQoSzQH2MjjnMHiv6Sd6DygU6N6HqNKPw0iCyrgJ?=
 =?us-ascii?Q?h1f47UtrEAskOpn8djBM6xYF5hRQKR2MJXn0tdxl+2C8QSmHL+gTOXyIGNDR?=
 =?us-ascii?Q?y8frpdrNHLHu+7O6YPOHmYzbOIEaV0MPVYGkew6jCbaJppY21q3tfIbRPSre?=
 =?us-ascii?Q?2MkPOhDJCZtttmpGGFu1+Li8A1X0t1v5cytFe0fbPZ8Mu6yMegGZDVhm6Q/i?=
 =?us-ascii?Q?IfwO4mnwoBxTiDY3GUNzID9gNF0hr36F7PRO4GQWmHdroCmWR8k0BsKjN+1t?=
 =?us-ascii?Q?go5LVZ8bQCHrse4rTxnM/RSus6MClN4JhtQRSuHJSeJtQ/hHoPqN5huF9uQk?=
 =?us-ascii?Q?gdfuXSPdOuPjRk+kdZ8V/WJ8031RQRRbuKRwrzIBqvFcQVanCbmcWCPsUbol?=
 =?us-ascii?Q?n4Se+hDBVgrugqmm7w4K8NYqsdYw/iLlOZqXktDB?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dac2cb6b-6b45-492a-167e-08db039d9e76
X-MS-Exchange-CrossTenant-AuthSource: LV2PR12MB5869.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 31 Jan 2023 15:12:50.8219
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: Q/D+IbDMlh6+AYJxwzbu1l9+r4UDzT+LiJhMVAdjtrgCwTkerS+VDi3NHYeMaujy
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB6089
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Tue, Jan 31, 2023 at 10:00:14AM -0500, Matthew Rosato wrote:
> On 1/31/23 9:48 AM, Jason Gunthorpe wrote:
> > On Tue, Jan 31, 2023 at 09:46:18AM -0500, Anthony Krowiak wrote:
> > 
> >>> Maybe you should split that lock and have a dedicated apcb lock?
> >>
> >> I don't think that would suffice for taking the vCPUs out of SIE.
> > 
> > Then I think we have to keep this patch and also do Matthew's patch to
> > keep kvm refs inside vfio as well.
> > 
> 
> I don't think keeping kvm refs inside vfio solves this issue though
> -- Even if we handle the kvm_put_kvm asynchronously within vfio as
> previously proposed, kvm_vfio_release will eventually get called and
> it gets called with the kvm->lock already held, then proceeds to
> call vfio_file_set_kvm which gets the group->lock.  That order
> conflicts with the hierarchy used by the driver during open_device
> of vfio->group_lock ... kvm->lock.

The group lock is held by vfio_file_set_kvm() only because we don't
have a refcount and we have to hold it across the open call to keep
the pointer alive.

With proper refcounting you'd split this to a spinlock and hold it
only while obtaining the get ref for the open thread.

Jason