| Message ID | 20191111095308.2550-1-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/perf: always consider holding preemption a privileged op | expand |
Quoting Lionel Landwerlin (2019-11-11 09:53:08) > The ordering of the checks in the existing code can lead to holding > preemption not being considered as privileged op. > > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Fixes: 9cd20ef7803c ("drm/i915/perf: allow holding preemption on filtered ctx") Oops, that'll teach me for not looking at the result more carefully. Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> -Chris
On 11/11/2019 12:05, Chris Wilson wrote: > Quoting Lionel Landwerlin (2019-11-11 09:53:08) >> The ordering of the checks in the existing code can lead to holding >> preemption not being considered as privileged op. >> >> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> >> Fixes: 9cd20ef7803c ("drm/i915/perf: allow holding preemption on filtered ctx") > Oops, that'll teach me for not looking at the result more carefully. > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> > -Chris My bad... and thanks! Pushed. -Lionel
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c index a8c2318d3d5e..00317ea19a4a 100644 --- a/drivers/gpu/drm/i915/i915_perf.c +++ b/drivers/gpu/drm/i915/i915_perf.c @@ -3307,15 +3307,6 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, } } - if (props->hold_preemption) { - if (!props->single_context) { - DRM_DEBUG("preemption disable with no context\n"); - ret = -EINVAL; - goto err; - } - privileged_op = true; - } - /* * On Haswell the OA unit supports clock gating off for a specific * context and in this mode there's no visibility of metrics for the @@ -3335,12 +3326,21 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf, * doesn't request global stream access (i.e. query based sampling * using MI_RECORD_PERF_COUNT. */ - if (IS_HASWELL(perf->i915) && specific_ctx && !props->hold_preemption) + if (IS_HASWELL(perf->i915) && specific_ctx) privileged_op = false; else if (IS_GEN(perf->i915, 12) && specific_ctx && (props->sample_flags & SAMPLE_OA_REPORT) == 0) privileged_op = false; + if (props->hold_preemption) { + if (!props->single_context) { + DRM_DEBUG("preemption disable with no context\n"); + ret = -EINVAL; + goto err; + } + privileged_op = true; + } + /* Similar to perf's kernel.perf_paranoid_cpu sysctl option * we check a dev.i915.perf_stream_paranoid sysctl option * to determine if it's ok to access system wide OA counters
The ordering of the checks in the existing code can lead to holding preemption not being considered as privileged op. Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Fixes: 9cd20ef7803c ("drm/i915/perf: allow holding preemption on filtered ctx") --- drivers/gpu/drm/i915/i915_perf.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)