Re: [PATCH v1 3/3] drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Pre-existing deadlock risk: `amdgpu_gmc_flush_gpu_tlb_pasid()` uses plain `spin_lock()` for `ring_lock` while KIQ accessor functions use `spin_lock_irqsave()` and run in hardirq context.
--

--- Patch [3]: [PATCH v1 3/3] drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged ---
commit 24e4f92f6cbb016df09e7fbd1d0ec02c367162a1
Author: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>

drm/amdgpu: skip amdgpu_gmc_flush_gpu_tlb_pasid if device is wedged

This commit skips the GPU TLB flush for a specific pasid when the device
is wedged. This avoids failing to wait for the register write, which could
fill the MES buffer and prevent a reset from succeeding.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index f43d85ba4b78b..c33320a6413e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -786,6 +786,9 @@ int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, uint16_t pasid,
>  	int r, cnt = 0;
>  	uint32_t seq;
>  
> +	if (amdgpu_device_is_wedged(adev))
> +		return -ENODEV;
> +
>  	/*
>  	 * A GPU reset should flush all TLBs anyway, so no need to do
>  	 * this while one is ongoing.

[ ... ]

>  	} else {
>  		/* 2 dwords flush + 8 dwords fence */
>  		ndw = kiq->pmf->invalidate_tlbs_size + 8;
> 
>  		if (adev->gmc.flush_tlb_needs_extra_type_2)
>  			ndw += kiq->pmf->invalidate_tlbs_size;
> 
>  		if (adev->gmc.flush_tlb_needs_extra_type_0)
>  			ndw += kiq->pmf->invalidate_tlbs_size;
> 
>  		spin_lock(&adev->gfx.kiq[inst].ring_lock);

[Severity: High]
This isn't a bug introduced by this patch, but I noticed that
amdgpu_gmc_flush_gpu_tlb_pasid() acquires the KIQ ring_lock using plain
spin_lock() here, while other KIQ accessor functions like amdgpu_kiq_wreg()
acquire it with spin_lock_irqsave() because they can run in hardirq context.

If a hardware interrupt occurs on the same CPU while this lock is held,
could an interrupt handler attempting to read or write a register using KIQ
(such as via WREG32 on an SR-IOV guest) deadlock when trying to acquire the
same ring_lock?

Does this lock acquisition need to be changed to spin_lock_irqsave()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260810165322.24682-1-pierre-eric.pelloux-prayer@amd.com?part=3