Fix detection of invalid 'GetSensorReading'
This change fixes incorrect way to determine Get Sensor Reading value
availability.
In previous implementation proper value of '0' was treated as error.
IPMI spec defines 'reading/state unavailable' bit, which ME uses to
inform user about sensor reading being absent.
This change monitors that bit to determine outcome of Get Sensor Reading
call.
Alternative considered was to also look at 'sensor scanning disabled',
but after reading more documentation and checking ME implementation it
turns out that it affects only threshold events generation.
Testing:
- verified that 'nan' is set for unavailable sensor (PSU with PMBUS
cable cut-off)
- verified that for available sensor reporting '0', '0' value is
present instead of 'nan'
Signed-off-by: Adrian Ambrożewicz <adrian.ambrozewicz@linux.intel.com>
Change-Id: Iaa940b808e36b9606fe78f7cef7bab2593275889
diff --git a/include/IpmbSensor.hpp b/include/IpmbSensor.hpp
index e1b1ddb..3b3ee15 100644
--- a/include/IpmbSensor.hpp
+++ b/include/IpmbSensor.hpp
@@ -37,6 +37,42 @@
elevenBitShift,
};
+namespace ipmi
+{
+namespace sensor
+{
+constexpr uint8_t netFn = 0x04;
+constexpr uint8_t getSensorReading = 0x2d;
+
+static bool isValid(const std::vector<uint8_t>& data)
+{
+ constexpr auto ReadingUnavailableBit = 5;
+
+ // Proper 'Get Sensor Reading' response has at least 4 bytes, including
+ // Completion Code. Our IPMB stack strips Completion Code from payload so we
+ // compare here against the rest of payload
+ if (data.size() < 3)
+ {
+ return false;
+ }
+
+ // Per IPMI 'Get Sensor Reading' specification
+ if (data[1] & (1 << ReadingUnavailableBit))
+ {
+ return false;
+ }
+
+ return true;
+}
+
+} // namespace sensor
+namespace me_bridge
+{
+constexpr uint8_t netFn = 0x2e;
+constexpr uint8_t sendRawPmbus = 0xd9;
+} // namespace me_bridge
+} // namespace ipmi
+
struct IpmbSensor : public Sensor
{
IpmbSensor(std::shared_ptr<sdbusplus::asio::connection>& conn,
diff --git a/src/IpmbSensor.cpp b/src/IpmbSensor.cpp
index d821982..cb7f6ef 100644
--- a/src/IpmbSensor.cpp
+++ b/src/IpmbSensor.cpp
@@ -138,17 +138,17 @@
if (type == IpmbType::meSensor)
{
commandAddress = meAddress;
- netfn = 0x4; // sensor
- command = 0x2d; // get sensor reading
+ netfn = ipmi::sensor::netFn;
+ command = ipmi::sensor::getSensorReading;
commandData = {deviceAddress};
readingFormat = ReadingFormat::byte0;
}
else if (type == IpmbType::PXE1410CVR)
{
commandAddress = meAddress;
- netfn = 0x2e; // me bridge
- command = 0xd9; // send raw pmbus
- initCommand = 0xd9; // send raw pmbus
+ netfn = ipmi::me_bridge::netFn;
+ command = ipmi::me_bridge::sendRawPmbus;
+ initCommand = ipmi::me_bridge::sendRawPmbus;
// pmbus read temp
commandData = {0x57, 0x01, 0x00, 0x16, 0x3, deviceAddress, 0x00,
0x00, 0x00, 0x00, 0x01, 0x02, 0x8d};
@@ -160,8 +160,8 @@
else if (type == IpmbType::IR38363VR)
{
commandAddress = meAddress;
- netfn = 0x2e; // me bridge
- command = 0xd9; // send raw pmbus
+ netfn = ipmi::me_bridge::netFn;
+ command = ipmi::me_bridge::sendRawPmbus;
// pmbus read temp
commandData = {0x57, 0x01, 0x00, 0x16, 0x03, deviceAddress, 00,
0x00, 0x00, 0x00, 0x01, 0x02, 0x8D};
@@ -179,16 +179,16 @@
snsNum = 0x8d;
else
snsNum = 0x8c;
- netfn = 0x2e; // me bridge
- command = 0xd9; // send raw pmbus
+ netfn = ipmi::me_bridge::netFn;
+ command = ipmi::me_bridge::sendRawPmbus;
commandData = {0x57, 0x01, 0x00, 0x86, deviceAddress,
0x00, 0x00, 0x01, 0x02, snsNum};
readingFormat = ReadingFormat::elevenBit;
break;
case IpmbSubType::power:
case IpmbSubType::volt:
- netfn = 0x4; // sensor
- command = 0x2d; // get sensor reading
+ netfn = ipmi::sensor::netFn;
+ command = ipmi::sensor::getSensorReading;
commandData = {deviceAddress};
readingFormat = ReadingFormat::byte0;
break;
@@ -199,9 +199,9 @@
else if (type == IpmbType::mpsVR)
{
commandAddress = meAddress;
- netfn = 0x2e; // me bridge
- command = 0xd9; // send raw pmbus
- initCommand = 0xd9; // send raw pmbus
+ netfn = ipmi::me_bridge::netFn;
+ command = ipmi::me_bridge::sendRawPmbus;
+ initCommand = ipmi::me_bridge::sendRawPmbus;
// pmbus read temp
commandData = {0x57, 0x01, 0x00, 0x16, 0x3, deviceAddress, 0x00,
0x00, 0x00, 0x00, 0x01, 0x02, 0x8d};
@@ -234,9 +234,8 @@
switch (readingFormat)
{
case (ReadingFormat::byte0):
- // from node manager app-note, response of 0 means scanning disabled
- // for get sensor reading command
- if (data[0] == 0)
+ if (command == ipmi::sensor::getSensorReading &&
+ !ipmi::sensor::isValid(data))
{
return false;
}