[Skiboot] [PATCH 1/2] OPAL/FWTS: Power Management DT Validation tests
ppaidipe
ppaidipe at linux.vnet.ibm.com
Sat Apr 8 17:54:20 AEST 2017
Hi Vaidy
Thanks for the review.
On 2017-04-07 23:31, Vaidyanathan Srinivasan wrote:
> * Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com> [2017-04-06
> 19:03:52]:
>
>> Sending here to get some initial review.
>> Will send it to fwts upstream once patches are looking good.
>>
>> This patch contains testcases for below Power Processor
>> energey management subsystems.
>> a. cpuidle
>> b. cpufreq
>>
>> These testcases validate the device tree properties for these two
>> subsystems which are got exposed to linux from the system
>> firmware(OPAL).
>>
>> This patch is enhanced based on the initial patch developed by shilpa
>> for pstates.
>> https://lists.ubuntu.com/archives/fwts-devel/2016-May/007874.html
>>
>> Added cpuidle states DT Validation tests.
>
> Hi Pridhivi,
>
> You have covered good details in this test across PStates and cpuidle.
>
>
>> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
>> ---
>> src/Makefile.am | 1 +
>> src/lib/include/fwts_cpu.h | 18 ++
>> src/lib/include/fwts_devicetree.h | 29 ++++
>> src/lib/src/fwts_devicetree.c | 152 ++++++++++++++++
>> src/opal/power_mgmt_info.c | 357
>> ++++++++++++++++++++++++++++++++++++++
>> 5 files changed, 557 insertions(+)
>> create mode 100644 src/opal/power_mgmt_info.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index c1eb285..e833554 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -147,6 +147,7 @@ fwts_SOURCES = main.c \
>> kernel/version/version.c \
>> opal/mtd_info.c \
>> opal/prd_info.c \
>> + opal/power_mgmt_info.c \
>> pci/aspm/aspm.c \
>> pci/crs/crs.c \
>> pci/maxreadreq/maxreadreq.c \
>> diff --git a/src/lib/include/fwts_cpu.h b/src/lib/include/fwts_cpu.h
>> index 9ab2ccf..42cca25 100644
>> --- a/src/lib/include/fwts_cpu.h
>> +++ b/src/lib/include/fwts_cpu.h
>> @@ -33,6 +33,24 @@ typedef struct cpuinfo_x86 {
>> char *flags; /* String containing flags */
>> } fwts_cpuinfo_x86;
>>
>> +/* PowerPC Processor specific bits */
>> +/* PVR definitions */
>> +#define PVR_TYPE_P7 0x003f
>> +#define PVR_TYPE_P7P 0x004a
>> +#define PVR_TYPE_P8E 0x004b /* Murano */
>> +#define PVR_TYPE_P8 0x004d /* Venice */
>> +#define PVR_TYPE_P8NVL 0x004c /* Naples */
>> +#define PVR_TYPE_P9 0x004e
>> +
>> +/* Processor generation */
>> +enum proc_gen {
>> + proc_gen_unknown,
>> + proc_gen_p7, /* P7 and P7+ */
>> + proc_gen_p8,
>> + proc_gen_p9,
>> +};
>> +extern enum proc_gen proc_gen;
>> +
>> typedef struct cpu_benchmark_result {
>> bool cycles_valid;
>> uint64_t loops;
>> diff --git a/src/lib/include/fwts_devicetree.h
>> b/src/lib/include/fwts_devicetree.h
>> index 662f6ec..8ad867a 100644
>> --- a/src/lib/include/fwts_devicetree.h
>> +++ b/src/lib/include/fwts_devicetree.h
>> @@ -42,6 +42,15 @@
>> #if FWTS_HAS_DEVICETREE
>>
>> int fwts_devicetree_read(fwts_framework *fwts);
>> +int fwts_dt_property_read_u32(void *fdt, int offset, const char
>> *pname,
>> + int *value);
>> +int fwts_dt_property_read_u32_arr(void *fdt, int offset, const char
>> *pname,
>> + int *value, int *len);
>> +int fwts_dt_stringlist_count(fwts_framework *fw, const void *fdt,
>> + int nodeoffset, const char *property);
>> +int validate_dt_prop_sizes(fwts_framework *fw,
>> + const char *prop1, int prop1_len,
>> + const char *prop2, int prop2_len);
>>
>> #else /* !FWTS_HAS_DEVICETREE */
>> static inline int fwts_devicetree_read(fwts_framework *fwts)
>> @@ -50,6 +59,24 @@ static inline int
>> fwts_devicetree_read(fwts_framework *fwts)
>>
>> return FWTS_OK;
>> }
>> +
>> +static inline int fwts_dt_property_read_u32(void *fdt
>> __attribute__((unused)),
>> + int offset __attribute__((unused)),
>> + const char *pname __attribute__((unused)),
>> + int *value __attribute__((unused)))
>> +{
>> + return FWTS_OK;
>> +}
>> +
>> +static inline int fwts_dt_property_read_u32_arr(void *fdt
>> + __attribute__((unused)),
>> + int offset __attribute__((unused)),
>> + const char *pname __attribute__((unused)),
>> + int *value __attribute__((unused)),
>> + int *len __attribute__((unused)))
>> +{
>> + return FWTS_OK;
>> +}
>> #endif
>>
>> bool check_status_property_okay(fwts_framework *fw,
>> @@ -60,4 +87,6 @@ int check_property_printable(fwts_framework *fw,
>>
>> char *hidewhitespace(char *name);
>>
>> +int get_proc_gen(fwts_framework *fw);
>> +
>> #endif
>> diff --git a/src/lib/src/fwts_devicetree.c
>> b/src/lib/src/fwts_devicetree.c
>> index bf5686a..f86cd13 100644
>> --- a/src/lib/src/fwts_devicetree.c
>> +++ b/src/lib/src/fwts_devicetree.c
>> @@ -26,6 +26,8 @@
>>
>> #include <libfdt.h>
>>
>> +enum proc_gen proc_gen;
>> +
>> int fwts_devicetree_read(fwts_framework *fwts)
>> {
>> char *command, *data = NULL;
>> @@ -171,3 +173,153 @@ char *hidewhitespace(char *name)
>> return name;
>>
>> }
>> +
>> +int fwts_dt_property_read_u32(void *fdt, int offset, const char
>> *pname,
>> + int *value)
>> +{
>> + int len;
>> + const int *buf;
>> +
>> + buf = fdt_getprop(fdt, offset, pname, &len);
>> + if (buf == NULL) {
>> + *value = len;
>> + return FWTS_ERROR;
>> + }
>> + *value = be32toh(*buf);
>> + return FWTS_OK;
>> +}
>> +
>> +int fwts_dt_property_read_u32_arr(void *fdt, int offset, const char
>> *pname,
>> + int *value, int *len)
>> +{
>> + int i;
>> + const int *buf;
>> +
>> + buf = fdt_getprop(fdt, offset, pname, len);
>> + if (buf == NULL) {
>> + *value = *len;
>> + return FWTS_ERROR;
>> + }
>> +
>> + *len = *len / sizeof(int);
>> + for (i = 0; i < *len; i++)
>> + value[i] = be32toh(buf[i]);
>
> Can you explain what you expect in value from caller? Is it one u32
> or the full array?
>
> I think on failure sending *value = *len to caller will confuse the
> caller.
>
> fdt_getprop will return len in bytes or number of u32?
>
>
fdt_getprop stores the both length and return error code in len based on
property existence.
if property exists:
it returns pointer to the property's value
if len is non-NULL, *len contains the length of the property
value (>=0) in bytes
else
it returns NULL, on error
if len is non-NULL, *len contains an error code (<0)
so similar way
fwts_dt_property_read_u32_arr
if property exists this function will return FWTS_OK---> *value will
contain full arrray of u32's.
else function will return FWTS_ERROR ----> *value will contain error
code which comes from *len
fwts_dt_property_read_u32
Returns FWTS_OK on success----*value will contain one u32
FWTS_ERROR on error---*value will contain error code
I will add these on top of functions to understand
usage of the callers.
>> + return FWTS_OK;
>> +}
>> +
>> +int fwts_dt_stringlist_count(fwts_framework *fw, const void *fdt,
>> + int nodeoffset, const char *property)
>> +{
>> + const char *list, *end;
>> + int length, count = 0;
>> +
>> + list = fdt_getprop(fdt, nodeoffset, property, &length);
>> + if (!list) {
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "PropertyNotFound",
>> + "Failed to get property %s rc %d", property,length);
>> + return FWTS_ERROR;
>> + }
>> +
>> + end = list + length;
>> +
>> + while (list < end) {
>> + length = strnlen(list, end - list) + 1;
>> +
>> + /* Abort if the last string isn't properly NUL-terminated. */
>> + if (list + length > end)
>> + return -1;
>
> strnlen() will make this condition false. We will not get a length
> past the end.
>
> Or you are expecting we will get the strnlen's maxlen (end - list) and
> this is how you will abort the test case?
>
>
Here comment is misleading, we should return error if it is not properly
null terminated. Will change the error path accordingly.
yes, strnlen will return length atmost of (end - list).
>> +
>> + list += length;
>> + count++;
>> + }
>> +
>> + return count;
>> +}
>> +
>> +int validate_dt_prop_sizes(fwts_framework *fw,
>> + const char *prop1, int prop1_len,
>> + const char *prop2, int prop2_len)
>> +{
>> + if (prop1_len == prop2_len)
>> + return FWTS_OK;
>> +
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "SizeMismatch",
>> + "array sizes don't match for %s len %d and %s len %d\n",
>> + prop1, prop1_len, prop2, prop2_len);
>> + return FWTS_ERROR;
>
> Do you need this helper routine to just compare the given lengths?
>
yeah, it may not required. But will reduce the duplicated code
for multiple times checking. We will keep it in our test case
power_mgmt_info.c file, as this helper may not be suitable for
device tree library
>> +}
>> +
>> +static int get_cpu_version(fwts_framework *fw, int *value)
>> +{
>> + const char *cpus_path = "/cpus/";
>> + int offset;
>> + int cpu_version;int ret;
>> +
>> + if (!fw->fdt) {
>> + fwts_skipped(fw, "Device tree not found");
>> + return FWTS_SKIP;
>> + }
>> +
>> + offset = fdt_path_offset(fw->fdt, cpus_path);
>> + if (offset < 0) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
>> + "/cpus node is missing");
>> + return FWTS_ERROR;
>> + }
>> +
>> + offset = fdt_node_offset_by_prop_value(fw->fdt, -1,
>> "device_type",
>> + "cpu", sizeof("cpu"));
>> + if (offset < 0) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
>> + "cpu node is missing");
>> + return FWTS_ERROR;
>> + }
>> +
>> + ret = fwts_dt_property_read_u32(fw->fdt, offset,
>> "cpu-version",
>> + &cpu_version);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> "DTPropertyReadError",
>> + "Failed to read property cpu-version %s",
>> + fdt_strerror(cpu_version));
>> + return FWTS_ERROR;
>> + }
>> + *value = cpu_version;
>> + return FWTS_OK;
>> +}
>> +
>> +int get_proc_gen(fwts_framework *fw)
>> +{
>> + int version; int ret;
>> + int mask = 0xFFFF0000;
>> + int pvr;
>> +
>> + ret = get_cpu_version(fw, &version);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "DTNoCPUVersion",
>> + "Not able to get the CPU version");
>> + return FWTS_ERROR;
>> + }
>> +
>> + pvr = (mask & version) >> 16;
>> + switch(pvr) {
>> + /* Get CPU family and other flags based on PVR */
>> + case PVR_TYPE_P7:
>> + case PVR_TYPE_P7P:
>> + proc_gen = proc_gen_p7;
>> + break;
>> + case PVR_TYPE_P8E:
>> + case PVR_TYPE_P8:
>> + proc_gen = proc_gen_p8;
>> + break;
>> + case PVR_TYPE_P8NVL:
>> + proc_gen = proc_gen_p8;
>> + break;
>> + case PVR_TYPE_P9:
>> + proc_gen = proc_gen_p9;
>> + break;
>> + default:
>> + proc_gen = proc_gen_unknown;
>> + }
>> +
>> + return FWTS_OK;
>> +}
>> diff --git a/src/opal/power_mgmt_info.c b/src/opal/power_mgmt_info.c
>> new file mode 100644
>> index 0000000..93a420b
>> --- /dev/null
>> +++ b/src/opal/power_mgmt_info.c
>> @@ -0,0 +1,357 @@
>> +/*
>> + * Copyright (C) 2010-2017 Canonical
>> + * Some of this work - Copyright (C) 2016-2017 IBM
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License
>> + * as published by the Free Software Foundation; either version 2
>> + * of the License, or (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program; if not, write to the Free Software
>> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
>> 02110-1301, USA.
>> + *
>> + */
>> +
>> +#include <fcntl.h>
>> +#include <stdlib.h>
>> +#include <sys/ioctl.h>
>> +
>> +#include "fwts.h"
>> +
>> +#ifdef HAVE_LIBFDT
>> +#include <libfdt.h>
>> +#endif
>> +
>> +#define MAX_PSTATES 256
>> +
>> +#define CPUIDLE_STATE_MAX 10
>> +
>> +enum proc_gen proc_gen;
>> +
>> +static const char *power_mgt_path = "/ibm,opal/power-mgt/";
>> +
>> +static const char *root_node = "/";
>> +
>> +/**
>> + * cmp_pstates: Compares the given two pstates and determines which
>> + * among them is associated with a higher pstate.
>> + *
>> + * @a, at b: The pstate ids of the pstates being compared.
>> + *
>> + * Returns: -1 : If pstate associated with @a is smaller than
>> + * the pstate associated with @b.
>> + * 0 : If pstates associated with @a and @b are equal.
>> + * 1 : If pstate associated with @a is greater than
>> + * the pstate associated with @b.
>> + */
>> +static int (*cmp_pstates)(int a, int b);
>> +
>> +static int cmp_positive_pstates(int a, int b)
>> +{
>> + if (a > b)
>> + return -1;
>> + else if (a < b)
>> + return 1;
>> +
>> + return 0;
>> +}
>> +
>> +static int cmp_negative_pstates(int a, int b)
>> +{
>> + if (a < b)
>> + return -1;
>> + else if (a > b)
>> + return 1;
>> +
>> + return 0;
>> +}
>
> This will be a challenge. On P8, PState is signed value with +ve and
> -ve 8bit values. But on P9 the spec removed this confusion and makes
> PState a 8bit unsigned value with 0 as max and 254 (0xfe) and min.
> 0xff is reserved.
>
you mean on P9 above helpers will break?
I guess even if we move signed pstates to unsigned pstates
the above helpers still work.
or let me know if it breaks?
>> +static int power_mgmt_init(fwts_framework *fw)
>> +{
>> + int ret;
>> +
>> + if (fwts_firmware_detect() != FWTS_FIRMWARE_OPAL) {
>> + fwts_skipped(fw,
>> + "The firmware type detected was non OPAL"
>> + "so skipping the OPAL Power Management DT checks.");
>> + return FWTS_SKIP;
>> + }
>> +
>> + if (!fw->fdt) {
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "NoDeviceTree",
>> + "Device tree not found");
>> + return FWTS_ERROR;
>> + }
>> +
>> + ret = get_proc_gen(fw);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "ProcGenFail",
>> + "Failed to get the Processor generation");
>> + return FWTS_ERROR;
>> + }
>> +
>> + return FWTS_OK;
>> +}
>> +
>> +
>> +static int pstate_limits_test(fwts_framework *fw)
>> +{
>> + int pstate_min, pstate_max, pstates[MAX_PSTATES], nr_pstates;
>> + bool ok = true;
>> + int offset, len, ret, i;
>> +
>> + switch (proc_gen) {
>> + case proc_gen_p8:
>> + cmp_pstates = cmp_negative_pstates;
>> + break;
>> + case proc_gen_p9:
>> + cmp_pstates = cmp_positive_pstates;
>> + break;
>> + default:
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "UnknownProcessorChip",
>> + "Unknown processor generation %d", proc_gen);
>> + return FWTS_ERROR;
>> + }
>
> Lets do this way... check for monotonicity in Pstate and note min and
> max. Depending on P8 vs P9, you can verify which direction the value
> should have increased.
>
I think this one is implemented by verifying the monotonicity
from Pmax to Pmin with the use of above helpers.
>> +
>> + offset = fdt_path_offset(fw->fdt, power_mgt_path);
>> + if (offset < 0) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
>> + "power management node %s is missing", power_mgt_path);
>> + return FWTS_ERROR;
>> + }
>> +
>> + ret = fwts_dt_property_read_u32(fw->fdt, offset,
>> "ibm,pstate-min",
>> + &pstate_min);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyReadError",
>> + "Failed to read property ibm,pstate-min %s",
>> + fdt_strerror(pstate_min));
>> + return FWTS_ERROR;
>> + }
>> +
>> + ret = fwts_dt_property_read_u32(fw->fdt, offset,
>> "ibm,pstate-max",
>> + &pstate_max);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyReadError",
>> + "Failed to read property ibm,pstate-max %s",
>> + fdt_strerror(pstate_max));
>> + return FWTS_ERROR;
>> + }
>> +
>> + ret = fwts_dt_property_read_u32_arr(fw->fdt, offset,
>> "ibm,pstate-ids",
>> + pstates, &len);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyReadError",
>> + "Failed to read property ibm,pstate-ids %s",
>> + fdt_strerror(len));
>> + return FWTS_ERROR;
>> + }
>> +
>> + nr_pstates = abs(pstate_max - pstate_min + 1);
>> +
>> + if (nr_pstates <= 1 || nr_pstates > 128)
>> + fwts_log_warning(fw, "Pstates range %d is not valid",
>> + nr_pstates);
>
> As per spec p9 can have more than 128, since the PState ID in
> unsigned. But we want to know if it is more than 128. It will
> certainly be an error.
>
So i should verify on P8 it should'nt be greater than 128,
on P9 it shouldn't be greater than 255?
>> +
>> + if (len != nr_pstates)
>> + fwts_log_warning(fw, "Wrong number of pstates."
>> + "Expected %d pstates, found %d pstates",
>> + nr_pstates, len);
>> +
>> + for (i = 0; i < nr_pstates; i++) {
>> + if (cmp_pstates(pstate_max, pstates[i]) < 0) {
>> + fwts_log_warning(fw, "Invalid Pstate id %d"
>> + "greater than max pstate %d",
>> + pstates[i], pstate_max);
>> + ok = false;
>> + }
>> + if (cmp_pstates(pstates[i], pstate_min) < 0) {
>> + fwts_log_warning(fw, "Invalid Pstate id %d"
>> + "lesser than min pstate %d",
>> + pstates[i], pstate_min);
>> + ok = false;
>> + }
>> + }
>> +
>> + /* Pstates should be in monotonic descending order */
>> + for (i = 0; i < nr_pstates; i++) {
>> + if ((i == 0) && (cmp_pstates(pstates[i], pstate_max) != 0)) {
>> + fwts_log_warning(fw, "Pstates mismatch: Expected Pmin
>> %d,"
>> + "Actual Pmin %d",
>> + pstate_min, pstates[i]);
>> + ok = false;
>> + }
>> + else if((i == nr_pstates-1) &&
>> + (cmp_pstates(pstates[i], pstate_min) != 0)) {
>> + fwts_log_warning(fw, "Pstates mismatch: Expected Pmax
>> %d,"
>> + "Actual Pmax %d",
>> + pstate_max, pstates[i]);
>> + ok = false;
>> + }
>> + else {
>> + int previous_pstate;
>> + previous_pstate = pstates[i-1];
>> + if (cmp_pstates(pstates[i], previous_pstate) > 0) {
>> + fwts_log_warning(fw, "Non monotonicity observed,"
>> + "Pstate %d greater then previous Pstate %d",
>> + pstates[i], previous_pstate);
>> + ok = false;
>> + }
>> + }
>> + }
>> +
>> + if (ok)
>> + fwts_passed(fw, "CPU Frequency pstates are validated");
>> + else
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "CPUPstateLimitsTestFail",
>> + "One or few CPU Pstates DT validation tests failed");
>> + return FWTS_OK;
>> +}
>> +
>> +static int cpuidle_states_test(fwts_framework *fw)
>> +{
>> + int offset, len, test_len, ret;
>> + int latency_ns[CPUIDLE_STATE_MAX];
>> + int residency_ns[CPUIDLE_STATE_MAX];
>> + int flags[CPUIDLE_STATE_MAX];
>> + bool has_stop_inst = false;
>> + char *supported_states;
>> + bool ok = true;
>> +
>> + switch (proc_gen) {
>> + case proc_gen_p8:
>> + has_stop_inst = false;
>> + supported_states = "ibm,enabled-idle-states";
>> + break;
>> + case proc_gen_p9:
>> + has_stop_inst = true;
>> + supported_states = "ibm,enabled-stop-levels";
>
> This ibm,enabled-stop-levels is from hostboot (HDAT) to OPAL.
> OPAL->Linux should look at ibm,enabled-idle-states only as in P8.
>
> Also the format between the 2 properties are different.
>
ibm,enabled-idle-states is an array of strings
What is the format for ibm,enabled-stop-levels?
Here we need the length of stop levels to compare
other properties lengths in p9.
>> + break;
>> + default:
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "UnknownProcessorChip",
>> + "Unknown processor generation %d", proc_gen);
>> + return FWTS_ERROR;
>> + }
>> +
>> + offset = fdt_path_offset(fw->fdt, power_mgt_path);
>> + if (offset < 0) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
>> + "power management node %s is missing",
>> power_mgt_path);
>> + return FWTS_ERROR;
>> + }
>> +
>> + /* Validate ibm,cpu-idle-state-flags property */
>> + ret = fwts_dt_property_read_u32_arr(fw->fdt, offset,
>> + "ibm,cpu-idle-state-flags", flags, &len);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyReadError",
>> + "Failed to read property ibm,cpu-idle-state-flags
>> %s",
>> + fdt_strerror(len));
>> + return FWTS_ERROR;
>> + }
>> +
>> + if (len < 0) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNoIdleStates",
>> + "No idle states found in DT");
>> + return FWTS_ERROR;
>> + }
>> +
>> + if (len > CPUIDLE_STATE_MAX-1) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTMoreIdleStates",
>> + "More idle states found in DT than the
>> expected");
>> + return FWTS_ERROR;
>> + }
>> +
>> + /* Validate ibm,cpu-idle-state-latencies-ns property */
>> + ret = fwts_dt_property_read_u32_arr(fw->fdt, offset,
>> + "ibm,cpu-idle-state-latencies-ns", latency_ns,
>> &test_len);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyReadError",
>> + "Failed to read property
>> ibm,cpu-idle-state-latencies-ns %s",
>> + fdt_strerror(test_len));
>> + return FWTS_ERROR;
>> + }
>> +
>> + if (validate_dt_prop_sizes(fw, "ibm,cpu-idle-state-flags", len,
>> + "ibm,cpu-idle-state-latencies-ns", test_len) !=
>> FWTS_OK)
>> + ok = false;
>> +
>> + /* Validate ibm,cpu-idle-state-names property */
>> + test_len = fwts_dt_stringlist_count(fw, fw->fdt, offset,
>> + "ibm,cpu-idle-state-names");
>> + if (test_len > 0) {
>> + if (validate_dt_prop_sizes(fw, "ibm,cpu-idle-state-flags",
>> len,
>> + "ibm,cpu-idle-state-names", test_len) !=
>> FWTS_OK)
>> + ok = false;
>> + }
>> +
>> + /* Validate ibm,cpu-idle-state-residency-ns property */
>> + ret = fwts_dt_property_read_u32_arr(fw->fdt, offset,
>> + "ibm,cpu-idle-state-residency-ns", residency_ns,
>> &test_len);
>> + if (ret != FWTS_OK) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyReadError",
>> + "Failed to read property
>> ibm,cpu-idle-state-residency-ns %s",
>> + fdt_strerror(test_len));
>> + return FWTS_ERROR;
>> + }
>> +
>> + if (validate_dt_prop_sizes(fw, "ibm,cpu-idle-state-flags", len,
>> + "ibm,cpu-idle-state-residency-ns", test_len) !=
>> FWTS_OK)
>> + ok = false;
>> +
>> +
>> + if (has_stop_inst) {
>> + /* In P9 ibm,enabled-stop-levels present under
>> /ibm,opal/power-mgt/ */
>> + test_len = fwts_dt_stringlist_count(fw, fw->fdt, offset,
>> + supported_states);
>> + }
>> + else {
>> + /* In P8 ibm,enabled-idle-states present under root node*/
>> + offset = fdt_path_offset(fw->fdt, root_node);
>> + if (offset < 0) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTRootNodeMissing",
>> + "root node %s is missing", root_node);
>> + return FWTS_ERROR;
>> + }
>> + test_len = fwts_dt_stringlist_count(fw, fw->fdt, offset,
>> + supported_states);
>> + }
>> +
>> + if (test_len > 0) {
>> + if (validate_dt_prop_sizes(fw, "ibm,cpu-idle-state-flags",
>> len,
>> + supported_states, test_len) !=
>> FWTS_OK)
>> + ok = false;
>> + }
>> +
>> + /* TODO: add tests for pmicr and psscr value and mask bits */
>> +
>> + if (ok)
>> + fwts_passed(fw, "CPU IDLE States are validated");
>> + else
>> + fwts_failed(fw, LOG_LEVEL_HIGH, "CPUIDLEStatesFail",
>> + "One or few CPU IDLE DT Validation tests failed");
>> +
>> + return FWTS_OK;
>> +}
>> +
>> +static fwts_framework_minor_test power_mgmt_tests[] = {
>> + { pstate_limits_test, "OPAL Processor Frequency States Info" },
>> + { cpuidle_states_test, "OPAL Processor Idle States Info" },
>> + { NULL, NULL }
>> +};
>> +
>> +static fwts_framework_ops power_mgmt_tests_ops = {
>> + .description = "OPAL Processor Power Management DT Validation",
>> + .init = power_mgmt_init,
>> + .minor_tests = power_mgmt_tests
>> +};
>> +
>> +FWTS_REGISTER_FEATURES("power_mgmt", &power_mgmt_tests_ops,
>> FWTS_TEST_EARLY,
>> + FWTS_FLAG_BATCH | FWTS_FLAG_ROOT_PRIV,
>> + FWTS_FW_FEATURE_DEVICETREE)
>
> Thanks for adding the test case for all the fine details in the device
> tree.
>
> --Vaidy
More information about the Skiboot
mailing list