diff mbox

cpufreq: Add test cases to validate the frequency and pstate table

Message ID 1461300695-20159-1-git-send-email-shilpa.bhat@linux.vnet.ibm.com
State Superseded
Headers show

Commit Message

Shilpasri G Bhat April 22, 2016, 4:51 a.m. UTC
Add test case to parse CPU frequency and pstate table to check for
invalid entries that are beyond the specified minimum and maximum
limits.

Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 src/cpu/cpufreq/cpufreq.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 139 insertions(+)

Comments

Colin Ian King April 22, 2016, 8:17 a.m. UTC | #1
On 22/04/16 05:51, Shilpasri G Bhat wrote:
> Add test case to parse CPU frequency and pstate table to check for
> invalid entries that are beyond the specified minimum and maximum
> limits.
> 
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> ---
>  src/cpu/cpufreq/cpufreq.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 139 insertions(+)
> 
> diff --git a/src/cpu/cpufreq/cpufreq.c b/src/cpu/cpufreq/cpufreq.c
> index 8ab5292..1a9c9fd 100644
> --- a/src/cpu/cpufreq/cpufreq.c
> +++ b/src/cpu/cpufreq/cpufreq.c
> @@ -30,6 +30,7 @@
>  #include <unistd.h>
>  #include <sys/types.h>
>  #include <sys/stat.h>
> +#include <fcntl.h>
>  #include <limits.h>
>  #include <dirent.h>
>  #include <stdint.h>
> @@ -393,6 +394,142 @@ static int cpufreq_test_cpu_performance(fwts_framework *fw)
>  	return FWTS_OK;
>  }
>  
> +static int cpufreq_test_frequency_limits(fwts_framework *fw)
> +{
> +	char path[PATH_MAX];
> +	int min, max, i;
> +	bool ok = true;
> +
> +	cpu_mkpath(path, sizeof(path), &cpus[0], "cpuinfo_min_freq");
> +	if (fwts_get_int(path, &min) != FWTS_OK) {
> +		fwts_log_warning(fw, "Failed to read the minimum CPU frequency %s",
> +				 path);
> +		return FWTS_ERROR;
> +	}
> +
> +	cpu_mkpath(path, sizeof(path), &cpus[0], "cpuinfo_max_freq");
> +	if (fwts_get_int(path, &max) != FWTS_OK) {
> +		fwts_log_warning(fw, "Failed to read the maximum CPU frequency %s",
> +				 path);
> +		return FWTS_ERROR;
> +	}
> +
> +	for (i = 0; i < cpus[0].n_freqs; i++) {
> +		if (cpus[0].freqs[i].Hz > (uint64_t)max) {
> +			fwts_log_warning(fw, "%s is greater than max frequency %s",
> +					 hz_to_human(cpus[0].freqs[i].Hz),
> +					 hz_to_human(max));
> +			ok = false;
> +		}
> +		if (cpus[0].freqs[i].Hz < (uint64_t)min) {
> +			fwts_log_warning(fw, "%s is lesser than min frequency %s",
> +					 hz_to_human(cpus[0].freqs[i].Hz),
> +					 hz_to_human(min));
> +			ok = false;
> +		}
> +	}
> +	if (ok)
> +		fwts_passed(fw, "CPU frequencies are within the min and max limits");
> +	else
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUFreqLimitsTest",
> +			     "Contains frequencies beyond max and min limits");
> +	return FWTS_OK;
> +}
> +
> +static int read_device_tree_file(const char *path, int *val)
> +{
> +	int fd;
> +
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +
> +	if (read(fd, val, sizeof(*val)) < 0)
> +		return FWTS_ERROR;
> +
> +	*val = be32toh(*val);
> +	close(fd);
> +	return FWTS_OK;
> +}
> +
> +static int read_device_tree_array(const char *path, int *arr, int *len)
> +{
> +	int fd;
> +	int i = 0;
> +
> +	fd = open(path, O_RDONLY);
> +	if (fd < 0)
> +		return fd;
> +
> +	while(1) {
> +		if (read(fd, (arr + i), sizeof(arr[i])) <= 0)
> +			break;
> +		arr[i] = be32toh(arr[i]);
> +		i++;
> +	}
> +
> +	*len = (!i) ? i : i - 1;
> +	close(fd);
> +	return FWTS_OK;
> +}
> +
> +static int cpufreq_test_pstate_limits(fwts_framework *fw)
> +{
> +	char *dev_tree_path = "/proc/device-tree/ibm,opal/power-mgt/";
> +	int pstate_min, pstate_max, pstates[MAX_FREQS], nr_pstates, len;
> +	char path[PATH_MAX];
> +	bool ok = true;
> +	int i;
> +
> +#if !defined(__PPC64__)
> +	fwts_skipped(fw, "Test case applicable to PPC64 only");
> +	return FWTS_SKIP;
> +#endif
> +
> +	snprintf(path, sizeof(path), "%sibm,pstate-max", dev_tree_path);
> +	if (read_device_tree_file(path, &pstate_max) < 0) {
> +		fwts_log_warning(fw, "Failed to read the max pstate %s\n", path);
> +		return FWTS_ERROR;
> +	}
> +
> +	snprintf(path, sizeof(path), "%sibm,pstate-min", dev_tree_path);
> +	if (read_device_tree_file(path, &pstate_min) < 0) {
> +		fwts_log_warning(fw, "Failed to read the min pstate %s\n", path);
> +		return FWTS_ERROR;
> +	}
> +
> +	snprintf(path, sizeof(path), "%sibm,pstate-ids", dev_tree_path);
> +	if (read_device_tree_array(path, pstates, &len) < 0 || !len) {
> +		fwts_log_warning(fw, "Failed to read the pstate-id array %s\n", path);
> +		return FWTS_ERROR;
> +	}
> +
> +	nr_pstates = pstate_max - pstate_min;
> +	if (len != nr_pstates)
> +		fwts_log_warning(fw, "Wrong number of pstates. Expected %d pstates, but found %d pstates\n",
> +				 nr_pstates, len);
> +
> +	for (i = 0; i < nr_pstates; i++) {
> +		if (pstates[i] > pstate_max) {
> +			fwts_log_warning(fw, "Invalid Pstate id %d greater than max pstate %d\n",
> +					  pstates[i], pstate_max);
> +			ok = false;
> +		}
> +		if (pstates[i] < pstate_min) {
> +			fwts_log_warning(fw, "Invalid Pstate id %d lesser than min pstate %d\n",
> +					  pstates[i], pstate_min);
> +			ok = false;
> +		}
> +	}
> +
> +	if (ok)
> +		fwts_passed(fw, "CPU pstates are within the min and max limits");
> +	else
> +		fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUPstateLimitsTest",
> +			     "Contains pstates beyond max and min limits");
> +	return FWTS_OK;
> +}
> +
>  static int sw_tests_possible(fwts_framework *fw)
>  {
>  	int i, online_cpus = 0;
> @@ -861,6 +998,8 @@ static fwts_framework_minor_test cpufreq_tests[] = {
>  	{ cpufreq_test_sw_any,		"CPU frequency SW_ANY control" },
>  	{ cpufreq_test_sw_all,		"CPU frequency SW_ALL control" },
>  	{ cpufreq_test_cpu_performance,	"CPU frequency performance tests." },
> +	{ cpufreq_test_frequency_limits, "CPU frequency limits test" },

Would it make more sense to make the following line a powerpc64 build
only option:

+ #if defined(__PPC64__)
> +	{ cpufreq_test_pstate_limits,	"CPU Pstate limits test (device_tree)" },
+ #endif

>  	{ NULL, NULL }
>  };
>  
>
Shilpasri G Bhat April 22, 2016, 8:24 a.m. UTC | #2
Hi,

On 04/22/2016 01:47 PM, Colin Ian King wrote:
> On 22/04/16 05:51, Shilpasri G Bhat wrote:
>> Add test case to parse CPU frequency and pstate table to check for
>> invalid entries that are beyond the specified minimum and maximum
>> limits.
>>
>> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
>> ---
>>  src/cpu/cpufreq/cpufreq.c | 139 ++++++++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 139 insertions(+)
>>
>> diff --git a/src/cpu/cpufreq/cpufreq.c b/src/cpu/cpufreq/cpufreq.c
>> index 8ab5292..1a9c9fd 100644
>> --- a/src/cpu/cpufreq/cpufreq.c
>> +++ b/src/cpu/cpufreq/cpufreq.c
>> @@ -30,6 +30,7 @@
>>  #include <unistd.h>
>>  #include <sys/types.h>
>>  #include <sys/stat.h>
>> +#include <fcntl.h>
>>  #include <limits.h>
>>  #include <dirent.h>
>>  #include <stdint.h>
>> @@ -393,6 +394,142 @@ static int cpufreq_test_cpu_performance(fwts_framework *fw)
>>  	return FWTS_OK;
>>  }
>>  
>> +static int cpufreq_test_frequency_limits(fwts_framework *fw)
>> +{
>> +	char path[PATH_MAX];
>> +	int min, max, i;
>> +	bool ok = true;
>> +
>> +	cpu_mkpath(path, sizeof(path), &cpus[0], "cpuinfo_min_freq");
>> +	if (fwts_get_int(path, &min) != FWTS_OK) {
>> +		fwts_log_warning(fw, "Failed to read the minimum CPU frequency %s",
>> +				 path);
>> +		return FWTS_ERROR;
>> +	}
>> +
>> +	cpu_mkpath(path, sizeof(path), &cpus[0], "cpuinfo_max_freq");
>> +	if (fwts_get_int(path, &max) != FWTS_OK) {
>> +		fwts_log_warning(fw, "Failed to read the maximum CPU frequency %s",
>> +				 path);
>> +		return FWTS_ERROR;
>> +	}
>> +
>> +	for (i = 0; i < cpus[0].n_freqs; i++) {
>> +		if (cpus[0].freqs[i].Hz > (uint64_t)max) {
>> +			fwts_log_warning(fw, "%s is greater than max frequency %s",
>> +					 hz_to_human(cpus[0].freqs[i].Hz),
>> +					 hz_to_human(max));
>> +			ok = false;
>> +		}
>> +		if (cpus[0].freqs[i].Hz < (uint64_t)min) {
>> +			fwts_log_warning(fw, "%s is lesser than min frequency %s",
>> +					 hz_to_human(cpus[0].freqs[i].Hz),
>> +					 hz_to_human(min));
>> +			ok = false;
>> +		}
>> +	}
>> +	if (ok)
>> +		fwts_passed(fw, "CPU frequencies are within the min and max limits");
>> +	else
>> +		fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUFreqLimitsTest",
>> +			     "Contains frequencies beyond max and min limits");
>> +	return FWTS_OK;
>> +}
>> +
>> +static int read_device_tree_file(const char *path, int *val)
>> +{
>> +	int fd;
>> +
>> +	fd = open(path, O_RDONLY);
>> +	if (fd < 0)
>> +		return fd;
>> +
>> +	if (read(fd, val, sizeof(*val)) < 0)
>> +		return FWTS_ERROR;
>> +
>> +	*val = be32toh(*val);
>> +	close(fd);
>> +	return FWTS_OK;
>> +}
>> +
>> +static int read_device_tree_array(const char *path, int *arr, int *len)
>> +{
>> +	int fd;
>> +	int i = 0;
>> +
>> +	fd = open(path, O_RDONLY);
>> +	if (fd < 0)
>> +		return fd;
>> +
>> +	while(1) {
>> +		if (read(fd, (arr + i), sizeof(arr[i])) <= 0)
>> +			break;
>> +		arr[i] = be32toh(arr[i]);
>> +		i++;
>> +	}
>> +
>> +	*len = (!i) ? i : i - 1;
>> +	close(fd);
>> +	return FWTS_OK;
>> +}
>> +
>> +static int cpufreq_test_pstate_limits(fwts_framework *fw)
>> +{
>> +	char *dev_tree_path = "/proc/device-tree/ibm,opal/power-mgt/";
>> +	int pstate_min, pstate_max, pstates[MAX_FREQS], nr_pstates, len;
>> +	char path[PATH_MAX];
>> +	bool ok = true;
>> +	int i;
>> +
>> +#if !defined(__PPC64__)
>> +	fwts_skipped(fw, "Test case applicable to PPC64 only");
>> +	return FWTS_SKIP;
>> +#endif
>> +
>> +	snprintf(path, sizeof(path), "%sibm,pstate-max", dev_tree_path);
>> +	if (read_device_tree_file(path, &pstate_max) < 0) {
>> +		fwts_log_warning(fw, "Failed to read the max pstate %s\n", path);
>> +		return FWTS_ERROR;
>> +	}
>> +
>> +	snprintf(path, sizeof(path), "%sibm,pstate-min", dev_tree_path);
>> +	if (read_device_tree_file(path, &pstate_min) < 0) {
>> +		fwts_log_warning(fw, "Failed to read the min pstate %s\n", path);
>> +		return FWTS_ERROR;
>> +	}
>> +
>> +	snprintf(path, sizeof(path), "%sibm,pstate-ids", dev_tree_path);
>> +	if (read_device_tree_array(path, pstates, &len) < 0 || !len) {
>> +		fwts_log_warning(fw, "Failed to read the pstate-id array %s\n", path);
>> +		return FWTS_ERROR;
>> +	}
>> +
>> +	nr_pstates = pstate_max - pstate_min;
>> +	if (len != nr_pstates)
>> +		fwts_log_warning(fw, "Wrong number of pstates. Expected %d pstates, but found %d pstates\n",
>> +				 nr_pstates, len);
>> +
>> +	for (i = 0; i < nr_pstates; i++) {
>> +		if (pstates[i] > pstate_max) {
>> +			fwts_log_warning(fw, "Invalid Pstate id %d greater than max pstate %d\n",
>> +					  pstates[i], pstate_max);
>> +			ok = false;
>> +		}
>> +		if (pstates[i] < pstate_min) {
>> +			fwts_log_warning(fw, "Invalid Pstate id %d lesser than min pstate %d\n",
>> +					  pstates[i], pstate_min);
>> +			ok = false;
>> +		}
>> +	}
>> +
>> +	if (ok)
>> +		fwts_passed(fw, "CPU pstates are within the min and max limits");
>> +	else
>> +		fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUPstateLimitsTest",
>> +			     "Contains pstates beyond max and min limits");
>> +	return FWTS_OK;
>> +}
>> +
>>  static int sw_tests_possible(fwts_framework *fw)
>>  {
>>  	int i, online_cpus = 0;
>> @@ -861,6 +998,8 @@ static fwts_framework_minor_test cpufreq_tests[] = {
>>  	{ cpufreq_test_sw_any,		"CPU frequency SW_ANY control" },
>>  	{ cpufreq_test_sw_all,		"CPU frequency SW_ALL control" },
>>  	{ cpufreq_test_cpu_performance,	"CPU frequency performance tests." },
>> +	{ cpufreq_test_frequency_limits, "CPU frequency limits test" },
> 
> Would it make more sense to make the following line a powerpc64 build
> only option:
> 
> + #if defined(__PPC64__)
>> +	{ cpufreq_test_pstate_limits,	"CPU Pstate limits test (device_tree)" },
> + #endif
> 

Yes I will make this change and post.

Thanks and Regards,
Shilpa


>>  	{ NULL, NULL }
>>  };
>>  
>>
>
Jeremy Kerr April 22, 2016, 8:54 a.m. UTC | #3
Hi Shilpasri,

> +static int read_device_tree_file(const char *path, int *val)

[...]

> +static int read_device_tree_array(const char *path, int *arr, int *len)

[...]

> +static int cpufreq_test_pstate_limits(fwts_framework *fw)
> +{
> +	char *dev_tree_path = "/proc/device-tree/ibm,opal/power-mgt/";

[...]

We already have a representation of the device tree loaded, in fw->fdt.
You can use the standard libfdt accessors on this data.

Then, the compile time check becomes FWTS_HAS_DEVICETREE (to determine
whether you can use the libfdt functions), and you'd skip the test if
you have no fdt data:

	if (!fw->fdt)
		return FWTS_SKIPPED;

Cheers,


Jeremy
Jeremy Kerr April 22, 2016, 8:58 a.m. UTC | #4
Hi Shilpasri,

> We already have a representation of the device tree loaded, in fw->fdt.
> You can use the standard libfdt accessors on this data.

.. and if you need higher-level abstractions for this, then they'd be
useful as generic additions to fwts_devicetree.c.

Cheers,


Jeremy
Shilpasri G Bhat April 22, 2016, 9:22 a.m. UTC | #5
Hi Jeremy,

On 04/22/2016 02:24 PM, Jeremy Kerr wrote:
> Hi Shilpasri,
> 
>> +static int read_device_tree_file(const char *path, int *val)
> 
> [...]
> 
>> +static int read_device_tree_array(const char *path, int *arr, int *len)
> 
> [...]
> 
>> +static int cpufreq_test_pstate_limits(fwts_framework *fw)
>> +{
>> +	char *dev_tree_path = "/proc/device-tree/ibm,opal/power-mgt/";
> 
> [...]
> 
> We already have a representation of the device tree loaded, in fw->fdt.
> You can use the standard libfdt accessors on this data.
> 
> Then, the compile time check becomes FWTS_HAS_DEVICETREE (to determine
> whether you can use the libfdt functions), and you'd skip the test if
> you have no fdt data:
> 
> 	if (!fw->fdt)
> 		return FWTS_SKIPPED;

Cool. I will use fw->fdt to parse the pstate table. Thanks for pointing this out.

Regards,
Shilpa

> 
> Cheers,
> 
> 
> Jeremy
>
diff mbox

Patch

diff --git a/src/cpu/cpufreq/cpufreq.c b/src/cpu/cpufreq/cpufreq.c
index 8ab5292..1a9c9fd 100644
--- a/src/cpu/cpufreq/cpufreq.c
+++ b/src/cpu/cpufreq/cpufreq.c
@@ -30,6 +30,7 @@ 
 #include <unistd.h>
 #include <sys/types.h>
 #include <sys/stat.h>
+#include <fcntl.h>
 #include <limits.h>
 #include <dirent.h>
 #include <stdint.h>
@@ -393,6 +394,142 @@  static int cpufreq_test_cpu_performance(fwts_framework *fw)
 	return FWTS_OK;
 }
 
+static int cpufreq_test_frequency_limits(fwts_framework *fw)
+{
+	char path[PATH_MAX];
+	int min, max, i;
+	bool ok = true;
+
+	cpu_mkpath(path, sizeof(path), &cpus[0], "cpuinfo_min_freq");
+	if (fwts_get_int(path, &min) != FWTS_OK) {
+		fwts_log_warning(fw, "Failed to read the minimum CPU frequency %s",
+				 path);
+		return FWTS_ERROR;
+	}
+
+	cpu_mkpath(path, sizeof(path), &cpus[0], "cpuinfo_max_freq");
+	if (fwts_get_int(path, &max) != FWTS_OK) {
+		fwts_log_warning(fw, "Failed to read the maximum CPU frequency %s",
+				 path);
+		return FWTS_ERROR;
+	}
+
+	for (i = 0; i < cpus[0].n_freqs; i++) {
+		if (cpus[0].freqs[i].Hz > (uint64_t)max) {
+			fwts_log_warning(fw, "%s is greater than max frequency %s",
+					 hz_to_human(cpus[0].freqs[i].Hz),
+					 hz_to_human(max));
+			ok = false;
+		}
+		if (cpus[0].freqs[i].Hz < (uint64_t)min) {
+			fwts_log_warning(fw, "%s is lesser than min frequency %s",
+					 hz_to_human(cpus[0].freqs[i].Hz),
+					 hz_to_human(min));
+			ok = false;
+		}
+	}
+	if (ok)
+		fwts_passed(fw, "CPU frequencies are within the min and max limits");
+	else
+		fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUFreqLimitsTest",
+			     "Contains frequencies beyond max and min limits");
+	return FWTS_OK;
+}
+
+static int read_device_tree_file(const char *path, int *val)
+{
+	int fd;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return fd;
+
+	if (read(fd, val, sizeof(*val)) < 0)
+		return FWTS_ERROR;
+
+	*val = be32toh(*val);
+	close(fd);
+	return FWTS_OK;
+}
+
+static int read_device_tree_array(const char *path, int *arr, int *len)
+{
+	int fd;
+	int i = 0;
+
+	fd = open(path, O_RDONLY);
+	if (fd < 0)
+		return fd;
+
+	while(1) {
+		if (read(fd, (arr + i), sizeof(arr[i])) <= 0)
+			break;
+		arr[i] = be32toh(arr[i]);
+		i++;
+	}
+
+	*len = (!i) ? i : i - 1;
+	close(fd);
+	return FWTS_OK;
+}
+
+static int cpufreq_test_pstate_limits(fwts_framework *fw)
+{
+	char *dev_tree_path = "/proc/device-tree/ibm,opal/power-mgt/";
+	int pstate_min, pstate_max, pstates[MAX_FREQS], nr_pstates, len;
+	char path[PATH_MAX];
+	bool ok = true;
+	int i;
+
+#if !defined(__PPC64__)
+	fwts_skipped(fw, "Test case applicable to PPC64 only");
+	return FWTS_SKIP;
+#endif
+
+	snprintf(path, sizeof(path), "%sibm,pstate-max", dev_tree_path);
+	if (read_device_tree_file(path, &pstate_max) < 0) {
+		fwts_log_warning(fw, "Failed to read the max pstate %s\n", path);
+		return FWTS_ERROR;
+	}
+
+	snprintf(path, sizeof(path), "%sibm,pstate-min", dev_tree_path);
+	if (read_device_tree_file(path, &pstate_min) < 0) {
+		fwts_log_warning(fw, "Failed to read the min pstate %s\n", path);
+		return FWTS_ERROR;
+	}
+
+	snprintf(path, sizeof(path), "%sibm,pstate-ids", dev_tree_path);
+	if (read_device_tree_array(path, pstates, &len) < 0 || !len) {
+		fwts_log_warning(fw, "Failed to read the pstate-id array %s\n", path);
+		return FWTS_ERROR;
+	}
+
+	nr_pstates = pstate_max - pstate_min;
+	if (len != nr_pstates)
+		fwts_log_warning(fw, "Wrong number of pstates. Expected %d pstates, but found %d pstates\n",
+				 nr_pstates, len);
+
+	for (i = 0; i < nr_pstates; i++) {
+		if (pstates[i] > pstate_max) {
+			fwts_log_warning(fw, "Invalid Pstate id %d greater than max pstate %d\n",
+					  pstates[i], pstate_max);
+			ok = false;
+		}
+		if (pstates[i] < pstate_min) {
+			fwts_log_warning(fw, "Invalid Pstate id %d lesser than min pstate %d\n",
+					  pstates[i], pstate_min);
+			ok = false;
+		}
+	}
+
+	if (ok)
+		fwts_passed(fw, "CPU pstates are within the min and max limits");
+	else
+		fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUPstateLimitsTest",
+			     "Contains pstates beyond max and min limits");
+	return FWTS_OK;
+}
+
 static int sw_tests_possible(fwts_framework *fw)
 {
 	int i, online_cpus = 0;
@@ -861,6 +998,8 @@  static fwts_framework_minor_test cpufreq_tests[] = {
 	{ cpufreq_test_sw_any,		"CPU frequency SW_ANY control" },
 	{ cpufreq_test_sw_all,		"CPU frequency SW_ALL control" },
 	{ cpufreq_test_cpu_performance,	"CPU frequency performance tests." },
+	{ cpufreq_test_frequency_limits, "CPU frequency limits test" },
+	{ cpufreq_test_pstate_limits,	"CPU Pstate limits test (device_tree)" },
 	{ NULL, NULL }
 };