Claude review: drm/amdgpu: implement drm_pagemap SDMA migration callbacks

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

Patch Review

This is the largest and most complex patch. Overall well-structured.

**`pre_migrate_fence` unused in both callbacks:**
```c
static int
amdgpu_svm_copy_to_devmem(struct page **pages,
			   struct drm_pagemap_addr *pagemap_addr,
			   unsigned long npages,
			   struct dma_fence *pre_migrate_fence)
```
The `pre_migrate_fence` is passed by the framework but completely ignored. If the framework provides a fence to wait on before migrating (e.g., for serialization), ignoring it could cause data corruption. Either wait on it before starting SDMA copies, or add a comment explaining why it can safely be ignored in this context.

**`copy_to_devmem` and `copy_to_ram` are nearly identical** — ~60 lines of the same batch/flush logic. The only differences are: (1) direction constant, (2) `copy_to_ram` also checks `!pages[i]` in the loop. A shared helper parameterized on direction would reduce duplication and maintenance burden.

**GART window usage — only window 0 is used:**
```c
*gart_addr = amdgpu_compute_gart_address(&adev->gmc, entity, 0);
```
The entity has two GART windows (`gart_window_offs[2]`). Using only window 0 means that in `amdgpu_svm_copy_memory_gart`, for `FROM_RAM_TO_VRAM` the source is GART-mapped and the destination is a direct VRAM MC address, which is correct. But it means only one GART mapping per SDMA copy submission — the existing KFD migrate code uses both windows (0 for src, 1 for dst) for VRAM↔VRAM copies. For RAM↔VRAM this is fine since one side is always direct-mapped, but worth a comment.

**`amdgpu_svm_gart_map` submits a fire-and-forget fence:**
```c
fence = amdgpu_job_submit(job);
dma_fence_put(fence);
```
The GART update fence is dropped immediately. The subsequent `amdgpu_copy_buffer` call will naturally serialize on the same ring, so this is correct as long as both jobs go through the same entity. Since both use `entity->base`, this holds.

**`amdgpu_svm_copy_memory_gart` uses `move_entity` (singular):**
```c
struct amdgpu_ttm_buffer_entity *entity = &adev->mman.move_entity;
```
The current drm-next tree has `move_entities[]` (plural array). This must depend on the base SVM series changing the struct. This is fine if the dependency is correct, but will cause a build failure if someone tries to apply without the base.

**`amdgpu_bo_svm_destroy` doesn't handle imported BOs:**
```c
static void amdgpu_bo_svm_destroy(struct ttm_buffer_object *tbo)
{
	struct amdgpu_bo *bo = ttm_to_amdgpu_bo(tbo);
	struct amdgpu_bo_svm *svm_bo = to_amdgpu_bo_svm(bo);

	amdgpu_bo_kunmap(bo);
	drm_gem_object_release(&bo->tbo.base);
	amdgpu_bo_unref(&bo->parent);
	kvfree(svm_bo);
}
```
Compare to `amdgpu_bo_destroy` which has:
```c
if (drm_gem_is_imported(&bo->tbo.base))
	drm_prime_gem_destroy(&bo->tbo.base, bo->tbo.sg);
```
SVM BOs should never be imported, so this is likely fine, but a `WARN_ON(drm_gem_is_imported(...))` would make the assumption explicit and catch bugs.

**BO created with VRAM_CLEARED but migration will overwrite immediately:**
```c
bp.flags = AMDGPU_GEM_CREATE_NO_CPU_ACCESS |
	   AMDGPU_GEM_CREATE_VRAM_CONTIGUOUS |
	   AMDGPU_GEM_CREATE_VRAM_CLEARED;
```
Clearing VRAM before immediately overwriting it with SDMA copy is wasteful. Consider dropping `AMDGPU_GEM_CREATE_VRAM_CLEARED` — the data will be written by `copy_to_devmem` before any read.

**`populate_mm` unreserves after `drm_pagemap_migrate_to_devmem`:**
```c
ret = drm_pagemap_migrate_to_devmem(&svm_bo->devmem,
				     mm, start, end,
				     &mdetails);

amdgpu_bo_unreserve(&svm_bo->bo);
```
`amdgpu_bo_create()` returns with the BO reserved. The unreserve happens after migration. But if `drm_pagemap_migrate_to_devmem` fails, does the framework still hold any reference to the BO? If `devmem_release` can be called asynchronously after a partial migration, the unreserve here might race. The lifecycle needs careful thought — who drops the last ref on the BO if migration fails completely?

---
Generated by Claude Code Patch Reviewer