Commit 426960be authored by Chris Wilson's avatar Chris Wilson Committed by Tvrtko Ursulin

drm/i915: Seal busy-ioctl uABI and prevent leaking of internal ids

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.

v2: Update the uAPI documentation to explain the identifiers.
Signed-off-by: default avatarChris Wilson <chris@chris-wilson.co.uk>
Testcase: igt/gem_busy
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: default avatarTvrtko Ursulin <tvrtko.ursulin@intel.com>
Acked-by: default avatarDaniel Vetter <daniel.vetter@ffwll.ch>
Link: http://patchwork.freedesktop.org/patch/msgid/1452876706-21620-1-git-send-email-chris@chris-wilson.co.uk
parent de1add36
...@@ -4358,10 +4358,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, ...@@ -4358,10 +4358,20 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data,
if (ret) if (ret)
goto unref; goto unref;
BUILD_BUG_ON(I915_NUM_RINGS > 16); args->busy = 0;
args->busy = obj->active << 16; if (obj->active) {
if (obj->last_write_req) int i;
args->busy |= obj->last_write_req->ring->id;
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 (obj->last_write_req)
args->busy |= obj->last_write_req->ring->exec_id;
}
unref: unref:
drm_gem_object_unreference(&obj->base); drm_gem_object_unreference(&obj->base);
......
...@@ -2085,6 +2085,7 @@ static int logical_render_ring_init(struct drm_device *dev) ...@@ -2085,6 +2085,7 @@ static int logical_render_ring_init(struct drm_device *dev)
ring->name = "render ring"; ring->name = "render ring";
ring->id = RCS; ring->id = RCS;
ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE; ring->mmio_base = RENDER_RING_BASE;
logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT); logical_ring_default_irqs(ring, GEN8_RCS_IRQ_SHIFT);
...@@ -2135,6 +2136,7 @@ static int logical_bsd_ring_init(struct drm_device *dev) ...@@ -2135,6 +2136,7 @@ static int logical_bsd_ring_init(struct drm_device *dev)
ring->name = "bsd ring"; ring->name = "bsd ring";
ring->id = VCS; ring->id = VCS;
ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN6_BSD_RING_BASE; ring->mmio_base = GEN6_BSD_RING_BASE;
logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT); logical_ring_default_irqs(ring, GEN8_VCS1_IRQ_SHIFT);
...@@ -2150,6 +2152,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev) ...@@ -2150,6 +2152,7 @@ static int logical_bsd2_ring_init(struct drm_device *dev)
ring->name = "bsd2 ring"; ring->name = "bsd2 ring";
ring->id = VCS2; ring->id = VCS2;
ring->exec_id = I915_EXEC_BSD;
ring->mmio_base = GEN8_BSD2_RING_BASE; ring->mmio_base = GEN8_BSD2_RING_BASE;
logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT); logical_ring_default_irqs(ring, GEN8_VCS2_IRQ_SHIFT);
...@@ -2165,6 +2168,7 @@ static int logical_blt_ring_init(struct drm_device *dev) ...@@ -2165,6 +2168,7 @@ static int logical_blt_ring_init(struct drm_device *dev)
ring->name = "blitter ring"; ring->name = "blitter ring";
ring->id = BCS; ring->id = BCS;
ring->exec_id = I915_EXEC_BLT;
ring->mmio_base = BLT_RING_BASE; ring->mmio_base = BLT_RING_BASE;
logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT); logical_ring_default_irqs(ring, GEN8_BCS_IRQ_SHIFT);
...@@ -2180,6 +2184,7 @@ static int logical_vebox_ring_init(struct drm_device *dev) ...@@ -2180,6 +2184,7 @@ static int logical_vebox_ring_init(struct drm_device *dev)
ring->name = "video enhancement ring"; ring->name = "video enhancement ring";
ring->id = VECS; ring->id = VECS;
ring->exec_id = I915_EXEC_VEBOX;
ring->mmio_base = VEBOX_RING_BASE; ring->mmio_base = VEBOX_RING_BASE;
logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT); logical_ring_default_irqs(ring, GEN8_VECS_IRQ_SHIFT);
......
...@@ -2683,6 +2683,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ...@@ -2683,6 +2683,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
ring->name = "render ring"; ring->name = "render ring";
ring->id = RCS; ring->id = RCS;
ring->exec_id = I915_EXEC_RENDER;
ring->mmio_base = RENDER_RING_BASE; ring->mmio_base = RENDER_RING_BASE;
if (INTEL_INFO(dev)->gen >= 8) { if (INTEL_INFO(dev)->gen >= 8) {
...@@ -2831,6 +2832,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ...@@ -2831,6 +2832,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
ring->name = "bsd ring"; ring->name = "bsd ring";
ring->id = VCS; ring->id = VCS;
ring->exec_id = I915_EXEC_BSD;
ring->write_tail = ring_write_tail; ring->write_tail = ring_write_tail;
if (INTEL_INFO(dev)->gen >= 6) { if (INTEL_INFO(dev)->gen >= 6) {
...@@ -2907,6 +2909,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev) ...@@ -2907,6 +2909,7 @@ int intel_init_bsd2_ring_buffer(struct drm_device *dev)
ring->name = "bsd2 ring"; ring->name = "bsd2 ring";
ring->id = VCS2; ring->id = VCS2;
ring->exec_id = I915_EXEC_BSD;
ring->write_tail = ring_write_tail; ring->write_tail = ring_write_tail;
ring->mmio_base = GEN8_BSD2_RING_BASE; ring->mmio_base = GEN8_BSD2_RING_BASE;
...@@ -2937,6 +2940,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) ...@@ -2937,6 +2940,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
ring->name = "blitter ring"; ring->name = "blitter ring";
ring->id = BCS; ring->id = BCS;
ring->exec_id = I915_EXEC_BLT;
ring->mmio_base = BLT_RING_BASE; ring->mmio_base = BLT_RING_BASE;
ring->write_tail = ring_write_tail; ring->write_tail = ring_write_tail;
...@@ -2994,6 +2998,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev) ...@@ -2994,6 +2998,7 @@ int intel_init_vebox_ring_buffer(struct drm_device *dev)
ring->name = "video enhancement ring"; ring->name = "video enhancement ring";
ring->id = VECS; ring->id = VECS;
ring->exec_id = I915_EXEC_VEBOX;
ring->mmio_base = VEBOX_RING_BASE; ring->mmio_base = VEBOX_RING_BASE;
ring->write_tail = ring_write_tail; ring->write_tail = ring_write_tail;
......
...@@ -157,6 +157,7 @@ struct intel_engine_cs { ...@@ -157,6 +157,7 @@ struct intel_engine_cs {
} id; } id;
#define I915_NUM_RINGS 5 #define I915_NUM_RINGS 5
#define _VCS(n) (VCS + (n)) #define _VCS(n) (VCS + (n))
unsigned int exec_id;
u32 mmio_base; u32 mmio_base;
struct drm_device *dev; struct drm_device *dev;
struct intel_ringbuffer *buffer; struct intel_ringbuffer *buffer;
......
...@@ -812,10 +812,35 @@ struct drm_i915_gem_busy { ...@@ -812,10 +812,35 @@ struct drm_i915_gem_busy {
/** Handle of the buffer to check for busy */ /** Handle of the buffer to check for busy */
__u32 handle; __u32 handle;
/** Return busy status (1 if busy, 0 if idle). /** Return busy status
* The high word is used to indicate on which rings the object *
* currently resides: * A return of 0 implies that the object is idle (after
* 16:31 - busy (r or r/w) rings (16 render, 17 bsd, 18 blt, etc) * having flushed any pending activity), and a non-zero return that
* the object is still in-flight on the GPU. (The GPU has not yet
* signaled completion for all pending requests that reference the
* object.)
*
* The returned dword is split into two fields to indicate both
* the engines on which the object is being read, and the
* engine on which it is currently being written (if any).
*
* The low word (bits 0:15) indicate if the object is being written
* to by any engine (there can only be one, as the GEM implicit
* synchronisation rules force writes to be serialised). Only the
* engine for the last write is reported.
*
* The high word (bits 16:31) are a bitmask of which engines are
* currently reading from the object. Multiple engines may be
* reading from the object simultaneously.
*
* The value of each engine is the same as specified in the
* EXECBUFFER2 ioctl, i.e. I915_EXEC_RENDER, I915_EXEC_BSD etc.
* Note I915_EXEC_DEFAULT is a symbolic value and is mapped to
* the I915_EXEC_RENDER engine for execution, and so it is never
* reported as active itself. Some hardware may have parallel
* execution engines, e.g. multiple media engines, which are
* mapped to the same identifier in the EXECBUFFER2 ioctl and
* so are not separately reported for busyness.
*/ */
__u32 busy; __u32 busy;
}; };
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment