RE: [PATCH RFC v2 08/15] vfio/nvgrace-egm: Expose EGM region as char device

[Shameer Kolothum Thodi]

> -----Original Message-----
> From: Ankit Agrawal <ankita@xxxxxxxxxx>
> Sent: 23 February 2026 15:55
> To: Ankit Agrawal <ankita@xxxxxxxxxx>; Vikram Sethi <vsethi@xxxxxxxxxx>;
> Jason Gunthorpe <jgg@xxxxxxxxxx>; Matt Ochs <mochs@xxxxxxxxxx>;
> jgg@xxxxxxxx; Shameer Kolothum Thodi <skolothumtho@xxxxxxxxxx>;
> alex@xxxxxxxxxxx
> Cc: Neo Jia <cjia@xxxxxxxxxx>; Zhi Wang <zhiw@xxxxxxxxxx>; Krishnakant
> Jaju <kjaju@xxxxxxxxxx>; Yishai Hadas <yishaih@xxxxxxxxxx>;
> kevin.tian@xxxxxxxxx; kvm@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH RFC v2 08/15] vfio/nvgrace-egm: Expose EGM region as char
> device
> 
> From: Ankit Agrawal <ankita@xxxxxxxxxx>
> 
> The EGM module expose the various EGM regions as a char device. A
> usermode app such as Qemu may mmap to the region and use as VM
> sysmem.
> Each EGM region is represented with a unique char device /dev/egmX
> bearing a distinct minor number.
> 
> 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@xxxxxxxxxx>
> Signed-off-by: Ankit Agrawal <ankita@xxxxxxxxxx>
> ---
>  drivers/vfio/pci/nvgrace-gpu/egm.c | 99
> ++++++++++++++++++++++++++++++
>  1 file changed, 99 insertions(+)
> 
> diff --git a/drivers/vfio/pci/nvgrace-gpu/egm.c b/drivers/vfio/pci/nvgrace-
> gpu/egm.c
> index 6fd6302a004a..d7e4f61a241c 100644
> --- a/drivers/vfio/pci/nvgrace-gpu/egm.c
> +++ b/drivers/vfio/pci/nvgrace-gpu/egm.c
> @@ -10,15 +10,114 @@
> 
>  static dev_t dev;
>  static struct class *class;
> +static DEFINE_XARRAY(egm_chardevs);
> +
> +struct chardev {
> +	struct device device;
> +	struct cdev cdev;
> +};
> +
> +static int nvgrace_egm_open(struct inode *inode, struct file *file)
> +{
> +	return 0;
> +}
> +
> +static int nvgrace_egm_release(struct inode *inode, struct file *file)
> +{
> +	return 0;
> +}
> +
> +static int nvgrace_egm_mmap(struct file *file, struct vm_area_struct *vma)
> +{
> +	return 0;
> +}
> +
> +static const struct file_operations file_ops = {
> +	.owner = THIS_MODULE,
> +	.open = nvgrace_egm_open,
> +	.release = nvgrace_egm_release,
> +	.mmap = nvgrace_egm_mmap,
> +};
> +
> +static void egm_chardev_release(struct device *dev)
> +{
> +	struct chardev *egm_chardev = container_of(dev, struct chardev,
> device);
> +
> +	kfree(egm_chardev);
> +}
> +
> +static struct chardev *
> +setup_egm_chardev(struct nvgrace_egm_dev *egm_dev)
> +{
> +	struct chardev *egm_chardev;
> +	int ret;
> +
> +	egm_chardev = kzalloc(sizeof(*egm_chardev), GFP_KERNEL);
> +	if (!egm_chardev)
> +		goto create_err;

return NULL; instead and get rid of create_err.

> +
> +	device_initialize(&egm_chardev->device);
> +
> +	/*
> +	 * Use the proximity domain number as the device minor
> +	 * number. So the EGM corresponding to node X would be
> +	 * /dev/egmX.
> +	 */
> +	egm_chardev->device.devt = MKDEV(MAJOR(dev), egm_dev-
> >egmpxm);
> +	egm_chardev->device.class = class;
> +	egm_chardev->device.release = egm_chardev_release;
> +	egm_chardev->device.parent = &egm_dev->aux_dev.dev;
> +	cdev_init(&egm_chardev->cdev, &file_ops);
> +	egm_chardev->cdev.owner = THIS_MODULE;
> +
> +	ret = dev_set_name(&egm_chardev->device, "egm%lld", egm_dev-
> >egmpxm);
> +	if (ret)
> +		goto error_exit;
> +
> +	ret = cdev_device_add(&egm_chardev->cdev, &egm_chardev-
> >device);
> +	if (ret)
> +		goto error_exit;
> +
> +	return egm_chardev;
> +
> +error_exit:
> +	put_device(&egm_chardev->device);
> +create_err:
> +	return NULL;
> +}
> +
> +static void del_egm_chardev(struct chardev *egm_chardev)
> +{
> +	cdev_device_del(&egm_chardev->cdev, &egm_chardev->device);
> +	put_device(&egm_chardev->device);
> +}
> 
>  static int egm_driver_probe(struct auxiliary_device *aux_dev,
>  			    const struct auxiliary_device_id *id)
>  {
> +	struct nvgrace_egm_dev *egm_dev =
> +		container_of(aux_dev, struct nvgrace_egm_dev, aux_dev);
> +	struct chardev *egm_chardev;
> +
> +	egm_chardev = setup_egm_chardev(egm_dev);
> +	if (!egm_chardev)
> +		return -EINVAL;
> +
> +	xa_store(&egm_chardevs, egm_dev->egmpxm, egm_chardev,
> GFP_KERNEL);
> +
>  	return 0;
>  }
> 
>  static void egm_driver_remove(struct auxiliary_device *aux_dev)
>  {
> +	struct nvgrace_egm_dev *egm_dev =
> +		container_of(aux_dev, struct nvgrace_egm_dev, aux_dev);
> +	struct chardev *egm_chardev = xa_erase(&egm_chardevs, egm_dev-
> >egmpxm);
> +
> +	if (!egm_chardev)
> +		return;
> +
> +	del_egm_chardev(egm_chardev);

Is this safe if there is still a file in use e.g. QEMU has /dev/egm0 open.

Thanks,
Shameer

>  }
> 
>  static const struct auxiliary_device_id egm_id_table[] = {
> --
> 2.34.1