Re: [PATCH] drm/amdgpu: Add lock around VF RLCG interface

[Christian König <ckoenig.leichtzumerken@xxxxxxxxx>]

Am 27.05.24 um 22:19 schrieb Victor Skvortsov:
> flush_gpu_tlb may be called from another thread while
> device_gpu_recover is running.

No, that would be illegal. Where do you see that?

Regards,
Christian.

> Both of these threads access registers through the VF
> RLCG interface during VF Full Access. Add a lock around this interface
> to prevent race conditions between these threads.
>
> Signed-off-by: Victor Skvortsov <victor.skvortsov@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c   | 6 ++++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h   | 2 ++
>   3 files changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index f5168b4c3b03..6711836054f9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -4049,6 +4049,7 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	mutex_init(&adev->grbm_idx_mutex);
>   	mutex_init(&adev->mn_lock);
>   	mutex_init(&adev->virt.vf_errors.lock);
> +	mutex_init(&adev->virt.rlcg_reg_lock);
>   	hash_init(adev->mn_hash);
>   	mutex_init(&adev->psp.mutex);
>   	mutex_init(&adev->notifier_lock);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 3d5f58e76f2d..a72683f83390 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -982,6 +982,9 @@ u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device *adev, u32 offset, u32 v, u32 f
>   	scratch_reg1 = (void __iomem *)adev->rmmio + 4 * reg_access_ctrl->scratch_reg1;
>   	scratch_reg2 = (void __iomem *)adev->rmmio + 4 * reg_access_ctrl->scratch_reg2;
>   	scratch_reg3 = (void __iomem *)adev->rmmio + 4 * reg_access_ctrl->scratch_reg3;
> +
> +	mutex_lock(&adev->virt.rlcg_reg_lock);
> +
>   	if (reg_access_ctrl->spare_int)
>   		spare_int = (void __iomem *)adev->rmmio + 4 * reg_access_ctrl->spare_int;
>   
> @@ -1038,6 +1041,9 @@ u32 amdgpu_virt_rlcg_reg_rw(struct amdgpu_device *adev, u32 offset, u32 v, u32 f
>   	}
>   
>   	ret = readl(scratch_reg0);
> +
> +	mutex_unlock(&adev->virt.rlcg_reg_lock);
> +
>   	return ret;
>   }
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> index 642f1fd287d8..0ec246c74570 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.h
> @@ -272,6 +272,8 @@ struct amdgpu_virt {
>   
>   	/* the ucode id to signal the autoload */
>   	uint32_t autoload_ucode_id;
> +
> +	struct mutex rlcg_reg_lock;
>   };
>   
>   struct amdgpu_video_codec_info;