On 3/12/26 19:46, Matt Evans wrote:

vfio_pci_zap_bars() and the wrapper vfio_pci_zap_and_down_write_memory_lock() are redundant as of "vfio/pci: Convert BAR mmap() to use a DMABUF". The DMABUFs used for BAR mappings already zap PTEs via the existing vfio_pci_dma_buf_move(), which notifies changes to the BAR space (e.g. around reset).

Remove the old functions, and the various points needing to zap BARs become slightly cleaner.

No a full review, but it looks like you now take the DMA buf reservation lock while holding vdev->memory_lock.

I strongly recommend enabling lockdep while testing that, just to be on the sure side that all locks are taken in a consistend order.

Regards, Christian.

Signed-off-by: Matt Evans mattev@meta.com

drivers/vfio/pci/vfio_pci_config.c | 18 ++++++------------ drivers/vfio/pci/vfio_pci_core.c | 30 +++++++----------------------- drivers/vfio/pci/vfio_pci_priv.h | 1 - 3 files changed, 13 insertions(+), 36 deletions(-)

diff --git a/drivers/vfio/pci/vfio_pci_config.c b/drivers/vfio/pci/vfio_pci_config.c index b4e39253f98d..c7ed28be1104 100644 --- a/drivers/vfio/pci/vfio_pci_config.c +++ b/drivers/vfio/pci/vfio_pci_config.c @@ -590,12 +590,9 @@ static int vfio_basic_config_write(struct vfio_pci_core_device *vdev, int pos, virt_mem = !!(le16_to_cpu(*virt_cmd) & PCI_COMMAND_MEMORY); new_mem = !!(new_cmd & PCI_COMMAND_MEMORY);

• if (!new_mem) {
  

• 	vfio_pci_zap_and_down_write_memory_lock(vdev);
  

• down_write(&vdev->memory_lock);
  

• if (!new_mem)
  vfio_pci_dma_buf_move(vdev, true);
  

• } else {
  

• 	down_write(&vdev->memory_lock);
  

• }
  

/* * If the user is writing mem/io enable (new_mem/io) and we @@ -712,12 +709,9 @@ static int __init init_pci_cap_basic_perm(struct perm_bits *perm) static void vfio_lock_and_set_power_state(struct vfio_pci_core_device *vdev, pci_power_t state) {

• if (state >= PCI_D3hot) {

• vfio_pci_zap_and_down_write_memory_lock(vdev);
  

• down_write(&vdev->memory_lock);
• if (state >= PCI_D3hot) vfio_pci_dma_buf_move(vdev, true);

• } else {

• down_write(&vdev->memory_lock);
  

• }

vfio_pci_set_power_state(vdev, state); if (__vfio_pci_memory_enabled(vdev)) @@ -908,7 +902,7 @@ static int vfio_exp_config_write(struct vfio_pci_core_device *vdev, int pos, &cap); if (!ret && (cap & PCI_EXP_DEVCAP_FLR)) {

• 	vfio_pci_zap_and_down_write_memory_lock(vdev);
  

• 	down_write(&vdev->memory_lock);
  vfio_pci_dma_buf_move(vdev, true);
  pci_try_reset_function(vdev->pdev);
  if (__vfio_pci_memory_enabled(vdev))
  

@@ -993,7 +987,7 @@ static int vfio_af_config_write(struct vfio_pci_core_device *vdev, int pos, &cap); if (!ret && (cap & PCI_AF_CAP_FLR) && (cap & PCI_AF_CAP_TP)) {

• 	vfio_pci_zap_and_down_write_memory_lock(vdev);
  

• 	down_write(&vdev->memory_lock);
  vfio_pci_dma_buf_move(vdev, true);
  pci_try_reset_function(vdev->pdev);
  if (__vfio_pci_memory_enabled(vdev))
  

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c index 41224efa58d8..9e9ad97c2f7f 100644 --- a/drivers/vfio/pci/vfio_pci_core.c +++ b/drivers/vfio/pci/vfio_pci_core.c @@ -319,7 +319,7 @@ static int vfio_pci_runtime_pm_entry(struct vfio_pci_core_device *vdev, * The vdev power related flags are protected with 'memory_lock' * semaphore. */

• vfio_pci_zap_and_down_write_memory_lock(vdev);

• down_write(&vdev->memory_lock); vfio_pci_dma_buf_move(vdev, true);

if (vdev->pm_runtime_engaged) { @@ -1229,7 +1229,7 @@ static int vfio_pci_ioctl_reset(struct vfio_pci_core_device *vdev, if (!vdev->reset_works) return -EINVAL;

• vfio_pci_zap_and_down_write_memory_lock(vdev);

• down_write(&vdev->memory_lock);

/* * This function can be invoked while the power state is non-D0. If @@ -1613,22 +1613,6 @@ ssize_t vfio_pci_core_write(struct vfio_device *core_vdev, const char __user *bu } EXPORT_SYMBOL_GPL(vfio_pci_core_write); -static void vfio_pci_zap_bars(struct vfio_pci_core_device *vdev) -{

• struct vfio_device *core_vdev = &vdev->vdev;
• loff_t start = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX);
• loff_t end = VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX);
• loff_t len = end - start;
• unmap_mapping_range(core_vdev->inode->i_mapping, start, len, true);

-}

-void vfio_pci_zap_and_down_write_memory_lock(struct vfio_pci_core_device *vdev) -{

• down_write(&vdev->memory_lock);
• vfio_pci_zap_bars(vdev);

-}

u16 vfio_pci_memory_lock_and_enable(struct vfio_pci_core_device *vdev) { u16 cmd; @@ -2487,10 +2471,11 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, } /*

•  * Take the memory write lock for each device and zap BAR
  

•  * mappings to prevent the user accessing the device while in
  

•  * reset.  Locking multiple devices is prone to deadlock,
  

•  * runaway and unwind if we hit contention.
  

•  * Take the memory write lock for each device and
  

•  * revoke all DMABUFs, which will prevent any access
  

•  * to the device while in reset.  Locking multiple
  

•  * devices is prone to deadlock, runaway and unwind if
  

•  * we hit contention.
  

  */ if (!down_write_trylock(&vdev->memory_lock)) { ret = -EBUSY;

@@ -2498,7 +2483,6 @@ static int vfio_pci_dev_set_hot_reset(struct vfio_device_set *dev_set, } vfio_pci_dma_buf_move(vdev, true);

• vfio_pci_zap_bars(vdev);
  

  }

if (!list_entry_is_head(vdev, diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h index 37ece9b4b5bd..e201c96bbb14 100644 --- a/drivers/vfio/pci/vfio_pci_priv.h +++ b/drivers/vfio/pci/vfio_pci_priv.h @@ -78,7 +78,6 @@ void vfio_config_free(struct vfio_pci_core_device *vdev); 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); 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);