On Tue, 22 Nov 2022 18:07:00 -0800, Umesh Nerlige Ramappa wrote: > Hi Umesh, > An earlier commit introduced a mechanism to parse the context image to > find the OA context control offset. This resulted in an NPD on haswell > when gem_context was passed into i915_perf_open_ioctl params. Haswell > does not support logical ring contexts, so ensure that the context image > is parsed only for platforms with logical ring contexts and also > validate lrc_reg_state. > > v2: Fix build failure > > Closes: https://gitlab.freedesktop.org/drm/intel/-/issues/7432 > Fixes: a5c3a3cbf029 ("drm/i915/perf: Determine gen12 oa ctx offset at runtime") > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_perf.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c > index 00e09bb18b13..dbd785974f20 100644 > --- a/drivers/gpu/drm/i915/i915_perf.c > +++ b/drivers/gpu/drm/i915/i915_perf.c > @@ -1383,6 +1383,9 @@ static u32 oa_context_image_offset(struct intel_context *ce, u32 reg) > u32 offset, len = (ce->engine->context_size - PAGE_SIZE) / 4; > u32 *state = ce->lrc_reg_state; > > + if (drm_WARN_ON(&ce->engine->i915->drm, state == NULL)) > + return U32_MAX; > + So if we didn't add the HAS_LOGICAL_RING_CONTEXTS check below state would be NULL correct? I couldn't figure out how it is NULL on HSW looking at the code (with the context image pin/unpin). > for (offset = 0; offset < len; ) { > if (IS_MI_LRI_CMD(state[offset])) { > /* > @@ -1447,7 +1450,8 @@ static int oa_get_render_ctx_id(struct i915_perf_stream *stream) > if (IS_ERR(ce)) > return PTR_ERR(ce); > > - if (engine_supports_mi_query(stream->engine)) { > + if (engine_supports_mi_query(stream->engine) && > + HAS_LOGICAL_RING_CONTEXTS(stream->perf->i915)) { This check looks fine since we seem to be looking inside ce->lrc_reg_state for oactxctrl. Overall looks fine so this is: Reviewed-by: Ashutosh Dixit <ashutosh.dixit@xxxxxxxxx> > /* > * We are enabling perf query here. If we don't find the context > * offset here, just return an error. > -- > 2.36.1 >
