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

[Kenneth Graunke <kenneth@whitecape.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2770 bytes --]

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@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.

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.

The other alternative is to have the kernel write OACONTROL to 0 after
any batch that alters it.  Then the kernel wouldn't need to care what
userspace does with it.  (I think Daniel preferred having userspace
reset it, though.)

--Ken


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx