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>

8 files changed, 135 insertions, 178 deletions

diff --git a/Makefile.am b/Makefile.am
index b350948a3..d6720abdf 100755
--- a/Makefile.am
+++ b/Makefile.am
@@ -10,7 +10,8 @@ openpower_update_manager_SOURCES = \
 	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 @@ endif
 
 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 1a64fcc27..2c05ea745 100644
--- a/activation.cpp
+++ b/activation.cpp
@@ -208,7 +208,8 @@ bool Activation::validateSignature()
 
 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 @@ bool Activation::fieldModeEnabled()
     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 29ef0e42e..1fcae61c5 100644
--- a/activation.hpp
+++ b/activation.hpp
@@ -313,19 +313,6 @@ class Activation : public ActivationInherit
      * @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 d5940b638..b79028bc7 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 software
 {
 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::processPNORImage()
 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 ItemUpdaterStatic::updateFunctionalAssociation(
 void GardResetStatic::reset()
 {
     // Clear gard partition
-    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);
+    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>();
-    }
-
-    // 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 467c7d93a..86ff68831 100755
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -33,6 +33,7 @@ utest_SOURCES = \
 	../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 83a514d7f..188b0bffd 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 @@ using namespace phosphor::logging;
 
 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::removeReadWritePartition(const std::string& versionId)
 
 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 @@ void ItemUpdaterUbi::reset()
         }
     }
 
-    // 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 @@ void GardResetUbi::reset()
     // 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 95fc2e036..04b24bfd5 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 @@ void EVP_MD_CTX_free(EVP_MD_CTX* ctx)
 }
 
 #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 90569bfb7..9d119b3a0 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