From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Addresses-Coverity: ("Out-of-bounds read") Fixes: c27c9778a19e ("drm/amd/powerplay: support BOOTUP_DEFAULT power profile mode") Signed-off-by: Colin Ian King colin.king@canonical.com --- drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c index 75ddcadf3802..4763cb095820 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c @@ -774,6 +774,7 @@ static int vangogh_get_power_profile_mode(struct smu_context *smu, char *buf) { static const char *profile_name[] = { + "BOOTUP_DEFAULT", "FULL_SCREEN_3D", "VIDEO", "VR",
On Mon, Jan 11, 2021 at 6:46 AM Colin King colin.king@canonical.com wrote:
From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Addresses-Coverity: ("Out-of-bounds read") Fixes: c27c9778a19e ("drm/amd/powerplay: support BOOTUP_DEFAULT power profile mode") Signed-off-by: Colin Ian King colin.king@canonical.com
Applied. Thanks!
Alex
drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c index 75ddcadf3802..4763cb095820 100644 --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/vangogh_ppt.c @@ -774,6 +774,7 @@ static int vangogh_get_power_profile_mode(struct smu_context *smu, char *buf) { static const char *profile_name[] = {
"BOOTUP_DEFAULT", "FULL_SCREEN_3D", "VIDEO", "VR",-- 2.29.2
dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Jan 11, 2021 at 11:46:38AM +0000, Colin King wrote:
From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Still not enough to prevent the array overflow. It needs POWERSAVE as well.
regards, dan carpenter
On 12/01/2021 10:07, Dan Carpenter wrote:
On Mon, Jan 11, 2021 at 11:46:38AM +0000, Colin King wrote:
From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Still not enough to prevent the array overflow. It needs POWERSAVE as well.
Thanks for checking, but there is a 1-to-1 relation ship now:
enum PP_SMC_POWER_PROFILE { PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT = 0x0, PP_SMC_POWER_PROFILE_FULLSCREEN3D = 0x1, PP_SMC_POWER_PROFILE_POWERSAVING = 0x2, PP_SMC_POWER_PROFILE_VIDEO = 0x3, PP_SMC_POWER_PROFILE_VR = 0x4, PP_SMC_POWER_PROFILE_COMPUTE = 0x5, PP_SMC_POWER_PROFILE_CUSTOM = 0x6, PP_SMC_POWER_PROFILE_COUNT, };
vs
static const char *profile_name[] = { "BOOTUP_DEFAULT", "3D_FULL_SCREEN", "POWER_SAVING", "VIDEO", "VR", "COMPUTE", "CUSTOM"};
unless I'm missing something because I've not had enough coffee.
Colin
regards, dan carpenter
Le 15/01/2021 à 10:37, Colin Ian King a écrit :
On 12/01/2021 10:07, Dan Carpenter wrote:
On Mon, Jan 11, 2021 at 11:46:38AM +0000, Colin King wrote:
From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Still not enough to prevent the array overflow. It needs POWERSAVE as well.
Thanks for checking, but there is a 1-to-1 relation ship now:
enum PP_SMC_POWER_PROFILE { PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT = 0x0, PP_SMC_POWER_PROFILE_FULLSCREEN3D = 0x1, PP_SMC_POWER_PROFILE_POWERSAVING = 0x2, PP_SMC_POWER_PROFILE_VIDEO = 0x3, PP_SMC_POWER_PROFILE_VR = 0x4, PP_SMC_POWER_PROFILE_COMPUTE = 0x5, PP_SMC_POWER_PROFILE_CUSTOM = 0x6, PP_SMC_POWER_PROFILE_COUNT, };
vs
static const char *profile_name[] = { "BOOTUP_DEFAULT", "3D_FULL_SCREEN", "POWER_SAVING",
This line has been added yesterday in commit f727ebeb589d. So Dan was right when he sent his patch, but some else fixed it.
CJ
"VIDEO", "VR", "COMPUTE", "CUSTOM"};unless I'm missing something because I've not had enough coffee.
Colin
regards, dan carpenter
On 15/01/2021 10:07, Christophe JAILLET wrote:
Le 15/01/2021 à 10:37, Colin Ian King a écrit :
On 12/01/2021 10:07, Dan Carpenter wrote:
On Mon, Jan 11, 2021 at 11:46:38AM +0000, Colin King wrote:
From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Still not enough to prevent the array overflow. It needs POWERSAVE as well.
Thanks for checking, but there is a 1-to-1 relation ship now:
enum PP_SMC_POWER_PROFILE { PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT = 0x0, PP_SMC_POWER_PROFILE_FULLSCREEN3D = 0x1, PP_SMC_POWER_PROFILE_POWERSAVING = 0x2, PP_SMC_POWER_PROFILE_VIDEO = 0x3, PP_SMC_POWER_PROFILE_VR = 0x4, PP_SMC_POWER_PROFILE_COMPUTE = 0x5, PP_SMC_POWER_PROFILE_CUSTOM = 0x6, PP_SMC_POWER_PROFILE_COUNT, };
vs
static const char *profile_name[] = { "BOOTUP_DEFAULT", "3D_FULL_SCREEN", "POWER_SAVING",
This line has been added yesterday in commit f727ebeb589d. So Dan was right when he sent his patch, but some else fixed it.
Ah, my bad for not seeing that. :-/
CJ
"VIDEO", "VR", "COMPUTE", "CUSTOM"};
unless I'm missing something because I've not had enough coffee.
Colin
regards, dan carpenter
Le 15/01/2021 à 11:10, Colin Ian King a écrit :
On 15/01/2021 10:07, Christophe JAILLET wrote:
Le 15/01/2021 à 10:37, Colin Ian King a écrit :
On 12/01/2021 10:07, Dan Carpenter wrote:
On Mon, Jan 11, 2021 at 11:46:38AM +0000, Colin King wrote:
From: Colin Ian King colin.king@canonical.com
A recent change added a new BOOTUP_DEFAULT power profile mode to the PP_SMC_POWER_PROFILE enum but omitted updating the corresponding profile_name array. Fix this by adding in the missing BOOTUP_DEFAULT to profile_name[].
Still not enough to prevent the array overflow. It needs POWERSAVE as well.
Thanks for checking, but there is a 1-to-1 relation ship now:
enum PP_SMC_POWER_PROFILE { PP_SMC_POWER_PROFILE_BOOTUP_DEFAULT = 0x0, PP_SMC_POWER_PROFILE_FULLSCREEN3D = 0x1, PP_SMC_POWER_PROFILE_POWERSAVING = 0x2, PP_SMC_POWER_PROFILE_VIDEO = 0x3, PP_SMC_POWER_PROFILE_VR = 0x4, PP_SMC_POWER_PROFILE_COMPUTE = 0x5, PP_SMC_POWER_PROFILE_CUSTOM = 0x6, PP_SMC_POWER_PROFILE_COUNT, };
vs
static const char *profile_name[] = { "BOOTUP_DEFAULT", "3D_FULL_SCREEN", "POWER_SAVING",
This line has been added yesterday in commit f727ebeb589d. So Dan was right when he sent his patch, but some else fixed it.
Ah, my bad for not seeing that. :-/
However, I wonder if this commit is complete. The description of the commit is about 5 modes, but 6 are listed in PP_SMC_POWER_PROFILE.
In the hunk: +static struct cmn2asic_mapping vangogh_workload_map[PP_SMC_POWER_PROFILE_COUNT] = { + WORKLOAD_MAP(PP_SMC_POWER_PROFILE_FULLSCREEN3D, WORKLOAD_PPLIB_FULL_SCREEN_3D_BIT), + WORKLOAD_MAP(PP_SMC_POWER_PROFILE_VIDEO, WORKLOAD_PPLIB_VIDEO_BIT), + WORKLOAD_MAP(PP_SMC_POWER_PROFILE_VR, WORKLOAD_PPLIB_VR_BIT), + WORKLOAD_MAP(PP_SMC_POWER_PROFILE_COMPUTE, WORKLOAD_PPLIB_COMPUTE_BIT), + WORKLOAD_MAP(PP_SMC_POWER_PROFILE_CUSTOM, WORKLOAD_PPLIB_CUSTOM_BIT), +};
It would look logical to have something like: + WORKLOAD_MAP(PP_SMC_POWER_PROFILE_POWERSAVING, WORKLOAD_PPLIB_POWER_SAVING_BIT),
Not sure at all if correct.
Just my 2c,
CJ
CJ
"VIDEO", "VR", "COMPUTE", "CUSTOM"};
unless I'm missing something because I've not had enough coffee.
Colin
regards, dan carpenter
dri-devel@lists.freedesktop.org
-
Alex Deucher -
Christophe JAILLET -
Colin Ian King -
Colin King
-
Dan Carpenter