Factor out getPowerState() helper function

This same pattern had been open-coded in many of the sensor daemons;
let's reduce the duplication a bit.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>

Factor out getPowerState() helper function

This same pattern had been open-coded in many of the sensor daemons;
let's reduce the duplication a bit.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Change-Id: I8556af30fdcd53cdf06e5a4911f9152d3265c7af

show more ...

Use SensorBaseConfigMap where applicable

If we've got a more concise, descriptive name for it, we might as well
use it.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Change-Id: Id2f1bd8f983b4e0c

Use SensorBaseConfigMap where applicable

If we've got a more concise, descriptive name for it, we might as well
use it.

Signed-off-by: Zev Weiss <zev@bewilderbeest.net>
Change-Id: Id2f1bd8f983b4e0c2ca24b24c9fc9b875f4191f5

show more ...

Enable an io-uring build

There exists a bug where calls that we previously thought were
non-blocking, actually do block when used with hwmon or filesystem fds.
This causes high latencies on the dbus

Enable an io-uring build

There exists a bug where calls that we previously thought were
non-blocking, actually do block when used with hwmon or filesystem fds.
This causes high latencies on the dbus interfaces when lots of sensors
are used in a single daemon, as is the case in PSUSensor.

This patchset moves the PSUSensor code over to using io-uring, through
boost asio, using the random_access_file class. This helps with
performance in a number of ways, the largest of which being that sensor
reads are no longer blocking.

Tested:

Booted the sensor system on Tyan S7106; dbus-monitor shows sensors
scanning normally.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I654eafcfab5a24b65b89c204ab43d81c7ae064cf

show more ...

Clang-tidy-14 fixes

Do as the robot commands. All changes made automatically by tidy.

Tested: (Thanks Zhikui)
Downloaded and run on system. Sensors scan normally.

Signed-off-by: Ed Tanous <edtan

Clang-tidy-14 fixes

Do as the robot commands. All changes made automatically by tidy.

Tested: (Thanks Zhikui)
Downloaded and run on system. Sensors scan normally.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I752f37c9e7a95aa3be8e6980ba6e4b2b48b3395a

show more ...

Fix clang-tidy 14 issues

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Idb76ae375eff9a143497942013d94ca3b3252495

Revert "sensor: remove exception in error path"

This reverts commit 379b11316d55b114bdb1bfc5a49839b09ade9038, bringing
it back into line with expected ASIO callback behavior, as documented in
Ic3a36

Revert "sensor: remove exception in error path"

This reverts commit 379b11316d55b114bdb1bfc5a49839b09ade9038, bringing
it back into line with expected ASIO callback behavior, as documented in
Ic3a36027fa95a77469439b296a1497634cfe030f. The reverted commit was based
on a misunderstanding of the ASIO error handling path.

Tested: With accompanying sdbusplus change, was able to override sensor
value only when ValidationUnsecure mode was enabled, and proper error
response was returned:

root@openbmc:~# busctl set-property xyz.openbmc_project.IpmbSensor \
/xyz/openbmc_project/sensors/temperature/SSB_Temp \
xyz.openbmc_project.Sensor.Value Value d 52
Failed to set property Value on interface xyz.openbmc_project.Sensor.Value: Not allowed to set property value.
root@openbmc:~# busctl set-property xyz.openbmc_project.SpecialMode \
/xyz/openbmc_project/security/special_mode \
xyz.openbmc_project.Security.SpecialMode SpecialMode s \
xyz.openbmc_project.Control.Security.SpecialMode.Modes.ValidationUnsecure
root@openbmc:~# busctl set-property xyz.openbmc_project.IpmbSensor \
/xyz/openbmc_project/sensors/temperature/SSB_Temp \
xyz.openbmc_project.Sensor.Value Value d 52
root@openbmc:~#

Change-Id: I05c41318954d5d1549752c5ef2c227f3f22a45d3
Signed-off-by: Jonathan Doman <jonathan.doman@intel.com>

show more ...

Update link to the Value.interface.yaml

Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
Change-Id: I95b1db1f07612a3e5604bbf043cc00f731bd3a8c

Apply LambdaBodyIndentation to dbus-sensors

Per the transform being done in bmcweb, do the same for dbus-sensors.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If21489607759f3cdf20fad17

Apply LambdaBodyIndentation to dbus-sensors

