[PATCH] V2 Introduce IPMI BMC Info
Deb McLemore
debmc at linux.vnet.ibm.com
Wed May 4 12:53:35 UTC 2016
Hi Cedric,
Thank you for the review and comments.
In future FWTS IPMI updates we can restructure the mappings for the IPMI
responses,
initial thoughts were that the fwts generic ipmi response would be the
initial query
for the IPMI_GET_DEVICE_ID_CMD and then any other queries for deeper
probes we would build union structures, etc and develop a usable
overlay, etc.
The 5 second timeout in the poll for the FWTS tool was to allow more than
sufficient time,if there is not any delay then this timeout is not hit
normally, but in
the FWTS tooling having a loop with retries seemed a bit much and
of course never knowing how long and how many times to retry.
On 05/04/2016 07:42 AM, Cédric Le Goater wrote:
> Hello Deb,
>
> Some comments,
>
> On 05/04/2016 04:14 AM, Deb McLemore wrote:
>> This feature adds the foundation to perform an IPMI BMC Info
>> check that will determine if a Host is capable of
>> IPMI messaging and if so will perform a basic IPMI message
>> exchange to determine the version of IPMI running
>> on the hardware. In the future the IPMI infrastructure
>> can be used to further interrogate the FRU Inventory and other
>> Sensors to help correlate data and surface any
>> discrepancies on inventory or hardware characteristics.
>>
>> Signed-off-by: Deb McLemore <debmc at linux.vnet.ibm.com>
>> ---
>> .../arg-show-tests-0001/arg-show-tests-0001.log | 1 +
>> .../arg-show-tests-full-0001.log | 2 +
>> src/Makefile.am | 1 +
>> src/ipmi/bmc/bmc_info.c | 76 +++++++++++++
>> src/lib/include/fwts.h | 1 +
>> src/lib/include/fwts_firmware.h | 4 +-
>> src/lib/include/fwts_ipmi.h | 52 +++++++++
>> src/lib/src/Makefile.am | 1 +
>> src/lib/src/fwts_firmware.c | 8 +-
>> src/lib/src/fwts_framework.c | 20 ++--
>> src/lib/src/fwts_ipmi.c | 120 +++++++++++++++++++++
>> 11 files changed, 273 insertions(+), 13 deletions(-)
>> create mode 100644 src/ipmi/bmc/bmc_info.c
>> create mode 100644 src/lib/include/fwts_ipmi.h
>> create mode 100644 src/lib/src/fwts_ipmi.c
>>
>> diff --git a/fwts-test/arg-show-tests-0001/arg-show-tests-0001.log b/fwts-test/arg-show-tests-0001/arg-show-tests-0001.log
>> index 0168202..10c7f0c 100644
>> --- a/fwts-test/arg-show-tests-0001/arg-show-tests-0001.log
>> +++ b/fwts-test/arg-show-tests-0001/arg-show-tests-0001.log
>> @@ -63,6 +63,7 @@ Batch tests:
>> bgrt BGRT Boot Graphics Resource Table test.
>> bios32 BIOS32 Service Directory test.
>> bios_info Gather BIOS DMI information.
>> + bmc_info BMC Info
>> boot BOOT Table test.
>> checksum ACPI table checksum test.
>> cpep CPEP Corrected Platform Error Polling Table test.
>> 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 8c9cd10..86ae2d8 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
>> @@ -332,6 +332,8 @@ Batch tests:
>> BIOS32 Service Directory test.
>> bios_info (1 test):
>> Gather BIOS DMI information
>> + bmc_info (1 test):
>> + BMC Info
>> boot (1 test):
>> BOOT Table test.
>> checksum (1 test):
>> diff --git a/src/Makefile.am b/src/Makefile.am
>> index 59f8506..00cde32 100644
>> --- a/src/Makefile.am
>> +++ b/src/Makefile.am
>> @@ -123,6 +123,7 @@ fwts_SOURCES = main.c \
>> cpu/microcode/microcode.c \
>> dmi/dmicheck/dmicheck.c \
>> hotkey/hotkey/hotkey.c \
>> + ipmi/bmc/bmc_info.c \
>> kernel/klog/klog.c \
>> kernel/olog/olog.c \
>> kernel/oops/oops.c \
>> diff --git a/src/ipmi/bmc/bmc_info.c b/src/ipmi/bmc/bmc_info.c
>> new file mode 100644
>> index 0000000..9ffe49a
>> --- /dev/null
>> +++ b/src/ipmi/bmc/bmc_info.c
>> @@ -0,0 +1,76 @@
>> +/*
>> + * 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 <unistd.h>
>> +
>> +#include "fwts.h"
>> +
>> +int fwts_bmc_info_check(
>> + fwts_framework *fw)
>> +{
>> + struct fwts_ipmi_rsp fwts_bmc_info;
>> +
>> + fwts_progress(fw, 50);
>> +
>> + if ((fwts_ipmi_base_query(&fwts_bmc_info))) {
>> + return FWTS_ERROR;
>> + }
>> +
>> + fwts_log_info(fw, "IPMI Version is %x.%x \n",
>> + IPMI_DEV_IPMI_VERSION_MAJOR(fwts_bmc_info.ipmi_version),
>> + IPMI_DEV_IPMI_VERSION_MINOR(fwts_bmc_info.ipmi_version));
>> +
>> + fwts_progress(fw, 100);
>> +
>> + return FWTS_OK;
>> +}
>> +
>> +static int bmc_info_test1(fwts_framework *fw)
>> +{
>> +
>> + if (!fwts_ipmi_present(W_OK)) {
>> + fwts_failed(fw, LOG_LEVEL_CRITICAL, "BMC Info",
>> + "Cannot write to the IPMI device interface,"
>> + " check your user privileges.");
>> + return FWTS_ERROR;
>> + }
>> + if (fwts_bmc_info_check(fw)) {
>> + fwts_failed(fw, LOG_LEVEL_CRITICAL, "BMC Info",
>> + "Internal processing errors.");
>> + return FWTS_ERROR;
>> + }
>> +
>> + fwts_passed(fw, "BMC info passed.");
>> +
>> + return FWTS_OK;
>> +}
>> +
>> +static fwts_framework_minor_test bmc_info_tests[] = {
>> + { bmc_info_test1, "BMC Info" },
>> + { NULL, NULL }
>> +};
>> +
>> +static fwts_framework_ops bmc_info_ops = {
>> + .description = "BMC Info",
>> + .minor_tests = bmc_info_tests
>> +};
>> +
>> +FWTS_REGISTER_FEATURES("bmc_info", &bmc_info_ops, FWTS_TEST_EARLY,
>> + FWTS_FLAG_BATCH, FWTS_FW_FEATURE_IPMI)
>> diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h
>> index fb6dafa..780c308 100644
>> --- a/src/lib/include/fwts.h
>> +++ b/src/lib/include/fwts.h
>> @@ -71,6 +71,7 @@
>> #include "fwts_firmware.h"
>> #include "fwts_gpe.h"
>> #include "fwts_iasl.h"
>> +#include "fwts_ipmi.h"
>> #include "fwts_klog.h"
>> #include "fwts_olog.h"
>> #include "fwts_pipeio.h"
>> diff --git a/src/lib/include/fwts_firmware.h b/src/lib/include/fwts_firmware.h
>> index 835d922..9221afc 100644
>> --- a/src/lib/include/fwts_firmware.h
>> +++ b/src/lib/include/fwts_firmware.h
>> @@ -32,8 +32,10 @@ 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_DEVICETREE |
>> + FWTS_FW_FEATURE_IPMI
>> };
>>
>> int fwts_firmware_detect(void);
>> diff --git a/src/lib/include/fwts_ipmi.h b/src/lib/include/fwts_ipmi.h
>> new file mode 100644
>> index 0000000..f8808b6
>> --- /dev/null
>> +++ b/src/lib/include/fwts_ipmi.h
>> @@ -0,0 +1,52 @@
>> +/*
>> + * 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.
>> + *
>> + */
>> +
>> +#ifndef __FWTS_IPMI_H__
>> +#define __FWTS_IPMI_H__
>> +
>> +#include "fwts.h"
>> +
>> +#define IPMI_DEV_IPMI_VERSION_MAJOR(x) \
>> + (x & IPMI_DEV_IPMI_VER_MAJOR_MASK)
>> +#define IPMI_DEV_IPMI_VERSION_MINOR(x) \
>> + ((x & IPMI_DEV_IPMI_VER_MINOR_MASK) >> IPMI_DEV_IPMI_VER_MINOR_SHIFT)
>> +
>> +#define IPMI_DEV_IPMI_VER_MAJOR_MASK (0x0F)
>> +#define IPMI_DEV_IPMI_VER_MINOR_MASK (0xF0)
>> +
>> +#define IPMI_DEV_IPMI_VER_MINOR_SHIFT (4)
>> +
>> +typedef struct fwts_ipmi_rsp {
>> + uint8_t completion_code;
>> + uint8_t device_id;
>> + uint8_t device_revision;
>> + uint8_t fw_rev1;
>> + uint8_t fw_rev2;
>> + uint8_t ipmi_version;
>> + uint8_t adtl_device_support;
>> + uint8_t manufacturer_id[3];
>> + uint8_t product_id[2];
>> + uint8_t aux_fw_rev[4];
>> +} __attribute__((packed)) fwts_ipmi_rsp;
> struct fwts_ipmi_rsp is defined as a response to an IPMI_GET_DEVICE_ID_CMD
> command. With such a name, shouldn't it be a little more generic ? something
> like :
>
> typedef struct fwts_ipmi_rsp {
> uint8_t completion_code;
> uint8_t bytes[IPMI_MAX_MSG_LENGTH-1];
> }
>
> and then we could map specific response structs to this generic one ?
> Just a suggestion. This can be done when adding new commands.
>
> I don't think we would have a lot of commands to handle to validate
> the BMC. We should check the FRUs and the SDRs mostly and signal
> missing ones.
>
>> +
>> +int fwts_ipmi_base_query(fwts_ipmi_rsp *fwts_bmc_rsp);
>> +bool fwts_ipmi_present(int fwts_ipmi_flags);
>> +
>> +#endif
>> diff --git a/src/lib/src/Makefile.am b/src/lib/src/Makefile.am
>> index d9e5a8d..e96b75f 100644
>> --- a/src/lib/src/Makefile.am
>> +++ b/src/lib/src/Makefile.am
>> @@ -57,6 +57,7 @@ libfwts_la_SOURCES = \
>> fwts_iasl.c \
>> fwts_interactive.c \
>> fwts_ioport.c \
>> + fwts_ipmi.c \
>> fwts_keymap.c \
>> fwts_klog.c \
>> fwts_olog.c \
>> diff --git a/src/lib/src/fwts_firmware.c b/src/lib/src/fwts_firmware.c
>> index 5092ac9..604c6d6 100644
>> --- a/src/lib/src/fwts_firmware.c
>> +++ b/src/lib/src/fwts_firmware.c
>> @@ -32,6 +32,7 @@ static struct {
>> } feature_names[] = {
>> { FWTS_FW_FEATURE_ACPI, "ACPI" },
>> { FWTS_FW_FEATURE_DEVICETREE, "devicetree" },
>> + { FWTS_FW_FEATURE_IPMI, "IPMI READ" },
>> };
>>
>> /*
>> @@ -63,7 +64,6 @@ int fwts_firmware_features(void)
>> {
>> int features = 0;
>>
>> - /* we just have static feature definitions for now */
>> switch (fwts_firmware_detect()) {
>> case FWTS_FIRMWARE_BIOS:
>> case FWTS_FIRMWARE_UEFI:
>> @@ -76,6 +76,10 @@ int fwts_firmware_features(void)
>> break;
>> }
>>
>> + /* we check for IPMI presence dynamically to assure available */
>> + if (fwts_ipmi_present(R_OK))
>> + features |= FWTS_FW_FEATURE_IPMI;
>> +
>> return features;
>> }
>>
>> @@ -83,7 +87,7 @@ const char *fwts_firmware_feature_string(const int features)
>> {
>> const int n = FWTS_ARRAY_LEN(feature_names);
>> const char sep[] = ", ";
>> - static char str[50];
>> + static char str[60];
>> size_t len;
>> char *p;
>> int i;
>> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
>> index d0979c9..2faeb67 100644
>> --- a/src/lib/src/fwts_framework.c
>> +++ b/src/lib/src/fwts_framework.c
>> @@ -593,16 +593,6 @@ static int fwts_framework_run_test(fwts_framework *fw, fwts_framework_test *test
>>
>> fwts_framework_minor_test_progress(fw, 0, "");
>>
>> - if (!fwts_firmware_has_features(test->fw_features)) {
>> - int missing = test->fw_features & ~fwts_firmware_features();
>> - fwts_log_info(fw, "Test skipped, missing features: %s",
>> - fwts_firmware_feature_string(missing));
>> - fw->current_major_test->results.skipped +=
>> - test->ops->total_tests;
>> - fw->total.skipped += test->ops->total_tests;
>> - goto done;
>> - }
>> -
>> if ((test->flags & FWTS_FLAG_ROOT_PRIV) &&
>> (fwts_check_root_euid(fw, true) != FWTS_OK)) {
>> fwts_log_error(fw, "Aborted test, insufficient privilege.");
>> @@ -615,6 +605,16 @@ 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();
>> + fwts_log_info(fw, "Test skipped, missing features: %s",
>> + fwts_firmware_feature_string(missing));
>> + fw->current_major_test->results.skipped +=
>> + test->ops->total_tests;
>> + fw->total.skipped += test->ops->total_tests;
>> + goto done;
>> + }
>> +
>> if ((test->ops->init) &&
>> ((ret = test->ops->init(fw)) != FWTS_OK)) {
>> char *msg = NULL;
>> diff --git a/src/lib/src/fwts_ipmi.c b/src/lib/src/fwts_ipmi.c
>> new file mode 100644
>> index 0000000..737f6fa
>> --- /dev/null
>> +++ b/src/lib/src/fwts_ipmi.c
>> @@ -0,0 +1,120 @@
>> +/*
>> + * 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 <sys/fcntl.h>
>> +#include <sys/poll.h>
>> +#include <sys/ioctl.h>
>> +#include <linux/ipmi.h>
>> +
>> +#include "fwts.h"
>> +
>> +unsigned int fwts_ipmi_msg_id = 1;
>> +static const char *fwts_ipmi_devnode = "/dev/ipmi0";
>> +
>> +bool fwts_ipmi_present(int fwts_ipmi_flags) {
>> + return !access(fwts_ipmi_devnode, fwts_ipmi_flags);
>> +}
>> +
>> +int fwts_ipmi_exec_query(
>> + fwts_ipmi_rsp *fwts_base_rsp,
>> + struct ipmi_req *fwts_ipmi_req)
>> +{
>> +
>> + int fd = 0, fwts_pollfd_rc = 0, fwts_send_rc = 0, fwts_recv_rc = 0;
>> + uint8_t recv_data[IPMI_MAX_MSG_LENGTH];
>> + struct ipmi_recv fwts_ipmi_recv;
>> + struct ipmi_addr fwts_ipmi_addr;
>> + struct pollfd fwts_pfd;
>> +
>> + if ((fd = open(fwts_ipmi_devnode, O_RDWR)) < 0){
>> + close(fd);
>> + return FWTS_ERROR;
>> + };
>> +
>> + fwts_send_rc = ioctl(fd, IPMICTL_SEND_COMMAND, (char *)fwts_ipmi_req);
>> +
>> + if (fwts_send_rc != 0) {
>> + close(fd);
>> + return FWTS_ERROR;
>> + }
>> +
>> + for (;;) {
>> + fwts_pfd.events = POLLIN | POLLPRI;
>> + fwts_pfd.fd = fd;
>> + fwts_pollfd_rc = poll(&fwts_pfd,1,5000);
> The BT interface is slow but I think 5 secs is a lot.
>
>> + if (fwts_pollfd_rc > 0) {
>> + break;
>> + } else {
>> + close(fd);
>> + return FWTS_ERROR;
>> + }
>> + }
>> +
>> + memset(&recv_data, 0, sizeof(IPMI_MAX_MSG_LENGTH));
>> + fwts_ipmi_recv.msg.data = recv_data;
>> + fwts_ipmi_recv.msg.data_len = sizeof (recv_data);
>> + fwts_ipmi_recv.addr = (unsigned char *)&fwts_ipmi_addr;
>> + fwts_ipmi_recv.addr_len = sizeof (fwts_ipmi_addr);
>> + fwts_recv_rc = ioctl(fd, IPMICTL_RECEIVE_MSG_TRUNC, &fwts_ipmi_recv);
>> +
>> + if (fwts_recv_rc != 0) {
>> + close(fd);
>> + return FWTS_ERROR;
>> + }
>> +
>> + memcpy(fwts_base_rsp, fwts_ipmi_recv.msg.data, sizeof(fwts_ipmi_rsp));
>> +
>> + /* Future completion_code non-zero with good results to pass back */
>> + if (fwts_base_rsp->completion_code != 0) {
>> + close(fd);
>> + return FWTS_ERROR;
>> + }
>> +
>> + close(fd);
>> + return FWTS_OK;
>> +}
>> +
>> +int fwts_ipmi_base_query(fwts_ipmi_rsp *fwts_base_rsp)
>> +{
>> +
>> + struct ipmi_req fwts_ipmi_req;
>> + struct ipmi_system_interface_addr fwts_ipmi_bmc_addr;
>> +
>> + memset(&fwts_ipmi_bmc_addr, 0, sizeof(fwts_ipmi_bmc_addr));
>> +
>> + fwts_ipmi_bmc_addr.addr_type = IPMI_SYSTEM_INTERFACE_ADDR_TYPE;
>> + fwts_ipmi_bmc_addr.channel = IPMI_BMC_CHANNEL;
>> + fwts_ipmi_bmc_addr.lun = 0;
>> +
>> + memset(&fwts_ipmi_req, 0, sizeof(fwts_ipmi_req));
>> +
>> + fwts_ipmi_req.addr = (unsigned char *)&fwts_ipmi_bmc_addr;
>> + fwts_ipmi_req.addr_len = sizeof (fwts_ipmi_bmc_addr);
>> + fwts_ipmi_req.msgid = fwts_ipmi_msg_id ++;
>> + fwts_ipmi_req.msg.netfn = IPMI_NETFN_APP_REQUEST;
>> + fwts_ipmi_req.msg.cmd = IPMI_GET_DEVICE_ID_CMD;
>> + fwts_ipmi_req.msg.data_len = 0;
>> + fwts_ipmi_req.msg.data = NULL;
>> +
>> + if (fwts_ipmi_exec_query(fwts_base_rsp, &fwts_ipmi_req))
>> + return FWTS_ERROR;
>> +
>> + return FWTS_OK;
>> +}
>>
--
==========================================
Deb McLemore
IBM OpenPower - IBM Systems
(512) 286 9980
debmc at us.ibm.com
debmc at linux.vnet.ibm.com - (plain text)
==========================================
More information about the fwts-devel
mailing list