[PATCH] power/amd_pstate: fix frequency matching for continuous scaling

[Stephen Hemminger]

The power_init_for_setting_freq() function fails on systems using the
amd-pstate-epp driver because the current CPU frequency read from
scaling_setspeed does not exactly match any of the synthesized
frequency buckets. Unlike acpi_cpufreq which provides a discrete list
of frequencies, amd-pstate operates with continuously variable
frequencies, so an exact match will rarely succeed.

For example, on a Ryzen 9 7945HX the sysfs file reports 2797172
which rounds to 2797000, but this value does not appear in the
generated frequency table.

Replace the exact match lookup with a nearest-frequency search.

Also fix several issues in the same function:
 - strtoul() was called with NULL endptr, making parse failures
   undetectable
 - errno was not checked, so stale errors could cause false positives
 - freq was declared as uint32_t, truncating the unsigned long
   return value of strtoul() on LP64 platforms
 - no error was logged when frequency matching failed

Bugzilla ID: 1915
Fixes: 1ed04d33cf19 ("power: support amd-pstate cpufreq driver")
Cc: stable at dpdk.org

Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
---
 drivers/power/amd_pstate/amd_pstate_cpufreq.c | 56 ++++++++++++++-----
 1 file changed, 41 insertions(+), 15 deletions(-)

diff --git a/drivers/power/amd_pstate/amd_pstate_cpufreq.c b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
index 95495bff7d..14625ffd73 100644
--- a/drivers/power/amd_pstate/amd_pstate_cpufreq.c
+++ b/drivers/power/amd_pstate/amd_pstate_cpufreq.c
@@ -5,6 +5,7 @@
  */
 
 #include <stdlib.h>
+#include <errno.h>
 
 #include <rte_memcpy.h>
 #include <rte_stdatomic.h>
@@ -291,15 +292,21 @@ power_get_available_freqs(struct amd_pstate_power_info *pi)
 	return ret;
 }
 
-/**
- * It is to fopen the sys file for the future setting the lcore frequency.
- */
+static inline unsigned long
+abs_diff(unsigned long a, unsigned long b)
+{
+	return (a > b) ? a - b : b - a;
+}
+
 static int
 power_init_for_setting_freq(struct amd_pstate_power_info *pi)
 {
-	FILE *f = NULL;
+	FILE *f;
 	char buf[BUFSIZ];
-	uint32_t i, freq;
+	char *endptr;
+	unsigned long freq, freq_conv;
+	unsigned long best_diff, diff;
+	uint32_t i, best_idx;
 	int ret;
 
 	open_core_sysfs_file(&f, "rw+", POWER_SYSFILE_SETSPEED, pi->lcore_id);
@@ -308,7 +315,6 @@ power_init_for_setting_freq(struct amd_pstate_power_info *pi)
 				POWER_SYSFILE_SETSPEED);
 		goto err;
 	}
-
 	ret = read_core_sysfs_s(f, buf, sizeof(buf));
 	if (ret < 0) {
 		POWER_LOG(ERR, "Failed to read %s",
@@ -316,25 +322,45 @@ power_init_for_setting_freq(struct amd_pstate_power_info *pi)
 		goto err;
 	}
 
-	freq = strtoul(buf, NULL, POWER_CONVERT_TO_DECIMAL);
+	errno = 0;
+	freq = strtoul(buf, &endptr, POWER_CONVERT_TO_DECIMAL);
+	if (errno != 0 || endptr == buf || freq == 0) {
+		POWER_LOG(ERR, "Failed to parse frequency '%s' for lcore %u",
+				buf, pi->lcore_id);
+		goto err;
+	}
 
 	/* convert the frequency to nearest 1000 value
 	 * Ex: if freq=1396789 then freq_conv=1397000
 	 * Ex: if freq=800030 then freq_conv=800000
 	 */
-	unsigned int freq_conv = 0;
-	freq_conv = (freq + FREQ_ROUNDING_DELTA)
-				/ ROUND_FREQ_TO_N_1000;
+	freq_conv = (freq + FREQ_ROUNDING_DELTA) / ROUND_FREQ_TO_N_1000;
 	freq_conv = freq_conv * ROUND_FREQ_TO_N_1000;
 
-	for (i = 0; i < pi->nb_freqs; i++) {
-		if (freq_conv == pi->freqs[i]) {
-			pi->curr_idx = i;
-			pi->f = f;
-			return 0;
+	/* Find the nearest frequency in the table.
+	 * With amd-pstate the CPU runs at continuously variable
+	 * frequencies so the current frequency will not exactly
+	 * match one of the synthesized frequency buckets.
+	 */
+	best_idx = 0;
+	best_diff = abs_diff(freq_conv, pi->freqs[0]);
+
+	for (i = 1; i < pi->nb_freqs; i++) {
+		diff = abs_diff(freq_conv, pi->freqs[i]);
+		if (diff < best_diff) {
+			best_diff = diff;
+			best_idx = i;
 		}
 	}
 
+	POWER_DEBUG_LOG("Freq %lu rounded to %lu matched bucket [%u] = %u "
+			"for lcore %u", freq, freq_conv, best_idx,
+			pi->freqs[best_idx], pi->lcore_id);
+
+	pi->curr_idx = best_idx;
+	pi->f = f;
+	return 0;
+
 err:
 	if (f != NULL)
 		fclose(f);
-- 
2.53.0