Re: [PATCH] drm/amd: fix some dead code in `gfx_v9_0_init_cp_compute_microcode`

[Alex Deucher]

On Thu, Jan 12, 2023 at 5:37 PM Mario Limonciello
 wrote:
>
> Some dead code was introdcued as part of utilizing the `amdgpu_ucode_*`
> helpers. Adjust the control flow to make sure that firmware is released
> in the appropriate error flows.
>
> Reported-by: coverity-bot 
> Addresses-Coverity-ID: 1530548 ("Control flow issues")
> Fixes: ec787deb2ddf ("drm/amd: Use `amdgpu_ucode_*` helpers for GFX9")
> Signed-off-by: Mario Limonciello 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 11 ++-
>  1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index e80685d1e6c6c..d4b0fa4b62a44 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1345,7 +1345,7 @@ static int gfx_v9_0_init_cp_compute_microcode(struct 
> amdgpu_device *adev,
>
> err = amdgpu_ucode_request(adev, >gfx.mec_fw, fw_name);
> if (err)
> -   return err;
> +   goto out;
> amdgpu_gfx_cp_init_microcode(adev, AMDGPU_UCODE_ID_CP_MEC1);
> amdgpu_gfx_cp_init_microcode(adev, AMDGPU_UCODE_ID_CP_MEC1_JT);
>
> @@ -1355,13 +1355,14 @@ static int gfx_v9_0_init_cp_compute_microcode(struct 
> amdgpu_device *adev,
> else
> snprintf(fw_name, sizeof(fw_name), 
> "amdgpu/%s_mec2.bin", chip_name);
>
> +   /* ignore failures to load */
> err = amdgpu_ucode_request(adev, >gfx.mec2_fw, fw_name);
> if (!err) {
> amdgpu_gfx_cp_init_microcode(adev, 
> AMDGPU_UCODE_ID_CP_MEC2);
> amdgpu_gfx_cp_init_microcode(adev, 
> AMDGPU_UCODE_ID_CP_MEC2_JT);
> } else {
> err = 0;
> -   adev->gfx.mec2_fw = NULL;
> +   amdgpu_ucode_release(>gfx.mec2_fw);
> }
> } else {
> adev->gfx.mec2_fw_version = adev->gfx.mec_fw_version;
> @@ -1370,10 +1371,10 @@ static int gfx_v9_0_init_cp_compute_microcode(struct 
> amdgpu_device *adev,
>
> gfx_v9_0_check_if_need_gfxoff(adev);
> gfx_v9_0_check_fw_write_wait(adev);
> -   if (err) {
> +
> +out:
> +   if (err)
> amdgpu_ucode_release(>gfx.mec_fw);
> -   amdgpu_ucode_release(>gfx.mec2_fw);
> -   }
> return err;
>  }
>
> --
> 2.34.1
>

Re: [PATCH] drm/amd: fix some dead code in `gfx_v9_0_init_cp_compute_microcode`

[Kees Cook]

On Thu, Jan 12, 2023 at 04:37:01PM -0600, Mario Limonciello wrote:
> Some dead code was introdcued as part of utilizing the `amdgpu_ucode_*`
> helpers. Adjust the control flow to make sure that firmware is released
> in the appropriate error flows.
> 
> Reported-by: coverity-bot 
> Addresses-Coverity-ID: 1530548 ("Control flow issues")
> Fixes: ec787deb2ddf ("drm/amd: Use `amdgpu_ucode_*` helpers for GFX9")
> Signed-off-by: Mario Limonciello 

Reviewed-by: Kees Cook 

-- 
Kees Cook

[PATCH] drm/amd: fix some dead code in `gfx_v9_0_init_cp_compute_microcode`

[Mario Limonciello]

Some dead code was introdcued as part of utilizing the `amdgpu_ucode_*`
helpers. Adjust the control flow to make sure that firmware is released
in the appropriate error flows.

Reported-by: coverity-bot 
Addresses-Coverity-ID: 1530548 ("Control flow issues")
Fixes: ec787deb2ddf ("drm/amd: Use `amdgpu_ucode_*` helpers for GFX9")
Signed-off-by: Mario Limonciello 
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c 
b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index e80685d1e6c6c..d4b0fa4b62a44 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1345,7 +1345,7 @@ static int gfx_v9_0_init_cp_compute_microcode(struct 
amdgpu_device *adev,
 
err = amdgpu_ucode_request(adev, >gfx.mec_fw, fw_name);
if (err)
-   return err;
+   goto out;
amdgpu_gfx_cp_init_microcode(adev, AMDGPU_UCODE_ID_CP_MEC1);
amdgpu_gfx_cp_init_microcode(adev, AMDGPU_UCODE_ID_CP_MEC1_JT);
 
@@ -1355,13 +1355,14 @@ static int gfx_v9_0_init_cp_compute_microcode(struct 
amdgpu_device *adev,
else
snprintf(fw_name, sizeof(fw_name), 
"amdgpu/%s_mec2.bin", chip_name);
 
+   /* ignore failures to load */
err = amdgpu_ucode_request(adev, >gfx.mec2_fw, fw_name);
if (!err) {
amdgpu_gfx_cp_init_microcode(adev, 
AMDGPU_UCODE_ID_CP_MEC2);
amdgpu_gfx_cp_init_microcode(adev, 
AMDGPU_UCODE_ID_CP_MEC2_JT);
} else {
err = 0;
-   adev->gfx.mec2_fw = NULL;
+   amdgpu_ucode_release(>gfx.mec2_fw);
}
} else {
adev->gfx.mec2_fw_version = adev->gfx.mec_fw_version;
@@ -1370,10 +1371,10 @@ static int gfx_v9_0_init_cp_compute_microcode(struct 
amdgpu_device *adev,
 
gfx_v9_0_check_if_need_gfxoff(adev);
gfx_v9_0_check_fw_write_wait(adev);
-   if (err) {
+
+out:
+   if (err)
amdgpu_ucode_release(>gfx.mec_fw);
-   amdgpu_ucode_release(>gfx.mec2_fw);
-   }
return err;
 }
 
-- 
2.34.1