Re: [Intel-gfx] [PATCH 3/3] drm/i915: Check that we do find forcewake domain on gen11+

[Chris Wilson]

Quoting Mika Kuoppala (2019-09-13 15:16:52)
> By always requiring a valid forcewake domain, even
> FORCEWAKE_NONE, we can make assertions that accesses
> need to land on a valid domain and not go out of bounds.

So since we only look up restricted ranges in the fw_table, we could
just have a short selftest (intel_fw_table_check)  to assert those ranges
are filled and we have no holes.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH 3/3] drm/i915: Check that we do find forcewake domain on gen11+

[Mika Kuoppala]

By always requiring a valid forcewake domain, even
FORCEWAKE_NONE, we can make assertions that accesses
need to land on a valid domain and not go out of bounds.

Cc: Daniele Ceraolo Spurio 
Cc: Tvrtko Ursulin 
Cc: Chris Wilson 
Signed-off-by: Mika Kuoppala 
---
 drivers/gpu/drm/i915/intel_uncore.c | 46 +++--
 drivers/gpu/drm/i915/intel_uncore.h |  4 ++-
 2 files changed, 40 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
b/drivers/gpu/drm/i915/intel_uncore.c
index 94a97bf8c021..8e12b5334018 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -845,7 +845,7 @@ static int fw_range_cmp(u32 offset, const struct 
intel_forcewake_range *entry)
 })
 
 static enum forcewake_domains
-find_fw_domain(struct intel_uncore *uncore, u32 offset)
+__find_fw_domain(struct intel_uncore *uncore, u32 offset)
 {
const struct intel_forcewake_range *entry;
 
@@ -855,7 +855,7 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
fw_range_cmp);
 
if (!entry)
-   return 0;
+   return FORCEWAKE_INVALID;
 
/*
 * The list of FW domains depends on the SKU in gen11+ so we
@@ -872,6 +872,34 @@ find_fw_domain(struct intel_uncore *uncore, u32 offset)
return entry->domains;
 }
 
+static enum forcewake_domains
+find_fw_domain(struct intel_uncore *uncore, u32 offset)
+{
+   enum forcewake_domains fw_domains;
+
+   fw_domains = __find_fw_domain(uncore, offset);
+
+   if (fw_domains == FORCEWAKE_INVALID)
+   return FORCEWAKE_NONE;
+   else
+   return fw_domains;
+}
+
+static enum forcewake_domains
+find_fw_domain_check(struct intel_uncore *uncore, u32 offset)
+{
+   enum forcewake_domains fw_domains;
+
+   fw_domains = __find_fw_domain(uncore, offset);
+
+   if (WARN(fw_domains == FORCEWAKE_INVALID,
+"Unknown forcewake domain(s) accessed at 0x%x\n",
+offset))
+   return FORCEWAKE_ALL;
+
+   return fw_domains;
+}
+
 #define GEN_FW_RANGE(s, e, d) \
{ .start = (s), .end = (e), .domains = (d) }
 
@@ -900,10 +928,10 @@ static const struct intel_forcewake_range 
__vlv_fw_ranges[] = {
 })
 
 #define __gen11_fwtable_reg_read_fw_domains(uncore, offset) \
-   find_fw_domain(uncore, offset)
+   find_fw_domain_check(uncore, offset)
 
 #define __gen12_fwtable_reg_read_fw_domains(uncore, offset) \
-   find_fw_domain(uncore, offset)
+   find_fw_domain_check(uncore, offset)
 
 /* *Must* be sorted by offset! See intel_shadow_table_check(). */
 static const i915_reg_t gen8_shadowed_regs[] = {
@@ -1033,7 +1061,7 @@ static const struct intel_forcewake_range 
__chv_fw_ranges[] = {
 /* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
 static const struct intel_forcewake_range __gen9_fw_ranges[] = {
GEN_FW_RANGE(0x0, 0xaff, FORCEWAKE_BLITTER),
-   GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
+   GEN_FW_RANGE(0xb00, 0x1fff, FORCEWAKE_NONE), /* uncore range */
GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_BLITTER),
GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
@@ -1069,7 +1097,7 @@ static const struct intel_forcewake_range 
__gen9_fw_ranges[] = {
 /* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
 static const struct intel_forcewake_range __gen11_fw_ranges[] = {
GEN_FW_RANGE(0x0, 0xaff, FORCEWAKE_BLITTER),
-   GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
+   GEN_FW_RANGE(0xb00, 0x1fff, FORCEWAKE_NONE), /* uncore range */
GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_BLITTER),
GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
@@ -1092,7 +1120,7 @@ static const struct intel_forcewake_range 
__gen11_fw_ranges[] = {
GEN_FW_RANGE(0x1a000, 0x243ff, FORCEWAKE_BLITTER),
GEN_FW_RANGE(0x24400, 0x247ff, FORCEWAKE_RENDER),
GEN_FW_RANGE(0x24800, 0x3, FORCEWAKE_BLITTER),
-   GEN_FW_RANGE(0x4, 0x1b, 0),
+   GEN_FW_RANGE(0x4, 0x1b, FORCEWAKE_NONE),
GEN_FW_RANGE(0x1c, 0x1c3fff, FORCEWAKE_MEDIA_VDBOX0),
GEN_FW_RANGE(0x1c4000, 0x1c7fff, FORCEWAKE_MEDIA_VDBOX1),
GEN_FW_RANGE(0x1c8000, 0x1cbfff, FORCEWAKE_MEDIA_VEBOX0),
@@ -1105,7 +1133,7 @@ static const struct intel_forcewake_range 
__gen11_fw_ranges[] = {
 /* *Must* be sorted by offset ranges! See intel_fw_table_check(). */
 static const struct intel_forcewake_range __gen12_fw_ranges[] = {
GEN_FW_RANGE(0x0, 0xaff, FORCEWAKE_BLITTER),
-   GEN_FW_RANGE(0xb00, 0x1fff, 0), /* uncore range */
+   GEN_FW_RANGE(0xb00, 0x1fff, FORCEWAKE_NONE), /* uncore range */
GEN_FW_RANGE(0x2000, 0x26ff, FORCEWAKE_RENDER),
GEN_FW_RANGE(0x2700, 0x2fff, FORCEWAKE_BLITTER),
GEN_FW_RANGE(0x3000, 0x3fff, FORCEWAKE_RENDER),
@@ -1132