RE: [PATCH v5 2/7] vfio: export function to map the VMA

Shameer Kolothum
2025-11-24T17:22:23Z

> -----Original Message-----
> From: Ankit Agrawal <ankita@nvidia.com>
> Sent: 24 November 2025 11:59
> To: Ankit Agrawal <ankita@nvidia.com>; jgg@ziepe.ca; Yishai Hadas
> <yishaih@nvidia.com>; Shameer Kolothum <skolothumtho@nvidia.com>;
> kevin.tian@intel.com; alex@shazbot.org; Aniket Agashe
> <aniketa@nvidia.com>; Vikram Sethi <vsethi@nvidia.com>; Matt Ochs
> <mochs@nvidia.com>
> Cc: Yunxiang.Li@amd.com; yi.l.liu@intel.com;
> zhangdongdong@eswincomputing.com; Avihai Horon <avihaih@nvidia.com>;
> bhelgaas@google.com; peterx@redhat.com; pstanner@redhat.com; Alistair
> Popple <apopple@nvidia.com>; kvm@vger.kernel.org; linux-
> kernel@vger.kernel.org; Neo Jia <cjia@nvidia.com>; Kirti Wankhede
> <kwankhede@nvidia.com>; Tarun Gupta (SW-GPU) <targupta@nvidia.com>;
> Zhi Wang <zhiw@nvidia.com>; Dan Williams <danw@nvidia.com>; Dheeraj
> Nigam <dnigam@nvidia.com>; Krishnakant Jaju <kjaju@nvidia.com>
> Subject: [PATCH v5 2/7] vfio: export function to map the VMA
>
> From: Ankit Agrawal <ankita@nvidia.com>
>
> Take out the implementation to map the VMA to the PTE/PMD/PUD
> as a separate function.
>
> Export the function to be used by nvgrace-gpu module.
>
> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
> ---
> drivers/vfio/pci/vfio_pci_core.c | 46 ++++++++++++++++++++------------
> include/linux/vfio_pci_core.h | 2 ++
> 2 files changed, 31 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index 7dcf5439dedc..ede410e0ae1c 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1640,6 +1640,34 @@ static unsigned long vma_to_pfn(struct
> vm_area_struct *vma)
> return (pci_resource_start(vdev->pdev, index) >> PAGE_SHIFT) +
> pgoff;
> }
> +vm_fault_t vfio_pci_vmf_insert_pfn(struct vm_fault *vmf,
> + unsigned long pfn,
> + unsigned int order)

Does this need to be called with vdev->memory_lock held?
Please add a comment if so.

> +{
> + vm_fault_t ret;
> +
> + switch (order) {
> + case 0:
> + ret = vmf_insert_pfn(vmf->vma, vmf->address, pfn);
> + break;
> +#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
> + case PMD_ORDER:
> + ret = vmf_insert_pfn_pmd(vmf, pfn, false);
> + break;
> +#endif
> +#ifdef CONFIG_ARCH_SUPPORTS_PUD_PFNMAP
> + case PUD_ORDER:
> + ret = vmf_insert_pfn_pud(vmf, pfn, false);
> + break;
> +#endif
> + default:
> + ret = VM_FAULT_FALLBACK;
> + }
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(vfio_pci_vmf_insert_pfn);
> +
> static vm_fault_t vfio_pci_mmap_huge_fault(struct vm_fault *vmf,
> unsigned int order)
> {
> @@ -1662,23 +1690,7 @@ static vm_fault_t
> vfio_pci_mmap_huge_fault(struct vm_fault *vmf,
> if (vdev->pm_runtime_engaged ||
> !__vfio_pci_memory_enabled(vdev))
> goto out_unlock;

Is the above check not required for the common helper?

>
> - switch (order) {
> - case 0:
> - ret = vmf_insert_pfn(vma, vmf->address, pfn);
> - break;
> -#ifdef CONFIG_ARCH_SUPPORTS_PMD_PFNMAP
> - case PMD_ORDER:
> - ret = vmf_insert_pfn_pmd(vmf, pfn, false);
> - break;
> -#endif
> -#ifdef CONFIG_ARCH_SUPPORTS_PUD_PFNMAP
> - case PUD_ORDER:
> - ret = vmf_insert_pfn_pud(vmf, pfn, false);
> - break;
> -#endif
> - default:
> - ret = VM_FAULT_FALLBACK;
> - }
> + ret = vfio_pci_vmf_insert_pfn(vmf, pfn, order);
>
> out_unlock:
> up_read(&vdev->memory_lock);
> diff --git a/include/linux/vfio_pci_core.h b/include/linux/vfio_pci_core.h
> index f541044e42a2..970b3775505e 100644
> --- a/include/linux/vfio_pci_core.h
> +++ b/include/linux/vfio_pci_core.h
> @@ -119,6 +119,8 @@ ssize_t vfio_pci_core_read(struct vfio_device
> *core_vdev, char __user *buf,
> size_t count, loff_t *ppos);
> ssize_t vfio_pci_core_write(struct vfio_device *core_vdev, const char __user
> *buf,
> size_t count, loff_t *ppos);
> +vm_fault_t vfio_pci_vmf_insert_pfn(struct vm_fault *vmf, unsigned long pfn,
> + unsigned int order);
> int vfio_pci_core_mmap(struct vfio_device *core_vdev, struct vm_area_struct
> *vma);
> void vfio_pci_core_request(struct vfio_device *core_vdev, unsigned int
> count);
> int vfio_pci_core_match(struct vfio_device *core_vdev, char *buf);

Otherwise LGTM:
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>