nvmesensor: dynamic MCTP matching via BusInfo Removed the requirement for static MctpReactorConfigPath in NVME1000 Entity Manager configurations. Instead, implemented a dynamic matching mechanism based on physical topology (BusInfo). Added a background listener in MctpUtil that monitors associations between MCTP endpoints and Entity Manager configurations. When an association appears, it queries the configuration properties on demand and caches the full SensorData. Updated handleConfigurations in NVMeSensorMain to perform the matching by iterating over the cached configurations and comparing BusInfo with the sensor's BusInfo on the fly. This pushes the matching logic to the consumer and makes it future-proof against schema changes. Added cache priming on startup to handle pre-existing associations and configurations, ensuring listeners are registered before queries to avoid race conditions. Added a new constructor to MctpReactorDevice to accept a pre-resolved endpoint path, bypassing the Object Mapper lookup in setup(). This change decouples nvmesensor from static paths and specific setup agents, enabling support for dynamic buses like USB. Change-Id: I827a25b7e7d6cba2f7cf1e60c351b39f03b4d4dc Google-Bug-Id: 490106522 Signed-off-by: Hao Jiang <jianghao@google.com>
diff --git a/src/MctpReactorDevice.cpp b/src/MctpReactorDevice.cpp index 7157894..3b979b8 100644 --- a/src/MctpReactorDevice.cpp +++ b/src/MctpReactorDevice.cpp
@@ -120,10 +120,27 @@ i2cAddress(isI2cAccessible ? address.value() : -1) {} +MctpReactorDevice::MctpReactorDevice( + const std::shared_ptr<sdbusplus::asio::connection>& connection, + const std::string& deviceObjectPath, const std::string& endpointPath, + std::optional<int> busNumber, std::optional<int> address) : + connection(connection), deviceObjectPath(deviceObjectPath), + resolvedEndpointPath(endpointPath), + prettyObjectName(extractSuffixFromObjectPath(deviceObjectPath)), + isI2cAccessible(busNumber.has_value() && address.has_value()), + i2cBusNumber(isI2cAccessible ? busNumber.value() : -1), + i2cAddress(isI2cAccessible ? address.value() : -1) +{} + void MctpReactorDevice::setup( std::function<void(const std::error_code& ec, const std::shared_ptr<MctpEndpoint>& ep)>&& action) { + if (!resolvedEndpointPath.empty()) + { + finaliseEndpoint(resolvedEndpointPath, std::move(action)); + return; + } auto onGetPropertyReturned = [weak{weak_from_this()}, action{std::move(action)}]( const boost::system::error_code& ec,
diff --git a/src/MctpReactorDevice.hpp b/src/MctpReactorDevice.hpp index f506e98..321fcdb 100644 --- a/src/MctpReactorDevice.hpp +++ b/src/MctpReactorDevice.hpp
@@ -25,6 +25,10 @@ const std::shared_ptr<sdbusplus::asio::connection>& connection, const std::string& deviceObjectPath, std::optional<int> busNumber, std::optional<int> address); + MctpReactorDevice( + const std::shared_ptr<sdbusplus::asio::connection>& connection, + const std::string& deviceObjectPath, const std::string& endpointPath, + std::optional<int> busNumber, std::optional<int> address); MctpReactorDevice(const MctpDevice& other) = delete; MctpReactorDevice(MctpDevice&& other) = delete; ~MctpReactorDevice() override = default; @@ -49,6 +53,7 @@ std::shared_ptr<sdbusplus::asio::connection> connection; const std::string deviceObjectPath; + const std::string resolvedEndpointPath; const std::string prettyObjectName; // Suffix of deviceObjectPath. const bool isI2cAccessible; const int i2cBusNumber; // Invalid if isI2cAccessible is false.
diff --git a/src/MctpUtil.cpp b/src/MctpUtil.cpp new file mode 100644 index 0000000..106cb10 --- /dev/null +++ b/src/MctpUtil.cpp
@@ -0,0 +1,287 @@ +#include "MctpUtil.hpp" + +#include "Utils.hpp" +#include "VariantVisitors.hpp" + +#include <boost/asio/steady_timer.hpp> +#include <boost/container/flat_map.hpp> +#include <phosphor-logging/lg2.hpp> +#include <sdbusplus/bus/match.hpp> + +#include <chrono> +#include <filesystem> +#include <iostream> + +std::map<std::string, SensorData> mctpEndpointConfigMap; + +enum class State : uint8_t +{ + None, + Add, + Remove +}; + +struct EndpointState +{ + State state = State::None; + bool querying = false; +}; + +static std::map<std::string, EndpointState> endpointStates; + +static std::unique_ptr<sdbusplus::bus::match_t> associationMatch = nullptr; +static std::unique_ptr<sdbusplus::bus::match_t> associationRemoveMatch = + nullptr; + +static void performEmConfigQuery( + const std::shared_ptr<sdbusplus::asio::connection>& conn, + const std::string& endpointPath, const std::string& emConfigPath) +{ + std::filesystem::path p(emConfigPath); + std::string parentPath = p.parent_path().string(); + + conn->async_method_call( + [conn, endpointPath, emConfigPath](const boost::system::error_code& ec, + const ManagedObjectType& objects) { + auto& state = endpointStates[endpointPath]; + state.querying = false; + + State lastState = state.state; + state.state = State::None; // Reset for next query + + if (lastState == State::Remove) + { + lg2::info("Ignoring query reply for removed endpoint {ENDPOINT}", + "ENDPOINT", endpointPath); + auto it = mctpEndpointConfigMap.find(endpointPath); + if (it != mctpEndpointConfigMap.end()) + { + mctpEndpointConfigMap.erase(it); + } + return; + } + + if (ec) + { + lg2::error("Failed to get managed objects for {PATH}: {ERROR}", + "PATH", emConfigPath, "ERROR", ec.message()); + return; + } + + auto objIt = + objects.find(sdbusplus::message::object_path(emConfigPath)); + if (objIt == objects.end()) + { + lg2::warning("EM config path {PATH} not found in managed objects", + "PATH", emConfigPath); + return; + } + + if (!objIt->second.empty()) + { + lg2::info( + "Recorded MCTP endpoint {ENDPOINT} with config from {PATH}", + "ENDPOINT", endpointPath, "PATH", emConfigPath); + mctpEndpointConfigMap[endpointPath] = objIt->second; + } + + if (lastState == State::Add) + { + state.querying = true; + performEmConfigQuery(conn, endpointPath, emConfigPath); + } + }, + "xyz.openbmc_project.EntityManager", parentPath, + "org.freedesktop.DBus.ObjectManager", "GetManagedObjects"); +} + +void setupMctpEndpointListener( + const std::shared_ptr<sdbusplus::asio::connection>& conn) +{ + // Match 1: Listen for Associations from mctp-reactor + const std::string associationMatchSpec = + "type='signal',interface='org.freedesktop.DBus.ObjectManager',member='InterfacesAdded',path_namespace='/au/com/codeconstruct/mctp1'"; + + associationMatch = std::make_unique<sdbusplus::bus::match_t>( + static_cast<sdbusplus::bus_t&>(*conn), associationMatchSpec, + [conn](sdbusplus::message_t& msg) { + sdbusplus::message::object_path path; + boost::container::flat_map< + std::string, + boost::container::flat_map< + std::string, + std::variant<std::string, std::vector<std::string>>>> + interfaces; + msg.read(path, interfaces); + + auto it = interfaces.find("xyz.openbmc_project.Association"); + if (it == interfaces.end()) + { + return; + } + + auto propIt = it->second.find("endpoints"); + if (propIt == it->second.end()) + { + return; + } + + const auto* endpoints = + std::get_if<std::vector<std::string>>(&propIt->second); + if (!endpoints || endpoints->empty()) + { + return; + } + + std::string emConfigPath = endpoints->front(); + std::string endpointPath = path.str; + constexpr std::string_view suffix = "/configured_by"; + if (endpointPath.ends_with("/configured_by")) + { + endpointPath = + endpointPath.substr(0, endpointPath.size() - suffix.length()); + } + + auto& state = endpointStates[endpointPath]; + + if (state.querying) + { + state.state = State::Add; + return; + } + + state.querying = true; + performEmConfigQuery(conn, endpointPath, emConfigPath); + }); + + // Match 2: Listen for Associations (Removed) + const std::string associationRemoveMatchSpec = + "type='signal',interface='org.freedesktop.DBus.ObjectManager',member='InterfacesRemoved',path_namespace='/au/com/codeconstruct/mctp1'"; + + associationRemoveMatch = std::make_unique<sdbusplus::bus::match_t>( + static_cast<sdbusplus::bus_t&>(*conn), associationRemoveMatchSpec, + [](sdbusplus::message_t& msg) { + sdbusplus::message::object_path path; + std::vector<std::string> interfaces; + msg.read(path, interfaces); + + std::string endpointPath = path.str; + constexpr std::string_view suffix = "/configured_by"; + if (endpointPath.ends_with("/configured_by")) + { + endpointPath = + endpointPath.substr(0, endpointPath.size() - suffix.length()); + } + + auto& state = endpointStates[endpointPath]; + + if (state.querying) + { + state.state = State::Remove; + } + else + { + auto it = mctpEndpointConfigMap.find(endpointPath); + if (it != mctpEndpointConfigMap.end()) + { + lg2::info("Removing MCTP endpoint {ENDPOINT} from map", + "ENDPOINT", endpointPath); + mctpEndpointConfigMap.erase(it); + } + } + }); + + // Prime the cache: Query existing Associations + constexpr auto associationInterfaces = + std::to_array({"xyz.openbmc_project.Association"}); + conn->async_method_call( + [conn](const boost::system::error_code& ec, + const GetSubTreeType& subtree) { + if (ec) + { + lg2::error("Failed to get associations from Object Mapper: {ERROR}", + "ERROR", ec.message()); + return; + } + + for (const auto& [path, services] : subtree) + { + if (!path.ends_with("/configured_by")) + { + continue; + } + + conn->async_method_call( + [conn, + path](const boost::system::error_code& ec, + const std::variant<std::vector<std::string>>& value) { + if (ec) + { + lg2::error("Failed to get endpoints for {PATH}: {ERROR}", + "PATH", path, "ERROR", ec.message()); + return; + } + + const auto* endpoints = + std::get_if<std::vector<std::string>>(&value); + if (!endpoints || endpoints->empty()) + { + return; + } + + std::string emConfigPath = endpoints->front(); + std::string endpointPath = path; + constexpr std::string_view suffix = "/configured_by"; + if (endpointPath.ends_with("/configured_by")) + { + endpointPath = endpointPath.substr(0, endpointPath.size() - + suffix.length()); + } + + // Now query the EM config path for its properties + std::filesystem::path p(emConfigPath); + std::string parentPath = p.parent_path().string(); + + conn->async_method_call( + [endpointPath, + emConfigPath](const boost::system::error_code& ec, + const ManagedObjectType& objects) { + if (ec) + { + lg2::error( + "Failed to get managed objects for {PATH}: {ERROR}", + "PATH", emConfigPath, "ERROR", ec.message()); + return; + } + + auto objIt = objects.find( + sdbusplus::message::object_path(emConfigPath)); + if (objIt == objects.end()) + { + lg2::warning( + "EM config path {PATH} not found in managed objects", + "PATH", emConfigPath); + return; + } + + if (!objIt->second.empty()) + { + lg2::info( + "Primed match for endpoint {ENDPOINT} with config from {PATH}", + "ENDPOINT", endpointPath, "PATH", emConfigPath); + mctpEndpointConfigMap[endpointPath] = objIt->second; + } + }, + "xyz.openbmc_project.EntityManager", parentPath, + "org.freedesktop.DBus.ObjectManager", "GetManagedObjects"); + }, + "xyz.openbmc_project.ObjectMapper", path, + "org.freedesktop.DBus.Properties", "Get", + "xyz.openbmc_project.Association", "endpoints"); + } + }, + "xyz.openbmc_project.ObjectMapper", + "/xyz/openbmc_project/object_mapper", + "xyz.openbmc_project.ObjectMapper", "GetSubTree", + "/au/com/codeconstruct/mctp1", 0, associationInterfaces); +}
diff --git a/src/MctpUtil.hpp b/src/MctpUtil.hpp new file mode 100644 index 0000000..580f7a0 --- /dev/null +++ b/src/MctpUtil.hpp
@@ -0,0 +1,20 @@ +#pragma once + +#include <sdbusplus/asio/connection.hpp> + +#include <map> +#include <memory> +#include <string> +#include <variant> +#include <vector> + +using BusInfo = std::map<std::string, std::string>; + +// Global maps (declared extern) + +#include "Utils.hpp" + +extern std::map<std::string, SensorData> mctpEndpointConfigMap; + +void setupMctpEndpointListener( + const std::shared_ptr<sdbusplus::asio::connection>& conn);
diff --git a/src/NVMeSensorMain.cpp b/src/NVMeSensorMain.cpp index 23f3b7a..c89eafc 100644 --- a/src/NVMeSensorMain.cpp +++ b/src/NVMeSensorMain.cpp
@@ -14,8 +14,11 @@ // limitations under the License. */ +#include "absl/strings/match.h" + #include "MctpEndpoint.hpp" #include "MctpReactorDevice.hpp" +#include "MctpUtil.hpp" #include "NVMeBasic.hpp" #include "NVMeDevice.hpp" #include "NVMeIntf.hpp" @@ -312,7 +315,7 @@ "PATH", nvmeObjectPath.str, "ERROR", ex.what()); } } - else if (*nvmeProtocol == "mi_i2c") + else if (*nvmeProtocol == "mi_i2c" || *nvmeProtocol == "mi_mctp") { // defualt i2c nvme-mi port is 0x1d if (!address) @@ -349,11 +352,98 @@ try { - // Note that in this codepath, despite using "mi_i2c" protocol, - // we can be talking to NVMe devices not accessible through I2C - // directly. std::shared_ptr<MctpDevice> mctpDev; - if (!isMctpReactorDevice) + if (*nvmeProtocol == "mi_mctp") + { + BusInfo busInfo; + auto extractProp = [](const auto& properties, BusInfo& info, + const std::string& key) { + auto it = properties.find(key); + if (it != properties.end()) + { + info[key] = std::visit(VariantToStringVisitor(), + it->second); + } + }; + + for (const auto& [intf, props] : configData) + { + if (absl::StrContains( + intf, + "xyz.openbmc_project.Configuration.BusInfo")) + { + std::string busType; + auto it = props.find("BusType"); + if (it != props.end()) + { + busType = std::visit(VariantToStringVisitor(), + it->second); + } + + if (busType == "USB") + { + extractProp(props, busInfo, "RootHubPath"); + extractProp(props, busInfo, "Port"); + extractProp(props, busInfo, "Configuration"); + extractProp(props, busInfo, "InterfaceNum"); + + busInfo["BusType"] = busType; + } + else if (!busType.empty()) + { + lg2::warning( + "Unsupported BusType {TYPE} for {PATH}", + "TYPE", busType, "PATH", + nvmeObjectPath.str); + } + } + } + + if (busInfo.empty()) + { + lg2::error("Missing BusInfo for mi_mctp device {PATH}", + "PATH", nvmeObjectPath.str); + continue; + } + + std::string endpointPath; + for (const auto& [epPath, epConfig] : mctpEndpointConfigMap) + { + BusInfo epBusInfo; + for (const auto& [intf, props] : epConfig) + { + extractProp(props, epBusInfo, "RootHubPath"); + extractProp(props, epBusInfo, "Port"); + extractProp(props, epBusInfo, "Configuration"); + extractProp(props, epBusInfo, "InterfaceNum"); + + if (epBusInfo.count("RootHubPath") > 0 && + epBusInfo.count("BusType") == 0) + { + epBusInfo["BusType"] = "USB"; + } + } + + if (epBusInfo == busInfo) + { + endpointPath = epPath; + break; + } + } + + if (endpointPath.empty()) + { + lg2::warning( + "No matching MCTP endpoint found for BusInfo on {PATH}", + "PATH", nvmeObjectPath.str); + continue; + } + + mctpDev = std::make_shared<MctpReactorDevice>( + dbusConnection, nvmeObjectPath.str, endpointPath, + busNumber, address); + } + else if (!isMctpReactorDevice) { mctpDev = std::make_shared<SmbusMctpdDevice>( dbusConnection, *busNumber, *address); @@ -546,6 +636,8 @@ objectServer.add_manager("/xyz/openbmc_project/sensors"); objectServer.add_manager("/xyz/openbmc_project/inventory"); + setupMctpEndpointListener(systemBus); + boost::asio::post( io, [&]() { createNVMeSubsystems(io, objectServer, systemBus); });
diff --git a/src/meson.build b/src/meson.build index 2363a2c..886ac8d 100644 --- a/src/meson.build +++ b/src/meson.build
@@ -30,6 +30,7 @@ 'utils_a', [ 'FileHandle.cpp', + 'MctpUtil.cpp', 'SensorPaths.cpp', 'Utils.cpp', ], @@ -282,6 +283,7 @@ install_headers( 'Utils.hpp', + 'MctpUtil.hpp', 'NVMePlugin.hpp', 'NVMeUtil.hpp', 'VariantVisitors.hpp',