[PATCH] vfio: Decrement open_count before close_device()

[Anthony DeRossi <ajderossi@gmail.com>]

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@gmail.com>
---
 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