Re: [PATCH RFC v2 09/15] vfio/nvgrace-egm: Add chardev ops for EGM management

[Alex Williamson <alex@shazbot.org>]

On Mon, 23 Feb 2026 15:55:08 +0000
<ankita@nvidia.com> wrote:

> From: Ankit Agrawal <ankita@nvidia.com>
> 
> EGM module implements the mmap file_ops to manage the usermode app's
> VMA mapping to the EGM region. The appropriate region is determined
> from the minor number.
> 
> Note that the EGM memory region is invisible to the host kernel as it
> is not present in the host EFI map. The host Linux MM thus cannot manage
> the memory, even though it is accessible on the host SPA. The EGM module
> thus use remap_pfn_range() to perform the VMA mapping to the EGM region.
> 
> Suggested-by: Aniket Agashe <aniketa@nvidia.com>
> Signed-off-by: Ankit Agrawal <ankita@nvidia.com>
> ---
>  drivers/vfio/pci/nvgrace-gpu/egm.c | 41 +++++++++++++++++++++++++++++-
>  include/linux/nvgrace-egm.h        |  1 +
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/pci/nvgrace-gpu/egm.c b/drivers/vfio/pci/nvgrace-gpu/egm.c
> index d7e4f61a241c..5786ebe374a5 100644
> --- a/drivers/vfio/pci/nvgrace-gpu/egm.c
> +++ b/drivers/vfio/pci/nvgrace-gpu/egm.c
> @@ -17,19 +17,58 @@ struct chardev {
>  	struct cdev cdev;
>  };
>  
> +static struct nvgrace_egm_dev *
> +egm_chardev_to_nvgrace_egm_dev(struct chardev *egm_chardev)
> +{
> +	struct auxiliary_device *aux_dev =
> +		container_of(egm_chardev->device.parent, struct auxiliary_device, dev);
> +
> +	return container_of(aux_dev, struct nvgrace_egm_dev, aux_dev);
> +}
> +
>  static int nvgrace_egm_open(struct inode *inode, struct file *file)
>  {
> +	struct chardev *egm_chardev =
> +		container_of(inode->i_cdev, struct chardev, cdev);
> +
> +	file->private_data = egm_chardev;
> +

No reference taken to egm device, nothing blocks it being removed.

>  	return 0;
>  }
>  
>  static int nvgrace_egm_release(struct inode *inode, struct file *file)
>  {
> +	file->private_data = NULL;

Unnecessary.

> +
>  	return 0;
>  }
>  
>  static int nvgrace_egm_mmap(struct file *file, struct vm_area_struct *vma)
>  {
> -	return 0;
> +	struct chardev *egm_chardev = file->private_data;
> +	struct nvgrace_egm_dev *egm_dev =
> +		egm_chardev_to_nvgrace_egm_dev(egm_chardev);
> +	u64 req_len, pgoff, end;
> +	unsigned long start_pfn;
> +
> +	pgoff = vma->vm_pgoff &
> +		((1U << (EGM_OFFSET_SHIFT - PAGE_SHIFT)) - 1);

I don't know what you're doing here with EGM_OFFSET_SHIFT other than
ignoring the high bits and creating aliases across the device file
address space for no(?) reason.  Looks like pointlessly copying vfio's
region segmentation.

> +
> +	if (check_sub_overflow(vma->vm_end, vma->vm_start, &req_len) ||
> +	    check_add_overflow(PHYS_PFN(egm_dev->egmphys), pgoff, &start_pfn) ||
> +	    check_add_overflow(PFN_PHYS(pgoff), req_len, &end))
> +		return -EOVERFLOW;
> +
> +	if (end > egm_dev->egmlength)
> +		return -EINVAL;
> +
> +	/*
> +	 * EGM memory is invisible to the host kernel and is not managed
> +	 * by it. Map the usermode VMA to the EGM region.
> +	 */
> +	return remap_pfn_range(vma, vma->vm_start,
> +			       start_pfn, req_len,
> +			       vma->vm_page_prot);

Obviously there are concerns about how this relates not only to the
state of the device in routing access, but also the lifetime of this as
there's no reference tracking whatsoever.  Thanks,

Alex

>  }
>  
>  static const struct file_operations file_ops = {
> diff --git a/include/linux/nvgrace-egm.h b/include/linux/nvgrace-egm.h
> index a66906753267..b9956e7e5a0e 100644
> --- a/include/linux/nvgrace-egm.h
> +++ b/include/linux/nvgrace-egm.h
> @@ -9,6 +9,7 @@
>  #include <linux/auxiliary_bus.h>
>  
>  #define NVGRACE_EGM_DEV_NAME "egm"
> +#define EGM_OFFSET_SHIFT   40
>  
>  struct gpu_node {
>  	struct list_head list;