Enable OEM creation of non-Type1 SDRs
Enabling the dbus-sdrs feature is useful for managing Type 1 SDR's
using D-Bus. What doesn't work quite as well in the model is creating
non-Type 1 records. The current method works alright for Type 11 FRU
records. Any other SDR's beyond the FRU are problematic because the
code depends on manually defining custom 'if' clauses to compare the
incoming SDR ID, and determining what kind of SDR to create. It is a
fixed process that is inflexible, and assumes every BMC vendor wants
the same SDR arrangement.
This commit creates a model that allows the each OEM to customize
creating each SDR using their own algorithm. The OEM creates a
sensorcommands_oem.cpp/hpp file containing code for handling the
creation of custom SDRs. The code here is compiled and linked based on
enabling a Meson build switch.
The code follows the model that was already present in
dbus-sdr/sensorcommands.cpp. There are two functions that maintain the
original inflexible code, which is now expected to be primarily used
as a template for the OEM cpp/hpp contents.
Tested:
Created sensorcommands_oem.cpp/hpp files with OEM functionality
Enabled the original code, and compiled
Used ipmitool sdr dump sdrs.bin in the BMC Console
Confirmed the SDRs matched the values prior to this commit.
Enabled the OEM code and compiled
Used ipmitool sdr dump sdrs.bin in the BMC console
Confirmed the contents of the BIN file contained the SDRs generated by
the sensorcommands_oem.cpp source.
Change-Id: I100e747b52677be53b499713d51c6c1126411570
Signed-off-by: Johnathan Mantey <johnathanx.mantey@intel.com>
diff --git a/dbus-sdr/meson.build b/dbus-sdr/meson.build
index a0363ec..2afb8a6 100644
--- a/dbus-sdr/meson.build
+++ b/dbus-sdr/meson.build
@@ -17,6 +17,11 @@
]
endif
+sensorsoem_src = []
+if not get_option('sensors-oem').disabled()
+ sensorsoem_src = ['dbus-sdr/sensorcommands_oem.cpp']
+endif
+
dbus_sdr_pre = declare_dependency(
include_directories: root_inc,
dependencies: [
@@ -32,4 +37,5 @@
'dbus-sdr/sensorcommands.cpp',
'dbus-sdr/storagecommands.cpp',
hybrid_src,
+ sensorsoem_src,
]
diff --git a/dbus-sdr/sensorcommands.cpp b/dbus-sdr/sensorcommands.cpp
index 5d23a19..1fbd5cf 100644
--- a/dbus-sdr/sensorcommands.cpp
+++ b/dbus-sdr/sensorcommands.cpp
@@ -40,7 +40,6 @@
#include <format>
#include <iostream>
#include <map>
-#include <memory>
#include <optional>
#include <stdexcept>
#include <string>
@@ -154,6 +153,47 @@
.count();
});
+ipmi_ret_t getSensorConnection(ipmi::Context::ptr ctx, uint8_t sensnum,
+ std::string& connection, std::string& path,
+ std::vector<std::string>* interfaces)
+{
+ auto& sensorTree = getSensorTree();
+ if (!getSensorSubtree(sensorTree) && sensorTree.empty())
+ {
+ return IPMI_CC_RESPONSE_ERROR;
+ }
+
+ if (ctx == nullptr)
+ {
+ return IPMI_CC_RESPONSE_ERROR;
+ }
+
+ path = getPathFromSensorNumber((ctx->lun << 8) | sensnum);
+ if (path.empty())
+ {
+ return IPMI_CC_INVALID_FIELD_REQUEST;
+ }
+
+ for (const auto& sensor : sensorTree)
+ {
+ if (path == sensor.first)
+ {
+ connection = sensor.second.begin()->first;
+ if (interfaces)
+ *interfaces = sensor.second.begin()->second;
+ break;
+ }
+ }
+
+ return 0;
+}
+
+SensorSubTree& getSensorTree()
+{
+ static SensorSubTree sensorTree;
+ return sensorTree;
+}
+
// this keeps track of deassertions for sensor event status command. A
// deasertion can only happen if an assertion was seen first.
static boost::container::flat_map<
@@ -566,6 +606,115 @@
}
return true;
}
+
+/*
+ * Handle every Sensor Data Record besides Type 01
+ *
+ * The D-Bus sensors work well for generating Type 01 SDRs.
+ * After the Type 01 sensors are processed the remaining sensor types require
+ * special handling. Each BMC vendor is going to have their own requirements for
+ * insertion of non-Type 01 records.
+ * Manage non-Type 01 records:
+ *
+ * Create a new file: dbus-sdr/sensorcommands_oem.cpp
+ * Populate it with the two weakly linked functions below, without adding the
+ * 'weak' attribute definition prior to the function definition.
+ * getOtherSensorsCount(...)
+ * getOtherSensorsDataRecord(...)
+ * Example contents are provided in the weak definitions below
+ * Enable 'sensors-oem' in your phosphor-ipmi-host bbappend file
+ * 'EXTRA_OEMESON:append = " -Dsensors-oem=enabled"'
+ * The contents of the sensorcommands_oem.cpp file will then override the code
+ * provided below.
+ */
+
+size_t getOtherSensorsCount(ipmi::Context::ptr ctx) __attribute__((weak));
+size_t getOtherSensorsCount(ipmi::Context::ptr ctx)
+{
+ size_t fruCount = 0;
+
+ ipmi::Cc ret = ipmi::storage::getFruSdrCount(ctx, fruCount);
+ if (ret != ipmi::ccSuccess)
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(
+ "getOtherSensorsCount: getFruSdrCount error");
+ return std::numeric_limits<size_t>::max();
+ }
+
+ const auto& entityRecords =
+ ipmi::sensor::EntityInfoMapContainer::getContainer()
+ ->getIpmiEntityRecords();
+ size_t entityCount = entityRecords.size();
+
+ return fruCount + ipmi::storage::type12Count + entityCount;
+}
+
+int getOtherSensorsDataRecord(ipmi::Context::ptr ctx, uint16_t recordID,
+ std::vector<uint8_t>& recordData)
+ __attribute__((weak));
+int getOtherSensorsDataRecord(ipmi::Context::ptr ctx, uint16_t recordID,
+ std::vector<uint8_t>& recordData)
+{
+ size_t otherCount{ipmi::sensor::getOtherSensorsCount(ctx)};
+ if (otherCount == std::numeric_limits<size_t>::max())
+ {
+ return GENERAL_ERROR;
+ }
+ const auto& entityRecords =
+ ipmi::sensor::EntityInfoMapContainer::getContainer()
+ ->getIpmiEntityRecords();
+
+ size_t sdrIndex(recordID - ipmi::getNumberOfSensors());
+ size_t entityCount{entityRecords.size()};
+ size_t fruCount{otherCount - ipmi::storage::type12Count - entityCount};
+
+ if (sdrIndex > otherCount)
+ {
+ return std::numeric_limits<int>::min();
+ }
+ else if (sdrIndex >= fruCount + ipmi::storage::type12Count)
+ {
+ // handle type 8 entity map records
+ ipmi::sensor::EntityInfoMap::const_iterator entity =
+ entityRecords.find(static_cast<uint8_t>(
+ sdrIndex - fruCount - ipmi::storage::type12Count));
+
+ if (entity == entityRecords.end())
+ {
+ return GENERAL_ERROR;
+ }
+ recordData = ipmi::storage::getType8SDRs(entity, recordID);
+ }
+ else if (sdrIndex >= fruCount)
+ {
+ // handle type 12 hardcoded records
+ size_t type12Index = sdrIndex - fruCount;
+ if (type12Index >= ipmi::storage::type12Count)
+ {
+ phosphor::logging::log<phosphor::logging::level::ERR>(
+ "getSensorDataRecord: type12Index error");
+ return GENERAL_ERROR;
+ }
+ recordData = ipmi::storage::getType12SDRs(type12Index, recordID);
+ }
+ else
+ {
+ // handle fru records
+ get_sdr::SensorDataFruRecord data;
+ if (ipmi::Cc ret = ipmi::storage::getFruSdrs(ctx, sdrIndex, data);
+ ret != IPMI_CC_OK)
+ {
+ return GENERAL_ERROR;
+ }
+ data.header.record_id_msb = recordID >> 8;
+ data.header.record_id_lsb = recordID & 0xFF;
+ recordData.insert(recordData.end(), reinterpret_cast<uint8_t*>(&data),
+ reinterpret_cast<uint8_t*>(&data) + sizeof(data));
+ }
+
+ return 0;
+}
+
} // namespace sensor
ipmi::RspType<> ipmiSenPlatformEvent(ipmi::Context::ptr ctx,
@@ -1671,7 +1820,7 @@
if (sensorObject == sensorMap.end())
{
phosphor::logging::log<phosphor::logging::level::ERR>(
- "getSensorDataRecord: sensorObject error");
+ "constructSensorSdr: sensorObject error");
return false;
}
@@ -1699,7 +1848,7 @@
if (!getSensorAttributes(max, min, mValue, rExp, bValue, bExp, bSigned))
{
phosphor::logging::log<phosphor::logging::level::ERR>(
- "getSensorDataRecord: getSensorAttributes error");
+ "constructSensorSdr: getSensorAttributes error");
return false;
}
@@ -1784,7 +1933,7 @@
catch (const std::exception&)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
- "getSensorDataRecord: getIPMIThresholds error");
+ "constructSensorSdr: getIPMIThresholds error");
return false;
}
@@ -1955,7 +2104,7 @@
return true;
}
-static inline uint16_t getNumberOfSensors()
+uint16_t getNumberOfSensors()
{
return std::min(getSensorTree().size(), maxIPMISensors);
}
@@ -1966,79 +2115,36 @@
std::vector<uint8_t>& recordData, uint16_t recordID,
uint8_t readBytes = std::numeric_limits<uint8_t>::max())
{
- size_t fruCount = 0;
- ipmi::Cc ret = ipmi::storage::getFruSdrCount(ctx, fruCount);
- if (ret != ipmi::ccSuccess)
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(
- "getSensorDataRecord: getFruSdrCount error");
- return GENERAL_ERROR;
- }
-
- const auto& entityRecords =
- ipmi::sensor::EntityInfoMapContainer::getContainer()
- ->getIpmiEntityRecords();
- size_t entityCount = entityRecords.size();
-
recordData.clear();
- size_t lastRecord = getNumberOfSensors() + fruCount +
- ipmi::storage::type12Count + entityCount - 1;
+ size_t lastRecord = ipmi::getNumberOfSensors() +
+ ipmi::sensor::getOtherSensorsCount(ctx) - 1;
+ uint16_t nextRecord(recordID + 1);
+
if (recordID == lastRecordIndex)
{
recordID = lastRecord;
}
+ if (recordID == lastRecord)
+ {
+ nextRecord = lastRecordIndex;
+ }
if (recordID > lastRecord)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
"getSensorDataRecord: recordID > lastRecord error");
return GENERAL_ERROR;
}
-
- if (recordID >= getNumberOfSensors())
+ if (recordID >= ipmi::getNumberOfSensors())
{
- size_t sdrIndex = recordID - getNumberOfSensors();
-
- if (sdrIndex >= fruCount + ipmi::storage::type12Count)
+ if (auto err = ipmi::sensor::getOtherSensorsDataRecord(ctx, recordID,
+ recordData);
+ err < 0)
{
- // handle type 8 entity map records
- ipmi::sensor::EntityInfoMap::const_iterator entity =
- entityRecords.find(static_cast<uint8_t>(
- sdrIndex - fruCount - ipmi::storage::type12Count));
- if (entity == entityRecords.end())
- {
- return IPMI_CC_SENSOR_INVALID;
- }
- recordData = ipmi::storage::getType8SDRs(entity, recordID);
+ // phosphor::logging::log<phosphor::logging::level::ERR>(
+ // "getSensorDataRecord: Error getting custom record");
+ return lastRecordIndex;
}
- else if (sdrIndex >= fruCount)
- {
- // handle type 12 hardcoded records
- size_t type12Index = sdrIndex - fruCount;
- if (type12Index >= ipmi::storage::type12Count)
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(
- "getSensorDataRecord: type12Index error");
- return GENERAL_ERROR;
- }
- recordData = ipmi::storage::getType12SDRs(type12Index, recordID);
- }
- else
- {
- // handle fru records
- get_sdr::SensorDataFruRecord data;
- ret = ipmi::storage::getFruSdrs(ctx, sdrIndex, data);
- if (ret != IPMI_CC_OK)
- {
- return GENERAL_ERROR;
- }
- data.header.record_id_msb = recordID >> 8;
- data.header.record_id_lsb = recordID & 0xFF;
- recordData.insert(recordData.end(),
- reinterpret_cast<uint8_t*>(&data),
- reinterpret_cast<uint8_t*>(&data) + sizeof(data));
- }
-
- return 0;
+ return nextRecord;
}
// Perform a incremental scan of the SDR Record ID's and translate the
@@ -2117,7 +2223,7 @@
recordData.insert(recordData.end(), reinterpret_cast<uint8_t*>(&record),
reinterpret_cast<uint8_t*>(&record) + sizeof(record));
- return 0;
+ return nextRecord;
}
#ifdef FEATURE_HYBRID_SENSORS
@@ -2142,7 +2248,7 @@
recordData.insert(recordData.end(), reinterpret_cast<uint8_t*>(&record),
reinterpret_cast<uint8_t*>(&record) + sizeof(record));
- return 0;
+ return nextRecord;
}
#endif
@@ -2167,7 +2273,7 @@
reinterpret_cast<uint8_t*>(&record) + sizeof(record));
}
- return 0;
+ return nextRecord;
}
/** @brief implements the get SDR Info command
@@ -2197,7 +2303,7 @@
{
return ipmi::responseResponseError();
}
- uint16_t numSensors = getNumberOfSensors();
+ uint16_t numSensors = ipmi::getNumberOfSensors();
if (count.value_or(0) == getSdrCount)
{
auto& ipmiDecoratorPaths = getIpmiDecoratorPaths(ctx);
@@ -2213,9 +2319,10 @@
// Count the number of Type 1h, Type 2h, Type 11h, Type 12h SDR entries
// assigned to the LUN
- while (!getSensorDataRecord(
- ctx, ipmiDecoratorPaths.value_or(std::unordered_set<std::string>()),
- record, recordID++))
+ while (getSensorDataRecord(ctx,
+ ipmiDecoratorPaths.value_or(
+ std::unordered_set<std::string>()),
+ record, recordID++) >= 0)
{
get_sdr::SensorDataRecordHeader* hdr =
reinterpret_cast<get_sdr::SensorDataRecordHeader*>(
@@ -2343,7 +2450,7 @@
return ipmi::response(ret);
}
- uint16_t recordCount = getNumberOfSensors() + fruCount +
+ uint16_t recordCount = ipmi::getNumberOfSensors() + fruCount +
ipmi::storage::type12Count;
uint8_t operationSupport = static_cast<uint8_t>(
@@ -2408,7 +2515,6 @@
ipmiStorageGetSDR(ipmi::Context::ptr ctx, uint16_t reservationID,
uint16_t recordID, uint8_t offset, uint8_t bytesToRead)
{
- size_t fruCount = 0;
// reservation required for partial reads with non zero offset into
// record
if ((sdrReservationID == 0 || reservationID != sdrReservationID) && offset)
@@ -2417,24 +2523,8 @@
"ipmiStorageGetSDR: responseInvalidReservationId");
return ipmi::responseInvalidReservationId();
}
- ipmi::Cc ret = ipmi::storage::getFruSdrCount(ctx, fruCount);
- if (ret != ipmi::ccSuccess)
- {
- phosphor::logging::log<phosphor::logging::level::ERR>(
- "ipmiStorageGetSDR: getFruSdrCount error");
- return ipmi::response(ret);
- }
-
- const auto& entityRecords =
- ipmi::sensor::EntityInfoMapContainer::getContainer()
- ->getIpmiEntityRecords();
- int entityCount = entityRecords.size();
auto& sensorTree = getSensorTree();
- size_t lastRecord = getNumberOfSensors() + fruCount +
- ipmi::storage::type12Count + entityCount - 1;
- uint16_t nextRecordId = lastRecord > recordID ? recordID + 1 : 0XFFFF;
-
if (!getSensorSubtree(sensorTree) && sensorTree.empty())
{
phosphor::logging::log<phosphor::logging::level::ERR>(
@@ -2445,9 +2535,11 @@
auto& ipmiDecoratorPaths = getIpmiDecoratorPaths(ctx);
std::vector<uint8_t> record;
- if (getSensorDataRecord(
- ctx, ipmiDecoratorPaths.value_or(std::unordered_set<std::string>()),
- record, recordID, offset + bytesToRead))
+ int nextRecordId = getSensorDataRecord(
+ ctx, ipmiDecoratorPaths.value_or(std::unordered_set<std::string>()),
+ record, recordID, offset + bytesToRead);
+
+ if (nextRecordId < 0)
{
phosphor::logging::log<phosphor::logging::level::ERR>(
"ipmiStorageGetSDR: fail to get SDR");
diff --git a/include/dbus-sdr/sensorcommands.hpp b/include/dbus-sdr/sensorcommands.hpp
index cde3ffb..df51f47 100644
--- a/include/dbus-sdr/sensorcommands.hpp
+++ b/include/dbus-sdr/sensorcommands.hpp
@@ -119,47 +119,13 @@
namespace ipmi
{
-SensorSubTree& getSensorTree()
-{
- static SensorSubTree sensorTree;
- return sensorTree;
-}
+uint16_t getNumberOfSensors();
-static ipmi_ret_t
- getSensorConnection(ipmi::Context::ptr ctx, uint8_t sensnum,
- std::string& connection, std::string& path,
- std::vector<std::string>* interfaces = nullptr)
-{
- auto& sensorTree = getSensorTree();
- if (!getSensorSubtree(sensorTree) && sensorTree.empty())
- {
- return IPMI_CC_RESPONSE_ERROR;
- }
+SensorSubTree& getSensorTree();
- if (ctx == nullptr)
- {
- return IPMI_CC_RESPONSE_ERROR;
- }
-
- path = getPathFromSensorNumber((ctx->lun << 8) | sensnum);
- if (path.empty())
- {
- return IPMI_CC_INVALID_FIELD_REQUEST;
- }
-
- for (const auto& sensor : sensorTree)
- {
- if (path == sensor.first)
- {
- connection = sensor.second.begin()->first;
- if (interfaces)
- *interfaces = sensor.second.begin()->second;
- break;
- }
- }
-
- return 0;
-}
+ipmi_ret_t getSensorConnection(ipmi::Context::ptr ctx, uint8_t sensnum,
+ std::string& connection, std::string& path,
+ std::vector<std::string>* interfaces = nullptr);
struct IPMIThresholds
{
@@ -169,6 +135,31 @@
std::optional<uint8_t> criticalHigh;
};
+namespace sensor
+{
+/**
+ * @brief Retrieve the number of sensors that are not included in the list of
+ * sensors published via D-Bus
+ *
+ * @param[in]: ctx: the pointer to the D-Bus context
+ * @return: The number of additional sensors separate from those published
+ * dynamically on D-Bus
+ */
+size_t getOtherSensorsCount(ipmi::Context::ptr ctx);
+
+/**
+ * @brief Retrieve the record data for the sensors not published via D-Bus
+ *
+ * @param[in]: ctx: the pointer to the D-Bus context
+ * @param[in]: recordID: the integer index for the sensor to retrieve
+ * @param[out]: SDR data for the indexed sensor
+ * @return: 0: success
+ * negative number: error condition
+ */
+int getOtherSensorsDataRecord(ipmi::Context::ptr ctx, uint16_t recordID,
+ std::vector<uint8_t>& recordData);
+} // namespace sensor
+
namespace dcmi
{
diff --git a/meson.options b/meson.options
index c6d2425..fff1651 100644
--- a/meson.options
+++ b/meson.options
@@ -56,6 +56,7 @@
option('dynamic-sensors', type: 'feature', value: 'disabled', description: 'Dynamic sensors stack is enabled by default; offer a way to disable it')
option('dynamic-sensors-write', type: 'feature', value: 'disabled', description: 'Dynamic sensors stack is enabled by default; offer a way to disable it')
option('hybrid-sensors', type: 'feature', value: 'disabled', description: 'Hybrid sensors stack is disabled by default; offer a way to enable it')
+option('sensors-oem', type: 'feature', value: 'disabled', description: 'OEM sensor SDR parsing is disabled by default; offer a way to enable it')
# Sensor Cache
option('sensors-cache', type: 'feature', value: 'disabled', description: 'Sensor cache stack is disabled by default; offer a way to enable it')