nvmed: Support NVMe Get/Set Feature for OCP Latency Monitor (FID 0xC5) Support NVMe Get and Set Feature commands for OCP Latency Monitor (FID 0xC5) across Controller scope in NVMeFeatureStore. This allows querying and configuring OCP latency monitor settings via 4096-byte payload buffer and exposing them over D-Bus. Extract getFeatureDataLen helper to support variable-length feature payloads and reject unsupported feature IDs with InvalidArgument. Tested: Verified via gbmc ci run nvmed (8/8 tests passed). Google-Bug-Id: 549207952 Change-Id: Ie4b2b0055b49f7daa9aefcab0713319abfefd91a Signed-off-by: Guangzong Chen <guangzong@google.com> TAG=agy CONV=c36db835-257f-47bf-84bb-b773d9cd7e86
diff --git a/proto/nvme_features.proto b/proto/nvme_features.proto index 84af23d..36898c7 100644 --- a/proto/nvme_features.proto +++ b/proto/nvme_features.proto
@@ -155,3 +155,28 @@ // Bit 31 - Discovery Log Page Change Notification EventState discovery_log_page_change_config = 11; } + +// C5h - OCP Latency Monitor +// This feature controls OCP Latency Monitoring configuration. +message OcpLatencyMonitor { + // Byte 1:0 - Active Bucket Timer Threshold (in milliseconds) + uint32 active_bucket_timer_threshold = 1; + // Byte 2 - Active Threshold A + uint32 active_threshold_a = 2; + // Byte 3 - Active Threshold B + uint32 active_threshold_b = 3; + // Byte 4 - Active Threshold C + uint32 active_threshold_c = 4; + // Byte 5 - Active Threshold D + uint32 active_threshold_d = 5; + // Byte 7:6 - Active Latency Config + uint32 active_latency_config = 6; + // Byte 8 - Active Latency Minimum Window + uint32 active_latency_minimum_window = 7; + // Byte 10:9 - Debug Log Trigger Enable + uint32 debug_log_trigger_enable = 8; + // Byte 11 - Discard Debug Log + uint32 discard_debug_log = 9; + // Byte 12 - Latency Monitor Feature Enable + uint32 latency_monitor_feature_enable = 10; +}
diff --git a/src/NVMeFeatureStore.cpp b/src/NVMeFeatureStore.cpp index 41ac1c4..cb41cd6 100644 --- a/src/NVMeFeatureStore.cpp +++ b/src/NVMeFeatureStore.cpp
@@ -9,6 +9,7 @@ #include <phosphor-logging/lg2.hpp> #include <algorithm> +#include <array> #include <cstring> #include <iomanip> #include <map> @@ -57,8 +58,32 @@ {"AsynchronousEventConfiguration", {0x0B, "google.gbmc.nvme.base.AsynchronousEventConfiguration", FeatureScope::Controller}}, + {"OcpLatencyMonitor", + {0xC5, "google.gbmc.nvme.base.OcpLatencyMonitor", + FeatureScope::Controller}}, }; +/** + * @brief OCP Latency Monitor Feature Buffer (FID 0xC5) as defined by OCP NVMe + * Spec. Matches platforms/storage/libnvme/nvme_types.h. + */ +struct __attribute__((packed)) OcpLatencyMonitorFeatureBuffer +{ + uint16_t active_bucket_timer_threshold; + uint8_t active_threshold_a; + uint8_t active_threshold_b; + uint8_t active_threshold_c; + uint8_t active_threshold_d; + uint16_t active_latency_config; + uint8_t active_latency_minimum_window; + uint16_t debug_log_trigger_enable; + uint8_t discard_debug_log; + uint8_t latency_monitor_feature_enable; + std::array<uint8_t, 4083> reserved; +}; +static_assert(sizeof(OcpLatencyMonitorFeatureBuffer) == 4096, + "Size of OcpLatencyMonitorFeatureBuffer must be 4096"); + template <typename T> static T parseProto(const std::vector<uint8_t>& data) { @@ -426,6 +451,31 @@ }); } +size_t FeatureStore::getFeatureDataLen(uint8_t fid) +{ + switch (fid) + { + case 0x01: // Arbitration + case 0x02: // PowerManagement + case 0x04: // TemperatureThreshold + case 0x05: // ErrorRecovery + case 0x06: // VolatileWriteCache + case 0x07: // NumberOfQueues + case 0x08: // InterruptCoalescing + case 0x09: // InterruptVectorConfiguration + case 0x0A: // WriteAtomicityNormal + case 0x0B: // AsynchronousEventConfiguration + return 0; + case 0xC5: // OcpLatencyMonitor + return 4096; + default: + lg2::error("Unsupported feature ID for data length: {FID}", "FID", + lg2::hex, fid); + throw sdbusplus::xyz::openbmc_project::Common::Error:: + InvalidArgument(); + } +} + std::vector<uint8_t> FeatureStore::getFeature(const boost::asio::yield_context& yield, const std::string& featureName) @@ -474,7 +524,7 @@ args.sel = sel; args.nsid = (scope == FeatureScope::Namespace) ? nsid : NVME_NSID_NONE; args.cdw11 = 0; - args.data_len = 0; + args.data_len = getFeatureDataLen(fid); auto [ex, cdw0, data] = asyncAdminGetFeatures(io, nvmeIntf, ctrl, args, yield); @@ -645,8 +695,52 @@ } break; } - default: + case 0xC5: + { + if (data.size() < sizeof(OcpLatencyMonitorFeatureBuffer)) + { + lg2::error( + "[{PATH}]OcpLatencyMonitor data buffer too small: {SIZE}", + "PATH", path, "SIZE", data.size()); + throw sdbusplus::xyz::openbmc_project::Common::Error:: + Unavailable(); + } + OcpLatencyMonitorFeatureBuffer buffer = {}; + std::memcpy(&buffer, data.data(), sizeof(buffer)); + + google::gbmc::nvme::base::OcpLatencyMonitor msg; + msg.set_active_bucket_timer_threshold( + le16toh(buffer.active_bucket_timer_threshold)); + msg.set_active_threshold_a(buffer.active_threshold_a); + msg.set_active_threshold_b(buffer.active_threshold_b); + msg.set_active_threshold_c(buffer.active_threshold_c); + msg.set_active_threshold_d(buffer.active_threshold_d); + msg.set_active_latency_config( + le16toh(buffer.active_latency_config)); + msg.set_active_latency_minimum_window( + buffer.active_latency_minimum_window); + msg.set_debug_log_trigger_enable( + le16toh(buffer.debug_log_trigger_enable)); + msg.set_discard_debug_log(buffer.discard_debug_log); + msg.set_latency_monitor_feature_enable( + buffer.latency_monitor_feature_enable); + + protoData.resize(msg.ByteSizeLong()); + if (!msg.SerializeToArray(protoData.data(), + static_cast<int>(protoData.size()))) + { + lg2::error("[{PATH}]Failed to serialize OcpLatencyMonitor", + "PATH", path); + throw sdbusplus::xyz::openbmc_project::Common::Error:: + InternalFailure(); + } break; + } + default: + lg2::error("[{PATH}]GetFeature: Unsupported FID {FID}", "PATH", + path, "FID", lg2::hex, fid); + throw sdbusplus::xyz::openbmc_project::Common::Error:: + InvalidArgument(); } return protoData; @@ -714,6 +808,7 @@ } uint32_t cdw11 = 0; + std::vector<uint8_t> payload; switch (fid) { @@ -846,8 +941,55 @@ cdw11 = msg.smart_critical_warning_config() & 0xFF; break; } - default: + case 0xC5: + { + auto msg = + parseProto<google::gbmc::nvme::base::OcpLatencyMonitor>(data); + if (msg.active_bucket_timer_threshold() > 0xFFFF || + msg.active_threshold_a() > 0xFF || + msg.active_threshold_b() > 0xFF || + msg.active_threshold_c() > 0xFF || + msg.active_threshold_d() > 0xFF || + msg.active_latency_config() > 0xFFFF || + msg.active_latency_minimum_window() > 0xFF || + msg.debug_log_trigger_enable() > 0xFFFF || + msg.discard_debug_log() > 0xFF || + msg.latency_monitor_feature_enable() > 0xFF) + { + throw sdbusplus::xyz::openbmc_project::Common::Error:: + InvalidArgument(); + } + OcpLatencyMonitorFeatureBuffer buffer = {}; + buffer.active_bucket_timer_threshold = htole16( + static_cast<uint16_t>(msg.active_bucket_timer_threshold())); + buffer.active_threshold_a = + static_cast<uint8_t>(msg.active_threshold_a()); + buffer.active_threshold_b = + static_cast<uint8_t>(msg.active_threshold_b()); + buffer.active_threshold_c = + static_cast<uint8_t>(msg.active_threshold_c()); + buffer.active_threshold_d = + static_cast<uint8_t>(msg.active_threshold_d()); + buffer.active_latency_config = + htole16(static_cast<uint16_t>(msg.active_latency_config())); + buffer.active_latency_minimum_window = + static_cast<uint8_t>(msg.active_latency_minimum_window()); + buffer.debug_log_trigger_enable = + htole16(static_cast<uint16_t>(msg.debug_log_trigger_enable())); + buffer.discard_debug_log = + static_cast<uint8_t>(msg.discard_debug_log()); + buffer.latency_monitor_feature_enable = + static_cast<uint8_t>(msg.latency_monitor_feature_enable()); + + payload.resize(sizeof(buffer), 0); + std::memcpy(payload.data(), &buffer, sizeof(buffer)); break; + } + default: + lg2::error("[{PATH}]SetFeature: Unsupported FID {FID}", "PATH", + path, "FID", lg2::hex, fid); + throw sdbusplus::xyz::openbmc_project::Common::Error:: + InvalidArgument(); } NVMeMiIntf::SetFeaturesRequest args{}; @@ -855,7 +997,7 @@ args.save = save; args.cdw11 = cdw11; args.nsid = (scope == FeatureScope::Namespace) ? nsid : NVME_NSID_NONE; - args.data = {}; + args.data = std::move(payload); auto [ex, resp] = asyncAdminSetFeatures(io, nvmeIntf, ctrl, args, yield);
diff --git a/src/NVMeFeatureStore.hpp b/src/NVMeFeatureStore.hpp index 90c6a3c..d42eb84 100644 --- a/src/NVMeFeatureStore.hpp +++ b/src/NVMeFeatureStore.hpp
@@ -41,6 +41,8 @@ // Initialize the feature collection by querying the device void init(std::function<void(const std::error_code&)>&& cb); + static size_t getFeatureDataLen(uint8_t fid); + private: boost::asio::io_context& io; sdbusplus::asio::object_server& objServer;
diff --git a/tests/test_nvme_feature_store.cpp b/tests/test_nvme_feature_store.cpp index 2346abc..b4f8112 100644 --- a/tests/test_nvme_feature_store.cpp +++ b/tests/test_nvme_feature_store.cpp
@@ -777,7 +777,6 @@ EXPECT_FALSE(FeatureStoreTest::callIsFeatureSupported( featureStore, "TemperatureThreshold_Current")); } - TEST_F(FeatureStoreTest, DiscoverFeaturesNoONCSBit4Supported) { boost::asio::io_context io; @@ -941,6 +940,281 @@ featureStore, "Arbitration_Current")); } +TEST_F(FeatureStoreTest, GetFeatureOcpLatencyMonitor) +{ + boost::asio::io_context io; + auto conn = std::make_shared<sdbusplus::asio::connection>(io); + sdbusplus::asio::object_server objServer(conn); + std::string objectPath = "/xyz/openbmc_project/nvme/ctrl0"; + + auto mockNvme = std::make_shared<NVMeMiMock>(); + nvme_mi_ctrl_t ctrl = nullptr; + + auto featureStore = std::make_shared<FeatureStore>( + io, objServer, conn, objectPath, mockNvme, ctrl, 0, + FeatureScope::Controller); + + FeatureStoreTest::addSupportedFeature( + featureStore, "OcpLatencyMonitor_Current", "0xc5", + "google.gbmc.nvme.base.OcpLatencyMonitor"); + + std::vector<uint8_t> mockData(4096, 0); + // Byte 0-1: 1000 ms (0x03E8) + mockData[0] = 0xE8; + mockData[1] = 0x03; + mockData[2] = 1; // Threshold A + mockData[3] = 2; // Threshold B + mockData[4] = 3; // Threshold C + mockData[5] = 4; // Threshold D + // Byte 6-7: 5 (0x0005) + mockData[6] = 5; + mockData[7] = 0; + mockData[8] = 6; // Minimum Window + // Byte 9-10: 7 (0x0007) + mockData[9] = 7; + mockData[10] = 0; + mockData[11] = 1; // Discard Debug Log + mockData[12] = 1; // Feature Enable + + EXPECT_CALL(*mockNvme, + adminGetFeatures(::testing::_, ::testing::_, ::testing::_)) + .WillOnce([&mockData](nvme_mi_ctrl_t, + const NVMeMiIntf::GetFeaturesRequest& req, + std::function<void(nvme_ex_ptr, uint32_t, + std::span<uint8_t>)>&& cb) { + EXPECT_EQ(req.fid, 0xC5); // OcpLatencyMonitor + EXPECT_EQ(req.data_len, 4096); + cb(nullptr, 0, mockData); + }); + + std::vector<uint8_t> result; + bool done = false; + boost::asio::spawn(io, [&](const boost::asio::yield_context& yield) { + result = FeatureStoreTest::callGetFeature(featureStore, yield, + "OcpLatencyMonitor_Current"); + done = true; + }); + while (!done) + { + io.run_one(); + } + EXPECT_TRUE(done); + + google::gbmc::nvme::base::OcpLatencyMonitor msg; + ASSERT_TRUE( + msg.ParseFromArray(result.data(), static_cast<int>(result.size()))); + EXPECT_EQ(msg.active_bucket_timer_threshold(), 1000); + EXPECT_EQ(msg.active_threshold_a(), 1); + EXPECT_EQ(msg.active_threshold_b(), 2); + EXPECT_EQ(msg.active_threshold_c(), 3); + EXPECT_EQ(msg.active_threshold_d(), 4); + EXPECT_EQ(msg.active_latency_config(), 5); + EXPECT_EQ(msg.active_latency_minimum_window(), 6); + EXPECT_EQ(msg.debug_log_trigger_enable(), 7); + EXPECT_EQ(msg.discard_debug_log(), 1); + EXPECT_EQ(msg.latency_monitor_feature_enable(), 1); +} + +TEST_F(FeatureStoreTest, GetFeatureOcpLatencyMonitorBufferTooSmall) +{ + boost::asio::io_context io; + auto conn = std::make_shared<sdbusplus::asio::connection>(io); + sdbusplus::asio::object_server objServer(conn); + std::string objectPath = "/xyz/openbmc_project/nvme/ctrl0"; + + auto mockNvme = std::make_shared<NVMeMiMock>(); + nvme_mi_ctrl_t ctrl = nullptr; + + auto featureStore = std::make_shared<FeatureStore>( + io, objServer, conn, objectPath, mockNvme, ctrl, 0, + FeatureScope::Controller); + + FeatureStoreTest::addSupportedFeature( + featureStore, "OcpLatencyMonitor_Current", "0xc5", + "google.gbmc.nvme.base.OcpLatencyMonitor"); + + std::vector<uint8_t> mockData(10, 0); // Less than 13 bytes + + EXPECT_CALL(*mockNvme, + adminGetFeatures(::testing::_, ::testing::_, ::testing::_)) + .WillOnce([&mockData](nvme_mi_ctrl_t, + const NVMeMiIntf::GetFeaturesRequest& req, + std::function<void(nvme_ex_ptr, uint32_t, + std::span<uint8_t>)>&& cb) { + EXPECT_EQ(req.fid, 0xC5); // OcpLatencyMonitor + EXPECT_EQ(req.data_len, 4096); + cb(nullptr, 0, mockData); + }); + + bool threw = false; + bool done = false; + boost::asio::spawn(io, [&](const boost::asio::yield_context& yield) { + try + { + FeatureStoreTest::callGetFeature(featureStore, yield, + "OcpLatencyMonitor_Current"); + } + catch ( + const sdbusplus::xyz::openbmc_project::Common::Error::Unavailable&) + { + threw = true; + } + done = true; + }); + while (!done) + { + io.run_one(); + } + EXPECT_TRUE(done); + EXPECT_TRUE(threw); +} + +TEST_F(FeatureStoreTest, SetFeatureOcpLatencyMonitor) +{ + boost::asio::io_context io; + auto conn = std::make_shared<sdbusplus::asio::connection>(io); + sdbusplus::asio::object_server objServer(conn); + std::string objectPath = "/xyz/openbmc_project/nvme/ctrl0"; + + auto mockNvme = std::make_shared<NVMeMiMock>(); + nvme_mi_ctrl_t ctrl = nullptr; + + auto featureStore = std::make_shared<FeatureStore>( + io, objServer, conn, objectPath, mockNvme, ctrl, 0, + FeatureScope::Controller); + + FeatureStoreTest::addSupportedFeature( + featureStore, "OcpLatencyMonitor_Current", "0xc5", + "google.gbmc.nvme.base.OcpLatencyMonitor"); + + google::gbmc::nvme::base::OcpLatencyMonitor msg; + msg.set_active_bucket_timer_threshold(1000); + msg.set_active_threshold_a(1); + msg.set_active_threshold_b(2); + msg.set_active_threshold_c(3); + msg.set_active_threshold_d(4); + msg.set_active_latency_config(5); + msg.set_active_latency_minimum_window(6); + msg.set_debug_log_trigger_enable(7); + msg.set_discard_debug_log(1); + msg.set_latency_monitor_feature_enable(1); + + std::vector<uint8_t> data(msg.ByteSizeLong()); + ASSERT_TRUE( + msg.SerializeToArray(data.data(), static_cast<int>(data.size()))); + + EXPECT_CALL(*mockNvme, + adminSetFeatures(::testing::_, ::testing::_, ::testing::_)) + .WillOnce([](nvme_mi_ctrl_t, const NVMeMiIntf::SetFeaturesRequest& req, + std::function<void(nvme_ex_ptr, uint32_t)>&& cb) { + EXPECT_EQ(req.fid, 0xC5); // OcpLatencyMonitor + EXPECT_EQ(req.data.size(), 4096); + EXPECT_EQ(req.data[0], 0xE8); + EXPECT_EQ(req.data[1], 0x03); + EXPECT_EQ(req.data[2], 1); + EXPECT_EQ(req.data[3], 2); + EXPECT_EQ(req.data[4], 3); + EXPECT_EQ(req.data[5], 4); + EXPECT_EQ(req.data[6], 5); + EXPECT_EQ(req.data[7], 0); + EXPECT_EQ(req.data[8], 6); + EXPECT_EQ(req.data[9], 7); + EXPECT_EQ(req.data[10], 0); + EXPECT_EQ(req.data[11], 1); + EXPECT_EQ(req.data[12], 1); + for (size_t i = 13; i < 4096; ++i) + { + EXPECT_EQ(req.data[i], 0); + } + cb(nullptr, 0); + }); + + bool done = false; + boost::asio::spawn(io, [&](const boost::asio::yield_context& yield) { + FeatureStoreTest::callSetFeature(featureStore, yield, + "OcpLatencyMonitor_Current", data); + done = true; + }); + while (!done) + { + io.run_one(); + } + EXPECT_TRUE(done); +} + +TEST_F(FeatureStoreTest, SetFeatureOcpLatencyMonitorInvalidArgumentBounds) +{ + boost::asio::io_context io; + auto conn = std::make_shared<sdbusplus::asio::connection>(io); + sdbusplus::asio::object_server objServer(conn); + std::string objectPath = "/xyz/openbmc_project/nvme/ctrl0"; + + auto mockNvme = std::make_shared<NVMeMiMock>(); + nvme_mi_ctrl_t ctrl = nullptr; + + auto featureStore = std::make_shared<FeatureStore>( + io, objServer, conn, objectPath, mockNvme, ctrl, 0, + FeatureScope::Controller); + + FeatureStoreTest::addSupportedFeature( + featureStore, "OcpLatencyMonitor_Current", "0xc5", + "google.gbmc.nvme.base.OcpLatencyMonitor"); + + google::gbmc::nvme::base::OcpLatencyMonitor msg; + // Set field out of uint16 bounds + msg.set_active_bucket_timer_threshold(0x10000); + + std::vector<uint8_t> data(msg.ByteSizeLong()); + ASSERT_TRUE( + msg.SerializeToArray(data.data(), static_cast<int>(data.size()))); + + bool threw = false; + bool done = false; + boost::asio::spawn(io, [&](const boost::asio::yield_context& yield) { + try + { + FeatureStoreTest::callSetFeature(featureStore, yield, + "OcpLatencyMonitor_Current", data); + } + catch (const sdbusplus::xyz::openbmc_project::Common::Error:: + InvalidArgument&) + { + threw = true; + } + done = true; + }); + while (!done) + { + io.run_one(); + } + EXPECT_TRUE(threw); +} + +TEST_F(FeatureStoreTest, GetFeatureDataLen) +{ + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x01), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x02), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x04), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x05), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x06), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x07), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x08), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x09), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x0A), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0x0B), 0); + EXPECT_EQ(FeatureStore::getFeatureDataLen(0xC5), 4096); + + EXPECT_THROW( + FeatureStore::getFeatureDataLen(0x00), + sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument); + EXPECT_THROW( + FeatureStore::getFeatureDataLen(0x03), + sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument); + EXPECT_THROW( + FeatureStore::getFeatureDataLen(0xFF), + sdbusplus::xyz::openbmc_project::Common::Error::InvalidArgument); +} + int main(int argc, char** argv) { ::testing::InitGoogleTest(&argc, argv);