[PATCH AUTOSEL 6.19-6.12] drm/amdkfd: Handle GPU reset and drain retry fault race

Sasha Levin
2026-02-14T01:05:42Z

From: Philip Yang <Philip.Yang@amd.com>

[ Upstream commit 5b57c3c3f22336e8fd5edb7f0fef3c7823f8eac1 ]

Only check and drain IH1 ring if CAM is not enabled.

If GPU is under reset, don't access IH to drain retry fault.

Signed-off-by: Philip Yang <Philip.Yang@amd.com>
Reviewed-by: Harish Kasiviswanathan <Harish.Kasiviswanathan@amd.com>
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---

LLM Generated explanations, may be completely bogus:

Now I have all the information needed for a thorough analysis. Let me
compile my findings.

---

## Detailed Analysis

### 1. Commit Message Analysis

**Subject:** "drm/amdkfd: Handle GPU reset and drain retry fault race"

The commit addresses a **race condition** between GPU reset and the
drain retry fault mechanism. The message describes two specific fixes:

1. "Only check and drain IH1 ring if CAM is not enabled" - a correctness
fix for interrupt handler ring selection logic
2. "If GPU is under reset, don't access IH to drain retry fault" -
protecting against hardware access during GPU reset

Both are bug fix descriptions, not feature additions.

### 2. Code Change Analysis

The patch makes two distinct changes in `kfd_svm.c`:

#### Change A: `svm_range_drain_retry_fault()` - GPU reset protection

**Before (current code):**

```2337:2364:drivers/gpu/drm/amd/amdkfd/kfd_svm.c
static void svm_range_drain_retry_fault(struct svm_range_list *svms)
{
// ... iterates over GPUs ...
amdgpu_ih_wait_on_checkpoint_process_ts(pdd->dev->adev,
pdd->dev->adev->irq.retry_cam_enabled ?
&pdd->dev->adev->irq.ih :
&pdd->dev->adev->irq.ih1);
// ... no reset protection ...
}
```

**After (patched code):** Adds
`down_read_trylock(&pdd->dev->adev->reset_domain->sem)` before accessing
the IH hardware and `up_read(...)` after. If the trylock fails (GPU is
resetting), it `continue`s to the next GPU.

**Why this matters:** `svm_range_drain_retry_fault()` calls
`amdgpu_ih_wait_on_checkpoint_process_ts()` which calls
`amdgpu_ih_get_wptr()`. Looking at `vega20_ih_get_wptr()`, it does
`RREG32_NO_KIQ(ih_regs->ih_rb_wptr)` - a **direct hardware register
read**. During GPU reset (`amdgpu_device_lock_reset_domain` takes a
write lock on `reset_domain->sem`), the hardware is being torn down and
reinitialized. Accessing registers during this window can cause:
- **System hangs** (MMIO reads to offline hardware can hang the CPU bus)
- **Garbage data reads** leading to incorrect behavior
- **Kernel crashes** if the driver acts on invalid data

This is the **exact same pattern** used throughout the amdgpu driver -
there are **20+ existing call sites** that use
`down_read_trylock(&adev->reset_domain->sem)` to protect hardware
access. The fix follows an established, well-tested pattern.

#### Change B: `svm_range_unmap_from_cpu()` - CAM-aware IH1 check

**Before:**
```c
if (adev->irq.ih1.ring_size) {
```

**After:**
```c
if (!adev->irq.retry_cam_enabled && adev->irq.ih1.ring_size) {
```

The comment already says "Check and drain ih1 ring if cam not available"
but the code was missing the `!retry_cam_enabled` check. When CAM
(Content Addressable Memory) is enabled for retry filtering, retry
faults go through the primary IH ring (`ih`), not `ih1`. Checking `ih1`
when CAM is enabled is incorrect because:
- The timestamp from `ih1` would be stale/irrelevant
- It could cause the code to `continue` early (bypassing the `ih_soft`
check below) with an incorrect `checkpoint_ts`
- This leads to retry faults being incorrectly dropped or not dropped
when they should be

This same logic (`if (adev->irq.retry_cam_enabled) return;` before `ih1`
access) already exists in `amdgpu_gmc_filter_faults_remove()`,
confirming that the fix aligns with the intended design.

### 3. Bug Classification

- **Change A:** Race condition fix - accessing hardware registers
without reset domain protection. This is a **system hang / crash**
bug.
- **Change B:** Logic bug - checking the wrong interrupt handler ring
when CAM is enabled, leading to incorrect retry fault handling
(potential data corruption or stale page mappings).

### 4. Scope and Risk Assessment

