[Proposal] Regions on an eeprom to serve as logical eeproms

fboss/platform/platform_manager/ConfigValidator.cpp

@@ -506,6 +506,14 @@ bool ConfigValidator::isValidI2cDeviceConfig(
         *i2cDeviceConfig.pmUnitScopedName());
     return false;
   }
+  if (*i2cDeviceConfig.isEeprom() &&
+      !i2cDeviceConfig.pmUnitScopedName()->ends_with("_EEPROM")) {
+    XLOGF(
+        ERR,
+        "isEeprom is true but pmUnitScopedName '{}' does not contain 'EEPROM'",
+        *i2cDeviceConfig.pmUnitScopedName());
+    return false;
+  }
   return true;
 }
 
@@ -830,6 +838,13 @@ bool ConfigValidator::isValidPmUnitConfig(
       return false;
     }
   }
+
+  // Validate logical eeprom regions for overlaps
+  if (!isValidLogicalEepromRegions(
+          slotTypeConfigs, *pmUnitConfig.pluggedInSlotType(), pmUnitConfig)) {
+    return false;
+  }
+
   return true;
 }
 
@@ -1240,4 +1255,75 @@ bool ConfigValidator::isValidChassisEepromDevicePath(
   return true;
 }
 
+bool ConfigValidator::isValidLogicalEepromRegions(
+    const std::map<std::string, SlotTypeConfig>& slotTypeConfigs,
+    const std::string& slotType,
+    const PmUnitConfig& pmUnitConfig) {
+  constexpr int kEepromSize = 512;
+  using EepromKey = std::pair<std::string, std::string>;
+  using EepromRegion = std::tuple<int, std::string, std::string>;
+  std::map<EepromKey, std::vector<EepromRegion>> eepromRegions;
+
+  if (slotTypeConfigs.contains(slotType)) {
+    const auto& slotTypeConfig = slotTypeConfigs.at(slotType);
+    if (slotTypeConfig.idpromConfig()) {
+      const auto& idprom = *slotTypeConfig.idpromConfig();
+      eepromRegions[{*idprom.busName(), *idprom.address()}].emplace_back(
+          *idprom.offset(), "IDPROM", *idprom.kernelDeviceName());
+    }
+  }
+
+  for (const auto& i2cDevice : *pmUnitConfig.i2cDeviceConfigs()) {
+    if (!*i2cDevice.isEeprom()) {
+      continue;
+    }
+    int offset = i2cDevice.eepromOffset() ? *i2cDevice.eepromOffset() : 0;
+    eepromRegions[{*i2cDevice.busName(), *i2cDevice.address()}].emplace_back(
+        offset, *i2cDevice.pmUnitScopedName(), *i2cDevice.kernelDeviceName());
+  }
+
+  for (const auto& [key, regions] : eepromRegions) {
+    if (regions.size() <= 1) {
+      continue;
+    }
+    const auto& [bus, addr] = key;
+    const auto& firstKernelDeviceName = std::get<2>(regions[0]);
+    for (size_t i = 1; i < regions.size(); ++i) {
+      if (std::get<2>(regions[i]) != firstKernelDeviceName) {
+        XLOG(ERR) << fmt::format(
+            "Logical eeproms {} and {} at (bus: {}, addr: {}) have different "
+            "kernelDeviceNames: {} vs {}",
+            std::get<1>(regions[0]),
+            std::get<1>(regions[i]),
+            bus,
+            addr,
+            firstKernelDeviceName,
+            std::get<2>(regions[i]));
+        return false;
+      }
+    }
+    for (size_t i = 0; i < regions.size(); ++i) {
+      for (size_t j = i + 1; j < regions.size(); ++j) {
+        int off1 = std::get<0>(regions[i]);
+        int off2 = std::get<0>(regions[j]);
+        if (off1 < off2 + kEepromSize && off2 < off1 + kEepromSize) {
+          XLOG(ERR) << fmt::format(
+              "Logical eeproms {} and {} at (bus: {}, addr: {}) have "
+              "overlapping regions: [{}, {}) and [{}, {})",
+              std::get<1>(regions[i]),
+              std::get<1>(regions[j]),
+              bus,
+              addr,
+              off1,
+              off1 + kEepromSize,
+              off2,
+              off2 + kEepromSize);
+          return false;
+        }
+      }
+    }
+  }
+  return true;
+}
+
 } // namespace facebook::fboss::platform::platform_manager

fboss/platform/platform_manager/ConfigValidator.h

