On 1/20/23 10:08 AM, Liu, Yi L wrote:
>> From: Liu, Yi L <yi.l.liu@xxxxxxxxx>
>> Sent: Friday, January 20, 2023 11:05 PM
>>
>> Currently it is possible that the final put of a KVM reference comes from
>> vfio during its device close operation. This occurs while the vfio group
>> lock is held; however, if the vfio device is still in the kvm device list,
>> then the following call chain could result in a deadlock:
>>
>> VFIO holds group->group_lock/group_rwsem
>> -> kvm_put_kvm
>> -> kvm_destroy_vm
>> -> kvm_destroy_devices
>> -> kvm_vfio_destroy
>> -> kvm_vfio_file_set_kvm
>> -> vfio_file_set_kvm
>> -> try to hold group->group_lock/group_rwsem
>>
>> The key function is the kvm_destroy_devices() which triggers destroy cb
>> of kvm_device_ops. It calls back to vfio and try to hold group_lock. So
>> if this path doesn't call back to vfio, this dead lock would be fixed.
>> Actually, there is a way for it. KVM provides another point to free the
>> kvm-vfio device which is the point when the device file descriptor is
>> closed. This can be achieved by providing the release cb instead of the
>> destroy cb. Also rename kvm_vfio_destroy() to be kvm_vfio_release().
>>
>> /*
>> * Destroy is responsible for freeing dev.
>> *
>> * Destroy may be called before or after destructors are called
>> * on emulated I/O regions, depending on whether a reference is
>> * held by a vcpu or other kvm component that gets destroyed
>> * after the emulated I/O.
>> */
>> void (*destroy)(struct kvm_device *dev);
>>
>> /*
>> * Release is an alternative method to free the device. It is
>> * called when the device file descriptor is closed. Once
>> * release is called, the destroy method will not be called
>> * anymore as the device is removed from the device list of
>> * the VM. kvm->lock is held.
>> */
>> void (*release)(struct kvm_device *dev);
>>
>> Fixes: 421cfe6596f6 ("vfio: remove VFIO_GROUP_NOTIFY_SET_KVM")
>> Reported-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
>> Suggested-by: Kevin Tian <kevin.tian@xxxxxxxxx>
>> Reviewed-by: Jason Gunthorpe <jgg@xxxxxxxxxx>
>> Signed-off-by: Yi Liu <yi.l.liu@xxxxxxxxx>
>
> More background can be found in Mathew's work.
> https://lore.kernel.org/kvm/20230114000351.115444-1-mjrosato@xxxxxxxxxxxxx/T/#u
>
Thanks Yi.
Reviewed-by: Matthew Rosato <mjrosato@xxxxxxxxxxxxx>
One small nit: There is a comment at the very end of kvm_vfio_release on the kfree(dev) that still references .destroy, this should be updated to .release
