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

[sashiko-bot@kernel.org]

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