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;
> @@ -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
> 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
