Re: [PATCH] drm/amdgpu: Initialize SPM_VMID with 0xf

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

Am 27.02.20 um 14:25 schrieb Jacob He:
> SPM_VMID is a global resource, SPM access the video memory according to
> SPM_VMID. The initial valude of SPM_VMID is 0 which is used by kernel.
> That means UMD can overwrite the memory of VMID0 by enabling SPM, that
> is really dangerous.
>
> Initialize SPM_VMID with 0xf, it messes up other user mode process at
> most.
>
> Change-Id: Ieb54f6a16869b827504355a90a98663908d5087c
> Signed-off-by: Jacob He <jacob.he@xxxxxxx>

Reviewed-by: Christian König <christian.koenig@xxxxxxx>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h |  1 +
>   drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c  | 19 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c   | 19 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c   | 19 ++++++++++++++++++-
>   drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c   | 19 ++++++++++++++++++-
>   5 files changed, 73 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
> index d3d4707f2168..52509c254cbd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_rlc.h
> @@ -126,6 +126,7 @@ struct amdgpu_rlc_funcs {
>   	void (*stop)(struct amdgpu_device *adev);
>   	void (*reset)(struct amdgpu_device *adev);
>   	void (*start)(struct amdgpu_device *adev);
> +	void (*update_spm_vmid)(struct amdgpu_device *adev, unsigned vmid);
>   };
>   
>   struct amdgpu_rlc {
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> index 7b6158320400..9fe5bdfa87ae 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v10_0.c
> @@ -1016,6 +1016,10 @@ static int gfx_v10_0_rlc_init(struct amdgpu_device *adev)
>   			return r;
>   	}
>   
> +    /* init spm vmid with 0xf */
> +    if (adev->gfx.rlc.funcs->update_spm_vmid)
> +        adev->gfx.rlc.funcs->update_spm_vmid(adev, 0xf);
> +
>   	return 0;
>   }
>   
> @@ -4214,6 +4218,18 @@ static int gfx_v10_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> +static void gfx_v10_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
> +{
> +	u32 data;
> +
> +	data = RREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL);
> +
> +	data &= ~RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK;
> +	data |= (vmid & RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK) << RLC_SPM_MC_CNTL__RLC_SPM_VMID__SHIFT;
> +
> +	WREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL, data);
> +}
> +
>   static const struct amdgpu_rlc_funcs gfx_v10_0_rlc_funcs = {
>   	.is_rlc_enabled = gfx_v10_0_is_rlc_enabled,
>   	.set_safe_mode = gfx_v10_0_set_safe_mode,
> @@ -4224,7 +4240,8 @@ static const struct amdgpu_rlc_funcs gfx_v10_0_rlc_funcs = {
>   	.resume = gfx_v10_0_rlc_resume,
>   	.stop = gfx_v10_0_rlc_stop,
>   	.reset = gfx_v10_0_rlc_reset,
> -	.start = gfx_v10_0_rlc_start
> +	.start = gfx_v10_0_rlc_start,
> +	.update_spm_vmid = gfx_v10_0_update_spm_vmid
>   };
>   
>   static int gfx_v10_0_set_powergating_state(void *handle,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> index 8f20a5dd44fe..80a959755cf3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
> @@ -3346,6 +3346,10 @@ static int gfx_v7_0_rlc_init(struct amdgpu_device *adev)
>   			return r;
>   	}
>   
> +    /* init spm vmid with 0xf */
> +    if (adev->gfx.rlc.funcs->update_spm_vmid)
> +        adev->gfx.rlc.funcs->update_spm_vmid(adev, 0xf);
> +
>   	return 0;
>   }
>   
> @@ -3570,6 +3574,18 @@ static int gfx_v7_0_rlc_resume(struct amdgpu_device *adev)
>   	return 0;
>   }
>   
> +static void gfx_v7_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
> +{
> +	u32 data;
> +
> +	data = RREG32(mmRLC_SPM_VMID);
> +
> +	data &= ~RLC_SPM_VMID__RLC_SPM_VMID_MASK;
> +	data |= (vmid & RLC_SPM_VMID__RLC_SPM_VMID_MASK) << RLC_SPM_VMID__RLC_SPM_VMID__SHIFT;
> +
> +	WREG32(mmRLC_SPM_VMID, data);
> +}
> +
>   static void gfx_v7_0_enable_cgcg(struct amdgpu_device *adev, bool enable)
>   {
>   	u32 data, orig, tmp, tmp2;
> @@ -4221,7 +4237,8 @@ static const struct amdgpu_rlc_funcs gfx_v7_0_rlc_funcs = {
>   	.resume = gfx_v7_0_rlc_resume,
>   	.stop = gfx_v7_0_rlc_stop,
>   	.reset = gfx_v7_0_rlc_reset,
> -	.start = gfx_v7_0_rlc_start
> +	.start = gfx_v7_0_rlc_start,
> +	.update_spm_vmid = gfx_v7_0_update_spm_vmid
>   };
>   
>   static int gfx_v7_0_early_init(void *handle)
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index fa245973de12..2174f9dc5335 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -1318,6 +1318,10 @@ static int gfx_v8_0_rlc_init(struct amdgpu_device *adev)
>   			return r;
>   	}
>   
> +    /* init spm vmid with 0xf */
> +    if (adev->gfx.rlc.funcs->update_spm_vmid)
> +        adev->gfx.rlc.funcs->update_spm_vmid(adev, 0xf);
> +
>   	return 0;
>   }
>   
> @@ -5589,6 +5593,18 @@ static void gfx_v8_0_unset_safe_mode(struct amdgpu_device *adev)
>   	}
>   }
>   
> +static void gfx_v8_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
> +{
> +	u32 data;
> +
> +	data = RREG32(mmRLC_SPM_VMID);
> +
> +	data &= ~RLC_SPM_VMID__RLC_SPM_VMID_MASK;
> +	data |= (vmid & RLC_SPM_VMID__RLC_SPM_VMID_MASK) << RLC_SPM_VMID__RLC_SPM_VMID__SHIFT;
> +
> +	WREG32(mmRLC_SPM_VMID, data);
> +}
> +
>   static const struct amdgpu_rlc_funcs iceland_rlc_funcs = {
>   	.is_rlc_enabled = gfx_v8_0_is_rlc_enabled,
>   	.set_safe_mode = gfx_v8_0_set_safe_mode,
> @@ -5600,7 +5616,8 @@ static const struct amdgpu_rlc_funcs iceland_rlc_funcs = {
>   	.resume = gfx_v8_0_rlc_resume,
>   	.stop = gfx_v8_0_rlc_stop,
>   	.reset = gfx_v8_0_rlc_reset,
> -	.start = gfx_v8_0_rlc_start
> +	.start = gfx_v8_0_rlc_start,
> +	.update_spm_vmid = gfx_v8_0_update_spm_vmid
>   };
>   
>   static void gfx_v8_0_update_medium_grain_clock_gating(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 1c7a16b91686..db388383faa8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1847,6 +1847,10 @@ static int gfx_v9_0_rlc_init(struct amdgpu_device *adev)
>   		break;
>   	}
>   
> +    /* init spm vmid with 0xf */
> +    if (adev->gfx.rlc.funcs->update_spm_vmid)
> +        adev->gfx.rlc.funcs->update_spm_vmid(adev, 0xf);
> +
>   	return 0;
>   }
>   
> @@ -4705,6 +4709,18 @@ static int gfx_v9_0_update_gfx_clock_gating(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> +static void gfx_v9_0_update_spm_vmid(struct amdgpu_device *adev, unsigned vmid)
> +{
> +	u32 data;
> +
> +	data = RREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL);
> +
> +	data &= ~RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK;
> +	data |= (vmid & RLC_SPM_MC_CNTL__RLC_SPM_VMID_MASK) << RLC_SPM_MC_CNTL__RLC_SPM_VMID__SHIFT;
> +
> +	WREG32_SOC15(GC, 0, mmRLC_SPM_MC_CNTL, data);
> +}
> +
>   static const struct amdgpu_rlc_funcs gfx_v9_0_rlc_funcs = {
>   	.is_rlc_enabled = gfx_v9_0_is_rlc_enabled,
>   	.set_safe_mode = gfx_v9_0_set_safe_mode,
> @@ -4716,7 +4732,8 @@ static const struct amdgpu_rlc_funcs gfx_v9_0_rlc_funcs = {
>   	.resume = gfx_v9_0_rlc_resume,
>   	.stop = gfx_v9_0_rlc_stop,
>   	.reset = gfx_v9_0_rlc_reset,
> -	.start = gfx_v9_0_rlc_start
> +	.start = gfx_v9_0_rlc_start,
> +	.update_spm_vmid = gfx_v9_0_update_spm_vmid
>   };
>   
>   static int gfx_v9_0_set_powergating_state(void *handle,

_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx