[RFC PATCH 0/1] drm/amdgpu: MMIO TLB invalidation fallback when KIQ is wedged after S4 resume

[RFC PATCH 0/1] drm/amdgpu: MMIO TLB invalidation fallback when KIQ is wedged after S4 resume

[Denis Pisarev]

Hi,

This RFC follows up on bugzilla 219492 ("failed to write reg 28b4 wait
reg 28c6" after hibernation) and on the resume_gpu_stable proposal
rejected in January:

  https://lore.kernel.org/amd-gfx/20260106125929.25214-6-sunlightlinux@gmail.com/

The rejection argued that (a) kiq sched.ready is managed correctly -
set false in suspend, set true again only after ring tests pass on
resume - and (b) the proposed flag would have disabled the KIQ flush
path permanently. Both points are correct, and this RFC takes a
different route: nothing is disabled proactively; the driver reacts to
observed failures.

MEASURED FAILURE MODE (Cezanne iGPU, Ryzen 7 PRO 5850U, 7.1.8, KDE
Wayland, no ROCm):

- S4 resume -> "failed to write reg 28b4/28c6" + "1a6f4/1a706" pairs
  begin in the same second as the thaw and continue at 80-140/hour
  for 9+ hours until reboot. kiq sched.ready is true throughout (the
  ring test at resume passed).
- Two user-visible consequences: each failed flush burns the ~5 s KIQ
  retry window (desktop-wide sluggishness), and the invalidation is
  then silently dropped (stale TLBs - correctness).

TRIGGER ISOLATION (single-variable tests, one hibernate cycle each):

- stock (GFXOFF allowed across S4):    ~70-140 errors / 30 min of use
- GFXOFF held off across S4:                    0 errors (30 min)
- GFXOFF re-enabled 40 min after the clean
  resume:                                      0 errors (15 min;
  gating resumption not independently verified)

So the wedge forms in the S4 resume window while GFXOFF is allowed,
and is consistent with the existing semaphore workaround comment in
gmc_v9_0.c about losing invalidate-acknowledge state across
power-gating cycles. A boot-time amdgpu.pg_mask=0 does not even
initialize on this part (probe -ETIMEDOUT), so runtime debugfs
control was used.

THE PATCH (1/1):

- amdgpu_gmc_fw_reg_write_reg_wait() returns -ETIME on failure and
  counts consecutive failures
- gmc_v9_0_flush_gpu_tlb() uses its existing MMIO path as fallback
  when the KIQ submit fails (invalidations no longer dropped), and
  after 3 consecutive failures skips the KIQ path entirely until the
  counter resets - so wedged systems stop paying 5 s per flush
- counter resets on every success and in gmc_v9_0_hw_fini(); each
  suspend/resume cycle re-arms KIQ. No proactive disabling anywhere.
- gmc_v10/v11/v12 call sites unchanged and compile-safe (int return
  used as statement); extending the fallback to them is trivial once
  the approach is agreed.

This mirrors what the sibling PASID path already does (KIQ submit,
-ETIME on timeout, MMIO variant as fallback), applied to the per-VMID
path.

OPEN QUESTIONS:

1. Root cause in the S4 window: RLC/ME not processing KIQ invalidates
   while GFXOFF is allowed? Happy to run tracing (amdgpu.vm_debug,
   VM_INVALIDATE ack registers) on the affected hardware on request.
2. Should the latch be per-xcc-inst?
3. Should amdgpu_gmc_flush_gpu_tlb_pasid()'s KIQ branch share the
   latch?

Patch 1/1 follows. Comments very welcome - especially on whether the
failure-driven fallback is the right shape vs. fixing the S4 resume
ordering directly.

Denis Pisarev

Denis Pisarev (1):
  drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is
    unresponsive

 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 15 +++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  4 +++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 18 ++++++++++++++----
 4 files changed, 30 insertions(+), 9 deletions(-)

-- 
2.55.0

[RFC PATCH 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

[Denis Pisarev]

After resume from S4 (hibernation) on gmc_v9 parts with GFXOFF
(observed on Cezanne / Ryzen 7 PRO 5850U, kernel 7.1.8), KIQ-based TLB
flushes start failing at the moment of the thaw and keep failing for
hours of normal desktop use:

  amdgpu 0000:07:00.0: failed to write reg 28b4 wait reg 28c6
  amdgpu 0000:07:00.0: failed to write reg 1a6f4 wait reg 1a706

(80-140 errors/hour measured over 9+ hours; bugzilla 219492)

Two problems follow from the current code:

1. Every failed flush burns the full retry window
   (MAX_KIQ_REG_TRY * MAX_KIQ_REG_BAILOUT_INTERVAL = ~5 s) before
   erroring out, which makes the whole desktop sluggish.

2. The invalidation is then silently dropped - stale TLB entries are
   left in place - because gmc_v9_0_flush_gpu_tlb() returns as soon as
   amdgpu_gmc_fw_reg_write_reg_wait() finishes, whether it succeeded
   or not.

The KIQ ring is marked ready during resume after its ring test passes,
but on affected systems the ring subsequently stops completing
invalidation commands. sched.ready therefore does not reflect the
state of the hardware in this failure mode, and there is no path back
to the direct MMIO invalidation that already exists in
gmc_v9_0_flush_gpu_tlb() for the pre-KIQ stage.

Make the failure observable and self-healing:

- amdgpu_gmc_fw_reg_write_reg_wait() returns 0/-ETIME and counts
  consecutive failures in adev->gmc.kiq_flush_failures
  (dev_err_ratelimited instead of dev_err, since affected systems
  print this 80-140x/hour for hours)

- gmc_v9_0_flush_gpu_tlb() falls back to its existing MMIO path when
  the KIQ submit fails, so the invalidation is no longer dropped

- after AMDGPU_KIQ_FLUSH_MAX_FAIL (3) consecutive failures the KIQ
  path is skipped entirely until the counter is reset, so wedged
  systems stop paying the 5 s retry window per flush

- the counter is reset on every success and in gmc_v9_0_hw_fini(),
  i.e. every suspend/resume cycle re-arms the KIQ path; nothing is
  disabled proactively

gmc_v10/v11/v12 call sites are unchanged (statement calls compile
fine against the new int return; behavior identical). They can get
the same fallback once this approach is agreed for gmc_v9.

The sibling PASID path (amdgpu_gmc_flush_gpu_tlb_pasid) already has
an -ETIME/MMIO split; this brings the per-VMID path in line with it.

RFC questions for maintainers:

- Is a per-xcd-inst latch preferred over the global gmc one? (single
  inst on the affected hardware here)
- Should the latch also gate the KIQ branch of
  amdgpu_gmc_flush_gpu_tlb_pasid()? (no failures observed on that
  path on the affected system)
- Root cause: with GFXOFF disabled across the S4 cycle (debugfs
  amdgpu_gfxoff), zero errors occur across resume and 30 min of use
  vs ~70-140 in the control arm. The wedge forms in the S4 resume
  window while GFXOFF is allowed, consistent with the existing
  semaphore workaround comment about losing invalidate acknowledge
  state across power-gating cycles in this file.

Tested on Cezanne (Ryzen 7 PRO 5850U, Manjaro 7.1.8): S4 resume with
GFXOFF enabled reproduces the failure storm on stock; hibernate loop
testing of this patch pending maintainer feedback on the approach.

Signed-off-by: Denis Pisarev <pisarevden@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 15 +++++++++++----
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |  4 +++-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 18 ++++++++++++++----
 4 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b09410d6..cd5d9e56e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -360,6 +360,8 @@ enum amdgpu_kiq_irq {
 #define MAX_KIQ_REG_WAIT       5000 /* in usecs, 5ms */
 #define MAX_KIQ_REG_BAILOUT_INTERVAL   5 /* in msecs, 5ms */
 #define MAX_KIQ_REG_TRY 1000
+/* consecutive KIQ TLB flush failures before falling back to MMIO */
+#define AMDGPU_KIQ_FLUSH_MAX_FAIL 3
 
 /*
  * BIOS.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 5d6149ba7..000a1d107 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -874,7 +874,7 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 	return r;
 }
 
-void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
+int amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
 				      uint32_t xcc_inst)
@@ -888,7 +888,7 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 	if (adev->mes.ring[MES_PIPE_INST(xcc_inst, 0)].sched.ready) {
 		amdgpu_mes_reg_write_reg_wait(adev, reg0, reg1,
 					      ref, mask, xcc_inst);
-		return;
+		return 0;
 	}
 
 	spin_lock_irqsave(&kiq->ring_lock, flags);
@@ -919,13 +919,20 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 	if (cnt > MAX_KIQ_REG_TRY)
 		goto failed_kiq;
 
-	return;
+	atomic_set(&adev->gmc.kiq_flush_failures, 0);
+	return 0;
 
 failed_undo:
 	amdgpu_ring_undo(ring);
 	spin_unlock_irqrestore(&kiq->ring_lock, flags);
 failed_kiq:
-	dev_err(adev->dev, "failed to write reg %x wait reg %x\n", reg0, reg1);
+	if (atomic_inc_return(&adev->gmc.kiq_flush_failures) ==
+			AMDGPU_KIQ_FLUSH_MAX_FAIL)
+		dev_warn(adev->dev,
+			 "KIQ reg access keeps failing, falling back to MMIO\n");
+	dev_err_ratelimited(adev->dev,
+			    "failed to write reg %x wait reg %x\n", reg0, reg1);
+	return -ETIME;
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index ddb0d500e..3e5c152ad 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -366,6 +366,8 @@ struct amdgpu_gmc {
 	bool flush_tlb_needs_extra_type_0;
 	bool flush_tlb_needs_extra_type_2;
 	bool flush_pasid_uses_kiq;
+	/* consecutive KIQ TLB flush failures; MMIO fallback when latched */
+	atomic_t kiq_flush_failures;
 
 	bool override_pte;
 };
@@ -447,7 +449,7 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 				   uint32_t flush_type, bool all_hub,
 				   uint32_t inst);
-void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
+int amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
 				      uint32_t xcc_inst);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 8a5c44810..a262df837 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -798,13 +798,18 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	 * properly under bare metal
 	 */
 	if (adev->gfx.kiq[inst].ring.sched.ready &&
-	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
+	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev)) &&
+	    atomic_read(&adev->gmc.kiq_flush_failures) <
+			AMDGPU_KIQ_FLUSH_MAX_FAIL) {
 		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
 		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
 
-		amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack, inv_req,
-						 1 << vmid, inst);
-		return;
+		if (!amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack, inv_req,
+						      1 << vmid, inst))
+			return;
+		/* KIQ submit failed - fall through to the MMIO path below
+		 * so the invalidation is not silently dropped
+		 */
 	}
 
 	/* This path is needed before KIQ/MES/GFXOFF are set up */
