From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Volkin, Bradley D" Subject: Re: [PATCH] drm/i915: Add OACONTROL to the command parser register whitelist. Date: Wed, 26 Mar 2014 11:26:05 -0700 Message-ID: <20140326182605.GA28133@bdvolkin-ubuntu-desktop> References: <1395813123-2027-1-git-send-email-kenneth@whitecape.org> <20140326062123.GO26878@phenom.ffwll.local> <20140326160358.GA11367@bdvolkin-ubuntu-desktop> <20140326163820.GV26878@phenom.ffwll.local> <53331068.7090007@whitecape.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id B33D26E73C for ; Wed, 26 Mar 2014 11:26:23 -0700 (PDT) Content-Disposition: inline In-Reply-To: <53331068.7090007@whitecape.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Kenneth Graunke Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org 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 > >>> > >>> 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? Thanks, Brad > > 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 >