| Message ID | 20250110002851.3584310-1-daniele.ceraolospurio@intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/guc: Debug print LRC state entries only if the context is pinned | expand |
Hi Daniele, > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -5519,12 +5519,20 @@ static inline void guc_log_context(struct drm_printer *p, > { > drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id.id); > drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca); > - drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", > - ce->ring->head, > - ce->lrc_reg_state[CTX_RING_HEAD]); > - drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", > - ce->ring->tail, > - ce->lrc_reg_state[CTX_RING_TAIL]); > + if (intel_context_pin_if_active(ce)) { > + drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", > + ce->ring->head, > + ce->lrc_reg_state[CTX_RING_HEAD]); > + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", > + ce->ring->tail, > + ce->lrc_reg_state[CTX_RING_TAIL]); > + intel_context_unpin(ce); > + } else { > + drm_printf(p, "\t\tLRC Head: Internal %u, Memory not pinned\n", > + ce->ring->head); > + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory not pinned\n", > + ce->ring->tail); > + } Please note the warnings from checkpatch.pl job. These lines should be aligned differently. Krzysztof >
On 1/9/2025 16:28, Daniele Ceraolo Spurio wrote: > After the context is unpinned the backing memory can also be unpinned, > so any accesses via the lrc_reg_state pointer can end up in unmapped > memory. To avoid that, make sure to only access that memory if the > context is pinned when printing its info. > > Fixes: 28ff6520a34d ("drm/i915/guc: Update GuC debugfs to support new GuC") > Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> > Cc: John Harrison <John.C.Harrison@Intel.com> > Cc: Matthew Brost <matthew.brost@intel.com> > --- > > I believe this should fix issue #13343, but I wasn't able to repro > the bug to confirm that it is indeed gone with this change. > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 20 +++++++++++++------ > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 12f1ba7ca9c1..22a73e2e6340 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -5519,12 +5519,20 @@ static inline void guc_log_context(struct drm_printer *p, > { > drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id.id); > drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca); > - drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", > - ce->ring->head, > - ce->lrc_reg_state[CTX_RING_HEAD]); > - drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", > - ce->ring->tail, > - ce->lrc_reg_state[CTX_RING_TAIL]); > + if (intel_context_pin_if_active(ce)) { > + drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", > + ce->ring->head, > + ce->lrc_reg_state[CTX_RING_HEAD]); > + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", > + ce->ring->tail, > + ce->lrc_reg_state[CTX_RING_TAIL]); > + intel_context_unpin(ce); > + } else { > + drm_printf(p, "\t\tLRC Head: Internal %u, Memory not pinned\n", > + ce->ring->head); > + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory not pinned\n", > + ce->ring->tail); > + } > drm_printf(p, "\t\tContext Pin Count: %u\n", > atomic_read(&ce->pin_count)); > drm_printf(p, "\t\tGuC ID Ref Count: %u\n", Looks good except for the whitespace alignment issue that checkpatch is complaining about. With that fixed: Reviewed-by: John Harrison <John.C.Harrison@Intel.com>
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 12f1ba7ca9c1..22a73e2e6340 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -5519,12 +5519,20 @@ static inline void guc_log_context(struct drm_printer *p, { drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id.id); drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca); - drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", - ce->ring->head, - ce->lrc_reg_state[CTX_RING_HEAD]); - drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", - ce->ring->tail, - ce->lrc_reg_state[CTX_RING_TAIL]); + if (intel_context_pin_if_active(ce)) { + drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n", + ce->ring->head, + ce->lrc_reg_state[CTX_RING_HEAD]); + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n", + ce->ring->tail, + ce->lrc_reg_state[CTX_RING_TAIL]); + intel_context_unpin(ce); + } else { + drm_printf(p, "\t\tLRC Head: Internal %u, Memory not pinned\n", + ce->ring->head); + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory not pinned\n", + ce->ring->tail); + } drm_printf(p, "\t\tContext Pin Count: %u\n", atomic_read(&ce->pin_count)); drm_printf(p, "\t\tGuC ID Ref Count: %u\n",
After the context is unpinned the backing memory can also be unpinned, so any accesses via the lrc_reg_state pointer can end up in unmapped memory. To avoid that, make sure to only access that memory if the context is pinned when printing its info. Fixes: 28ff6520a34d ("drm/i915/guc: Update GuC debugfs to support new GuC") Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com> Cc: John Harrison <John.C.Harrison@Intel.com> Cc: Matthew Brost <matthew.brost@intel.com> --- I believe this should fix issue #13343, but I wasn't able to repro the bug to confirm that it is indeed gone with this change. .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 20 +++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)