[PATCH] V2 Introduce IPMI BMC Info
Cédric Le Goater
clg at fr.ibm.com
Wed May 4 13:14:25 UTC 2016
On 05/04/2016 02:53 PM, Deb McLemore wrote:
> 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.
ok. I agree. Let's not over specify.
> 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.
The BT device advertises a timeout value of 2s. So we could lower it a bit
but this is clearly not a problem. I gave it a try on an habanero. Looks good.
Reviewed-by: Cedric Le Goater <clg at fr.ibm.com>
Thanks,
C.
> 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;
>>> +}
>>>
>
More information about the fwts-devel
mailing list