fru_device: Add the ability to skip addresses for all buses
In certain situations, we won't know the buses ahead of time,
like when probing muxes dynamically using EntityManager. We do
however know the general range of where our eeproms are. Ideally
we would have the ability to set a range of addresses, but this
is a lower lift change, to allow addresses to blocklisted across
all buses.
Tested:
Ran on an nvl32-obmc model with i2c tracing enabled.
Added a blocked list entry and checked logging as well
as that no i2c transactions were issued by fru-device
to the blocked addresses. Existing FRU's worked as
expected.
Change-Id: I9fbebc426a8a5244aa9ea07e41d6a38458088cbb
Signed-off-by: Marc Olberding <molberding@nvidia.com>
diff --git a/src/fru_device/fru_device.cpp b/src/fru_device/fru_device.cpp
index 2363477..3d806a2 100644
--- a/src/fru_device/fru_device.cpp
+++ b/src/fru_device/fru_device.cpp
@@ -81,6 +81,7 @@
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap,
size_t& unknownBusObjectCount, const bool& powerIsOn,
+ const std::set<size_t>& addressBlocklist,
sdbusplus::asio::object_server& objServer);
// Given a bus/address, produce the path in sysfs for an eeprom.
@@ -438,6 +439,7 @@
int getBusFRUs(int file, int first, int last, int bus,
std::shared_ptr<DeviceMap> devices, const bool& powerIsOn,
+ const std::set<size_t>& addressBlocklist,
sdbusplus::asio::object_server& objServer)
{
std::future<int> future = std::async(std::launch::async, [&]() {
@@ -456,6 +458,8 @@
std::set<size_t>& foundItems = fruAddresses[bus];
foundItems.clear();
+ skipList.insert_range(addressBlocklist);
+
auto busFind = busBlocklist.find(bus);
if (busFind != busBlocklist.end())
{
@@ -606,14 +610,84 @@
return future.get();
}
-void loadBlocklist(const char* path)
+struct AddressBlocklistResult
+{
+ int rc;
+ std::set<size_t> list;
+};
+
+AddressBlocklistResult loadAddressBlocklist(const nlohmann::json& data)
+{
+ auto addrIt = data.find("addresses");
+ if (addrIt == data.end())
+ {
+ return {0, std::set<size_t>()};
+ }
+
+ const auto* const addr =
+ data["addresses"].get_ptr<const nlohmann::json::array_t*>();
+
+ if (addr == nullptr)
+ {
+ lg2::error("addresses must be an array");
+ return {EINVAL, std::set<size_t>()};
+ }
+
+ std::set<size_t> addressBlocklist = {};
+
+ for (const auto& address : *addr)
+ {
+ const auto* addrS = address.get_ptr<const std::string*>();
+ if (addrS == nullptr)
+ {
+ lg2::error("address must be a string\n");
+ return {EINVAL, std::set<size_t>()};
+ }
+
+ if (!(addrS->starts_with("0x") || addrS->starts_with("0X")))
+ {
+ lg2::error("address must start with 0x or 0X\n");
+ return {EINVAL, std::set<size_t>()};
+ }
+
+ // The alternative offered here relies on undefined behavior
+ // of dereferencing iterators given by .end()
+ // this pointer access is checked above by the calls to starts_with
+ // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ size_t addressInt = 0;
+ auto [ptr, ec] = std::from_chars(
+ addrS->data() + 2, addrS->data() + addrS->length(), addressInt, 16);
+
+ const auto erc = std::make_error_condition(ec);
+ if (ptr != (addrS->data() + addrS->length()) || erc)
+ {
+ lg2::error("Invalid address type: {ADDR} {MSG}\n", "ADDR", *addrS,
+ "MSG", erc.message());
+ return {EINVAL, std::set<size_t>()};
+ }
+ // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
+ if (addressInt > 0x77)
+ {
+ lg2::error("Invalid address {ADDR}\n", "ADDR", *addrS);
+ return {EINVAL, std::set<size_t>()};
+ }
+
+ addressBlocklist.insert(addressInt);
+ }
+
+ return {0, addressBlocklist};
+}
+
+// once the bus blocklist is made non global,
+// return it here too.
+std::set<size_t> loadBlocklist(const char* path)
{
std::ifstream blocklistStream(path);
if (!blocklistStream.good())
{
// File is optional.
lg2::error("Cannot open blocklist file.\n");
- return;
+ return {};
}
nlohmann::json data =
@@ -685,10 +759,19 @@
std::exit(EXIT_FAILURE);
}
}
+
+ const auto [rc, addressBlocklist] = loadAddressBlocklist(data);
+ if (rc != 0)
+ {
+ std::exit(EXIT_FAILURE);
+ }
+
+ return addressBlocklist;
}
static void findI2CDevices(const std::vector<fs::path>& i2cBuses,
BusMap& busmap, const bool& powerIsOn,
+ const std::set<size_t>& addressBlocklist,
sdbusplus::asio::object_server& objServer)
{
for (const auto& i2cBus : i2cBuses)
@@ -752,7 +835,8 @@
lg2::debug("Scanning bus {BUS}", "BUS", bus);
// fd is closed in this function in case the bus locks up
- getBusFRUs(file, 0x03, 0x77, bus, device, powerIsOn, objServer);
+ getBusFRUs(file, 0x03, 0x77, bus, device, powerIsOn, addressBlocklist,
+ objServer);
lg2::debug("Done scanning bus {BUS}", "BUS", bus);
}
@@ -765,9 +849,11 @@
FindDevicesWithCallback(const std::vector<fs::path>& i2cBuses,
BusMap& busmap, const bool& powerIsOn,
sdbusplus::asio::object_server& objServer,
+ const std::set<size_t>& addressBlocklist,
std::function<void()>&& callback) :
_i2cBuses(i2cBuses), _busMap(busmap), _powerIsOn(powerIsOn),
- _objServer(objServer), _callback(std::move(callback))
+ _objServer(objServer), _callback(std::move(callback)),
+ _addressBlocklist{addressBlocklist}
{}
~FindDevicesWithCallback()
{
@@ -775,7 +861,8 @@
}
void run()
{
- findI2CDevices(_i2cBuses, _busMap, _powerIsOn, _objServer);
+ findI2CDevices(_i2cBuses, _busMap, _powerIsOn, _addressBlocklist,
+ _objServer);
}
const std::vector<fs::path>& _i2cBuses;
@@ -783,6 +870,7 @@
const bool& _powerIsOn;
sdbusplus::asio::object_server& _objServer;
std::function<void()> _callback;
+ std::set<size_t> _addressBlocklist;
};
void addFruObjectToDbus(
@@ -791,7 +879,8 @@
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap,
uint32_t bus, uint32_t address, size_t& unknownBusObjectCount,
- const bool& powerIsOn, sdbusplus::asio::object_server& objServer)
+ const bool& powerIsOn, const std::set<size_t>& addressBlocklist,
+ sdbusplus::asio::object_server& objServer)
{
boost::container::flat_map<std::string, std::string> formattedFRU;
@@ -822,12 +911,12 @@
iface->register_method(
"UpdateFruField",
[bus, address, &dbusInterfaceMap, &unknownBusObjectCount,
- &powerIsOn, &objServer](const std::string& fieldName,
- const std::string& fieldValue) {
+ &powerIsOn, &objServer, addressBlocklist](
+ const std::string& fieldName, const std::string& fieldValue) {
// Update the property
if (!updateFruProperty(fieldValue, bus, address, fieldName,
dbusInterfaceMap, unknownBusObjectCount,
- powerIsOn, objServer))
+ powerIsOn, addressBlocklist, objServer))
{
lg2::debug(
"Failed to Add Field: Name = {NAME}, Value = {VALUE}",
@@ -857,15 +946,15 @@
iface->register_property(
key, property.second + '\0',
[bus, address, propertyName, &dbusInterfaceMap,
- &unknownBusObjectCount, &powerIsOn,
- &objServer](const std::string& req, std::string& resp) {
+ &unknownBusObjectCount, &powerIsOn, &objServer,
+ &addressBlocklist](const std::string& req, std::string& resp) {
if (strcmp(req.c_str(), resp.c_str()) != 0)
{
// call the method which will update
if (updateFruProperty(req, bus, address, propertyName,
dbusInterfaceMap,
unknownBusObjectCount, powerIsOn,
- objServer))
+ addressBlocklist, objServer))
{
resp = req;
}
@@ -1061,6 +1150,7 @@
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap,
bool dbusCall, size_t& unknownBusObjectCount, const bool& powerIsOn,
+ const std::set<size_t>& addressBlocklist,
sdbusplus::asio::object_server& objServer)
{
for (auto device = foundDevices.begin(); device != foundDevices.end();)
@@ -1092,9 +1182,9 @@
i2cBuses.emplace_back(busPath);
auto scan = std::make_shared<FindDevicesWithCallback>(
- i2cBuses, busmap, powerIsOn, objServer,
+ i2cBuses, busmap, powerIsOn, objServer, addressBlocklist,
[busNum, &busmap, &dbusInterfaceMap, &unknownBusObjectCount, &powerIsOn,
- &objServer]() {
+ &objServer, &addressBlocklist]() {
for (auto busIface = dbusInterfaceMap.begin();
busIface != dbusInterfaceMap.end();)
{
@@ -1117,7 +1207,8 @@
{
addFruObjectToDbus(device.second, dbusInterfaceMap,
static_cast<uint32_t>(busNum), device.first,
- unknownBusObjectCount, powerIsOn, objServer);
+ unknownBusObjectCount, powerIsOn,
+ addressBlocklist, objServer);
}
});
scan->run();
@@ -1129,6 +1220,7 @@
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap,
size_t& unknownBusObjectCount, const bool& powerIsOn,
+ const std::set<size_t>& addressBlocklist,
sdbusplus::asio::object_server& objServer)
{
static boost::asio::steady_timer timer(io);
@@ -1170,7 +1262,7 @@
foundDevices.clear();
auto scan = std::make_shared<FindDevicesWithCallback>(
- i2cBuses, busmap, powerIsOn, objServer, [&]() {
+ i2cBuses, busmap, powerIsOn, objServer, addressBlocklist, [&]() {
for (auto& busIface : dbusInterfaceMap)
{
objServer.remove_interface(busIface.second);
@@ -1195,7 +1287,7 @@
addFruObjectToDbus(device.second, dbusInterfaceMap,
devicemap.first, device.first,
unknownBusObjectCount, powerIsOn,
- objServer);
+ addressBlocklist, objServer);
}
}
});
@@ -1210,6 +1302,7 @@
std::pair<size_t, size_t>,
std::shared_ptr<sdbusplus::asio::dbus_interface>>& dbusInterfaceMap,
size_t& unknownBusObjectCount, const bool& powerIsOn,
+ const std::set<size_t>& addressBlocklist,
sdbusplus::asio::object_server& objServer)
{
lg2::debug(
@@ -1241,7 +1334,7 @@
}
rescanBusses(busMap, dbusInterfaceMap, unknownBusObjectCount, powerIsOn,
- objServer);
+ addressBlocklist, objServer);
return true;
}
@@ -1263,7 +1356,9 @@
}
// check for and load blocklist with initial buses.
- loadBlocklist(blocklistPath);
+ // once busBlocklist is moved to be non global,
+ // add it here
+ auto addressBlocklist = loadBlocklist(blocklistPath);
systemBus->request_name("xyz.openbmc_project.FruDevice");
@@ -1279,12 +1374,12 @@
iface->register_method("ReScan", [&]() {
rescanBusses(busMap, dbusInterfaceMap, unknownBusObjectCount, powerIsOn,
- objServer);
+ addressBlocklist, objServer);
});
iface->register_method("ReScanBus", [&](uint16_t bus) {
rescanOneBus(busMap, bus, dbusInterfaceMap, true, unknownBusObjectCount,
- powerIsOn, objServer);
+ powerIsOn, addressBlocklist, objServer);
});
iface->register_method("GetRawFru", getFRUInfo);
@@ -1299,7 +1394,7 @@
}
// schedule rescan on success
rescanBusses(busMap, dbusInterfaceMap, unknownBusObjectCount,
- powerIsOn, objServer);
+ powerIsOn, addressBlocklist, objServer);
});
iface->initialize();
@@ -1324,7 +1419,7 @@
if (powerIsOn)
{
rescanBusses(busMap, dbusInterfaceMap, unknownBusObjectCount,
- powerIsOn, objServer);
+ powerIsOn, addressBlocklist, objServer);
}
};
@@ -1377,12 +1472,12 @@
static_cast<uint16_t>(rootBus),
dbusInterfaceMap, false,
unknownBusObjectCount, powerIsOn,
- objServer);
+ addressBlocklist, objServer);
}
rescanOneBus(busMap, static_cast<uint16_t>(bus),
dbusInterfaceMap, false,
unknownBusObjectCount, powerIsOn,
- objServer);
+ addressBlocklist, objServer);
}
}
break;
@@ -1399,7 +1494,7 @@
dirWatch.async_read_some(boost::asio::buffer(readBuffer), watchI2cBusses);
// run the initial scan
rescanBusses(busMap, dbusInterfaceMap, unknownBusObjectCount, powerIsOn,
- objServer);
+ addressBlocklist, objServer);
io.run();
return 0;