* [bug report] drm/amdgpu/gfx6: clean up cu configuration
@ 2017-03-31 15:13 Dan Carpenter
2017-04-01 2:43 ` Flora Cui
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2017-03-31 15:13 UTC (permalink / raw)
To: Flora.Cui-5C7GfCeVMHo
Cc: Ken Wang, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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...
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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/amdgpu/gfx6: clean up cu configuration
2017-03-31 15:13 [bug report] drm/amdgpu/gfx6: clean up cu configuration Dan Carpenter
@ 2017-04-01 2:43 ` Flora Cui
2017-04-03 13:54 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Flora Cui @ 2017-04-01 2:43 UTC (permalink / raw)
To: Dan Carpenter
Cc: Ken Wang, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/amdgpu/gfx6: clean up cu configuration
2017-04-01 2:43 ` Flora Cui
@ 2017-04-03 13:54 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-04-03 13:54 UTC (permalink / raw)
To: Flora Cui; +Cc: Ken Wang, dri-devel, amd-gfx
On Sat, Apr 01, 2017 at 10:43:46AM +0800, Flora Cui wrote:
> > 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.
Could you remove the condition then? We should either check it always
or not at all. We shouldn't check it half the time and assume it's
correct the other half.
regards,
dan carpenter
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-04-03 13:54 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-31 15:13 [bug report] drm/amdgpu/gfx6: clean up cu configuration Dan Carpenter
2017-04-01 2:43 ` Flora Cui
2017-04-03 13:54 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.