Re: [PATCH 2/4] platform/nvidia: Implement mmap and memory scrubbing for EGM chardev

[Jason Gunthorpe]

>  static int nvgrace_egm_release(struct inode *inode, struct file *file)
>  {
> +	struct nvgrace_egm_dev *egm_dev =
> +		container_of(inode->i_cdev, struct nvgrace_egm_dev, cdev);
> +
> +	guard(mutex)(&egm_dev->open_lock);
> +
> +	if (!--egm_dev->open_count)
> +		file->private_data = NULL;


You do not need to zero the private data.

open_count is only zero or one, so if the only write is =1 then then only
store should be =0 and it doesn't make sense to call it open *count*, it is
just 'bool opened'

>  static int nvgrace_egm_mmap(struct file *file, struct vm_area_struct *vma)
>  {
> +	struct nvgrace_egm_dev *egm_dev = file->private_data;
> +	u64 req_len, pgoff, end;
> +	unsigned long start_pfn, num_pages;
> +
> +	pgoff = vma->vm_pgoff;
> +	num_pages = egm_dev->egmlength >> PAGE_SHIFT;
> +
> +	/* Reject a page offset that already lies outside the EGM region. */
> +	if (pgoff >= num_pages)
> +		return -EINVAL;
> +
> +	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;
> +
>  	/*
> -	 * Mapping the EGM region into userspace is implemented by a later
> -	 * patch. Until then refuse the mmap.
> +	 * EGM memory is invisible to the host kernel and is not managed
> +	 * by it. Map the usermode VMA to the EGM region.
>  	 */
> -	return -EOPNOTSUPP;

Don't add an empty stub in the prior patch for an optional op fully implemented in a later patch

The logic looks fine otherwise

-- 
Jason