[Intel-gfx] [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist.

[Daniel Vetter]

On Wed, Mar 26, 2014 at 11:26:05AM -0700, Volkin, Bradley D wrote:
> On Wed, Mar 26, 2014 at 10:37:44AM -0700, Kenneth Graunke wrote:
> > On 03/26/2014 09:38 AM, Daniel Vetter wrote:
> > > On Wed, Mar 26, 2014 at 09:03:58AM -0700, Volkin, Bradley D wrote:
> > >> On Tue, Mar 25, 2014 at 11:21:23PM -0700, Daniel Vetter wrote:
> > >>> On Tue, Mar 25, 2014 at 10:52:03PM -0700, Kenneth Graunke wrote:
> > >>>> Mesa needs to be able to write OACONTROL in order to expose the
> > >>>> Observability Architecture's performance counters via OpenGL.
> > >>>>
> > >>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> > >>>
> > >>> Thanks a lot for quickly tracking this down. Now when we've talked about
> > >>> OA a little while ago we concluded that mesa should clear OACONTROL again
> > >>> before the batch ends to make sure that userspace can't unduly observe
> > >>> other processes. So I think it'd be worth to keep track of this with a
> > >>> flag (set when OACONTROL is != 0 and reset when the batch loads 0). Also
> > >>> we need to make sure that userspace sets the right OACONTROL modes (not
> > >>> the one which streams into a global gtt buffer essentially). So some
> > >>> additional work required.
> > >>
> > >> Ok, I'll look into this. And apologies for not catching it myself.
> > >>
> > >> If we have to do additional checks on fields within the registers then I
> > >> suppose we'll need to limit those registers to MI_LOAD_REGISTER_IMM. That
> > >> might require separate whitelists for MI_LOAD_REGISTER_IMM/MEM. Not the end
> > >> of the world, but certainly some additional complexity.
> > >>
> > >> For the resetting check, are there other registers in the current list that
> > >> should have this tracking? If so, is 0 the reset value in all cases?
> > >>
> > >> Let me know if there is anything in the works that would require additional
> > >> registers or different uses of any registers.
> > > 
> > > Afaik there's no other register we want to reset again. I think all other
> > > register we might want to clear are already part of hw contexts, so no
> > > chance to leak stuff (e.g. the streamout registers and a end-of-pipe
> > > counters). Ken might know of something I've missed.
> > > -Daniel
> > 
> > Right, I think it's just OACONTROL.  I don't think there's a need to
> > filter particular values.
> 
> Sorry, I don't follow you here. Can you clarify what you mean r.e. filtering?
> 
> > 
> > I don't really buy the snooping problem, though...just because I leave
> > OACONTROL set doesn't mean I'll get useful data.  Another context might
> > clobber it, and empirically the numbers seem to reset across RC6 anyway.
> >  So in actuality, they're likely to get bogus data.
> > 
> > Even if they did somehow miraculously get decent values, it basically
> > gives information akin to 'top', which is unprivileged on every system
> > I've ever used.
> 
> I tend to agree with this assesment. The data doesn't seem particularly
> sensitive. Also, I assume this data is similar in nature to what you can
> already get from graphics perf tools, right?

Another one that blows is igt/gen7_forcewake_mt. Not sure yet whether it's
an issue with the test or the checker:

https://bugs.freedesktop.org/show_bug.cgi?id=76670

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch