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

[Shameer Kolothum]

> -----Original Message-----
> From: Ankit Agrawal <ankita@xxxxxxxxxx>
> Sent: 24 November 2025 11:59
> To: Ankit Agrawal <ankita@xxxxxxxxxx>; jgg@xxxxxxxx; Yishai Hadas
> <yishaih@xxxxxxxxxx>; Shameer Kolothum <skolothumtho@xxxxxxxxxx>;
> kevin.tian@xxxxxxxxx; alex@xxxxxxxxxxx; Aniket Agashe
> <aniketa@xxxxxxxxxx>; Vikram Sethi <vsethi@xxxxxxxxxx>; Matt Ochs
> <mochs@xxxxxxxxxx>
> Cc: Yunxiang.Li@xxxxxxx; yi.l.liu@xxxxxxxxx;
> zhangdongdong@xxxxxxxxxxxxxxxxxx; Avihai Horon <avihaih@xxxxxxxxxx>;
> bhelgaas@xxxxxxxxxx; peterx@xxxxxxxxxx; pstanner@xxxxxxxxxx; Alistair
> Popple <apopple@xxxxxxxxxx>; kvm@xxxxxxxxxxxxxxx; linux-
> kernel@xxxxxxxxxxxxxxx; Neo Jia <cjia@xxxxxxxxxx>; Kirti Wankhede
> <kwankhede@xxxxxxxxxx>; Tarun Gupta (SW-GPU) <targupta@xxxxxxxxxx>;
> Zhi Wang <zhiw@xxxxxxxxxx>; Dan Williams <danw@xxxxxxxxxx>; Dheeraj
> Nigam <dnigam@xxxxxxxxxx>; Krishnakant Jaju <kjaju@xxxxxxxxxx>
> Subject: [PATCH v5 2/7] vfio: export function to map the VMA
> 
> From: Ankit Agrawal <ankita@xxxxxxxxxx>
> 
> 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@xxxxxxxxxx>
> ---
>  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@xxxxxxxxxx>