Per the transform being done in bmcweb, do the same for dbus-sensors.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: If21489607759f3cdf20fad17eede50fb4e228e5e

show more ...

Always fill both values for a threshold interface

If one of the thresholds for a threshold dbus interface is provided we
have to set the other one as dbus properties are never optional.

Tested:
B

Always fill both values for a threshold interface

If one of the thresholds for a threshold dbus interface is provided we
have to set the other one as dbus properties are never optional.

Tested:
Before:
xyz.openbmc_project.Sensor.Threshold.Warning interface - -
.WarningAlarmHigh property b false
.WarningHigh property d 14

After:
xyz.openbmc_project.Sensor.Threshold.Warning interface - -
.WarningAlarmHigh property b false
.WarningAlarmLow property b false
.WarningHigh property d 14
.WarningLow property d nan

Signed-off-by: Konstantin Aladyshev <aladyshev22@gmail.com>
Change-Id: I0e9b885e8c0494d46d39af965047a7582255a48e

show more ...

NVMeContext: Rework sensor removal concurrent to polling

Concurrent removal of a sensor's configuration while the sensor list is
being iterated for polling can lead to undefined behaviour via access

NVMeContext: Rework sensor removal concurrent to polling

Concurrent removal of a sensor's configuration while the sensor list is
being iterated for polling can lead to undefined behaviour via access
through a deleted iterator:

Program terminated with signal SIGSEGV, Segmentation fault.
#0 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count (__r=..., this=<optimised out>, this=<optimised out>, __r=...)
at /usr/include/c++/11.2.0/bits/stl_list.h:224
224 /usr/include/c++/11.2.0/bits/stl_list.h: No such file or directory.
[Current thread is 1 (LWP 6649)]
#0 std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count (__r=..., this=<optimised out>, this=<optimised out>, __r=...)
at /usr/include/c++/11.2.0/bits/stl_list.h:224
#1 std::__shared_ptr<NVMeSensor, (__gnu_cxx::_Lock_policy)2>::__shared_ptr (this=<optimised out>, this=<optimised out>)
at /usr/include/c++/11.2.0/bits/shared_ptr_base.h:1152
#2 std::shared_ptr<NVMeSensor>::shared_ptr (this=<optimised out>, this=<optimised out>) at /usr/include/c++/11.2.0/bits/shared_ptr.h:150
#3 NVMeBasicContext::readAndProcessNVMeSensor (this=0x1ac8a90, iter=non-dereferenceable iterator for std::list) at ../git/src/NVMeBasicContext.cpp:299
#4 0x004dd8b8 in NVMeBasicContext::readAndProcessNVMeSensor (this=0x1ac8a90, iter=non-dereferenceable iterator for std::list) at ../git/src/NVMeBasicContext.cpp:312
#5 0x004dd8b8 in NVMeBasicContext::readAndProcessNVMeSensor (this=0x1ac8a90, iter=std::shared_ptr<NVMeSensor> (use count 26, weak count 28153871) = {get() = 0x1ad8db0})
at ../git/src/NVMeBasicContext.cpp:312

Rework polling and sensor removal to uphold the requirement that the
iterator remains valid.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I69b005fe3dad7ddf21d1762731f9cdfd2408cae1

show more ...

Fix for cpusensor going into D state.

When PECI driver returns the error code EAGAIN, async_read_until will
go into a keep trying again loop. As async_read_until does not handle
EAGAIN reliably.

Sa

Fix for cpusensor going into D state.

When PECI driver returns the error code EAGAIN, async_read_until will
go into a keep trying again loop. As async_read_until does not handle
EAGAIN reliably.

Same issue was identified and fix for PSUSensor:
https://github.com/openbmc/dbus-sensors/commit/bcf76717697238e380be785bd28150963ecf0e9e

Fix for this is to use async_wait instead of async_read_until.

Tested:
In Intel system verified cpusensor daemon is not getting into D state
when there is a peci error EAGAIN.
Verified sensors are getting populated as expected when there is no PECI
error.

Signed-off-by: Arun Lal K M <arun.lal@intel.com>
Signed-off-by: Arun P. Mohanan <arun.p.m@linux.intel.com>
Change-Id: I52e35075e522d0ae0d99e1c893db76156e299871

show more ...

NVMeBasicContext: Account for invalid drive status

Respect the drive status bits in the basic management command response
before parsing the drive temperature value, as it may not yet be valid.

