On Thu, 6 Oct 2022 08:37:09 -0300 Jason Gunthorpe <jgg@xxxxxxxxxx> wrote: > On Wed, Oct 05, 2022 at 04:03:56PM -0600, Alex Williamson wrote: > > We can't have a .remove callback that does nothing, this breaks > > removing the device while it's in use. Once we have the > > vfio_unregister_group_dev() fix below, we'll block until the device is > > unused, at which point vgpu->attached becomes false. Unless I'm > > missing something, I think we should also follow-up with a patch to > > remove that bogus warn-on branch, right? Thanks, > > Yes, looks right to me. > > I question all the logical arround attached, where is the locking? Zhenyu, Zhi, Kevin, Could someone please take a look at use of vgpu->attached in the GVT-g driver? It's use in intel_vgpu_remove() is bogus, the .release callback needs to use vfio_unregister_group_dev() to wait for the device to be unused. The WARN_ON/return here breaks all future use of the device. I assume @attached has something to do with the page table interface with KVM, but it all looks racy anyway. Also, whatever purpose vgpu->released served looks unnecessary now. Thanks, Alex
