[Intel-gfx] [CI 17/25] drm/i915: Assign every HW context a unique ID

[Chris Wilson]

On Thu, Apr 28, 2016 at 07:08:18PM +0100, Dave Gordon wrote:
> On 28/04/16 16:32, Chris Wilson wrote:
> >On Thu, Apr 28, 2016 at 03:55:44PM +0100, Dave Gordon wrote:
> >>On 28/04/16 09:56, Chris Wilson wrote:
> >>>The hardware tracks contexts and expects all live contexts (those active
> >>>on the hardware) to have a unique identifier. This is used by the
> >>>hardware to assign pagefaults and the like to a particular context.
> >>>
> >>>v2: Reorder to make sure ctx->link is not left dangling if the
> >>>assignment of a hw_id fails (Mika).
> >>>
> >>>v3: We have 21bits of context space, not 20.
> >>>
> >>>Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >>>Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> >>>---
> >>>  drivers/gpu/drm/i915/i915_debugfs.c     |  2 +-
> >>>  drivers/gpu/drm/i915/i915_drv.h         | 10 +++++++++
> >>>  drivers/gpu/drm/i915/i915_gem_context.c | 36 +++++++++++++++++++++++++++++++++
> >>>  3 files changed, 47 insertions(+), 1 deletion(-)
> >>
> >>[snip]
> >>
> >>>+static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
> >>>+{
> >>>+	int ret;
> >>>+
> >>>+	ret = ida_simple_get(&dev_priv->context_hw_ida,
> >>>+			     0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
> >>>+	if (ret < 0) {
> >>>+		/* Contexts are only released when no longer active.
> >>>+		 * Flush any pending retires to hopefully release some
> >>>+		 * stale contexts and try again.
> >>>+		 */
> >>>+		i915_gem_retire_requests(dev_priv->dev);
> >>>+		ret = ida_simple_get(&dev_priv->context_hw_ida,
> >>>+				     0, MAX_CONTEXT_HW_ID, GFP_KERNEL);
> >>
> >>Should we once again reserve ID 0 to make it easier to identify
> >>things that have not yet been initialised properly?
> >
> >Otoh, 0 currently is the kernel context. Since we can only get an
> >uninitialised value for a use-after-free, I don't see an advantage here
> >above and beyond using poisoning, kasan or kmemcheck.
> >-Chris
> 
> I was thinking more of a use-before-complete-initialisation case,
> rather than a use-after-free. Obviously shouldn't happen, but it
> does (or used to) with the list-head in the request structure.

It's assigned to the context before use - that's a window we have
control over.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre