Hi Dan Carpenter,
Thank you for the info. This commit is just a clean up to keep align
with gfx7/8.
On Fri, Mar 31, 2017 at 06:13:25PM +0300, Dan Carpenter wrote:
> Hello Flora Cui,
>
> The patch 375d6f7057a9: "drm/amdgpu/gfx6: clean up cu configuration"
> from Feb 7, 2017, leads to the following static checker warning:
>
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c:3737 gfx_v6_0_get_cu_info()
> warn: potential off by one cu_info->bitmap[4]
>
> drivers/gpu/drm/amd/amdgpu/gfx_v6_0.c
> 3715 static void gfx_v6_0_get_cu_info(struct amdgpu_device *adev)
> 3716 {
> 3717 int i, j, k, counter, active_cu_number = 0;
> 3718 u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
> 3719 struct amdgpu_cu_info *cu_info = &adev->gfx.cu_info;
> 3720 unsigned disable_masks[4 * 2];
> 3721
> 3722 memset(cu_info, 0, sizeof(*cu_info));
> 3723
> 3724 amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2);
> 3725
> 3726 mutex_lock(&adev->grbm_idx_mutex);
> 3727 for (i = 0; i < adev->gfx.config.max_shader_engines; i++) {
> 3728 for (j = 0; j < adev->gfx.config.max_sh_per_se; j++) {
> 3729 mask = 1;
> 3730 ao_bitmap = 0;
> 3731 counter = 0;
> 3732 gfx_v6_0_select_se_sh(adev, i, j, 0xffffffff);
> 3733 if (i < 4 && j < 2)
>
> If i == 4
>
> 3734 gfx_v6_0_set_user_cu_inactive_bitmap(
> 3735 adev, disable_masks[i * 2 + j]);
> 3736 bitmap = gfx_v6_0_get_cu_enabled(adev);
> 3737 cu_info->bitmap[i][j] = bitmap;
>
> then we are beyond the end of this array. Also, why was this patch even
> applied when it has no commit message? It's totally not clear to me
> what the patch is trying to do or why it exists...
adev->gfx.config.max_shader_engines is set in gfx_v6_0_gpu_init() and
it must be < 4 so we'll never be beyond the end of the array.
Regards,
Flora Cui
> 3738
> 3739 for (k = 0; k < 16; k++) {
> 3740 if (bitmap & mask) {
> 3741 if (counter < 2)
> 3742 ao_bitmap |= mask;
> 3743 counter ++;
> 3744 }
> 3745 mask <<= 1;
> 3746 }
> 3747 active_cu_number += counter;
> 3748 ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
> 3749 }
> 3750 }
> 3751
> 3752 gfx_v6_0_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
> 3753 mutex_unlock(&adev->grbm_idx_mutex);
> 3754
> 3755 cu_info->number = active_cu_number;
> 3756 cu_info->ao_cu_mask = ao_cu_mask;
> 3757 }
>
> regards,
> dan carpenter
