The implementation of close_device() for vfio-pci inspects the
open_count of every device in the device set to determine whether a
reset is needed. Unless open_count is decremented before invoking
close_device(), the device set will always contain a device with
open_count > 0, effectively disabling the reset logic.
After commit 2cd8b14aaa66 ("vfio/pci: Move to the device set
infrastructure"), failure to create a new file for a device would cause
the reset to be skipped when closing the device in the error path.
After commit eadd86f835c6 ("vfio: Remove calls to
vfio_group_add_container_user()"), releasing a device would always skip
the reset.
Failing to reset the device leaves it in an unknown state, potentially
causing errors when it is bound to a different driver.
This issue was observed with a Radeon RX Vega 56 [1002:687f] (rev c3)
assigned to a Windows guest. After shutting down the guest, unbinding
the device from vfio-pci, and binding the device to amdgpu:
[ 548.007102] [drm:psp_hw_start [amdgpu]] *ERROR* PSP create ring failed!
[ 548.027174] [drm:psp_hw_init [amdgpu]] *ERROR* PSP firmware loading failed
[ 548.027242] [drm:amdgpu_device_fw_loading [amdgpu]] *ERROR* hw_init of IP block <psp> failed -22
[ 548.027306] amdgpu 0000:0a:00.0: amdgpu: amdgpu_device_ip_init failed
[ 548.027308] amdgpu 0000:0a:00.0: amdgpu: Fatal error during GPU init
Fixes: 2cd8b14aaa66 ("vfio/pci: Move to the device set infrastructure")
Fixes: eadd86f835c6 ("vfio: Remove calls to vfio_group_add_container_user()")
Signed-off-by: Anthony DeRossi <ajderossi@xxxxxxxxx>
---
drivers/vfio/vfio_main.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
index 2d168793d4e1..7c3f1734fb35 100644
--- a/drivers/vfio/vfio_main.c
+++ b/drivers/vfio/vfio_main.c
@@ -763,8 +763,10 @@ static struct file *vfio_device_open(struct vfio_device *device)
if (device->ops->open_device) {
ret = device->ops->open_device(device);
- if (ret)
- goto err_undo_count;
+ if (ret) {
+ device->open_count--;
+ goto err_unlock;
+ }
}
vfio_device_container_register(device);
mutex_unlock(&device->group->group_lock);
@@ -801,14 +803,13 @@ static struct file *vfio_device_open(struct vfio_device *device)
err_close_device:
mutex_lock(&device->dev_set->lock);
mutex_lock(&device->group->group_lock);
- if (device->open_count == 1 && device->ops->close_device) {
+ if (!--device->open_count && device->ops->close_device) {
device->ops->close_device(device);
vfio_device_container_unregister(device);
}
-err_undo_count:
+err_unlock:
mutex_unlock(&device->group->group_lock);
- device->open_count--;
if (device->open_count == 0 && device->kvm)
device->kvm = NULL;
mutex_unlock(&device->dev_set->lock);
@@ -1017,12 +1018,11 @@ static int vfio_device_fops_release(struct inode *inode, struct file *filep)
mutex_lock(&device->dev_set->lock);
vfio_assert_device_open(device);
mutex_lock(&device->group->group_lock);
- if (device->open_count == 1 && device->ops->close_device)
+ if (!--device->open_count && device->ops->close_device)
device->ops->close_device(device);
vfio_device_container_unregister(device);
mutex_unlock(&device->group->group_lock);
- device->open_count--;
if (device->open_count == 0)
device->kvm = NULL;
mutex_unlock(&device->dev_set->lock);
--
2.37.4