@@ -2238,6 +2243,11 @@ static int gmc_v9_0_hw_fini(struct amdgpu_ip_block *ip_block)
 {
 	struct amdgpu_device *adev = ip_block->adev;
 
+	/* KIQ is re-initialized on the next resume; give it a clean
+	 * start for the MMIO fallback latch
+	 */
+	atomic_set(&adev->gmc.kiq_flush_failures, 0);
+
 	gmc_v9_0_gart_disable(adev);
 
 	if (amdgpu_sriov_vf(adev)) {
-- 
2.55.0

Re: [RFC PATCH 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:

New issues:
- [Critical] Falling back to direct MMIO for TLB invalidations during runtime circumvents KIQ's GFXOFF protection, risking fatal system hangs. The MMIO path directly accesses Graphics Compute (GC) registers via `WREG32_SOC15_IP_NO_KIQ(GC, ...)`. As noted in the existing comment ("This path is needed before KIQ/MES/GFXOFF are set up"), this is only safe before GFXOFF is active. If KIQ fails during normal runtime and the system falls back to this path while GFXOFF has power-gated the GC block, the CPU will attempt to access powered-off MMIO registers without first waking them via `amdgpu_gfx_off_ctrl(adev, false)`.
- [High] The patch implements a self-healing fallback mechanism that relies on a global counter (`adev->gmc.kiq_flush_failures`), which fails to correctly track errors on multi-XCC devices. KIQ instances operate per-XCC. If one XCC's KIQ becomes wedged, it increments the global counter. However, any concurrent successful TLB flush on a healthy XCC will call `atomic_set(&adev->gmc.kiq_flush_failures, 0)`, wiping the failure history. This cross-talk prevents the wedged XCC from ever reaching the `AMDGPU_KIQ_FLUSH_MAX_FAIL` threshold.
- [High] The MMIO fallback allows concurrent, unsynchronized access to the TLB invalidation engine from both the CPU and the CP (KIQ) microengine. When a KIQ command times out, the unexecuted TLB invalidation commands remain in the KIQ ring buffer. The CPU then falls through to the MMIO path, directly writing to the same invalidation request and acknowledgment registers. The `adev->gmc.invalidate_lock` spinlock only serializes CPU threads. If the KIQ subsequently recovers or processes its delayed ring, it will concurrently write to the same registers.
- [High] The patch allows an SR-IOV Virtual Function (VF) to fall back to direct MMIO for TLB invalidations if the KIQ path times out. However, VFs lack the necessary privileges to directly write to global GMC invalidation registers (which is why KIQ proxying is strictly required). If a VF executes the MMIO fallback, the hardware will block the register writes or trigger a fatal security violation trap in the Physical Function (PF).
- [Medium] The MMIO fallback path acquires `adev->gmc.invalidate_lock` with a plain `spin_lock()`. Originally, this path was only executed before initialization completed, avoiding contention. With the patch, this path is reachable during normal runtime when KIQ fails. The `amdgpu_gmc_fw_reg_write_reg_wait()` function explicitly anticipates being called from interrupt context (via its `in_interrupt()` check). If `gmc_v9_0_flush_gpu_tlb()` is invoked from an interrupt context, an interrupt preempting a process-context thread that currently holds the plain spinlock will attempt to acquire the same lock, resulting in an AB-BA deadlock.
- [Medium] The patch introduces a `dev_warn` message in `amdgpu_gmc_fw_reg_write_reg_wait()` claiming "falling back to MMIO" when the KIQ failure threshold is reached. However, the callers in v10, v11, and v12 GMC implementations were not updated to check this return value or the failure counter. Therefore, they do not bypass the KIQ path or fall back to MMIO. This results in a highly misleading log message.

Pre-existing issues:
- [High] The patch modifies `amdgpu_gmc_fw_reg_write_reg_wait()` to return an `int` so callers can fall back to MMIO on failure. However, for the MES firmware path, the patch explicitly hardcodes `return 0;` instead of propagating the error code returned by `amdgpu_mes_reg_write_reg_wait()`. Consequently, if the MES path fails, the caller is falsely informed of success, bypasses the newly introduced MMIO fallback, and silently drops the TLB invalidation.
--

--- Patch [1]: [RFC PATCH 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive ---
commit 551bd0b3bbdf086c6bb58a378bdcd03f9d151f96
Author: Denis Pisarev <pisarevden@gmail.com>
Subject: drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

This patch implements a fallback mechanism for TLB invalidations on gmc_v9
parts when the KIQ ring becomes unresponsive. It counts consecutive failures
and falls back to direct MMIO, bypassing the KIQ path entirely after a
threshold is reached.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index 2f6d20c00ce29..566f42140e13d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -886,7 +886,7 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
>  	if (adev->mes.ring[MES_PIPE_INST(xcc_inst, 0)].sched.ready) {
>  		amdgpu_mes_reg_write_reg_wait(adev, reg0, reg1,
>  					      ref, mask, xcc_inst);
> -		return;
> +		return 0;
>  	}

[Severity: High]
This is a pre-existing issue, but amdgpu_mes_reg_write_reg_wait() returns an
error code on failure. By hardcoding a return of 0 here, does this mask MES
failures and prevent the new MMIO fallback from executing when the MES ring
fails? If the error isn't propagated, the caller will assume success and
silently drop the TLB invalidation.

[ ... ]

> @@ -917,13 +917,20 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
>  	if (cnt > MAX_KIQ_REG_TRY)
>  		goto failed_kiq;
>  
> -	return;
> +	atomic_set(&adev->gmc.kiq_flush_failures, 0);
> +	return 0;

[Severity: High]
On multi-XCC devices, KIQ instances operate per-XCC. Since this is a global
counter, will a successful TLB flush on a healthy XCC unconditionally clear
the failure history for a wedged XCC? This cross-talk might prevent the
wedged instance from ever reaching the AMDGPU_KIQ_FLUSH_MAX_FAIL threshold.

>  failed_undo:
>  	amdgpu_ring_undo(ring);
>  	spin_unlock_irqrestore(&kiq->ring_lock, flags);
>  failed_kiq:
> -	dev_err(adev->dev, "failed to write reg %x wait reg %x\n", reg0, reg1);
> +	if (atomic_inc_return(&adev->gmc.kiq_flush_failures) ==
> +			AMDGPU_KIQ_FLUSH_MAX_FAIL)
> +		dev_warn(adev->dev,
> +			 "KIQ reg access keeps failing, falling back to MMIO\n");

[Severity: Medium]
Is this log message accurate for v10, v11, and v12 GMC implementations? Since
the call sites for newer GPU architectures were not updated to check the
return value or failure counter, they won't actually fall back to MMIO,
making this warning potentially misleading.

> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index b46b87291c512..271c179ed31ff 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -799,13 +799,18 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
>  	 * properly under bare metal
>  	 */
>  	if (adev->gfx.kiq[inst].ring.sched.ready &&
> -	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
> +	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev)) &&
> +	    atomic_read(&adev->gmc.kiq_flush_failures) <
> +			AMDGPU_KIQ_FLUSH_MAX_FAIL) {

[Severity: High]
If KIQ fails on an SR-IOV Virtual Function (VF) during runtime, the threshold
check will fail and execution will fall through to the direct MMIO path.
Since VFs lack privileges to write to global GMC invalidation registers
directly, will this trigger a fatal security violation trap in the Physical
Function (PF)?

>  		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
>  		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
>  
> -		amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack, inv_req,
> -						 1 << vmid, inst);
> -		return;
> +		if (!amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack, inv_req,
> +						      1 << vmid, inst))
> +			return;
> +		/* KIQ submit failed - fall through to the MMIO path below
> +		 * so the invalidation is not silently dropped
> +		 */
>  	}

