[Skiboot] [PATCH 2/2] OPAL/FWTS: Reserved memory DT validation tests.
ppaidipe
ppaidipe at linux.vnet.ibm.com
Sat Apr 8 18:06:20 AEST 2017
On 2017-04-07 23:41, Vaidyanathan Srinivasan wrote:
> * Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com> [2017-04-06
> 19:04:38]:
>
>> This patch includes the testcases for validating the
>> several DT properties for reserved memory.
>>
>> Properties:
>> reserved-names
>> reserved-ranges
>>
>> Nodes:
>> reserved-memory/
>> |sub-regions at unitaddress
>> |sub-regions at unitaddress
>>
>> If patch looks good, will send it to fwts upstream.
>>
>> Signed-off-by: Pridhiviraj Paidipeddi <ppaidipe at linux.vnet.ibm.com>
>> ---
>> src/Makefile.am | 1 +
>> src/opal/reserv_mem.c | 241
>> ++++++++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 242 insertions(+)
>> create mode 100644 src/opal/reserv_mem.c
>>
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index e833554..ffbda0c 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..7ae1edd
>> --- /dev/null
>> +++ b/src/opal/reserv_mem.c
>> @@ -0,0 +1,241 @@
>> +/*
>> + * 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 SLW_IMAGE_SIZE 0x100000UL /* 1MB */
>> +#define OCC_COMMON_SIZE 0x800000UL /* 8MB */
>> +#define HOMER_IMAGE_SIZE 0x400000UL /* 4MB */
>
> This can change, maybe there is a platform specific range/limit, but
> we should not check for a given size.
>
I checked with shilpa, she mentioned the sizes are fixed for these
three regions for both P8/P9. If it is fixed in size we can verify.
if it varies no need.
And verifying the size will be done only if we get the region name
from reserved-names property, else we will skip it.
>> +
>> +static const char *root_node = "/";
>> +
>> +static const char *reserv_mem_node = "/reserved-memory/";
>> +
>> +struct reserv_range {
>> + uint64_t start;
>> + uint64_t len;
>> +};
>> +
>
> Why do you need this if you are already storing these in reserv_region
> below.
yes, will remove it, not needed.
>
>> +struct reserv_region {
>> + const char *name;
>> + uint64_t start;
>> + uint64_t len;
>> +};
>> +
>> +struct reserv_region *regions;
>
> You setup the memory and free it in reserv_mem_limits_test(), why do
> you need this global?
>
yes, not required. will remove it.
>> +
>> +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 struct reserv_range *reserv_range;
>> + int offset, len, nr_regions;
>> +
>> + 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 */
>> + reserv_range = fdt_getprop(fw->fdt, offset, "reserved-ranges",
>> &len);
>> + if (reserv_range == 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(reserv_range[j].start);
>> + regions[j].len = (uint64_t )be64toh(reserv_range[j].len);
>> + fwts_log_info(fw,"region name %s ,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;
>> + }
>> +
>> + /* Validate different Known image fixed sizes here */
>> + if (strstr(regions[j].name, "homer-image")) {
>> + if(regions[j].len != HOMER_IMAGE_SIZE) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> "ImageSizeMismatch",
>> + "Mismatch in homer-image size, expected: 0x%lx,
>> actual: 0x%lx",
>> + HOMER_IMAGE_SIZE, regions[j].len);
>> + ok = false;
>> + }
>> + else
>> + fwts_log_info(fw, "homer-image size is validated");
>> + }
>> +
>> + if (strstr(regions[j].name, "slw-image")) {
>
> We may get a new name in P9 for CME/STOP engine.
>
We can add the new name once we know the name and size.
One more point here is all these image size verification will
be done if we get the corresponding region in regions structure.
If the regions are absent(As you mentioned Due to OCC didn't came up),
we will not verify. so there will not be any false positives
>> + if(regions[j].len != SLW_IMAGE_SIZE) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> "ImageSizeMismatch",
>> + "Mismatch in slw-image size, expected: 0x%lx, actual:
>> 0x%lx",
>> + SLW_IMAGE_SIZE, 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 != OCC_COMMON_SIZE) {
>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>> "ImageSizeMismatch",
>> + "Mismatch in occ-common-area size, expected: 0x%lx,
>> actual: 0x%lx",
>> + OCC_COMMON_SIZE, 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)
>> --
>> 2.7.4
>>
More information about the Skiboot
mailing list