drm/i915: Add test for invalid flag bits in whitelist entries

As per review feedback by Tvrtko, added a check that no invalid bits
are being set in the whitelist flags fields.

Also updated the read/write access definitions to make it clearer that
they are an enum field not a set of single bit flags.

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
CC: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190712070745.35239-2-John.C.Harrison@Intel.com

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 9e06928..95be0f1 100644 (file)
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -1011,6 +1011,20 @@ bool intel_gt_verify_workarounds(struct intel_gt *gt, const char *from)
        return wa_list_verify(gt->uncore, &gt->i915->gt_wa_list, from);
 }
 
+static inline bool is_nonpriv_flags_valid(u32 flags)
+{
+       /* Check only valid flag bits are set */
+       if (flags & ~RING_FORCE_TO_NONPRIV_MASK_VALID)
+               return false;
+
+       /* NB: Only 3 out of 4 enum values are valid for access field */
+       if ((flags & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+           RING_FORCE_TO_NONPRIV_ACCESS_INVALID)
+               return false;
+
+       return true;
+}
+
 static void
 whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
 {
@@ -1021,6 +1035,9 @@ whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
        if (GEM_DEBUG_WARN_ON(wal->count >= RING_MAX_NONPRIV_SLOTS))
                return;
 
+       if (GEM_DEBUG_WARN_ON(!is_nonpriv_flags_valid(flags)))
+               return;
+
        wa.reg.reg |= flags;
        _wa_add(wal, &wa);
 }
@@ -1028,7 +1045,7 @@ whitelist_reg_ext(struct i915_wa_list *wal, i915_reg_t reg, u32 flags)
 static void
 whitelist_reg(struct i915_wa_list *wal, i915_reg_t reg)
 {
-       whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_RW);
+       whitelist_reg_ext(wal, reg, RING_FORCE_TO_NONPRIV_ACCESS_RW);
 }
 
 static void gen9_whitelist_build(struct i915_wa_list *w)
@@ -1109,7 +1126,7 @@ static void cfl_whitelist_build(struct intel_engine_cs *engine)
         *   - PS_DEPTH_COUNT_UDW
         */
        whitelist_reg_ext(w, PS_INVOCATION_COUNT,
-                         RING_FORCE_TO_NONPRIV_RD |
+                         RING_FORCE_TO_NONPRIV_ACCESS_RD |
                          RING_FORCE_TO_NONPRIV_RANGE_4);
 }
 
@@ -1149,20 +1166,20 @@ static void icl_whitelist_build(struct intel_engine_cs *engine)
                 *   - PS_DEPTH_COUNT_UDW
                 */
                whitelist_reg_ext(w, PS_INVOCATION_COUNT,
-                                 RING_FORCE_TO_NONPRIV_RD |
+                                 RING_FORCE_TO_NONPRIV_ACCESS_RD |
                                  RING_FORCE_TO_NONPRIV_RANGE_4);
                break;
 
        case VIDEO_DECODE_CLASS:
                /* hucStatusRegOffset */
                whitelist_reg_ext(w, _MMIO(0x2000 + engine->mmio_base),
-                                 RING_FORCE_TO_NONPRIV_RD);
+                                 RING_FORCE_TO_NONPRIV_ACCESS_RD);
                /* hucUKernelHdrInfoRegOffset */
                whitelist_reg_ext(w, _MMIO(0x2014 + engine->mmio_base),
-                                 RING_FORCE_TO_NONPRIV_RD);
+                                 RING_FORCE_TO_NONPRIV_ACCESS_RD);
                /* hucStatus2RegOffset */
                whitelist_reg_ext(w, _MMIO(0x23B0 + engine->mmio_base),
-                                 RING_FORCE_TO_NONPRIV_RD);
+                                 RING_FORCE_TO_NONPRIV_ACCESS_RD);
                break;
 
        default:

diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
index fa01ea7..466dcc8 100644 (file)
--- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
@@ -397,6 +397,10 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
        enum intel_platform platform = INTEL_INFO(engine->i915)->platform;
        int i;
 
+       if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+            RING_FORCE_TO_NONPRIV_ACCESS_WR)
+               return true;
+
        for (i = 0; i < ARRAY_SIZE(wo_registers); i++) {
                if (wo_registers[i].platform == platform &&
                    wo_registers[i].reg == reg)
@@ -408,7 +412,8 @@ static bool wo_register(struct intel_engine_cs *engine, u32 reg)
 
 static bool ro_register(u32 reg)
 {
-       if (reg & RING_FORCE_TO_NONPRIV_RD)
+       if ((reg & RING_FORCE_TO_NONPRIV_ACCESS_MASK) ==
+            RING_FORCE_TO_NONPRIV_ACCESS_RD)
                return true;
 
        return false;
@@ -760,8 +765,8 @@ static int read_whitelisted_registers(struct i915_gem_context *ctx,
                u64 offset = results->node.start + sizeof(u32) * i;
                u32 reg = i915_mmio_reg_offset(engine->whitelist.list[i].reg);
 
-               /* Clear RD only and WR only flags */
-               reg &= ~(RING_FORCE_TO_NONPRIV_RD | RING_FORCE_TO_NONPRIV_WR);
+               /* Clear access permission field */
+               reg &= ~RING_FORCE_TO_NONPRIV_ACCESS_MASK;
 
                *cs++ = srm;
                *cs++ = reg;
@@ -931,7 +936,8 @@ check_whitelisted_registers(struct intel_engine_cs *engine,
        for (i = 0; i < engine->whitelist.count; i++) {
                const struct i915_wa *wa = &engine->whitelist.list[i];
 
-               if (i915_mmio_reg_offset(wa->reg) & RING_FORCE_TO_NONPRIV_RD)
+               if (i915_mmio_reg_offset(wa->reg) &
+                   RING_FORCE_TO_NONPRIV_ACCESS_RD)
                        continue;
 
                if (!fn(engine, a[i], b[i], wa->reg))

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 3ff659a..e14c9b7 100644 (file)
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2522,13 +2522,19 @@ enum i915_power_well_id {
 #define   RING_WAIT_SEMAPHORE  (1 << 10) /* gen6+ */
 
 #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + (i) * 4)
-#define   RING_FORCE_TO_NONPRIV_RW             (0 << 28)    /* CFL+ & Gen11+ */
-#define   RING_FORCE_TO_NONPRIV_RD             (1 << 28)
-#define   RING_FORCE_TO_NONPRIV_WR             (2 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_RW      (0 << 28)    /* CFL+ & Gen11+ */
+#define   RING_FORCE_TO_NONPRIV_ACCESS_RD      (1 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_WR      (2 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_INVALID (3 << 28)
+#define   RING_FORCE_TO_NONPRIV_ACCESS_MASK    (3 << 28)
 #define   RING_FORCE_TO_NONPRIV_RANGE_1                (0 << 0)     /* CFL+ & Gen11+ */
 #define   RING_FORCE_TO_NONPRIV_RANGE_4                (1 << 0)
 #define   RING_FORCE_TO_NONPRIV_RANGE_16       (2 << 0)
 #define   RING_FORCE_TO_NONPRIV_RANGE_64       (3 << 0)
+#define   RING_FORCE_TO_NONPRIV_RANGE_MASK     (3 << 0)
+#define   RING_FORCE_TO_NONPRIV_MASK_VALID     \
+                                       (RING_FORCE_TO_NONPRIV_RANGE_MASK \
+                                       | RING_FORCE_TO_NONPRIV_ACCESS_MASK)
 #define   RING_MAX_NONPRIV_SLOTS  12
 
 #define GEN7_TLB_RD_ADDR       _MMIO(0x4700)