<html><body>Hi Ivan, I think my comment was more on the scenario of say my x86_64 RHEL 7.2 system where libfdt is not installed, it seems the dt_* cases fail there for make check currently as well. So the question I have is what architecture/process should be done to include/exclude the tests which are failing the make check on all platforms consistently ? ===================================== Deb McLemore IBM OpenPower - IBM Systems (512) 286 9980 debmc@us.ibm.com debmc@linux.vnet.ibm.com - (plain text) ===================================== <img width="16" height="16" src="cid:1__=09BBF549DFDCF3358f9e8a93df938690918c09B@" border="0" alt="Inactive hide details for ivanhu ---06/22/2016 09:26:12 AM---On 2016年06月22日 19:52, Deborah McLemore wrote: > Hi Ivan,">ivanhu ---06/22/2016 09:26:12 AM---On 2016年06月22日 19:52, Deborah McLemore wrote: > Hi Ivan, From: ivanhu <ivan.hu@canonical.com> To: Deborah McLemore/Austin/IBM@IBMUS Cc: Deb McLemore <debmc@linux.vnet.ibm.com>, fwts-devel@lists.ubuntu.com Date: 06/22/2016 09:26 AM Subject: Re: [PATCH] V2 opal/prd_info: Add OPAL Processor Recovery Diagnostics <hr width="100%" size="2" align="left" noshade style="color:#8091A5; "> <tt> On 2016年06月22日 19:52, Deborah McLemore wrote: > Hi Ivan, > > I think I'm going to need some assistance on this one, the same > conditional build infrastructure was done for this patch as was done for > the dt_base and dt_sysinfo tests (these tests used the HAVE_LIBFDT) > check, but same > conditional type object code inclusion, so not sure how that passed (if > you did not have LIBFDT installed either). > I believe we do install the LIBFDT you can check the commit a889d696e65a80089061bb43bec731210d05a75f > FAIL: fwts-test/arg-show-progress-dialog-0001/test-0001.sh > FAIL: fwts-test/arg-show-progress-dialog-0001/test-0002.s > FAIL: fwts-test/arg-show-tests-0001/test-0001.sh > FAIL: fwts-test/arg-show-tests-0001/test-0002.sh > FAIL: fwts-test/arg-show-tests-full-0001/test-0001.sh > > fwts/src/Makefile.am > > if HAVE_LIBFDT > dt_tests = \ > devicetree/dt_base/dt_base.c \ > devicetree/dt_sysinfo/dt_sysinfo.c > endif > > if HAVE_ASM_OPAL_PRD_H > opal_tests = \ > opal/prd_info.c > endif > > ===================================== > Deb McLemore > IBM OpenPower - IBM Systems > (512) 286 9980 > > debmc@us.ibm.com > debmc@linux.vnet.ibm.com - (plain text) > ===================================== > > Inactive hide details for ivanhu ---06/22/2016 04:01:30 AM---Hi Deb, > This patch failed the regression test "make check" on thosivanhu > ---06/22/2016 04:01:30 AM---Hi Deb, This patch failed the regression > test "make check" on those machines > > From: ivanhu <ivan.hu@canonical.com> > To: Deb McLemore <debmc@linux.vnet.ibm.com> > Cc: fwts-devel@lists.ubuntu.com > Date: 06/22/2016 04:01 AM > Subject: Re: [PATCH] V2 opal/prd_info: Add OPAL Processor Recovery > Diagnostics > Sent by: fwts-devel-bounces@lists.ubuntu.com > > ------------------------------------------------------------------------ > > > > Hi Deb, > > This patch failed the regression test "make check" on those machines > which don't have the header, asm/opal-prd.h, could you help to check that? > > Cheers, > Ivan > > On 2016年06月21日 21:58, Deb McLemore wrote: > > We added a new capability to perform PRD integrity checks. > > The OPAL PRD firmware feature will be set when the device tree > > has the proper node configured. Runtime checks will use the > > opal-prd service to query the version of the opal-prd service to > > confirm operational state. > > > > Signed-off-by: Deb McLemore <debmc@linux.vnet.ibm.com> > > --- > > configure.ac | 2 + > > .../arg-show-tests-full-0001.log | 2 + > > src/Makefile.am | 8 +- > > src/lib/include/fwts_firmware.h | 22 ++- > > src/lib/include/fwts_pipeio.h | 1 + > > src/lib/src/Makefile.am | 3 +- > > src/lib/src/fwts_devicetree.c | 12 +- > > src/lib/src/fwts_firmware.c | 25 ++- > > src/lib/src/fwts_framework.c | 5 +- > > src/lib/src/fwts_pipeio.c | 29 ++++ > > src/opal/prd_info.c | 188 > +++++++++++++++++++++ > > 11 files changed, 277 insertions(+), 20 deletions(-) > > create mode 100644 src/opal/prd_info.c > > > > diff --git a/configure.ac b/configure.ac > > index 6ada384..e3e7512 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -63,6 +63,8 @@ > > AC_CHECK_HEADERS([json/json.h]) > > AC_CHECK_HEADERS([glib.h]) > > AC_CHECK_HEADERS([gio/gio.h]) > > + AC_CHECK_HEADERS([asm/opal-prd.h]) > > + AM_CONDITIONAL([HAVE_ASM_OPAL_PRD_H], [test > "x$ac_cv_header_asm_opal_prd_h" = "xyes"]) > > #AC_CHECK_LIB(pcre, pcre_compile) > > AC_SEARCH_LIBS([fdt_check_header], [fdt], [ > > AC_DEFINE([HAVE_LIBFDT], [1], [Define if we have libfdt]) > > diff --git > a/fwts-test/arg-show-tests-full-0001/arg-show-tests-full-0001.log > b/fwts-test/arg-show-tests-full-0001/arg-show-tests-full-0001.log > > index 6784ed3..695c5d6 100644 > > --- a/fwts-test/arg-show-tests-full-0001/arg-show-tests-full-0001.log > > +++ b/fwts-test/arg-show-tests-full-0001/arg-show-tests-full-0001.log > > @@ -663,6 +663,8 @@ Batch tests: > > PCI IRQ Routing Table test. > > pnp (1 test): > > PnP BIOS Support Installation structure test. > > + prd_info (1 test): > > + OPAL Processor Recovery Diagnostics Info > > rsdp (1 test): > > RSDP Root System Description Pointer test. > > rsdt (1 test): > > diff --git a/src/Makefile.am b/src/Makefile.am > > index 00cde32..b86fe52 100644 > > --- a/src/Makefile.am > > +++ b/src/Makefile.am > > @@ -26,6 +26,11 @@ dt_tests = \ > > devicetree/dt_sysinfo/dt_sysinfo.c > > endif > > > > +if HAVE_ASM_OPAL_PRD_H > > +opal_tests = \ > > + opal/prd_info.c > > +endif > > + > > # > > # fwts main + tests > > # > > @@ -142,7 +147,8 @@ fwts_SOURCES = main.c \ > > uefi/uefirtauthvar/uefirtauthvar.c \ > > uefi/esrtdump/esrtdump.c \ > > uefi/esrt/esrt.c \ > > - $(dt_tests) > > + $(dt_tests) \ > > + $(opal_tests) > > > > fwts_LDFLAGS = -lm `pkg-config --libs glib-2.0 gio-2.0` > > > > diff --git a/src/lib/include/fwts_firmware.h > b/src/lib/include/fwts_firmware.h > > index 9221afc..fa8f621 100644 > > --- a/src/lib/include/fwts_firmware.h > > +++ b/src/lib/include/fwts_firmware.h > > @@ -1,5 +1,6 @@ > > /* > > * Copyright (C) 2010-2016 Canonical > > + * Some of this work - Copyright (C) 2016 IBM > > * > > * This program is free software; you can redistribute it and/or > > * modify it under the terms of the GNU General Public License > > @@ -30,21 +31,24 @@ enum firmware_type { > > }; > > > > enum firmware_feature { > > - FWTS_FW_FEATURE_ACPI = 0x1, > > - FWTS_FW_FEATURE_DEVICETREE = 0x2, > > - FWTS_FW_FEATURE_IPMI = 0x4, > > - FWTS_FW_FEATURE_ALL = FWTS_FW_FEATURE_ACPI | > > - FWTS_FW_FEATURE_DEVICETREE | > > - FWTS_FW_FEATURE_IPMI > > + FWTS_FW_FEATURE_ACPI = 0x1, > > + FWTS_FW_FEATURE_DEVICETREE = 0x2, > > + FWTS_FW_FEATURE_IPMI = 0x4, > > + FWTS_FW_FEATURE_OPAL_PRD = 0x8, > > + FWTS_FW_FEATURE_ALL = FWTS_FW_FEATURE_ACPI | > > + FWTS_FW_FEATURE_DEVICETREE | > > + FWTS_FW_FEATURE_IPMI | > > + FWTS_FW_FEATURE_OPAL_PRD > > }; > > > > int fwts_firmware_detect(void); > > -int fwts_firmware_features(void); > > +int fwts_firmware_features(fwts_framework *fw); > > const char *fwts_firmware_feature_string(const int features); > > > > -static inline bool fwts_firmware_has_features(const int features) > > +static inline bool fwts_firmware_has_features(fwts_framework *fw, > > + const int features) > > { > > - return (fwts_firmware_features() & features) == features; > > + return (fwts_firmware_features(fw) & features) == features; > > } > > > > #endif > > diff --git a/src/lib/include/fwts_pipeio.h > b/src/lib/include/fwts_pipeio.h > > index 9a874dd..8c1b664 100644 > > --- a/src/lib/include/fwts_pipeio.h > > +++ b/src/lib/include/fwts_pipeio.h > > @@ -41,6 +41,7 @@ int fwts_pipe_close(const int fd, const pid_t pid); > > int fwts_pipe_close2(const int in_fd, const int out_fd, const > pid_t pid); > > int fwts_pipe_exec(const char *command, fwts_list **list, int > *status); > > int fwts_exec(const char *command, int *status); > > +int fwts_exec2(const char *command, char **output); > > int fwts_write_string_to_file(const fwts_framework *fw, FILE > *file, const char *str); > > int fwts_write_string_file(const fwts_framework *fw, const char > *file_name, const char *str); > > int fwts_read_file_first_line(const fwts_framework *fw, const > char *file_name, char **line); > > diff --git a/src/lib/src/Makefile.am b/src/lib/src/Makefile.am > > index e96b75f..1b8fd74 100644 > > --- a/src/lib/src/Makefile.am > > +++ b/src/lib/src/Makefile.am > > @@ -19,7 +19,8 @@ libfwts_la_LDFLAGS = \ > > libfwts_la_CPPFLAGS = $(AM_CPPFLAGS) -DACPI_DEBUG_OUTPUT > > > > if HAVE_LIBFDT > > -dt_sources = fwts_devicetree.c > > +dt_sources = \ > > + fwts_devicetree.c > > endif > > > > # > > diff --git a/src/lib/src/fwts_devicetree.c > b/src/lib/src/fwts_devicetree.c > > index 5978ab9..0c62953 100644 > > --- a/src/lib/src/fwts_devicetree.c > > +++ b/src/lib/src/fwts_devicetree.c > > @@ -20,7 +20,7 @@ > > #define _GNU_SOURCE > > > > #include <stdio.h> > > - > > +#include <libfdt.h> > > #include "fwts.h" > > > > static const char *devicetree_fs_path = > "/sys/firmware/devicetree/base"; > > @@ -32,10 +32,12 @@ int fwts_devicetree_read(fwts_framework *fwts) > > ssize_t len; > > pid_t pid; > > > > - if (!fwts_firmware_has_features(FWTS_FW_FEATURE_DEVICETREE)) > > + if (!fwts_firmware_has_features(fwts, > > + FWTS_FW_FEATURE_DEVICETREE)) > > return FWTS_OK; > > > > - rc = asprintf(&command, "dtc -I fs -O dtb %s", devicetree_fs_path); > > + rc = asprintf(&command, "dtc -I fs -O dtb %s", > > + devicetree_fs_path); > > if (rc < 0) > > return FWTS_ERROR; > > > > @@ -55,7 +57,8 @@ int fwts_devicetree_read(fwts_framework *fwts) > > status = fwts_pipe_close(fd, pid); > > > > if (!WIFEXITED(status) || WEXITSTATUS(status) != 0 || len == 0) { > > - fprintf(stderr, "Cannot read devicetree data: dtc failed\n"); > > + fprintf(stderr, > > + "Cannot read devicetree data: dtc failed\n"); > > free(data); > > return FWTS_ERROR; > > } > > @@ -64,4 +67,3 @@ int fwts_devicetree_read(fwts_framework *fwts) > > > > return FWTS_OK; > > } > > - > > diff --git a/src/lib/src/fwts_firmware.c b/src/lib/src/fwts_firmware.c > > index 972303a..f0bde63 100644 > > --- a/src/lib/src/fwts_firmware.c > > +++ b/src/lib/src/fwts_firmware.c > > @@ -1,5 +1,6 @@ > > /* > > * Copyright (C) 2010-2016 Canonical > > + * Some of this work - Copyright (C) 2016 IBM > > * > > * This program is free software; you can redistribute it and/or > > * modify it under the terms of the GNU General Public License > > @@ -23,6 +24,10 @@ > > > > #include "fwts.h" > > > > +#if defined HAVE_LIBFDT > > +#include <libfdt.h> > > +#endif > > + > > static enum firmware_type firmware_type; > > static bool firmware_type_valid; > > > > @@ -33,6 +38,7 @@ static struct { > > { FWTS_FW_FEATURE_ACPI, "ACPI" }, > > { FWTS_FW_FEATURE_DEVICETREE, "devicetree" }, > > { FWTS_FW_FEATURE_IPMI, "IPMI" }, > > + { FWTS_FW_FEATURE_OPAL_PRD, "OPAL PRD" }, > > }; > > > > /* > > @@ -60,7 +66,7 @@ int fwts_firmware_detect(void) > > return firmware_type; > > } > > > > -int fwts_firmware_features(void) > > +int fwts_firmware_features(fwts_framework *fw) > > { > > int features = 0; > > > > @@ -83,6 +89,21 @@ int fwts_firmware_features(void) > > if (!stat("/dev/ipmi0", &ipmi_statbuf)) > > features |= FWTS_FW_FEATURE_IPMI; > > > > + if (fw->fdt) { /* condition suppresses compile to use fw parm */ > > +#if defined HAVE_LIBFDT > > + /* conditionally check to make sure the timing has fdt set */ > > + int node; > > + node = fdt_path_offset(fw->fdt, > > + "/ibm,opal/diagnostics"); > > + if (node >= 0) { > > + if (!fdt_node_check_compatible(fw->fdt, node, > > + "ibm,opal-prd")) { > > + features |= FWTS_FW_FEATURE_OPAL_PRD; > > + } > > + } > > +#endif > > + } > > + > > return features; > > } > > > > @@ -90,7 +111,7 @@ const char *fwts_firmware_feature_string(const int > features) > > { > > const int n = FWTS_ARRAY_LEN(feature_names); > > const char sep[] = ", "; > > - static char str[60]; > > + static char str[70]; > > size_t len; > > char *p; > > int i; > > diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c > > index c53306a..1e75085 100644 > > --- a/src/lib/src/fwts_framework.c > > +++ b/src/lib/src/fwts_framework.c > > @@ -605,8 +605,9 @@ static int fwts_framework_run_test(fwts_framework > *fw, fwts_framework_test *test > > goto done; > > } > > > > - if (!fwts_firmware_has_features(test->fw_features)) { > > - int missing = test->fw_features & ~fwts_firmware_features(); > > + if (!fwts_firmware_has_features(fw, test->fw_features)) { > > + int missing = test->fw_features & > > + ~fwts_firmware_features(fw); > > fwts_log_info(fw, "Test skipped, missing features: %s", > > fwts_firmware_feature_string(missing)); > > fw->current_major_test->results.skipped += > > diff --git a/src/lib/src/fwts_pipeio.c b/src/lib/src/fwts_pipeio.c > > index b501f7b..f62621a 100644 > > --- a/src/lib/src/fwts_pipeio.c > > +++ b/src/lib/src/fwts_pipeio.c > > @@ -321,6 +321,35 @@ int fwts_exec(const char *command, int *status) > > } > > > > /* > > + * fwts_exec2() > > + * execute a command > > + * Return to the parent/caller the exit status from the command > > + * status is -1 if errors. > > + */ > > + > > +int fwts_exec2(const char *command, char **output) > > +{ > > + pid_t pid; > > + int status = -1, in_fd, out_fd; > > + ssize_t out_len; > > + > > + if (fwts_pipe_open_rw(command, &pid, > > + &in_fd, &out_fd) < 0) { > > + return -1; > > + } > > + > > + if (fwts_pipe_readwrite(in_fd, > > + command, sizeof(command), > > + out_fd, output, &out_len)) { > > + return -1; > > + } > > + > > + status = fwts_pipe_close2(in_fd, out_fd, pid); > > + > > + return status; > > +} > > + > > +/* > > * fwts_write_string_to_file() > > * write a string to a file pointer > > * Return FWTS_OK if writing worked, FWTS_ERROR if it failed. > > diff --git a/src/opal/prd_info.c b/src/opal/prd_info.c > > new file mode 100644 > > index 0000000..2f12625 > > --- /dev/null > > +++ b/src/opal/prd_info.c > > @@ -0,0 +1,188 @@ > > +/* > > + * Copyright (C) 2010-2016 Canonical > > + * Some of this work - Copyright (C) 2016 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 <sys/ioctl.h> > > + > > +#include <asm/opal-prd.h> > > + > > +#include "fwts.h" > > + > > +static const char *prd_devnode = "/dev/opal-prd"; > > + > > +bool prd_present(int fwts_prd_flags) { > > + return !access(prd_devnode, fwts_prd_flags); > > +} > > + > > +int prd_dev_query(fwts_framework *fw) > > +{ > > + > > + int fd = 0; > > + struct opal_prd_info info; > > + > > + if ((fd = open(prd_devnode, O_RDWR)) < 0) { > > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "OPAL PRD Info", > > + "Cannot get data from the OPAL PRD " > > + " device interface," > > + " check if opal-prd daemon may be in use " > > + "or check your user privileges."); > > + return FWTS_ERROR; > > + } > > + > > + memset(&info, 0, sizeof(info)); > > + > > + if (ioctl(fd, OPAL_PRD_GET_INFO, &info)) { > > + close(fd); > > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "OPAL PRD Info", > > + "Cannot get data from the" > > + " OPAL PRD device interface."); > > + return FWTS_ERROR; > > + } else { > > + fwts_log_info(fw, "OPAL PRD Version is %lu", > > + info.version); > > + close(fd); > > + return FWTS_OK; > > + } > > +} > > + > > +static int prd_service_check(fwts_framework *fw, int *restart) > > +{ > > + int rc = FWTS_OK, status = 0, stop_status = 0; > > + char *command = NULL; > > + char *output = NULL; > > + > > + command = "systemctl status opal-prd.service 2>&1"; > > + status = fwts_exec2(command, &output); > > + > > + if (output) > > + free(output); > > + > > + switch (status) { > > + case -1: /* status when nothing was successful */ > > + fwts_failed(fw, LOG_LEVEL_HIGH, "OPAL PRD Info", > > + "Attempt was made to stop the " > > + "opal-prd.service but was not " > > + "successful. Try to " > > + "\"sudo systemctl stop " > > + "opal-prd.service\" and retry."); > > + rc = FWTS_ERROR; > > + goto out; > > + case 0: /* "running" */ > > + command = "systemctl stop opal-prd.service 2>&1"; > > + stop_status = fwts_exec2(command, &output); > > + > > + if (output) > > + free(output); > > + > > + switch (stop_status) { > > + case 0: > > + *restart = 1; > > + break; > > + default: > > + fwts_failed(fw, LOG_LEVEL_HIGH, "OPAL PRD Info", > > + "Attempt was made to stop the " > > + "opal-prd.service but was not " > > + "successful. Try to " > > + "\"sudo systemctl stop " > > + "opal-prd.service\" and retry."); > > + rc = FWTS_ERROR; > > + goto out; > > + } > > + default: > > + break; > > + } > > + > > +out: > > + return rc; > > +} > > + > > +static int prd_restart(fwts_framework *fw) > > +{ > > + int status = 0; > > + char *command = NULL; > > + char *output = NULL; > > + > > + command = "systemctl start opal-prd.service 2>&1"; > > + status = fwts_exec2(command, &output); > > + > > + if (output) > > + free(output); > > + > > + if (status) { > > + fwts_log_info(fw, "OPAL PRD service (opal-prd.service)" > > + " was restarted after stopping it to allow " > > + "checks. Please re-check since processing " > > + "was not able to be confirmed, " > > + "\"sudo systemctl status opal-prd.service\""); > > + } else { > > + fwts_log_info(fw, "OPAL PRD service (opal-prd.service)" > > + " was restarted after stopping it to allow " > > + "checks. This is informational only, " > > + "no action needed."); > > + } > > + > > + return status; /* ignored by caller */ > > +} > > + > > +static int prd_info_test1(fwts_framework *fw) > > +{ > > + > > + int restart = 0; > > + > > + if (prd_service_check(fw, &restart)) { > > + /* failures logged in subroutine */ > > + return FWTS_ERROR; > > + } > > + > > + if (!prd_present(R_OK | W_OK)) { > > + fwts_failed(fw, LOG_LEVEL_CRITICAL, "OPAL PRD Info", > > + "Cannot read and write to the OPAL PRD" > > + " device interface," > > + " check your user privileges."); > > + return FWTS_ERROR; > > + } > > + > > + if (prd_dev_query(fw)) { > > + /* failures logged in subroutine */ > > + return FWTS_ERROR; > > + } > > + > > + if (restart) { > > + prd_restart(fw); /* ignore rc */ > > + } > > + > > + fwts_passed(fw, "OPAL PRD info passed."); > > + > > + return FWTS_OK; > > +} > > + > > +static fwts_framework_minor_test prd_info_tests[] = { > > + { prd_info_test1, "OPAL Processor Recovery Diagnostics Info" }, > > + { NULL, NULL } > > +}; > > + > > +static fwts_framework_ops prd_info_ops = { > > + .description = "OPAL Processor Recovery Diagnostics Info", > > + .minor_tests = prd_info_tests > > +}; > > + > > +FWTS_REGISTER_FEATURES("prd_info", &prd_info_ops, FWTS_TEST_EARLY, > > + FWTS_FLAG_BATCH | FWTS_FLAG_ROOT_PRIV, > > + FWTS_FW_FEATURE_OPAL_PRD) > > > > -- > fwts-devel mailing list > fwts-devel@lists.ubuntu.com > Modify settings or unsubscribe at: > </tt><tt><a href="https://lists.ubuntu.com/mailman/listinfo/fwts-devel">https://lists.ubuntu.com/mailman/listinfo/fwts-devel</a></tt> <tt>> > > </tt> </body></html>