Guarantee SDR Type12 byte alignment matches the IPMI spec
The Type12 structure used a uint24_t to assign storage for 3 reserved
bytes in the Type12 SDR definition. This turned out to be an issue as
the compiler promoted the uint24_t to a uint32_t, which added one
additional byte to the structure. This in turn pushed the trailing
bytes out of alignment.
The defintion has been changed to use an explicit uint8_t array that
is three bytes in size. This defines the structure in a way that
guarantees the size of the record is correct.
The structure has also had a constructor added to it to eliminate code
duplication, and to reduce the likelihood of errors caused by manual
calculation of values.
Tested:
Issued 'ipmitool sdr dump /tmp/sdrs.bin'
Performed a hex dump on the binary data and reviewed the Type12
records.
Confirmed all fields were correctly assigned, including the inclusion
of only 3 reserved bytes.
Change-Id: I3fef5a1fe67877e5a6cf6e7928c5f15599bfb6f6
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
diff --git a/include/storagecommands.hpp b/include/storagecommands.hpp
index c9e2bd2..42cc85b 100644
--- a/include/storagecommands.hpp
+++ b/include/storagecommands.hpp
@@ -103,12 +103,34 @@
uint8_t channelNumber;
uint8_t powerStateNotification;
uint8_t deviceCapabilities;
- uint24_t reserved;
+ // define reserved bytes explicitly. The uint24_t is silently expanded to
+ // uint32_t, which ruins the byte alignment required by this structure.
+ uint8_t reserved[3];
uint8_t entityID;
uint8_t entityInstance;
uint8_t oem;
uint8_t typeLengthCode;
char name[16];
+
+ Type12Record(uint16_t recordID, uint8_t address, uint8_t chNumber,
+ uint8_t pwrStateNotification, uint8_t capabilities,
+ uint8_t eid, uint8_t entityInst, uint8_t mfrDefined,
+ const std::string& sensorname) :
+ slaveAddress(address),
+ channelNumber(chNumber), powerStateNotification(pwrStateNotification),
+ deviceCapabilities(capabilities), reserved{}, entityID(eid),
+ entityInstance(entityInst), oem(mfrDefined)
+ {
+ get_sdr::header::set_record_id(recordID, &header);
+ header.sdr_version = ipmiSdrVersion;
+ header.record_type = 0x12;
+ size_t nameLen = std::min(sensorname.size(), sizeof(name));
+ header.record_length = sizeof(Type12Record) -
+ sizeof(get_sdr::SensorDataRecordHeader) -
+ sizeof(name) + nameLen;
+ typeLengthCode = 0xc0 | nameLen;
+ std::copy(sensorname.begin(), sensorname.begin() + nameLen, name);
+ }
};
#pragma pack(pop)
diff --git a/src/storagecommands.cpp b/src/storagecommands.cpp
index bd43a34..e9dc344 100644
--- a/src/storagecommands.cpp
+++ b/src/storagecommands.cpp
@@ -1219,45 +1219,15 @@
std::vector<uint8_t> resp;
if (index == 0)
{
- Type12Record bmc = {};
- bmc.header.record_id_lsb = recordId;
- bmc.header.record_id_msb = recordId >> 8;
- bmc.header.sdr_version = ipmiSdrVersion;
- bmc.header.record_type = 0x12;
- bmc.header.record_length = 0x1b;
- bmc.slaveAddress = 0x20;
- bmc.channelNumber = 0;
- bmc.powerStateNotification = 0;
- bmc.deviceCapabilities = 0xBF;
- bmc.reserved = 0;
- bmc.entityID = 0x2E;
- bmc.entityInstance = 1;
- bmc.oem = 0;
- bmc.typeLengthCode = 0xD0;
std::string bmcName = "Basbrd Mgmt Ctlr";
- std::copy(bmcName.begin(), bmcName.end(), bmc.name);
+ Type12Record bmc(recordId, 0x20, 0, 0, 0xbf, 0x2e, 1, 0, bmcName);
uint8_t* bmcPtr = reinterpret_cast<uint8_t*>(&bmc);
resp.insert(resp.end(), bmcPtr, bmcPtr + sizeof(Type12Record));
}
else if (index == 1)
{
- Type12Record me = {};
- me.header.record_id_lsb = recordId;
- me.header.record_id_msb = recordId >> 8;
- me.header.sdr_version = ipmiSdrVersion;
- me.header.record_type = 0x12;
- me.header.record_length = 0x16;
- me.slaveAddress = 0x2C;
- me.channelNumber = 6;
- me.powerStateNotification = 0x24;
- me.deviceCapabilities = 0x21;
- me.reserved = 0;
- me.entityID = 0x2E;
- me.entityInstance = 2;
- me.oem = 0;
- me.typeLengthCode = 0xCB;
std::string meName = "Mgmt Engine";
- std::copy(meName.begin(), meName.end(), me.name);
+ Type12Record me(recordId, 0x2c, 6, 0x24, 0x21, 0x2e, 2, 0, meName);
uint8_t* mePtr = reinterpret_cast<uint8_t*>(&me);
resp.insert(resp.end(), mePtr, mePtr + sizeof(Type12Record));
}