[Mesa-dev] [PATCH v3 15/44] i965: Work around L3 state leaks during context switches.

[Jordan Justen]

From: Francisco Jerez <currojerez at riseup.net>

This is going to require some rather intrusive kernel changes to fix
properly, in the meantime (and forever on at least pre-v4.1 kernels)
we'll have to restore the hardware defaults at the end of every batch
in which the L3 configuration was changed to avoid interfering with
the DDX and GL clients that use an older non-L3-aware version of Mesa.

Reviewed-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
---
 src/mesa/drivers/dri/i965/brw_state.h         |  4 +++
 src/mesa/drivers/dri/i965/gen7_l3_state.c     | 48 +++++++++++++++++++++++++++
 src/mesa/drivers/dri/i965/intel_batchbuffer.c |  7 ++++
 src/mesa/drivers/dri/i965/intel_batchbuffer.h |  6 +++-
 4 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/src/mesa/drivers/dri/i965/brw_state.h b/src/mesa/drivers/dri/i965/brw_state.h
index 49f301a..b7c0039 100644
--- a/src/mesa/drivers/dri/i965/brw_state.h
+++ b/src/mesa/drivers/dri/i965/brw_state.h
@@ -380,6 +380,10 @@ void gen7_update_binding_table_from_array(struct brw_context *brw,
 void gen7_disable_hw_binding_tables(struct brw_context *brw);
 void gen7_reset_hw_bt_pool_offsets(struct brw_context *brw);
 
+/* gen7_l3_state.c */
+void
+gen7_restore_default_l3_config(struct brw_context *brw);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/src/mesa/drivers/dri/i965/gen7_l3_state.c b/src/mesa/drivers/dri/i965/gen7_l3_state.c
index b3b5b2e..022c01a 100644
--- a/src/mesa/drivers/dri/i965/gen7_l3_state.c
+++ b/src/mesa/drivers/dri/i965/gen7_l3_state.c
@@ -516,3 +516,51 @@ const struct brw_tracked_state gen7_l3_state = {
    },
    .emit = emit_l3_state
 };
+
+/**
+ * Hack to restore the default L3 configuration.
+ *
+ * This will be called at the end of every batch in order to reset the L3
+ * configuration to the default values for the time being until the kernel is
+ * fixed.  Until kernel commit 6702cf16e0ba8b0129f5aa1b6609d4e9c70bc13b
+ * (included in v4.1) we would set the MI_RESTORE_INHIBIT bit when submitting
+ * batch buffers for the default context used by the DDX, which meant that any
+ * context state changed by the GL would leak into the DDX, the assumption
+ * being that the DDX would initialize any state it cares about manually.  The
+ * DDX is however not careful enough to program an L3 configuration
+ * explicitly, and it makes assumptions about it (URB size) which won't hold
+ * and cause it to misrender if we let our L3 set-up to leak into the DDX.
+ *
+ * Since v4.1 of the Linux kernel the default context is saved and restored
+ * normally, so it's far less likely for our L3 programming to interfere with
+ * other contexts -- In fact restoring the default L3 configuration at the end
+ * of the batch will be redundant most of the time.  A kind of state leak is
+ * still possible though if the context making assumptions about L3 state is
+ * created immediately after our context was active (e.g. without the DDX
+ * default context being scheduled in between) because at present the DRM
+ * doesn't fully initialize the contents of newly created contexts and instead
+ * sets the MI_RESTORE_INHIBIT flag causing it to inherit the state from the
+ * last active context.
+ *
+ * It's possible to realize such a scenario if, say, an X server (or a GL
+ * application using an outdated non-L3-aware Mesa version) is started while
+ * another GL application is running and happens to have modified the L3
+ * configuration, or if no X server is running at all and a GL application
+ * using a non-L3-aware Mesa version is started after another GL application
+ * ran and modified the L3 configuration -- The latter situation can actually
+ * be reproduced easily on IVB in our CI system.
+ */
+void
+gen7_restore_default_l3_config(struct brw_context *brw)
+{
+   const struct brw_l3_weights w =
+      get_default_l3_weights(brw->intelScreen->devinfo, false, false);
+   const struct brw_l3_config *const cfg =
+      get_l3_config(brw->intelScreen->devinfo, w);
+
+   if (cfg != brw->l3.config && brw->can_do_pipelined_register_writes) {
+      setup_l3_config(brw, cfg);
+      update_urb_size(brw, cfg);
+      brw->l3.config = cfg;
+   }
+}
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
index 0363bd3..f778074 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
@@ -208,6 +208,13 @@ brw_finish_batch(struct brw_context *brw)
    brw_emit_query_end(brw);
 
    if (brw->batch.ring == RENDER_RING) {
+      /* Work around L3 state leaks into contexts set MI_RESTORE_INHIBIT which
+       * assume that the L3 cache is configured according to the hardware
+       * defaults.
+       */
+      if (brw->gen >= 7)
+         gen7_restore_default_l3_config(brw);
+
       /* We may also need to snapshot and disable OA counters. */
       brw_perf_monitor_finish_batch(brw);
 
diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
index 2b177d3..f473690 100644
--- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
+++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
@@ -30,8 +30,12 @@ extern "C" {
  *     - 5 dwords for initial mi_flush
  *     - 2 dwords for CC state setup
  *     - 5 dwords for the required pipe control at the end
+ *   - Restoring L3 configuration: (24 dwords = 96 bytes)
+ *     - 2*6 dwords for two PIPE_CONTROL flushes.
+ *     - 7 dwords for L3 configuration set-up.
+ *     - 5 dwords for L3 atomic set-up (on HSW).
  */
-#define BATCH_RESERVED 152
+#define BATCH_RESERVED 248
 
 struct intel_batchbuffer;
 
-- 
2.6.2