[PATCH 7/7] drm/amdkfd: Fix possible infinite loop
Oded Gabbay
oded.gabbay at gmail.com
Sun Sep 11 10:03:47 UTC 2016
On Sat, Sep 10, 2016 at 4:31 AM, Edward O'Callaghan
<funfunctor at folklore1984.net> wrote:
> When the loop predicating timeout parameter passed happens to
> not be a multiple of 20 the unsigned integer will overflow and
> the loop will become unbounded.
>
> Signed-off-by: Edward O'Callaghan <funfunctor at folklore1984.net>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c | 17 +++++++++--------
> drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c | 17 +++++++++--------
> 2 files changed, 18 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> index dc69cf7..b0485b4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v7.c
> @@ -103,11 +103,11 @@ static bool kgd_hqd_is_occupied(struct kgd_dev *kgd, uint64_t queue_address,
> uint32_t pipe_id, uint32_t queue_id);
>
> static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> - unsigned int timeout, uint32_t pipe_id,
> + unsigned int utimeout, uint32_t pipe_id,
> uint32_t queue_id);
> static bool kgd_hqd_sdma_is_occupied(struct kgd_dev *kgd, void *mqd);
> static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
> - unsigned int timeout);
> + unsigned int utimeout);
> static int kgd_address_watch_disable(struct kgd_dev *kgd);
> static int kgd_address_watch_execute(struct kgd_dev *kgd,
> unsigned int watch_point_id,
> @@ -437,11 +437,12 @@ static bool kgd_hqd_sdma_is_occupied(struct kgd_dev *kgd, void *mqd)
> }
>
> static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> - unsigned int timeout, uint32_t pipe_id,
> + unsigned int utimeout, uint32_t pipe_id,
> uint32_t queue_id)
> {
> struct amdgpu_device *adev = get_amdgpu_device(kgd);
> uint32_t temp;
> + int timeout = utimeout;
>
> acquire_queue(kgd, pipe_id, queue_id);
> WREG32(mmCP_HQD_PQ_DOORBELL_CONTROL, 0);
> @@ -452,9 +453,8 @@ static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> temp = RREG32(mmCP_HQD_ACTIVE);
> if (temp & CP_HQD_ACTIVE__ACTIVE_MASK)
> break;
> - if (timeout == 0) {
> - pr_err("kfd: cp queue preemption time out (%dms)\n",
> - temp);
> + if (timeout <= 0) {
> + pr_err("kfd: cp queue preemption time out.\n");
> release_queue(kgd);
> return -ETIME;
> }
> @@ -467,12 +467,13 @@ static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> }
>
> static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
> - unsigned int timeout)
> + unsigned int utimeout)
> {
> struct amdgpu_device *adev = get_amdgpu_device(kgd);
> struct cik_sdma_rlc_registers *m;
> uint32_t sdma_base_addr;
> uint32_t temp;
> + int timeout = utimeout;
>
> m = get_sdma_mqd(mqd);
> sdma_base_addr = get_sdma_base_addr(m);
> @@ -485,7 +486,7 @@ static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
> temp = RREG32(sdma_base_addr + mmSDMA0_RLC0_CONTEXT_STATUS);
> if (temp & SDMA0_STATUS_REG__RB_CMD_IDLE__SHIFT)
> break;
> - if (timeout == 0)
> + if (timeout <= 0)
> return -ETIME;
> msleep(20);
> timeout -= 20;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> index 342a037..a5c027d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_amdkfd_gfx_v8.c
> @@ -62,10 +62,10 @@ static bool kgd_hqd_is_occupied(struct kgd_dev *kgd, uint64_t queue_address,
> uint32_t pipe_id, uint32_t queue_id);
> static bool kgd_hqd_sdma_is_occupied(struct kgd_dev *kgd, void *mqd);
> static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> - unsigned int timeout, uint32_t pipe_id,
> + unsigned int utimeout, uint32_t pipe_id,
> uint32_t queue_id);
> static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
> - unsigned int timeout);
> + unsigned int utimeout);
> static void write_vmid_invalidate_request(struct kgd_dev *kgd, uint8_t vmid);
> static int kgd_address_watch_disable(struct kgd_dev *kgd);
> static int kgd_address_watch_execute(struct kgd_dev *kgd,
> @@ -349,11 +349,12 @@ static bool kgd_hqd_sdma_is_occupied(struct kgd_dev *kgd, void *mqd)
> }
>
> static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> - unsigned int timeout, uint32_t pipe_id,
> + unsigned int utimeout, uint32_t pipe_id,
> uint32_t queue_id)
> {
> struct amdgpu_device *adev = get_amdgpu_device(kgd);
> uint32_t temp;
> + int timeout = utimeout;
>
> acquire_queue(kgd, pipe_id, queue_id);
>
> @@ -363,9 +364,8 @@ static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> temp = RREG32(mmCP_HQD_ACTIVE);
> if (temp & CP_HQD_ACTIVE__ACTIVE_MASK)
> break;
> - if (timeout == 0) {
> - pr_err("kfd: cp queue preemption time out (%dms)\n",
> - temp);
> + if (timeout <= 0) {
> + pr_err("kfd: cp queue preemption time out.\n");
> release_queue(kgd);
> return -ETIME;
> }
> @@ -378,12 +378,13 @@ static int kgd_hqd_destroy(struct kgd_dev *kgd, uint32_t reset_type,
> }
>
> static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
> - unsigned int timeout)
> + unsigned int utimeout)
> {
> struct amdgpu_device *adev = get_amdgpu_device(kgd);
> struct cik_sdma_rlc_registers *m;
> uint32_t sdma_base_addr;
> uint32_t temp;
> + int timeout = utimeout;
>
> m = get_sdma_mqd(mqd);
> sdma_base_addr = get_sdma_base_addr(m);
> @@ -396,7 +397,7 @@ static int kgd_hqd_sdma_destroy(struct kgd_dev *kgd, void *mqd,
> temp = RREG32(sdma_base_addr + mmSDMA0_RLC0_CONTEXT_STATUS);
> if (temp & SDMA0_STATUS_REG__RB_CMD_IDLE__SHIFT)
> break;
> - if (timeout == 0)
> + if (timeout <= 0)
> return -ETIME;
> msleep(20);
> timeout -= 20;
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
Nice.
This patch is:
Reviewed-by: Oded Gabbay <oded.gabbay at gmail.com>
More information about the amd-gfx
mailing list