[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?

I've thought perf is priviledged ... And I've looked at the trigger based
stuff again in Bspec and it looks like by default that's all disabled, so
even when batches enable the timer nothing should happen.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch