[Intel-gfx] [PATCH 1/2] drm/i915: Remove nonpriv flags when srm/lrm

[Lionel Landwerlin]

On 24/10/2019 13:38, Mika Kuoppala wrote:
> On testing the whitelists, using any of the nonpriv
> flags when trying to access the register offset will lead
> to failure.
>
> Define address mask to get the mmio offset in order
> to guard against any current and future flag usage.
>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/selftest_workarounds.c | 7 +++++--
>   drivers/gpu/drm/i915/i915_reg.h                | 1 +
>   2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/selftest_workarounds.c b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> index ef02920cec29..54324c6577dc 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_workarounds.c
> @@ -513,6 +513,9 @@ static int check_dirty_whitelist(struct i915_gem_context *ctx,
>   
>   		ro_reg = ro_register(reg);
>   
> +		/* Clear non priv flags */
> +		reg &= RING_FORCE_TO_NONPRIV_ADDRESS_MASK;
> +
>   		srm = MI_STORE_REGISTER_MEM;
>   		lrm = MI_LOAD_REGISTER_MEM;
>   		if (INTEL_GEN(ctx->i915) >= 8)
> @@ -810,8 +813,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 access permission field */
> -		reg &= ~RING_FORCE_TO_NONPRIV_ACCESS_MASK;
> +		/* Clear non priv flags */
> +		reg &= RING_FORCE_TO_NONPRIV_ADDRESS_MASK;
>   
>   		*cs++ = srm;
>   		*cs++ = reg;


Aren't you missing the scrub_whitelisted_registers() function below?


> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 855db888516c..3ba503b5e0d9 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2490,6 +2490,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define GEN8_RING_CS_GPR_UDW(base, n)	_MMIO((base) + 0x600 + (n) * 8 + 4)
>   
>   #define RING_FORCE_TO_NONPRIV(base, i) _MMIO(((base) + 0x4D0) + (i) * 4)
> +#define   RING_FORCE_TO_NONPRIV_ADDRESS_MASK	REG_GENMASK(25, 2)
>   #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)