[Intel-gfx] [PATCH v2 1/8] drm/i915/gen9: Add framework to whitelist specific GPU registers

[Chris Wilson]

On Wed, Jan 13, 2016 at 12:28:17PM +0000, Arun Siluvery wrote:
> Some of the HW registers are privileged and cannot be written to from a
> non-privileged batch buffers coming from userspace unless they are on whitelist.
> Userspace need write access to them to implement preemption related WA.

You need to be clear that this is the hw whitelist and not our sw
whitelist.
 
> The reason for using this approach is, the register bits that control preemption
> granularity at the HW level are not context save/restored; so even if we set these
> bits always in kernel they are going to change once the context is switched out.
> We can consider making them non-privileged by default but these registers also
> contain other chicken bits which should not be allowed to be modified.
> 
> In the later revisions controlling bits are save/restored at context level but
> in the existing revisions these are exported via other debug registers and should
> be on the whitelist. This patch adds changes to provide HW with a list of registers
> to be whitelisted. HW checks this list during execution and provides access accordingly.
> 
> HW imposes a limit on the number of registers on whitelist and it is per-engine.
> At this point we are only enabling whitelist for RCS and we don't foresee any
> requirement for other engines.
> 
> The registers to be whitelisted are added using generic workaround list mechanism,
> even these are only enablers for userspace workarounds. But by sharing this
> mechanism we get some test assets without additional cost (Mika).
> 
> v2: rebase
> 
> v3: parameterize RING_FORCE_TO_NONPRIV() as _MMIO() should be limited to
> i915_reg.h (Ville), drop inline for wa_ring_whitelist_reg (Mika).
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  9 ++++++++-
>  drivers/gpu/drm/i915/i915_reg.h         |  3 +++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 17 +++++++++++++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 104bd18..660afe1 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1653,11 +1653,18 @@ struct i915_wa_reg {
>  	u32 mask;
>  };
>  
> -#define I915_MAX_WA_REGS 16
> +/*
> + * RING_MAX_NONPRIV_SLOTS is per-engine but at this point we are only
> + * allowing it for RCS as we don't foresee any requirement of having
> + * a whitelist for other engines. When it is really required for
> + * other engines then the limit need to be increased.
> + */
> +#define I915_MAX_WA_REGS (16 + RING_MAX_NONPRIV_SLOTS)
>  
>  struct i915_workarounds {
>  	struct i915_wa_reg reg[I915_MAX_WA_REGS];
>  	u32 count;
> +	u32 hw_whitelist_index[I915_NUM_RINGS];

This is a counter only to be used whilst constructing the reg list. It
also implies that this list of wa_reg is now engine specific. Hmm, looks
like it always has been, just been relying on RCS running first.

I would fix that up first.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre