| Message ID | 1462220309-25257-1-git-send-email-debmc@linux.vnet.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
Hi Deb, > 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. Nice! A few comments: > +#include <unistd.h> > + > +#include "fwts.h" > + > +int fwts_bmc_info_check( > + fwts_framework *fw) > +{ > + fwts_ipmi_rsp *fwts_bmc_info; > + fwts_bmc_info = calloc(1, sizeof(fwts_ipmi_rsp)); Do we need to calloc() that? Can we just use it directly from the stack instead? > diff --git a/src/lib/src/fwts_firmware.c b/src/lib/src/fwts_firmware.c > index 5092ac9..6001cd5 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" }, > }; > > /* > @@ -63,14 +64,18 @@ int fwts_firmware_features(void) > { > int features = 0; > > - /* we just have static feature definitions for now */ > + /* we check for IPMI presence dynamically to assure available */ > + > + if (fwts_ipmi_present()) > + features = FWTS_FW_FEATURE_IPMI; > + I'd probably do this after the switch (fwts_firmware_detect()) statement; that assigns the base features, then we can OR-in more non-type-specific features with these checks. So, we'd do something like: switch (fwts_firmware_detect()) { case FWTS_FIRMWARE_BIOS: case FWTS_FIRMWARE_UEFI: features = FWTS_FW_FEATURE_ACPI; break; case FWTS_FIRMWARE_OPAL: features = FWTS_FW_FEATURE_DEVICETREE; break; default: break; } if (fwts_ipmi_present()) features |= FWTS_FW_FEATURE_IPMI; if (fwts_other_new_feature_present()) features |= FWTS_FW_FEATURE_NEW_THING; > @@ -83,7 +88,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]; Did you hit the FWTS_ASSERT before adding this change? I would have thought that 50 bytes was large enough to contain the new "IPMI" string too (strlen("ACPI, devicetree, IPMI") + 1 == 22). Or did I get the FWTS_ASSERT check wrong? > 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 I'm reading it correctly, this change moves the feature check to *after* the ROOT_PRIV check. Was that your intention? Do we need to do that? Or is that related to... > +unsigned int fwts_ipmi_msg_id = 1; > +static const char *fwts_ipmi_devnode = "/dev/ipmi0"; > + > +bool fwts_ipmi_present(void) { > + return !access(fwts_ipmi_devnode, R_OK | W_OK); > +} Semantic nit: the platform supports IPMI if !access(..., R_OK); however, we can only use it if W_OK. So, the feature check should be on R_OK, but the test should require ROOT_PRIV (or even better, we'd do another W_OK check in the test, as the IPMI device may be writable by non-root...). Cheers, Jeremy
Hi Jeremy, comments in-line below: On 05/03/2016 02:58 AM, Jeremy Kerr wrote: > Hi Deb, > >> 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. > Nice! A few comments: > >> +#include <unistd.h> >> + >> +#include "fwts.h" >> + >> +int fwts_bmc_info_check( >> + fwts_framework *fw) >> +{ >> + fwts_ipmi_rsp *fwts_bmc_info; >> + fwts_bmc_info = calloc(1, sizeof(fwts_ipmi_rsp)); > Do we need to calloc() that? Can we just use it directly from the stack > instead? Will fix, we did a bit of restructuring on the code, so will change to stack. > >> diff --git a/src/lib/src/fwts_firmware.c b/src/lib/src/fwts_firmware.c >> index 5092ac9..6001cd5 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" }, >> }; >> >> /* >> @@ -63,14 +64,18 @@ int fwts_firmware_features(void) >> { >> int features = 0; >> >> - /* we just have static feature definitions for now */ >> + /* we check for IPMI presence dynamically to assure available */ >> + >> + if (fwts_ipmi_present()) >> + features = FWTS_FW_FEATURE_IPMI; >> + > I'd probably do this after the switch (fwts_firmware_detect()) > statement; that assigns the base features, then we can OR-in more > non-type-specific features with these checks. > > So, we'd do something like: > > switch (fwts_firmware_detect()) { > case FWTS_FIRMWARE_BIOS: > case FWTS_FIRMWARE_UEFI: > features = FWTS_FW_FEATURE_ACPI; > break; > case FWTS_FIRMWARE_OPAL: > features = FWTS_FW_FEATURE_DEVICETREE; > break; > default: > break; > } > > if (fwts_ipmi_present()) > features |= FWTS_FW_FEATURE_IPMI; > > if (fwts_other_new_feature_present()) > features |= FWTS_FW_FEATURE_NEW_THING; Will re-order. > > >> @@ -83,7 +88,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]; > Did you hit the FWTS_ASSERT before adding this change? I would have > thought that 50 bytes was large enough to contain the new "IPMI" > string too (strlen("ACPI, devicetree, IPMI") + 1 == 22). > > Or did I get the FWTS_ASSERT check wrong? Here's some output on the ASSERT check, let me know if this was not the original intent: static struct { enum firmware_feature feature; const char name[16]; } feature_names[] = { { FWTS_FW_FEATURE_ACPI, "ACPI" }, { FWTS_FW_FEATURE_DEVICETREE, "devicetree" }, { FWTS_FW_FEATURE_IPMI, "IPMI" }, }; const int n = FWTS_ARRAY_LEN(feature_names); const char sep[] = ", "; static char str[60]; size_t len; char *p; int i; /* ensure we have enough space in str to store n names, plus n-1 * separators, plus a trailing nul */ printf("size of feature_names[0].name is %ld.\n", sizeof(feature_names[0].name)); printf("size of str is %ld.\n", sizeof(str)); printf("size of sep is %ld.\n", sizeof(sep)); printf("n is %i.\n", n); printf("assert value is %ld.\n", (n*(sizeof(feature_names[0].name)-1)+( (n-1)*(sizeof(sep)-1) )+1) ); FWTS_ASSERT((n * (sizeof(feature_names[0].name) - 1)) + ((n-1) * (sizeof(sep) - 1)) + 1 < sizeof(str), str_too_small); bmc_info: BMC Info -------------------------------------------------------------------------------------------------------------------------------------------------------- size of feature_names[0].name is 16. size of str is 60. size of sep is 3. n is 3. assert value is 50. 3 * (16-1) +( 2*2 ) + 1 = 45 + 4 + 1 = 50 > > >> 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 I'm reading it correctly, this change moves the feature check to > *after* the ROOT_PRIV check. Was that your intention? Do we need to do > that? Moved the ROOT_PRIV check first to bail if ROOT_PRIV's are required first, so that seems most likely the overall usage flows. This was changed for IPMI cases since we needed ROOT_PRIV to do the access check for R_OK | W_OK to successfully function, but as you point out other users for IPMI may be able to W_OK, so we can add access check for W_OK in IPMI BMC Info check. > > Or is that related to... > >> +unsigned int fwts_ipmi_msg_id = 1; >> +static const char *fwts_ipmi_devnode = "/dev/ipmi0"; >> + >> +bool fwts_ipmi_present(void) { >> + return !access(fwts_ipmi_devnode, R_OK | W_OK); >> +} > Semantic nit: the platform supports IPMI if !access(..., R_OK); however, > we can only use it if W_OK. > > So, the feature check should be on R_OK, but the test should require > ROOT_PRIV (or even better, we'd do another W_OK check in the test, as > the IPMI device may be writable by non-root...). > > Cheers, > > > Jeremy >
Hi Deb, > FWTS_ASSERT((n * (sizeof(feature_names[0].name) - 1)) + > ((n-1) * (sizeof(sep) - 1)) + 1 < > sizeof(str), str_too_small); > > size of feature_names[0].name is 16. Ah, of course - we're using the maximum name length here. All good :) > Moved the ROOT_PRIV check first to bail if ROOT_PRIV's are > required first, so that seems most likely the overall usage flows. > This was changed for IPMI cases since we needed ROOT_PRIV to do the > access check for R_OK | W_OK to successfully function, but > as you point out other users for IPMI may be able to W_OK, so we > can add access check for W_OK in IPMI BMC Info check. OK, as long as FWTS reports IPMI capability (ie, we check with just R_OK), regardless of whether we have privileges to use the ipmiX device. Cheers, Jeremy
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..8e7b9d8 --- /dev/null +++ b/src/ipmi/bmc/bmc_info.c @@ -0,0 +1,75 @@ +/* + * 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) +{ + fwts_ipmi_rsp *fwts_bmc_info; + fwts_bmc_info = calloc(1, sizeof(fwts_ipmi_rsp)); + + if (!fwts_bmc_info) + return FWTS_ERROR; + + fwts_progress(fw, 50); + + if ((fwts_ipmi_base_query(fwts_bmc_info))) { + free(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)); + + free(fwts_bmc_info); + fwts_progress(fw, 100); + + return FWTS_OK; +} + +static int bmc_info_test1(fwts_framework *fw) +{ + 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_FLAG_ROOT_PRIV, 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..d8e55e4 --- /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; + +int fwts_ipmi_base_query(fwts_ipmi_rsp *fwts_bmc_rsp); +bool fwts_ipmi_present(); + +#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..6001cd5 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" }, }; /* @@ -63,14 +64,18 @@ int fwts_firmware_features(void) { int features = 0; - /* we just have static feature definitions for now */ + /* we check for IPMI presence dynamically to assure available */ + + if (fwts_ipmi_present()) + features = FWTS_FW_FEATURE_IPMI; + switch (fwts_firmware_detect()) { case FWTS_FIRMWARE_BIOS: case FWTS_FIRMWARE_UEFI: - features = FWTS_FW_FEATURE_ACPI; + features = features | FWTS_FW_FEATURE_ACPI; break; case FWTS_FIRMWARE_OPAL: - features = FWTS_FW_FEATURE_DEVICETREE; + features = features | FWTS_FW_FEATURE_DEVICETREE; break; default: break; @@ -83,7 +88,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..cdd20e0 --- /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(void) { + return !access(fwts_ipmi_devnode, R_OK | W_OK); +} + +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); + 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; +}
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@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 | 75 +++++++++++++ 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 | 13 ++- src/lib/src/fwts_framework.c | 20 ++-- src/lib/src/fwts_ipmi.c | 120 +++++++++++++++++++++ 11 files changed, 275 insertions(+), 15 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