commit f0a89946f1e3ce2e8fe97525e6b057463aa2a169
author Jie Yang <jjy@google.com> Thu Jul 29 15:30:25 2021 -0700
committer Jie Yang <jjy@google.com> Fri Oct 29 19:52:54 2021 +0000
tree 1f6d7f8bf89c5d04b54732c30895f0eab732c273
parent a291f49a5a0353c9e841ff7ae004d9b2eb956bae

dbus-sdr: Add sensor mutability

Sensor ValueMutability interface has already been merged into
openbmc/phosphor-dbus-interfaces here:
https://gerrit.openbmc-project.xyz/c/openbmc/phosphor-dbus-interfaces/+/36333

This change adds the IPMI server-side changes, namely, the test for
the "Mutable" member of this interface existing and being "true".
If so, it grants external write permission to this sensor, otherwise,
it will remain read-only (which is the default).

It replaces a previous compile-time constant that could only be changed
at compilation time, and would affect all sensors globally, neither of
which was desirable.

This "Mutable" interface boolean can be used to grant write
permission to sensors, such as external sensors and fan PWM sensors in
manual mode. IPMI setting sensor reading will check the mutability
first.

It achieves feature parity with the old
"mutability: Mutability::Write|Mutability::Read" settings,
in the old hardcoded YAML configuration files.

Also see the dbus-sensors changes, namely, the reading of this
parameter from entity-manager configuration, and setting
this D-Bus property accordingly, if "Mutable" is true:
https://gerrit.openbmc-project.xyz/c/openbmc/dbus-sensors/+/45405

Tested: With both 45405 and 45407 changes in, this feature has been
working nicely in our local environment for some time now.

Signed-off-by: Jie Yang <jjy@google.com>
Change-Id: I4ecff1a0424c0bc23d3a90466e1bb4b655f07859
Signed-off-by: Josh Lehan <krellan@google.com>

dbus-sdr/sdrutils.cpp

diff --git a/dbus-sdr/sdrutils.cpp b/dbus-sdr/sdrutils.cpp
index ede14bf..3df0dc1 100644
--- a/dbus-sdr/sdrutils.cpp
+++ b/dbus-sdr/sdrutils.cpp
@@ -93,6 +93,7 @@
     // Add sensors to SensorTree
     static constexpr const std::array sensorInterfaces = {
         "xyz.openbmc_project.Sensor.Value",
+        "xyz.openbmc_project.Sensor.ValueMutability",
         "xyz.openbmc_project.Sensor.Threshold.Warning",
         "xyz.openbmc_project.Sensor.Threshold.Critical"};
     static constexpr const std::array vrInterfaces = {
@@ -139,6 +140,7 @@
     sensorUpdatedIndex++;
     // The SDR is being regenerated, wipe the old stats
     sdrStatsTable.wipeTable();
+    sdrWriteTable.wipeTable();
     return sensorUpdatedIndex;
 }
 

dbus-sdr/sensorcommands.cpp

diff --git a/dbus-sdr/sensorcommands.cpp b/dbus-sdr/sensorcommands.cpp
index 3cebeb0..854d55f 100644
--- a/dbus-sdr/sensorcommands.cpp
+++ b/dbus-sdr/sensorcommands.cpp
@@ -545,6 +545,12 @@
             return ipmi::responseResponseError();
         }
 
+        // Only allow external SetSensor if write permission granted
+        if (!details::sdrWriteTable.getWritePermission(sensorNumber))
+        {
+            return ipmi::responseResponseError();
+        }
+
         auto value =
             sensor::calculateValue(reading, sensorMap, sensorObject->second);
         if (!value)
@@ -1605,10 +1611,18 @@
     // Remember the sensor name, as determined for this sensor number
     details::sdrStatsTable.updateName(sensornumber, name);
 
-#ifdef FEATURE_DYNAMIC_SENSORS_WRITE
-    // Set the sensor settable state to true by default
-    get_sdr::body::init_settable_state(true, &record.body);
-#endif
+    bool sensorSettable = false;
+    auto mutability =
+        sensorMap.find("xyz.openbmc_project.Sensor.ValueMutability");
+    if (mutability != sensorMap.end())
+    {
+        sensorSettable =
+            mappedVariant<bool>(mutability->second, "Mutable", false);
+    }
+    get_sdr::body::init_settable_state(sensorSettable, &record.body);
+
+    // Grant write permission to sensors deemed externally settable
+    details::sdrWriteTable.setWritePermission(sensornumber, sensorSettable);
 
     IPMIThresholds thresholdData;
     try
@@ -2212,12 +2226,10 @@
                           ipmi::sensor_event::cmdPlatformEvent,
                           ipmi::Privilege::Operator, ipmiSenPlatformEvent);
 
-#ifdef FEATURE_DYNAMIC_SENSORS_WRITE
     // <Set Sensor Reading and Event Status>
     ipmi::registerHandler(ipmi::prioOpenBmcBase, ipmi::netFnSensor,
                           ipmi::sensor_event::cmdSetSensorReadingAndEvtSts,
                           ipmi::Privilege::Operator, ipmiSetSensorReading);
-#endif
 
     // <Get Sensor Reading>
     ipmi::registerHandler(ipmi::prioOpenBmcBase, ipmi::netFnSensor,