<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<style type="text/css" style="display:none;"> P {margin-top:0;margin-bottom:0;} </style>
</head>
<body dir="ltr">
<p style="font-family:Arial;font-size:10pt;color:#0000FF;margin:5pt;" align="Left">
[AMD Official Use Only - General]<br>
</p>
<br>
<div>
<div style="font-family: Calibri, Arial, Helvetica, sans-serif; font-size: 12pt; color: rgb(0, 0, 0);">
Ping <span id="🙂">🙂</span></div>
<div id="appendonsend"></div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Powell, Darren <Darren.Powell@amd.com><br>
<b>Sent:</b> Wednesday, May 11, 2022 1:44 AM<br>
<b>To:</b> amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org><br>
<b>Cc:</b> Lazar, Lijo <Lijo.Lazar@amd.com>; Wang, Yang(Kevin) <KevinYang.Wang@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com>; Quan, Evan <Evan.Quan@amd.com>; Ma, Le <Le.Ma@amd.com>; Powell, Darren <Darren.Powell@amd.com><br>
<b>Subject:</b> [PATCH v2 2/2] amdgpu/pm: Fix possible array out-of-bounds if SCLK levels != 2</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">[v2]<br>
simplified fix after Lijo's feedback<br>
removed clocks.num_levels from calculation of loop count<br>
removed unsafe accesses to shim table freq_values<br>
retained corner case output only min,now if<br>
clocks.num_levels == 1 && now > min<br>
<br>
[v1]<br>
added a check to populate and use SCLK shim table freq_values only<br>
if using dpm_level == AMD_DPM_FORCED_LEVEL_MANUAL or<br>
AMD_DPM_FORCED_LEVEL_PERF_DETERMINISM<br>
removed clocks.num_levels from calculation of shim table size<br>
removed unsafe accesses to shim table freq_values<br>
output gfx_table values if using other dpm levels<br>
added check for freq_match when using freq_values for when now == min_clk<br>
<br>
== Test ==<br>
LOGFILE=aldebaran-sclk.test.log<br>
AMDGPU_PCI_ADDR=`lspci -nn | grep "VGA\|Display" | cut -d " " -f 1`<br>
AMDGPU_HWMON=`ls -la /sys/class/hwmon | grep $AMDGPU_PCI_ADDR | awk '{print $9}'`<br>
HWMON_DIR=/sys/class/hwmon/${AMDGPU_HWMON}<br>
<br>
lspci -nn | grep "VGA\|Display" > $LOGFILE<br>
FILES="pp_od_clk_voltage<br>
pp_dpm_sclk"<br>
<br>
for f in $FILES<br>
do<br>
echo === $f === >> $LOGFILE<br>
cat $HWMON_DIR/device/$f >> $LOGFILE<br>
done<br>
cat $LOGFILE<br>
<br>
Signed-off-by: Darren Powell <darren.powell@amd.com><br>
---<br>
.../drm/amd/pm/swsmu/smu13/aldebaran_ppt.c | 34 +++++++------------<br>
1 file changed, 12 insertions(+), 22 deletions(-)<br>
<br>
diff --git a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c<br>
index 6a4fca47ae53..3eb82bc88890 100644<br>
--- a/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c<br>
+++ b/drivers/gpu/drm/amd/pm/swsmu/smu13/aldebaran_ppt.c<br>
@@ -740,7 +740,7 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,<br>
struct smu_13_0_dpm_table *single_dpm_table;<br>
struct smu_dpm_context *smu_dpm = &smu->smu_dpm;<br>
struct smu_13_0_dpm_context *dpm_context = NULL;<br>
- uint32_t display_levels;<br>
+ int display_levels;<br>
uint32_t freq_values[3] = {0};<br>
uint32_t min_clk, max_clk;<br>
<br>
@@ -772,7 +772,7 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,<br>
return ret;<br>
}<br>
<br>
- display_levels = clocks.num_levels;<br>
+ display_levels = (clocks.num_levels == 1) ? 1 : 2;<br>
<br>
min_clk = pstate_table->gfxclk_pstate.curr.min;<br>
max_clk = pstate_table->gfxclk_pstate.curr.max;<br>
@@ -782,30 +782,20 @@ static int aldebaran_print_clk_levels(struct smu_context *smu,<br>
<br>
/* fine-grained dpm has only 2 levels */<br>
if (now > min_clk && now < max_clk) {<br>
- display_levels = clocks.num_levels + 1;<br>
+ display_levels++;<br>
freq_values[2] = max_clk;<br>
freq_values[1] = now;<br>
}<br>
<br>
- /*<br>
- * For DPM disabled case, there will be only one clock level.<br>
- * And it's safe to assume that is always the current clock.<br>
- */<br>
- if (display_levels == clocks.num_levels) {<br>
- for (i = 0; i < clocks.num_levels; i++)<br>
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,<br>
- freq_values[i],<br>
- (clocks.num_levels == 1) ?<br>
- "*" :<br>
- (aldebaran_freqs_in_same_level(<br>
- freq_values[i], now) ?<br>
- "*" :<br>
- ""));<br>
- } else {<br>
- for (i = 0; i < display_levels; i++)<br>
- size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,<br>
- freq_values[i], i == 1 ? "*" : "");<br>
- }<br>
+ for (i = 0; i < display_levels; i++)<br>
+ size += sysfs_emit_at(buf, size, "%d: %uMhz %s\n", i,<br>
+ freq_values[i],<br>
+ (display_levels == 1) ?<br>
+ "*" :<br>
+ (aldebaran_freqs_in_same_level(<br>
+ freq_values[i], now) ?<br>
+ "*" :<br>
+ ""));<br>
<br>
break;<br>
<br>
-- <br>
2.35.1<br>
<br>
</div>
</span></font></div>
</div>
</body>
</html>