[PATCH] drm/i915/guc: Debug print LRC state entries only if the context is pinned

[John Harrison]

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 at intel.com>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Cc: Matthew Brost <matthew.brost at 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 at Intel.com>