[Severity: High]
When falling through to MMIO here, the original timed-out command remains
pending in the autonomous KIQ ring buffer. If the KIQ experiences a transient
delay and then resumes processing its queued commands, could it concurrently
write to the same invalidation request and acknowledgment registers while the
CPU is accessing them? This might corrupt the hardware state machine.

>  
>  	/* This path is needed before KIQ/MES/GFXOFF are set up */

[Severity: Medium]
With this patch, the direct MMIO path is now reachable during normal runtime
when KIQ fails, where it will acquire adev->gmc.invalidate_lock via a plain
spin_lock(). The amdgpu_gmc_fw_reg_write_reg_wait() function anticipates
being called from interrupt context (it checks in_interrupt()). If
gmc_v9_0_flush_gpu_tlb() is invoked from an interrupt context, could it
preempt a process-context thread holding this plain spin_lock and attempt
to acquire it again, causing an AB-BA deadlock?

[Severity: Critical]
As the comment notes, this MMIO path is only safe before GFXOFF is set up.
If KIQ fails during normal runtime and we fall back to this path while GFXOFF
has power-gated the GC block, will accessing the GC registers directly cause
a machine check exception or hard system hang?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260819185349.29407-2-pisarevden@gmail.com?part=1

[RFC PATCH v2 0/1] drm/amdgpu: MMIO TLB invalidation fallback when KIQ is wedged after S4 resume

