Hi Vitaly,

On 10/31/26 01:02, [email protected] wrote:
> From: Vitaly Prosyak <[email protected]>
> 
> GPU recovery held reset_domain->sem while invalidating DRM and KFD mappings.
> unmap_mapping_range() takes mapping->i_mmap_rwsem, creating the reported
> lockdep dependency cycle through MMU notifier and dma_fence_map locking.

That is unfortunately not correct either, it just makes the lockdep complain go 
away but doesn't fix the underlying issue.

One of the main purposes of the reset handler is to unlock HW operations and 
guarantee that dma_fence objects signal.

So when the MMU notifier is called with mapping->i_mmap_rwsem held and it must 
be able to wait for a dma_fence we *CAN'T* call unmap_mapping_range() from the 
reset handler at all.

There has been some discussion to handle the requirement to not access the VRAM 
BAR during a reset in the BIF instead of doing the unmap here. MI* HW already 
seems to do exactly that.

So my suggestion is to completely remove unmap_mapping_range() and 
amdgpu_amdkfd_clear_kfd_mapping() here and push for this HW handling inside the 
BIF. That seems to be the only way out of this problem.

Regards,
Christian.

> 
> Invalidate both mapping sets before taking reset_domain->sem. Keep the
> semaphore held around the recovery operations that require reset
> serialization.
> 
> Cc: Christian König <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: Prike Liang <[email protected]>
> Signed-off-by: Vitaly Prosyak <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ef01d280cd0d..407bf3ff9b4b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5789,15 +5789,15 @@ int amdgpu_device_gpu_recover(struct amdgpu_device 
> *adev,
>       /* Cannot be called after locking reset domain */
>       amdgpu_ras_pre_reset(adev, &device_list);
>  
> -     /* We need to lock reset domain only once both for XGMI and single 
> device */
> -     amdgpu_device_recovery_get_reset_lock(adev, &device_list);
> -
>       /* unmap all the mappings of doorbell and framebuffer to prevent user 
> space from
>        * accessing them
>        */
>       unmap_mapping_range(adev->ddev.anon_inode->i_mapping, 0, 0, 1);
>       amdgpu_amdkfd_clear_kfd_mapping(adev);
>  
> +     /* We need to lock reset domain only once both for XGMI and single 
> device */
> +     amdgpu_device_recovery_get_reset_lock(adev, &device_list);
> +
>       amdgpu_device_halt_activities(adev, job, reset_context, &device_list,
>                                     hive, need_emergency_restart);
>       if (need_emergency_restart) {

Reply via email to