[PATCH v6 15/31] vfio/pci: Route every reset point through a try-reset helper

[mhonap]

From: Manish Honap <mhonap@xxxxxxxxxx>

A provider such as vfio-cxl must reset its device itself: a CXL Type-2
function must not take an FLR, and its reset changes what the provider
lets the fault path map. vfio-pci-core resets a function at five points:
enable, close, VFIO_DEVICE_RESET, a virtualized PCIe or AF FLR, and the
guest's CXL Reset doorbell in the virtualized CXL DVSEC.

Add a mandatory reset op to the provider ops, and refuse to register ops
without it. Add vfio_pci_try_reset_function(), which keeps the trylock
of pci_try_reset_function(): it returns -EAGAIN when the device lock is
contended, and otherwise runs the provider's reset with the device lock
and memory_lock held. The close path keeps its bridge and device trylock
and calls a locked variant. vfio_pci_try_reset_function() saves the PCI
state first, as pci_try_reset_function() does, for the provider's reset
to restore. The locked variant does not save: on close the reset must
restore the open-time state that vfio_pci_core_disable() has just
loaded. A failed save is returned and the reset is not attempted.

The enable path still fails only on -EAGAIN. A provider checks at bind
that it can reset the device, so reset_works is set for a provider
device whatever the result of the first reset.

The CXL Reset doorbell now runs the reset and reports its result in
Status2: Reset Error on failure, Reset Done on success.

Suggested-by: Alex Williamson <alex@xxxxxxxxxxx>
Assisted-by: LLM
Signed-off-by: Manish Honap <mhonap@xxxxxxxxxx>
---
 drivers/vfio/pci/vfio_pci_config.c | 19 ++++++--
 drivers/vfio/pci/vfio_pci_core.c   | 71 ++++++++++++++++++++++++++++--
 drivers/vfio/pci/vfio_pci_priv.h   |  1 +
 include/linux/vfio_pci_core.h      |  7 +++
 4 files changed, 90 insertions(+), 8 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c
index 1fc463540211..6fbece95cb9f 100644
--- a/drivers/vfio/pci/vfio_pci_config.c
+++ b/drivers/vfio/pci/vfio_pci_config.c
@@ -910,7 +910,7 @@ static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos,
 		if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) {
 			vfio_pci_zap_and_down_write_memory_lock(vdev);
 			vfio_pci_dma_buf_move(vdev, true);
-			pci_try_reset_function(vdev->pdev);
+			vfio_pci_try_reset_function(vdev);
 			if (__vfio_pci_memory_enabled(vdev))
 				vfio_pci_dma_buf_move(vdev, false);
 			up_write(&vdev->memory_lock);
@@ -995,7 +995,7 @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos,
 		if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP)) {
 			vfio_pci_zap_and_down_write_memory_lock(vdev);
 			vfio_pci_dma_buf_move(vdev, true);
-			pci_try_reset_function(vdev->pdev);
+			vfio_pci_try_reset_function(vdev);
 			if (__vfio_pci_memory_enabled(vdev))
 				vfio_pci_dma_buf_move(vdev, false);
 			up_write(&vdev->memory_lock);
@@ -1786,9 +1786,20 @@ static int vfio_cxl_dvsec_write(struct vfio_pci_core_device *vdev, int pos,
 	if (!(ctrl2 & PCI_DVSEC_CXL_DISABLE_CACHING))
 		status2 &= ~PCI_DVSEC_CXL_CACHE_INV;
 	if (ctrl2 & PCI_DVSEC_CXL_INIT_CXL_RST) {
+		int ret;
+
 		ctrl2 &= ~PCI_DVSEC_CXL_INIT_CXL_RST;
-		status2 &= ~PCI_DVSEC_CXL_RST_ERR;
-		status2 |= PCI_DVSEC_CXL_RST_DONE;
+
+		/* Run the real reset here and report its result in Status2. */
+		vfio_pci_zap_and_down_write_memory_lock(vdev);
+		vfio_pci_dma_buf_move(vdev, true);
+		ret = vfio_pci_try_reset_function(vdev);
+		if (__vfio_pci_memory_enabled(vdev))
+			vfio_pci_dma_buf_move(vdev, false);
+		up_write(&vdev->memory_lock);
+
+		status2 &= ~(PCI_DVSEC_CXL_RST_DONE | PCI_DVSEC_CXL_RST_ERR);
+		status2 |= ret ? PCI_DVSEC_CXL_RST_ERR : PCI_DVSEC_CXL_RST_DONE;
 	}
 
 	*pctrl2 = cpu_to_le16(ctrl2);
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 1e0ac28b7564..6676c561465b 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -605,6 +605,62 @@ static void vfio_pci_core_unmap_bars(struct vfio_pci_core_device *vdev)
 	}
 }
 
