Message ID | 20240829220106.80449-2-gustavo.sousa@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | Miscelaneous fixes for display tracepoints | expand |
On Thu, Aug 29, 2024 at 07:00:44PM -0300, Gustavo Sousa wrote: > Some display trace events use array members to store frame and scanline > counts for each pipe. However, those arrays are declared with 3 as the > hardcoded size, which cause out-of-bounds access when the trace event is > enabled on a platform that contains pipe D. > > For example, when looking at the last 10 intel_pipe_enable events after > running IGT's testdisplay, we see the following on a MTL machine that > has pipe D available: > > $ trace-cmd report -R -F intel_pipe_enable \ > > | tail \ > > | sed 's,\(frame=.*\) \(scanline=.*\),\n\t \1\n\t\2,' > testdisplay-6715 [002] 17591.063491: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[83, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [003] 17591.264742: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[89, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [003] 17591.464541: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[8f, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [001] 17591.695827: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[95, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [000] 17591.915841: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[9a, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [000] 17592.127114: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[a0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [002] 17592.358351: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[a8, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [002] 17592.580467: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[ae, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [000] 17592.950946: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[b8, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-6715 [004] 17593.079597: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[bf, 01, 00, 00, 01, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[00, 00, 00, 00, 3a, 04, 00, 00, 00, 00, 00, 00] pipe=1 > > Which shows zeros for pipe A's scanline counts. That happens because > pipe D's frame counts are overwriting them. > > Let's fix that by making the arrays bring able to store info for all > possible pipes. > > With the fix, we get the following: > > $ trace-cmd report -R -F intel_pipe_enable \ > > | tail \ > > | sed 's,\(frame=.*\) \(scanline=.*\),\n\t \1\n\t\2,' > testdisplay-7040 [003] 18067.489565: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[8c, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[8e, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [002] 18067.699312: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[92, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[58, 02, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [002] 18067.908868: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[98, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[58, 02, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [002] 18068.122802: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[9d, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[58, 02, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [003] 18068.331019: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[a2, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[e0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [002] 18068.529067: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[a8, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[e0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [003] 18068.742033: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[ae, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[e0, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [002] 18068.956229: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[b3, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[1f, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [002] 18069.295322: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[bb, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[7b, 08, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=0 > testdisplay-7040 [010] 18069.423527: intel_pipe_enable: dev=0000:00:02.0 > frame=ARRAY[c2, 01, 00, 00, 01, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] > scanline=ARRAY[d0, 05, 00, 00, 3a, 04, 00, 00, 00, 00, 00, 00, 00, 00, 00, 00] pipe=1 > > Which makes more sense now. > > Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com> Reviewed-by: Matt Roper <matthew.d.roper@intel.com> I guess nobody has really needed to use these tracepoints much for debugging since TGL added the 4th pipe. Matt > --- > drivers/gpu/drm/i915/display/intel_display_trace.h | 13 +++++++------ > 1 file changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h > index c734ef1fba3c..8a3185862089 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_trace.h > +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h > @@ -15,6 +15,7 @@ > > #include "i915_drv.h" > #include "intel_crtc.h" > +#include "intel_display_limits.h" > #include "intel_display_types.h" > #include "intel_vblank.h" > > @@ -27,8 +28,8 @@ TRACE_EVENT(intel_pipe_enable, > > TP_STRUCT__entry( > __string(dev, __dev_name_kms(crtc)) > - __array(u32, frame, 3) > - __array(u32, scanline, 3) > + __array(u32, frame, I915_MAX_PIPES) > + __array(u32, scanline, I915_MAX_PIPES) > __field(enum pipe, pipe) > ), > TP_fast_assign( > @@ -55,8 +56,8 @@ TRACE_EVENT(intel_pipe_disable, > > TP_STRUCT__entry( > __string(dev, __dev_name_kms(crtc)) > - __array(u32, frame, 3) > - __array(u32, scanline, 3) > + __array(u32, frame, I915_MAX_PIPES) > + __array(u32, scanline, I915_MAX_PIPES) > __field(enum pipe, pipe) > ), > > @@ -184,8 +185,8 @@ TRACE_EVENT(intel_memory_cxsr, > > TP_STRUCT__entry( > __string(dev, __dev_name_i915(dev_priv)) > - __array(u32, frame, 3) > - __array(u32, scanline, 3) > + __array(u32, frame, I915_MAX_PIPES) > + __array(u32, scanline, I915_MAX_PIPES) > __field(bool, old) > __field(bool, new) > ), > -- > 2.46.0 >
diff --git a/drivers/gpu/drm/i915/display/intel_display_trace.h b/drivers/gpu/drm/i915/display/intel_display_trace.h index c734ef1fba3c..8a3185862089 100644 --- a/drivers/gpu/drm/i915/display/intel_display_trace.h +++ b/drivers/gpu/drm/i915/display/intel_display_trace.h @@ -15,6 +15,7 @@ #include "i915_drv.h" #include "intel_crtc.h" +#include "intel_display_limits.h" #include "intel_display_types.h" #include "intel_vblank.h" @@ -27,8 +28,8 @@ TRACE_EVENT(intel_pipe_enable, TP_STRUCT__entry( __string(dev, __dev_name_kms(crtc)) - __array(u32, frame, 3) - __array(u32, scanline, 3) + __array(u32, frame, I915_MAX_PIPES) + __array(u32, scanline, I915_MAX_PIPES) __field(enum pipe, pipe) ), TP_fast_assign( @@ -55,8 +56,8 @@ TRACE_EVENT(intel_pipe_disable, TP_STRUCT__entry( __string(dev, __dev_name_kms(crtc)) - __array(u32, frame, 3) - __array(u32, scanline, 3) + __array(u32, frame, I915_MAX_PIPES) + __array(u32, scanline, I915_MAX_PIPES) __field(enum pipe, pipe) ), @@ -184,8 +185,8 @@ TRACE_EVENT(intel_memory_cxsr, TP_STRUCT__entry( __string(dev, __dev_name_i915(dev_priv)) - __array(u32, frame, 3) - __array(u32, scanline, 3) + __array(u32, frame, I915_MAX_PIPES) + __array(u32, scanline, I915_MAX_PIPES) __field(bool, old) __field(bool, new) ),