[Intel-gfx] [RFC 8/8] drm/i915: Support for retrieving MMIO register values alongwith timestamps through perf

[Chris Wilson]

On Wed, Aug 05, 2015 at 10:18:50AM +0000, Gupta, Sourab wrote:
> On Wed, 2015-08-05 at 10:03 +0000, Chris Wilson wrote:
> > On Wed, Aug 05, 2015 at 11:25:44AM +0530, sourab.gupta at intel.com wrote:
> > > From: Sourab Gupta <sourab.gupta at intel.com>
> > > 
> > > This patch adds support for retrieving MMIO register values alongwith
> > > timestamps and forwarding them to userspace through perf.
> > > The userspace can request upto 8 MMIO register values to be dumped.
> > > The addresses of upto 8 MMIO registers can be passed through perf attr
> > > config. The registers are checked against a whitelist before passing them
> > > on. The commands to dump the values of these MMIO registers are then
> > > inserted into the ring alongwith commands to dump the timestamps.
> > 
> > The values reported to userspace are deltas across batches right? We
> > don't expose the global value to an unprivileged user? It would be nice
> > to clarify that in perf_init so that the reviewer is aware that
> > the issue of unprivileged information leak is addressed (or at least
> > reminded that the register values do not leak!).
> > -Chris
> > 
> Hi Chris,
> Two things here:
> 1) Only root is allowed to call event_init for gen pmu. This restriction
> is there in event_init. (The thought behind this restriction being that
> we are profiling data across contexts here, so a process wishing to
> listen to global activity happening in system across all contexts ought
> to have root priviliges). Is this thought process correct? Should we be
> supporting non-root users too?

That is not clear in this patch, so you need to address such concerns at
least in the changelog, and preferrably with a reminder in the
whitelist (that these register reads are safe because they are being
done from a privileged context only - we then have a red flag in case we
lower it).

What is the privilege check you are using here exactly?

For gen pmu, I want it user accessible. How long does it take to execute
my batches is a common developer query. We may even be able to make
anonymised information freely available ala top (per-process GPU usage,
memory usage, though cgroups/namespacing rules probably apply here).

> 2) Being already a root, do we need to worry about the unauthorized mmio
> access while exposing these mmio values through the interface?

Yes. See above, the information here can be anonymised and useful for
user processes exactly like TIMESTAMP.
 
> In the current patches, the full mmio register value is dumped to be
> passed on to userspace (no deltas across batches), provided the register
> is there in the whitelist. Does the question of unpriviliged information
> leak arise here(the user being root)?

Not for root.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre