RE: [PATCH 2/2] vfio/pci: Use unmap_mapping_range()

Tian, Kevin
2024-05-30T00:09:53Z

> From: Alex Williamson <alex.williamson@redhat.com>
> Sent: Friday, May 24, 2024 3:56 AM
>
> -/* Caller holds vma_lock */
> -static int __vfio_pci_add_vma(struct vfio_pci_core_device *vdev,
> - struct vm_area_struct *vma)
> +static int vma_to_pfn(struct vm_area_struct *vma, unsigned long *pfn)
> {
> - struct vfio_pci_mmap_vma *mmap_vma;
> -
> - mmap_vma = kmalloc(sizeof(*mmap_vma), GFP_KERNEL_ACCOUNT);
> - if (!mmap_vma)
> - return -ENOMEM;
> -
> - mmap_vma->vma = vma;
> - list_add(&mmap_vma->vma_next, &vdev->vma_list);
> + struct vfio_pci_core_device *vdev = vma->vm_private_data;
> + int index = vma->vm_pgoff >> (VFIO_PCI_OFFSET_SHIFT -
> PAGE_SHIFT);
> + u64 pgoff;
>
> - return 0;
> -}
> + if (index >= VFIO_PCI_ROM_REGION_INDEX ||
> + !vdev->bar_mmap_supported[index] || !vdev->barmap[index])
> + return -EINVAL;

Is a WARN_ON() required here? If those checks fail vfio_pci_core_mmap()
will return error w/o installing vm_ops.

> @@ -2506,17 +2373,11 @@ static int vfio_pci_dev_set_hot_reset(struct
> vfio_device_set *dev_set,
> struct vfio_pci_group_info *groups,
> struct iommufd_ctx *iommufd_ctx)

the comment before this function should be updated too:

/*
* We need to get memory_lock for each device, but devices can share mmap_lock,
* therefore we need to zap and hold the vma_lock for each device, and only then
* get each memory_lock.
*/

otherwise looks good:

Reviewed-by: Kevin Tian <kevin.tian@intel.com>