On Tue, Dec 15, 2015 at 05:50:36PM +0000, Dave Gordon wrote: > 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@xxxxxxxxx wrote: > >>>From: Ankitprasad Sharma <ankitprasad.r.sharma@xxxxxxxxx> > >>> > >>>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@xxxxxxxxx> > >>>--- > >>> 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. We don't though. As for these objects, they are exposed no matter what since the user can access them concurrently and remap them to other devices without our intervention if they should so chose. The reloc patching depends on a userspace handshake, we have no idea if they are lying - let alone changing the contents on the fly. > And I thought we couldn't kmap stolen anyway? We can on gen2-5, but that isn't the point. The point is that the API is consistent and not piecemeal. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
