[bug report] drm/amdgpu/gfx7: set USER_SHADER_ARRAY_CONFIG based on disable_cu parameter

[Nicolai Hähnle]

On 03.08.2016 11:09, Dan Carpenter wrote:
> Hello Nicolai Hähnle,
>
> The patch 324c614a819a: "drm/amdgpu/gfx7: set
> USER_SHADER_ARRAY_CONFIG based on disable_cu parameter" from Jun 17,
> 2016, leads to the following static checker warning:
>
> 	drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c:5057 gfx_v7_0_get_cu_info()
> 	error: buffer overflow 'cu_info->bitmap' 4 <= 4
>
> drivers/gpu/drm/amd/amdgpu/gfx_v7_0.c
>   5035  static void gfx_v7_0_get_cu_info(struct amdgpu_device *adev)
>   5036  {
>   5037          int i, j, k, counter, active_cu_number = 0;
>   5038          u32 mask, bitmap, ao_bitmap, ao_cu_mask = 0;
>   5039          struct amdgpu_cu_info *cu_info = &adev->gfx.cu_info;
>   5040          unsigned disable_masks[4 * 2];
>   5041
>   5042          memset(cu_info, 0, sizeof(*cu_info));
>   5043
>   5044          amdgpu_gfx_parse_disable_cu(disable_masks, 4, 2);
>   5045
>   5046          mutex_lock(&adev->grbm_idx_mutex);
>   5047          for (i = 0; i < adev->gfx.config.max_shader_engines; i++) {
>   5048                  for (j = 0; j < adev->gfx.config.max_sh_per_se; j++) {
>   5049                          mask = 1;
>   5050                          ao_bitmap = 0;
>   5051                          counter = 0;
>   5052                          gfx_v7_0_select_se_sh(adev, i, j, 0xffffffff);
>   5053                          if (i < 4 && j < 2)
>                                     ^^^^^
> Is it really possible for i to be >= 4?

No, because for that to happen we would have to add support for hardware 
with > 4 shader engines. What's the idiomatic way to express this kind 
of assumption in the kernel? BUG_ON(adev->gfx.config.max_shader_engines 
 > 4)? Some other form of assert?

Nicolai

>
>   5054                                  gfx_v7_0_set_user_cu_inactive_bitmap(
>   5055                                          adev, disable_masks[i * 2 + j]);
>   5056                          bitmap = gfx_v7_0_get_cu_active_bitmap(adev);
>   5057                          cu_info->bitmap[i][j] = bitmap;
>                                 ^^^^^^^^^^^^^^^^^^^^^
> Because if so, then we are screwed here.
>
>   5058
>   5059                          for (k = 0; k < 16; k ++) {
>   5060                                  if (bitmap & mask) {
>   5061                                          if (counter < 2)
>   5062                                                  ao_bitmap |= mask;
>   5063                                          counter ++;
>   5064                                  }
>   5065                                  mask <<= 1;
>   5066                          }
>   5067                          active_cu_number += counter;
>   5068                          ao_cu_mask |= (ao_bitmap << (i * 16 + j * 8));
>   5069                  }
>   5070          }
>   5071          gfx_v7_0_select_se_sh(adev, 0xffffffff, 0xffffffff, 0xffffffff);
>   5072          mutex_unlock(&adev->grbm_idx_mutex);
>   5073
>   5074          cu_info->number = active_cu_number;
>   5075          cu_info->ao_cu_mask = ao_cu_mask;
>   5076  }
>
> regards,
> dan carpenter
>