* [PATCH 2/2] drm/amdgpu/pm: Fix the param type of set_power_profile_mode
2024-05-06 9:56 [PATCH 1/2] drm/amdgpu: Fix uninitialized variable warning in amdgpu_info_ioctl Ma Jun
@ 2024-05-06 9:56 ` Ma Jun
2024-05-06 15:21 ` [PATCH 1/2] drm/amdgpu: Fix uninitialized variable warning in amdgpu_info_ioctl Alex Deucher
1 sibling, 0 replies; 3+ messages in thread
From: Ma Jun @ 2024-05-06 9:56 UTC (permalink / raw)
To: amd-gfx
Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang, christian.koenig,
Ma Jun
Function .set_power_profile_mode need an array as input
parameter. So define variable workload as an array to fix
the below coverity warning.
"Passing &workload to function hwmgr->hwmgr_func->set_power_profile_mode
which uses it as an array. This might corrupt or misinterpret adjacent
memory locations"
Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
---
drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c | 8 ++++----
drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c | 8 ++++----
drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 16 ++++++++--------
3 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
index 1c40a362d5ab..a71c6117d7e5 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/amd_powerplay.c
@@ -929,7 +929,7 @@ static int pp_dpm_switch_power_profile(void *handle,
enum PP_SMC_POWER_PROFILE type, bool en)
{
struct pp_hwmgr *hwmgr = handle;
- long workload;
+ long workload[1];
uint32_t index;
if (!hwmgr || !hwmgr->pm_en)
@@ -947,12 +947,12 @@ static int pp_dpm_switch_power_profile(void *handle,
hwmgr->workload_mask &= ~(1 << hwmgr->workload_prority[type]);
index = fls(hwmgr->workload_mask);
index = index > 0 && index <= Workload_Policy_Max ? index - 1 : 0;
- workload = hwmgr->workload_setting[index];
+ workload[0] = hwmgr->workload_setting[index];
} else {
hwmgr->workload_mask |= (1 << hwmgr->workload_prority[type]);
index = fls(hwmgr->workload_mask);
index = index <= Workload_Policy_Max ? index - 1 : 0;
- workload = hwmgr->workload_setting[index];
+ workload[0] = hwmgr->workload_setting[index];
}
if (type == PP_SMC_POWER_PROFILE_COMPUTE &&
@@ -962,7 +962,7 @@ static int pp_dpm_switch_power_profile(void *handle,
}
if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL)
- hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, &workload, 0);
+ hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, workload, 0);
return 0;
}
diff --git a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c
index 1d829402cd2e..f4bd8e9357e2 100644
--- a/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c
+++ b/drivers/gpu/drm/amd/pm/powerplay/hwmgr/pp_psm.c
@@ -269,7 +269,7 @@ int psm_adjust_power_state_dynamic(struct pp_hwmgr *hwmgr, bool skip_display_set
struct pp_power_state *new_ps)
{
uint32_t index;
- long workload;
+ long workload[1];
if (hwmgr->not_vf) {
if (!skip_display_settings)
@@ -294,10 +294,10 @@ int psm_adjust_power_state_dynamic(struct pp_hwmgr *hwmgr, bool skip_display_set
if (hwmgr->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL) {
index = fls(hwmgr->workload_mask);
index = index > 0 && index <= Workload_Policy_Max ? index - 1 : 0;
- workload = hwmgr->workload_setting[index];
+ workload[0] = hwmgr->workload_setting[index];
- if (hwmgr->power_profile_mode != workload && hwmgr->hwmgr_func->set_power_profile_mode)
- hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, &workload, 0);
+ if (hwmgr->power_profile_mode != workload[0] && hwmgr->hwmgr_func->set_power_profile_mode)
+ hwmgr->hwmgr_func->set_power_profile_mode(hwmgr, workload, 0);
}
return 0;
diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
index dc0d3a9f1d6d..e61aa4418d44 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
@@ -2210,7 +2210,7 @@ static int smu_adjust_power_state_dynamic(struct smu_context *smu,
{
int ret = 0;
int index = 0;
- long workload;
+ long workload[1];
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
if (!skip_display_settings) {
@@ -2250,10 +2250,10 @@ static int smu_adjust_power_state_dynamic(struct smu_context *smu,
smu_dpm_ctx->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM) {
index = fls(smu->workload_mask);
index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
- workload = smu->workload_setting[index];
+ workload[0] = smu->workload_setting[index];
- if (smu->power_profile_mode != workload)
- smu_bump_power_profile_mode(smu, &workload, 0);
+ if (smu->power_profile_mode != workload[0])
+ smu_bump_power_profile_mode(smu, workload, 0);
}
return ret;
@@ -2303,7 +2303,7 @@ static int smu_switch_power_profile(void *handle,
{
struct smu_context *smu = handle;
struct smu_dpm_context *smu_dpm_ctx = &(smu->smu_dpm);
- long workload;
+ long workload[1];
uint32_t index;
if (!smu->pm_enabled || !smu->adev->pm.dpm_enabled)
@@ -2316,17 +2316,17 @@ static int smu_switch_power_profile(void *handle,
smu->workload_mask &= ~(1 << smu->workload_prority[type]);
index = fls(smu->workload_mask);
index = index > 0 && index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
- workload = smu->workload_setting[index];
+ workload[0] = smu->workload_setting[index];
} else {
smu->workload_mask |= (1 << smu->workload_prority[type]);
index = fls(smu->workload_mask);
index = index <= WORKLOAD_POLICY_MAX ? index - 1 : 0;
- workload = smu->workload_setting[index];
+ workload[0] = smu->workload_setting[index];
}
if (smu_dpm_ctx->dpm_level != AMD_DPM_FORCED_LEVEL_MANUAL &&
smu_dpm_ctx->dpm_level != AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM)
- smu_bump_power_profile_mode(smu, &workload, 0);
+ smu_bump_power_profile_mode(smu, workload, 0);
return 0;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 1/2] drm/amdgpu: Fix uninitialized variable warning in amdgpu_info_ioctl
2024-05-06 9:56 [PATCH 1/2] drm/amdgpu: Fix uninitialized variable warning in amdgpu_info_ioctl Ma Jun
2024-05-06 9:56 ` [PATCH 2/2] drm/amdgpu/pm: Fix the param type of set_power_profile_mode Ma Jun
@ 2024-05-06 15:21 ` Alex Deucher
1 sibling, 0 replies; 3+ messages in thread
From: Alex Deucher @ 2024-05-06 15:21 UTC (permalink / raw)
To: Ma Jun
Cc: amd-gfx, Kenneth.Feng, Alexander.Deucher, kevinyang.wang,
christian.koenig
On Mon, May 6, 2024 at 6:22 AM Ma Jun <Jun.Ma2@amd.com> wrote:
>
> Check the return value of amdgpu_xcp_get_inst_details, otherwise we
> may use an uninitialized variable inst_mask
>
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
Acked-by: Alex Deucher <alexander.deucher@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index a0ea6fe8d060..977cde6d1362 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -623,25 +623,32 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
> switch (type) {
> case AMD_IP_BLOCK_TYPE_GFX:
> ret = amdgpu_xcp_get_inst_details(xcp, AMDGPU_XCP_GFX, &inst_mask);
> + if (ret)
> + return ret;
> count = hweight32(inst_mask);
> break;
> case AMD_IP_BLOCK_TYPE_SDMA:
> ret = amdgpu_xcp_get_inst_details(xcp, AMDGPU_XCP_SDMA, &inst_mask);
> + if (ret)
> + return ret;
> count = hweight32(inst_mask);
> break;
> case AMD_IP_BLOCK_TYPE_JPEG:
> ret = amdgpu_xcp_get_inst_details(xcp, AMDGPU_XCP_VCN, &inst_mask);
> + if (ret)
> + return ret;
> count = hweight32(inst_mask) * adev->jpeg.num_jpeg_rings;
> break;
> case AMD_IP_BLOCK_TYPE_VCN:
> ret = amdgpu_xcp_get_inst_details(xcp, AMDGPU_XCP_VCN, &inst_mask);
> + if (ret)
> + return ret;
> count = hweight32(inst_mask);
> break;
> default:
> return -EINVAL;
> }
> - if (ret)
> - return ret;
> +
> return copy_to_user(out, &count, min(size, 4u)) ? -EFAULT : 0;
> }
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread