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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxxxxxxxxxxxxxx>
Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
---
  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.

Regards,

Tvrtko

+		}
+		if (obj->last_write_req)
+			args->busy |= obj->last_write_req->ring->exec_id;
+	}

  unref:
  	drm_gem_object_unreference(&obj->base);
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f5d89c845ede..4aa209483237 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2024,6 +2024,7 @@ static int logical_render_ring_init(struct drm_device *dev)

  	ring->name = "render ring";
  	ring->id = RCS;
+	ring->exec_id = I915_EXEC_RENDER;
  	ring->mmio_base = RENDER_RING_BASE;

  	logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT);
@@ -2073,6 +2074,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)

  	ring->name = "bsd ring";
  	ring->id = VCS;
+	ring->exec_id = I915_EXEC_BSD;
  	ring->mmio_base = GEN6_BSD_RING_BASE;

  	logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT);
@@ -2088,6 +2090,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev)

  	ring->name = "bsd2 ring";
  	ring->id = VCS2;
+	ring->exec_id = I915_EXEC_BSD;
  	ring->mmio_base = GEN8_BSD2_RING_BASE;

  	logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT);
@@ -2103,6 +2106,7 @@ static int logical_blt_ring_init(struct drm_device *dev)

  	ring->name = "blitter ring";
  	ring->id = BCS;
+	ring->exec_id = I915_EXEC_BLT;
  	ring->mmio_base = BLT_RING_BASE;

  	logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT);
@@ -2118,6 +2122,7 @@ static int logical_vebox_ring_init(struct drm_device *dev)

  	ring->name = "video enhancement ring";
  	ring->id = VECS;
+	ring->exec_id = I915_EXEC_VEBOX;
  	ring->mmio_base = VEBOX_RING_BASE;

  	logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8cd8aabcc3ff..310d151c0db2 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2680,6 +2680,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)

  	ring->name = "render ring";
  	ring->id = RCS;
+	ring->exec_id = I915_EXEC_RENDER;
  	ring->mmio_base = RENDER_RING_BASE;

  	if (INTEL_INFO(dev)->gen >= 8) {
@@ -2828,6 +2829,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)

  	ring->name = "bsd ring";
  	ring->id = VCS;
+	ring->exec_id = I915_EXEC_BSD;

  	ring->write_tail = ring_write_tail;
  	if (INTEL_INFO(dev)->gen >= 6) {
@@ -2904,6 +2906,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)

  	ring->name = "bsd2 ring";
  	ring->id = VCS2;
+	ring->exec_id = I915_EXEC_BSD;

  	ring->write_tail = ring_write_tail;
  	ring->mmio_base = GEN8_BSD2_RING_BASE;
@@ -2934,6 +2937,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)

  	ring->name = "blitter ring";
  	ring->id = BCS;
+	ring->exec_id = I915_EXEC_BLT;

  	ring->mmio_base = BLT_RING_BASE;
  	ring->write_tail = ring_write_tail;
@@ -2991,6 +2995,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)

  	ring->name = "video enhancement ring";
  	ring->id = VECS;
+	ring->exec_id = I915_EXEC_VEBOX;

  	ring->mmio_base = VEBOX_RING_BASE;
  	ring->write_tail = ring_write_tail;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 7349d9258191..2067f4700caa 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -156,6 +156,7 @@ struct  intel_engine_cs {
  	} id;
  #define I915_NUM_RINGS 5
  #define LAST_USER_RING (VECS + 1)
+	unsigned int exec_id;
  	u32		mmio_base;
  	struct		drm_device *dev;
  	struct intel_ringbuffer *buffer;

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux