On 13-10-2026 03:21 pm, Christian König wrote:
Sure i will add the wait for vm to be idle here there is amdgpu_sync_wait which make sure that this vmid all jobs are done and the next step is just setting pt to 0 and flush which doesnt depend upong the root pd here.On 10/13/26 10:49, Sunil Khatri wrote:MES userqueues bypass amdgpu_vm_flush(), so a reservation granted to a userqueue never got its VMID's page table programmed. Add amdgpu_vm_spm_update() to bind the fixed SPM VMID's page-table-base to the reserving VM's root PD when that VM holds the reservation; it is a no-op for every other VM. It is called right after a successful AMDGPU_VM_OP_RESERVE_VMID (amdgpu_vm_ioctl()) and from amdgpu_userq_vm_validate_and_restore_queue(), the two paths that can actually change the binding. amdgpu_vmid_free_reserved() unbinds it directly to a dummy address under the same lock that clears the reservation, so stray SPM accesses fault instead of racing a new reservation's binding. Both binds go through the new amdgpu_gmc_bind_vmid() helper, which pairs setup_vm_pt_regs() with the matching TLB flush for a VMID bound outside the normal per-job VM flush path. Signed-off-by: Sunil Khatri <[email protected]> --- drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c | 24 +++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h | 2 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 4 +++ drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c | 7 ++++ drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 43 +++++++++++++++++++++-- drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 + 6 files changed, 79 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c index 5d4276cd0f7d3..4204c3c750a8d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c @@ -787,6 +787,30 @@ void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid, dev_err(adev->dev, "Error flushing GPU TLB using the SDMA (%d)!\n", r); }+/**+ * amdgpu_gmc_bind_vmid - bind a VMID's page table base and flush its TLB + * + * @adev: amdgpu device pointer + * @vmhub: vmhub the VMID belongs to + * @vmid: VMID to bind + * @pd_addr: new page directory address, 0 to unbind to a faulting address + * + * For VMIDs that are bound directly to a VM's page table outside the + * normal per-job VM flush path, e.g. a dedicated out-of-pool VMID. + */ +void amdgpu_gmc_bind_vmid(struct amdgpu_device *adev, unsigned int vmhub, + unsigned int vmid, uint64_t pd_addr) +{ + bool is_mmhub = AMDGPU_IS_MMHUB0(vmhub) || AMDGPU_IS_MMHUB1(vmhub); + + if (is_mmhub) + adev->mmhub.funcs->setup_vm_pt_regs(adev, vmid, pd_addr); + else + adev->gfxhub.funcs->setup_vm_pt_regs(adev, vmid, pd_addr); + + amdgpu_gmc_flush_gpu_tlb(adev, vmid, vmhub, TLB_FLUSH_LEGACY); +} + static void amdgpu_gmc_flush_pasid_regs(struct amdgpu_device *adev, u16 pasid, u32 flush_type, bool all_hub, u32 inst) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h index 76e6f1f96913c..d17055d4c50a9 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.h @@ -444,6 +444,8 @@ int amdgpu_gmc_ras_sw_init(struct amdgpu_device *adev); int amdgpu_gmc_allocate_vm_inv_eng(struct amdgpu_device *adev); void amdgpu_gmc_flush_gpu_tlb(struct amdgpu_device *adev, uint32_t vmid, uint32_t vmhub, uint32_t flush_type); +void amdgpu_gmc_bind_vmid(struct amdgpu_device *adev, unsigned int vmhub, + unsigned int vmid, uint64_t pd_addr); int amdgpu_gmc_flush_gpu_tlb_pasid(struct amdgpu_device *adev, u16 pasid, u32 flush_type, bool all_hub, u32 inst); int amdgpu_gmc_flush_gpu_tlb_pasid_xccs(struct amdgpu_device *adev, u16 pasid, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c index 992f7cb3a3102..8e60e5fdd210d 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c @@ -581,6 +581,10 @@ void amdgpu_vmid_free_reserved(struct amdgpu_device *adev, struct amdgpu_vm *vm, if (vm->reserved_vmid[vmhub]) { vm->reserved_vmid[vmhub] = NULL; id_mgr->reserved_vmid = false; + + if (adev->vm_manager.spm_vmid && adev->gfxhub.funcs) + amdgpu_gmc_bind_vmid(adev, AMDGPU_GFXHUB(0), + adev->vm_manager.spm_vmid, 0);Yeah that will break the uAPI and is not something we can do. The problem is that we need to wait for the VM to be idle. Is the VM locked when that is called? If yes we can just wait for it to be idle before calling this.
} mutex_unlock(&id_mgr->lock); } diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c index e29823ae8672e..d45357e092bb5 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_userq.c @@ -1075,6 +1075,13 @@ amdgpu_userq_vm_validate_and_restore_queue(struct amdgpu_userq_mgr *uq_mgr) if (unlikely(ret)) goto unlock_all;+ /*+ * Userqueues never go through amdgpu_vm_flush(), so this is + * the only place that can rebind the SPM VMID's page table + * for a userqueue-held reservation. + */ + amdgpu_vm_spm_update(adev, vm); + /* This locks and validates the remaining evicted BOs */ ret = amdgpu_userq_bo_validate(adev, &exec, vm); drm_exec_retry_on_contention(&exec); diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c index de992405ed718..05358e7d81cb8 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c @@ -568,6 +568,39 @@ uint64_t amdgpu_vm_generation(struct amdgpu_device *adev, struct amdgpu_vm *vm) return result; }+/**+ * amdgpu_vm_spm_update - bind the SPM VMID's page table base to @vm + * + * @adev: amdgpu device pointer + * @vm: vm to check against the SPM reservation + * + * The SPM VMID is fixed (adev->vm_manager.spm_vmid), so its page-table-base + * register must track whichever VM currently holds the reservation instead + * of being set per-job like a normal VMID. Bind it to @vm's real root PD + * address if @vm holds the reservation; otherwise do nothing, since @vm is + * not the one that could legitimately change the current binding. + */ +void amdgpu_vm_spm_update(struct amdgpu_device *adev, struct amdgpu_vm *vm) +{ + struct amdgpu_kq_vmid_mgr *id_mgr = + &adev->vm_manager.id_mgr[AMDGPU_GFXHUB(0)]; + uint64_t pd_addr; + + if (!adev->vm_manager.spm_vmid || !adev->gfxhub.funcs) + return; + + mutex_lock(&id_mgr->lock); + if (vm->reserved_vmid[AMDGPU_GFXHUB(0)] != + &id_mgr->ids[adev->vm_manager.spm_vmid] || !vm->root.bo) {Drop checking vm->root.bo, that makes no sense. Rather call amdgpu_vm_ready() to check if the VM is actually useable.
Noted
+ mutex_unlock(&id_mgr->lock); + return; + } + + pd_addr = amdgpu_gmc_pd_addr(vm->root.bo);This will probably cause lockdep to warn that the VM isn't locked, but I'm not 100% sure.
VM is locked in this loop here in exec manager before making this call.
Please double check if the VM is locked here. Regards, Christian.+ amdgpu_gmc_bind_vmid(adev, AMDGPU_GFXHUB(0), adev->vm_manager.spm_vmid, pd_addr); + mutex_unlock(&id_mgr->lock); +} + /** * amdgpu_vm_validate - validate evicted BOs tracked in the VM * @@ -2958,9 +2991,15 @@ int amdgpu_vm_ioctl(struct drm_device *dev, void *data, struct drm_file *filp) return -EINVAL;switch (args->in.op) {- case AMDGPU_VM_OP_RESERVE_VMID: + case AMDGPU_VM_OP_RESERVE_VMID: { + int r; + /* We only have requirement to reserve vmid from gfxhub */ - return amdgpu_vmid_alloc_reserved(adev, vm, AMDGPU_GFXHUB(0)); + r = amdgpu_vmid_alloc_reserved(adev, vm, AMDGPU_GFXHUB(0)); + if (!r) + amdgpu_vm_spm_update(adev, vm); + return r; + } case AMDGPU_VM_OP_UNRESERVE_VMID: amdgpu_vmid_free_reserved(adev, vm, AMDGPU_GFXHUB(0)); break; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h index b8aa45db1d844..2a2a2ccbaa2cc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h @@ -443,6 +443,7 @@ int amdgpu_vm_lock_individual(struct amdgpu_vm *vm, struct drm_exec *exec, unsigned int num_fences); bool amdgpu_vm_ready(struct amdgpu_vm *vm); uint64_t amdgpu_vm_generation(struct amdgpu_device *adev, struct amdgpu_vm *vm); +void amdgpu_vm_spm_update(struct amdgpu_device *adev, struct amdgpu_vm *vm); int amdgpu_vm_validate(struct amdgpu_device *adev, struct amdgpu_vm *vm, struct ww_acquire_ctx *ticket, int (*callback)(void *p, struct amdgpu_bo *bo),
