RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[Public]
> -Original Message-
> From: Wang, Yang(Kevin)
> Sent: Tuesday, April 30, 2024 12:14 PM
> To: Huang, Tim ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
>
> Subject: RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
>
> [Public]
>
> -Original Message-
> From: amd-gfx On Behalf Of Huang,
> Tim
> Sent: Tuesday, April 30, 2024 11:32 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
>
> Subject: RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
>
> [Public]
>
> [Public]
>
> Ping ...
> > -Original Message-
> > From: Huang, Tim
> > Sent: Friday, April 26, 2024 9:14 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: Deucher, Alexander ; Koenig, Christian
> > ; Huang, Tim
> > Subject: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
> >
> > Clear warning that field bp is uninitialized when calling
> > amdgpu_virt_ras_add_bps.
> >
> > Signed-off-by: Tim Huang
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > index 54ab51a4ada7..a2f15edfe812 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> > @@ -395,6 +395,8 @@ static void amdgpu_virt_add_bad_page(struct
> > amdgpu_device *adev,
> > else
> > vram_usage_va = adev->mman.drv_vram_usage_va;
> >
> > + memset(&bp, 0, sizeof(struct eeprom_table_record));
> [Kevin]:
>
> It is better to change code to "sizeof (bp)".
Yes, agree, will change to this. Thanks.
Tim
>
> Reviewed-by: Yang Wang
>
> Best Regards,
> Kevin
> > +
> > if (bp_block_size) {
> > bp_cnt = bp_block_size / sizeof(uint64_t);
> > for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> > --
> > 2.39.2
>
RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[Public]
-Original Message-
From: amd-gfx On Behalf Of Huang, Tim
Sent: Tuesday, April 30, 2024 11:32 AM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Koenig, Christian
Subject: RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[Public]
[Public]
Ping ...
> -Original Message-
> From: Huang, Tim
> Sent: Friday, April 26, 2024 9:14 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Huang, Tim
> Subject: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
>
> Clear warning that field bp is uninitialized when calling
> amdgpu_virt_ras_add_bps.
>
> Signed-off-by: Tim Huang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 54ab51a4ada7..a2f15edfe812 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -395,6 +395,8 @@ static void amdgpu_virt_add_bad_page(struct
> amdgpu_device *adev,
> else
> vram_usage_va = adev->mman.drv_vram_usage_va;
>
> + memset(&bp, 0, sizeof(struct eeprom_table_record));
[Kevin]:
It is better to change code to "sizeof (bp)".
Reviewed-by: Yang Wang
Best Regards,
Kevin
> +
> if (bp_block_size) {
> bp_cnt = bp_block_size / sizeof(uint64_t);
> for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> --
> 2.39.2
RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[Public]
Ping ...
> -Original Message-
> From: Huang, Tim
> Sent: Friday, April 26, 2024 9:14 AM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Huang, Tim
> Subject: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
>
> Clear warning that field bp is uninitialized when calling
> amdgpu_virt_ras_add_bps.
>
> Signed-off-by: Tim Huang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> index 54ab51a4ada7..a2f15edfe812 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_virt.c
> @@ -395,6 +395,8 @@ static void amdgpu_virt_add_bad_page(struct
> amdgpu_device *adev,
> else
> vram_usage_va = adev->mman.drv_vram_usage_va;
>
> + memset(&bp, 0, sizeof(struct eeprom_table_record));
> +
> if (bp_block_size) {
> bp_cnt = bp_block_size / sizeof(uint64_t);
> for (bp_idx = 0; bp_idx < bp_cnt; bp_idx++) {
> --
> 2.39.2
Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
Am 23.04.24 um 10:12 schrieb Huang, Tim:
[AMD Official Use Only - General]
-Original Message-
From: amd-gfx On Behalf Of Huang, Tim
Sent: Tuesday, April 23, 2024 4:01 PM
To: Koenig, Christian ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[AMD Official Use Only - General]
[AMD Official Use Only - General]
Hi Christian,
-Original Message-
From: Koenig, Christian
Sent: Tuesday, April 23, 2024 3:43 PM
To: Huang, Tim ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Koenig, Christian
Subject: Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
Am 23.04.24 um 08:28 schrieb Tim Huang:
Clear warning that uses uninitialized value fw_size.
In which case is the fw_size uninitialized and why setting it to zero helps in
that case?
It's a warning that reported by the Coverity scan. When the switch case " switch (ucode_id) " got to
default and Condition "adev->firmware.load_type == AMDGPU_FW_LOAD_PSP", taking true branch, it
reports " uses uninitialized value fw_size " by this line.
"adev->firmware.fw_size += ALIGN(fw_size, PAGE_SIZE);“
It may not happen if we call this function correctly, but it just clears the
warning and looks harmless.
Hi Christian,
I think it more to fix this warning, maybe I need to print an error and just return when
go to the default case of "switch (ucode_id)" , will send out a v2 patch.
Thanks.
Yeah, exactly that's the right idea.
Regards,
Christian.
Regards,
Christian.
Signed-off-by: Tim Huang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index d9dc5485..6b8a58f501d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -1084,7 +1084,7 @@ void amdgpu_gfx_cp_init_microcode(struct amdgpu_device
*adev,
const struct gfx_firmware_header_v2_0 *cp_hdr_v2_0;
struct amdgpu_firmware_info *info = NULL;
const struct firmware *ucode_fw;
- unsigned int fw_size;
+ unsigned int fw_size = 0;
switch (ucode_id) {
case AMDGPU_UCODE_ID_CP_PFP:
Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
The problem is that it's a hit all case and that's usually seen as bad
coding style.
In other words when one branch by accident forgets to set the fw_size we
wouldn't get a warning any more and just use zero.
Please rather add setting the fw_size to zero to the default branch and
maybe even add a warning when that happens.
Regards,
Christian.
Am 23.04.24 um 10:01 schrieb Huang, Tim:
[AMD Official Use Only - General]
Hi Christian,
-Original Message-
From: Koenig, Christian
Sent: Tuesday, April 23, 2024 3:43 PM
To: Huang, Tim ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Koenig, Christian
Subject: Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
Am 23.04.24 um 08:28 schrieb Tim Huang:
Clear warning that uses uninitialized value fw_size.
In which case is the fw_size uninitialized and why setting it to zero helps in
that case?
It's a warning that reported by the Coverity scan. When the switch case " switch (ucode_id)
" got to default and Condition "adev->firmware.load_type == AMDGPU_FW_LOAD_PSP",
taking true branch,
it reports " uses uninitialized value fw_size " by this line.
"adev->firmware.fw_size += ALIGN(fw_size, PAGE_SIZE);“
It may not happen if we call this function correctly, but it just clears the
warning and looks harmless.
Regards,
Christian.
Signed-off-by: Tim Huang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index d9dc5485..6b8a58f501d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -1084,7 +1084,7 @@ void amdgpu_gfx_cp_init_microcode(struct amdgpu_device
*adev,
const struct gfx_firmware_header_v2_0 *cp_hdr_v2_0;
struct amdgpu_firmware_info *info = NULL;
const struct firmware *ucode_fw;
- unsigned int fw_size;
+ unsigned int fw_size = 0;
switch (ucode_id) {
case AMDGPU_UCODE_ID_CP_PFP:
RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[AMD Official Use Only - General]
-Original Message-
From: amd-gfx On Behalf Of Huang, Tim
Sent: Tuesday, April 23, 2024 4:01 PM
To: Koenig, Christian ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander
Subject: RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[AMD Official Use Only - General]
[AMD Official Use Only - General]
Hi Christian,
-Original Message-
From: Koenig, Christian
Sent: Tuesday, April 23, 2024 3:43 PM
To: Huang, Tim ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Koenig, Christian
Subject: Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
Am 23.04.24 um 08:28 schrieb Tim Huang:
> Clear warning that uses uninitialized value fw_size.
> In which case is the fw_size uninitialized and why setting it to zero helps
> in that case?
> It's a warning that reported by the Coverity scan. When the switch case "
> switch (ucode_id) " got to default and Condition "adev->firmware.load_type ==
> AMDGPU_FW_LOAD_PSP", taking true branch, it reports " uses uninitialized
> value fw_size " by this line.
> "adev->firmware.fw_size += ALIGN(fw_size, PAGE_SIZE);“
> It may not happen if we call this function correctly, but it just clears the
> warning and looks harmless.
Hi Christian,
I think it more to fix this warning, maybe I need to print an error and just
return when go to the default case of "switch (ucode_id)" , will send out a v2
patch. Thanks.
> Regards,
> Christian.
>
> Signed-off-by: Tim Huang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index d9dc5485..6b8a58f501d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1084,7 +1084,7 @@ void amdgpu_gfx_cp_init_microcode(struct amdgpu_device
> *adev,
> const struct gfx_firmware_header_v2_0 *cp_hdr_v2_0;
> struct amdgpu_firmware_info *info = NULL;
> const struct firmware *ucode_fw;
> - unsigned int fw_size;
> + unsigned int fw_size = 0;
>
> switch (ucode_id) {
> case AMDGPU_UCODE_ID_CP_PFP:
RE: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
[AMD Official Use Only - General]
Hi Christian,
-Original Message-
From: Koenig, Christian
Sent: Tuesday, April 23, 2024 3:43 PM
To: Huang, Tim ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Koenig, Christian
Subject: Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
Am 23.04.24 um 08:28 schrieb Tim Huang:
> Clear warning that uses uninitialized value fw_size.
> In which case is the fw_size uninitialized and why setting it to zero helps
> in that case?
It's a warning that reported by the Coverity scan. When the switch case "
switch (ucode_id) " got to default and Condition "adev->firmware.load_type ==
AMDGPU_FW_LOAD_PSP", taking true branch,
it reports " uses uninitialized value fw_size " by this line.
"adev->firmware.fw_size += ALIGN(fw_size, PAGE_SIZE);“
It may not happen if we call this function correctly, but it just clears the
warning and looks harmless.
Regards,
Christian.
>
> Signed-off-by: Tim Huang
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index d9dc5485..6b8a58f501d3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -1084,7 +1084,7 @@ void amdgpu_gfx_cp_init_microcode(struct amdgpu_device
> *adev,
> const struct gfx_firmware_header_v2_0 *cp_hdr_v2_0;
> struct amdgpu_firmware_info *info = NULL;
> const struct firmware *ucode_fw;
> - unsigned int fw_size;
> + unsigned int fw_size = 0;
>
> switch (ucode_id) {
> case AMDGPU_UCODE_ID_CP_PFP:
Re: [PATCH] drm/amdgpu: fix uninitialized scalar variable warning
Am 23.04.24 um 08:28 schrieb Tim Huang:
Clear warning that uses uninitialized value fw_size.
In which case is the fw_size uninitialized and why setting it to zero
helps in that case?
Regards,
Christian.
Signed-off-by: Tim Huang
---
drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
index d9dc5485..6b8a58f501d3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
@@ -1084,7 +1084,7 @@ void amdgpu_gfx_cp_init_microcode(struct amdgpu_device
*adev,
const struct gfx_firmware_header_v2_0 *cp_hdr_v2_0;
struct amdgpu_firmware_info *info = NULL;
const struct firmware *ucode_fw;
- unsigned int fw_size;
+ unsigned int fw_size = 0;
switch (ucode_id) {
case AMDGPU_UCODE_ID_CP_PFP:
