[PATCH 8/8] drm/amdkfd: Use kfd fd to mmap mmio

[Kuehling, Felix]

Hi Oak,

I'm not sure why this is part of the GWS patch series. It's unrelated to 
GWS. Anyway, see one comment inline.

On 2019-05-23 6:41 p.m., Zeng, Oak wrote:
> TTM doesn't support CPU mapping of sg type bo (under which
> mmio bo is created). Switch mmaping of mmio page to kfd
> device file.
>
> Change-Id: I1a1a24f2ac0662be3783d460c137731ade007b83
> Signed-off-by: Oak Zeng <Oak.Zeng at amd.com>
> ---
>   drivers/gpu/drm/amd/amdkfd/kfd_chardev.c | 46 ++++++++++++++++++++++++++++++++
>   drivers/gpu/drm/amd/amdkfd/kfd_priv.h    |  1 +
>   2 files changed, 47 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> index 455a3db..67d269b 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_chardev.c
> @@ -1309,6 +1309,15 @@ static int kfd_ioctl_alloc_memory_of_gpu(struct file *filep,
>   	args->handle = MAKE_HANDLE(args->gpu_id, idr_handle);
>   	args->mmap_offset = offset;
>   
> +	/* MMIO is mapped through kfd device
> +	 * Generate a kfd mmap offset
> +	 */
> +	if (flags & KFD_IOC_ALLOC_MEM_FLAGS_MMIO_REMAP) {
> +		args->mmap_offset = KFD_MMAP_TYPE_MMIO | KFD_MMAP_GPU_ID(args->gpu_id);
> +		args->mmap_offset <<= PAGE_SHIFT;
> +		args->mmap_offset |= amdgpu_amdkfd_get_mmio_remap_phys_addr(dev->kgd);

It seems the mmio_remap_phys_addr doesn't need to be part of the 
mmap_offset. It's not used for anything in kfd_mmap. And it 
unnecessarily exposes a physical address to user mode, which we usually 
tend to avoid.

With that fixes, the patch is Reviewed-by: Felix Kuehling 
<Felix.Kuehling at amd.com>

Regards,
   Felix


> +	}
> +
>   	return 0;
>   
>   err_free:
> @@ -1880,6 +1889,39 @@ static long kfd_ioctl(struct file *filep, unsigned int cmd, unsigned long arg)
>   	return retcode;
>   }
>   
> +static int kfd_mmio_mmap(struct kfd_dev *dev, struct kfd_process *process,
> +		      struct vm_area_struct *vma)
> +{
> +	phys_addr_t address;
> +	int ret;
> +
> +	if (vma->vm_end - vma->vm_start != PAGE_SIZE)
> +		return -EINVAL;
> +
> +	address = amdgpu_amdkfd_get_mmio_remap_phys_addr(dev->kgd);
> +
> +	vma->vm_flags |= VM_IO | VM_DONTCOPY | VM_DONTEXPAND | VM_NORESERVE |
> +				VM_DONTDUMP | VM_PFNMAP;
> +
> +	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
> +
> +	pr_debug("Process %d mapping mmio page\n"
> +		 "     target user address == 0x%08llX\n"
> +		 "     physical address    == 0x%08llX\n"
> +		 "     vm_flags            == 0x%04lX\n"
> +		 "     size                == 0x%04lX\n",
> +		 process->pasid, (unsigned long long) vma->vm_start,
> +		 address, vma->vm_flags, PAGE_SIZE);
> +
> +	ret = io_remap_pfn_range(vma,
> +				vma->vm_start,
> +				address >> PAGE_SHIFT,
> +				PAGE_SIZE,
> +				vma->vm_page_prot);
> +	return ret;
> +}
> +
> +
>   static int kfd_mmap(struct file *filp, struct vm_area_struct *vma)
>   {
>   	struct kfd_process *process;
> @@ -1910,6 +1952,10 @@ static int kfd_mmap(struct file *filp, struct vm_area_struct *vma)
>   		if (!dev)
>   			return -ENODEV;
>   		return kfd_reserved_mem_mmap(dev, process, vma);
> +	case KFD_MMAP_TYPE_MMIO:
> +		if (!dev)
> +			return -ENODEV;
> +		return kfd_mmio_mmap(dev, process, vma);
>   	}
>   
>   	return -EFAULT;
> diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> index 40a5c67..b61dc53 100644
> --- a/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> +++ b/drivers/gpu/drm/amd/amdkfd/kfd_priv.h
> @@ -59,6 +59,7 @@
>   #define KFD_MMAP_TYPE_DOORBELL	(0x3ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_EVENTS	(0x2ULL << KFD_MMAP_TYPE_SHIFT)
>   #define KFD_MMAP_TYPE_RESERVED_MEM	(0x1ULL << KFD_MMAP_TYPE_SHIFT)
> +#define KFD_MMAP_TYPE_MMIO	(0x0ULL << KFD_MMAP_TYPE_SHIFT)
>   
>   #define KFD_MMAP_GPU_ID_SHIFT (46 - PAGE_SHIFT)
>   #define KFD_MMAP_GPU_ID_MASK (((1ULL << KFD_GPU_ID_HASH_WIDTH) - 1) \