[PATCH] V2 Introduce IPMI BMC Info

Cédric Le Goater clg at fr.ibm.com
Wed May 4 12:42:01 UTC 2016


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