From: Manish Honap <[email protected]> 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 <[email protected]> Assisted-by: LLM Signed-off-by: Manish Honap <[email protected]> --- 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
