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 VM's own page
table jobs (the root clear from amdgpu_vm_init(), and the PD/PT clears
and the seq64 mapping from the render node's open) are added by
amdgpu_vm_sdma_commit() with DMA_RESV_USAGE_BOOKKEEP. 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.

Wait for every fence on the root PD's reservation before switching the
update functions. With this the same client reads the handler 10 of 10
times, 8 busy queues on the 4 compute slots complete in 5 of 5 runs
(1 of 6 before), and two processes with 8 queues each complete together,
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]>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index 29187fabc..8d0c03a40 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -2736,11 +2736,20 @@ int amdgpu_vm_make_compute(struct amdgpu_device *adev, 
struct amdgpu_vm *vm)
                  "CPU update of VM recommended only for large BAR system\n");
 
        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)
+               /*
+                * Sync with the last SDMA update/clear before switching to CPU
+                * updates. The VM's own page table updates are tracked with
+                * DMA_RESV_USAGE_BOOKKEEP on the root PD, which
+                * amdgpu_bo_sync_wait() does not wait for: a clear of a PD 
still
+                * pending here lands after the CPU has written its entries.
+                */
+               long t = dma_resv_wait_timeout(vm->root.bo->tbo.base.resv,
+                                              DMA_RESV_USAGE_BOOKKEEP, true,
+                                              MAX_SCHEDULE_TIMEOUT);
+               if (t < 0) {
+                       r = t;
                        goto unreserve_bo;
+               }
 
                vm->update_funcs = &amdgpu_vm_cpu_funcs;
                r = amdgpu_vm_pt_map_tables(adev, vm);
-- 
2.55.0

Reply via email to