[PATCH 2/2] fwts/opal: Reserved memory DT validation tests.
Colin Ian King
colin.king at canonical.com
Fri Apr 21 09:33:38 UTC 2017
Forgot to note there are some memory leaks too (see below)
On 21/04/17 06:16, Pridhiviraj Paidipeddi wrote:
> Properties:
> reserved-names
> reserved-ranges
>
> Nodes:
> reserved-memory/
> |sub-regions at unitaddress
> |sub-regions at unitaddress
>
> And also this testcase validates different fixed region sizes
> like occ-common-area, homer etc.
>
> These image sizes can be vary for different platforms. So
> in order to validate this additional check user need to
> provide a platform config file in below path.
>
> /usr/local/share/fwts/platform.conf
>
> The contents of file should be key=value for different image sizes
>
> cat /usr/local/share/fwts/platform.conf
> occ-common-area=800000
> homer-image=400000
> slw-image=100000
>
> If user won't provide this config file, this additional validation checks
> can be skipped.
>
> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
> ---
> src/Makefile.am | 1 +
> src/opal/reserv_mem.c | 301 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 302 insertions(+)
> create mode 100644 src/opal/reserv_mem.c
>
> diff --git a/src/Makefile.am b/src/Makefile.am
> index 4cf6201..c43503e 100644
> --- a/src/Makefile.am
> +++ b/src/Makefile.am
> @@ -148,6 +148,7 @@ fwts_SOURCES = main.c \
> opal/mtd_info.c \
> opal/prd_info.c \
> opal/power_mgmt_info.c \
> + opal/reserv_mem.c \
> pci/aspm/aspm.c \
> pci/crs/crs.c \
> pci/maxreadreq/maxreadreq.c \
> diff --git a/src/opal/reserv_mem.c b/src/opal/reserv_mem.c
> new file mode 100644
> index 0000000..7e38cce
> --- /dev/null
> +++ b/src/opal/reserv_mem.c
> @@ -0,0 +1,301 @@
> +/*
> + * 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 FILENAME "/usr/local/share/fwts/platform.conf"
> +#define MAXBUF 1024
> +#define DELIM "="
> +
> +static const char *root_node = "/";
> +
> +static const char *reserv_mem_node = "/reserved-memory/";
> +
> +struct reserv_region {
> + const char *name;
> + uint64_t start;
> + uint64_t len;
> +};
> +
> +/* Update new image names here */
> +
> +struct config
> +{
> + uint64_t occ_common;
> + uint64_t homer;
> + uint64_t slw;
> +};
> +
> +bool skip = false;
> +
> +struct config get_config(fwts_framework *fw, char *filename)
> +{
> + struct config configstruct;
> + FILE *file = fopen(filename, "r");
> + char *p;
> + uint64_t value;
> +
> + if (file != NULL)
> + {
> + char line[MAXBUF];
> + int i = 0;
> +
> + while(fgets(line, sizeof(line), file) != NULL)
> + {
> + char *cfline;
> +
> + cfline = strstr((char *)line,DELIM);
> + cfline = cfline + strlen(DELIM);
> + value = strtoul(cfline, &p, 16);
> +
> + if (strstr(line, "homer")) {
> + configstruct.homer = value;
> + }
> + else if (strstr(line, "occ-common-area")) {
> + configstruct.occ_common = value;
> + }
> + else if (strstr(line, "slw-image")) {
> + configstruct.slw = value;
> + }
> +
> + i++;
> + }
> + fclose(file);
> + }
> + else {
> + skip = true;
> + fwts_log_error(fw, "Platform config file doesn't exist,"
> + "skipping region size validation check");
> + }
> +
> + return configstruct;
> +}
> +
> +static char *make_message(const char *fmt, ...) {
> + char *p = NULL;
> + size_t size = 128;
> + va_list ap;
> +
> + if((p = malloc(size)) == NULL)
> + return NULL;
> +
> + va_start(ap, fmt);
> + vsnprintf(p, size, fmt, ap);
> + va_end(ap);
> + return p;
> +}
> +
> +static int reserv_mem_init(fwts_framework *fw)
> +{
> + if (fwts_firmware_detect() != FWTS_FIRMWARE_OPAL) {
> + fwts_skipped(fw,
> + "The firmware type detected was non OPAL"
> + "so skipping the OPAL Reserve Memory DT checks.");
> + return FWTS_SKIP;
> + }
> +
> + /* On an OPAL based system Device tree should be present */
> + if (!fw->fdt) {
> + fwts_failed(fw, LOG_LEVEL_HIGH, "NoDeviceTree",
> + "Device tree not found");
> + return FWTS_ERROR;
> + }
> +
> + return FWTS_OK;
> +}
> +
> +static int reserv_mem_limits_test(fwts_framework *fw)
> +{
> + bool ok = true;
> + char *region_names;
> + const uint64_t *ranges;
> + struct reserv_region *regions;
> + int offset, len, nr_regions;
> + struct config configstruct;
> +
> + configstruct = get_config(fw, FILENAME);
> +
> + offset = fdt_path_offset(fw->fdt, root_node);
> + if (offset < 0) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
> + "DT root node %s is missing", root_node);
> + return FWTS_ERROR;
> + }
> +
> + /* Get the number of memory reserved regions */
> + nr_regions = fwts_dt_stringlist_count(fw, fw->fdt, offset,
> + "reserved-names");
> +
> + /* Check for the reservd-names property */
> + region_names = (char *)fdt_getprop(fw->fdt, offset,
> + "reserved-names", &len);
> + if (!region_names) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyMissing",
> + "DT Property reserved-names is missing %s", fdt_strerror(len));
> + return FWTS_ERROR;
> + }
> +
> + regions = malloc(nr_regions*sizeof(struct reserv_region));
> + if(!regions) {
> + fwts_skipped(fw,
> + "Unable to allocate memory for reserv_region structure");
> + return FWTS_SKIP;
> + }
> +
> + for(int i = 0; i < nr_regions; i++) {
> + regions[i].name = strdup(region_names);
> + region_names += strlen(regions[i].name)+1;
> + }
> +
> + /* Check for the reserved-ranges property */
> + ranges = fdt_getprop(fw->fdt, offset, "reserved-ranges", &len);
> + if (ranges == NULL) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTPropertyMissing",
> + "DT Property reserved-ranges is missing %s", fdt_strerror(len));
> + return FWTS_ERROR;
Static analysis reports:
CID 1374472 (#1 of 2): Resource leak (RESOURCE_LEAK)
leaked_storage: Variable regions going out of scope leaks the storage it
points to
> + }
> +
> +
> + for(int j=0; j < nr_regions; j++) {
> + regions[j].start = (uint64_t )be64toh(ranges[2*j]);
> + regions[j].len = (uint64_t )be64toh(ranges[2*j+1]);
> + fwts_log_info(fw,"Region name %80s"
> + " start: 0x%08lx, len: 0x%08lx\n",
> + regions[j].name, regions[j].start, regions[j].len);
> + }
> +
> + offset = fdt_path_offset(fw->fdt, reserv_mem_node);
> + if (offset < 0) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
> + "reserve memory node %s is missing", reserv_mem_node);
> + return FWTS_ERROR;
CID 1374472 (#2 of 2): Resource leak (RESOURCE_LEAK)
leaked_storage: Variable regions going out of scope leaks the storage it
points to.
I'd recommend double checking all the return paths in the code below for
further potential leaks.
> + }
> +
> + /* Validate different cases */
> + for(int j=0; j < nr_regions; j++) {
> +
> + char *buf=NULL;
> +
> + /* Check for zero offset's */
> + if (regions[j].start == 0) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "ZeroStartAddress",
> + "memory region got zero start address");
> + ok = false;
> + }
> +
> + /* Check for zero region sizes */
> + if (regions[j].len == 0) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "ZeroRegionSize",
> + "memory region got zero size");
> + ok = false;
> + }
> +
> + /* Check corresponding memory region got reserved by looking
> + reserved-memory node entries */
> +
> + /* Form the reserved-memory sub nodes for all the regions*/
> + if (!strstr(regions[j].name, "@"))
> + buf = make_message("%s%s@%lx", reserv_mem_node, regions[j].name,
> + regions[j].start);
> + else
> + buf = make_message("/%s/%s", reserv_mem_node, regions[j].name);
> +
> + if(buf ==NULL) {
> + fwts_skipped(fw, "Unable to allocate memory for message buffer");
> + return FWTS_SKIP;
> + }
> +
> + offset = fdt_path_offset(fw->fdt, buf);
> + if (offset < 0) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "DTNodeMissing",
> + "reserve memory region node %s is missing", buf);
> + ok = false;
> + }
> +
> + if (skip)
> + continue;
> +
> + /* Validate different Known image fixed sizes here */
> + if (strstr(regions[j].name, "homer-image")) {
> + if(regions[j].len != configstruct.homer) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "ImageSizeMismatch",
> + "Mismatch in homer-image size, expected: 0x%lx, actual: 0x%lx",
> + configstruct.homer, regions[j].len);
> + ok = false;
> + }
> + else
> + fwts_log_info(fw, "homer-image size is validated");
> + }
> +
> + if (strstr(regions[j].name, "slw-image")) {
> + if(regions[j].len != configstruct.slw) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "ImageSizeMismatch",
> + "Mismatch in slw-image size, expected: 0x%lx, actual: 0x%lx",
> + configstruct.slw, regions[j].len);
> + ok = false;
> + }
> + else
> + fwts_log_info(fw, "slw-image size is validated");
> + }
> +
> + if (strstr(regions[j].name, "occ-common-area")) {
> + if(regions[j].len != configstruct.occ_common) {
> + fwts_failed(fw, LOG_LEVEL_MEDIUM, "ImageSizeMismatch",
> + "Mismatch in occ-common-area size,"
> + "expected: 0x%lx, actual: 0x%lx",
> + configstruct.occ_common, regions[j].len);
> + ok = false;
> + }
> + else
> + fwts_log_info(fw, "occ-common-area size is validated");
> + }
> + }
> +
> + if (ok)
> + fwts_passed(fw, "Reserved memory validation tests passed");
> + else
> + fwts_failed(fw, LOG_LEVEL_HIGH, "ReservMemTestFail",
> + "One or few Reserved Memory DT validation tests failed");
> + free(regions);
> + return FWTS_OK;
> +}
> +
> +static fwts_framework_minor_test reserv_mem_tests[] = {
> + { reserv_mem_limits_test, "OPAL Reserved memory DT Validation Info" },
> + { NULL, NULL }
> +};
> +
> +static fwts_framework_ops reserv_mem_tests_ops = {
> + .description = "OPAL Reserved memory DT Validation Test",
> + .init = reserv_mem_init,
> + .minor_tests = reserv_mem_tests
> +};
> +
> +FWTS_REGISTER_FEATURES("reserv_mem", &reserv_mem_tests_ops, FWTS_TEST_EARLY,
> + FWTS_FLAG_BATCH | FWTS_FLAG_ROOT_PRIV,
> + FWTS_FW_FEATURE_DEVICETREE)
>
More information about the fwts-devel
mailing list