fru-device: use FRUReader API to simplify FRU parsing I/O
FRUReader::read() abstracts away SMBus block-size limits, which allows
us to replace the loop at the end of readFRUContents() with a single
reader.read() call.
FRUReader also (due its caching) eliminates the need to share I/O
buffers between the IPMI and Tyan parsers. We can thus restructure the
parsing code to try each of them separately in turn, reusing the same
FRUReader to avoid duplicating I/O. This eliminates the findFRUHeader()
function (each parser just looks for a header where it expects it);
tests are updated accordingly to exercise readFRUContents() instead.
testFRUData was generated via https://github.com/ipmitool/frugen using
the '--ascii' flag and the following JSON:
{
"chassis": {
"type": 17,
"pn": "OBMC_TEST_CHASSIS",
"serial": "XYZ123"
},
"board": {
"mfg": "OpenBMC Project",
"pname": "OBMC_TEST_BOARD",
"serial": "ABC012",
"pn": "BOARD_PARTNUM"
},
"product": {
"lang": 1,
"mfg": "OpenBMC Project",
"pname": "OBMC_TEST_PRODUCT",
"serial": "9999",
}
}
Tested: on an ASRock Rack romed8hm3, fru-device successfully recognizes
and parses the baseboard FRU EEPROM as it did prior to this patch.
Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Change-Id: I215bb2f6da0e1b23e5f52727c749993452c031e9
diff --git a/include/fru_utils.hpp b/include/fru_utils.hpp
index ed019ad..2d0c719 100644
--- a/include/fru_utils.hpp
+++ b/include/fru_utils.hpp
@@ -137,18 +137,6 @@
ssize_t getFieldLength(uint8_t fruFieldTypeLenValue);
-/// \brief Find a FRU header.
-/// \param reader the BaseFRUReader to read via
-/// \param errorHelp and a helper string for failures
-/// \param blockData buffer to return the last read block
-/// \param baseOffset the offset to start the search at;
-/// set to 0 to perform search;
-/// returns the offset at which a header was found
-/// \return whether a header was found
-bool findFRUHeader(BaseFRUReader& reader, const std::string& errorHelp,
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX>& blockData,
- off_t& baseOffset);
-
/// \brief Read and validate FRU contents.
/// \param reader the BaseFRUReader to read via
/// \param errorHelp and a helper string for failures
@@ -157,9 +145,9 @@
const std::string& errorHelp);
/// \brief Validate an IPMI FRU common header
-/// \param blockData the bytes comprising the common header
+/// \param hdr the bytes comprising the common header
/// \return true if valid
-bool validateHeader(const std::array<uint8_t, I2C_SMBUS_BLOCK_MAX>& blockData);
+bool validateIPMIHeader(const std::vector<uint8_t>& hdr);
/// \brief Get offset for a common header area
/// \param area - the area
diff --git a/src/fru_utils.cpp b/src/fru_utils.cpp
index 5e8e695..05b7dc5 100644
--- a/src/fru_utils.cpp
+++ b/src/fru_utils.cpp
@@ -27,12 +27,6 @@
#include <string>
#include <vector>
-extern "C"
-{
-// Include for I2C_SMBUS_BLOCK_MAX
-#include <linux/i2c.h>
-}
-
static constexpr bool debug = false;
constexpr size_t fruVersion = 1; // Current FRU spec version number is 1
@@ -529,14 +523,19 @@
return fruFieldTypeLenValue & typeLenMask;
}
-bool validateHeader(const std::array<uint8_t, I2C_SMBUS_BLOCK_MAX>& blockData)
+bool validateIPMIHeader(const std::vector<uint8_t>& hdr)
{
+ if (hdr.size() < 8)
+ {
+ return false;
+ }
+
// ipmi spec format version number is currently at 1, verify it
- if (blockData[0] != fruVersion)
+ if (hdr[0] != fruVersion)
{
if (debug)
{
- std::cerr << "FRU spec version " << (int)(blockData[0])
+ std::cerr << "FRU spec version " << (int)(hdr[0])
<< " not supported. Supported version is "
<< (int)(fruVersion) << "\n";
}
@@ -544,12 +543,12 @@
}
// verify pad is set to 0
- if (blockData[6] != 0x0)
+ if (hdr[6] != 0x0)
{
if (debug)
{
std::cerr << "PAD value in header is non zero, value is "
- << (int)(blockData[6]) << "\n";
+ << (int)(hdr[6]) << "\n";
}
return false;
}
@@ -558,11 +557,11 @@
std::set<uint8_t> foundOffsets;
for (int ii = 1; ii < 6; ii++)
{
- if (blockData[ii] == 0)
+ if (hdr[ii] == 0)
{
continue;
}
- auto inserted = foundOffsets.insert(blockData[ii]);
+ auto inserted = foundOffsets.insert(hdr[ii]);
if (!inserted.second)
{
return false;
@@ -573,15 +572,15 @@
size_t sum = 0;
for (int jj = 0; jj < 7; jj++)
{
- sum += blockData[jj];
+ sum += hdr[jj];
}
sum = (256 - sum) & 0xFF;
- if (sum != blockData[7])
+ if (sum != hdr[7])
{
if (debug)
{
- std::cerr << "Checksum " << (int)(blockData[7])
+ std::cerr << "Checksum " << (int)(hdr[7])
<< " is invalid. calculated checksum is " << (int)(sum)
<< "\n";
}
@@ -590,63 +589,25 @@
return true;
}
-bool findFRUHeader(BaseFRUReader& reader, const std::string& errorHelp,
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX>& blockData,
- off_t& baseOffset)
+static std::vector<uint8_t> readIPMIFRUContents(BaseFRUReader& reader,
+ const std::string& errorHelp)
{
- if (reader.read(baseOffset, 0x8, blockData.data()) < 0)
+ std::vector<uint8_t> device(8);
+ int64_t ret = reader.read(0, device.size(), device.data());
+
+ if (ret != static_cast<int64_t>(device.size()))
{
- std::cerr << "failed to read " << errorHelp << " base offset "
- << baseOffset << "\n";
- return false;
- }
-
- // check the header checksum
- if (validateHeader(blockData))
- {
- return true;
- }
-
- // only continue the search if we just looked at 0x0.
- if (baseOffset != 0)
- {
- return false;
- }
-
- // now check for special cases where the IPMI data is at an offset
-
- // check if blockData starts with tyanHeader
- const std::vector<uint8_t> tyanHeader = {'$', 'T', 'Y', 'A', 'N', '$'};
- if (blockData.size() >= tyanHeader.size() &&
- std::equal(tyanHeader.begin(), tyanHeader.end(), blockData.begin()))
- {
- // look for the FRU header at offset 0x6000
- baseOffset = 0x6000;
- return findFRUHeader(reader, errorHelp, blockData, baseOffset);
- }
-
- if (debug)
- {
- std::cerr << "Illegal header " << errorHelp << " base offset "
- << baseOffset << "\n";
- }
-
- return false;
-}
-
-std::vector<uint8_t> readFRUContents(BaseFRUReader& reader,
- const std::string& errorHelp)
-{
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
- off_t baseOffset = 0x0;
-
- if (!findFRUHeader(reader, errorHelp, blockData, baseOffset))
- {
+ if (ret < 0)
+ {
+ std::cerr << "failed to read " << errorHelp << "\n";
+ }
return {};
}
- std::vector<uint8_t> device;
- device.insert(device.end(), blockData.begin(), blockData.begin() + 8);
+ if (!validateIPMIHeader(device))
+ {
+ return {};
+ }
bool hasMultiRecords = false;
size_t fruLength = fruBlockSize; // At least FRU header is present
@@ -684,15 +645,16 @@
areaOffset *= fruBlockSize;
- if (reader.read(baseOffset + areaOffset, 0x2, blockData.data()) < 0)
+ std::array<uint8_t, 2> areaData{};
+ if (reader.read(areaOffset, areaData.size(), areaData.data()) !=
+ static_cast<ssize_t>(areaData.size()))
{
- std::cerr << "failed to read " << errorHelp << " base offset "
- << baseOffset << "\n";
+ std::cerr << "failed to read " << errorHelp << "\n";
return {};
}
- // Ignore data type (blockData is already unsigned).
- size_t length = blockData[1] * fruBlockSize;
+ // Ignore data type (areaData is already unsigned).
+ size_t length = areaData[1] * fruBlockSize;
areaOffset += length;
fruLength = (areaOffset > fruLength) ? areaOffset : fruLength;
}
@@ -714,21 +676,22 @@
{
// In multi-area, the area offset points to the 0th record, each
// record has 3 bytes of the header we care about.
- if (reader.read(baseOffset + areaOffset, 0x3, blockData.data()) < 0)
+ std::array<uint8_t, 3> areaData{};
+ if (reader.read(areaOffset, areaData.size(), areaData.data()) !=
+ static_cast<ssize_t>(areaData.size()))
{
- std::cerr << "failed to read " << errorHelp << " base offset "
- << baseOffset << "\n";
+ std::cerr << "failed to read " << errorHelp << "\n";
return {};
}
// Ok, let's check the record length, which is in bytes (unsigned,
- // up to 255, so blockData should hold uint8_t not char)
- size_t recordLength = blockData[2];
+ // up to 255, so areaData should hold uint8_t not char)
+ size_t recordLength = areaData[2];
areaOffset += (recordLength + multiRecordHeaderSize);
fruLength = (areaOffset > fruLength) ? areaOffset : fruLength;
// If this is the end of the list bail.
- if ((blockData[1] & multiRecordEndOfListMask) != 0)
+ if ((areaData[1] & multiRecordEndOfListMask) != 0)
{
break;
}
@@ -737,32 +700,60 @@
// You already copied these first 8 bytes (the ipmi fru header size)
fruLength -= std::min(fruBlockSize, fruLength);
+ device.resize(device.size() + fruLength);
- int readOffset = fruBlockSize;
-
- while (fruLength > 0)
+ int64_t nread = reader.read(static_cast<uint16_t>(fruBlockSize),
+ static_cast<uint8_t>(fruLength),
+ // NOLINTNEXTLINE(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ device.data() + (device.size() - fruLength));
+ if (nread != static_cast<int64_t>(fruLength))
{
- size_t requestLength =
- std::min(static_cast<size_t>(I2C_SMBUS_BLOCK_MAX), fruLength);
-
- if (reader.read(baseOffset + readOffset, requestLength,
- blockData.data()) < 0)
- {
- std::cerr << "failed to read " << errorHelp << " base offset "
- << baseOffset << "\n";
- return {};
- }
-
- device.insert(device.end(), blockData.begin(),
- blockData.begin() + requestLength);
-
- readOffset += requestLength;
- fruLength -= std::min(requestLength, fruLength);
+ std::cerr << "failed to read " << errorHelp << "\n";
+ return {};
}
return device;
}
+static std::vector<uint8_t> readTyanFRUContents(BaseFRUReader& reader,
+ const std::string& errorHelp)
+{
+ const std::array<uint8_t, 6> tyanMagic = {'$', 'T', 'Y', 'A', 'N', '$'};
+ std::array<uint8_t, tyanMagic.size()> magicBytes{};
+
+ if (reader.read(0, magicBytes.size(), magicBytes.data()) !=
+ magicBytes.size() ||
+ magicBytes != tyanMagic)
+ {
+ return {};
+ }
+
+ OffsetFRUReader tyanReader(reader, 0x6000);
+
+ return readIPMIFRUContents(tyanReader, errorHelp);
+}
+
+std::vector<uint8_t> readFRUContents(BaseFRUReader& reader,
+ const std::string& errorHelp)
+{
+ using FRUParser = std::vector<uint8_t> (*)(BaseFRUReader&, const std::string&);
+ static constexpr auto fruParsers = std::to_array<FRUParser>({
+ readIPMIFRUContents,
+ readTyanFRUContents,
+ });
+
+ for (const auto& parser : fruParsers)
+ {
+ std::vector<uint8_t> data = parser(reader, errorHelp);
+ if (!data.empty())
+ {
+ return data;
+ }
+ }
+
+ return {};
+}
+
unsigned int getHeaderAreaFieldOffset(fruAreas area)
{
return static_cast<unsigned int>(area) + 1;
diff --git a/test/test_fru-utils.cpp b/test/test_fru-utils.cpp
index 9470e4c..498afec 100644
--- a/test/test_fru-utils.cpp
+++ b/test/test_fru-utils.cpp
@@ -6,57 +6,51 @@
#include "gtest/gtest.h"
-extern "C"
-{
-// Include for I2C_SMBUS_BLOCK_MAX
-#include <linux/i2c.h>
-}
-
static constexpr size_t blockSize = I2C_SMBUS_BLOCK_MAX;
-TEST(ValidateHeaderTest, InvalidFruVersionReturnsFalse)
+TEST(ValidateIPMIHeaderTest, InvalidFruVersionReturnsFalse)
{
// Validates the FruVersion is checked for the only legal value.
- constexpr std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> fruHeader = {
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ const std::vector<uint8_t> fruHeader = {0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00};
- EXPECT_FALSE(validateHeader(fruHeader));
+ EXPECT_FALSE(validateIPMIHeader(fruHeader));
}
-TEST(ValidateHeaderTest, InvalidReservedReturnsFalse)
+TEST(ValidateIPMIHeaderTest, InvalidReservedReturnsFalse)
{
// Validates the reserved bit(7:4) of first bytes.
- constexpr std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> fruHeader = {
- 0xf0, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
+ const std::vector<uint8_t> fruHeader = {0xf0, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x00, 0x00};
- EXPECT_FALSE(validateHeader(fruHeader));
+ EXPECT_FALSE(validateIPMIHeader(fruHeader));
}
-TEST(ValidateHeaderTest, InvalidPaddingReturnsFalse)
+TEST(ValidateIPMIHeaderTest, InvalidPaddingReturnsFalse)
{
// Validates the padding byte (7th byte).
- constexpr std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> fruHeader = {
- 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00};
+ const std::vector<uint8_t> fruHeader = {0x00, 0x00, 0x00, 0x00,
+ 0x00, 0x00, 0x01, 0x00};
- EXPECT_FALSE(validateHeader(fruHeader));
+ EXPECT_FALSE(validateIPMIHeader(fruHeader));
}
-TEST(ValidateHeaderTest, InvalidChecksumReturnsFalse)
+TEST(ValidateIPMIHeaderTest, InvalidChecksumReturnsFalse)
{
// Validates the checksum, check for incorrect value.
- constexpr std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> fruHeader = {
- 0x01, 0x00, 0x01, 0x02, 0x03, 0x04, 0x00, 0x00};
+ const std::vector<uint8_t> fruHeader = {0x01, 0x00, 0x01, 0x02,
+ 0x03, 0x04, 0x00, 0x00};
- EXPECT_FALSE(validateHeader(fruHeader));
+ EXPECT_FALSE(validateIPMIHeader(fruHeader));
}
-TEST(ValidateHeaderTest, ValidChecksumReturnsTrue)
+TEST(ValidateIPMIHeaderTest, ValidChecksumReturnsTrue)
{
// Validates the checksum, check for correct value.
- constexpr std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> fruHeader = {
- 0x01, 0x00, 0x01, 0x02, 0x03, 0x04, 0x00, 0xf5};
+ const std::vector<uint8_t> fruHeader = {0x01, 0x00, 0x01, 0x02,
+ 0x03, 0x04, 0x00, 0xf5};
- EXPECT_TRUE(validateHeader(fruHeader));
+ EXPECT_TRUE(validateIPMIHeader(fruHeader));
}
TEST(VerifyOffsetTest, EmptyFruDataReturnsFalse)
@@ -302,89 +296,102 @@
EXPECT_EQ(reader.read(data.size() - 1, 2, rdbuf.data()), 1);
}
-TEST(FindFRUHeaderTest, InvalidHeader)
+static constexpr auto testFRUData = std::to_array<uint8_t>({
+ 0x01, 0x00, 0x01, 0x05, 0x0d, 0x00, 0x00, 0xec, 0x01, 0x04, 0x11, 0xd1,
+ 0x4f, 0x42, 0x4d, 0x43, 0x5f, 0x54, 0x45, 0x53, 0x54, 0x5f, 0x43, 0x48,
+ 0x41, 0x53, 0x53, 0x49, 0x53, 0xc6, 0x58, 0x59, 0x5a, 0x31, 0x32, 0x33,
+ 0xc1, 0x00, 0x00, 0xc4, 0x01, 0x08, 0x19, 0xd8, 0xe7, 0xd1, 0xcf, 0x4f,
+ 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x20, 0x50, 0x72, 0x6f, 0x6a, 0x65,
+ 0x63, 0x74, 0xcf, 0x4f, 0x42, 0x4d, 0x43, 0x5f, 0x54, 0x45, 0x53, 0x54,
+ 0x5f, 0x42, 0x4f, 0x41, 0x52, 0x44, 0xc6, 0x41, 0x42, 0x43, 0x30, 0x31,
+ 0x32, 0xcd, 0x42, 0x4f, 0x41, 0x52, 0x44, 0x5f, 0x50, 0x41, 0x52, 0x54,
+ 0x4e, 0x55, 0x4d, 0xc0, 0xc1, 0x00, 0x00, 0x73, 0x01, 0x06, 0x19, 0xcf,
+ 0x4f, 0x70, 0x65, 0x6e, 0x42, 0x4d, 0x43, 0x20, 0x50, 0x72, 0x6f, 0x6a,
+ 0x65, 0x63, 0x74, 0xd1, 0x4f, 0x42, 0x4d, 0x43, 0x5f, 0x54, 0x45, 0x53,
+ 0x54, 0x5f, 0x50, 0x52, 0x4f, 0x44, 0x55, 0x43, 0x54, 0xc0, 0xc0, 0xc4,
+ 0x39, 0x39, 0x39, 0x39, 0xc0, 0xc0, 0xc1, 0x3c,
+});
+
+TEST(ReadFRUContentsTest, InvalidHeader)
{
const std::vector<uint8_t> data = {255, 16};
- off_t offset = 0;
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
auto getData = [&data](auto o, auto l, auto* b) {
return getDataTempl(data, o, l, b);
};
FRUReader reader(getData);
- EXPECT_FALSE(findFRUHeader(reader, "error", blockData, offset));
+ EXPECT_TRUE(readFRUContents(reader, "error").empty());
}
-TEST(FindFRUHeaderTest, NoData)
+TEST(ReadFRUContentsTest, NoData)
{
const std::vector<uint8_t> data = {};
- off_t offset = 0;
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
auto getData = [&data](auto o, auto l, auto* b) {
return getDataTempl(data, o, l, b);
};
FRUReader reader(getData);
- EXPECT_FALSE(findFRUHeader(reader, "error", blockData, offset));
+ EXPECT_TRUE(readFRUContents(reader, "error").empty());
}
-TEST(FindFRUHeaderTest, ValidHeader)
+TEST(ReadFRUContentsTest, IPMIValidData)
{
- const std::vector<uint8_t> data = {0x01, 0x00, 0x01, 0x02,
- 0x03, 0x04, 0x00, 0xf5};
- off_t offset = 0;
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
+ const std::vector<uint8_t> data(testFRUData.begin(), testFRUData.end());
auto getData = [&data](auto o, auto l, auto* b) {
return getDataTempl(data, o, l, b);
};
FRUReader reader(getData);
- EXPECT_TRUE(findFRUHeader(reader, "error", blockData, offset));
- EXPECT_EQ(0, offset);
+ EXPECT_EQ(readFRUContents(reader, "error"), data);
}
-TEST(FindFRUHeaderTest, TyanInvalidHeader)
+TEST(ReadFRUContentsTest, IPMIInvalidData)
+{
+ std::vector<uint8_t> data(testFRUData.begin(), testFRUData.end());
+ // corrupt a byte to throw off the checksum
+ data[7] ^= 0xff;
+ auto getData = [&data](auto o, auto l, auto* b) {
+ return getDataTempl(data, o, l, b);
+ };
+ FRUReader reader(getData);
+
+ EXPECT_TRUE(readFRUContents(reader, "error").empty());
+}
+
+TEST(ReadFRUContentsTest, TyanInvalidHeader)
{
std::vector<uint8_t> data = {'$', 'T', 'Y', 'A', 'N', '$', 0, 0};
- data.resize(0x6000 + I2C_SMBUS_BLOCK_MAX);
- off_t offset = 0;
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
+ data.resize(0x6000 + 32);
auto getData = [&data](auto o, auto l, auto* b) {
return getDataTempl(data, o, l, b);
};
FRUReader reader(getData);
- EXPECT_FALSE(findFRUHeader(reader, "error", blockData, offset));
+ EXPECT_TRUE(readFRUContents(reader, "error").empty());
}
-TEST(FindFRUHeaderTest, TyanNoData)
+TEST(ReadFRUContentsTest, TyanNoData)
{
const std::vector<uint8_t> data = {'$', 'T', 'Y', 'A', 'N', '$', 0, 0};
- off_t offset = 0;
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
auto getData = [&data](auto o, auto l, auto* b) {
return getDataTempl(data, o, l, b);
};
FRUReader reader(getData);
- EXPECT_FALSE(findFRUHeader(reader, "error", blockData, offset));
+ EXPECT_TRUE(readFRUContents(reader, "error").empty());
}
-TEST(FindFRUHeaderTest, TyanValidHeader)
+TEST(ReadFRUContentsTest, TyanValidData)
{
std::vector<uint8_t> data = {'$', 'T', 'Y', 'A', 'N', '$', 0, 0};
data.resize(0x6000);
- constexpr std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> fruHeader = {
- 0x01, 0x00, 0x01, 0x02, 0x03, 0x04, 0x00, 0xf5};
- copy(fruHeader.begin(), fruHeader.end(), back_inserter(data));
+ copy(testFRUData.begin(), testFRUData.end(), back_inserter(data));
- off_t offset = 0;
- std::array<uint8_t, I2C_SMBUS_BLOCK_MAX> blockData{};
auto getData = [&data](auto o, auto l, auto* b) {
return getDataTempl(data, o, l, b);
};
FRUReader reader(getData);
- EXPECT_TRUE(findFRUHeader(reader, "error", blockData, offset));
- EXPECT_EQ(0x6000, offset);
+ std::vector<uint8_t> device = readFRUContents(reader, "error");
+ EXPECT_TRUE(std::equal(device.begin(), device.end(), testFRUData.begin()));
}