fw-update: Add StartUpdate D-Bus API support for firmware updates
This commit adds support for the StartUpdate D-Bus API to enable FW
updates through the xyz.openbmc_project.Software.Update interface. The
user would need to enable `fw-update-pkg-inotify` meson option to
switch to inotify mechanism.
The existing inotify mechanism has critical limitations:
- Race condition between a BMC code updater and PLDM updater when
accessing images from /tmp/images directory
- Lack of a standard D-Bus interface for firmware operations
- No proper error handling or status reporting
This implementation follows the upstream design for device & component
level inventory & update, based on the proposed code-update design
document [1]. The FW Update is triggered at the top-level i.e. all valid
targets will be updated with a single StartUpdate call.
Key changes:
- Add Update class implementing the Software.Update D-Bus interface
- Add startUpdate method that accepts file descriptors and processes
firmware packages via streams
- Refactor UpdateManager to support both file path and stream-based
package processing
Tests:
- Successful FW Update with both inotify and startUpdate flows on
gb200nvl-obmc platform with multiple components (GPU + SMA)
- FW Update failure case by modifying the package size. The Redfish task
immediately fails with Exception after creation.
[1]: https://gerrit.openbmc.org/c/openbmc/docs/+/76645
Change-Id: Ic2ca74431316161de844f6a3966f612760f5c298
Signed-off-by: P Arun Kumar Reddy <arunpapannagari23@gmail.com>
diff --git a/docs/fw_update_configuration.md b/docs/fw_update_configuration.md
new file mode 100644
index 0000000..98953c4
--- /dev/null
+++ b/docs/fw_update_configuration.md
@@ -0,0 +1,23 @@
+# Firmware Update Configuration
+
+PLDM supports firmware updates through two mechanisms:
+
+1. **D-Bus API**: Using the StartUpdate D-Bus interface for firmware updates
+2. **Inotify monitoring**: Automatic detection of firmware packages placed in
+ `/tmp/images`
+
+The inotify-based firmware update monitoring can be enabled or disabled using
+the meson option `fw-update-pkg-inotify`. When enabled, pldmd will automatically
+monitor the `/tmp/images` directory for new firmware packages and process them
+automatically. When disabled, only D-Bus API-based firmware updates will be
+supported. To disable inotify-based firmware update monitoring (default):
+
+```bash
+meson setup build -Dfw-update-pkg-inotify=disabled
+```
+
+To enable inotify-based firmware update monitoring:
+
+```bash
+meson setup build -Dfw-update-pkg-inotify=enabled
+```
diff --git a/fw-update/device_updater.hpp b/fw-update/device_updater.hpp
index 8b15dfd..790db76 100644
--- a/fw-update/device_updater.hpp
+++ b/fw-update/device_updater.hpp
@@ -54,7 +54,7 @@
* @param[in] updateManager - To update the status of fw update of the
* device
*/
- explicit DeviceUpdater(mctp_eid_t eid, std::ifstream& package,
+ explicit DeviceUpdater(mctp_eid_t eid, std::istream& package,
const FirmwareDeviceIDRecord& fwDeviceIDRecord,
const ComponentImageInfos& compImageInfos,
const ComponentInfo& compInfo,
@@ -199,7 +199,7 @@
mctp_eid_t eid;
/** @brief File stream for firmware update package */
- std::ifstream& package;
+ std::istream& package;
/** @brief FirmwareDeviceIDRecord in the fw update package that matches this
* firmware device
diff --git a/fw-update/update.cpp b/fw-update/update.cpp
new file mode 100644
index 0000000..5423cf8
--- /dev/null
+++ b/fw-update/update.cpp
@@ -0,0 +1,50 @@
+#include "update.hpp"
+
+#include "update_manager.hpp"
+
+namespace pldm
+{
+namespace fw_update
+{
+
+sdbusplus::message::object_path Update::startUpdate(
+ sdbusplus::message::unix_fd image,
+ ApplyTimeIntf::RequestedApplyTimes applyTime [[maybe_unused]])
+{
+ namespace software = sdbusplus::xyz::openbmc_project::Software::server;
+ // If a firmware activation of a package is in progress, don't proceed with
+ // package processing
+ if (updateManager->activation)
+ {
+ if (updateManager->activation->activation() ==
+ software::Activation::Activations::Activating)
+ {
+ throw sdbusplus::xyz::openbmc_project::Common::Error::Unavailable();
+ }
+ else
+ {
+ updateManager->clearActivationInfo();
+ }
+ }
+
+ info("Starting update for image {FD}", "FD", image.fd);
+ char buffer[4096];
+ ssize_t bytesRead;
+ imageStream.str(std::string());
+
+ while ((bytesRead = read(image, buffer, sizeof(buffer))) > 0)
+ {
+ imageStream.write(buffer, bytesRead);
+ }
+
+ if (bytesRead < 0)
+ {
+ throw std::runtime_error("Failed to read image file descriptor");
+ }
+
+ return sdbusplus::message::object_path(updateManager->processStreamDefer(
+ imageStream, imageStream.str().size()));
+}
+
+} // namespace fw_update
+} // namespace pldm
diff --git a/fw-update/update.hpp b/fw-update/update.hpp
new file mode 100644
index 0000000..307b702
--- /dev/null
+++ b/fw-update/update.hpp
@@ -0,0 +1,53 @@
+#pragma once
+
+#include <xyz/openbmc_project/Software/ApplyTime/server.hpp>
+#include <xyz/openbmc_project/Software/Update/server.hpp>
+
+namespace pldm
+{
+
+namespace fw_update
+{
+
+class UpdateManager;
+
+using UpdateIntf = sdbusplus::server::object_t<
+ sdbusplus::xyz::openbmc_project::Software::server::Update>;
+using ApplyTimeIntf =
+ sdbusplus::xyz::openbmc_project::Software::server::ApplyTime;
+
+/** @class Update
+ *
+ * Concrete implementation of xyz.openbmc_project.Software.Update D-Bus
+ * interface
+ */
+class Update : public UpdateIntf
+{
+ public:
+ /** @brief Constructor
+ *
+ * @param[in] bus - Bus to attach to
+ * @param[in] objPath - D-Bus object path
+ * @param[in] updateManager - Reference to FW update manager
+ */
+ Update(sdbusplus::bus::bus& bus, const std::string& path,
+ UpdateManager* updateManager) :
+ UpdateIntf(bus, path.c_str()), updateManager(updateManager),
+ objPath(path)
+ {}
+
+ virtual sdbusplus::message::object_path startUpdate(
+ sdbusplus::message::unix_fd image,
+ ApplyTimeIntf::RequestedApplyTimes applyTime) override;
+
+ ~Update() noexcept override = default;
+
+ private:
+ UpdateManager* updateManager;
+ const std::string objPath;
+ std::stringstream imageStream;
+};
+
+} // namespace fw_update
+
+} // namespace pldm
diff --git a/fw-update/update_manager.cpp b/fw-update/update_manager.cpp
index 946efb3..d545645 100644
--- a/fw-update/update_manager.cpp
+++ b/fw-update/update_manager.cpp
@@ -5,6 +5,7 @@
#include "package_parser.hpp"
#include <phosphor-logging/lg2.hpp>
+#include <sdeventplus/source/event.hpp>
#include <cassert>
#include <cmath>
@@ -23,6 +24,14 @@
namespace fs = std::filesystem;
namespace software = sdbusplus::xyz::openbmc_project::Software::server;
+std::string UpdateManager::getSwId()
+{
+ return std::to_string(
+ std::chrono::duration_cast<std::chrono::seconds>(
+ std::chrono::system_clock::now().time_since_epoch())
+ .count());
+}
+
int UpdateManager::processPackage(const std::filesystem::path& packageFilePath)
{
// If no devices discovered, take no action on the package.
@@ -64,15 +73,64 @@
}
uintmax_t packageSize = package.tellg();
+
+ auto swId = getSwId();
+ objPath = swRootPath + swId;
+
+ fwPackageFilePath = packageFilePath;
+
+ try
+ {
+ processStream(package, packageSize);
+ return 0;
+ }
+ catch (sdbusplus::exception_t& e)
+ {
+ error("Exception occurred while processing the package: {ERROR}",
+ "ERROR", e);
+ package.close();
+ std::filesystem::remove(packageFilePath);
+ return -1;
+ }
+}
+
+std::string UpdateManager::processStreamDefer(std::istream& package,
+ uintmax_t packageSize)
+{
+ auto swId = getSwId();
+ objPath = swRootPath + swId;
+
+ // If no devices discovered, take no action on the package.
+ if (!descriptorMap.size())
+ {
+ error(
+ "No devices discovered, cannot process the PLDM fw update package.");
+ throw sdbusplus::xyz::openbmc_project::Common::Error::Unavailable();
+ }
+
+ updateDeferHandler = std::make_unique<sdeventplus::source::Defer>(
+ event, [this, &package, packageSize](sdeventplus::source::EventBase&) {
+ this->processStream(package, packageSize);
+ });
+
+ return objPath;
+}
+
+void UpdateManager::processStream(std::istream& package, uintmax_t packageSize)
+{
+ startTime = std::chrono::steady_clock::now();
if (packageSize < sizeof(pldm_package_header_information))
{
error(
"PLDM fw update package length {SIZE} less than the length of the package header information '{PACKAGE_HEADER_INFO_SIZE}'.",
"SIZE", packageSize, "PACKAGE_HEADER_INFO_SIZE",
sizeof(pldm_package_header_information));
- package.close();
- std::filesystem::remove(packageFilePath);
- return -1;
+ activation = std::make_unique<Activation>(
+ pldm::utils::DBusHandler::getBus(), objPath,
+ software::Activation::Activations::Invalid, this);
+ parser.reset();
+ throw sdbusplus::error::xyz::openbmc_project::software::update::
+ InvalidImage();
}
package.seekg(0);
@@ -83,15 +141,14 @@
if (parser == nullptr)
{
error("Invalid PLDM package header information");
- package.close();
- std::filesystem::remove(packageFilePath);
- return -1;
+ activation = std::make_unique<Activation>(
+ pldm::utils::DBusHandler::getBus(), objPath,
+ software::Activation::Activations::Invalid, this);
+ parser.reset();
+ throw sdbusplus::error::xyz::openbmc_project::software::update::
+ InvalidImage();
}
- // Populate object path with the hash of the package version
- size_t versionHash = std::hash<std::string>{}(parser->pkgVersion);
- objPath = swRootPath + std::to_string(versionHash);
-
package.seekg(0);
try
{
@@ -103,9 +160,9 @@
activation = std::make_unique<Activation>(
pldm::utils::DBusHandler::getBus(), objPath,
software::Activation::Activations::Invalid, this);
- package.close();
parser.reset();
- return -1;
+ throw sdbusplus::error::xyz::openbmc_project::software::update::
+ InvalidImage();
}
auto deviceUpdaterInfos =
@@ -118,9 +175,9 @@
activation = std::make_unique<Activation>(
pldm::utils::DBusHandler::getBus(), objPath,
software::Activation::Activations::Invalid, this);
- package.close();
parser.reset();
- return 0;
+ throw sdbusplus::error::xyz::openbmc_project::software::update::
+ Incompatible();
}
const auto& fwDeviceIDRecords = parser->getFwDeviceIDRecords();
@@ -138,14 +195,15 @@
compImageInfos, search->second, MAXIMUM_TRANSFER_SIZE, this));
}
- fwPackageFilePath = packageFilePath;
activation = std::make_unique<Activation>(
pldm::utils::DBusHandler::getBus(), objPath,
software::Activation::Activations::Ready, this);
activationProgress = std::make_unique<ActivationProgress>(
pldm::utils::DBusHandler::getBus(), objPath);
- return 0;
+#ifndef FW_UPDATE_INOTIFY_ENABLED
+ activation->activation(software::Activation::Activations::Activating);
+#endif
}
DeviceUpdaterInfos UpdateManager::associatePkgToDevices(
@@ -258,10 +316,13 @@
activationProgress.reset();
objPath.clear();
+ if (package.is_open())
+ {
+ package.close();
+ }
deviceUpdaterMap.clear();
deviceUpdateCompletionMap.clear();
parser.reset();
- package.close();
std::filesystem::remove(fwPackageFilePath);
totalNumComponentUpdates = 0;
compUpdateCompletedCount = 0;
diff --git a/fw-update/update_manager.hpp b/fw-update/update_manager.hpp
index 012c83e..5aa80ec 100644
--- a/fw-update/update_manager.hpp
+++ b/fw-update/update_manager.hpp
@@ -4,15 +4,23 @@
#include "common/types.hpp"
#include "device_updater.hpp"
#include "fw-update/activation.hpp"
+#include "fw-update/update.hpp"
+#ifdef FW_UPDATE_INOTIFY_ENABLED
+#include "fw-update/watch.hpp"
+#endif
#include "package_parser.hpp"
#include "requester/handler.hpp"
-#include "watch.hpp"
#include <libpldm/base.h>
+#include <sdbusplus/async.hpp>
+#include <sdbusplus/server/object.hpp>
+#include <xyz/openbmc_project/Software/Activation/server.hpp>
+
#include <chrono>
#include <filesystem>
#include <fstream>
+#include <sstream>
#include <tuple>
#include <unordered_map>
@@ -48,11 +56,17 @@
const ComponentInfoMap& componentInfoMap) :
event(event), handler(handler), instanceIdDb(instanceIdDb),
descriptorMap(descriptorMap), componentInfoMap(componentInfoMap),
+#ifdef FW_UPDATE_INOTIFY_ENABLED
watch(event.get(),
[this](std::string& packageFilePath) {
return this->processPackage(
std::filesystem::path(packageFilePath));
}),
+#else
+ updater(std::make_unique<Update>(pldm::utils::DBusHandler::getBus(),
+ "/xyz/openbmc_project/software/pldm",
+ this)),
+#endif
totalNumComponentUpdates(0), compUpdateCompletedCount(0)
{}
@@ -71,6 +85,25 @@
int processPackage(const std::filesystem::path& packageFilePath);
+ /** @brief Process the firmware update package
+ *
+ * @param[in] packageStream - Stream of the firmware update package
+ * @param[in] packageSize - Size of the firmware update package
+ *
+ * @return Object path of the created Software object
+ */
+ void processStream(std::istream& packageStream, uintmax_t packageSize);
+
+ /** @brief Defers processing the package stream
+ *
+ * @param[in] packageStream - Stream of the firmware update package
+ * @param[in] packageSize - Size of the firmware update package
+ *
+ * @return Object path of the created Software object as a string
+ */
+ std::string processStreamDefer(std::istream& packageStream,
+ uintmax_t packageSize);
+
void updateDeviceCompletion(mctp_eid_t eid, bool status);
void updateActivationProgress();
@@ -91,20 +124,31 @@
const DescriptorMap& descriptorMap,
TotalComponentUpdates& totalNumComponentUpdates);
+ /** @brief Generate a unique software ID based on current timestamp
+ *
+ * @return String representation of the current timestamp in seconds
+ */
+ static std::string getSwId();
+
const std::string swRootPath{"/xyz/openbmc_project/software/"};
Event& event; //!< reference to PLDM daemon's main event loop
/** @brief PLDM request handler */
pldm::requester::Handler<pldm::requester::Request>& handler;
InstanceIdDb& instanceIdDb; //!< reference to an InstanceIdDb
+ std::unique_ptr<Activation> activation;
+
private:
/** @brief Device identifiers of the managed FDs */
const DescriptorMap& descriptorMap;
/** @brief Component information needed for the update of the managed FDs */
const ComponentInfoMap& componentInfoMap;
+#ifdef FW_UPDATE_INOTIFY_ENABLED
Watch watch;
+#else
+ std::unique_ptr<Update> updater;
+#endif
- std::unique_ptr<Activation> activation;
std::unique_ptr<ActivationProgress> activationProgress;
std::string objPath;
@@ -128,6 +172,7 @@
*/
size_t compUpdateCompletedCount;
decltype(std::chrono::steady_clock::now()) startTime;
+ std::unique_ptr<sdeventplus::source::Defer> updateDeferHandler;
};
} // namespace fw_update
diff --git a/meson.build b/meson.build
index 461d657..9b5b00b 100644
--- a/meson.build
+++ b/meson.build
@@ -142,6 +142,11 @@
conf_data.set('SENSOR_POLLING_TIME', get_option('sensor-polling-time'))
conf_data.set('UPDATE_TIMEOUT_SECONDS', get_option('update-timeout-seconds'))
+# Firmware update inotify option
+if get_option('fw-update-pkg-inotify').allowed()
+ conf_data.set('FW_UPDATE_INOTIFY_ENABLED', 1)
+endif
+
configure_file(output: 'config.h', configuration: conf_data)
add_project_arguments(
@@ -242,18 +247,26 @@
deps += [libpldmresponder_dep]
endif
-executable(
- 'pldmd',
- 'pldmd/pldmd.cpp',
- 'pldmd/dbus_impl_pdr.cpp',
+fw_update_sources = [
'fw-update/activation.cpp',
'fw-update/inventory_manager.cpp',
'fw-update/firmware_inventory_manager.cpp',
'fw-update/firmware_inventory.cpp',
'fw-update/package_parser.cpp',
'fw-update/device_updater.cpp',
- 'fw-update/watch.cpp',
'fw-update/update_manager.cpp',
+ 'fw-update/update.cpp',
+]
+
+if get_option('fw-update-pkg-inotify').allowed()
+ fw_update_sources += 'fw-update/watch.cpp'
+endif
+
+executable(
+ 'pldmd',
+ 'pldmd/pldmd.cpp',
+ 'pldmd/dbus_impl_pdr.cpp',
+ fw_update_sources,
'platform-mc/dbus_impl_fru.cpp',
'platform-mc/terminus_manager.cpp',
'platform-mc/terminus.cpp',
diff --git a/meson.options b/meson.options
index bd51f70..f49ffc0 100644
--- a/meson.options
+++ b/meson.options
@@ -223,3 +223,11 @@
`GetSensorReading` if the sensor need to be updated.''',
value: 249,
)
+
+# Firmware Update configuration parameters
+option(
+ 'fw-update-pkg-inotify',
+ type: 'feature',
+ value: 'disabled',
+ description: 'Enable inotify-based firmware update package monitoring',
+)