update DCMI Get Power Reading command
DCMI Get Power Reading command was still using the old IPMI API and
blocking dbus calls. This updates it to use the new API and yielding
dbus calls.
Tested: 'ipmitool dcmi power reading' works the same as before
Change-Id: Id8b732bd6ebf2a4d7087fa6fb47edcf35f20fee8
Signed-off-by: Vernon Mauery <vernon.mauery@linux.intel.com>
diff --git a/dcmihandler.cpp b/dcmihandler.cpp
index 26b32d1..2e626a3 100644
--- a/dcmihandler.cpp
+++ b/dcmihandler.cpp
@@ -52,9 +52,6 @@
// SendHostNameEnabled will set to true.
constexpr auto DHCP_OPT12_ENABLED = "SendHostNameEnabled";
-constexpr auto SENSOR_VALUE_INTF = "xyz.openbmc_project.Sensor.Value";
-constexpr auto SENSOR_VALUE_PROP = "Value";
-
using namespace phosphor::logging;
namespace dcmi
@@ -823,53 +820,6 @@
return ipmi::responseSuccess(totalInstances, numInstances, temps);
}
-int64_t getPowerReading(sdbusplus::bus_t& bus)
-{
- std::ifstream sensorFile(POWER_READING_SENSOR);
- std::string objectPath;
- if (!sensorFile.is_open())
- {
- log<level::ERR>("Power reading configuration file not found",
- entry("POWER_SENSOR_FILE=%s", POWER_READING_SENSOR));
- elog<InternalFailure>();
- }
-
- auto data = nlohmann::json::parse(sensorFile, nullptr, false);
- if (data.is_discarded())
- {
- log<level::ERR>("Error in parsing configuration file",
- entry("POWER_SENSOR_FILE=%s", POWER_READING_SENSOR));
- elog<InternalFailure>();
- }
-
- objectPath = data.value("path", "");
- if (objectPath.empty())
- {
- log<level::ERR>("Power sensor D-Bus object path is empty",
- entry("POWER_SENSOR_FILE=%s", POWER_READING_SENSOR));
- elog<InternalFailure>();
- }
-
- // Return default value if failed to read from D-Bus object
- int64_t power = 0;
- try
- {
- auto service = ipmi::getService(bus, SENSOR_VALUE_INTF, objectPath);
-
- // Read the sensor value and scale properties
- auto value = ipmi::getDbusProperty(
- bus, service, objectPath, SENSOR_VALUE_INTF, SENSOR_VALUE_PROP);
- power = std::get<double>(value);
- }
- catch (const std::exception& e)
- {
- log<level::ERR>("Failure to read power value from D-Bus object",
- entry("OBJECT_PATH=%s", objectPath.c_str()),
- entry("INTERFACE=%s", SENSOR_VALUE_INTF));
- }
- return power;
-}
-
ipmi_ret_t setDCMIConfParams(ipmi_netfn_t, ipmi_cmd_t, ipmi_request_t request,
ipmi_response_t, ipmi_data_len_t data_len,
ipmi_context_t)
@@ -1010,47 +960,117 @@
return IPMI_CC_OK;
}
-ipmi_ret_t getPowerReading(ipmi_netfn_t, ipmi_cmd_t, ipmi_request_t,
- ipmi_response_t response, ipmi_data_len_t data_len,
- ipmi_context_t)
+static std::optional<uint16_t> readPower(ipmi::Context::ptr& ctx)
{
- *data_len = 0;
+ std::ifstream sensorFile(POWER_READING_SENSOR);
+ std::string objectPath;
+ if (!sensorFile.is_open())
+ {
+ log<level::ERR>("Power reading configuration file not found",
+ entry("POWER_SENSOR_FILE=%s", POWER_READING_SENSOR));
+ return std::nullopt;
+ }
+
+ auto data = nlohmann::json::parse(sensorFile, nullptr, false);
+ if (data.is_discarded())
+ {
+ log<level::ERR>("Error in parsing configuration file",
+ entry("POWER_SENSOR_FILE=%s", POWER_READING_SENSOR));
+ return std::nullopt;
+ }
+
+ objectPath = data.value("path", "");
+ if (objectPath.empty())
+ {
+ log<level::ERR>("Power sensor D-Bus object path is empty",
+ entry("POWER_SENSOR_FILE=%s", POWER_READING_SENSOR));
+ return std::nullopt;
+ }
+
+ // Return default value if failed to read from D-Bus object
+ std::string service{};
+ boost::system::error_code ec = ipmi::getService(ctx, dcmi::sensorValueIntf,
+ objectPath, service);
+ if (ec.value())
+ {
+ log<level::ERR>("Failed to fetch service for D-Bus object",
+ entry("OBJECT_PATH=%s", objectPath.c_str()),
+ entry("INTERFACE=%s", dcmi::sensorValueIntf));
+ return std::nullopt;
+ }
+
+ // Read the sensor value and scale properties
+ double value{};
+ ec = ipmi::getDbusProperty(ctx, service, objectPath, dcmi::sensorValueIntf,
+ dcmi::sensorValueProp, value);
+ if (ec.value())
+ {
+ log<level::ERR>("Failure to read power value from D-Bus object",
+ entry("OBJECT_PATH=%s", objectPath.c_str()),
+ entry("INTERFACE=%s", dcmi::sensorValueIntf));
+ return std::nullopt;
+ }
+ auto power = static_cast<uint16_t>(value);
+ return power;
+}
+
+ipmi::RspType<uint16_t, // current power
+ uint16_t, // minimum power
+ uint16_t, // maximum power
+ uint16_t, // average power
+ uint32_t, // timestamp
+ uint32_t, // sample period ms
+ uint6_t, // reserved
+ bool, // power measurement active
+ bool // reserved
+ >
+ getPowerReading(ipmi::Context::ptr& ctx, uint8_t mode, uint8_t attributes,
+ uint8_t reserved)
+{
if (!dcmi::isDCMIPowerMgmtSupported())
{
log<level::ERR>("DCMI Power management is unsupported!");
- return IPMI_CC_INVALID;
+ return ipmi::responseInvalidCommand();
+ }
+ if (reserved)
+ {
+ return ipmi::responseInvalidFieldRequest();
}
- ipmi_ret_t rc = IPMI_CC_OK;
- auto responseData =
- reinterpret_cast<dcmi::GetPowerReadingResponse*>(response);
+ enum class PowerMode : uint8_t
+ {
+ SystemPowerStatistics = 1,
+ EnhancedSystemPowerStatistics = 2,
+ };
- sdbusplus::bus_t bus{ipmid_get_sd_bus_connection()};
- int64_t power = 0;
- try
+ if (static_cast<PowerMode>(mode) != PowerMode::SystemPowerStatistics)
{
- power = getPowerReading(bus);
+ return ipmi::responseInvalidFieldRequest();
}
- catch (const InternalFailure& e)
+ if (attributes)
{
- log<level::ERR>("Error in reading power sensor value",
- entry("INTERFACE=%s", SENSOR_VALUE_INTF),
- entry("PROPERTY=%s", SENSOR_VALUE_PROP));
- return IPMI_CC_UNSPECIFIED_ERROR;
+ return ipmi::responseInvalidFieldRequest();
}
+ std::optional<uint16_t> powerResp = readPower(ctx);
+ if (!powerResp)
+ {
+ return ipmi::responseUnspecifiedError();
+ }
+ auto& power = powerResp.value();
+
// TODO: openbmc/openbmc#2819
// Minimum, Maximum, Average power, TimeFrame, TimeStamp,
// PowerReadingState readings need to be populated
// after Telemetry changes.
- uint16_t totalPower = static_cast<uint16_t>(power);
- responseData->currentPower = totalPower;
- responseData->minimumPower = totalPower;
- responseData->maximumPower = totalPower;
- responseData->averagePower = totalPower;
-
- *data_len = sizeof(*responseData);
- return rc;
+ constexpr uint32_t samplePeriod = 1;
+ constexpr uint6_t reserved1 = 0;
+ constexpr bool measurementActive = true;
+ constexpr bool reserved2 = false;
+ auto timestamp = static_cast<uint32_t>(time(nullptr));
+ return ipmi::responseSuccess(power, power, power, power, timestamp,
+ samplePeriod, reserved1, measurementActive,
+ reserved2);
}
namespace dcmi
@@ -1278,8 +1298,10 @@
ipmi::Privilege::User, getTempReadings);
// <Get Power Reading>
- ipmi_register_callback(NETFUN_GRPEXT, dcmi::Commands::GET_POWER_READING,
- NULL, getPowerReading, PRIVILEGE_USER);
+ registerGroupHandler(ipmi::prioOpenBmcBase, ipmi::groupDCMI,
+ ipmi::dcmi::cmdGetPowerReading, ipmi::Privilege::User,
+ getPowerReading);
+
// The Get sensor should get the senor details dynamically when
// FEATURE_DYNAMIC_SENSORS is enabled.
#ifndef FEATURE_DYNAMIC_SENSORS
diff --git a/dcmihandler.hpp b/dcmihandler.hpp
index 4a739c2..c2f2e61 100644
--- a/dcmihandler.hpp
+++ b/dcmihandler.hpp
@@ -16,7 +16,6 @@
enum Commands
{
- GET_POWER_READING = 0x02,
GET_SENSOR_INFO = 0x07,
SET_CONF_PARAMS = 0x12,
GET_CONF_PARAMS = 0x13,
@@ -125,45 +124,6 @@
readAll(const std::string& type, uint8_t instanceStart, const Json& config);
} // namespace sensor_info
-/** @brief Read power reading from power reading sensor object
- *
- * @param[in] bus - dbus connection
- *
- * @return total power reading
- */
-int64_t getPowerReading(sdbusplus::bus_t& bus);
-
-/** @struct GetPowerReadingRequest
- *
- * DCMI Get Power Reading command request.
- * Refer DCMI specification Version 1.1 Section 6.6.1
- */
-struct GetPowerReadingRequest
-{
- uint8_t mode; //!< Mode
- uint8_t modeAttribute; //!< Mode Attributes
-} __attribute__((packed));
-
-/** @struct GetPowerReadingResponse
- *
- * DCMI Get Power Reading command response.
- * Refer DCMI specification Version 1.1 Section 6.6.1
- */
-struct GetPowerReadingResponse
-{
- uint16_t currentPower; //!< Current power in watts
- uint16_t minimumPower; //!< Minimum power over sampling duration
- //!< in watts
- uint16_t maximumPower; //!< Maximum power over sampling duration
- //!< in watts
- uint16_t averagePower; //!< Average power over sampling duration
- //!< in watts
- uint32_t timeStamp; //!< IPMI specification based time stamp
- uint32_t timeFrame; //!< Statistics reporting time period in milli
- //!< seconds.
- uint8_t powerReadingState; //!< Power Reading State
-} __attribute__((packed));
-
/** @struct GetSensorInfoRequest
*
* DCMI payload for Get Sensor Info request