pseq: Switch to new pgood isolation support
Switch to the new C++ implementation of pgood isolation support. When a
pgood fault occurs, this support attempts to find the voltage rail that
caused the fault.
The new implementation includes the following:
* PowerSequencerDevice class hierarchy
* Rail class
* Services class
* CompatibleSystemTypesFinder class
* DeviceFinder class
* config_file_parser functions
* New JSON configuration files
Tested:
* Verified all automated tests ran successfully.
* Verified all new/modified code in this commit via manual testing.
* Tested on Rainier and Everest systems
* Verified system powered on and off without errors.
* Tested with pgood injection in every rail both during and after the
power on sequence. Verified rail was identified and correct error
was logged.
Change-Id: I83d7fdc45bd0a000a31d98f67ecdd5a54f24b939
Signed-off-by: Shawn McCarney <shawnmm@us.ibm.com>
diff --git a/phosphor-power-sequencer/src/meson.build b/phosphor-power-sequencer/src/meson.build
index f853c7a..9c6cbe5 100644
--- a/phosphor-power-sequencer/src/meson.build
+++ b/phosphor-power-sequencer/src/meson.build
@@ -31,10 +31,6 @@
'power_control_main.cpp',
'power_control.cpp',
'power_interface.cpp',
- 'power_sequencer_monitor.cpp',
- 'ucd90x_monitor.cpp',
- 'ucd90160_monitor.cpp',
- 'ucd90320_monitor.cpp',
dependencies: [
libgpiodcxx,
nlohmann_json_dep,
@@ -44,6 +40,7 @@
stdplus,
],
link_with: [
+ phosphor_power_sequencer_library,
libpower
],
implicit_include_directories: false,
diff --git a/phosphor-power-sequencer/src/power_control.cpp b/phosphor-power-sequencer/src/power_control.cpp
index b4b993d..cc92ff4 100644
--- a/phosphor-power-sequencer/src/power_control.cpp
+++ b/phosphor-power-sequencer/src/power_control.cpp
@@ -16,45 +16,37 @@
#include "power_control.hpp"
+#include "config_file_parser.hpp"
+#include "format_utils.hpp"
#include "types.hpp"
-#include "ucd90160_monitor.hpp"
-#include "ucd90320_monitor.hpp"
-
-#include <fmt/chrono.h>
-#include <fmt/format.h>
-#include <fmt/ranges.h>
-
-#include <phosphor-logging/elog-errors.hpp>
-#include <phosphor-logging/elog.hpp>
-#include <phosphor-logging/log.hpp>
-#include <xyz/openbmc_project/Common/error.hpp>
+#include "ucd90160_device.hpp"
+#include "ucd90320_device.hpp"
+#include "utility.hpp"
#include <exception>
-#include <vector>
-
-using namespace phosphor::logging;
+#include <format>
+#include <functional>
+#include <map>
+#include <span>
+#include <stdexcept>
+#include <thread>
+#include <utility>
namespace phosphor::power::sequencer
{
-const std::vector<std::string>
- interfaceNames({"xyz.openbmc_project.Configuration.UCD90160",
- "xyz.openbmc_project.Configuration.UCD90320"});
-const std::string addressPropertyName = "Address";
-const std::string busPropertyName = "Bus";
-const std::string namePropertyName = "Name";
-const std::string typePropertyName = "Type";
+const std::string powerOnTimeoutError =
+ "xyz.openbmc_project.Power.Error.PowerOnTimeout";
+
+const std::string powerOffTimeoutError =
+ "xyz.openbmc_project.Power.Error.PowerOffTimeout";
+
+const std::string shutdownError = "xyz.openbmc_project.Power.Error.Shutdown";
PowerControl::PowerControl(sdbusplus::bus_t& bus,
const sdeventplus::Event& event) :
PowerObject{bus, POWER_OBJ_PATH, PowerObject::action::defer_emit},
- bus{bus}, device{std::make_unique<PowerSequencerMonitor>(bus)},
- match{bus,
- sdbusplus::bus::match::rules::interfacesAdded() +
- sdbusplus::bus::match::rules::sender(
- "xyz.openbmc_project.EntityManager"),
- std::bind(&PowerControl::interfacesAddedHandler, this,
- std::placeholders::_1)},
+ bus{bus}, services{bus},
pgoodWaitTimer{event, std::bind(&PowerControl::onFailureCallback, this)},
powerOnAllowedTime{std::chrono::steady_clock::now() + minimumColdStartTime},
timer{event, std::bind(&PowerControl::pollPgood, this), pollInterval}
@@ -62,66 +54,15 @@
// Obtain dbus service name
bus.request_name(POWER_IFACE);
- setUpDevice();
+ compatSysTypesFinder = std::make_unique<util::CompatibleSystemTypesFinder>(
+ bus, std::bind_front(&PowerControl::compatibleSystemTypesFound, this));
+
+ deviceFinder = std::make_unique<DeviceFinder>(
+ bus, std::bind_front(&PowerControl::deviceFound, this));
+
setUpGpio();
}
-void PowerControl::getDeviceProperties(const util::DbusPropertyMap& properties)
-{
- uint64_t i2cBus{0};
- uint64_t i2cAddress{0};
- std::string name;
- std::string type;
-
- for (const auto& [property, value] : properties)
- {
- try
- {
- if (property == busPropertyName)
- {
- i2cBus = std::get<uint64_t>(value);
- }
- else if (property == addressPropertyName)
- {
- i2cAddress = std::get<uint64_t>(value);
- }
- else if (property == namePropertyName)
- {
- name = std::get<std::string>(value);
- }
- else if (property == typePropertyName)
- {
- type = std::get<std::string>(value);
- }
- }
- catch (const std::exception& e)
- {
- log<level::INFO>(
- fmt::format("Error getting device properties, error: {}",
- e.what())
- .c_str());
- }
- }
-
- log<level::INFO>(
- fmt::format(
- "Found power sequencer device properties, name: {}, type: {}, bus: {} addr: {:#02x} ",
- name, type, i2cBus, i2cAddress)
- .c_str());
-
- // Create device object
- if (type == "UCD90320")
- {
- device = std::make_unique<UCD90320Monitor>(bus, i2cBus, i2cAddress);
- deviceFound = true;
- }
- else if (type == "UCD90160")
- {
- device = std::make_unique<UCD90160Monitor>(bus, i2cBus, i2cAddress);
- deviceFound = true;
- }
-}
-
int PowerControl::getPgood() const
{
return pgood;
@@ -137,59 +78,9 @@
return state;
}
-void PowerControl::interfacesAddedHandler(sdbusplus::message_t& message)
-{
- // Only continue if message is valid and device has not already been found
- if (!message || deviceFound)
- {
- return;
- }
-
- try
- {
- // Read the dbus message
- sdbusplus::message::object_path path;
- std::map<std::string, std::map<std::string, util::DbusVariant>>
- interfaces;
- message.read(path, interfaces);
-
- for (const auto& [interface, properties] : interfaces)
- {
- log<level::DEBUG>(
- fmt::format(
- "Interfaces added handler found path: {}, interface: {}",
- path.str, interface)
- .c_str());
-
- // Find the device interface, if present
- for (const auto& interfaceName : interfaceNames)
- {
- if (interface == interfaceName)
- {
- log<level::INFO>(
- fmt::format(
- "Interfaces added handler matched interface name: {}",
- interfaceName)
- .c_str());
- getDeviceProperties(properties);
- }
- }
- }
- }
- catch (const std::exception& e)
- {
- // Error trying to read interfacesAdded message.
- log<level::INFO>(
- fmt::format(
- "Error trying to read interfacesAdded message, error: {}",
- e.what())
- .c_str());
- }
-}
-
void PowerControl::onFailureCallback()
{
- log<level::INFO>("After onFailure wait");
+ services.logInfoMsg("After onFailure wait");
onFailure(false);
@@ -201,10 +92,49 @@
bus.call_noreply(method);
}
-void PowerControl::onFailure(bool timeout)
+void PowerControl::onFailure(bool wasTimeOut)
{
- // Call device on failure
- device->onFailure(timeout, powerSupplyError);
+ std::string error;
+ std::map<std::string, std::string> additionalData{};
+
+ // Check if pgood fault occurred on rail monitored by power sequencer device
+ if (device)
+ {
+ try
+ {
+ error = device->findPgoodFault(services, powerSupplyError,
+ additionalData);
+ }
+ catch (const std::exception& e)
+ {
+ services.logErrorMsg(e.what());
+ additionalData.emplace("ERROR", e.what());
+ }
+ }
+
+ // If fault was not isolated to a voltage rail, select a more generic error
+ if (error.empty())
+ {
+ if (!powerSupplyError.empty())
+ {
+ error = powerSupplyError;
+ }
+ else if (wasTimeOut)
+ {
+ error = powerOnTimeoutError;
+ }
+ else
+ {
+ error = shutdownError;
+ }
+ }
+
+ services.logError(error, Entry::Level::Critical, additionalData);
+
+ if (!wasTimeOut)
+ {
+ services.createBMCDump();
+ }
}
void PowerControl::pollPgood()
@@ -215,9 +145,8 @@
const auto now = std::chrono::steady_clock::now();
if (now > pgoodTimeoutTime)
{
- log<level::ERR>(
- fmt::format("Power state transition timeout, state: {}", state)
- .c_str());
+ services.logErrorMsg(std::format(
+ "Power state transition timeout, state: {}", state));
inStateTransition = false;
if (state)
@@ -229,9 +158,8 @@
{
// Time out powering off
std::map<std::string, std::string> additionalData{};
- device->logError(
- "xyz.openbmc_project.Power.Error.PowerOffTimeout",
- additionalData);
+ services.logError(powerOffTimeoutError, Entry::Level::Critical,
+ additionalData);
}
failureFound = true;
@@ -265,7 +193,7 @@
else if (!inStateTransition && (pgoodState == 0) && !failureFound)
{
// Not in power off state, not changing state, and power good is off
- log<level::ERR>("Chassis pgood failure");
+ services.logErrorMsg("Chassis pgood failure");
pgoodWaitTimer.restartOnce(std::chrono::seconds(7));
failureFound = true;
}
@@ -289,8 +217,8 @@
{
if (state == s)
{
- log<level::INFO>(
- fmt::format("Power already at requested state: {}", state).c_str());
+ services.logInfoMsg(
+ std::format("Power already at requested state: {}", state));
return;
}
if (s == 0)
@@ -304,18 +232,17 @@
// If minimum power off time has not passed, wait
if (powerOnAllowedTime > std::chrono::steady_clock::now())
{
- log<level::INFO>(
- fmt::format(
- "Waiting {} seconds until power on allowed",
- std::chrono::duration_cast<std::chrono::seconds>(
- powerOnAllowedTime - std::chrono::steady_clock::now())
- .count())
- .c_str());
+ services.logInfoMsg(std::format(
+ "Waiting {} seconds until power on allowed",
+ std::chrono::duration_cast<std::chrono::seconds>(
+ powerOnAllowedTime - std::chrono::steady_clock::now())
+ .count()));
}
std::this_thread::sleep_until(powerOnAllowedTime);
}
- log<level::INFO>(fmt::format("setState: {}", s).c_str());
+ services.logInfoMsg(std::format("setState: {}", s));
+ services.logInfoMsg(std::format("Powering chassis {}", (s ? "on" : "off")));
powerControlLine.request(
{"phosphor-power-control", gpiod::line_request::DIRECTION_OUTPUT, 0});
powerControlLine.set_value(s);
@@ -334,46 +261,39 @@
emitPropertyChangedSignal("state");
}
-void PowerControl::setUpDevice()
+void PowerControl::compatibleSystemTypesFound(
+ const std::vector<std::string>& types)
{
- try
+ // If we don't already have compatible system types
+ if (compatibleSystemTypes.empty())
{
- // Check if device information is already available
- auto objects = util::getSubTree(bus, "/", interfaceNames, 0);
+ std::string typesStr = format_utils::toString(std::span{types});
+ services.logInfoMsg(
+ std::format("Compatible system types found: {}", typesStr));
- // Search for matching interface in returned objects
- for (const auto& [path, services] : objects)
- {
- log<level::DEBUG>(
- fmt::format("Found path: {}, services: {}", path, services)
- .c_str());
- for (const auto& [service, interfaces] : services)
- {
- log<level::DEBUG>(
- fmt::format("Found service: {}, interfaces: {}", service,
- interfaces)
- .c_str());
- for (const auto& interface : interfaces)
- {
- log<level::DEBUG>(
- fmt::format("Found interface: {}", interface).c_str());
- // Get the properties for the device interface
- auto properties =
- util::getAllProperties(bus, path, interface, service);
+ // Store compatible system types
+ compatibleSystemTypes = types;
- getDeviceProperties(properties);
- }
- }
- }
+ // Load config file and create device object if possible
+ loadConfigFileAndCreateDevice();
}
- catch (const std::exception& e)
+}
+
+void PowerControl::deviceFound(const DeviceProperties& properties)
+{
+ // If we don't already have device properties
+ if (!deviceProperties)
{
- // Interface or property not found. Let the Interfaces Added
- // callback process the information once the interfaces are added to
- // D-Bus.
- log<level::DEBUG>(
- fmt::format("Error setting up device, error: {}", e.what())
- .c_str());
+ services.logInfoMsg(std::format(
+ "Power sequencer device found: type={}, name={}, bus={:d}, address={:#02x}",
+ properties.type, properties.name, properties.bus,
+ properties.address));
+
+ // Store device properties
+ deviceProperties = properties;
+
+ // Load config file and create device object if possible
+ loadConfigFileAndCreateDevice();
}
}
@@ -386,9 +306,7 @@
if (!pgoodLine)
{
std::string errorString{"GPIO line name not found: " + pgoodLineName};
- log<level::ERR>(errorString.c_str());
- report<
- sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure>();
+ services.logErrorMsg(errorString);
throw std::runtime_error(errorString);
}
powerControlLine = gpiod::find_line(powerControlLineName);
@@ -396,9 +314,7 @@
{
std::string errorString{"GPIO line name not found: " +
powerControlLineName};
- log<level::ERR>(errorString.c_str());
- report<
- sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure>();
+ services.logErrorMsg(errorString);
throw std::runtime_error(errorString);
}
@@ -407,7 +323,104 @@
int pgoodState = pgoodLine.get_value();
pgood = pgoodState;
state = pgoodState;
- log<level::INFO>(fmt::format("Pgood state: {}", pgoodState).c_str());
+ services.logInfoMsg(std::format("Pgood state: {}", pgoodState));
+}
+
+void PowerControl::loadConfigFileAndCreateDevice()
+{
+ // If compatible system types and device properties have been found
+ if (!compatibleSystemTypes.empty() && deviceProperties)
+ {
+ // Find the JSON configuration file
+ std::filesystem::path configFile = findConfigFile();
+ if (!configFile.empty())
+ {
+ // Parse the JSON configuration file
+ std::vector<std::unique_ptr<Rail>> rails;
+ if (parseConfigFile(configFile, rails))
+ {
+ // Create the power sequencer device object
+ createDevice(std::move(rails));
+ }
+ }
+ }
+}
+
+std::filesystem::path PowerControl::findConfigFile()
+{
+ // Find config file for current system based on compatible system types
+ std::filesystem::path configFile;
+ if (!compatibleSystemTypes.empty())
+ {
+ try
+ {
+ configFile = config_file_parser::find(compatibleSystemTypes);
+ if (!configFile.empty())
+ {
+ services.logInfoMsg(std::format(
+ "JSON configuration file found: {}", configFile.string()));
+ }
+ }
+ catch (const std::exception& e)
+ {
+ services.logErrorMsg(std::format(
+ "Unable to find JSON configuration file: {}", e.what()));
+ }
+ }
+ return configFile;
+}
+
+bool PowerControl::parseConfigFile(const std::filesystem::path& configFile,
+ std::vector<std::unique_ptr<Rail>>& rails)
+{
+ // Parse JSON configuration file
+ bool wasParsed{false};
+ try
+ {
+ rails = config_file_parser::parse(configFile);
+ wasParsed = true;
+ }
+ catch (const std::exception& e)
+ {
+ services.logErrorMsg(std::format(
+ "Unable to parse JSON configuration file: {}", e.what()));
+ }
+ return wasParsed;
+}
+
+void PowerControl::createDevice(std::vector<std::unique_ptr<Rail>> rails)
+{
+ // Create power sequencer device based on device properties
+ if (deviceProperties)
+ {
+ try
+ {
+ if (deviceProperties->type == UCD90160Device::deviceName)
+ {
+ device = std::make_unique<UCD90160Device>(
+ std::move(rails), services, deviceProperties->bus,
+ deviceProperties->address);
+ }
+ else if (deviceProperties->type == UCD90320Device::deviceName)
+ {
+ device = std::make_unique<UCD90320Device>(
+ std::move(rails), services, deviceProperties->bus,
+ deviceProperties->address);
+ }
+ else
+ {
+ throw std::runtime_error{std::format(
+ "Unsupported device type: {}", deviceProperties->type)};
+ }
+ services.logInfoMsg(std::format(
+ "Power sequencer device created: {}", device->getName()));
+ }
+ catch (const std::exception& e)
+ {
+ services.logErrorMsg(
+ std::format("Unable to create device object: {}", e.what()));
+ }
+ }
}
} // namespace phosphor::power::sequencer
diff --git a/phosphor-power-sequencer/src/power_control.hpp b/phosphor-power-sequencer/src/power_control.hpp
index f623fd8..d6f17d2 100644
--- a/phosphor-power-sequencer/src/power_control.hpp
+++ b/phosphor-power-sequencer/src/power_control.hpp
@@ -1,20 +1,25 @@
#pragma once
+#include "compatible_system_types_finder.hpp"
+#include "device_finder.hpp"
#include "power_interface.hpp"
-#include "power_sequencer_monitor.hpp"
-#include "utility.hpp"
+#include "power_sequencer_device.hpp"
+#include "rail.hpp"
+#include "services.hpp"
#include <gpiod.hpp>
#include <sdbusplus/bus.hpp>
-#include <sdbusplus/bus/match.hpp>
-#include <sdbusplus/message.hpp>
#include <sdbusplus/server/object.hpp>
#include <sdeventplus/clock.hpp>
#include <sdeventplus/event.hpp>
#include <sdeventplus/utility/timer.hpp>
#include <chrono>
+#include <filesystem>
+#include <memory>
+#include <optional>
#include <string>
+#include <vector>
namespace phosphor::power::sequencer
{
@@ -52,12 +57,6 @@
/** @copydoc PowerInterface::getState() */
int getState() const override;
- /**
- * Callback function to handle interfacesAdded D-Bus signals
- * @param msg Expanded sdbusplus message data
- */
- void interfacesAddedHandler(sdbusplus::message_t& msg);
-
/** @copydoc PowerInterface::setPgoodTimeout() */
void setPgoodTimeout(int timeout) override;
@@ -67,6 +66,21 @@
/** @copydoc PowerInterface::setPowerSupplyError() */
void setPowerSupplyError(const std::string& error) override;
+ /**
+ * Callback that is called when a list of compatible system types is found.
+ *
+ * @param types Compatible system types for the current system ordered from
+ * most to least specific
+ */
+ void compatibleSystemTypesFound(const std::vector<std::string>& types);
+
+ /**
+ * Callback that is called when a power sequencer device is found.
+ *
+ * @param properties Properties of device that was found
+ */
+ void deviceFound(const DeviceProperties& properties);
+
private:
/**
* The D-Bus bus object
@@ -74,14 +88,35 @@
sdbusplus::bus_t& bus;
/**
- * The power sequencer device to monitor.
+ * System services like hardware presence and the journal.
*/
- std::unique_ptr<PowerSequencerMonitor> device;
+ BMCServices services;
/**
- * Indicates if a specific power sequencer device has already been found.
+ * Object that finds the compatible system types for the current system.
*/
- bool deviceFound{false};
+ std::unique_ptr<util::CompatibleSystemTypesFinder> compatSysTypesFinder;
+
+ /**
+ * Compatible system types for the current system ordered from most to least
+ * specific.
+ */
+ std::vector<std::string> compatibleSystemTypes;
+
+ /**
+ * Object that finds the power sequencer device in the system.
+ */
+ std::unique_ptr<DeviceFinder> deviceFinder;
+
+ /**
+ * Power sequencer device properties.
+ */
+ std::optional<DeviceProperties> deviceProperties;
+
+ /**
+ * Power sequencer device that enables and monitors the voltage rails.
+ */
+ std::unique_ptr<PowerSequencerDevice> device;
/**
* Indicates if a failure has already been found. Cleared at power on.
@@ -94,11 +129,6 @@
bool inStateTransition{false};
/**
- * The match to Entity Manager interfaces added.
- */
- sdbusplus::bus::match_t match;
-
- /**
* Minimum time from cold start to power on constant
*/
static constexpr std::chrono::seconds minimumColdStartTime{15};
@@ -170,21 +200,17 @@
sdeventplus::utility::Timer<sdeventplus::ClockId::Monotonic> timer;
/**
- * Get the device properties
- * @param properties A map of property names and values
- */
- void getDeviceProperties(const util::DbusPropertyMap& properties);
-
- /**
* Callback to begin failure processing after observing pgood failure wait
*/
void onFailureCallback();
/**
- * Begin pgood failute processing
- * @param timeout if the failure state was determined by timing out
+ * Begin pgood failure processing
+ *
+ * @param wasTimeOut Indicates whether failure state was determined by
+ * timing out
*/
- void onFailure(bool timeout);
+ void onFailure(bool wasTimeOut);
/**
* Polling method for monitoring the system power good
@@ -192,14 +218,50 @@
void pollPgood();
/**
- * Set up power sequencer device
- */
- void setUpDevice();
-
- /**
* Set up GPIOs
*/
void setUpGpio();
+
+ /**
+ * Loads the JSON configuration file and creates the power sequencer device
+ * object.
+ *
+ * Does nothing if the compatible system types or device properties have not
+ * been found yet. These are obtained from D-Bus. The order in which they
+ * are found and the time to find them varies.
+ */
+ void loadConfigFileAndCreateDevice();
+
+ /**
+ * Finds the JSON configuration file for the current system based on the
+ * compatible system types.
+ *
+ * Does nothing if the compatible system types have not been found yet.
+ *
+ * @return absolute path to the config file, or empty path if file not found
+ */
+ std::filesystem::path findConfigFile();
+
+ /**
+ * Parses the specified JSON configuration file.
+ *
+ * Returns the resulting vector of Rail objects in the output parameter.
+ *
+ * @param configFile Absolute path to the config file
+ * @param rails Rail objects within the config file
+ * @return true if file was parsed successfully, false otherwise
+ */
+ bool parseConfigFile(const std::filesystem::path& configFile,
+ std::vector<std::unique_ptr<Rail>>& rails);
+
+ /**
+ * Creates the power sequencer device object based on the device properties.
+ *
+ * Does nothing if the device properties have not been found yet.
+ *
+ * @param rails Voltage rails that are enabled and monitored by the device
+ */
+ void createDevice(std::vector<std::unique_ptr<Rail>> rails);
};
} // namespace phosphor::power::sequencer