Applied. Thanks!
On Fri, Oct 16, 2026 at 5:11 AM Christian König <[email protected]> wrote: > > On 10/16/26 05:25, John Fisher wrote: > > Thanks Christian. Since I don't have commit access, I assume that if > > all looks good the commit will be passed on to be applied? > > Alex usually picks those up after a few days and merges it into > amd-staging-drm-next. > > Regards, > Christian. > > > > > Thanks, > > John > > > > On Fri, 9 Oct 2026 at 20:00, Christian König <[email protected]> > > wrote: > >> > >> On 10/8/26 22:45, John Fisher wrote: > >>> amdgpu_vm_make_compute() switches a VM from SDMA to CPU page table > >>> updates when a KFD process acquires it. It syncs "with the last SDMA > >>> update/clear" through amdgpu_bo_sync_wait() on the root PD, but that > >>> walks the reservation with DMA_RESV_USAGE_READ, and the clears of the > >>> VM's page tables (the root PD from amdgpu_vm_init(), and the PDs/PTs > >>> the seq64 mapping allocates at the render node's open) are added there > >>> by amdgpu_vm_sdma_commit() with DMA_RESV_USAGE_BOOKKEEP only: > >>> amdgpu_vm_pt_clear() commits them with a NULL fence, so they do not > >>> reach vm->last_update. The CPU path's own waits (amdgpu_vm_cpu_update(), > >>> amdgpu_bo_kmap()) use DMA_RESV_USAGE_KERNEL. None of them waits for a > >>> clear that is still queued. > >>> > >>> When one is, the CPU writes the first entries of the new compute VM (the > >>> KFD trap handler's TBA/TMA page at AMDGPU_VA_RESERVED_TRAP_START, mapped > >>> during ACQUIRE_VM) into a page directory the SDMA then clears. The > >>> entries are gone, the handler's page is unmapped, and the first wave > >>> save of any queue of the process faults in the SQC instruction fetch at > >>> the handler's address (GCVM_L2_PROTECTION_FAULT_STATUS 0x008012B1, > >>> PERMISSION_FAULTS 0xb). The MES then fails REMOVE_QUEUE and SUSPEND, and > >>> the device takes a MODE1 reset with VRAM lost on 7.2.9; on 7.3-rc6 the > >>> queue is reset instead. > >>> > >>> The window is the time between the render node's open and ACQUIRE_VM. > >>> ROCr spends milliseconds there and is practically never hit; a minimal > >>> KFD client that opens the render node and acquires the VM at once is hit > >>> most of the time. On a Radeon AI PRO R9700 (gfx1201, Resizable BAR, so > >>> vm_update_mode defaults to CPU updates for compute VMs), a wave reading > >>> the trap handler's page right after queue creation found it unmapped in > >>> 8 of 12 runs; a 5 ms sleep between the open and ACQUIRE_VM made that 0 > >>> of 9, and the same sleep after queue creation instead 3 of 3. > >>> > >>> The wait covered these jobs until commit 926c79ad6ecd ("drm/amdgpu: > >>> promote the implicit sync to the dependent read fences") narrowed > >>> amdgpu_sync_resv() from DMA_RESV_USAGE_BOOKKEEP to DMA_RESV_USAGE_READ; > >>> the VM's page table jobs have been added with BOOKKEEP since commit > >>> 4247084057cf ("drm/amdgpu: use DMA_RESV_USAGE_BOOKKEEP v2"), so from > >>> v6.16 on the sync in amdgpu_vm_make_compute() waits for nothing of > >>> them. > >>> > >>> Record the clears in vm->last_update, as amdgpu_vm_update_pdes() does > >>> with its updates, and wait for it, uninterruptibly, before switching the > >>> update functions. The clears all go through vm->delayed, so the last one > >>> covers the earlier ones; immediate clears are not recorded, as before. > >>> > >>> With this, on 7.3-rc6 and on 7.2.9, the same client reads the handler > >>> 10 of 10 times, 8 busy queues on the 4 compute slots complete in 5 of 5 > >>> runs on 7.3-rc6 and 3 of 3 on 7.2.9 (1 of 6 before), 16 and 32 queues > >>> complete on 7.3-rc6, and two processes with 8 queues each complete > >>> together on both, with no fault. > >>> > >>> Fixes: 926c79ad6ecd ("drm/amdgpu: promote the implicit sync to the > >>> dependent read fences") > >>> Cc: [email protected] # v6.16+ > >>> Cc: Prike Liang <[email protected]> > >>> Signed-off-by: John Fisher <[email protected]> > >> > >> Reviewed-by: Christian König <[email protected]> > >> > >>> --- > >>> v2: wait for vm->last_update, in which amdgpu_vm_pt_clear() now records > >>> the clears, instead of every BOOKKEEP fence on the root PD; wait > >>> uninterruptibly without checking the result; keep the original > >>> comment (Christian) > >>> v1: > >>> https://lore.kernel.org/amd-gfx/[email protected]/ > >>> > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 5 +---- > >>> drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c | 2 +- > >>> 2 files changed, 2 insertions(+), 5 deletions(-) > >>> > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>> index 29187fabc..757a037ed 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c > >>> @@ -2737,10 +2737,7 @@ int amdgpu_vm_make_compute(struct amdgpu_device > >>> *adev, struct amdgpu_vm *vm) > >>> > >>> if (vm->use_cpu_for_update) { > >>> /* Sync with last SDMA update/clear before switching to CPU > >>> */ > >>> - r = amdgpu_bo_sync_wait(vm->root.bo, > >>> - AMDGPU_FENCE_OWNER_UNDEFINED, true); > >>> - if (r) > >>> - goto unreserve_bo; > >>> + dma_fence_wait(vm->last_update, false); > >>> > >>> vm->update_funcs = &amdgpu_vm_cpu_funcs; > >>> r = amdgpu_vm_pt_map_tables(adev, vm); > >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > >>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > >>> index c03327f12..cdbc2e931 100644 > >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm_pt.c > >>> @@ -427,7 +427,7 @@ int amdgpu_vm_pt_clear(struct amdgpu_device *adev, > >>> struct amdgpu_vm *vm, > >>> if (r) > >>> goto exit; > >>> > >>> - r = vm->update_funcs->commit(¶ms, NULL); > >>> + r = vm->update_funcs->commit(¶ms, &vm->last_update); > >>> exit: > >>> drm_dev_exit(idx); > >>> return r; > >> >
