Claude review: drm/amdgpu: integrate VRAM migration into SVM range map path

[Claude Code Review Bot <claude-review@example.com>]

Patch Review

**Overall:** This is the integration patch tying everything together. It has the most issues.

**Bug: Migration failure is silently ignored:**
```c
+		/* Per-range migration */
+		if (migrate_mode != AMDGPU_SVM_MIGRATE_NONE)
+			amdgpu_svm_range_migrate_range(svm, range, migrate_mode);
```
The return value of `amdgpu_svm_range_migrate_range()` is discarded. If migration fails (e.g., VRAM full, SDMA error), the code proceeds to map the range as if nothing happened. This could result in stale system memory mappings when VRAM was expected, or worse, mapping garbage. At minimum, log the failure. Depending on the desired semantics, either fail the map or fall back gracefully with a warning.

**Bug: Variable declaration after statement (C89 violation):**
```c
 	lockdep_assert_held_write(&svm->svm_lock);
 
+	enum amdgpu_svm_migrate_mode migrate_mode = AMDGPU_SVM_MIGRATE_NONE;
 	bool old_access, new_access;
```
The `lockdep_assert_held_write()` is a statement, and variable declarations must come before any statements in kernel C (C89/C99-mixed style). Move the declaration before the `lockdep_assert_held_write()` call, or move the assert after all declarations.

**Concern: `amdgpu_svm_range_rebuild_locked()` behavior change.** The `!rebuild` path was:
```c
-	ret = amdgpu_svm_range_update_gpu(svm, rebuild_start, rebuild_last,
-					  0, NULL, true, true, true);
-	if (!ret)
-		svm->flush_tlb(svm);
-	return ret;
+	return 0;
```
This changes from "unmap GPU mappings and flush TLB" to "do nothing." If the `!rebuild` case represents an invalidation (pages gone, need to unmap GPU PTEs), then returning 0 silently leaves stale GPU mappings. What guarantees that the GPU unmap already happened by this point? This needs justification in the commit message.

**Issue: `#if IS_ENABLED(CONFIG_DRM_AMDGPU_SVM)` guards in `amdgpu_device.c` and `amdgpu_reset.c`:**
```c
+#if IS_ENABLED(CONFIG_DRM_AMDGPU_SVM)
+		amdgpu_svm_migration_init(adev);
+#endif
```
The header already provides a static inline stub returning 0 when `CONFIG_DRM_AMDGPU_SVM` is disabled. These `#if` guards are unnecessary and add noise — just call the function directly and rely on the stub.

**Issue: `range_invalidate_gpu_mapping()` reordering in notifier.** Moving this call:
```c
-		if (clear_pte) {
-			amdgpu_svm_range_gpu_unmap_in_notifier(svm, range,
-									   mmu_range);
-			range_invalidate_gpu_mapping(range);
-		}
+		if (clear_pte) {
+			amdgpu_svm_range_gpu_unmap_in_notifier(svm, range,
+									   mmu_range);
+		}
 
 		drm_gpusvm_range_unmap_pages(&svm->gpusvm, range, &ctx);
+		range_invalidate_gpu_mapping(range);
```
Now `range_invalidate_gpu_mapping()` is called unconditionally (outside the `if (clear_pte)` block), and after `drm_gpusvm_range_unmap_pages()`. This is a semantic change — previously it only happened when PTE clearing was needed. Is this intentional? The commit message should explain why invalidation now always happens and why the ordering change is correct.

**Minor: Deleted function `amdgpu_svm_range_update_gpu` still has callers?** The function is removed in this patch, but the `!rebuild` path in `amdgpu_svm_range_rebuild_locked()` was the only caller. Since that path now returns 0, the deletion is consistent. Good.

**Nit: Trailing whitespace removal:**
```c
-	
 		}
```
Line 1565 — this is fine cleanup but should ideally be in a separate trivial patch to keep the diff focused.

**Good:** The `devmem_possible` and `device_private_page_owner` additions to `gpusvm_ctx` are cleanly integrated:
```c
+	struct drm_gpusvm_ctx gpusvm_ctx = {
+		.read_only = !!(attrs->flags & AMDGPU_SVM_FLAG_GPU_RO),
+		.devmem_possible = hw_devmem,
+		.device_private_page_owner = hw_devmem ?
+			AMDGPU_SVM_PGMAP_OWNER(svm->adev) : NULL,
+	};
```

**Good:** The VRAM PTE flag handling is correct — clearing SYSTEM and SNOOPED bits for VRAM pages:
```c
+		if (entry->proto == AMDGPU_INTERCONNECT_VRAM)
+			seg_pte_flags &= ~(AMDGPU_PTE_SYSTEM | AMDGPU_PTE_SNOOPED);
```

---
Generated by Claude Code Patch Reviewer