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

[Lionel Landwerlin]

On 22/05/2019 10:25, Chris Wilson wrote:
> 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
>
You mean with something like Iris that uses 2 contexts?

I'm assuming things are properly synchronized.


There is also another problem with the 2 contexts which is that we only 
allow filtering a single ID at the moment...


Sorry, I'm not familiar with the TOCTOU bug :(


-Lionel