Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757134AbbGGMB4 (ORCPT ); Tue, 7 Jul 2015 08:01:56 -0400 Received: from mail-wi0-f177.google.com ([209.85.212.177]:36734 "EHLO mail-wi0-f177.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756802AbbGGMBs (ORCPT ); Tue, 7 Jul 2015 08:01:48 -0400 Date: Tue, 7 Jul 2015 14:04:33 +0200 From: Daniel Vetter To: Daniel Kurtz Cc: Daniel Vetter , Kees Cook , Douglas Anderson , stable@vger.kernel.org, Mark Yao , David Airlie , Heiko Stuebner , "open list:DRM DRIVERS FOR ROCKCHIP" , "moderated list:ARM/Rockchip SoC support" , "open list:ARM/Rockchip SoC support" , open list Subject: Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers Message-ID: <20150707120433.GO7568@phenom.ffwll.local> Mail-Followup-To: Daniel Kurtz , Kees Cook , Douglas Anderson , stable@vger.kernel.org, Mark Yao , David Airlie , Heiko Stuebner , "open list:DRM DRIVERS FOR ROCKCHIP" , "moderated list:ARM/Rockchip SoC support" , "open list:ARM/Rockchip SoC support" , open list References: <1436259816-31090-1-git-send-email-djkurtz@chromium.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1436259816-31090-1-git-send-email-djkurtz@chromium.org> X-Operating-System: Linux phenom 4.0.0-rc3+ User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4315 Lines: 132 On Tue, Jul 07, 2015 at 05:03:36PM +0800, Daniel Kurtz wrote: > Rather than (incompletely [0]) re-implementing drm_gem_mmap() and > drm_gem_mmap_obj() helpers, call them directly from the rockchip mmap > routines. > > Once the core functions return successfully, the rockchip mmap routines > can still use dma_mmap_attrs() to simply mmap the entire buffer. > > [0] Previously, we were performing the mmap() without first taking a > reference on the underlying gem buffer. This could leak ptes if the gem > object is destroyed while userspace is still holding the mapping. > > Signed-off-by: Daniel Kurtz > Reviewed-by: Daniel Vetter > Cc: stable@vger.kernel.org Applied to topic/drm-fixes to make sure it won't get lost, but I expect rockchip maintainers to pick this one up. -Daniel > > --- > drivers/gpu/drm/rockchip/rockchip_drm_gem.c | 67 +++++++++++++++-------------- > 1 file changed, 34 insertions(+), 33 deletions(-) > > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > index eb2282c..eba5f8a 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_gem.c > @@ -54,55 +54,56 @@ static void rockchip_gem_free_buf(struct rockchip_gem_object *rk_obj) > &rk_obj->dma_attrs); > } > > -int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > - struct vm_area_struct *vma) > +static int rockchip_drm_gem_object_mmap(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > + > { > + int ret; > struct rockchip_gem_object *rk_obj = to_rockchip_obj(obj); > struct drm_device *drm = obj->dev; > - unsigned long vm_size; > > - vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > - vm_size = vma->vm_end - vma->vm_start; > - > - if (vm_size > obj->size) > - return -EINVAL; > + /* > + * dma_alloc_attrs() allocated a struct page table for rk_obj, so clear > + * VM_PFNMAP flag that was set by drm_gem_mmap_obj()/drm_gem_mmap(). > + */ > + vma->vm_flags &= ~VM_PFNMAP; > > - return dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > + ret = dma_mmap_attrs(drm->dev, vma, rk_obj->kvaddr, rk_obj->dma_addr, > obj->size, &rk_obj->dma_attrs); > + if (ret) > + drm_gem_vm_close(vma); > + > + return ret; > } > > -/* drm driver mmap file operations */ > -int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +int rockchip_gem_mmap_buf(struct drm_gem_object *obj, > + struct vm_area_struct *vma) > { > - struct drm_file *priv = filp->private_data; > - struct drm_device *dev = priv->minor->dev; > - struct drm_gem_object *obj; > - struct drm_vma_offset_node *node; > + struct drm_device *drm = obj->dev; > int ret; > > - if (drm_device_is_unplugged(dev)) > - return -ENODEV; > + mutex_lock(&drm->struct_mutex); > + ret = drm_gem_mmap_obj(obj, obj->size, vma); > + mutex_unlock(&drm->struct_mutex); > + if (ret) > + return ret; > > - mutex_lock(&dev->struct_mutex); > + return rockchip_drm_gem_object_mmap(obj, vma); > +} > > - node = drm_vma_offset_exact_lookup(dev->vma_offset_manager, > - vma->vm_pgoff, > - vma_pages(vma)); > - if (!node) { > - mutex_unlock(&dev->struct_mutex); > - DRM_ERROR("failed to find vma node.\n"); > - return -EINVAL; > - } else if (!drm_vma_node_is_allowed(node, filp)) { > - mutex_unlock(&dev->struct_mutex); > - return -EACCES; > - } > +/* drm driver mmap file operations */ > +int rockchip_gem_mmap(struct file *filp, struct vm_area_struct *vma) > +{ > + struct drm_gem_object *obj; > + int ret; > > - obj = container_of(node, struct drm_gem_object, vma_node); > - ret = rockchip_gem_mmap_buf(obj, vma); > + ret = drm_gem_mmap(filp, vma); > + if (ret) > + return ret; > > - mutex_unlock(&dev->struct_mutex); > + obj = vma->vm_private_data; > > - return ret; > + return rockchip_drm_gem_object_mmap(obj, vma); > } > > struct rockchip_gem_object * > -- > 2.4.3.573.g4eafbef > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/