Nvmed: Add configuration to enable feature lockdown Added configuration code to enable command and feature lockdown if the feature is enabled for the SSD in entity manager This is to support E1.S SSD left shift development Tested: Tested by toggling the configuration and ensuring that the lockdown dbus interface is only exposed when intended Google-Bug-Id: 445994713 Change-Id: Ic0ee132b6df1cce8b4658ad7035420832d8403cb Signed-off-by: Agrim Bharat <agrimbharat@google.com>
diff --git a/src/NVMeController.cpp b/src/NVMeController.cpp index 537b748..61c4029 100644 --- a/src/NVMeController.cpp +++ b/src/NVMeController.cpp
@@ -142,35 +142,40 @@ protoSpecific, transferLength); }); - lockdownInterface = - objServer.add_interface(path, "xyz.openbmc_project.NVMe.Lockdown"); + if (this->featureLockdownEnabled) + { + lockdownInterface = + objServer.add_interface(path, "xyz.openbmc_project.NVMe.Lockdown"); - lockdownInterface->register_method( - "LockdownInband", - [selfWeak{weak_from_this()}](boost::asio::yield_context yield, - uint8_t prohibit, - const std::vector<uint8_t>& adminCmds, - const std::vector<uint8_t>& features, - const std::vector<uint8_t>& logPages) { - auto self = selfWeak.lock(); - if (!self) - { - checkLibNVMeError(std::make_error_code(std::errc::no_such_device), - -1, "LockdownInband"); - return std::tuple<uint32_t, uint32_t, uint32_t, std::string, - uint32_t>{0, 0, 0, "", 0}; - } + lockdownInterface->register_method( + "LockdownInband", + [selfWeak{weak_from_this()}](boost::asio::yield_context yield, + uint8_t prohibit, + const std::vector<uint8_t>& adminCmds, + const std::vector<uint8_t>& features, + const std::vector<uint8_t>& logPages) { + auto self = selfWeak.lock(); + if (!self) + { + checkLibNVMeError( + std::make_error_code(std::errc::no_such_device), -1, + "LockdownInband"); + return std::tuple<uint32_t, uint32_t, uint32_t, std::string, + uint32_t>{0, 0, 0, "", 0}; + } - if (self->status != Status::Enabled) - { - lg2::error("Controller has been disabled"); - throw sdbusplus::xyz::openbmc_project::Common::Error::Unavailable(); - } + if (self->status != Status::Enabled) + { + lg2::error("Controller has been disabled"); + throw sdbusplus::xyz::openbmc_project::Common::Error:: + Unavailable(); + } - return self->lockdownInbandMethod(std::move(yield), prohibit, adminCmds, - features, logPages); - }); - lockdownInterface->initialize(); + return self->lockdownInbandMethod(std::move(yield), prohibit, + adminCmds, features, logPages); + }); + lockdownInterface->initialize(); + } // StorageController interface is implemented manually to allow // async methods @@ -667,7 +672,10 @@ { objServer.remove_interface(securityInterface); objServer.remove_interface(passthruInterface); - objServer.remove_interface(lockdownInterface); + if (lockdownInterface) + { + objServer.remove_interface(lockdownInterface); + } SoftwareVersion::emit_removed(); SoftwareExtVersion::emit_removed(); NVMeAdmin::emit_removed(); @@ -679,10 +687,12 @@ boost::asio::io_context& io, sdbusplus::asio::object_server& objServer, std::shared_ptr<sdbusplus::asio::connection> conn, std::string path, const SensorData& configData, std::shared_ptr<NVMeMiIntf> nvmeIntf, - nvme_mi_ctrl_t ctrl, std::weak_ptr<NVMeSubsystem> subsys) : + nvme_mi_ctrl_t ctrl, std::weak_ptr<NVMeSubsystem> subsys, + bool enableFeatureLockdown) : isPrimary(true), io(io), objServer(objServer), conn(std::move(conn)), path(std::move(path)), config(configData), nvmeIntf(std::move(nvmeIntf)), - nvmeCtrl(ctrl), subsys(std::move(subsys)) + nvmeCtrl(ctrl), subsys(std::move(subsys)), + featureLockdownEnabled(enableFeatureLockdown) {} NVMeController::~NVMeController()
diff --git a/src/NVMeController.hpp b/src/NVMeController.hpp index db0a515..de838a0 100644 --- a/src/NVMeController.hpp +++ b/src/NVMeController.hpp
@@ -41,7 +41,8 @@ std::shared_ptr<sdbusplus::asio::connection> conn, std::string path, const SensorData& configData, std::shared_ptr<NVMeMiIntf> nvmeIntf, nvme_mi_ctrl_t ctrl, - std::weak_ptr<NVMeSubsystem> subsys); + std::weak_ptr<NVMeSubsystem> subsys, + bool enableFeatureLockdown); virtual ~NVMeController(); @@ -125,6 +126,8 @@ // NVMe Plug-in for vendor defined command/field std::weak_ptr<NVMeControllerPlugin> plugin; + bool featureLockdownEnabled = false; + private: void setSecAssoc( const std::vector<std::shared_ptr<NVMeController>>& secCntrls);
diff --git a/src/NVMeSensorMain.cpp b/src/NVMeSensorMain.cpp index 7189cdb..111dffb 100644 --- a/src/NVMeSensorMain.cpp +++ b/src/NVMeSensorMain.cpp
@@ -40,6 +40,7 @@ std::shared_ptr<MctpDevice> dev; NVMeIntf intf; std::shared_ptr<NVMeSubsystem> subsys; + bool enableFeatureLockdown = false; }; // a map with key value of {path, NVMeSubsystem} @@ -166,6 +167,38 @@ return std::get<std::string>(findMctpReactorConfigPath->second); } +static bool hasLockdownFeature(const std::string& path, + const SensorBaseConfigMap& properties) +{ + auto it = properties.find("SupportedFeatures"); + if (it == properties.end()) + { + return false; + } + + const auto& value = it->second; + + if (std::holds_alternative<std::vector<std::string>>(value)) + { + const auto& features = + std::get<std::vector<std::string>>(value); // Safe now + for (const auto& feature : features) + { + if (feature == "Lockdown") + { + return true; + } + } + } + else + { + lg2::warning( + "'{PATH}': 'SupportedFeatures' is not an array of strings.", "PATH", + path); + } + return false; +} + static void setupMctpDevice(const std::shared_ptr<MctpDevice>& dev, const std::weak_ptr<NVMeMiIntf>& weakIntf, @@ -284,6 +317,9 @@ std::optional<std::string> mctpReactorConfigPath = extractMctpReactorConfigPath(sensorConfig); + bool enableFeatureLockdown = hasLockdownFeature(nvmeObjectPath, + sensorConfig); + if (!sensorName) { continue; @@ -340,7 +376,7 @@ NVMeIntf nvmeBasic = NVMeIntf::create<NVMeBasic>(io, *busNumber, *address); - NVMeDevice dev{{}, nvmeBasic, {}}; + NVMeDevice dev{{}, nvmeBasic, {}, enableFeatureLockdown}; updatedDevices.emplace(nvmeObjectPath, std::move(dev)); } catch (std::exception& ex) @@ -411,7 +447,7 @@ nvmeMi.getInferface()); // Create a partial NVMeDevice entry in the temporary // updatedDevices map - NVMeDevice nvmeDev{mctpDev, nvmeMi, {}}; + NVMeDevice nvmeDev{mctpDev, nvmeMi, {}, enableFeatureLockdown}; updatedDevices.emplace(nvmeObjectPath, std::move(nvmeDev)); } catch (std::exception& ex) @@ -446,9 +482,11 @@ } try { + bool lockdownFeature = find->second.enableFeatureLockdown; + auto nvmeSubsys = NVMeSubsystem::create( io, objectServer, dbusConnection, interfacePath, *sensorName, - configData, find->second.intf); + configData, find->second.intf, lockdownFeature); // Complete the NVMeDevice entry with its subsystem and record it in // the persistent nvmeDeviceMap find->second.subsys = nvmeSubsys;
diff --git a/src/NVMeSubsys.cpp b/src/NVMeSubsys.cpp index 0ae9a2f..a507174 100644 --- a/src/NVMeSubsys.cpp +++ b/src/NVMeSubsys.cpp
@@ -113,10 +113,11 @@ boost::asio::io_context& io, sdbusplus::asio::object_server& objServer, const std::shared_ptr<sdbusplus::asio::connection>& conn, const std::string& path, const std::string& name, - const SensorData& configData, NVMeIntf intf) + const SensorData& configData, NVMeIntf intf, bool enableFeatureLockdown) { auto self = std::make_shared<NVMeSubsystem>(io, objServer, conn, path, name, - configData, std::move(intf)); + configData, std::move(intf), + enableFeatureLockdown); self->init(); return self; } @@ -125,11 +126,12 @@ boost::asio::io_context& io, sdbusplus::asio::object_server& objServer, const std::shared_ptr<sdbusplus::asio::connection>& conn, const std::string& path, const std::string& name, - const SensorData& configData, NVMeIntf intf) : + const SensorData& configData, NVMeIntf intf, bool enableFeatureLockdown) : NVMeStorage(objServer, *dynamic_cast<sdbusplus::bus_t*>(conn.get()), path.c_str()), path(path), io(io), objServer(objServer), conn(conn), name(name), - config(configData), nvmeIntf(std::move(intf)), status(Status::Stop) + config(configData), nvmeIntf(std::move(intf)), status(Status::Stop), + featureLockdownEnabled(enableFeatureLockdown) {} // Performs initialisation after shared_from_this() has been set up. @@ -442,7 +444,8 @@ { auto nvmeController = std::make_shared<NVMeController>( self->io, self->objServer, self->conn, path.string(), - self->config, nvme, c, self->weak_from_this()); + self->config, nvme, c, self->weak_from_this(), + self->featureLockdownEnabled); self->controllers.insert({*index, {nvmeController, {}}}); }
diff --git a/src/NVMeSubsys.hpp b/src/NVMeSubsys.hpp index b00a900..877a587 100644 --- a/src/NVMeSubsys.hpp +++ b/src/NVMeSubsys.hpp
@@ -28,7 +28,8 @@ sdbusplus::asio::object_server& objServer, const std::shared_ptr<sdbusplus::asio::connection>& conn, const std::string& path, const std::string& name, - const SensorData& configData, NVMeIntf intf); + const SensorData& configData, NVMeIntf intf, + bool enableFeatureLockdown); ~NVMeSubsystem() override; @@ -80,7 +81,8 @@ sdbusplus::asio::object_server& objServer, const std::shared_ptr<sdbusplus::asio::connection>& conn, const std::string& path, const std::string& name, - const SensorData& configData, NVMeIntf intf); + const SensorData& configData, NVMeIntf intf, + bool enableFeatureLockdown); void init(); @@ -168,6 +170,8 @@ */ std::map<uint16_t, std::set<uint32_t>> attached; + bool featureLockdownEnabled = false; + /* In-progress or completed create operations */
diff --git a/tests/test_nvme_mi.cpp b/tests/test_nvme_mi.cpp index c7b1a81..bf8d03e 100644 --- a/tests/test_nvme_mi.cpp +++ b/tests/test_nvme_mi.cpp
@@ -208,7 +208,7 @@ std::get<std::shared_ptr<NVMeMiIntf>>(nvme_intf.getInferface()))), subsys(std::make_shared<NVMeSubsystem>(io, object_server, systemBus, subsysPath, "NVMe_1", - SensorData{}, nvme_intf)) + SensorData{}, nvme_intf, false)) { subsys->unavailableMaxCount = 1; subsys->pollingInterval = subsysPollTime;