[Denis Pisarev]

Hi,

v2 of this RFC, addressing the automated sashiko-bot review of v1.
The failure data and trigger isolation are unchanged from v1
(bugzilla 219492):

- S4 resume on Cezanne (gmc_v9, GFXOFF) -> KIQ TLB flush reg failures
  at 80-140/hour for 9+ hours, sched.ready true throughout; each
  failure burns the ~5 s retry window and the invalidation is dropped
- GFXOFF held off across S4: 0 errors (vs ~70-140/30 min stock);
  re-enabled 40 min after a clean resume: 0 errors in 15 min
- so the wedge forms in the S4 resume window while GFXOFF is allowed

Changes since v1 (all from the bot review, all reasonable):

1. [Critical] the runtime MMIO fallback now holds the GC block awake
   with amdgpu_gfx_off_ctrl(adev, false) across the direct register
   access - the unguarded v1 could touch power-gated registers.
   Because that call may sleep, the fallback is restricted to process
   context.
2. [High] failure counter moved from a global in amdgpu_gmc to the
   per-instance struct amdgpu_kiq - no cross-talk between XCCs.
3. [High] SR-IOV VFs never take the runtime MMIO fallback (they lack
   privileges for direct GMC invalidation register writes).
4. [Medium] gmc_v9_0_flush_gpu_tlb_mmio() (extracted helper) takes
   invalidate_lock with irqsave since the path is now
   runtime-reachable.
