[Intel-gfx] [PATCH] drm/i915: Implement read-only support in whitelist selftest
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Thu Jun 20 15:43:12 UTC 2019
Hi,
You will need to send this not as reply to this thread so it is picked
up by CI and then can be merged.
But please also add a patch which adds that GEM_BUG_ON reg->flags check
we talked about.
Regards,
Tvrtko
On 18/06/2019 20:54, John.C.Harrison at Intel.com wrote:
> From: John Harrison <John.C.Harrison at Intel.com>
>
> Newer hardware supports extra feature in the whitelist registers. This
> patch updates the selftest to test that entries marked as read only
> are actually read only.
>
> 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 at Intel.com>
> CC: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 +--
> .../gpu/drm/i915/gt/selftest_workarounds.c | 53 +++++++++++++------
> drivers/gpu/drm/i915/i915_reg.h | 9 ++--
> 3 files changed, 48 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 93caa7b6d7a9..4429ee08b20e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -1028,7 +1028,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)
> @@ -1134,13 +1134,13 @@ printk(KERN_INFO "%-32s:%4d> Boo! [engine = %s, instance = %d, base = 0x%X, reg
>
> /* 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 eb6d3aa2c8cc..a0a88ec66861 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -399,6 +399,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)
> @@ -410,7 +414,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;
> @@ -482,12 +487,12 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> u32 srm, lrm, rsvd;
> u32 expect;
> int idx;
> + bool ro_reg;
>
> if (wo_register(engine, reg))
> continue;
>
> - if (ro_register(reg))
> - continue;
> + ro_reg = ro_register(reg);
>
> srm = MI_STORE_REGISTER_MEM;
> lrm = MI_LOAD_REGISTER_MEM;
> @@ -588,24 +593,36 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> }
>
> GEM_BUG_ON(values[ARRAY_SIZE(values) - 1] != 0xffffffff);
> - rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
> - if (!rsvd) {
> - pr_err("%s: Unable to write to whitelisted register %x\n",
> - engine->name, reg);
> - err = -EINVAL;
> - goto out_unpin;
> + if (ro_reg) {
> + rsvd = 0xFFFFFFFF;
> + } else {
> + rsvd = results[ARRAY_SIZE(values)]; /* detect write masking */
> + if (!rsvd) {
> + pr_err("%s: Unable to write to whitelisted register %x\n",
> + engine->name, reg);
> + err = -EINVAL;
> + goto out_unpin;
> + }
> }
>
> expect = results[0];
> idx = 1;
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> - expect = reg_write(expect, values[v], rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, values[v], rsvd);
> +
> if (results[idx] != expect)
> err++;
> idx++;
> }
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> - expect = reg_write(expect, ~values[v], rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, ~values[v], rsvd);
> +
> if (results[idx] != expect)
> err++;
> idx++;
> @@ -622,7 +639,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> u32 w = values[v];
>
> - expect = reg_write(expect, w, rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, w, rsvd);
> pr_info("Wrote %08x, read %08x, expect %08x\n",
> w, results[idx], expect);
> idx++;
> @@ -630,7 +650,10 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
> for (v = 0; v < ARRAY_SIZE(values); v++) {
> u32 w = ~values[v];
>
> - expect = reg_write(expect, w, rsvd);
> + if (ro_reg)
> + expect = results[0];
> + else
> + expect = reg_write(expect, w, rsvd);
> pr_info("Wrote %08x, read %08x, expect %08x\n",
> w, results[idx], expect);
> idx++;
> @@ -762,8 +785,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;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cc295a4f6e92..ba22e3b8f77e 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2513,13 +2513,16 @@ 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_MAX_NONPRIV_SLOTS 12
>
> #define GEN7_TLB_RD_ADDR _MMIO(0x4700)
>
More information about the Intel-gfx
mailing list