From 6ace96be5a7b6763545c1dfc572f8e2790d99d4b Mon Sep 17 00:00:00 2001 From: Zhikui Ren Date: Tue, 11 May 2021 11:14:55 -0700 Subject: [PATCH] CPUSensor: create RequiredTempSensor if defined When BMC fails to talk to CPU through PECI, no CPU sensors are created. Fan speed control only boost fan when input sensor is not available. It does not have the knowledge of which sensors are expected to be present and can't treat "missing" sensor as failed one. The failure on PECI goes unnoticed and we can have a thermal run away. Query sensor config for RequiredTempSensor for any present CPUs. Create a CPU sensor that is not available. This sensor will be replaced by a normal CPU sensor when peci detect the CPU and associated hwmon file is discovered. This is an initial patch that target to address the particular failure case. More work will follow to support a more generic "Required" sensor config. Signed-off-by: Zhikui Ren --- include/CPUSensor.hpp | 9 ++++++ src/CPUSensor.cpp | 68 +++++++++++++++++++++++++++++++++++++++++++ src/CPUSensorMain.cpp | 54 +++++++++++++++++++++++++++++----- 3 files changed, 123 insertions(+), 8 deletions(-) diff --git a/include/CPUSensor.hpp b/include/CPUSensor.hpp index 8b51b76..93b7fcc 100644 --- a/include/CPUSensor.hpp +++ b/include/CPUSensor.hpp @@ -26,6 +26,15 @@ class CPUSensor : public Sensor std::vector&& thresholds, const std::string& configuration, int cpuId, bool show, double dtsOffset); + + // Create a CPUSensor without a path to sensor value + CPUSensor(const std::string& objectType, + sdbusplus::asio::object_server& objectServer, + std::shared_ptr& conn, + boost::asio::io_service& io, const std::string& sensorName, + std::vector&& thresholdsIn, + const std::string& sensorConfiguration); + ~CPUSensor() override; static constexpr unsigned int sensorScaleFactor = 1000; static constexpr unsigned int sensorPollMs = 1000; diff --git a/src/CPUSensor.cpp b/src/CPUSensor.cpp index c330088..3861ade 100644 --- a/src/CPUSensor.cpp +++ b/src/CPUSensor.cpp @@ -98,6 +98,74 @@ CPUSensor::CPUSensor(const std::string& path, const std::string& objectType, setupRead(); } +// Create a dummy "not available" CPUSensor +// This is used to indicate a missing required sensor for +// other services like fan control +CPUSensor::CPUSensor(const std::string& objectType, + sdbusplus::asio::object_server& objectServer, + std::shared_ptr& conn, + boost::asio::io_service& io, const std::string& sensorName, + std::vector&& thresholdsIn, + const std::string& sensorConfiguration) : + Sensor(escapeName(sensorName), std::move(thresholdsIn), sensorConfiguration, + objectType, false, false, 0, 0, conn, PowerState::on), + objServer(objectServer), inputDev(io), waitTimer(io), + privTcontrol(std::numeric_limits::quiet_NaN()), dtsOffset(0), + show(true), pollTime(CPUSensor::sensorPollMs), minMaxReadCounter(0) +{ + // assume it is a temperature sensor for now + // support for other type can be added later + std::string interfacePath = + "/xyz/openbmc_project/sensors/temperature/" + name; + const char* units = sensor_paths::unitDegreesC; + minValue = -128; + maxValue = 127; + + sensorInterface = objectServer.add_interface( + interfacePath, "xyz.openbmc_project.Sensor.Value"); + + sensorInterface->register_property("Unit", units); + sensorInterface->register_property("MaxValue", maxValue); + sensorInterface->register_property("MinValue", minValue); + sensorInterface->register_property( + "Value", value, [&](const double& newValue, double& oldValue) { + return setSensorValue(newValue, oldValue); + }); + if (!sensorInterface->initialize()) + { + std::cerr << "error initializing value interface\n"; + } + + if (!availableInterface) + { + availableInterface = std::make_shared( + conn, sensorInterface->get_object_path(), availableInterfaceName); + availableInterface->register_property( + "Available", false, [this](const bool propIn, bool& old) { + if (propIn == old) + { + return 1; + } + old = propIn; + if (!propIn) + { + updateValue(std::numeric_limits::quiet_NaN()); + } + return 1; + }); + availableInterface->initialize(); + } + if (!operationalInterface) + { + operationalInterface = + std::make_shared( + conn, sensorInterface->get_object_path(), + operationalInterfaceName); + operationalInterface->register_property("Functional", true); + operationalInterface->initialize(); + } +} + CPUSensor::~CPUSensor() { // close the input dev to cancel async operations diff --git a/src/CPUSensorMain.cpp b/src/CPUSensorMain.cpp index 0d94e4b..1d12fa6 100644 --- a/src/CPUSensorMain.cpp +++ b/src/CPUSensorMain.cpp @@ -332,10 +332,9 @@ bool createSensors(boost::asio::io_service& io, { if (debug) { - std::cout << "Skipped: " << inputPath << ": " << sensorName - << " is already created\n"; + std::cout << "Will be replaced: " << inputPath << ": " + << sensorName << " is already created\n"; } - continue; } // check hidden properties @@ -636,9 +635,9 @@ void detectCpuAsync( }); } -bool getCpuConfig(const std::shared_ptr& systemBus, +bool getCpuConfig(std::shared_ptr& systemBus, boost::container::flat_set& cpuConfigs, - ManagedObjectType& sensorConfigs, + ManagedObjectType& sensorConfigs, boost::asio::io_service& io, sdbusplus::asio::object_server& objectServer) { bool useCache = false; @@ -700,6 +699,45 @@ bool getCpuConfig(const std::shared_ptr& systemBus, iface->register_property("Present", *present); iface->initialize(); inventoryIfaces[name] = std::move(iface); + if (*present) + { + // create required CPU sensors here in unavailable state + auto findRequiredTempSensor = + config.second.find("RequiredTempSensor"); + auto findCpuId = config.second.find("CpuID"); + if (findRequiredTempSensor != config.second.end() && + findCpuId != config.second.end()) + { + std::string label = + std::visit(VariantToStringVisitor(), + findRequiredTempSensor->second); + // for temp sensor hwmon sysfs use input + std::string item{"input"}; + int cpuId = + std::visit(VariantToUnsignedIntVisitor(), + findCpuId->second); + std::string requiredSensorName = + createSensorName(label, item, cpuId); + + auto& sensorPtr = gCpuSensors[requiredSensorName]; + if (sensorPtr == nullptr) + { + // created a dummy sensor for required sensor, + // will be replaced with a real one if it is + // detected + std::string objectType{}; + std::vector + emptyThreshold{}; + std::string emptyConfig{}; + sensorPtr = std::make_unique( + objectType, objectServer, systemBus, io, + requiredSensorName, + std::move(emptyThreshold), emptyConfig); + std::cout << "created required CPU sensor " + << requiredSensorName << "\n"; + } + } + } } auto findBus = config.second.find("Bus"); @@ -728,7 +766,6 @@ bool getCpuConfig(const std::shared_ptr& systemBus, std::cout << "name: " << name << "\n"; std::cout << "type: " << type << "\n"; } - cpuConfigs.emplace(bus, addr, name, State::OFF); } } @@ -764,7 +801,8 @@ int main() return; // we're being canceled } - if (getCpuConfig(systemBus, cpuConfigs, sensorConfigs, objectServer)) + if (getCpuConfig(systemBus, cpuConfigs, sensorConfigs, io, + objectServer)) { detectCpuAsync(pingTimer, creationTimer, io, objectServer, systemBus, cpuConfigs, sensorConfigs); @@ -792,7 +830,7 @@ int main() return; // we're being canceled } - if (getCpuConfig(systemBus, cpuConfigs, sensorConfigs, + if (getCpuConfig(systemBus, cpuConfigs, sensorConfigs, io, objectServer)) { detectCpuAsync(pingTimer, creationTimer, io, objectServer, -- 2.17.1