@@ -609,14 +609,18 @@ mi_set_context(struct intel_engine_cs *ring,
return ret;
}
-static void do_switch_fini_common(struct intel_engine_cs *ring,
- struct intel_context *from,
- struct intel_context *to)
+static struct intel_context *do_switch_fini_common(struct intel_engine_cs *ring,
+ struct intel_context *from,
+ struct intel_context *to)
{
+ struct intel_context *ret;
if (likely(from))
i915_gem_context_unreference(from);
i915_gem_context_reference(to);
+ ret = ring->last_context;
ring->last_context = to;
+
+ return ret;
}
static int do_switch_xcs(struct intel_engine_cs *ring,
@@ -762,14 +766,20 @@ static int do_switch_rcs(struct intel_engine_cs *ring,
*/
from->legacy_hw_ctx.rcs_state->dirty = 1;
BUG_ON(from->legacy_hw_ctx.rcs_state->ring != ring);
-
- /* obj is kept alive until the next request by its active ref */
- i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
}
uninitialized = !to->legacy_hw_ctx.initialized && from == NULL;
to->legacy_hw_ctx.initialized = true;
- do_switch_fini_common(ring, from, to);
+ /* From may have disappeared again after the context unref */
+ from = do_switch_fini_common(ring, from, to);
+ if (from != NULL) {
+ /* obj is kept alive until the next request by its active ref.
+ * XXX: The context needs to be unpinned last, or else we risk
+ * hitting evict/idle on the ppgtt free, which will call back
+ * into this, and we'll get a double unpin on this context
+ */
+ i915_gem_object_ggtt_unpin(from->legacy_hw_ctx.rcs_state);
+ }
if (uninitialized) {
ret = i915_gem_render_state_init(ring);
See the following for many more details. commit acc240d41ea1ab9c488a79219fb313b5b46265ae Author: Daniel Vetter <daniel.vetter@ffwll.ch> Date: Thu Dec 5 15:42:34 2013 +0100 drm/i915: Fix use-after-free in do_switch In this case, the issue is only for full PPGTT: do_switch context_unref ppgtt_release i915_gpu_idle switch_to_default from changes to default context This could be backported to the pre do_switch cleanup I did in this series. However, it's much cleaner and more obvious as a patch on top, so I'd really like to do this as a post cleanup patch. v2: There was a bug in the original patch where the ring->last_context was set too early. I am not sure how this wasn't being hit when I sent this previously. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Ben Widawsky <ben@bwidawsk.net> --- drivers/gpu/drm/i915/i915_gem_context.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)