| From c76bd9e41b07253efa28e034cce42d4d48b756ba Mon Sep 17 00:00:00 2001 |
| From: Hao Jiang <jianghao@google.com> |
| Date: Fri, 7 Apr 2023 23:51:14 +0000 |
| Subject: [PATCH 27/44] nvmesensor: Check SCS for secondary controllers |
| |
| The Secondary Controller State indicates if the secondary controller has |
| been enabled by CC.EN. For disabled controllers, the StorageController |
| and NVMeAdmin interface should not be created. |
| |
| It is because the ctrl_list will list all controllers in the system |
| regardless of the enable status. The number of supported controllers can |
| be large compare to the active ones in the system. In our example |
| system, the total controllers in one SSD can live up to 128 but we |
| only enble one or two of them. The unused controller can seriously slow |
| down the performance of dbus if we chose to expose them. |
| |
| Signed-off-by: Hao Jiang <jianghao@google.com> |
| Change-Id: I6fdbb503550968d013e16822f9d11e9962bb93a4 |
| --- |
| src/NVMeController.cpp | 100 ++++++++++++++++++++++++++++++++--------- |
| src/NVMeController.hpp | 54 ++++++++++++++++++---- |
| src/NVMeSubsys.cpp | 19 +++++++- |
| 3 files changed, 141 insertions(+), 32 deletions(-) |
| |
| diff --git a/src/NVMeController.cpp b/src/NVMeController.cpp |
| index 7e26643..e40da17 100644 |
| --- a/src/NVMeController.cpp |
| +++ b/src/NVMeController.cpp |
| @@ -14,35 +14,67 @@ using sdbusplus::xyz::openbmc_project::Inventory::Item::server:: |
| StorageController; |
| using sdbusplus::xyz::openbmc_project::NVMe::server::NVMeAdmin; |
| |
| -NVMeController::NVMeController( |
| +NVMeControllerEnabled::NVMeControllerEnabled( |
| boost::asio::io_context& io, sdbusplus::asio::object_server& objServer, |
| std::shared_ptr<sdbusplus::asio::connection> conn, std::string path, |
| std::shared_ptr<NVMeMiIntf> nvmeIntf, nvme_mi_ctrl_t ctrl) : |
| + NVMeController(io, objServer, conn, path, nvmeIntf, ctrl), |
| StorageController(dynamic_cast<sdbusplus::bus_t&>(*conn), path.c_str()), |
| NVMeAdmin(*conn, path.c_str(), |
| - {{"FirmwareCommitStatus", {FwCommitStatus::Ready}}}), |
| - io(io), objServer(objServer), conn(conn), path(path), nvmeIntf(nvmeIntf), |
| - nvmeCtrl(ctrl) |
| + {{"FirmwareCommitStatus", {FwCommitStatus::Ready}}}) |
| { |
| - StorageController::emit_added(); |
| - NVMeAdmin::emit_added(); |
| assocIntf = objServer.add_interface( |
| path, "xyz.openbmc_project.Association.Definitions"); |
| |
| // regiester a property with empty association |
| assocIntf->register_property("Associations", std::vector<Association>{}); |
| assocIntf->initialize(); |
| + |
| + StorageController::emit_added(); |
| + NVMeAdmin::emit_added(); |
| } |
| |
| -void NVMeController::start(std::shared_ptr<NVMeControllerPlugin> nvmePlugin) |
| +NVMeControllerEnabled::NVMeControllerEnabled(NVMeController&& nvmeController) : |
| + NVMeController(std::move(nvmeController)), |
| + StorageController( |
| + dynamic_cast<sdbusplus::bus_t&>(*this->NVMeController::conn), |
| + this->NVMeController::path.c_str()), |
| + NVMeAdmin(*this->NVMeController::conn, this->NVMeController::path.c_str(), |
| + {{"FirmwareCommitStatus", {FwCommitStatus::Ready}}}) |
| { |
| - plugin = nvmePlugin; |
| + assocIntf = objServer.add_interface( |
| + path, "xyz.openbmc_project.Association.Definitions"); |
| + |
| + // the association could have be set via NVMeController, set the association |
| + // dbus property accordingly |
| + std::vector<Association> associations; |
| + for (const auto& subsys : subsystems) |
| + { |
| + associations.emplace_back("storage", "storage_controller", subsys); |
| + } |
| + |
| + for (const auto& cntrl : secondaryControllers) |
| + { |
| + associations.emplace_back("secondary", "primary", cntrl); |
| + } |
| + |
| + assocIntf->register_property("Associations", associations); |
| + assocIntf->initialize(); |
| + |
| + StorageController::emit_added(); |
| + NVMeAdmin::emit_added(); |
| +} |
| + |
| +void NVMeControllerEnabled::start( |
| + std::shared_ptr<NVMeControllerPlugin> nvmePlugin) |
| +{ |
| + this->NVMeController::start(std::move(nvmePlugin)); |
| } |
| |
| -sdbusplus::message::unix_fd NVMeController::getLogPage(uint8_t lid, |
| - uint32_t nsid, |
| - uint8_t lsp, |
| - uint16_t lsi) |
| +sdbusplus::message::unix_fd NVMeControllerEnabled::getLogPage(uint8_t lid, |
| + uint32_t nsid, |
| + uint8_t lsp, |
| + uint16_t lsi) |
| { |
| std::array<int, 2> pipe; |
| if (::pipe(pipe.data()) < 0) |
| @@ -121,8 +153,8 @@ sdbusplus::message::unix_fd NVMeController::getLogPage(uint8_t lid, |
| return sdbusplus::message::unix_fd{pipe[0]}; |
| } |
| |
| -sdbusplus::message::unix_fd NVMeController::identify(uint8_t cns, uint32_t nsid, |
| - uint16_t cntid) |
| +sdbusplus::message::unix_fd |
| + NVMeControllerEnabled::identify(uint8_t cns, uint32_t nsid, uint16_t cntid) |
| { |
| std::array<int, 2> pipe; |
| if (::pipe(pipe.data()) < 0) |
| @@ -154,8 +186,8 @@ sdbusplus::message::unix_fd NVMeController::identify(uint8_t cns, uint32_t nsid, |
| return sdbusplus::message::unix_fd{pipe[0]}; |
| } |
| |
| -NVMeAdmin::FwCommitStatus |
| - NVMeController::firmwareCommitStatus(NVMeAdmin::FwCommitStatus status) |
| +NVMeAdmin::FwCommitStatus NVMeControllerEnabled::firmwareCommitStatus( |
| + NVMeAdmin::FwCommitStatus status) |
| { |
| auto commitStatus = this->NVMeAdmin::firmwareCommitStatus(); |
| // The function is only allowed to reset the status back to ready |
| @@ -168,8 +200,8 @@ NVMeAdmin::FwCommitStatus |
| return this->NVMeAdmin::firmwareCommitStatus(status); |
| } |
| |
| -void NVMeController::firmwareCommitAsync(uint8_t commitAction, |
| - uint8_t firmwareSlot, bool bpid) |
| +void NVMeControllerEnabled::firmwareCommitAsync(uint8_t commitAction, |
| + uint8_t firmwareSlot, bool bpid) |
| { |
| auto commitStatus = this->NVMeAdmin::firmwareCommitStatus(); |
| if (commitStatus != FwCommitStatus::Ready) |
| @@ -197,13 +229,31 @@ void NVMeController::firmwareCommitAsync(uint8_t commitAction, |
| }); |
| } |
| |
| -NVMeController::~NVMeController() |
| +NVMeControllerEnabled::~NVMeControllerEnabled() |
| { |
| - objServer.remove_interface(assocIntf); |
| NVMeAdmin::emit_removed(); |
| StorageController::emit_removed(); |
| } |
| |
| +NVMeController::NVMeController( |
| + boost::asio::io_context& io, sdbusplus::asio::object_server& objServer, |
| + std::shared_ptr<sdbusplus::asio::connection> conn, std::string path, |
| + std::shared_ptr<NVMeMiIntf> nvmeIntf, nvme_mi_ctrl_t ctrl) : |
| + io(io), |
| + objServer(objServer), conn(conn), path(path), nvmeIntf(nvmeIntf), |
| + nvmeCtrl(ctrl) |
| +{} |
| + |
| +NVMeController::~NVMeController() |
| +{ |
| + objServer.remove_interface(assocIntf); |
| +} |
| + |
| +void NVMeController::start(std::shared_ptr<NVMeControllerPlugin> nvmePlugin) |
| +{ |
| + plugin = nvmePlugin; |
| +} |
| + |
| void NVMeController::setSecAssoc( |
| const std::vector<std::shared_ptr<NVMeController>> secCntrls) |
| { |
| @@ -230,7 +280,10 @@ void NVMeController::setSecAssoc( |
| associations.emplace_back("secondary", "primary", cntrl); |
| } |
| |
| - assocIntf->set_property("Associations", associations); |
| + if (assocIntf) |
| + { |
| + assocIntf->set_property("Associations", associations); |
| + } |
| } |
| |
| void NVMeController::addSubsystemAssociation(const std::string& subsysPath) |
| @@ -248,5 +301,8 @@ void NVMeController::addSubsystemAssociation(const std::string& subsysPath) |
| associations.emplace_back("secondary", "primary", cntrl); |
| } |
| |
| - assocIntf->set_property("Associations", associations); |
| + if (assocIntf) |
| + { |
| + assocIntf->set_property("Associations", associations); |
| + } |
| } |
| diff --git a/src/NVMeController.hpp b/src/NVMeController.hpp |
| index c964b87..5a0f36f 100644 |
| --- a/src/NVMeController.hpp |
| +++ b/src/NVMeController.hpp |
| @@ -12,12 +12,18 @@ |
| #include <utility> |
| |
| class NVMeControllerPlugin; |
| -class NVMeController : |
| - private sdbusplus::xyz::openbmc_project::Inventory::Item::server:: |
| - StorageController, |
| - private sdbusplus::xyz::openbmc_project::NVMe::server::NVMeAdmin, |
| - public std::enable_shared_from_this<NVMeController> |
| |
| +/** |
| + * @brief A class to represent the NVMeController has not been enabled (CC.EN = |
| + * 0) |
| + * |
| + * The disabled controllers still have cntrl_id and are listed in the |
| + * cntrl_list. However the functionility has been disabled so neither |
| + * StorageController nor NVMeAdmin interface should be exposed for the disabled |
| + * controllers. |
| + * |
| + */ |
| +class NVMeController |
| { |
| public: |
| NVMeController(boost::asio::io_context& io, |
| @@ -26,9 +32,9 @@ class NVMeController : |
| std::string path, std::shared_ptr<NVMeMiIntf> nvmeIntf, |
| nvme_mi_ctrl_t ctrl); |
| |
| - ~NVMeController() override; |
| + virtual ~NVMeController(); |
| |
| - void start(std::shared_ptr<NVMeControllerPlugin> nvmePlugin); |
| + virtual void start(std::shared_ptr<NVMeControllerPlugin> nvmePlugin); |
| |
| // setup association to the secondary controllers. Clear the Association if |
| // empty. |
| @@ -60,7 +66,7 @@ class NVMeController : |
| */ |
| void addSubsystemAssociation(const std::string& subsysPath); |
| |
| - private: |
| + protected: |
| friend class NVMeControllerPlugin; |
| |
| boost::asio::io_context& io; |
| @@ -80,7 +86,39 @@ class NVMeController : |
| |
| // NVMe Plug-in for vendor defined command/field |
| std::weak_ptr<NVMeControllerPlugin> plugin; |
| +}; |
| + |
| +/** |
| + * @brief A class for the NVMe controller that has been enabled (CC.EN = 1) |
| + * |
| + * The premitted NVMe Admin cmds should be anable to processed via the enabled |
| + * controller (e.g reading the temletries or other admin tasks). Thus the |
| + * NVMeAmin and StorageController Dbus interface will be exposed via this class. |
| + * |
| + */ |
| +class NVMeControllerEnabled : |
| + public NVMeController, |
| + private sdbusplus::xyz::openbmc_project::Inventory::Item::server:: |
| + StorageController, |
| + private sdbusplus::xyz::openbmc_project::NVMe::server::NVMeAdmin, |
| + public std::enable_shared_from_this<NVMeControllerEnabled> |
| + |
| +{ |
| + public: |
| + NVMeControllerEnabled(boost::asio::io_context& io, |
| + sdbusplus::asio::object_server& objServer, |
| + std::shared_ptr<sdbusplus::asio::connection> conn, |
| + std::string path, |
| + std::shared_ptr<NVMeMiIntf> nvmeIntf, |
| + nvme_mi_ctrl_t ctrl); |
| |
| + NVMeControllerEnabled(NVMeController&& nvmeController); |
| + |
| + ~NVMeControllerEnabled() override; |
| + |
| + void start(std::shared_ptr<NVMeControllerPlugin> nvmePlugin) override; |
| + |
| + private: |
| /* NVMeAdmin method overload */ |
| |
| /** @brief Implementation for GetLogPage |
| diff --git a/src/NVMeSubsys.cpp b/src/NVMeSubsys.cpp |
| index 90db473..aebfd14 100644 |
| --- a/src/NVMeSubsys.cpp |
| +++ b/src/NVMeSubsys.cpp |
| @@ -194,6 +194,12 @@ void NVMeSubsystem::start(const SensorData& configData) |
| << std::endl; |
| return; |
| } |
| + |
| + // Enable primary controller since they are required to work |
| + auto& primaryController = findPrimary->second.first; |
| + primaryController.reset(new NVMeControllerEnabled( |
| + std::move(*primaryController.get()))); |
| + |
| std::vector<std::shared_ptr<NVMeController>> secCntrls; |
| for (int i = 0; i < listHdr.num; i++) |
| { |
| @@ -206,9 +212,18 @@ void NVMeSubsystem::start(const SensorData& configData) |
| << std::endl; |
| break; |
| } |
| - secCntrls.push_back(findSecondary->second.first); |
| + |
| + auto& secondaryController = findSecondary->second.first; |
| + |
| + // Check Secondary Controller State |
| + if (listHdr.sc_entry[i].scs != 0) |
| + { |
| + secondaryController.reset(new NVMeControllerEnabled( |
| + std::move(*secondaryController.get()))); |
| + } |
| + secCntrls.push_back(secondaryController); |
| } |
| - findPrimary->second.first->setSecAssoc(secCntrls); |
| + primaryController->setSecAssoc(secCntrls); |
| |
| // start controller |
| for (auto& [_, pair] : self->controllers) |
| -- |
| 2.34.1 |
| |
