[Intel-gfx] [PATCH v5 11/11] drm/i915: make LRC status page teardown code a bit more robust

[Dave Gordon]

Having recently fixed a few ordering/lifetime bugs in this area, it
seems worthwhile putting a few more checks and warnings here, to provide
early detection of any future code changes that cause problems;
especially as we hope to rework the pinning/refcounting of the kernel
context soon.

Specifically, we want to check that teardown_status_page is called
*before* the underlying storage is freed, and that by the time
intel_logical_ring_cleanup() is called, the status page mapping have
already been released. We'll also take care to undo the status page
kmapping only if it has been set up.

Signed-off-by: Dave Gordon <david.s.gordon at intel.com>
Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>
Cc: Chris Wilson <chris at chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 95ba8ec..b4deaca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2003,6 +2003,9 @@ void intel_logical_ring_cleanup(struct intel_engine_cs *ring)
 	i915_cmd_parser_fini_ring(ring);
 	i915_gem_batch_pool_fini(&ring->batch_pool);
 
+	/* Status page should have gone already */
+	WARN_ON(ring->status_page.page_addr);
+	WARN_ON(ring->status_page.obj);
 
 	ring->disable_lite_restore_wa = false;
 	ring->ctx_desc_template = 0;
@@ -2446,6 +2449,10 @@ void intel_lr_context_free(struct intel_context *ctx)
 			continue;
 
 		if (ctx == ctx->i915->kernel_context) {
+			/*
+			 * The HWSP is part of the kernel context
+			 * object in LRC mode, so tear it down *now*
+			 */
 			lrc_teardown_hardware_status_page(ringbuf->ring);
 			intel_unpin_ringbuffer_obj(ringbuf);
 			i915_gem_object_ggtt_unpin(ctx_obj);
@@ -2517,12 +2524,19 @@ static void lrc_setup_hardware_status_page(struct intel_engine_cs *ring)
 	POSTING_READ(RING_HWS_PGA(ring->mmio_base));
 }
 
+/* This should be called *before* the default context is destroyed */
 static void lrc_teardown_hardware_status_page(struct intel_engine_cs *ring)
 {
-	if (ring->status_page.obj) {
+	struct drm_i915_gem_object *dctx_obj = ring->status_page.obj;
+
+	WARN_ON(!dctx_obj);
+
+	if (ring->status_page.page_addr) {
 		kunmap(kmap_to_page(ring->status_page.page_addr));
-		ring->status_page.obj = NULL;
+		ring->status_page.page_addr = NULL;
 	}
+
+	ring->status_page.obj = NULL;
 }
 
 /**
-- 
1.9.1