Refactor: Use hiomapd instead of mboxd
mbox service is renamed to hiomapd and provides "Suspend" and
"Resume" method calls, which is more appropriate for raw "cmd" calls.
Use the new methods to suspend and resume hiomapd.
Move "getService()" function into utils so it is shared by multiple
files.
Tested: Verify the reset works on Romulus
Change-Id: I8f89de134b13126697bfc69a21a3148a01c34cca
Signed-off-by: Lei YU <mine260309@gmail.com>
diff --git a/Makefile.am b/Makefile.am
index b350948..d6720ab 100755
--- a/Makefile.am
+++ b/Makefile.am
@@ -10,7 +10,8 @@
activation.cpp \
version.cpp \
item_updater.cpp \
- item_updater_main.cpp
+ item_updater_main.cpp \
+ utils.cpp
if UBIFS_LAYOUT
include ubi/Makefile.am.include
@@ -24,8 +25,7 @@
if WANT_SIGNATURE_VERIFY_BUILD
openpower_update_manager_SOURCES += \
- image_verify.cpp \
- utils.cpp
+ image_verify.cpp
endif
nodist_openpower_update_manager_SOURCES = \
diff --git a/activation.cpp b/activation.cpp
index 1a64fcc..2c05ea7 100644
--- a/activation.cpp
+++ b/activation.cpp
@@ -208,7 +208,8 @@
bool Activation::fieldModeEnabled()
{
- auto fieldModeSvc = getService(bus, FIELDMODE_PATH, FIELDMODE_INTERFACE);
+ auto fieldModeSvc =
+ utils::getService(bus, FIELDMODE_PATH, FIELDMODE_INTERFACE);
auto method = bus.new_method_call(fieldModeSvc.c_str(), FIELDMODE_PATH,
"org.freedesktop.DBus.Properties", "Get");
@@ -226,40 +227,6 @@
return sdbusplus::message::variant_ns::get<bool>(fieldMode);
}
-std::string Activation::getService(sdbusplus::bus::bus& bus,
- const std::string& path,
- const std::string& intf)
-{
- auto mapperCall = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
- MAPPER_INTERFACE, "GetObject");
-
- mapperCall.append(path);
- mapperCall.append(std::vector<std::string>({intf}));
-
- auto mapperResponseMsg = bus.call(mapperCall);
-
- if (mapperResponseMsg.is_method_error())
- {
- log<level::ERR>("ERROR in getting service",
- entry("PATH=%s", path.c_str()),
- entry("INTERFACE=%s", intf.c_str()));
-
- elog<InternalFailure>();
- }
-
- std::map<std::string, std::vector<std::string>> mapperResponse;
- mapperResponseMsg.read(mapperResponse);
-
- if (mapperResponse.begin() == mapperResponse.end())
- {
- log<level::ERR>("ERROR reading mapper response",
- entry("PATH=%s", path.c_str()),
- entry("INTERFACE=%s", intf.c_str()));
-
- elog<InternalFailure>();
- }
- return mapperResponse.begin()->first;
-}
#endif
} // namespace updater
diff --git a/activation.hpp b/activation.hpp
index 29ef0e4..1fcae61 100644
--- a/activation.hpp
+++ b/activation.hpp
@@ -313,19 +313,6 @@
* @error InternalFailure exception thrown
*/
bool fieldModeEnabled();
-
- /**
- * @brief Gets the D-Bus Service name for the input D-Bus path
- *
- * @param[in] bus - Bus handler
- * @param[in] path - Object Path
- * @param[in] intf - Interface
- *
- * @return Service name
- * @error InternalFailure exception thrown
- */
- std::string getService(sdbusplus::bus::bus& bus, const std::string& path,
- const std::string& intf);
#endif
};
diff --git a/static/item_updater_static.cpp b/static/item_updater_static.cpp
index d5940b6..b79028b 100644
--- a/static/item_updater_static.cpp
+++ b/static/item_updater_static.cpp
@@ -3,6 +3,7 @@
#include "item_updater_static.hpp"
#include "activation_static.hpp"
+#include "utils.hpp"
#include "version.hpp"
#include <array>
@@ -199,9 +200,6 @@
{
namespace updater
{
-// TODO: Change paths once openbmc/openbmc#1663 is completed.
-constexpr auto MBOXD_INTERFACE = "org.openbmc.mboxd";
-constexpr auto MBOXD_PATH = "/org/openbmc/mboxd";
std::unique_ptr<Activation> ItemUpdaterStatic::createActivationObject(
const std::string& path, const std::string& versionId,
@@ -308,44 +306,15 @@
void ItemUpdaterStatic::reset()
{
auto partitions = utils::getPartsToClear();
- std::vector<uint8_t> mboxdArgs;
- // Suspend mboxd - no args required.
- auto dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH,
- MBOXD_INTERFACE, "cmd");
- dbusCall.append(static_cast<uint8_t>(3), mboxdArgs);
+ utils::hiomapdSuspend(bus);
- try
- {
- bus.call_noreply(dbusCall);
- }
- catch (const SdBusError& e)
- {
- log<level::ERR>("Error in mboxd suspend call",
- entry("ERROR=%s", e.what()));
- elog<InternalFailure>();
- }
for (auto p : partitions)
{
utils::pnorClear(p.first, p.second);
}
- // Resume mboxd with arg 1, indicating that the flash was modified.
- dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH, MBOXD_INTERFACE,
- "cmd");
- mboxdArgs.push_back(1);
- dbusCall.append(static_cast<uint8_t>(4), mboxdArgs);
-
- try
- {
- bus.call_noreply(dbusCall);
- }
- catch (const SdBusError& e)
- {
- log<level::ERR>("Error in mboxd resume call",
- entry("ERROR=%s", e.what()));
- elog<InternalFailure>();
- }
+ utils::hiomapdResume(bus);
}
bool ItemUpdaterStatic::isVersionFunctional(const std::string& versionId)
@@ -390,43 +359,11 @@
void GardResetStatic::reset()
{
// Clear gard partition
- std::vector<uint8_t> mboxdArgs;
+ utils::hiomapdSuspend(bus);
- auto dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH,
- MBOXD_INTERFACE, "cmd");
- // Suspend mboxd - no args required.
- dbusCall.append(static_cast<uint8_t>(3), mboxdArgs);
-
- try
- {
- bus.call_noreply(dbusCall);
- }
- catch (const SdBusError& e)
- {
- log<level::ERR>("Error in mboxd suspend call",
- entry("ERROR=%s", e.what()));
- elog<InternalFailure>();
- }
-
- // Clear guard partition
utils::pnorClear("GUARD");
- dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH, MBOXD_INTERFACE,
- "cmd");
- // Resume mboxd with arg 1, indicating that the flash is modified.
- mboxdArgs.push_back(1);
- dbusCall.append(static_cast<uint8_t>(4), mboxdArgs);
-
- try
- {
- bus.call_noreply(dbusCall);
- }
- catch (const SdBusError& e)
- {
- log<level::ERR>("Error in mboxd resume call",
- entry("ERROR=%s", e.what()));
- elog<InternalFailure>();
- }
+ utils::hiomapdResume(bus);
}
} // namespace updater
diff --git a/test/Makefile.am b/test/Makefile.am
index 467c7d9..86ff688 100755
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -33,6 +33,7 @@
../item_updater.cpp \
../org/openbmc/Associations/server.cpp \
../image_verify.cpp \
+ ../utils.cpp \
../msl_verify.cpp \
../ubi/activation_ubi.cpp \
../ubi/item_updater_ubi.cpp \
diff --git a/ubi/item_updater_ubi.cpp b/ubi/item_updater_ubi.cpp
index 83a514d..188b0bf 100644
--- a/ubi/item_updater_ubi.cpp
+++ b/ubi/item_updater_ubi.cpp
@@ -4,6 +4,7 @@
#include "activation_ubi.hpp"
#include "serialize.hpp"
+#include "utils.hpp"
#include "version.hpp"
#include "xyz/openbmc_project/Common/error.hpp"
@@ -31,10 +32,6 @@
constexpr auto squashFSImage = "pnor.xz.squashfs";
-// TODO: Change paths once openbmc/openbmc#1663 is completed.
-constexpr auto MBOXD_INTERFACE = "org.openbmc.mboxd";
-constexpr auto MBOXD_PATH = "/org/openbmc/mboxd";
-
std::unique_ptr<Activation> ItemUpdaterUbi::createActivationObject(
const std::string& path, const std::string& versionId,
const std::string& extVersion,
@@ -213,20 +210,7 @@
void ItemUpdaterUbi::reset()
{
- std::vector<uint8_t> mboxdArgs;
-
- // Suspend mboxd - no args required.
- auto dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH,
- MBOXD_INTERFACE, "cmd");
-
- dbusCall.append(static_cast<uint8_t>(3), mboxdArgs);
-
- auto responseMsg = bus.call(dbusCall);
- if (responseMsg.is_method_error())
- {
- log<level::ERR>("Error in mboxd suspend call");
- elog<InternalFailure>();
- }
+ utils::hiomapdSuspend(bus);
constexpr static auto patchDir = "/usr/local/share/pnor";
if (fs::is_directory(patchDir))
@@ -259,21 +243,7 @@
}
}
- // Resume mboxd with arg 1, indicating that the flash was modified.
- dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH, MBOXD_INTERFACE,
- "cmd");
-
- mboxdArgs.push_back(1);
- dbusCall.append(static_cast<uint8_t>(4), mboxdArgs);
-
- responseMsg = bus.call(dbusCall);
- if (responseMsg.is_method_error())
- {
- log<level::ERR>("Error in mboxd resume call");
- elog<InternalFailure>();
- }
-
- return;
+ utils::hiomapdResume(bus);
}
bool ItemUpdaterUbi::isVersionFunctional(const std::string& versionId)
@@ -425,39 +395,15 @@
// need to be updated in the future.
auto path = fs::path(PNOR_PRSV_ACTIVE_PATH);
path /= "GUARD";
- std::vector<uint8_t> mboxdArgs;
- auto dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH,
- MBOXD_INTERFACE, "cmd");
-
- // Suspend mboxd - no args required.
- dbusCall.append(static_cast<uint8_t>(3), mboxdArgs);
-
- auto responseMsg = bus.call(dbusCall);
- if (responseMsg.is_method_error())
- {
- log<level::ERR>("Error in mboxd suspend call");
- elog<InternalFailure>();
- }
+ utils::hiomapdSuspend(bus);
if (fs::is_regular_file(path))
{
fs::remove(path);
}
- dbusCall = bus.new_method_call(MBOXD_INTERFACE, MBOXD_PATH, MBOXD_INTERFACE,
- "cmd");
-
- // Resume mboxd with arg 1, indicating that the flash is modified.
- mboxdArgs.push_back(1);
- dbusCall.append(static_cast<uint8_t>(4), mboxdArgs);
-
- responseMsg = bus.call(dbusCall);
- if (responseMsg.is_method_error())
- {
- log<level::ERR>("Error in mboxd resume call");
- elog<InternalFailure>();
- }
+ utils::hiomapdResume(bus);
}
} // namespace updater
diff --git a/utils.cpp b/utils.cpp
index 95fc2e0..04b24bf 100644
--- a/utils.cpp
+++ b/utils.cpp
@@ -1,5 +1,12 @@
+#include "config.h"
+
#include "utils.hpp"
+#include <phosphor-logging/elog-errors.hpp>
+#include <phosphor-logging/elog.hpp>
+#include <phosphor-logging/log.hpp>
+#include <xyz/openbmc_project/Common/error.hpp>
+
#if OPENSSL_VERSION_NUMBER < 0x10100000L
#include <string.h>
@@ -27,3 +34,83 @@
}
#endif // OPENSSL_VERSION_NUMBER < 0x10100000L
+
+namespace utils
+{
+
+using sdbusplus::exception::SdBusError;
+using namespace phosphor::logging;
+
+constexpr auto HIOMAPD_PATH = "/xyz/openbmc_project/Hiomapd";
+constexpr auto HIOMAPD_INTERFACE = "xyz.openbmc_project.Hiomapd.Control";
+
+using InternalFailure =
+ sdbusplus::xyz::openbmc_project::Common::Error::InternalFailure;
+
+std::string getService(sdbusplus::bus::bus& bus, const std::string& path,
+ const std::string& intf)
+{
+ auto mapper = bus.new_method_call(MAPPER_BUSNAME, MAPPER_PATH,
+ MAPPER_INTERFACE, "GetObject");
+
+ mapper.append(path, std::vector<std::string>({intf}));
+ try
+ {
+ auto mapperResponseMsg = bus.call(mapper);
+
+ std::vector<std::pair<std::string, std::vector<std::string>>>
+ mapperResponse;
+ mapperResponseMsg.read(mapperResponse);
+ if (mapperResponse.empty())
+ {
+ log<level::ERR>("Error reading mapper response");
+ throw std::runtime_error("Error reading mapper response");
+ }
+ return mapperResponse[0].first;
+ }
+ catch (const sdbusplus::exception::SdBusError& ex)
+ {
+ log<level::ERR>("Mapper call failed", entry("METHOD=%d", "GetObject"),
+ entry("PATH=%s", path.c_str()),
+ entry("INTERFACE=%s", intf.c_str()));
+ throw std::runtime_error("Mapper call failed");
+ }
+}
+
+void hiomapdSuspend(sdbusplus::bus::bus& bus)
+{
+ auto service = getService(bus, HIOMAPD_PATH, HIOMAPD_INTERFACE);
+ auto method = bus.new_method_call(service.c_str(), HIOMAPD_PATH,
+ HIOMAPD_INTERFACE, "Suspend");
+
+ try
+ {
+ bus.call_noreply(method);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in mboxd suspend call",
+ entry("ERROR=%s", e.what()));
+ }
+}
+
+void hiomapdResume(sdbusplus::bus::bus& bus)
+{
+ auto service = getService(bus, HIOMAPD_PATH, HIOMAPD_INTERFACE);
+ auto method = bus.new_method_call(service.c_str(), HIOMAPD_PATH,
+ HIOMAPD_INTERFACE, "Resume");
+
+ method.append(true); // Indicate PNOR is modified
+
+ try
+ {
+ bus.call_noreply(method);
+ }
+ catch (const SdBusError& e)
+ {
+ log<level::ERR>("Error in mboxd suspend call",
+ entry("ERROR=%s", e.what()));
+ }
+}
+
+} // namespace utils
diff --git a/utils.hpp b/utils.hpp
index 90569bf..9d119b3 100644
--- a/utils.hpp
+++ b/utils.hpp
@@ -7,9 +7,41 @@
#include <openssl/evp.h>
+#include <sdbusplus/bus.hpp>
+
extern "C" {
EVP_MD_CTX* EVP_MD_CTX_new(void);
void EVP_MD_CTX_free(EVP_MD_CTX* ctx);
}
+namespace utils
+{
+
+/**
+ * @brief Gets the D-Bus Service name for the input D-Bus path
+ *
+ * @param[in] bus - Bus handler
+ * @param[in] path - Object Path
+ * @param[in] intf - Interface
+ *
+ * @return Service name
+ * @error InternalFailure exception thrown
+ */
+std::string getService(sdbusplus::bus::bus& bus, const std::string& path,
+ const std::string& intf);
+
+/** @brief Suspend hiomapd.
+ *
+ * @param[in] bus - The D-Bus bus object.
+ */
+void hiomapdSuspend(sdbusplus::bus::bus& bus);
+
+/** @brief Resume hiomapd.
+ *
+ * @param[in] bus - The D-Bus bus object.
+ */
+void hiomapdResume(sdbusplus::bus::bus& bus);
+
+} // namespace utils
+
#endif // OPENSSL_VERSION_NUMBER < 0x10100000L