From e6b89b287d24e2aff7759ab92d2c117f11eb5539 Mon Sep 17 00:00:00 2001 From: manikandan Date: Mon, 23 Feb 2026 16:38:46 -0800 Subject: [PATCH] [fboss/platform_manager] Implement AMD CPU_BUS resolution via ACPI firmware_node/path Summary: Implement resolveAmdCpuBusNums() for AMD FireRange CPUs. DesignWare I2C adapters share identical names, so bus identification uses ACPI firmware_node/path under /sys/devices/platform/AMDI0010:*. Maps \_SB_.I2CA to CPU_BUS@0 and \_SB_.I2CB to CPU_BUS@1. Also updates ConfigValidator to allow CPU_BUS@1 (needed for AMD two-bus configs) and adds duplicate name detection. Test Plan: config_validator_test i2c_explorer_test tested on a unit --- .../platform_manager/ConfigValidator.cpp | 7 ++ .../platform/platform_manager/I2cExplorer.cpp | 78 ++++++++++++++++++- fboss/platform/platform_manager/I2cExplorer.h | 5 +- .../platform_manager_config.thrift | 10 ++- .../tests/ConfigValidatorTest.cpp | 10 ++- 5 files changed, 99 insertions(+), 11 deletions(-) diff --git a/fboss/platform/platform_manager/ConfigValidator.cpp b/fboss/platform/platform_manager/ConfigValidator.cpp index 493d8c30911fa..8e878aa0c93eb 100644 --- a/fboss/platform/platform_manager/ConfigValidator.cpp +++ b/fboss/platform/platform_manager/ConfigValidator.cpp @@ -2,6 +2,8 @@ #include "fboss/platform/platform_manager/ConfigValidator.h" +#include + #include #include #include @@ -311,6 +313,7 @@ bool ConfigValidator::isValidI2cAdaptersFromCpu( static const re2::RE2 kCpuBusNameRegex{"CPU_BUS@\\d+"}; bool hasVirtual = false; bool hasExact = false; + std::set seen; for (const auto& name : i2cAdaptersFromCpu) { if (re2::RE2::FullMatch(name, kCpuBusNameRegex)) { if (name != "CPU_BUS@0" && name != "CPU_BUS@1") { @@ -320,6 +323,10 @@ bool ConfigValidator::isValidI2cAdaptersFromCpu( name); return false; } + if (!seen.insert(name).second) { + XLOG(ERR) << fmt::format("Duplicate virtual bus name '{}'", name); + return false; + } hasVirtual = true; } else { hasExact = true; diff --git a/fboss/platform/platform_manager/I2cExplorer.cpp b/fboss/platform/platform_manager/I2cExplorer.cpp index ae9ec155e3dbd..bd6804da242da 100644 --- a/fboss/platform/platform_manager/I2cExplorer.cpp +++ b/fboss/platform/platform_manager/I2cExplorer.cpp @@ -18,9 +18,16 @@ namespace { const re2::RE2 kI2cMuxChannelRegex{"channel-\\d+"}; const re2::RE2 kCpuBusPattern{"CPU_BUS@(\\d+)"}; +const re2::RE2 kAmdI2cPlatformDevRegex{"AMDI0010:\\d+"}; constexpr auto kI2cDevCreationWaitSecs = std::chrono::seconds(5); constexpr auto kCpuI2cBusNumsWaitSecs = 1; +// AMD FireRange DesignWare I2C: ACPI firmware_node/path → CPU_BUS index. +const std::map kAmdAcpiPathToBusIndex = { + {R"(\_SB_.I2CA)", 1}, + {R"(\_SB_.I2CB)", 0}, +}; + std::string getI2cAdapterName(const fs::path& busPath) { auto nameFile = busPath / "name"; if (!fs::exists(nameFile)) { @@ -95,9 +102,76 @@ std::map I2cExplorer::getBusNums( } std::map I2cExplorer::resolveAmdCpuBusNums( - const std::vector& /* i2cAdaptersFromCpu */) { + const std::vector& i2cAdaptersFromCpu) { + // AMD FireRange DesignWare I2C buses share identical adapter names, so + // we identify them by their ACPI firmware_node/path under + // /sys/devices/platform/AMDI0010:*. + // \_SB_.I2CA → CPU_BUS@1 + // \_SB_.I2CB → CPU_BUS@0 + const auto platformRoot = fs::path("/sys/devices/platform"); + constexpr int maxRetries = 10; + + std::map busNums; + + for (int attempt = 1; attempt <= maxRetries; attempt++) { + XLOG(INFO) << "Resolving AMD CPU_BUS names -- Attempt #" << attempt; + busNums.clear(); + + for (const auto& dirEntry : fs::directory_iterator(platformRoot)) { + if (!re2::RE2::FullMatch( + dirEntry.path().filename().string(), kAmdI2cPlatformDevRegex)) { + continue; + } + auto fwPathFile = dirEntry.path() / "firmware_node" / "path"; + if (!fs::exists(fwPathFile)) { + continue; + } + std::string acpiPath; + if (!folly::readFile(fwPathFile.string().c_str(), acpiPath)) { + continue; + } + acpiPath = folly::trimWhitespace(acpiPath).str(); + + auto it = kAmdAcpiPathToBusIndex.find(acpiPath); + if (it == kAmdAcpiPathToBusIndex.end()) { + continue; + } + auto virtualName = fmt::format("CPU_BUS@{}", it->second); + if (std::find( + i2cAdaptersFromCpu.begin(), + i2cAdaptersFromCpu.end(), + virtualName) == i2cAdaptersFromCpu.end()) { + continue; + } + + // Find the i2c adapter child (i2c-N) under this platform device. + for (const auto& child : fs::directory_iterator(dirEntry.path())) { + if (re2::RE2::FullMatch( + child.path().filename().string(), kI2cBusNameRegex)) { + auto busNum = extractBusNumFromPath(child.path()); + XLOG(INFO) << fmt::format( + "Resolved {} -> i2c-{} (ACPI: {})", + virtualName, + busNum, + acpiPath); + busNums[virtualName] = busNum; + break; + } + } + } + + if (busNums.size() == i2cAdaptersFromCpu.size()) { + return busNums; + } + sleep(kCpuI2cBusNumsWaitSecs); + } throw std::runtime_error( - "CPU_BUS virtual bus resolution is not yet implemented for AMD CPUs"); + fmt::format( + "Failed to resolve AMD CPU_BUS names over {}s. " + "Resolved {}/{} entries", + kCpuI2cBusNumsWaitSecs * maxRetries, + busNums.size(), + i2cAdaptersFromCpu.size())); } std::map I2cExplorer::resolveIntelCpuBusNums( diff --git a/fboss/platform/platform_manager/I2cExplorer.h b/fboss/platform/platform_manager/I2cExplorer.h index e77c3629b00b5..d171741337b87 100644 --- a/fboss/platform/platform_manager/I2cExplorer.h +++ b/fboss/platform/platform_manager/I2cExplorer.h @@ -40,8 +40,9 @@ class I2cExplorer { std::map getBusNums( const std::vector& i2cAdaptersFromCpu); - // Resolve virtual "CPU_BUS@0" name to a kernel bus number on AMD CPUs. - // Not yet implemented — throws at runtime. + // Resolve virtual "CPU_BUS@N" names to kernel bus numbers on AMD CPUs. + // Scans /sys/devices/platform/AMDI0010:* and reads firmware_node/path + // to map ACPI paths to bus indices (\_SB_.I2CA → @1, \_SB_.I2CB → @0). std::map resolveAmdCpuBusNums( const std::vector& i2cAdaptersFromCpu); diff --git a/fboss/platform/platform_manager/platform_manager_config.thrift b/fboss/platform/platform_manager/platform_manager_config.thrift index 6e7ed8be2ad48..2bf2d865e37f6 100644 --- a/fboss/platform/platform_manager/platform_manager_config.thrift +++ b/fboss/platform/platform_manager/platform_manager_config.thrift @@ -743,12 +743,14 @@ struct PlatformConfig { 12: map pmUnitConfigs; // List of the i2c buses created from the CPU. Entries can use either: - // (a) Virtual name "CPU_BUS@0" — resolved at runtime by detecting the + // (a) Virtual names "CPU_BUS@N" — resolved at runtime by detecting the // CPU vendor (via folly::CpuId) and scanning sysfs for the // corresponding adapter: - // - Intel: matches "SMBus I801 adapter at " (one per - // unit). Only CPU_BUS@0 is supported today. - // - AMD: not yet implemented (throws at runtime). + // - Intel: matches "SMBus I801 adapter at " by adapter + // name. Only CPU_BUS@0 is supported today. + // - AMD: identifies DesignWare I2C buses via ACPI + // firmware_node/path under /sys/devices/platform/AMDI0010:*. + // CPU_BUS@0 maps to \_SB_.I2CB, CPU_BUS@1 to \_SB_.I2CA. // (b) Exact adapter name matching /sys/bus/i2c/devices/i2c-N/name // (e.g. "SMBus I801 adapter at 5000"). // All entries in a single config must use the same style. diff --git a/fboss/platform/platform_manager/tests/ConfigValidatorTest.cpp b/fboss/platform/platform_manager/tests/ConfigValidatorTest.cpp index 6e98205d02d0e..4d56ba41fb597 100644 --- a/fboss/platform/platform_manager/tests/ConfigValidatorTest.cpp +++ b/fboss/platform/platform_manager/tests/ConfigValidatorTest.cpp @@ -108,13 +108,13 @@ TEST(ConfigValidatorTest, InvalidRootSlotType) { } TEST(ConfigValidatorTest, I2cAdaptersFromCpuValidation) { - // CPU_BUS@0 — valid + // CPU_BUS@0 — valid (Intel single-bus or AMD first bus) EXPECT_TRUE(ConfigValidator().isValidI2cAdaptersFromCpu({"CPU_BUS@0"})); - // CPU_BUS@1 — valid + // CPU_BUS@1 — valid (AMD second bus) EXPECT_TRUE(ConfigValidator().isValidI2cAdaptersFromCpu({"CPU_BUS@1"})); - // CPU_BUS@0 + CPU_BUS@1 — valid (two-bus config) + // CPU_BUS@0 + CPU_BUS@1 — valid (AMD two-bus config) EXPECT_TRUE( ConfigValidator().isValidI2cAdaptersFromCpu({"CPU_BUS@0", "CPU_BUS@1"})); @@ -126,6 +126,10 @@ TEST(ConfigValidatorTest, I2cAdaptersFromCpuValidation) { // CPU_BUS@2 — invalid (only @0 and @1 supported) EXPECT_FALSE(ConfigValidator().isValidI2cAdaptersFromCpu({"CPU_BUS@2"})); + // Duplicate CPU_BUS@0 — invalid + EXPECT_FALSE( + ConfigValidator().isValidI2cAdaptersFromCpu({"CPU_BUS@0", "CPU_BUS@0"})); + // Mixed styles — invalid EXPECT_FALSE( ConfigValidator().isValidI2cAdaptersFromCpu(