Linux-Rockchip Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Mark yao <mark.yao@rock-chips.com>
To: Daniel Kurtz <djkurtz@chromium.org>,
	Kees Cook <keescook@chromium.org>,
	Douglas Anderson <dianders@chromium.org>,
	stable@vger.kernel.org, David Airlie <airlied@linux.ie>,
	Heiko Stuebner <heiko@sntech.de>,
	open@263.net, list@263.net,
	DRM DRIVERS FOR ROCKCHIP <dri-devel@lists.freedesktop.org>,
	moderated@263.netlist@263.net,
	ARM/Rockchip SoC support
	<linux-arm-kernel@lists.infradead.org>open@263.netlist@263.net,
	ARM/Rockchip SoC support <linux-rockchip@lists.infradead.org>,
	open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] drm/rockchip: use drm_gem_mmap helpers
Date: Wed, 08 Jul 2015 12:06:53 +0800	[thread overview]
Message-ID: <559CA1DD.9070905@rock-chips.com> (raw)
In-Reply-To: <20150707120433.GO7568@phenom.ffwll.local>


[-- Attachment #1.1: Type: text/plain, Size: 4465 bytes --]

On 2015年07月07日 20:04, Daniel Vetter wrote:
> 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 <djkurtz@chromium.org>
>> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> 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
I try to pick this patch up, but found it conflicts with patch [0].  Can 
you fix it?

[0]https://patchwork.kernel.org/patch/6226591/

-Mark
>> ---
>>   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
>>


-- 
Mark


[-- Attachment #1.2: Type: text/html, Size: 5414 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

  reply	other threads:[~2015-07-08  4:06 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-07  9:03 [PATCH] drm/rockchip: use drm_gem_mmap helpers Daniel Kurtz
2015-07-07 12:04 ` Daniel Vetter
2015-07-08  4:06   ` Mark yao [this message]
2015-07-08  7:35     ` Daniel Vetter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=559CA1DD.9070905@rock-chips.com \
    --to=mark.yao@rock-chips.com \
    --cc=airlied@linux.ie \
    --cc=dianders@chromium.org \
    --cc=djkurtz@chromium.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=keescook@chromium.org \
    --cc=list@263.net \
    --cc=moderated@263.netlist \
    --cc=open@263.net \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox