On 13-10-2026 01:42 pm, Christian König wrote:
On 10/13/26 09:56, 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(), called from amdgpu_vm_validate(), 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.
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.
Signed-off-by: Sunil Khatri<[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c | 6 ++++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c | 37 +++++++++++++++++++++++++
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.h | 1 +
3 files changed, 44 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
index 992f7cb3a3102..0ad94b09c6b46 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ids.c
@@ -581,6 +581,12 @@ 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) {
+ adev->gfxhub.funcs->setup_vm_pt_regs(adev,
adev->vm_manager.spm_vmid, 0);
+ amdgpu_gmc_flush_gpu_tlb(adev,
adev->vm_manager.spm_vmid,
+ AMDGPU_GFXHUB(0),
TLB_FLUSH_LEGACY);
+ }
}
mutex_unlock(&id_mgr->lock);
}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index de992405ed718..3967dadb8d425 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -568,6 +568,41 @@ 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) {
+ mutex_unlock(&id_mgr->lock);
+ return;
+ }
+
+ pd_addr = amdgpu_gmc_pd_addr(vm->root.bo);
+ adev->gfxhub.funcs->setup_vm_pt_regs(adev, adev->vm_manager.spm_vmid,
pd_addr);
That function seriously needs a wrapper in amdgpu_gmc.c.
You mean to move this function totally in the amdgpu_gmc.c or let this
be here and create a wrapper in gmc layet calling this function?
+ amdgpu_gmc_flush_gpu_tlb(adev, adev->vm_manager.spm_vmid,
AMDGPU_GFXHUB(0),
+ TLB_FLUSH_LEGACY);
+ mutex_unlock(&id_mgr->lock);
+}
+
/**
* amdgpu_vm_validate - validate evicted BOs tracked in the VM
*
@@ -630,6 +665,8 @@ int amdgpu_vm_validate(struct amdgpu_device *adev, struct
amdgpu_vm *vm,
amdgpu_vm_bo_needs_update(bo_base);
}
+ amdgpu_vm_spm_update(adev, vm);
+
Wrong place for that, this needs to be in the caller of
amdgpu_vmid_alloc_reserved() and in
amdgpu_userq_vm_validate_and_restore_queue().
If i understood it correctly, amdgpu_vm_ioctl ->
amdgpu_vmid_alloc_reserved after this call returns when we should call
amdgpu_vm_spm_update?
For the userqueues,
amdgpu_userq_vm_validate_and_restore_queue{ ... ret = amdgpu_vm_update_pdes(adev, vm, false); if (ret)
goto unlock_all; is it safe to add after this point ? amdgpu_vm_spm_update(adev, vm);
}
And while at it please rename amdgpu_vmid_alloc_reserved() and
amdgpu_vmid_free_reserved() into amdgpu_vmid_alloc_spm() and
amdgpu_vmid_free_spm().
Noted
Regards
Sunil Khatri
Regards,
Christian.
if (!ticket)
return 0;
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),
