Message ID | 1492751812-8016-2-git-send-email-ppaidipe@linux.vnet.ibm.com |
---|---|
State | Superseded |
Headers | show |
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@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 \ please use tabs instead of white spaces. thanks! > 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" FILENAME is a bit generic, how about CONFIG_FILENAME or something like that? > +#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; > +}; again, I'd prefer the fwts coding style in making these typedefs, e.g. typedef struct { const char *name; uint64_t start; uint64_t len; } reserv_region_t; and is there any reason to call it reserv_region_t; would reserve_region_t be more readable? and also please use tabs for indentation :-) > + > +/* Update new image names here */ > + > +struct config > +{ > + uint64_t occ_common; > + uint64_t homer; > + uint64_t slw; > +}; > + > +bool skip = false; could skip be made static? > + > +struct config get_config(fwts_framework *fw, char *filename) > +{ > + struct config configstruct; > + FILE *file = fopen(filename, "r"); > + char *p; > + uint64_t value; tabs instead of spaces for indentation please. I'd prefer: FILE *file; ... ... file = fopen(filename, "r"); if (!file) { > + > + if (file != NULL) > + { > + char line[MAXBUF]; > + int i = 0; When it comes to if/while statements etc, can you use the fwts coding style: if ( ... ) { ... } else { ... } while ( ... ) { } rather than if ( ... ) ... { } else { ... } amd while ( ... ) { } > + > + 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 analysis reported the following issue: CID 1374474 (#1-3 of 3): Uninitialized scalar variable (UNINIT)18. uninit_use: Using uninitialized value configstruct. Field configstruct.slw is uninitialized. > +} > + > +static char *make_message(const char *fmt, ...) { > + char *p = NULL; > + size_t size = 128; can size be const? > + va_list ap; > + > + if((p = malloc(size)) == NULL) > + return NULL; I'd prefer: p = malloc(size); if (!p) 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); returning an entire data structure and assigning it like this is a bit sub-optimal. I'd prefer to pass it by reference and perhaps have a sanity check: if (get_config(fw, FILENAME, &configstruct) != FWTS_OK) { /* do some error handling */ } > + > + 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) { spacing between if and ( please. if (!regions) { > + fwts_skipped(fw, > + "Unable to allocate memory for reserv_region structure"); > + return FWTS_SKIP; > + } > + > + for(int i = 0; i < nr_regions; i++) { spacing: 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; > + } > + > + > + for(int j=0; j < nr_regions; j++) { spacing for ( > + 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; > + } > + > + /* Validate different cases */ > + for(int j=0; j < nr_regions; j++) { spacing for ( > + > + char *buf=NULL; spacing: 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) { spacing if (buf == NULL) { actually, it is preferable to do: if (!buf) { ..there are quite a few of these throughout the code. > + 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) { spacing: if (regesions[j].len... > + 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; indentation of ok is out by 1 level > + } > + 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; indentation of ok is out by 1 level > + } > + 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; indentation of ok is out by 1 level > + } > + 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) >
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@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) >
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; + } + + + 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; + } + + /* 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)
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@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