[RFC PATCH 02/19] vfio/pci: Serialize generic device lifetime with recovery

[Shameer Kolothum]

vfio_pci_core_disable() frees vconfig while holding only the vfio
device_set mutex. The PCI error callbacks never take that one. They run
under the PCI device_lock instead, and vfio's close path does not hold
that. So a callback still running when close starts can walk into state
which is being freed.

Publish a device_open flag under recovery_lock. enable() clears it before
it touches the device, finish_enable() sets it once vfio_config_init()
has allocated vconfig, and prepare_close() clears it again before the
teardown frees vconfig. All three take recovery_lock for writing, so a
callback either gets there first and close waits for it, or it finds the
flag clear and does nothing. The access guards added later test the same
flag.

recovery_lock is not held across vfio_pci_core_disable(). A later patch
has error_detected() take it from under pci_bus_sem, and disable() gets
to pci_reset_bus(), which takes pci_bus_sem the other way round.

access_blocked is only ever set while device_open is set. Nothing sets it
without testing device_open first, and close clears access_blocked before
it clears device_open. If close left it set, nothing could clear it
afterwards.
The transaction which set it cannot clear it once device_open is gone,
and every path which refuses work on a blocked device would go on
refusing. Clear it before device_open so a lock-free reader never sees
it set on a device which is closed.

open() now refuses a disconnected device with -ENODEV. That is new.

Signed-off-by: Shameer Kolothum <skolothumtho@xxxxxxxxxx>
---
 drivers/vfio/pci/vfio_pci_core.c | 69 +++++++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 1 deletion(-)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index e0be5ddf7039..8de586e4bb73 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -591,10 +591,23 @@ static const struct dev_pm_ops vfio_pci_core_pm_ops = {
 int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
 {
 	struct pci_dev *pdev = vdev->pdev;
+	bool supported = vdev->pci_recovery_supported;
 	int ret;
 	u16 cmd;
 	u8 msix_pos;
 
+	if (supported) {
+		down_write(&vdev->recovery_lock);
+		if (pci_dev_is_disconnected(pdev)) {
+			up_write(&vdev->recovery_lock);
+			return -ENODEV;
+		}
+
+		vdev->pci_recovery_command_valid = false;
+		WRITE_ONCE(vdev->pci_recovery_device_open, false);
+		up_write(&vdev->recovery_lock);
+	}
+
 	if (!vdev->disable_idle_d3) {
 		ret = pm_runtime_resume_and_get(&pdev->dev);
 		if (ret < 0)
@@ -815,7 +828,40 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_disable);
 
-void vfio_pci_core_close_device(struct vfio_device *core_vdev)
+static void vfio_pci_core_prepare_close(struct vfio_pci_core_device *vdev)
+{
+	if (!vdev->pci_recovery_supported)
+		return;
+
+	down_write(&vdev->recovery_lock);
+	WRITE_ONCE(vdev->pci_recovery_enabled, false);
+	vdev->pci_recovery_command_valid = false;
+	/*
+	 * Clear access_blocked before device_open, so a lock-free reader
+	 * never sees it set on a device which is no longer open. A
+	 * transaction which is still running cannot clear it once
+	 * device_open is gone, and paths which refuse work on a blocked
+	 * device would then refuse it for good.
+	 */
+	WRITE_ONCE(vdev->pci_recovery_access_blocked, false);
+	WRITE_ONCE(vdev->pci_recovery_device_open, false);
+	WRITE_ONCE(vdev->pci_recovery_flags, 0);
+
+	/*
+	 * Publish the closing state and drop recovery_lock before any
+	 * teardown. Recovery is disabled and its state cleared, so
+	 * slot_reset() and resume() become no-ops and a later
+	 * error_detected() only follows the legacy notification path.
+	 * Holding the lock across vfio_pci_core_disable() protects nothing
+	 * and inverts the lock order. disable() reaches pci_reset_bus(),
+	 * which takes pci_bus_sem, while error_detected() takes
+	 * recovery_lock from under pci_bus_sem.
+	 */
+	up_write(&vdev->recovery_lock);
+	wake_up_all(&vdev->pci_recovery_wait);
+}
+
+static void vfio_pci_core_finish_close(struct vfio_device *core_vdev)
 {
 	struct vfio_pci_core_device *vdev =
 		container_of(core_vdev, struct vfio_pci_core_device, vdev);
@@ -838,6 +884,15 @@ void vfio_pci_core_close_device(struct vfio_device *core_vdev)
 	vfio_pci_eventfd_replace_locked(vdev, &vdev->req_trigger, NULL);
 	mutex_unlock(&vdev->igate);
 }
+
+void vfio_pci_core_close_device(struct vfio_device *core_vdev)
+{
+	struct vfio_pci_core_device *vdev =
+		container_of(core_vdev, struct vfio_pci_core_device, vdev);
+
+	vfio_pci_core_prepare_close(vdev);
+	vfio_pci_core_finish_close(core_vdev);
+}
 EXPORT_SYMBOL_GPL(vfio_pci_core_close_device);
 
 void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
@@ -852,6 +907,18 @@ void vfio_pci_core_finish_enable(struct vfio_pci_core_device *vdev)
 		vdev->sriov_pf_core_dev->vf_token->users++;
 		mutex_unlock(&vdev->sriov_pf_core_dev->vf_token->lock);
 	}
+
+	if (vdev->pci_recovery_supported) {
+		down_write(&vdev->recovery_lock);
+		WRITE_ONCE(vdev->pci_recovery_flags, 0);
+		vdev->pci_recovery_sequence = 0;
+		WRITE_ONCE(vdev->pci_recovery_enabled, false);
+		/* Close clears this too. Start unblocked either way. */
+		WRITE_ONCE(vdev->pci_recovery_access_blocked, false);
+		WRITE_ONCE(vdev->pci_recovery_device_open, true);
+		WRITE_ONCE(vdev->pci_recovery_rom_disable, false);
+		up_write(&vdev->recovery_lock);
+	}
 }
 EXPORT_SYMBOL_GPL(vfio_pci_core_finish_enable);
 
-- 
2.43.0