On 25.09.25 08:27, Jesse.Zhang wrote:
> The amdgpu_vm_set_pasid function now requires the VM root PD's dma_resv lock
> to be held, as enforced by the amdgpu_vm_assert_locked check added in the
> lock refactoring.
Oh, that was probably just a copy&paste mistake on my side.
The amdgpu_vm_set_pasid() function is called directly after amdgpu_vm_init(),
so you actually don't need to hold the lock (because nobody else can use the VM
at that time).
We have removed the use case of KFD using a separate PASSID. So the function
could actually be merged back into amdgpu_vm_init().
Could you take care of that?
Thanks in advance and sorry for the noise,
Christian.
>
> This patch adds the necessary dma_resv_lock/dma_resv_unlock calls around
> both the initial amdgpu_vm_set_pasid call and the cleanup path in the error
> handler to properly satisfy the lock assertion.
>
> Without these locks, we hit lockdep warnings or potential race conditions
> when modifying the VM's PASID mapping, as the underlying VM state changes
> must be protected by the root PD's reservation lock.
>
> Fixes: ebe038089be("drm/amdgpu: revert to old status lock handling v3")
>
> Signed-off-by: Jesse Zhang <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index 8676400834fc..e411dccbe612 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -1386,6 +1386,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev,
> struct drm_file *file_priv)
> {
> struct amdgpu_device *adev = drm_to_adev(dev);
> struct amdgpu_fpriv *fpriv;
> + struct dma_resv *resv;
> int r, pasid;
>
> /* Ensure IB tests are run on ring */
> @@ -1425,9 +1426,14 @@ int amdgpu_driver_open_kms(struct drm_device *dev,
> struct drm_file *file_priv)
> if (r)
> goto error_pasid;
>
> + resv = fpriv->vm.root.bo->tbo.base.resv;
> + dma_resv_lock(resv, NULL);
> r = amdgpu_vm_set_pasid(adev, &fpriv->vm, pasid);
> - if (r)
> + if (r) {
> + dma_resv_unlock(resv);
> goto error_vm;
> + }
> + dma_resv_unlock(resv);
>
> fpriv->prt_va = amdgpu_vm_bo_add(adev, &fpriv->vm, NULL);
> if (!fpriv->prt_va) {
> @@ -1470,7 +1476,9 @@ int amdgpu_driver_open_kms(struct drm_device *dev,
> struct drm_file *file_priv)
> error_pasid:
> if (pasid) {
> amdgpu_pasid_free(pasid);
> + dma_resv_lock(resv, NULL);
> amdgpu_vm_set_pasid(adev, &fpriv->vm, 0);
> + dma_resv_unlock(resv);
> }
>
> kfree(fpriv);