- **Lines changed:** ~10 lines of actual logic (plus 1 include)
- **Files changed:** 1 file (`kfd_svm.c`)
- **Complexity:** Low - follows a well-established pattern (20+ examples
in the codebase)
- **Risk of regression:** Very low
- The `down_read_trylock` pattern is used everywhere in amdgpu and is
proven safe
- If trylock fails, we simply skip draining for that GPU (graceful
degradation)
- The CAM check aligns with existing logic in
`amdgpu_gmc_filter_faults_remove()`

### 5. User Impact

- **Affected users:** Anyone using AMD GPUs with KFD
(compute/ROCm/OpenCL workloads) who experiences GPU resets
- **Severity:** HIGH - GPU reset + drain retry fault = potential system
hang when register access hangs the CPU
- **Without fix:** If a GPU reset happens concurrently with
`svm_range_list_fini()` (process exit), the system could hang trying
to read hardware registers from an offline GPU

### 6. Dependencies

- Requires `amdgpu_reset.h` (available since v6.1)
- Requires `reset_domain->sem` infrastructure (available since v6.1)
- Requires `retry_cam_enabled` field (available since the CAM commit in
6.6)
- The `svm_range_drain_retry_fault()` function in its current form (with
the `retry_cam_enabled` ternary) was introduced in 6.12 (commit
`6ef29715ac06`)
- This patch applies cleanly only to trees with `6ef29715ac06` and
`96316211eb5c4` (both in 6.12+)

### 7. Stability Indicators

- **Reviewed-by:** Harish Kasiviswanathan (AMD kernel engineer)
- **Author:** Philip Yang (AMD KFD maintainer, regularly contributes SVM
retry fault fixes)
- **Signed-off-by:** Alex Deucher (AMD GPU subsystem maintainer)
- Pattern is well-established in the driver (20+ existing similar uses)

### Conclusion

This commit fixes two real bugs:

1. A **race condition** between GPU reset and IH ring access that can
cause **system hangs** - this is the critical fix
2. A **logic error** in IH ring selection when CAM is enabled that
causes incorrect retry fault handling

The fix is small (10 lines of logic), follows an extremely well-
established pattern in the amdgpu driver (20+ existing call sites use
the same `down_read_trylock` on `reset_domain->sem`), has been reviewed
by an AMD engineer, was authored by the KFD SVM subsystem expert, and
addresses a real crash/hang scenario. The risk of regression is minimal.

The fix applies to kernels 6.12+ where `6ef29715ac06` ("drm/amdkfd:
Change kfd/svm page fault drain handling") exists.

**YES**

drivers/gpu/drm/amd/amdkfd/kfd_svm.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
index 79ea138897fcf..a10cf8650c92b 100644
--- a/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
+++ b/drivers/gpu/drm/amd/amdkfd/kfd_svm.c
@@ -33,6 +33,7 @@
#include "amdgpu_hmm.h"
#include "amdgpu.h"
#include "amdgpu_xgmi.h"
+#include "amdgpu_reset.h"
#include "kfd_priv.h"
#include "kfd_svm.h"
#include "kfd_migrate.h"
@@ -2349,6 +2350,9 @@ static void svm_range_drain_retry_fault(struct svm_range_list *svms)

pr_debug("drain retry fault gpu %d svms %p\n", i, svms);

+ if (!down_read_trylock(&pdd->dev->adev->reset_domain->sem))
+ continue;
+
amdgpu_ih_wait_on_checkpoint_process_ts(pdd->dev->adev,
pdd->dev->adev->irq.retry_cam_enabled ?
&pdd->dev->adev->irq.ih :
@@ -2358,6 +2362,7 @@ static void svm_range_drain_retry_fault(struct svm_range_list *svms)
amdgpu_ih_wait_on_checkpoint_process_ts(pdd->dev->adev,
&pdd->dev->adev->irq.ih_soft);

+ up_read(&pdd->dev->adev->reset_domain->sem);

pr_debug("drain retry fault gpu %d svms 0x%p done\n", i, svms);
}
@@ -2541,7 +2546,7 @@ svm_range_unmap_from_cpu(struct mm_struct *mm, struct svm_range *prange,
adev = pdd->dev->adev;

/* Check and drain ih1 ring if cam not available */
- if (adev->irq.ih1.ring_size) {
+ if (!adev->irq.retry_cam_enabled && adev->irq.ih1.ring_size) {
ih = &adev->irq.ih1;
checkpoint_wptr = amdgpu_ih_get_wptr(adev, ih);
if (ih->rptr != checkpoint_wptr) {
--
2.51.0