From patchwork Fri Jun 29 07:53:37 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Chris Wilson X-Patchwork-Id: 10495731 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 6B5686016C for ; Fri, 29 Jun 2018 07:55:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 52C5329119 for ; Fri, 29 Jun 2018 07:55:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4725C29123; Fri, 29 Jun 2018 07:55:28 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00, MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id E6CCF29119 for ; Fri, 29 Jun 2018 07:55:27 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 25A3F6EF96; Fri, 29 Jun 2018 07:55:20 +0000 (UTC) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from fireflyinternet.com (mail.fireflyinternet.com [109.228.58.192]) by gabe.freedesktop.org (Postfix) with ESMTPS id F11C36EF77 for ; Fri, 29 Jun 2018 07:54:54 +0000 (UTC) X-Default-Received-SPF: pass (skip=forwardok (res=PASS)) x-ip-name=78.156.65.138; Received: from haswell.alporthouse.com (unverified [78.156.65.138]) by fireflyinternet.com (Firefly Internet (M1)) with ESMTP id 12195526-1500050 for multiple; Fri, 29 Jun 2018 08:54:47 +0100 Received: by haswell.alporthouse.com (sSMTP sendmail emulation); Fri, 29 Jun 2018 08:54:46 +0100 From: Chris Wilson To: intel-gfx@lists.freedesktop.org Date: Fri, 29 Jun 2018 08:53:37 +0100 Message-Id: <20180629075348.27358-26-chris@chris-wilson.co.uk> X-Mailer: git-send-email 2.18.0 In-Reply-To: <20180629075348.27358-1-chris@chris-wilson.co.uk> References: <20180629075348.27358-1-chris@chris-wilson.co.uk> X-Originating-IP: 78.156.65.138 X-Country: code=GB country="United Kingdom" ip=78.156.65.138 Subject: [Intel-gfx] [PATCH 26/37] drm/i915/execlists: Flush the tasklet before unpinning X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Virus-Scanned: ClamAV using ClamSMTP Inside the execlists submission tasklet, we often make the mistake of assuming that everything beneath the request is available for use. However, the submission and the request live on two separate timelines, and the request contents may be freed from an early retirement before we have had a chance to run the submission tasklet (think ksoftirqd). To safeguard ourselves against any mistakes, flush the tasklet before we unpin the context if execlists still has a reference to this context. References: 60367132a214 ("drm/i915: Avoid use-after-free of ctx in request tracepoints") Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin --- drivers/gpu/drm/i915/i915_gem_context.h | 1 + drivers/gpu/drm/i915/intel_lrc.c | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_gem_context.h b/drivers/gpu/drm/i915/i915_gem_context.h index 8c55f8582469..bc440a7fe894 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.h +++ b/drivers/gpu/drm/i915/i915_gem_context.h @@ -160,6 +160,7 @@ struct i915_gem_context { /** engine: per-engine logical HW state */ struct intel_context { struct i915_gem_context *gem_context; + struct intel_engine_cs *active; struct i915_vma *state; struct intel_ring *ring; u32 *lrc_reg_state; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 8a4a1c0b986e..d70973c51a33 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -504,8 +504,11 @@ static void execlists_submit_ports(struct intel_engine_cs *engine) rq = port_unpack(&port[n], &count); if (rq) { GEM_BUG_ON(count > !n); - if (!count++) + if (!count++) { + GEM_BUG_ON(rq->hw_context->active); execlists_context_schedule_in(rq); + rq->hw_context->active = engine; + } port_set(&port[n], port_pack(rq, count)); desc = execlists_update_context(rq); GEM_DEBUG_EXEC(port[n].context_id = upper_32_bits(desc)); @@ -812,6 +815,8 @@ execlists_cancel_port_requests(struct intel_engine_execlists * const execlists) intel_engine_get_seqno(rq->engine)); GEM_BUG_ON(!execlists->active); + + rq->hw_context->active = NULL; execlists_context_schedule_out(rq, i915_request_completed(rq) ? INTEL_CONTEXT_SCHEDULE_OUT : @@ -1105,6 +1110,7 @@ static void process_csb(struct intel_engine_cs *engine) */ GEM_BUG_ON(!i915_request_completed(rq)); + rq->hw_context->active = NULL; execlists_context_schedule_out(rq, INTEL_CONTEXT_SCHEDULE_OUT); GEM_TRACE("%s completed ctx=%d\n", @@ -1362,6 +1368,23 @@ static void execlists_context_destroy(struct intel_context *ce) static void execlists_context_unpin(struct intel_context *ce) { + struct intel_engine_cs *engine; + + /* + * The tasklet may still be using a pointer to our state, via an + * old request. However, since we know we only unpin the context + * on retirement of the following request, we know that the last + * request referencing us will have had a completion CS interrupt. + * If we see that it is still active, it means that the tasklet hasn't + * had the chance to run yet; let it run before we teardown the + * reference it may use. + */ + engine = READ_ONCE(ce->active); + if (unlikely(engine)) { + tasklet_kill(&engine->execlists.tasklet); + GEM_BUG_ON(READ_ONCE(ce->active)); + } + intel_ring_unpin(ce->ring); ce->state->obj->pin_global--;