Re: [RFC PATCH 3/3] drm/xe: Kick GuC while TLB invalidation acks are overdue

Thank you for your contribution! Sashiko AI review found 1 potential issue(=
s) to consider:
- [High] Unprotected MMIO access in `xe_tlb_inval_kick` can cause a crash i=
f the device suspends concurrently.
--

--- Patch [3]: [RFC PATCH 3/3] drm/xe: Kick GuC while TLB invalidation acks=
 are overdue ---
commit c7a516f0e42fcf29db699b7dce1432a3b42378cd
Author: Tales A. Mendon=C3=A7a <[email protected]>

drm/xe: Kick GuC while TLB invalidation acks are overdue

This commit adds a delayed work to periodically poke the GuC when a TLB
invalidation ack is delayed. This provides a workaround for a firmware stall
issue by re-reading the GuC status, flushing the CT fast-path, and ringing
the GuC doorbell until the ack arrives.

Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/8678

> diff --git a/drivers/gpu/drm/xe/xe_tlb_inval.c b/drivers/gpu/drm/xe/xe_tl=
b_inval.c
> index 9dd04d5bc4cd2..16c32d5669f36 100644
> --- a/drivers/gpu/drm/xe/xe_tlb_inval.c
> +++ b/drivers/gpu/drm/xe/xe_tlb_inval.c

[ ... ]

> @@ -268,6 +285,55 @@ static bool xe_tlb_inval_seqno_past(struct xe_tlb_in=
val *tlb_inval, int seqno)
>  	return seqno_recv >=3D seqno;
>  }
> =20
> +static void xe_tlb_inval_kick(struct work_struct *work)
> +{
> +	struct xe_tlb_inval *tlb_inval =3D container_of(work, struct xe_tlb_inv=
al,
> +						      kick_work.work);
> +	struct xe_gt *gt =3D tlb_inval_to_gt(tlb_inval);
> +	struct xe_tlb_inval_fence *fence;
> +	ktime_t inval_time =3D 0;
> +	int seqno =3D 0;
> +
> +	spin_lock_irq(&tlb_inval->pending_lock);
> +	fence =3D list_first_entry_or_null(&tlb_inval->pending_fences,
> +					 struct xe_tlb_inval_fence, link);
> +	if (fence) {
> +		seqno =3D fence->seqno;
> +		inval_time =3D fence->inval_time;
> +	}
> +	spin_unlock_irq(&tlb_inval->pending_lock);
> +
> +	if (!seqno)
> +		return;
> +
> +	/*
> +	 * Poke the GuC: read its status register, flush the CT fast-path and
> +	 * ring the doorbell. On ARL with GuC 70.53.0 the ack for a pending
> +	 * invalidation sometimes only arrives seconds after the request even
> +	 * though the H2G was consumed immediately; a doorbell ring while the
> +	 * ack is overdue usually unsticks it within ~250ms (see Link in the
> +	 * commit message). Keep kicking every interval until the ack shows
> +	 * up; the TDR bounds how long this can go on.
> +	 */
> +	if (gt->gtidle.idle_residency)
> +		xe_guc_pc_c_status(&gt->uc.guc.pc);
> +	tlb_inval->ops->flush(tlb_inval);
> +	xe_guc_notify(&gt->uc.guc);

[Severity: High]
Could this lead to an unprotected MMIO access if the device suspends
concurrently?

When pending_lock is dropped above, the device is kept awake by the
runtime PM reference tied to the pending fence. If the GuC ack arrives
concurrently on another CPU, xe_tlb_inval_done_handler() will acquire the
lock, signal the fence, and call xe_pm_runtime_put() via
xe_tlb_inval_fence_fini().

Since cancel_delayed_work() does not block if the worker is already
running, is it possible for the last PM reference to be dropped before
we reach these MMIO accesses in xe_guc_pc_c_status() and xe_guc_notify()?=20

If the device suspends, accessing these registers could trigger a PCIe
timeout or master abort and crash the system.

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260804021441.3054=
[email protected]?part=3D3