Sig

NVMeBasicContext: Account for invalid drive status

Respect the drive status bits in the basic management command response
before parsing the drive temperature value, as it may not yet be valid.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I5311eb3b07f9b52fce7342c9c7f670f777d68c1d

show more ...

NVMeBasicContext: Defer polling sensors that are in error

A sensor is only polled after N sampling periods if it is in error. This
necessitates the movement of some of the error handling so we can
i

NVMeBasicContext: Defer polling sensors that are in error

A sensor is only polled after N sampling periods if it is in error. This
necessitates the movement of some of the error handling so we can
increment the sensor's error counter as appropriate.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Ie19f6ceb7ad8f9b2f1408a4ca8f12c0ca385ce8c

show more ...

NVMeBasicContext: Poll all sensors each polling interval

Rework the polling protocol so that the completion of polling sensor N
invokes polling of sensor N+1 for the given context.

The timer is ret

NVMeBasicContext: Poll all sensors each polling interval

Rework the polling protocol so that the completion of polling sensor N
invokes polling of sensor N+1 for the given context.

The timer is retained in order to trigger polling the initial sensor.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I599547c8c790905549c0cad9fbfbe63cf6930de4

show more ...

nvmesensor: Remove MCTP transport implementation

This was implemented in terms of a series of patches against libmctp
that were never upstreamed (because they in-turn relied on kernel
patches that a

nvmesensor: Remove MCTP transport implementation

This was implemented in terms of a series of patches against libmctp
that were never upstreamed (because they in-turn relied on kernel
patches that also were never upstreamed). Further, there are no
configurations enabling the nvme-mi-mctp option in openbmc/openbmc.

This is motivated by the need to refactor NVMeContext to support polling
all sensors inside the polling period on a given root bus.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: Ib20b7f3ce218191a391d7b2d893f204fb97b7ab5

show more ...

NVMeBasicContext: Reject bad I2C bus IDs

We can't possibly query such sensors, so ensure they can't exist.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I899979db57a84272be9b537d87e547

NVMeBasicContext: Reject bad I2C bus IDs

We can't possibly query such sensors, so ensure they can't exist.

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I899979db57a84272be9b537d87e54756296695cf

show more ...

Move sensorClass and subtype to separate functions

Cleaning up the code by moving the sensor Class, sensor subtype
and sensor Scale & Offset value to separate functions in
IpmbSensor.

TESTED: Teste

Move sensorClass and subtype to separate functions

Cleaning up the code by moving the sensor Class, sensor subtype
and sensor Scale & Offset value to separate functions in
IpmbSensor.

TESTED: Tested on Facebook YosemiteV2 platform and verified the
IpmbSensor sensors.

Signed-off-by: Jayashree Dhanapal <jayashree-d@hcl.com>
Change-Id: Id185ce0d592039d2999902c5ae88a49891c7a807

show more ...

sensor: Add inError() method

Signed-off-by: Andrew Jeffery <andrew@aj.id.au>
Change-Id: I562505e0ce134f5a6978338e300615e0f9428b27

Enable cppcoreguidelines-avoid-goto checks

We've been pretty good about this one, the only usages seem to be in
nvme sensor and added recently. To move to more RAII compliant
containers, this commi

Enable cppcoreguidelines-avoid-goto checks

We've been pretty good about this one, the only usages seem to be in
nvme sensor and added recently. To move to more RAII compliant
containers, this commit creates a FileHandle class, which accepts either
a file path and a mode, or a direct file handle, to be handled via RAII.
Ideally we wouldn't have to create this ourselves, and could rely on
some library, but the alternatives all have compromises that are worse.

boost::asio::random_access_file requires iouring to be enabled, and
isn't available until boost 0.78, which we haven't rebased to yet.
https://www.boost.org/doc/libs/develop/doc/html/boost_asio/reference/random_access_file.html
Once available, this is what I would hope that a bunch of the nvme stuff
evolves into to be much simpler.

boost::iostream::file_descriptor requires linking against
boost::iostream, which would bloat our binary size for a lot of things
we don't use. There is a file_descriptor.cpp that we could compile
directly to solve this, but the existing yocto boost project (nor any
other distro project) installs these files for use, which would
complicate this solution

std::fstream doesn't allow direct access to the file handle, which would
be required for streaming operations.

https://www.boost.org/doc/libs/1_35_0/libs/iostreams/doc/installation.html
https://www.boost.org/doc/libs/1_78_0/libs/iostreams/doc/classes/file_descriptor.html#file_descriptor

A very similar class exists within ipmid here:
https://github.com/openbmc/phosphor-host-ipmid/blob/master/user_channel/file.hpp

This class differs in a couple minor ways, in that it throws exceptions
on open failures (similar to the boost classes), accepts the input by
std::filesystem::path, instead of string, and supports std::move, which
is required for the pipe usage.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I8be2419783e41e8f529030bb3d05c2e7aa890307

show more ...

Thresholds: Support all valid severity types

Add in PerformanceLoss and fix the value for SoftShutdown and
HardShutdown.

Tested: Added in to a hwmontempsensor config and thresholds appear in
busctl

Thresholds: Support all valid severity types

Add in PerformanceLoss and fix the value for SoftShutdown and
HardShutdown.

Tested: Added in to a hwmontempsensor config and thresholds appear in
busctl and alarms go high and low as expected.

Signed-off-by: Rashmica Gupta <rashmica.g@gmail.com>
Change-Id: Ic08a0e1fe171f39f8e537b60522149a9420bc62e

show more ...

Remove isValidLevel()

Every usage of isValidLevel is done after the levels have been parsed
and are already known to be correct. Checking that Level is valid is
wasteful, and in some cases, is alre

Remove isValidLevel()

Every usage of isValidLevel is done after the levels have been parsed
and are already known to be correct. Checking that Level is valid is
wasteful, and in some cases, is already checked by
getThresholdInterface, which can already bounds check this.

This check is not needed.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: I1d03c1c66ae37ca86a48175ec9c739c2d3966662

show more ...

sensors: don't pass excess conn handler

We don't need to pass connection handler to setInitialProperties method
since it already part of the sensor class.

Tested: no changes in behaviour.
Signed-of

sensors: don't pass excess conn handler

We don't need to pass connection handler to setInitialProperties method
since it already part of the sensor class.

Tested: no changes in behaviour.
Signed-off-by: Andrei Kartashev <a.kartashev@yadro.com>
Change-Id: Icaaccbeaf14e22648a6a1bce381f0f0a606b0ddd

show more ...

Rearrange thresholds to be pair based

It was pointed out in another review that the way the dbus interfaces
are set up, they are all pair-wise, with a positive and negative end
of the interface. Th

Rearrange thresholds to be pair based

It was pointed out in another review that the way the dbus interfaces
are set up, they are all pair-wise, with a positive and negative end
of the interface. This changes the data structures to match that
reality, inventing a "ThresholdDefinition" object. This reduces the
code, and allows us to promote the level definitions up to a higher
scope in the structure.

This greatly simplifies the thresProp structure to remove a number of
elements that were redundant, and allows adding new thresholds easier
in the future.

Tested: Tested on Vegman system; Thresholds work as designed.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ic9d5ba8282f78fe928689cb7fbd98d575a570200

show more ...

Fix clang-tidy header check

Apparently our exclusion of the linux headers covered up some issues in
our own headers, and was erroneously ignoring all headers, not just the
linux headers.

Apparently

Fix clang-tidy header check

Apparently our exclusion of the linux headers covered up some issues in
our own headers, and was erroneously ignoring all headers, not just the
linux headers.

Apparently this now detects recursion quite a bit, so disable that check
for the moment, as well as the special member variables checks. Also,
disable some cert checks that are duplicates of
bugprone-reserved-identifier, because of the aformentioned ignored linux
headers

All other changes are done automatically.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Ie640495146dbe155a85d8a9e81be78539137a88b

show more ...

Consistently use std::pair<double, double> limits

There are a number of places in tach sensor where we jump from having a
pair of doubles, so having a pair of ints, then back. All sensors take
this

Consistently use std::pair<double, double> limits

There are a number of places in tach sensor where we jump from having a
pair of doubles, so having a pair of ints, then back. All sensors take
this as a double, so we should be consistent, even if tachsensor is only
expected to have integer units.

This was flagged by clang-tidy as part of the
bugprone-misplaced-widening-cast check.

Signed-off-by: Ed Tanous <edtanous@google.com>
Change-Id: Iee1439f3af2bf8c15dc33f91c63859c5d1b776ca

show more ...