* [PATCH] drm/amdgpu/pm: Fix NULL pointer dereference when set/get power limit
@ 2024-03-06 10:26 Ma Jun
2024-03-06 11:16 ` Lazar, Lijo
0 siblings, 1 reply; 4+ messages in thread
From: Ma Jun @ 2024-03-06 10:26 UTC (permalink / raw)
To: amd-gfx
Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang, Ma Jun,
Yin Zhenguo
Because powerplay_table initialization is skipped under
sriov case, We set default lower and upper OD value to
avoid NULL pointer issue.
Fixes: 7968e9748fbb ("drm/amdgpu/pm: Fix the power1_min_cap value")
Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
Reported-by: Yin Zhenguo <zhenguo.yin@amd.com>
---
.../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 15 ++++++++++-----
drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 16 ++++++++++------
.../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++++-----
.../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 15 ++++++++++-----
.../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 15 ++++++++++-----
5 files changed, 50 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
index 1d96eb274d72..42c5e6b103e8 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
@@ -1303,12 +1303,17 @@ static int arcturus_get_power_limit(struct smu_context *smu,
if (default_power_limit)
*default_power_limit = power_limit;
- if (smu->od_enabled)
- od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
- else
- od_percent_upper = 0;
+ if (powerplay_table) {
+ if (smu->od_enabled)
+ od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
+ else
+ od_percent_upper = 0;
- od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
+ od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
+ } else {
+ od_percent_upper = 0;
+ od_percent_lower = 10;
+ }
dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
od_percent_upper, od_percent_lower, power_limit);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
index ed189a3878eb..6cc2e2d27a0d 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
@@ -2356,13 +2356,17 @@ static int navi10_get_power_limit(struct smu_context *smu,
if (default_power_limit)
*default_power_limit = power_limit;
- if (smu->od_enabled &&
- navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
- od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
- else
- od_percent_upper = 0;
+ if (powerplay_table) {
+ if (smu->od_enabled && navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
+ od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
+ else
+ od_percent_upper = 0;
- od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
+ od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
+ } else {
+ od_percent_upper = 0;
+ od_percent_lower = 10;
+ }
dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
od_percent_upper, od_percent_lower, power_limit);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
index e2ad2b972ab0..5daeac2e6239 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
@@ -640,12 +640,17 @@ static int sienna_cichlid_get_power_limit(struct smu_context *smu,
if (default_power_limit)
*default_power_limit = power_limit;
- if (smu->od_enabled)
- od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
- else
- od_percent_upper = 0;
+ if (powerplay_table) {
+ if (smu->od_enabled)
+ od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
+ else
+ od_percent_upper = 0;
- od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
+ od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
+ } else {
+ od_percent_upper = 0;
+ od_percent_lower = 10;
+ }
dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
od_percent_upper, od_percent_lower, power_limit);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
index 215f7c91ca73..271151c518e1 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
@@ -2369,12 +2369,17 @@ static int smu_v13_0_0_get_power_limit(struct smu_context *smu,
if (default_power_limit)
*default_power_limit = power_limit;
- if (smu->od_enabled)
- od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
- else
- od_percent_upper = 0;
+ if (powerplay_table) {
+ if (smu->od_enabled)
+ od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
+ else
+ od_percent_upper = 0;
- od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
+ od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
+ } else {
+ od_percent_upper = 0;
+ od_percent_lower = 10;
+ }
dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
od_percent_upper, od_percent_lower, power_limit);
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
index 3dc7b60cb075..533a3c1ba41e 100644
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
@@ -2333,12 +2333,17 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu,
if (default_power_limit)
*default_power_limit = power_limit;
- if (smu->od_enabled)
- od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
- else
- od_percent_upper = 0;
+ if (powerplay_table) {
+ if (smu->od_enabled)
+ od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
+ else
+ od_percent_upper = 0;
- od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
+ od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
+ } else {
+ od_percent_upper = 0;
+ od_percent_lower = 10;
+ }
dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
od_percent_upper, od_percent_lower, power_limit);
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/pm: Fix NULL pointer dereference when set/get power limit
2024-03-06 10:26 [PATCH] drm/amdgpu/pm: Fix NULL pointer dereference when set/get power limit Ma Jun
@ 2024-03-06 11:16 ` Lazar, Lijo
2024-03-07 2:12 ` Ma, Jun
0 siblings, 1 reply; 4+ messages in thread
From: Lazar, Lijo @ 2024-03-06 11:16 UTC (permalink / raw)
To: Ma Jun, amd-gfx
Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang, Yin Zhenguo
On 3/6/2024 3:56 PM, Ma Jun wrote:
> Because powerplay_table initialization is skipped under
> sriov case, We set default lower and upper OD value to
> avoid NULL pointer issue.
pp_od_clk_voltage is not enabled in SRIOV (except for GC 9.4.3 one VF
mode). Since the interface is not available for SRIOV, a better fix may
be to set smu->od_enabled itself to false for SRIOV.
Thanks,
Lijo
>
> Fixes: 7968e9748fbb ("drm/amdgpu/pm: Fix the power1_min_cap value")
> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> Reported-by: Yin Zhenguo <zhenguo.yin@amd.com>
> ---
> .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 15 ++++++++++-----
> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 16 ++++++++++------
> .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++++-----
> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 15 ++++++++++-----
> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 15 ++++++++++-----
> 5 files changed, 50 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> index 1d96eb274d72..42c5e6b103e8 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
> @@ -1303,12 +1303,17 @@ static int arcturus_get_power_limit(struct smu_context *smu,
> if (default_power_limit)
> *default_power_limit = power_limit;
>
> - if (smu->od_enabled)
> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> - else
> - od_percent_upper = 0;
> + if (powerplay_table) {
> + if (smu->od_enabled)
> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> + else
> + od_percent_upper = 0;
>
> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> + } else {
> + od_percent_upper = 0;
> + od_percent_lower = 10;
> + }
>
> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
> od_percent_upper, od_percent_lower, power_limit);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> index ed189a3878eb..6cc2e2d27a0d 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
> @@ -2356,13 +2356,17 @@ static int navi10_get_power_limit(struct smu_context *smu,
> if (default_power_limit)
> *default_power_limit = power_limit;
>
> - if (smu->od_enabled &&
> - navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> - else
> - od_percent_upper = 0;
> + if (powerplay_table) {
> + if (smu->od_enabled && navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> + else
> + od_percent_upper = 0;
>
> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
> + } else {
> + od_percent_upper = 0;
> + od_percent_lower = 10;
> + }
>
> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
> od_percent_upper, od_percent_lower, power_limit);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> index e2ad2b972ab0..5daeac2e6239 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
> @@ -640,12 +640,17 @@ static int sienna_cichlid_get_power_limit(struct smu_context *smu,
> if (default_power_limit)
> *default_power_limit = power_limit;
>
> - if (smu->od_enabled)
> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
> - else
> - od_percent_upper = 0;
> + if (powerplay_table) {
> + if (smu->od_enabled)
> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
> + else
> + od_percent_upper = 0;
>
> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
> + } else {
> + od_percent_upper = 0;
> + od_percent_lower = 10;
> + }
>
> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
> od_percent_upper, od_percent_lower, power_limit);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> index 215f7c91ca73..271151c518e1 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
> @@ -2369,12 +2369,17 @@ static int smu_v13_0_0_get_power_limit(struct smu_context *smu,
> if (default_power_limit)
> *default_power_limit = power_limit;
>
> - if (smu->od_enabled)
> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
> - else
> - od_percent_upper = 0;
> + if (powerplay_table) {
> + if (smu->od_enabled)
> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
> + else
> + od_percent_upper = 0;
>
> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
> + } else {
> + od_percent_upper = 0;
> + od_percent_lower = 10;
> + }
>
> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
> od_percent_upper, od_percent_lower, power_limit);
> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> index 3dc7b60cb075..533a3c1ba41e 100644
> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
> @@ -2333,12 +2333,17 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu,
> if (default_power_limit)
> *default_power_limit = power_limit;
>
> - if (smu->od_enabled)
> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
> - else
> - od_percent_upper = 0;
> + if (powerplay_table) {
> + if (smu->od_enabled)
> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
> + else
> + od_percent_upper = 0;
>
> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
> + } else {
> + od_percent_upper = 0;
> + od_percent_lower = 10;
> + }
>
> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
> od_percent_upper, od_percent_lower, power_limit);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/pm: Fix NULL pointer dereference when set/get power limit
2024-03-06 11:16 ` Lazar, Lijo
@ 2024-03-07 2:12 ` Ma, Jun
2024-03-07 3:32 ` Lazar, Lijo
0 siblings, 1 reply; 4+ messages in thread
From: Ma, Jun @ 2024-03-07 2:12 UTC (permalink / raw)
To: Lazar, Lijo, Ma Jun, amd-gfx
Cc: majun, Kenneth.Feng, Alexander.Deucher, kevinyang.wang,
Yin Zhenguo
Hi Lijo,
On 3/6/2024 7:16 PM, Lazar, Lijo wrote:
>
>
> On 3/6/2024 3:56 PM, Ma Jun wrote:
>> Because powerplay_table initialization is skipped under
>> sriov case, We set default lower and upper OD value to
>> avoid NULL pointer issue.
>
> pp_od_clk_voltage is not enabled in SRIOV (except for GC 9.4.3 one VF
> mode). Since the interface is not available for SRIOV, a better fix may
> be to set smu->od_enabled itself to false for SRIOV.
>
This is not about pp_od_clk_voltage and od_enabled. This problem only occurs
when getting power1_cap_* value, because we need to read the od_percent_lower
value from poweplay_table.
Regards,
Ma Jun
> Thanks,
> Lijo
>
>>
>> Fixes: 7968e9748fbb ("drm/amdgpu/pm: Fix the power1_min_cap value")
>> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
>> Reported-by: Yin Zhenguo <zhenguo.yin@amd.com>
>> ---
>> .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 15 ++++++++++-----
>> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 16 ++++++++++------
>> .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++++-----
>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 15 ++++++++++-----
>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 15 ++++++++++-----
>> 5 files changed, 50 insertions(+), 26 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>> index 1d96eb274d72..42c5e6b103e8 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>> @@ -1303,12 +1303,17 @@ static int arcturus_get_power_limit(struct smu_context *smu,
>> if (default_power_limit)
>> *default_power_limit = power_limit;
>>
>> - if (smu->od_enabled)
>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> - else
>> - od_percent_upper = 0;
>> + if (powerplay_table) {
>> + if (smu->od_enabled)
>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> + else
>> + od_percent_upper = 0;
>>
>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> + } else {
>> + od_percent_upper = 0;
>> + od_percent_lower = 10;
>> + }
>>
>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>> od_percent_upper, od_percent_lower, power_limit);
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> index ed189a3878eb..6cc2e2d27a0d 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>> @@ -2356,13 +2356,17 @@ static int navi10_get_power_limit(struct smu_context *smu,
>> if (default_power_limit)
>> *default_power_limit = power_limit;
>>
>> - if (smu->od_enabled &&
>> - navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> - else
>> - od_percent_upper = 0;
>> + if (powerplay_table) {
>> + if (smu->od_enabled && navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> + else
>> + od_percent_upper = 0;
>>
>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>> + } else {
>> + od_percent_upper = 0;
>> + od_percent_lower = 10;
>> + }
>>
>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>> od_percent_upper, od_percent_lower, power_limit);
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> index e2ad2b972ab0..5daeac2e6239 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>> @@ -640,12 +640,17 @@ static int sienna_cichlid_get_power_limit(struct smu_context *smu,
>> if (default_power_limit)
>> *default_power_limit = power_limit;
>>
>> - if (smu->od_enabled)
>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>> - else
>> - od_percent_upper = 0;
>> + if (powerplay_table) {
>> + if (smu->od_enabled)
>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>> + else
>> + od_percent_upper = 0;
>>
>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>> + } else {
>> + od_percent_upper = 0;
>> + od_percent_lower = 10;
>> + }
>>
>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>> od_percent_upper, od_percent_lower, power_limit);
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>> index 215f7c91ca73..271151c518e1 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>> @@ -2369,12 +2369,17 @@ static int smu_v13_0_0_get_power_limit(struct smu_context *smu,
>> if (default_power_limit)
>> *default_power_limit = power_limit;
>>
>> - if (smu->od_enabled)
>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>> - else
>> - od_percent_upper = 0;
>> + if (powerplay_table) {
>> + if (smu->od_enabled)
>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>> + else
>> + od_percent_upper = 0;
>>
>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>> + } else {
>> + od_percent_upper = 0;
>> + od_percent_lower = 10;
>> + }
>>
>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>> od_percent_upper, od_percent_lower, power_limit);
>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>> index 3dc7b60cb075..533a3c1ba41e 100644
>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>> @@ -2333,12 +2333,17 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu,
>> if (default_power_limit)
>> *default_power_limit = power_limit;
>>
>> - if (smu->od_enabled)
>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>> - else
>> - od_percent_upper = 0;
>> + if (powerplay_table) {
>> + if (smu->od_enabled)
>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>> + else
>> + od_percent_upper = 0;
>>
>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>> + } else {
>> + od_percent_upper = 0;
>> + od_percent_lower = 10;
>> + }
>>
>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>> od_percent_upper, od_percent_lower, power_limit);
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/pm: Fix NULL pointer dereference when set/get power limit
2024-03-07 2:12 ` Ma, Jun
@ 2024-03-07 3:32 ` Lazar, Lijo
0 siblings, 0 replies; 4+ messages in thread
From: Lazar, Lijo @ 2024-03-07 3:32 UTC (permalink / raw)
To: Ma, Jun, Ma Jun, amd-gfx
Cc: Kenneth.Feng, Alexander.Deucher, kevinyang.wang, Yin Zhenguo
On 3/7/2024 7:42 AM, Ma, Jun wrote:
> Hi Lijo,
>
> On 3/6/2024 7:16 PM, Lazar, Lijo wrote:
>>
>>
>> On 3/6/2024 3:56 PM, Ma Jun wrote:
>>> Because powerplay_table initialization is skipped under
>>> sriov case, We set default lower and upper OD value to
>>> avoid NULL pointer issue.
>>
>> pp_od_clk_voltage is not enabled in SRIOV (except for GC 9.4.3 one VF
>> mode). Since the interface is not available for SRIOV, a better fix may
>> be to set smu->od_enabled itself to false for SRIOV.
>>
> This is not about pp_od_clk_voltage and od_enabled. This problem only occurs
> when getting power1_cap_* value, because we need to read the od_percent_lower
> value from poweplay_table.
od_enabled = false means there is no OD capability and then OD
lower/higher percentage should be zero. For SRIOV, this will be the case.
Instead you are putting some default values which doesn't necessarily
work for all SKUs (that's the reason they are part of powerplay table).
Thanks,
Lijo
>
> Regards,
> Ma Jun
>> Thanks,
>> Lijo
>>
>>>
>>> Fixes: 7968e9748fbb ("drm/amdgpu/pm: Fix the power1_min_cap value")
>>> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
>>> Reported-by: Yin Zhenguo <zhenguo.yin@amd.com>
>>> ---
>>> .../gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c | 15 ++++++++++-----
>>> drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c | 16 ++++++++++------
>>> .../drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c | 15 ++++++++++-----
>>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c | 15 ++++++++++-----
>>> .../gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c | 15 ++++++++++-----
>>> 5 files changed, 50 insertions(+), 26 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>> index 1d96eb274d72..42c5e6b103e8 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/arcturus_ppt.c
>>> @@ -1303,12 +1303,17 @@ static int arcturus_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>> index ed189a3878eb..6cc2e2d27a0d 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/navi10_ppt.c
>>> @@ -2356,13 +2356,17 @@ static int navi10_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled &&
>>> - navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled && navi10_od_feature_is_supported(od_settings, SMU_11_0_ODCAP_POWER_LIMIT))
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> index e2ad2b972ab0..5daeac2e6239 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu11/sienna_cichlid_ppt.c
>>> @@ -640,12 +640,17 @@ static int sienna_cichlid_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_11_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>> index 215f7c91ca73..271151c518e1 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_0_ppt.c
>>> @@ -2369,12 +2369,17 @@ static int smu_v13_0_0_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_0_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>>> index 3dc7b60cb075..533a3c1ba41e 100644
>>> --- a/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/smu_v13_0_7_ppt.c
>>> @@ -2333,12 +2333,17 @@ static int smu_v13_0_7_get_power_limit(struct smu_context *smu,
>>> if (default_power_limit)
>>> *default_power_limit = power_limit;
>>>
>>> - if (smu->od_enabled)
>>> - od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> - else
>>> - od_percent_upper = 0;
>>> + if (powerplay_table) {
>>> + if (smu->od_enabled)
>>> + od_percent_upper = le32_to_cpu(powerplay_table->overdrive_table.max[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + else
>>> + od_percent_upper = 0;
>>>
>>> - od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + od_percent_lower = le32_to_cpu(powerplay_table->overdrive_table.min[SMU_13_0_7_ODSETTING_POWERPERCENTAGE]);
>>> + } else {
>>> + od_percent_upper = 0;
>>> + od_percent_lower = 10;
>>> + }
>>>
>>> dev_dbg(smu->adev->dev, "od percent upper:%d, od percent lower:%d (default power: %d)\n",
>>> od_percent_upper, od_percent_lower, power_limit);
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-03-07 3:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-06 10:26 [PATCH] drm/amdgpu/pm: Fix NULL pointer dereference when set/get power limit Ma Jun
2024-03-06 11:16 ` Lazar, Lijo
2024-03-07 2:12 ` Ma, Jun
2024-03-07 3:32 ` Lazar, Lijo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox