diff options
| -rw-r--r-- | .gitignore | 1 | ||||
| -rw-r--r-- | Makefile.am | 6 | ||||
| -rw-r--r-- | configure.ac | 2 | ||||
| -rw-r--r-- | flash-ipmi.cpp | 39 | ||||
| -rw-r--r-- | flash-ipmi.hpp | 54 | ||||
| -rw-r--r-- | ipmi.cpp | 40 | ||||
| -rw-r--r-- | ipmi.hpp | 18 | ||||
| -rw-r--r-- | main.cpp | 22 | ||||
| -rw-r--r-- | test/Makefile.am | 18 | ||||
| -rw-r--r-- | test/ipmi_starttransfer_unittest.cpp | 61 | ||||
| -rw-r--r-- | test/updater_mock.hpp | 13 |
11 files changed, 271 insertions, 3 deletions
@@ -31,3 +31,4 @@ x86_64-libtool *_unittest *_unittest.log *_unittest.trs +test-suite.log diff --git a/Makefile.am b/Makefile.am index 05f493e..0b76d38 100644 --- a/Makefile.am +++ b/Makefile.am @@ -2,7 +2,9 @@ AM_DEFAULT_SOURCE_EXT = .cpp libflashcmdsdir = ${libdir}/ipmid-providers libflashcmds_LTLIBRARIES = libflashcmds.la -libflashcmds_la_SOURCES = main.cpp +libflashcmds_la_SOURCES = main.cpp \ + flash-ipmi.cpp \ + ipmi.cpp libflashcmds_la_LDFLAGS = $(SYSTEMD_LIBS) \ $(PHOSPHOR_DBUS_INTERFACES_LIBS) \ @@ -12,4 +14,4 @@ libflashcmds_la_CXXFLAGS = $(SYSTEMD_CFLAGS) \ $(PHOSPHOR_DBUS_INTERFACES_CFLAGS) \ $(PHOSPHOR_LOGGING_CFLAGS) -SUBDIRS = . +SUBDIRS = . test diff --git a/configure.ac b/configure.ac index 0add2b8..0fe7382 100644 --- a/configure.ac +++ b/configure.ac @@ -69,5 +69,5 @@ AS_IF([test "x$enable_oe_sdk" == "xyes"], ) # Create configured output -AC_CONFIG_FILES([Makefile]) +AC_CONFIG_FILES([Makefile test/Makefile]) AC_OUTPUT diff --git a/flash-ipmi.cpp b/flash-ipmi.cpp new file mode 100644 index 0000000..4d05044 --- /dev/null +++ b/flash-ipmi.cpp @@ -0,0 +1,39 @@ +/* + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "flash-ipmi.hpp" + +void FlashUpdate::abortEverything() +{ + return; +} + +bool FlashUpdate::openEverything() +{ + return true; +} + +/* Prepare to receive a BMC image and then a signature. */ +bool FlashUpdate::start(uint32_t) +{ + /* TODO: Validate request->length */ + + /* Close out and delete everything. */ + abortEverything(); + + /* Start over! */ + return openEverything(); +} diff --git a/flash-ipmi.hpp b/flash-ipmi.hpp index 43cafe1..deef582 100644 --- a/flash-ipmi.hpp +++ b/flash-ipmi.hpp @@ -1,5 +1,10 @@ #pragma once +#include "host-ipmid/ipmid-api.h" + +/* Clearer way to represent the subcommand size. */ +#define SUBCMD_SZ sizeof(uint8_t) + /* * flashStartTransfer -- starts file upload. * flashDataBlock -- adds data to image file. @@ -55,3 +60,52 @@ enum FlashSubCmds flashHashExtData = 12, flashMapRegionLpc = 13, }; + +/* + * StartTransfer expects a basic structure providing some information. + */ +struct StartTx +{ + uint8_t cmd; + uint32_t length; /* Maximum image length is 4GiB (little-endian) */ +} __attribute__((packed)); + +class UpdateInterface +{ + public: + virtual ~UpdateInterface() = default; + + virtual bool start(uint32_t length) = 0; +}; + +class FlashUpdate : public UpdateInterface +{ + public: + FlashUpdate() = default; + ~FlashUpdate() = default; + FlashUpdate(const FlashUpdate&) = default; + FlashUpdate& operator=(const FlashUpdate&) = default; + FlashUpdate(FlashUpdate&&) = default; + FlashUpdate& operator=(FlashUpdate&&) = default; + + /** + * Prepare to receive a BMC image and then a signature. + * + * @param[in] length - the size of the flash image. + * @return true on success, false otherwise. + */ + bool start(uint32_t length) override; + + private: + /** + * Tries to close out and delete anything staged. + */ + void abortEverything(); + + /** + * Open all staged file handles you expect to use. + * + * @return false on failure. + */ + bool openEverything(); +}; diff --git a/ipmi.cpp b/ipmi.cpp new file mode 100644 index 0000000..65a719a --- /dev/null +++ b/ipmi.cpp @@ -0,0 +1,40 @@ +/* + * Copyright 2017 Google Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "flash-ipmi.hpp" +#include "ipmi.hpp" + +ipmi_ret_t startTransfer(UpdateInterface* updater, const uint8_t* reqBuf, + uint8_t* replyBuf, size_t* dataLen) +{ + /* Validate the request buffer. */ + if (sizeof(struct StartTx) > (*dataLen)) + { + return IPMI_CC_INVALID; + } + + auto request = reinterpret_cast<const struct StartTx*>(reqBuf); + + if (!updater->start(request->length)) + { + return IPMI_CC_INVALID; + } + + /* We were successful and set the response byte to 0. */ + replyBuf[0] = 0x00; + (*dataLen) = 1; + return IPMI_CC_OK; +} diff --git a/ipmi.hpp b/ipmi.hpp new file mode 100644 index 0000000..3b622b0 --- /dev/null +++ b/ipmi.hpp @@ -0,0 +1,18 @@ +#pragma once + +#include "host-ipmid/ipmid-api.h" + +#include "flash-ipmi.hpp" + +/** + * Prepare to receive a BMC image and then a signature. + * + * @param[in] updater - Pointer to Updater object. + * @param[in] reqBuf - the IPMI packet. + * @param[in] replyBuf - Pointer to buffer for any response. + * @param[in,out] dataLen - Initially reqBuf length, set to replyBuf + * length when done. + * @return corresponding IPMI return code. + */ +ipmi_ret_t startTransfer(UpdateInterface* updater, const uint8_t* reqBuf, + uint8_t* replyBuf, size_t* dataLen); @@ -14,10 +14,13 @@ * limitations under the License. */ +#include <memory> + #include "host-ipmid/ipmid-api.h" #include "host-ipmid/oemrouter.hpp" #include "flash-ipmi.hpp" +#include "ipmi.hpp" /* TODO: Once OEM IPMI number placement is settled, point to that. */ namespace oem @@ -29,6 +32,9 @@ constexpr int flashOverBTCmd = 127; } // namespace google } // namespace oem +/* We have one instance of the FlashUpdate object tracking commands. */ +std::unique_ptr<FlashUpdate> flashUpdateSingleton; + static ipmi_ret_t flashControl(ipmi_cmd_t cmd, const uint8_t* reqBuf, uint8_t* replyCmdBuf, size_t* dataLen) { @@ -38,6 +44,20 @@ static ipmi_ret_t flashControl(ipmi_cmd_t cmd, const uint8_t* reqBuf, return IPMI_CC_INVALID; } + uint8_t subCmd = reqBuf[0]; + + /* TODO: This could be cleaner to just have a function pointer table, may + * transition in later patchset. + */ + switch (subCmd) + { + case FlashSubCmds::flashStartTransfer: + return startTransfer(flashUpdateSingleton.get(), reqBuf, + replyCmdBuf, dataLen); + default: + return IPMI_CC_INVALID; + } + return IPMI_CC_INVALID; } @@ -58,6 +78,8 @@ void setupGlobalOemFlashControl() __attribute__((constructor)); void setupGlobalOemFlashControl() { + flashUpdateSingleton = std::make_unique<FlashUpdate>(); + #ifdef ENABLE_GOOGLE oem::Router* router = oem::mutableRouter(); diff --git a/test/Makefile.am b/test/Makefile.am new file mode 100644 index 0000000..7da78d8 --- /dev/null +++ b/test/Makefile.am @@ -0,0 +1,18 @@ +AM_CPPFLAGS = -I$(top_srcdir)/ \ + $(GTEST_CFLAGS) \ + $(GMOCK_CFLAGS) +AM_CXXFLAGS = \ + $(GTEST_MAIN_CFLAGS) +AM_LDFLAGS = \ + $(GMOCK_LIBS) \ + $(GTEST_MAIN_LIBS) \ + $(OESDK_TESTCASE_FLAGS) + +# Run all 'check' test programs +check_PROGRAMS = \ + ipmi_starttransfer_unittest + +TESTS = $(check_PROGRAMS) + +ipmi_starttransfer_unittest_SOURCES = ipmi_starttransfer_unittest.cpp +ipmi_starttransfer_unittest_LDADD = $(top_builddir)/ipmi.o diff --git a/test/ipmi_starttransfer_unittest.cpp b/test/ipmi_starttransfer_unittest.cpp new file mode 100644 index 0000000..448287b --- /dev/null +++ b/test/ipmi_starttransfer_unittest.cpp @@ -0,0 +1,61 @@ +#include "flash-ipmi.hpp" +#include "ipmi.hpp" + +#include "updater_mock.hpp" + +#include <cstring> +#include <gtest/gtest.h> + +using ::testing::NotNull; +using ::testing::Return; +using ::testing::StrictMock; + +// ipmid.hpp isn't installed where we can grab it and this value is per BMC +// SoC. +#define MAX_IPMI_BUFFER 64 +#define THIRTYTWO_MIB 33554432 + +TEST(IpmiStartTransferTest, InvalidRequestLengthReturnsFailure) +{ + // Verify that the request is sanity checked w.r.t length. + + StrictMock<UpdaterMock> updater; + + size_t dataLen; + uint8_t request[MAX_IPMI_BUFFER] = {0}; + uint8_t reply[MAX_IPMI_BUFFER] = {0}; + + struct StartTx tx; + tx.cmd = FlashSubCmds::flashStartTransfer; + tx.length = THIRTYTWO_MIB; + std::memcpy(request, &tx, sizeof(tx)); + + dataLen = sizeof(tx) - 1; // It's too small to be a valid packet. + + EXPECT_EQ(IPMI_CC_INVALID, + startTransfer(&updater, request, reply, &dataLen)); +} + +TEST(IpmiStartTransferTest, ValidRequestBoringCase) +{ + // Verify that if the request is valid it calls into the flash updater. + + StrictMock<UpdaterMock> updater; + + size_t dataLen; + uint8_t request[MAX_IPMI_BUFFER] = {0}; + uint8_t reply[MAX_IPMI_BUFFER] = {0}; + + struct StartTx tx; + tx.cmd = FlashSubCmds::flashStartTransfer; + tx.length = THIRTYTWO_MIB; + std::memcpy(request, &tx, sizeof(tx)); + + dataLen = sizeof(tx); + + EXPECT_CALL(updater, start(THIRTYTWO_MIB)).WillOnce(Return(true)); + + EXPECT_EQ(IPMI_CC_OK, startTransfer(&updater, request, reply, &dataLen)); + EXPECT_EQ(sizeof(uint8_t), dataLen); + EXPECT_EQ(0, reply[0]); +} diff --git a/test/updater_mock.hpp b/test/updater_mock.hpp new file mode 100644 index 0000000..3c2b5ab --- /dev/null +++ b/test/updater_mock.hpp @@ -0,0 +1,13 @@ +#pragma once + +#include <gmock/gmock.h> + +#include "flash-ipmi.hpp" + +class UpdaterMock : public UpdateInterface +{ + public: + virtual ~UpdaterMock() = default; + + MOCK_METHOD1(start, bool(uint32_t)); +}; |