+/**
+ * vfio_pci_try_reset_function() - reset the function without blocking
+ * @vdev: vfio PCI device, with memory_lock held for write
+ *
+ * Reset through the PCI core, or let the provider reset a device it serves.
+ * The provider resets the device with its PCI device lock held, so take that
+ * lock here, without waiting, as pci_try_reset_function() does.
+ *
+ * Return: 0 on success, -EAGAIN when the device lock is contended, or the
+ * reset error. For a provider device a failed pci_save_state() is also
+ * returned, and the reset is not attempted: unlike pci_try_reset_function(),
+ * the provider's reset needs a valid saved state to restore.
+ */
+int vfio_pci_try_reset_function(struct vfio_pci_core_device *vdev)
+{
+	struct pci_dev *pdev = vdev->pdev;
+	int ret;
+
+	lockdep_assert_held_write(&vdev->memory_lock);
+
+	if (!vdev->cxl_ops)
+		return pci_try_reset_function(pdev);
+
+	if (!pci_dev_trylock(pdev))
+		return -EAGAIN;
+
+	/*
+	 * The provider restores PCI state from the saved copy, so save the
+	 * current state first, as pci_try_reset_function() does. The locked
+	 * variant on the close path must not: it resets with the state that
+	 * vfio_pci_core_disable() has just loaded.
+	 */
+	ret = pci_save_state(pdev);
+	if (!ret)
+		ret = vdev->cxl_ops->reset(vdev);
+	pci_dev_unlock(pdev);
+
+	return ret;
+}
+
+/*
+ * As vfio_pci_try_reset_function(), for a caller that holds the device lock.
+ * Only the close path calls this. It takes memory_lock with the device lock
+ * held, so user config access is blocked meanwhile: the order a sysfs reset
+ * uses. That cannot deadlock against a PCI_COMMAND write holding memory_lock
+ * because no user remains at close.
+ */
+static int vfio_pci_reset_function_locked(struct vfio_pci_core_device *vdev)
+{
+	if (!vdev->cxl_ops)
+		return __pci_reset_function_locked(vdev->pdev);
+
+	guard(rwsem_write)(&vdev->memory_lock);
+	return vdev->cxl_ops->reset(vdev);
+}
+
 static void vfio_pci_core_free_dev_regions(struct vfio_pci_core_device *vdev)
 {
 	for (int i = 0; i < vdev->num_regions; i++)
@@ -636,11 +692,14 @@ int vfio_pci_core_enable(struct vfio_pci_core_device *vdev)
 		goto out_power;
 
 	/* If reset fails because of the device lock, fail this path entirely */
-	ret = pci_try_reset_function(pdev);
+	down_write(&vdev->memory_lock);
+	ret = vfio_pci_try_reset_function(vdev);
+	up_write(&vdev->memory_lock);
 	if (ret == -EAGAIN)
 		goto out_disable_device;
 
-	vdev->reset_works = !ret;
+	/* A provider checks at bind that it can reset the device. */
+	vdev->reset_works = vdev->cxl_ops || !ret;
 	pci_save_state(pdev);
 	vdev->pci_saved_state = pci_store_saved_state(pdev);
 	if (!vdev->pci_saved_state)
@@ -846,7 +905,7 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev)
 		if (bridge && !pci_dev_trylock(bridge))
 			goto out_restore_state;
 		if (pci_dev_trylock(pdev)) {
-			if (!__pci_reset_function_locked(pdev))
+			if (!vfio_pci_reset_function_locked(vdev))
 				vdev->needs_reset = false;
 			pci_dev_unlock(pdev);
 		}
@@ -1541,7 +1600,7 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev,
 	vfio_pci_set_power_state(vdev, PCI_D0);
 
 	vfio_pci_dma_buf_move(vdev, true);
-	ret = pci_try_reset_function(vdev->pdev);
+	ret = vfio_pci_try_reset_function(vdev);
 	if (__vfio_pci_memory_enabled(vdev))
 		vfio_pci_dma_buf_move(vdev, false);
 	up_write(&vdev->memory_lock);
@@ -2397,6 +2456,10 @@ int vfio_pci_core_register_cxl_ops(const struct vfio_cxl_ops *ops)
 {
 	guard(rwsem_write)(&vfio_pci_cxl_ops_rwsem);
 
+	/* vfio-pci-core never resets a provider's device by itself. */
+	if (!ops->reset)
+		return -EINVAL;
+
 	if (vfio_pci_cxl_ops)
 		return -EBUSY;
 
diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
index 09a68c99b43c..dbede6e24fa9 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -80,6 +80,7 @@ int vfio_pci_set_power_state(struct vfio_pci_core_device *vdev,
 			     pci_power_t state);
 
 void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev);
+int vfio_pci_try_reset_function(struct vfio_pci_core_device *vdev);
 u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev);
 void vfio_pci_memory_unlock_and_restore(struct vfio_pci_core_device *vdev,
 					u16 cmd);
diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
index 991a3fcd84eb..cb9ba92b696a 100644
--- a/include/linux/vfio_pci_core.h
+++ b/include/linux/vfio_pci_core.h
@@ -87,6 +87,13 @@ struct vfio_cxl_ops {
 	 */
 	void	(*reset_prepare)(struct vfio_pci_core_device *vdev);
 	int	(*reset_done)(struct vfio_pci_core_device *vdev);
+	/*
+	 * Reset the function in place of the PCI core; mandatory. Called with
+	 * the PCI device lock and memory_lock (for write) held. The caller
+	 * provides the saved PCI state the reset restores: the try-reset path
+	 * saves it first, the close path keeps the open-time copy.
+	 */
+	int	(*reset)(struct vfio_pci_core_device *vdev);
 	/* Pinned per bound CXL device so vfio-cxl cannot unload under usage */
 	struct module *owner;
 };
-- 
2.53.0