[Intel-gfx] [PATCH 4/5] drm/i915: add a new perf configuration execbuf parameter

[Chris Wilson]

Quoting Lionel Landwerlin (2019-05-22 10:19:46)
> On 21/05/2019 18:48, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2019-05-21 18:19:50)
> >> On 21/05/2019 18:07, Chris Wilson wrote:
> >>> Quoting Lionel Landwerlin (2019-05-21 15:08:54)
> >>>> +       if (eb->oa_config &&
> >>>> +           eb->oa_config != eb->i915->perf.oa.exclusive_stream->oa_config) {
> >>> But the oa_config is not ordered with respect to requests, and the
> >>> registers changed here are not context saved and so may be changed by a
> >>> third party before execution. The first party must presumably dropped
> >>> the perf_fd before then, so maybe you don't care? Hmm, doesn't even take
> >>> a third party as the perf_fd owner may change their mind for different
> >>> contexts and be surprised when the wrong set is used.
> >>
> >> The OA config batch should be ordered with regard to the MI_BBS we're
> >> doing just below right?
> > But you only emit if the oa_config has changed. Ergo, it may have
> > changed between submission and execution.
> >
> >> It's written before in the ring buffer.
> >>
> >>
> >> That essentially all we need so that as the perf queries run in the
> >> batch supplied by the application runs with the configuration needed.
> >>
> >> If the application shares the perf FD and someone else runs another
> >> configuration, it's the application fault.
> >>
> >> It needs to hold the perf FD for as long as it's doing perf queries and
> >> not allow anybody else to interact with the OA configuration.
> > If one app is trying to use different configs on different contexts
> > (which seems reasonable if it is trying to sample different stats?) then
> > it can be caught out. The order of execution is not the same as the
> > order of submission (unless we enforce it by e.g. defining the
> > perf.oa_config as a barrier).
> 
> 
> Thinking about this a bit more, the use case here was always that the 
> app is the single user of the OA unit.
> 
> In this scenario, the app is doing queries and should not share the 
> configuration of the OA HW with anybody else.

What about with itself? And does that excuse the kernel carrying a
TOCTOU bug?
-Chris