[PATCH 1/2] fwts/opal: power management DT Validation tests.

ppaidipe ppaidipe at linux.vnet.ibm.com
Fri Apr 21 09:34:18 UTC 2017


On 2017-04-21 14:21, Colin Ian King wrote:
> Thanks for the patch.  Mainly coding style issues more than anything
> else. Can you fix those up and then it's good. Thanks!
> 
> Colin
> 

Thanks Colin for the review. Will work on the comments and
send the patch.

Thanks
Pridhiviraj



> On 21/04/17 06:16, Pridhiviraj Paidipeddi wrote:
>> 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.
>> 
>> 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 |  28 +++
>>  src/lib/src/fwts_devicetree.c     | 178 ++++++++++++++++++
>>  src/opal/power_mgmt_info.c        | 379 
>> ++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 604 insertions(+)
>>  create mode 100644 src/opal/power_mgmt_info.c
>> 
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index c1eb285..4cf6201 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                  \
> 
> Please use tabs rather than spaces in the Makefile.
> 
>>  	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,
>> +};
> 
> I'd prefer it if enums are typedef'd and using the typedef'd type, e.g.
> 
> typedef enum {
> 	proc_gen_unknown,
> 	...
> 	proc_gen_p9
> } proc_gen_t;
> 
> 
>> +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..e7c4b8c 100644
>> --- a/src/lib/include/fwts_devicetree.h
>> +++ b/src/lib/include/fwts_devicetree.h
>> @@ -42,6 +42,14 @@
>>  #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_property_read_u64_arr(void *fdt, int offset, const char 
>> *pname,
>> +                                  uint64_t *value, int *len);
>> +int fwts_dt_stringlist_count(fwts_framework *fw, const void *fdt,
>> +				 int nodeoffset, const char *property);
>> 
>>  #else /* !FWTS_HAS_DEVICETREE */
>>  static inline int fwts_devicetree_read(fwts_framework *fwts)
>> @@ -50,6 +58,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;
>> +}
>> +
>> +s
>> +			__attribute__((unused)),
>> +			int offset __attribute__((unused)),
>> +			const char *pname __attribute__((unused)),
>> +			int *value __attribute__((unused)),
>> +			int *len __attribute__((unused)))
>> +{
>> +	return FWTS_OK;
>> +}
> 
> For unused args, please use the FWTS_UNUSED macro and also use the fwts
> convention for many arguments as:
> 
> static inline int fwts_dt_property_read_u32_arr(
> 	void *fdt,
> 	int offset,
> 	const char *pname,
> 	int *value,
> 	int *len)
> {
> 	FWTS_UNUSED(fdt);
> 	FWTS_UNUSED(offset);
> 	etc..
> }
> 
> 
>>  #endif
>> 
>>  bool check_status_property_okay(fwts_framework *fw,
>> @@ -60,4 +86,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..0d7e81d 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,179 @@ char *hidewhitespace(char *name)
>>  	return name;
>> 
>>  }
>> +
>> +/* fwts_dt_property_read_u32 This function reads one u32 DT property 
>> */
>> +/* Returns FWTS_OK on success----*value will contain one u32         
>> */
>> +/*      FWTS_ERROR on error---*value will contain error code         
>> */
> 
> Can you use the fwts block comment style in your patches, e.g.
> 
> /*
>  * fwts_dt_property_read_u32 This function reads one u32 DT property
>  * 	Returns FWTS_OK on success: *value will contain one u32
>  * 	FWTS_ERROR on error:        *value will contain error code
>  */
> 
> 
>> +
>> +int fwts_dt_property_read_u32(void *fdt, int offset, const char 
>> *pname,
>> +			int *value)
> 
> the comment states it will return one u32 in *value but the return type
> is int * and not uint32_t, so there is a discrepancy there.
> 
> for long function arg declarations can you use the fwts style of:
> 
> 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;
>> +}
>> +
>> +/* This function reads DT property array of u32's          */
>> +/* If property exists, return FWTS_OK-----                 */
>> +/*                     *value contain full arrray of u32's */
>> +/* else, return FWTS_ERROR ----                            */
>> +/*  *value will contain error code which comes from *len   */
>> +
>> +int fwts_dt_property_read_u32_arr(void *fdt, int offset, const char 
>> *pname,
>> +				  int *value, int *len)
> 
> the comment states it will return one an array of u32's in value but 
> the
> return type is int * and not uint32_t, so there is a discrepancy there.
> 
>> +{
>> +	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]);
>> +	return FWTS_OK;
>> +}
>> +
>> +/* This function reads DT property array of u64's          */
>> +/* If property exists, return FWTS_OK-----                 */
>> +/*                     *value contain full arrray of u64's */
> 
> 			   		array not arrray
> 
>> +/* else, return FWTS_ERROR ----                            */
>> +/*  *value will contain error code which comes from *len   */
>> +
>> +int fwts_dt_property_read_u64_arr(void *fdt, int offset, const char 
>> *pname,
>> +                                  uint64_t *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(uint64_t);
>> +        for (i = 0; i < *len; i++)
>> +                value[i] = be64toh(buf[i]);
>> +        return FWTS_OK;
>> +}
>> +
>> +/* Get's the length of DT property string list */
>> +
>> +int fwts_dt_stringlist_count(fwts_framework *fw, const void *fdt,
>> +				int nodeoffset, const char *property)
>> +{
>> +    const char *list, *end;
>> +    int length, count = 0;
> 
> please replace all space indentation with tab. 1 tab per indentation
> layer to keep with the fwts coding style. thanks.
> 
>> +
>> +    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);
> 
> spacing between commas:
> 		 "Failed to get property %s rc %d", property, length);
> 
>> +        return FWTS_ERROR;
>> +    }
>> +
>> +    end = list + length;
>> +
>> +    while (list < end) {
>> +        length = strnlen(list, end - list) + 1;
>> +
>> +        /* Check if the last string isn't properly NUL-terminated. */
>> +        if (list + length > end) {
>> +            fwts_failed(fw, LOG_LEVEL_HIGH, "NotNULLTerminated",
>> +                     "Last string is not properly NULL terminated");
>> +            return FWTS_ERROR;
>> +        }
>> +
>> +        list += length;
>> +        count++;
>> +    }
>> +
>> +    return count;
>> +}
>> +
>> +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;
> 
> I'd use: const int mask = 0xFFFF0000;
> 
>> +    int pvr;
> 
> again, tabs for indentation please
> 
>> +
>> +    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..901e870
>> --- /dev/null
>> +++ b/src/opal/power_mgmt_info.c
>> @@ -0,0 +1,379 @@
>> +/*
>> + * 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/";
>> +
>> +/**
>> + * 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;
>> +}
> 
> how about:
> 
> static int cmp_negative_pstates(int a, int b)
> {
> 	return cmp_positive_pstates(b, a);
> }
> 
> 
>> +
>> +static 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;
>> +}
>> +
>> +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;
>> +    }
>> +
>> +    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) {
>> +        if (proc_gen == proc_gen_p8)
>> +                fwts_log_warning(fw, "Pstates range %d is not valid",
>> +                         nr_pstates);
>> +        else if (proc_gen == proc_gen_p9)
>> +                fwts_log_warning(fw, "More than 128 pstates in pstate 
>> table %d",
>> +                         nr_pstates);
>> +    }
> 
> Perhaps we should be using "P-states" in the warning text rather than
> pstates. Just a thought.
> 
>> +
>> +    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];
>> +    uint64_t pm_cr[CPUIDLE_STATE_MAX];
>> +    uint64_t pm_cr_mask[CPUIDLE_STATE_MAX];
>> +    bool has_stop_inst = false;
>> +    bool ok = true;
>> +    char *control_prop, *mask_prop;
>> +
>> +    switch (proc_gen) {
>> +    case proc_gen_p8:
>> +        has_stop_inst = false;
>> +        control_prop = "ibm,cpu-idle-state-pmicr";
>> +        mask_prop = "ibm,cpu-idle-state-pmicr-mask";
>> +        break;
>> +    case proc_gen_p9:
>> +        has_stop_inst = true;
>> +        control_prop = "ibm,cpu-idle-state-psscr";
>> +        mask_prop = "ibm,cpu-idle-state-psscr-mask";
>> +        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;
>> +    }
>> +
>> +    if (has_stop_inst) {
>> +        /* In P9 ibm,enabled-stop-levels present under 
>> /ibm,opal/power-mgt/ */
>> +        const int *buf;
>> +
>> +        buf = fdt_getprop(fw->fdt, offset, "ibm,enabled-stop-levels", 
>> &len);
>> +        if (buf == NULL) {
>> +            fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyMissing",
>> +                "ibm,enabled-stop-levels missing under %s", 
>> 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;
>> +
>> +    /* Validate pmicr and psscr value and mask bits */
>> +    ret = fwts_dt_property_read_u64_arr(fw->fdt, offset,
>> +                control_prop, pm_cr, &test_len);
>> +
>> +    if (validate_dt_prop_sizes(fw, "ibm,cpu-idle-state-flags", len,
>> +                    control_prop, test_len) != FWTS_OK)
>> +        ok = false;
>> +
>> +    ret = fwts_dt_property_read_u64_arr(fw->fdt, offset,
>> +                mask_prop, pm_cr_mask, &test_len);
>> +
>> +    if (validate_dt_prop_sizes(fw, "ibm,cpu-idle-state-flags", len,
>> +                    mask_prop, test_len) != FWTS_OK)
>> +        ok = false;
>> +
>> +    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 
>> Tests",
>> +    .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)
>> 




More information about the fwts-devel mailing list