Re: [PATCH 9/9] drm/i915: Fail the execbuff using stolen objects as batchbuffers

From: Dave Gordon <david.s.gordon@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	ankitprasad.r.sharma@intel.com, intel-gfx@lists.freedesktop.org,
	akash.goel@intel.com, shashidhar.hiremath@intel.com
Subject: Re: [PATCH 9/9] drm/i915: Fail the execbuff using stolen objects as batchbuffers
Date: Tue, 15 Dec 2015 17:50:36 +0000	[thread overview]
Message-ID: <567052EC.2010209@intel.com> (raw)
In-Reply-To: <20151215145401.GH24300@nuc-i3427.alporthouse.com>

On 15/12/15 14:54, Chris Wilson wrote:
> On Tue, Dec 15, 2015 at 02:41:47PM +0000, Dave Gordon wrote:
>> On 14/12/15 05:46, ankitprasad.r.sharma@intel.com wrote:
>>> From: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>>>
>>> Using stolen backed objects as a batchbuffer may result into a kernel
>>> panic during relocation. Added a check to prevent the panic and fail
>>> the execbuffer call. It is not recommended to use stolen object as
>>> a batchbuffer.
>>>
>>> Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> index 48ec484..d342f10 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> @@ -462,7 +462,9 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj,
>>>   	if (obj->active && pagefault_disabled())
>>>   		return -EFAULT;
>>>
>>> -	if (use_cpu_reloc(obj))
>>> +	if (obj->stolen)
>>> +		ret = -EINVAL;
>>
>> I'd rather reject ALL "weird" gem objects at the first opportunity,
>> so that none of the execbuffer code has to worry about stolen, phys,
>> dmabuf, etc ...
>>
>> 	if (obj->ops != &i915_gem_object_ops))
>> 		ret = -EINVAL;		/* No exotica please */
>
> No. All GEM objects are supposed to be first-class so that they are
> interchangeable through all aspects of the API (that becomes even more
> important with dma-buf interoperation). We have had to relax that for a
> couple of special categories (basically CPU mmapping) for certain clases
> that are not struct file backed. Though in principle, a gemfs would work
> just fine.
>
> The only restrictions we should ideally impose are those determined by
> hardware.
> -Chris

I don't think it's reasonable to place objects that the kernel driver 
cares about -- i.e. understands and decodes -- in memory areas that it 
does not manage, and which may be subject to arbitrary uncontrolled 
access by external hardware and/or processes.

And I thought we couldn't kmap stolen anyway?

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx