From: Manish Honap <[email protected]> A CXL Type-2 function must not take an FLR: it resets the coherent CXL.mem state and corrupts the HDM decoder. The PCI core already reflects this, ordering cxl_reset ahead of flr in pci_reset_fn_methods[], so a function reset of a CXL device runs the DVSEC reset sequence rather than FLR.
Route the vfio function-reset points (VFIO_DEVICE_RESET and the virtualized PCIe/AF FLR writes) through a CXL reset op that runs cxl_reset_dvsec_sequence(). The sequence resets the function, always clearing device memory, and restores the HDM decoder and PCI config state, so it is a complete replacement for pci_try_reset_function() on a CXL device. The op runs under memory_lock and not the PCI device lock, so cxl_reset_dvsec_sequence() can take the device lock itself. Clear hdm_valid for the duration of the reset so a fault cannot insert a PFN into a decoder that is being torn down, and restore it once the sequence has put the decoder back. Assisted-by: LLM Signed-off-by: Manish Honap <[email protected]> --- drivers/vfio/pci/cxl/vfio_cxl_core.c | 41 +++++++++++++++ drivers/vfio/pci/vfio_pci_config.c | 49 +++++++++++++++--- drivers/vfio/pci/vfio_pci_core.c | 77 +++++++++++++++++++++++----- drivers/vfio/pci/vfio_pci_priv.h | 1 + include/linux/vfio_pci_core.h | 4 ++ 5 files changed, 152 insertions(+), 20 deletions(-) diff --git a/drivers/vfio/pci/cxl/vfio_cxl_core.c b/drivers/vfio/pci/cxl/vfio_cxl_core.c index 55fa1f86850d..795362aea344 100644 --- a/drivers/vfio/pci/cxl/vfio_cxl_core.c +++ b/drivers/vfio/pci/cxl/vfio_cxl_core.c @@ -632,6 +632,45 @@ static void vfio_cxl_reset_done(struct vfio_pci_core_device *vdev) cxl->hdm_valid = false; } +/* + * Run the CXL DVSEC reset sequence in place of a PCI function reset. A CXL + * Type-2 function must not take an FLR (it would corrupt CXL.mem), so the vfio + * reset points route here. The sequence resets the function, always clearing + * device memory, and restores the HDM decoder. The caller holds memory_lock, + * and this path does not hold the PCI device lock, so cxl_reset_dvsec_sequence() + * can take it. + */ +static int vfio_cxl_reset(struct vfio_pci_core_device *vdev) +{ + struct vfio_cxl_state *cxl = vdev->cxl; + int ret; + + lockdep_assert_held_write(&vdev->memory_lock); + + /* Host CPU access to the HDM range is unsafe until the decoder is back. */ + cxl->hdm_valid = false; + + ret = cxl_reset_dvsec_sequence(vdev->pdev); + if (!ret) + cxl->hdm_valid = true; + + return ret; +} + +/* + * The HDM dma-buf may be armed only while the decoder is valid. After a failed + * reset hdm_valid is clear, so the generic memory-enable re-arm must skip the + * dma-buf rather than map DMA onto an unrestored decoder. + */ +static bool vfio_cxl_hdm_active(struct vfio_pci_core_device *vdev) +{ + struct vfio_cxl_state *cxl = vdev->cxl; + + lockdep_assert_held_write(&vdev->memory_lock); + + return cxl->hdm_valid; +} + static const struct vfio_cxl_ops vfio_cxl_ops = { .init = vfio_cxl_init_device, .release = vfio_cxl_release_device, @@ -639,6 +678,8 @@ static const struct vfio_cxl_ops vfio_cxl_ops = { .close_device = vfio_cxl_close_device, .reset_prepare = vfio_cxl_reset_prepare, .reset_done = vfio_cxl_reset_done, + .reset = vfio_cxl_reset, + .hdm_active = vfio_cxl_hdm_active, .owner = THIS_MODULE, }; diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index 9a020a768055..8a5a737efa31 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -630,7 +630,14 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos, *virt_cmd &= cpu_to_le16(~mask); *virt_cmd |= cpu_to_le16(new_cmd & mask); - if (__vfio_pci_memory_enabled(vdev)) + /* + * Re-arm the dma-bufs on memory-enable, but keep a CXL device's + * HDM dma-buf revoked while the decoder is unrestored (a failed + * reset leaves hdm_valid clear); re-arming would map DMA onto a + * decoder the fault path still gates. Plain vfio-pci is unchanged. + */ + if (__vfio_pci_memory_enabled(vdev) && + (!vdev->cxl_ops || vdev->cxl_ops->hdm_active(vdev))) vfio_pci_dma_buf_move(vdev, false); up_write(&vdev->memory_lock); } @@ -720,7 +727,8 @@ static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev, } vfio_pci_set_power_state(vdev, state); - if (__vfio_pci_memory_enabled(vdev)) + if (__vfio_pci_memory_enabled(vdev) && + (!vdev->cxl_ops || vdev->cxl_ops->hdm_active(vdev))) vfio_pci_dma_buf_move(vdev, false); up_write(&vdev->memory_lock); } @@ -910,8 +918,14 @@ 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); - if (__vfio_pci_memory_enabled(vdev)) + ret = vfio_pci_reset_function(vdev); + /* + * Keep the HDM dma-buf revoked if a CXL reset + * failed; re-arming would map DMA onto an + * unrestored decoder. Mirrors the reset ioctl. + */ + if (__vfio_pci_memory_enabled(vdev) && + (!vdev->cxl_ops || !ret)) vfio_pci_dma_buf_move(vdev, false); up_write(&vdev->memory_lock); } @@ -995,8 +1009,14 @@ 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); - if (__vfio_pci_memory_enabled(vdev)) + ret = vfio_pci_reset_function(vdev); + /* + * Keep the HDM dma-buf revoked if a CXL reset + * failed; re-arming would map DMA onto an + * unrestored decoder. Mirrors the reset ioctl. + */ + if (__vfio_pci_memory_enabled(vdev) && + (!vdev->cxl_ops || !ret)) vfio_pci_dma_buf_move(vdev, false); up_write(&vdev->memory_lock); } @@ -1781,9 +1801,22 @@ static int vfio_cxl_dvsec_write(struct vfio_pci_core_device *vdev, int pos, status2 |= PCI_DVSEC_CXL_CACHE_INV; } if (ctrl2 & PCI_DVSEC_CXL_INIT_CXL_RST) { + int ret = 0; + ctrl2 &= ~PCI_DVSEC_CXL_INIT_CXL_RST; - status2 &= ~PCI_DVSEC_CXL_RST_ERR; - status2 |= PCI_DVSEC_CXL_RST_DONE; + + if (vdev->cxl_ops && vdev->cxl_ops->reset) { + vfio_pci_zap_and_down_write_memory_lock(vdev); + vfio_pci_dma_buf_move(vdev, true); + ret = vfio_pci_reset_function(vdev); + if (__vfio_pci_memory_enabled(vdev) && + (!vdev->cxl_ops || !ret)) + 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 f02a5240aa71..8bd4db7afefe 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -643,8 +643,27 @@ 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); - if (ret == -EAGAIN) + if (vdev->cxl_ops && vdev->cxl_ops->reset) { + /* + * VM power-on resets a CXL Type-2 device through its DVSEC + * sequence. vconfig is not built yet here, so take memory_lock + * and call the op directly rather than the wrapper. + */ + down_write(&vdev->memory_lock); + ret = vdev->cxl_ops->reset(vdev); + up_write(&vdev->memory_lock); + } else { + ret = pci_try_reset_function(pdev); + } + + /* + * -EAGAIN means the reset could not run. For a CXL device any reset + * error must also fail the open: a failed DVSEC reset can leave the HDM + * decoder cleared or unrestored, and continuing would expose the HDM + * region for host access through a decoder in an unknown state. + */ + if (ret == -EAGAIN || + (vdev->cxl_ops && vdev->cxl_ops->reset && ret)) goto out_disable_device; vdev->reset_works = !ret; @@ -845,16 +864,30 @@ void vfio_pci_core_disable(struct vfio_pci_core_device *vdev) * overwrite the previously restored configuration information. */ if (vdev->reset_works) { - bridge = pci_upstream_bridge(pdev); - if (bridge && !pci_dev_trylock(bridge)) - goto out_restore_state; - if (pci_dev_trylock(pdev)) { - if (!__pci_reset_function_locked(pdev)) + if (vdev->cxl_ops && vdev->cxl_ops->reset) { + /* + * VM power-off resets a CXL Type-2 device through its + * DVSEC sequence. The sequence takes its own device lock, + * so run it outside the lock below. + * vconfig is already freed here, so call the op directly + * under memory_lock rather than the wrapper. + */ + down_write(&vdev->memory_lock); + if (!vdev->cxl_ops->reset(vdev)) vdev->needs_reset = false; - pci_dev_unlock(pdev); + up_write(&vdev->memory_lock); + } else { + bridge = pci_upstream_bridge(pdev); + if (bridge && !pci_dev_trylock(bridge)) + goto out_restore_state; + if (pci_dev_trylock(pdev)) { + if (!__pci_reset_function_locked(pdev)) + vdev->needs_reset = false; + pci_dev_unlock(pdev); + } + if (bridge) + pci_dev_unlock(bridge); } - if (bridge) - pci_dev_unlock(bridge); } out_restore_state: @@ -1592,6 +1625,20 @@ static int vfio_pci_ioctl_set_irqs(struct vfio_pci_core_device *vdev, return ret; } +/* + * Reset the function. A CXL device runs the CXL DVSEC reset sequence in place + * of a PCI function reset: it replaces FLR (which would corrupt CXL.mem), + * always clears device memory, and restores the HDM decoder. Callers hold + * memory_lock for write. + */ +int vfio_pci_reset_function(struct vfio_pci_core_device *vdev) +{ + if (!vdev->cxl_ops || !vdev->cxl_ops->reset) + return pci_try_reset_function(vdev->pdev); + + return vdev->cxl_ops->reset(vdev); +} + static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, void __user *arg) { @@ -1614,8 +1661,14 @@ 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); - if (__vfio_pci_memory_enabled(vdev)) + ret = vfio_pci_reset_function(vdev); + /* + * Re-arm the dma-bufs on success. A CXL device whose reset failed leaves + * the HDM decoder unrestored and hdm_valid clear, so re-arming its HDM + * dma-buf would map device DMA onto a decoder the fault path still gates; + * keep it revoked until a reset succeeds. Plain vfio-pci is unchanged. + */ + if (__vfio_pci_memory_enabled(vdev) && (!vdev->cxl_ops || !ret)) vfio_pci_dma_buf_move(vdev, false); up_write(&vdev->memory_lock); diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index c268c99aea82..e1ef21806a2f 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -78,6 +78,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_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 39a28cc6ae8c..231679dead45 100644 --- a/include/linux/vfio_pci_core.h +++ b/include/linux/vfio_pci_core.h @@ -74,6 +74,10 @@ struct vfio_cxl_ops { void (*close_device)(struct vfio_pci_core_device *vdev); void (*reset_prepare)(struct vfio_pci_core_device *vdev); void (*reset_done)(struct vfio_pci_core_device *vdev); + /* Run the CXL reset (always clears CXL.mem) in place of FLR */ + int (*reset)(struct vfio_pci_core_device *vdev); + /* True while the HDM range is valid and its dma-buf may be armed */ + bool (*hdm_active)(struct vfio_pci_core_device *vdev); /* Pinned per bound CXL device so vfio-cxl cannot unload under usage */ struct module *owner; }; -- 2.25.1
