[Intel-gfx] [PATCH] drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

[Chris Wilson]

On Fri, Jan 15, 2016 at 11:58:32AM +0000, Tvrtko Ursulin wrote:
> 
> On 15/01/16 11:06, Chris Wilson wrote:
> >Tvrtko was looking through the execbuffer-ioctl and noticed that the
> >uABI was tightly coupled to our internal engine identifiers. Close
> >inspection also revealed that we leak those internal engine identifiers
> >through the busy-ioctl, and those internal identifiers already do not
> >match the user identifiers. Fortuitiously, there is only one user of the
> >set of busy rings from the busy-ioctl, and they only wish to choose
> >between the RENDER and the BLT engines.
> >
> >Let's fix the userspace ABI while we still can.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> >---
> >  drivers/gpu/drm/i915/i915_gem.c         | 18 ++++++++++++++----
> >  drivers/gpu/drm/i915/intel_lrc.c        |  5 +++++
> >  drivers/gpu/drm/i915/intel_ringbuffer.c |  5 +++++
> >  drivers/gpu/drm/i915/intel_ringbuffer.h |  1 +
> >  4 files changed, 25 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> >index bb44bad15403..85797813a3de 100644
> >--- a/drivers/gpu/drm/i915/i915_gem.c
> >+++ b/drivers/gpu/drm/i915/i915_gem.c
> >@@ -4328,10 +4328,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
> >  	if (ret)
> >  		goto unref;
> >
> >-	BUILD_BUG_ON(I915_NUM_RINGS > 16);
> >-	args->busy = obj->active << 16;
> >-	if (obj->last_write_req)
> >-		args->busy |= obj->last_write_req->ring->id;
> >+	args->busy = 0;
> >+	if (obj->active) {
> >+		int i;
> >+
> >+		for (i = 0; i < I915_NUM_RINGS; i++) {
> >+			struct drm_i915_gem_request *req;
> >+
> >+			req = obj->last_read_req[i];
> >+			if (req)
> >+				args->busy |= 1 << (16 + req->ring->exec_id);
> 
> If I got it right bit 16 was RCS, now will always be clear. And
> blitter was bit 17 and now is 19.

Sssh. You weren't meant to point that out.

I am willing to take the hit in order to decouple the uABI from
internals.

I am also willing to codify this busy-ioctl ABI into igt!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre