[PATCH] drm/dma-helpers: Don't change vma flags

[Robin Murphy]

On 2022-11-23 17:28, Daniel Vetter wrote:
> This code was added in b65e64f7ccd4 ("drm/cma: Use
> dma_mmap_writecombine() to mmap buffer"), but does not explain why
> it's needed.
> 
> It should be entirely unnecessary, because remap_pfn_range(), which is
> what the various dma_mmap functiosn are built on top of, does already
> unconditionally adjust the vma flags:

Not all dma_mmap_*() implementations use remap_pfn_range() though. For 
instance on ARM where one set of DMA ops uses vm_map_pages(), but AFAICS 
not all the relevant drivers would set VM_MIXEDMAP to prevent reaching 
the BUG_ON(vma->vm_flags & VM_PFNMAP) in there.

Robin.

> https://elixir.bootlin.com/linux/v6.1-rc6/source/mm/memory.c#L2518
> 
> More importantly, it does uncondtionally set VM_PFNMAP, so clearing
> that does not make much sense.
> 
> Patch motived by discussions around enforcing VM_PFNMAP semantics for
> all dma-buf users, where Thomas asked why dma helpers will work with
> that dma_buf_mmap() contract.
> 
> References: https://lore.kernel.org/dri-devel/5c3c8d4f-2c06-9210-b00a-4d0ff6f6fbb7@suse.de/
> Cc: Laurent Pinchart <laurent.pinchart+renesas at ideasonboard.com>
> Cc: Dave Airlie <airlied at redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Maxime Ripard <mripard at kernel.org>
> Cc: Thomas Zimmermann <tzimmermann at suse.de>
> Cc: Sumit Semwal <sumit.semwal at linaro.org>
> Cc: "Christian König" <christian.koenig at amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> ---
>   drivers/gpu/drm/drm_gem_dma_helper.c | 7 ++-----
>   1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_dma_helper.c b/drivers/gpu/drm/drm_gem_dma_helper.c
> index 1e658c448366..637a5cc62457 100644
> --- a/drivers/gpu/drm/drm_gem_dma_helper.c
> +++ b/drivers/gpu/drm/drm_gem_dma_helper.c
> @@ -525,13 +525,10 @@ int drm_gem_dma_mmap(struct drm_gem_dma_object *dma_obj, struct vm_area_struct *
>   	int ret;
>   
>   	/*
> -	 * Clear the VM_PFNMAP flag that was set by drm_gem_mmap(), and set the
> -	 * vm_pgoff (used as a fake buffer offset by DRM) to 0 as we want to map
> -	 * the whole buffer.
> +	 * Set the vm_pgoff (used as a fake buffer offset by DRM) to 0 as we
> +	 * want to map the whole buffer.
>   	 */
>   	vma->vm_pgoff -= drm_vma_node_start(&obj->vma_node);
> -	vma->vm_flags &= ~VM_PFNMAP;
> -	vma->vm_flags |= VM_DONTEXPAND;
>   
>   	if (dma_obj->map_noncoherent) {
>   		vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);