From 9cf85627cf6fe224b7bf9399a60022397ebeb970 Mon Sep 17 00:00:00 2001
From: Brandon Kim <brandonkim@google.com>
Date: Wed, 19 Jun 2019 12:05:08 -0700
Subject: sensordatahandler: Throw on sensor's OperationalStatus

Add UPDATE_FUNCTIONAL_ON_FAIL and only when defined, read sensor's
OperationalStatus interface for the functional property and throw if the
sensor is not funcitonal.

Bug: openbmc/phosphor-hwmon#10
Signed-off-by: Brandon Kim <brandonkim@google.com>
Change-Id: I1144a6d3f8145bda73f3363664ca48b848a295db
---
 configure.ac          | 20 ++++++++++++++++++++
 sensordatahandler.hpp | 28 ++++++++++++++++++++++++++++
 sensorhandler.cpp     |  8 ++++++++
 sensorhandler.hpp     | 11 +++++++++++
 4 files changed, 67 insertions(+)

diff --git a/configure.ac b/configure.ac
index 869c53d..5ed0506 100644
--- a/configure.ac
+++ b/configure.ac
@@ -192,6 +192,26 @@ AC_ARG_VAR(HOST_IPMI_LIB_PATH, [The file path to search for libraries.])
 AS_IF([test "x$HOST_IPMI_LIB_PATH" == "x"], [HOST_IPMI_LIB_PATH="/usr/lib/ipmid-providers/"])
 AC_DEFINE_UNQUOTED([HOST_IPMI_LIB_PATH], ["$HOST_IPMI_LIB_PATH"], [The file path to search for libraries.])
 
+# When a sensor read fails, hwmon will update the OperationalState interface's Functional property.
+# This will mark the sensor as not functional and we will skip reading from that sensor.
+AC_ARG_ENABLE([update-functional-on-fail],
+    AS_HELP_STRING(
+        [--enable-update-functional-on-fail],
+        [Check functional property to skip reading from faulty sensors.]
+    )
+)
+
+AC_ARG_VAR(UPDATE_FUNCTIONAL_ON_FAIL, [Check functional property to skip reading from faulty sensors.])
+AS_IF(
+    [test "x$enable_update_functional_on_fail" == "xyes"],
+    [UPDATE_FUNCTIONAL_ON_FAIL="yes"]
+    AC_DEFINE_UNQUOTED(
+        [UPDATE_FUNCTIONAL_ON_FAIL],
+        ["$UPDATE_FUNCTIONAL_ON_FAIL"],
+        [Check functional property to skip reading from faulty sensors.]
+    )
+)
+
 # When disable-libuserlayer flag is set, libuserlayer won't be included in the build.
 AC_ARG_ENABLE([libuserlayer],
     AS_HELP_STRING([--disable-libuserlayer], [Set a flag to exclude libuserlayer])
diff --git a/sensordatahandler.hpp b/sensordatahandler.hpp
index 73f0dbf..129cbb4 100644
--- a/sensordatahandler.hpp
+++ b/sensordatahandler.hpp
@@ -1,5 +1,7 @@
 #pragma once
 
+#include "config.h"
+
 #include "sensorhandler.hpp"
 
 #include <cmath>
@@ -190,6 +192,32 @@ GetSensorResponse readingData(const Info& sensorInfo)
     auto service = ipmi::getService(bus, sensorInfo.sensorInterface,
                                     sensorInfo.sensorPath);
 
+#ifdef UPDATE_FUNCTIONAL_ON_FAIL
+    // Check the OperationalStatus interface for functional property
+    if (sensorInfo.propertyInterfaces.begin()->first ==
+        "xyz.openbmc_project.Sensor.Value")
+    {
+        bool functional = true;
+        try
+        {
+            auto funcValue = ipmi::getDbusProperty(
+                bus, service, sensorInfo.sensorPath,
+                "xyz.openbmc_project.State.Decorator.OperationalStatus",
+                "Functional");
+            functional = std::get<bool>(funcValue);
+        }
+        catch (...)
+        {
+            // No-op if Functional property could not be found since this
+            // check is only valid for Sensor.Value read for hwmonio
+        }
+        if (!functional)
+        {
+            throw SensorFunctionalError();
+        }
+    }
+#endif
+
     auto propValue = ipmi::getDbusProperty(
         bus, service, sensorInfo.sensorPath,
         sensorInfo.propertyInterfaces.begin()->first,
diff --git a/sensorhandler.cpp b/sensorhandler.cpp
index d67e7f5..b321e96 100644
--- a/sensorhandler.cpp
+++ b/sensorhandler.cpp
@@ -1,3 +1,5 @@
+#include "config.h"
+
 #include "sensorhandler.hpp"
 
 #include "fruread.hpp"
@@ -420,6 +422,12 @@ ipmi_ret_t ipmi_sen_get_sensor_reading(ipmi_netfn_t netfn, ipmi_cmd_t cmd,
         resp->operation = 1 << scanningEnabledBit;
         return IPMI_CC_OK;
     }
+#ifdef UPDATE_FUNCTIONAL_ON_FAIL
+    catch (const SensorFunctionalError& e)
+    {
+        return IPMI_CC_RESPONSE_ERROR;
+    }
+#endif
     catch (const std::exception& e)
     {
         *data_len = getResponse.size();
diff --git a/sensorhandler.hpp b/sensorhandler.hpp
index 56a7021..f5a8b41 100644
--- a/sensorhandler.hpp
+++ b/sensorhandler.hpp
@@ -2,6 +2,7 @@
 
 #include <stdint.h>
 
+#include <exception>
 #include <ipmid/api.hpp>
 #include <ipmid/types.hpp>
 
@@ -38,6 +39,16 @@ enum ipmi_sensor_types
     IPMI_SENSOR_TPM = 0xCC,
 };
 
+/** @brief Custom exception for reading sensors that are not funcitonal.
+ */
+struct SensorFunctionalError : public std::exception
+{
+    const char* what() const noexcept
+    {
+        return "Sensor not functional";
+    }
+};
+
 #define MAX_DBUS_PATH 128
 struct dbus_interface_t
 {
-- 
cgit v1.2.1