5. [High, pre-existing] the MES branch now propagates
   amdgpu_mes_reg_write_reg_wait()'s error instead of hardcoding
   success.
6. [Medium] threshold warning no longer claims a fallback that
   gmc_v10/v11/v12 do not implement.

Open question kept from v1: a KIQ command that timed out remains
queued in the ring; if the ring recovers late, a duplicate
(idempotent) invalidation could race a CPU MMIO flush. Input on
whether the req/ack handshake needs protection for that case is
welcome. Alternative direction also still open: fix the S4 resume
ordering itself (RLC/ME vs GFXOFF) instead of a runtime fallback -
happy to run tracing on the affected hardware.

Patch 1/1 follows.

Denis Pisarev

Denis Pisarev (1):
  drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is
    unresponsive

 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  18 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |   2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 111 ++++++++++++++++++------
 5 files changed, 102 insertions(+), 33 deletions(-)

-- 
2.55.0

[RFC PATCH v2 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

[Denis Pisarev]

After resume from S4 (hibernation) on gmc_v9 parts with GFXOFF
(observed on Cezanne / Ryzen 7 PRO 5850U, kernel 7.1.8), KIQ-based TLB
flushes start failing at the moment of the thaw and keep failing for
hours of normal desktop use:

  amdgpu 0000:07:00.0: failed to write reg 28b4 wait reg 28c6
  amdgpu 0000:07:00.0: failed to write reg 1a6f4 wait reg 1a706

(80-140 errors/hour measured over 9+ hours; bugzilla 219492). The KIQ
ring stays sched.ready throughout, so readiness does not reflect the
state of the hardware in this failure mode.

Two problems follow from the current code: every failed flush burns
the full ~5 s KIQ retry window before erroring out (desktop-wide
sluggishness), and the invalidation is then silently dropped (stale
TLBs - correctness).

Make the failure observable and self-healing:

- amdgpu_gmc_fw_reg_write_reg_wait() returns 0/-ETIME, propagates MES
  errors, and counts consecutive failures per KIQ instance

- gmc_v9_0_flush_gpu_tlb() falls back to a new
  gmc_v9_0_flush_gpu_tlb_mmio() helper (the former pre-KIQ MMIO code,
  now with irqsave locking since it is reachable at runtime) when the
  KIQ submit fails, so the invalidation is no longer dropped

- after AMDGPU_KIQ_FLUSH_MAX_FAIL (3) consecutive failures the KIQ
  path is skipped entirely until the counter resets, so wedged systems
  stop paying the 5 s retry window per flush

- the MMIO fallback is restricted to process context on bare metal:
  amdgpu_gfx_off_ctrl() is used to hold the GC block awake across the
  direct register access (it may sleep), and SR-IOV VFs keep
  proxying through KIQ as before

- the counter resets on every success and in gmc_v9_0_hw_fini(); every
  suspend/resume cycle re-arms the KIQ path; nothing is disabled
  proactively

Changes since v1 (addressing the sashiko-bot review):
- hold GFXOFF off around runtime MMIO access (was: unguarded - could
  hit power-gated registers)
- per-KIQ-instance failure counter instead of a global one (multi-XCC
  cross-talk)
- invalidate_lock is now taken irqsave (the path is runtime-reachable)
- VFs and interrupt context never take the runtime MMIO fallback
- MES path errors are propagated instead of hardcoded success
- reworded the threshold warning to not promise a fallback that
  gmc_v10/v11/v12 do not implement

Known limitation / open question: a KIQ command that already timed out
stays queued in the ring; if the ring recovers late it could emit a
duplicate invalidation concurrently with a CPU MMIO flush. The command
is semantically an idempotent invalidate, but reviewer input on
whether the req/ack handshake needs protection here is welcome.

gmc_v10/v11/v12 call sites are unchanged and compile-safe (int return
used as statement). They can get the same fallback once the approach
is agreed for gmc_v9.

The sibling PASID path (amdgpu_gmc_flush_gpu_tlb_pasid) already has an
-ETIME/MMIO split; this brings the per-VMID path in line with it.

Root-cause note: with GFXOFF held off across the S4 cycle (debugfs
amdgpu_gfxoff), zero errors occur across resume and 30 min of use vs
~70-140 in the control arm; the wedge forms in the S4 resume window
while GFXOFF is allowed, consistent with the existing semaphore
workaround comment in gmc_v9_0.c.

Signed-off-by: Denis Pisarev <pisarevden@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h     |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h |   2 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c |  18 ++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h |   2 +-
 drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c   | 111 ++++++++++++++++++------
 5 files changed, 102 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b09410d6..cd5d9e56e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -360,6 +360,8 @@ enum amdgpu_kiq_irq {
 #define MAX_KIQ_REG_WAIT       5000 /* in usecs, 5ms */
 #define MAX_KIQ_REG_BAILOUT_INTERVAL   5 /* in msecs, 5ms */
 #define MAX_KIQ_REG_TRY 1000
+/* consecutive KIQ TLB flush failures before falling back to MMIO */
+#define AMDGPU_KIQ_FLUSH_MAX_FAIL 3
 
 /*
  * BIOS.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
index 54c1eb9c4..e2aceb99c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
@@ -167,6 +167,8 @@ struct amdgpu_kiq {
 	struct amdgpu_irq_src	irq;
 	const struct kiq_pm4_funcs *pmf;
 	void			*mqd_backup;
+	/* consecutive TLB flush reg access failures on this instance */
+	atomic_t		flush_failures;
 };
 
 /*
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
index 5d6149ba7..49d3d6651 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
@@ -874,7 +874,7 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 	return r;
 }
 
-void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
+int amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
 				      uint32_t xcc_inst)
@@ -886,9 +886,8 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 	uint32_t seq;
 
 	if (adev->mes.ring[MES_PIPE_INST(xcc_inst, 0)].sched.ready) {
-		amdgpu_mes_reg_write_reg_wait(adev, reg0, reg1,
-					      ref, mask, xcc_inst);
-		return;
+		return amdgpu_mes_reg_write_reg_wait(adev, reg0, reg1,
+						     ref, mask, xcc_inst);
 	}
 
 	spin_lock_irqsave(&kiq->ring_lock, flags);
@@ -919,13 +918,20 @@ void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 	if (cnt > MAX_KIQ_REG_TRY)
 		goto failed_kiq;
 
-	return;
+	atomic_set(&adev->gfx.kiq[xcc_inst].flush_failures, 0);
+	return 0;
 
 failed_undo:
 	amdgpu_ring_undo(ring);
 	spin_unlock_irqrestore(&kiq->ring_lock, flags);
 failed_kiq:
-	dev_err(adev->dev, "failed to write reg %x wait reg %x\n", reg0, reg1);
+	if (atomic_inc_return(&adev->gfx.kiq[xcc_inst].flush_failures) ==
+			AMDGPU_KIQ_FLUSH_MAX_FAIL)
+		dev_warn(adev->dev,
+			 "KIQ reg access keeps failing, MMIO fallback recommended\n");
+	dev_err_ratelimited(adev->dev,
+			    "failed to write reg %x wait reg %x\n", reg0, reg1);
+	return -ETIME;
 }
 
 /**
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
index ddb0d500e..303e0ee98 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h
@@ -447,7 +447,7 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
 				   uint32_t flush_type, bool all_hub,
 				   uint32_t inst);
-void amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
+int amdgpu_gmc_fw_reg_write_reg_wait(struct amdgpu_device *adev,
 				      uint32_t reg0, uint32_t reg1,
 				      uint32_t ref, uint32_t mask,
 				      uint32_t xcc_inst);
diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
index 8a5c44810..11fc9085e 100644
--- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
@@ -764,27 +764,28 @@ static bool gmc_v9_0_get_atc_vmid_pasid_mapping_info(struct amdgpu_device *adev,
  */
 
 /**
- * gmc_v9_0_flush_gpu_tlb - tlb flush with certain type
+ * gmc_v9_0_flush_gpu_tlb_mmio - tlb flush via direct MMIO
  *
  * @adev: amdgpu_device pointer
+ * @hub: vmhub to flush
  * @vmid: vm instance to flush
  * @vmhub: which hub to flush
- * @flush_type: the flush type
+ * @inv_req: invalidation request payload
  *
- * Flush the TLB for the requested page table using certain type.
+ * Direct CPU access to the invalidation engine. Callers must ensure
+ * the target block cannot power gate across the access (GFXOFF needs
+ * to be held off at runtime) and must hold no other locks.
  */
-static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
-					uint32_t vmhub, uint32_t flush_type)
+static void gmc_v9_0_flush_gpu_tlb_mmio(struct amdgpu_device *adev,
+					struct amdgpu_vmhub *hub,
+					uint32_t vmid, uint32_t vmhub,
+					u32 inv_req)
 {
 	bool use_semaphore = gmc_v9_0_use_invalidate_semaphore(adev, vmhub);
-	u32 j, inv_req, tmp, sem, req, ack, inst;
 	const unsigned int eng = 17;
-	struct amdgpu_vmhub *hub;
-
-	BUG_ON(vmhub >= AMDGPU_MAX_VMHUBS);
+	unsigned long flags;
+	u32 j, tmp, sem, req, ack, inst;
 
-	hub = &adev->vmhub[vmhub];
-	inv_req = gmc_v9_0_get_invalidate_req(vmid, flush_type);
 	sem = hub->vm_inv_eng0_sem + hub->eng_distance * eng;
 	req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
 	ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
@@ -794,21 +795,8 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	else
 		inst = vmhub;
 
-	/* This is necessary for SRIOV as well as for GFXOFF to function
-	 * properly under bare metal
-	 */
-	if (adev->gfx.kiq[inst].ring.sched.ready &&
-	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
-		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
-		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
-
-		amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack, inv_req,
-						 1 << vmid, inst);
-		return;
-	}
-
 	/* This path is needed before KIQ/MES/GFXOFF are set up */
