[Intel-gfx] [PATCH] drm/i915: Allow userptr backchannel for passing aroung GTT mappings

[Tvrtko Ursulin]

On 04/02/2015 05:27 PM, Chris Wilson wrote:
> On Thu, Apr 02, 2015 at 05:11:58PM +0100, Tvrtko Ursulin wrote:
>>> +static struct drm_i915_gem_object *
>>> +find_object_from_vma(struct drm_device *dev,
>>> +		     struct drm_i915_gem_userptr *args)
>>> +{
>>> +	struct drm_i915_gem_object *obj = NULL;
>>> +	struct vm_area_struct *vma;
>>> +
>>> +	down_read(&current->mm->mmap_sem);
>>> +	vma = find_vma(current->mm, args->user_ptr);
>>> +	if (vma == NULL)
>>> +		goto out;
>>> +
>>> +	if (vma->vm_ops != dev->driver->gem_vm_ops)
>>> +		goto out;
>>> +
>>> +	if (vma->vm_start != args->user_ptr ||
>>> +	    vma->vm_end != args->user_ptr + args->user_size) {
>>> +		obj = ERR_PTR(-EINVAL);
>>> +		goto out;
>>> +	}
>>> +
>>> +	obj = to_intel_bo(vma->vm_private_data);
>>> +	drm_gem_object_reference(obj);
>>
>> Hm, can't this race with last unreference in general, and with
>> cleanup worker with userptr objects?
>
> The vma holds a reference to the object and that reference is dropped
> whilst holding down_write(current->mm->mmap_sem), hence I think the
> down_read(current->mm->mmap_sem) is sufficient locking to acquire a
> reference for ourselves.

Yes, I completely forgot about this little detail. :)

>>> +out:		 ret = drm_gem_handle_create(file, &obj->base, &handle);
>>>
>>>   	/* drop reference from allocate - handle holds it now */
>>>   	drm_gem_object_unreference_unlocked(&obj->base);
>>
>> Thing I don't like is how the user of this has no idea what kind of
>> object it "imported". Maybe it doesn't matter, hm. Need to think
>> about it more.
>
> Indeed. But since the userptr is a strict subset of the general bo, if
> they follow the rules for userptr bo then they won't notice a
> difference. read/writes into the memory block are coherent (since the
> pointer is wc) so as far the caller is concerned I think it just ends up
> being slower cpu side, faster gpu side than a system memory snooped
> userptr bo.

Is access to non-userptr bo's interesting to users of this - could we 
reject that and not lose out?

Regards,

Tvrtko