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@xxxxxxxxx wrote: > > > From: Sourab Gupta <sourab.gupta@xxxxxxxxx> > > > > > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