-	spin_lock(&adev->gmc.invalidate_lock);
+	spin_lock_irqsave(&adev->gmc.invalidate_lock, flags);
 
 	/*
 	 * It may lose gpuvm invalidate acknowldege state across power-gating
@@ -870,7 +858,7 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 			WREG32_SOC15_IP_NO_KIQ(GC, sem, 0, GET_INST(GC, inst));
 	}
 
-	spin_unlock(&adev->gmc.invalidate_lock);
+	spin_unlock_irqrestore(&adev->gmc.invalidate_lock, flags);
 
 	if (j < adev->usec_timeout)
 		return;
@@ -878,6 +866,70 @@ static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
 	DRM_ERROR("Timeout waiting for VM flush ACK!\n");
 }
 
+/**
+ * gmc_v9_0_flush_gpu_tlb - tlb flush with certain type
+ *
+ * @adev: amdgpu_device pointer
+ * @vmid: vm instance to flush
+ * @vmhub: which hub to flush
+ * @flush_type: the flush type
+ *
+ * Flush the TLB for the requested page table using certain type.
+ */
+static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
+					uint32_t vmhub, uint32_t flush_type)
+{
+	u32 inv_req;
+	const unsigned int eng = 17;
+	struct amdgpu_vmhub *hub;
+	u32 inst;
+
+	BUG_ON(vmhub >= AMDGPU_MAX_VMHUBS);
+
+	hub = &adev->vmhub[vmhub];
+	inv_req = gmc_v9_0_get_invalidate_req(vmid, flush_type);
+
+	if (vmhub >= AMDGPU_MMHUB0(0))
+		inst = 0;
+	else
+		inst = vmhub;
+
+	/* This is necessary for SRIOV as well as for GFXOFF to function
+	 * properly under bare metal
+	 */
+	if (adev->gfx.kiq[inst].ring.sched.ready &&
+	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
+		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
+		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
+
+		if (atomic_read(&adev->gfx.kiq[inst].flush_failures) <
+				AMDGPU_KIQ_FLUSH_MAX_FAIL) {
+			if (!amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack,
+							      inv_req,
+							      1 << vmid, inst))
+				return;
+			/* KIQ submit failed; error already logged */
+		}
+
+		/*
+		 * MMIO fallback: invalidation must not be silently dropped
+		 * when KIQ is unresponsive. Direct register access is only
+		 * safe in process context (amdgpu_gfx_off_ctrl may sleep)
+		 * and on bare metal; SR-IOV VFs must keep proxying through
+		 * KIQ.
+		 */
+		if (in_interrupt() || amdgpu_sriov_vf(adev))
+			return;
+
+		amdgpu_gfx_off_ctrl(adev, false);
+		gmc_v9_0_flush_gpu_tlb_mmio(adev, hub, vmid, vmhub, inv_req);
+		amdgpu_gfx_off_ctrl(adev, true);
+		return;
+	}
+
+	gmc_v9_0_flush_gpu_tlb_mmio(adev, hub, vmid, vmhub, inv_req);
+}
+
 /**
  * gmc_v9_0_flush_gpu_tlb_pasid - tlb flush via pasid
  *
@@ -2237,6 +2289,13 @@ static void gmc_v9_0_gart_disable(struct amdgpu_device *adev)
 static int gmc_v9_0_hw_fini(struct amdgpu_ip_block *ip_block)
 {
 	struct amdgpu_device *adev = ip_block->adev;
+	int i;
+
+	/* KIQ instances are re-initialized on the next resume; re-arm
+	 * the MMIO fallback logic
+	 */
+	for (i = 0; i < AMDGPU_MAX_GC_INSTANCES; i++)
+		atomic_set(&adev->gfx.kiq[i].flush_failures, 0);
 
 	gmc_v9_0_gart_disable(adev);
 
