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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 03.08.2016 um 11:16 schrieb 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?

Either WARN_ON() or BUG_ON().

BUG_ON() stops any current processing because we run into such a fatal issue that continuing would cause more damage than just stopping the current process and waiting forever.

So for this case WARN_ON() sounds more appropriate.

Regards,
Christian.


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

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux