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.
> + 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().
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().
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),
