Re: [RFC PATCH 04/10] vfio/pci: Use vfio_device_unmap_mapping_range()
From: Jason Gunthorpe <jgg@nvidia.com>
Date: 2021-02-22 17:23:21
Also in:
lkml
On Mon, Feb 22, 2021 at 09:51:13AM -0700, Alex Williamson wrote:
+ vfio_device_unmap_mapping_range(vdev->device, + VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX), + VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_ROM_REGION_INDEX) - + VFIO_PCI_INDEX_TO_OFFSET(VFIO_PCI_BAR0_REGION_INDEX));
Isn't this the same as invalidating everything? I see in vfio_pci_mmap(): if (index >= VFIO_PCI_ROM_REGION_INDEX) return -EINVAL;
quoted hunk ↗ jump to hunk
@@ -2273,15 +2112,13 @@ static int vfio_pci_try_zap_and_vma_lock_cb(struct pci_dev *pdev, void *data) vdev = vfio_device_data(device); - /* - * Locking multiple devices is prone to deadlock, runaway and - * unwind if we hit contention. - */ - if (!vfio_pci_zap_and_vma_lock(vdev, true)) { + if (!down_write_trylock(&vdev->memory_lock)) { vfio_device_put(device); return -EBUSY; }
And this is only done as part of VFIO_DEVICE_PCI_HOT_RESET? It looks like VFIO_DEVICE_PCI_HOT_RESET effects the entire slot? How about putting the inode on the reflck structure, which is also per-slot, and then a single unmap_mapping_range() will take care of everything, no need to iterate over things in the driver core. Note the vm->pg_off space doesn't have any special meaning, it is fine that two struct vfio_pci_device's are sharing the same address space and using an incompatible overlapping pg_offs
quoted hunk ↗ jump to hunk
diff --git a/drivers/vfio/pci/vfio_pci_private.h b/drivers/vfio/pci/vfio_pci_private.h index 9cd1882a05af..ba37f4eeefd0 100644 +++ b/drivers/vfio/pci/vfio_pci_private.h@@ -101,6 +101,7 @@ struct vfio_pci_mmap_vma { struct vfio_pci_device { struct pci_dev *pdev; + struct vfio_device *device;
Ah, I did this too, but I didn't use a pointer :) All the places trying to call vfio_device_put() when they really want a vfio_pci_device * become simpler now. Eg struct vfio_devices wants to have an array of vfio_pci_device, and get_pf_vdev() only needs to return one pointer. Jason