@@ -53,7 +53,10 @@ class ConfigValidator {
   bool isValidChassisEepromDevicePath(
       const PlatformConfig& platformConfig,
       const std::string& chassisEepromDevicePath);
-
+  bool isValidLogicalEepromRegions(
+      const std::map<std::string, SlotTypeConfig>& slotTypeConfigs,
+      const std::string& slotType,
+      const PmUnitConfig& pmUnitConfig);
   // Used by other platform services config validation.
   virtual bool isValidSlotPath(
       const PlatformConfig& platformConfig,

fboss/platform/platform_manager/I2cExplorer.cpp

@@ -141,13 +141,25 @@ void I2cExplorer::createI2cDevice(
   if (isI2cDevicePresent(busNum, addr)) {
     auto existingDeviceName = getI2cDeviceName(busNum, addr);
     if (existingDeviceName && existingDeviceName.value() == deviceName) {
-      XLOG(INFO) << fmt::format(
-          "Device {} ({}) already exists at bus: {}, addr: {}. "
-          "Skipping creation.",
-          pmUnitScopedName,
-          deviceName,
-          busNum,
-          addr.hex2Str());
+      bool isEeprom = pmUnitScopedName.find("EEPROM") != std::string::npos;
+      if (isEeprom) {
+        XLOG(INFO) << fmt::format(
+            "Device {} ({}) already exists at bus: {}, addr: {}. "
+            "Skipping creation. This is expected for logical eeproms sharing "
+            "the same physical device.",
+            pmUnitScopedName,
+            deviceName,
+            busNum,
+            addr.hex2Str());
+      } else {
+        XLOG(INFO) << fmt::format(
+            "Device {} ({}) already exists at bus: {}, addr: {}. "
+            "Skipping creation.",
+            pmUnitScopedName,
+            deviceName,
+            busNum,
+            addr.hex2Str());
+      }
       return;
     }
     throw std::runtime_error(

fboss/platform/platform_manager/tests/ConfigValidatorTest.cpp

@@ -1356,3 +1356,71 @@ TEST(ConfigValidatorTest, ChassisEepromDevicePath) {
   config.chassisEepromDevicePath() = "/[SOME_OTHER_EEPROM]";
   EXPECT_FALSE(ConfigValidator().isValid(config));
 }
+
+TEST(ConfigValidatorTest, LogicalEepromRegions) {
+  auto config = getBasicConfig();
+
+  // Get the chassis eeprom from the basic config
+  auto chassisEeprom =
+      config.pmUnitConfigs()->at("SCM").i2cDeviceConfigs()->at(0);
+
+  // Create two logical eeproms on the same physical device
+  I2cDeviceConfig eeprom1, eeprom2;
+  eeprom1.pmUnitScopedName() = "CHASSIS_EEPROM";
+  eeprom1.busName() = "SMB_BUS";
+  eeprom1.address() = "0x51";
+  eeprom1.kernelDeviceName() = "24c512";
+  eeprom1.isEeprom() = true;
+  eeprom1.eepromOffset() = 15360;
+
+  eeprom2.pmUnitScopedName() = "LOGICAL_EEPROM";
+  eeprom2.busName() = "SMB_BUS";
+  eeprom2.address() = "0x51";
+  eeprom2.kernelDeviceName() = "24c512";
+  eeprom2.isEeprom() = true;
+  eeprom2.eepromOffset() = 15872;
+
+  auto pmUnitConfig = config.pmUnitConfigs()->at("SCM");
+  pmUnitConfig.i2cDeviceConfigs() = {chassisEeprom, eeprom1, eeprom2};
+  config.pmUnitConfigs() = {{"SCM", pmUnitConfig}};
+
+  // Valid: Non-overlapping regions
+  EXPECT_TRUE(ConfigValidator().isValid(config));
+
+  // Invalid: Overlapping regions (both start at offset 0)
+  eeprom1.eepromOffset() = 0;
+  eeprom2.eepromOffset() = 0;
+  pmUnitConfig.i2cDeviceConfigs() = {chassisEeprom, eeprom1, eeprom2};
+  config.pmUnitConfigs() = {{"SCM", pmUnitConfig}};
+  EXPECT_FALSE(ConfigValidator().isValid(config));
+
+  // Invalid: Partially overlapping regions
+  eeprom1.eepromOffset() = 0;
+  eeprom2.eepromOffset() = 511;
+  pmUnitConfig.i2cDeviceConfigs() = {chassisEeprom, eeprom1, eeprom2};
+  config.pmUnitConfigs() = {{"SCM", pmUnitConfig}};
+  EXPECT_FALSE(ConfigValidator().isValid(config));
+
+  // Invalid: Different kernelDeviceNames for same physical device
+  eeprom1.eepromOffset() = 0;
+  eeprom2.eepromOffset() = 512;
+  eeprom2.kernelDeviceName() = "24c128";
+  pmUnitConfig.i2cDeviceConfigs() = {chassisEeprom, eeprom1, eeprom2};
+  config.pmUnitConfigs() = {{"SCM", pmUnitConfig}};
+  EXPECT_FALSE(ConfigValidator().isValid(config));
+
+  // Valid: Different physical devices (two different eeproms)
+  eeprom2.address() = "0x52";
+  eeprom1.eepromOffset() = 0;
+  eeprom2.eepromOffset() = 0;
+  pmUnitConfig.i2cDeviceConfigs() = {chassisEeprom, eeprom1, eeprom2};
+  config.pmUnitConfigs() = {{"SCM", pmUnitConfig}};
+  EXPECT_TRUE(ConfigValidator().isValid(config));
+
+  // Valid: Different physical devices (different buses)
+  eeprom2.address() = "0x51";
+  eeprom2.busName() = "OTHER_BUS";
+  pmUnitConfig.i2cDeviceConfigs() = {chassisEeprom, eeprom1, eeprom2};
+  config.pmUnitConfigs() = {{"SCM", pmUnitConfig}};
+  EXPECT_TRUE(ConfigValidator().isValid(config));
+}