Re: [PATCH 1/2] drm/amdgpu: Fix MMIO access page fault

[James Zhu <jamesz@xxxxxxx>]

Hi Andrey

Can you apply this improvement  on vcn_v3_0_sw_init also?

With this adding, This patch is Reviewed-by: James Zhu <James.Zhu@xxxxxxx>

Thanks & Best Regards!

James

On 2021-09-17 7:30 a.m., Andrey Grodzovsky wrote:
> Add more guards to MMIO access post device
> unbind/unplug
>
> Bug:https://bugs.archlinux.org/task/72092?project=1&order=dateopened&sort=desc&pagenum=1
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c |  8 ++++++--
>   drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c | 17 +++++++++++------
>   2 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index e6e9ef50719e..a03c0fc8338f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -22,6 +22,7 @@
>    */
>   
>   #include <linux/firmware.h>
> +#include <drm/drm_drv.h>
>   
>   #include "amdgpu.h"
>   #include "amdgpu_vcn.h"
> @@ -194,11 +195,14 @@ static int vcn_v2_0_sw_init(void *handle)
>    */
>   static int vcn_v2_0_sw_fini(void *handle)
>   {
> -	int r;
> +	int r, idx;
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   	volatile struct amdgpu_fw_shared *fw_shared = adev->vcn.inst->fw_shared_cpu_addr;
>   
> -	fw_shared->present_flag_0 = 0;
> +	if (drm_dev_enter(&adev->ddev, &idx)) {
> +		fw_shared->present_flag_0 = 0;
> +		drm_dev_exit(idx);
> +	}
>   
>   	amdgpu_virt_free_mm_table(adev);
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> index 2e6b7913bf6c..1780ad1eacd6 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c
> @@ -22,6 +22,7 @@
>    */
>   
>   #include <linux/firmware.h>
> +#include <drm/drm_drv.h>
>   
>   #include "amdgpu.h"
>   #include "amdgpu_vcn.h"
> @@ -235,17 +236,21 @@ static int vcn_v2_5_sw_init(void *handle)
>    */
>   static int vcn_v2_5_sw_fini(void *handle)
>   {
> -	int i, r;
> +	int i, r, idx;
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   	volatile struct amdgpu_fw_shared *fw_shared;
>   
> -	for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> -		if (adev->vcn.harvest_config & (1 << i))
> -			continue;
> -		fw_shared = adev->vcn.inst[i].fw_shared_cpu_addr;
> -		fw_shared->present_flag_0 = 0;
> +	if (drm_dev_enter(&adev->ddev, &idx)) {
> +		for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> +			if (adev->vcn.harvest_config & (1 << i))
> +				continue;
> +			fw_shared = adev->vcn.inst[i].fw_shared_cpu_addr;
> +			fw_shared->present_flag_0 = 0;
> +		}
> +		drm_dev_exit(idx);
>   	}
>   
> +
>   	if (amdgpu_sriov_vf(adev))
>   		amdgpu_virt_free_mm_table(adev);
>