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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