-- 
2.55.0

Re: [RFC PATCH v2 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] SR-IOV VFs and interrupt contexts silently drop TLB flushes once KIQ failure threshold is reached.
- [High] KIQ and MMIO fallback use the same invalidation engine, causing hardware register corruption when racing.
--

--- Patch [1]: [RFC PATCH v2 1/1] drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive ---
commit 9356e0619308f1d18159aa347d78c9e8c47151eb
Author: Denis Pisarev <pisarevden@gmail.com>
drm/amdgpu: fall back to MMIO TLB invalidation when KIQ is unresponsive

This commit implements an MMIO fallback for TLB invalidation on gmc_v9 when
KIQ becomes unresponsive after resume. It limits KIQ retries to 3 consecutive
failures before switching to direct MMIO, bypassing the long timeout window.
The fallback is restricted to bare metal process contexts.

> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
[ ... ]
> +static void gmc_v9_0_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid,
> +					uint32_t vmhub, uint32_t flush_type)
> +{
[ ... ]
> +	if (adev->gfx.kiq[inst].ring.sched.ready &&
> +	    (amdgpu_sriov_runtime(adev) || !amdgpu_sriov_vf(adev))) {
> +		uint32_t req = hub->vm_inv_eng0_req + hub->eng_distance * eng;
> +		uint32_t ack = hub->vm_inv_eng0_ack + hub->eng_distance * eng;
> +
> +		if (atomic_read(&adev->gfx.kiq[inst].flush_failures) <
> +				AMDGPU_KIQ_FLUSH_MAX_FAIL) {
> +			if (!amdgpu_gmc_fw_reg_write_reg_wait(adev, req, ack,
> +							      inv_req,
> +							      1 << vmid, inst))
> +				return;
> +			/* KIQ submit failed; error already logged */
> +		}
> +
> +		/*
> +		 * MMIO fallback: invalidation must not be silently dropped
> +		 * when KIQ is unresponsive. Direct register access is only
> +		 * safe in process context (amdgpu_gfx_off_ctrl may sleep)
> +		 * and on bare metal; SR-IOV VFs must keep proxying through
> +		 * KIQ.
> +		 */
> +		if (in_interrupt() || amdgpu_sriov_vf(adev))
> +			return;

[Severity: High]
Does this silently drop TLB flushes for SR-IOV VFs and interrupt contexts?

If the KIQ times out 3 consecutive times, the flush_failures threshold is
reached. On subsequent TLB flush requests from a VF or interrupt context,
the threshold check fails and KIQ submission is bypassed entirely.

The code then hits this check and returns immediately, permanently dropping
all subsequent TLB invalidations until the counter resets. Could this lead to
stale TLBs, memory corruption, or arbitrary memory faults for the VF?

> +
> +		amdgpu_gfx_off_ctrl(adev, false);
> +		gmc_v9_0_flush_gpu_tlb_mmio(adev, hub, vmid, vmhub, inv_req);

[Severity: High]
Can a hardware race condition occur here if KIQ recovers?

When the driver encounters a KIQ timeout, the command is left in the KIQ
ring. Once the failure threshold is met, the CPU begins writing directly
to engine 17 via gmc_v9_0_flush_gpu_tlb_mmio().

If the KIQ command processor unwedges and processes its delayed queue,
it will emit writes to the same REQ and ACK registers concurrently with
the CPU's MMIO accesses. Could overwriting the VM_INVALIDATE_ENG0_REQ
register lead to lost TLB flush requests, or cause KIQ to hang waiting
for an ACK bit that never arrives?

> +		amdgpu_gfx_off_ctrl(adev, true);
> +		return;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260820105716.148732-2-pisarevden@gmail.com?part=1