diff options
Diffstat (limited to 'meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0003-Fix-missing-threshold-de-assert-event-when-threshold.patch')
-rw-r--r-- | meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0003-Fix-missing-threshold-de-assert-event-when-threshold.patch | 147 |
1 files changed, 68 insertions, 79 deletions
diff --git a/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0003-Fix-missing-threshold-de-assert-event-when-threshold.patch b/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0003-Fix-missing-threshold-de-assert-event-when-threshold.patch index c9175fd64..6815b5563 100644 --- a/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0003-Fix-missing-threshold-de-assert-event-when-threshold.patch +++ b/meta-openbmc-mods/meta-common/recipes-phosphor/sensors/dbus-sensors/0003-Fix-missing-threshold-de-assert-event-when-threshold.patch @@ -1,105 +1,109 @@ -From 17e3ed85f2ff919ff52b4a3fe7a1eb0026f28898 Mon Sep 17 00:00:00 2001 +From 235bb8a9b809c2449e3f5bf4e999db881012c144 Mon Sep 17 00:00:00 2001 From: Zhikui Ren <zhikui.ren@intel.com> -Date: Thu, 24 Sep 2020 14:27:32 -0700 -Subject: [PATCH] Fix missing threshold de-assert event when threshold changes. +Date: Tue, 22 Jun 2021 11:35:12 -0700 +Subject: [PATCH] Fix missing de-assert event when threshold changes +Issue: Sensor can be re-constructed when sensor configuration changes like a new threshold value. Threshold deassert can be missed if the new threshold value fixes the alarm because the default state for new threshold interface is de-asserted. -Send threshold de-assert message after interfaces are initialized to -ensure de-assert event is logged if there is an active assert -event. + +Resolution: +Add a member variable hadValidSensor that is initialized to false +for new sensor. When hadValidSensor is false, threshold property changed +message will be emitted even if threshold property did not change, +If the previous sensor instance had the threshold raised, +Phosphor-sel-logger would notice the change and log a de-assert event. +If the previous sensor instance did not have the threshold raised, +Phosphor-sel-logger would notice this is not a change and not create +new SEL log. +Set hadValidSensor to true when sensor value is updated with a value +that is not NaN. This is done after threshold property changed message +is emitted. Tested: -step1: -busctl set-property xyz.openbmc_project.ADCSensor /xyz/openbmc_project/sensors/voltage/P3VBAT xyz.openbmc_project.Sensor.Threshold.Warning WarningLow d 2.457 -ipmitool sel list -SEL has no entries -step2: -busctl set-property xyz.openbmc_project.ADCSensor /xyz/openbmc_project/sensors/voltage/P3VBAT xyz.openbmc_project.Sensor.Threshold.Warning WarningLow d 3.1 -ipmitool sel list - 1 | 09/24/20 | 21:30:15 UTC | Voltage #0x2d | Lower Non-critical going low | Asserted -step3: -busctl set-property xyz.openbmc_project.ADCSensor /xyz/openbmc_project/sensors/voltage/P3VBAT xyz.openbmc_project.Sensor.Threshold.Warning WarningLow d 2.457 -ipmitool sel list - 1 | 09/24/20 | 21:30:15 UTC | Voltage #0x2d | Lower Non-critical going low | Asserted - 2 | 09/24/20 | 21:30:33 UTC | Voltage #0x2d | Lower Non-critical going low | Deasserted +1. Change threshold value for a voltage sensor to force a SEL. + ipmitool raw 0x04 0x26 0x60 0x1b 0x95 0x6b 0x00 0x99 0xa6 0x00 + +2. Verify SEL logged threshold assert event as expected + ipmitool sel list + 1 | Pre-Init |0000007277| Voltage #0x60 | Upper Non-critical going high | Asserted + +3. Use ipmitool to change threshold value back to normal + ipmitool raw 0x04 0x26 0x60 0x1b 0x95 0x6b 0x00 0xa4 0xa6 0x00 + +4. Verify SEL logged threshold de-assert event as expected + ipmitool sel list + 1 | Pre-Init |0000007277| Voltage #0x60 | Upper Non-critical going high | Asserted + 2 | Pre-Init |0000007304| Voltage #0x60 | Upper Non-critical going high | Deasserted Signed-off-by: Zhikui Ren <zhikui.ren@intel.com> -Change-Id: If28870ac1e0d09be4a631a3145408ec70390dfc5 --- - include/Thresholds.hpp | 5 ++++- - include/sensor.hpp | 13 +++++++++++++ - src/ADCSensor.cpp | 1 + - src/Thresholds.cpp | 15 +++++++++++++-- - 4 files changed, 31 insertions(+), 3 deletions(-) + include/Thresholds.hpp | 2 +- + include/sensor.hpp | 2 ++ + src/Thresholds.cpp | 17 ++++++++++++++--- + 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/include/Thresholds.hpp b/include/Thresholds.hpp -index ca2b0a0..c1d0baf 100644 +index af63f72..fd507d0 100644 --- a/include/Thresholds.hpp +++ b/include/Thresholds.hpp -@@ -45,7 +45,10 @@ struct Threshold +@@ -44,7 +44,7 @@ struct Threshold void assertThresholds(Sensor* sensor, double assertValue, thresholds::Level level, thresholds::Direction direction, - bool assert); + bool assert, bool force = false); -+ -+void forceDeassertThresholds(Sensor* sensor, thresholds::Level level, -+ thresholds::Direction direction); struct TimerUsed { diff --git a/include/sensor.hpp b/include/sensor.hpp -index 0ef87d5..d50b2ff 100644 +index b98241b..6235674 100644 --- a/include/sensor.hpp +++ b/include/sensor.hpp -@@ -312,6 +312,19 @@ struct Sensor - operationalInterface->register_property("Functional", true); - operationalInterface->initialize(); +@@ -71,6 +71,7 @@ struct Sensor + std::shared_ptr<sdbusplus::asio::dbus_interface> operationalInterface; + double value = std::numeric_limits<double>::quiet_NaN(); + double rawValue = std::numeric_limits<double>::quiet_NaN(); ++ bool hadValidValue = false; + bool overriddenState = false; + bool internalSet = false; + double hysteresisTrigger; +@@ -432,6 +433,7 @@ struct Sensor + { + markFunctional(true); + markAvailable(true); ++ hadValidValue = true; } -+ -+ // Sensor can be reconstructed when sensor configuration changes -+ // like a new threshold value. Threshold deassert can be missed -+ // if the new threshold value fixes the alarm because -+ // default state for new threshold interface is de-asserted. -+ // Send threshold de-assert message during initialization to -+ // ensure de-assert events are logged if there is an active assert -+ // event. -+ for (auto& threshold : thresholds) -+ { -+ thresholds::forceDeassertThresholds(this, threshold.level, -+ threshold.direction); -+ } } - bool readingStateGood() -diff --git a/src/ADCSensor.cpp b/src/ADCSensor.cpp -index fe600d7..632fc8c 100644 ---- a/src/ADCSensor.cpp -+++ b/src/ADCSensor.cpp -@@ -88,6 +88,7 @@ ADCSensor::~ADCSensor() - // close the input dev to cancel async operations - inputDev.close(); - waitTimer.cancel(); -+ - objServer.remove_interface(thresholdInterfaceWarning); - objServer.remove_interface(thresholdInterfaceCritical); - objServer.remove_interface(sensorInterface); diff --git a/src/Thresholds.cpp b/src/Thresholds.cpp -index f4d4ed0..3c791c9 100644 +index bbe8e20..78ded55 100644 --- a/src/Thresholds.cpp +++ b/src/Thresholds.cpp -@@ -344,6 +344,7 @@ bool checkThresholds(Sensor* sensor) +@@ -418,10 +418,19 @@ bool checkThresholds(Sensor* sensor) { bool status = true; std::vector<ChangeParam> changes = checkThresholds(sensor, sensor->value); + ++ // Sensor can be reconstructed when sensor configuration changes ++ // like a new threshold value. Threshold deassert can be missed ++ // if the new threshold value fixes the alarm because ++ // default state for new threshold interface is de-asserted. ++ // force sending assert/de-assert message when a not NaN value is updated ++ // for the first time even when threshold property did not change. ++ bool forceAssert = !sensor->hadValidValue; for (const auto& change : changes) { assertThresholds(sensor, change.assertValue, change.threshold.level, -@@ -392,7 +393,7 @@ void checkThresholdsPowerDelay(Sensor* sensor, ThresholdTimer& thresholdTimer) +- change.threshold.direction, change.asserted); ++ change.threshold.direction, change.asserted, ++ forceAssert); + if (change.threshold.level == thresholds::Level::CRITICAL && + change.asserted) + { +@@ -473,7 +482,7 @@ void checkThresholdsPowerDelay(const std::weak_ptr<Sensor>& weakSensor, void assertThresholds(Sensor* sensor, double assertValue, thresholds::Level level, thresholds::Direction direction, @@ -108,7 +112,7 @@ index f4d4ed0..3c791c9 100644 { std::string property; std::shared_ptr<sdbusplus::asio::dbus_interface> interface; -@@ -432,7 +433,9 @@ void assertThresholds(Sensor* sensor, double assertValue, +@@ -513,7 +522,9 @@ void assertThresholds(Sensor* sensor, double assertValue, return; } @@ -119,21 +123,6 @@ index f4d4ed0..3c791c9 100644 { try { -@@ -452,6 +455,14 @@ void assertThresholds(Sensor* sensor, double assertValue, - } - } - -+// Explicitely de-assert a threshold with existing sensor value -+// Should only be called on sensor desctruction -+void forceDeassertThresholds(Sensor* sensor, thresholds::Level level, -+ thresholds::Direction direction) -+{ -+ assertThresholds(sensor, sensor->value, level, direction, false, true); -+} -+ - bool parseThresholdsFromAttr( - std::vector<thresholds::Threshold>& thresholdVector, - const std::string& inputPath, const double& scaleFactor, -- 2.17.1 |