From 654ee84e9ed59090ff30c6c50d1c233d23c4306e Mon Sep 17 00:00:00 2001
From: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
Date: Tue, 12 Dec 2023 10:59:51 +0100
Subject: [PATCH] make FPGA device interface agnostic

remove explicit mentioning of PCIe in the use of Device as a preparation
for integrating platform devices. auto formatted some files.

Signed-off-by: Niklas Eiling <niklas.eiling@eonerc.rwth-aachen.de>
---
 fpga/common                            |   2 +-
 fpga/etc/fpgas.json                    |   3 +-
 fpga/include/villas/fpga/card.hpp      |   1 +
 fpga/include/villas/fpga/pcie_card.hpp |  55 ++-----
 fpga/include/villas/fpga/utils.hpp     |   5 +-
 fpga/lib/ips/dma.cpp                   |   4 +-
 fpga/lib/pcie_card.cpp                 | 220 ++++++++++++-------------
 fpga/lib/utils.cpp                     | 160 ++++++++++--------
 8 files changed, 228 insertions(+), 222 deletions(-)

diff --git a/fpga/common b/fpga/common
index 7bef5e020..0cb6cd23c 160000
--- a/fpga/common
+++ b/fpga/common
@@ -1 +1 @@
-Subproject commit 7bef5e02015805a5f8988be3da78f4df844a7bff
+Subproject commit 0cb6cd23caacdd504753fd7f32e0a606d1b964f3
diff --git a/fpga/etc/fpgas.json b/fpga/etc/fpgas.json
index 0527e0dc0..7a4811ce7 100644
--- a/fpga/etc/fpgas.json
+++ b/fpga/etc/fpgas.json
@@ -5,7 +5,8 @@
         "slot": "0000:88:00.0",
         "do_reset": true,
         "ips": "vc707-xbar-pcie/vc707-xbar-pcie.json",
-        "polling": false
+        "polling": false,
+        "interface": "pcie"
       }
     }
 }
diff --git a/fpga/include/villas/fpga/card.hpp b/fpga/include/villas/fpga/card.hpp
index dea210b86..f58a4c681 100644
--- a/fpga/include/villas/fpga/card.hpp
+++ b/fpga/include/villas/fpga/card.hpp
@@ -23,6 +23,7 @@ class Card
 public:
         bool polling;
 
+        std::string name; // The name of the FPGA card
         std::shared_ptr<kernel::vfio::Container> vfioContainer;
         std::shared_ptr<kernel::vfio::Device> vfioDevice;
 
diff --git a/fpga/include/villas/fpga/pcie_card.hpp b/fpga/include/villas/fpga/pcie_card.hpp
index 24567c2e5..1f3afa151 100644
--- a/fpga/include/villas/fpga/pcie_card.hpp
+++ b/fpga/include/villas/fpga/pcie_card.hpp
@@ -64,55 +64,32 @@ public:	// TODO: make this private
 	bool doReset;					// Reset VILLASfpga during startup?
 	int affinity;					// Affinity for MSI interrupts
 
-	std::string name;				// The name of the FPGA card
+        std::shared_ptr<kernel::pci::Device> pdev; // PCI device handle
 
-	std::shared_ptr<kernel::pci::Device> pdev;	// PCI device handle
-
-protected:
-	Logger
-	getLogger() const
-	{
-		return villas::logging.get(name);
-	}
+      protected:
+        Logger getLogger() const { return villas::logging.get(name); }
 };
 
 class PCIeCardFactory : public plugin::Plugin {
 public:
+  static std::shared_ptr<PCIeCard>
+  make(json_t *json, std::string card_name,
+       std::shared_ptr<kernel::vfio::Container> vc,
+       const std::filesystem::path &searchPath);
 
-	static std::list<std::shared_ptr<PCIeCard>> make(json_t *json,
-		std::shared_ptr<kernel::pci::DeviceList> pci,
-		std::shared_ptr<kernel::vfio::Container> vc,
-		const std::filesystem::path& searchPath = std::filesystem::path());
+  static PCIeCard *make() { return new PCIeCard(); }
 
-	static
-	PCIeCard* make()
-	{
-		return new PCIeCard();
-	}
+  static Logger getStaticLogger() {
+    return villas::logging.get("pcie:card:factory");
+  }
 
-	static Logger
-	getStaticLogger()
-	{
-		return villas::logging.get("pcie:card:factory");
-	}
+  virtual std::string getName() const { return "pcie"; }
 
-	virtual std::string
-	getName() const
-	{
-		return "pcie";
-	}
+  virtual std::string getDescription() const {
+    return "Xilinx PCIe FPGA cards";
+  }
 
-	virtual std::string
-	getDescription() const
-	{
-		return "Xilinx PCIe FPGA cards";
-	}
-
-	virtual
-	std::string getType() const
-	{
-		return "card";
-	}
+  virtual std::string getType() const { return "card"; }
 };
 
 } // namespace fpga
diff --git a/fpga/include/villas/fpga/utils.hpp b/fpga/include/villas/fpga/utils.hpp
index 71d651c31..649089a21 100644
--- a/fpga/include/villas/fpga/utils.hpp
+++ b/fpga/include/villas/fpga/utils.hpp
@@ -13,9 +13,12 @@
 namespace villas {
 namespace fpga {
 
-std::shared_ptr<fpga::PCIeCard>
+std::shared_ptr<fpga::Card>
 setupFpgaCard(const std::string &configFile, const std::string &fpgaName);
 
+int createCards(json_t *config, std::list<std::shared_ptr<fpga::Card>> &cards,
+              std::filesystem::path &searchPath);
+
 void setupColorHandling();
 
 class ConnectString {
diff --git a/fpga/lib/ips/dma.cpp b/fpga/lib/ips/dma.cpp
index b744fd98b..1da604901 100644
--- a/fpga/lib/ips/dma.cpp
+++ b/fpga/lib/ips/dma.cpp
@@ -284,8 +284,8 @@ bool Dma::writeScatterGather(const void *buf, size_t len)
 	ret = XAxiDma_BdRingAlloc(txRing, 1, &bd);
 	if (ret != XST_SUCCESS) {
 		hwLock.unlock();
-		throw RuntimeError("BdRingAlloc returned {}.", ret);
-	}
+                throw RuntimeError("Write: BdRingAlloc returned {}.", ret);
+        }
 
 	ret = XAxiDma_BdSetBufAddr(bd, (uintptr_t) buf);
 	if (ret != XST_SUCCESS) {
diff --git a/fpga/lib/pcie_card.cpp b/fpga/lib/pcie_card.cpp
index f68da1533..f86852ac4 100644
--- a/fpga/lib/pcie_card.cpp
+++ b/fpga/lib/pcie_card.cpp
@@ -25,140 +25,134 @@ static PCIeCardFactory PCIeCardFactoryInstance;
 
 static const kernel::pci::Device defaultFilter((kernel::pci::Id(FPGA_PCI_VID_XILINX, FPGA_PCI_PID_VFPGA)));
 
-std::list<std::shared_ptr<PCIeCard>> PCIeCardFactory::make(json_t *json,
-	std::shared_ptr<kernel::pci::DeviceList> pci,
-	std::shared_ptr<kernel::vfio::Container> vc,
-	const std::filesystem::path& searchPath)
-{
-	std::list<std::shared_ptr<PCIeCard>> cards;
-	auto logger = getStaticLogger();
+std::shared_ptr<PCIeCard>
+PCIeCardFactory::make(json_t *json_card, std::string card_name,
+                      std::shared_ptr<kernel::vfio::Container> vc,
+		const std::filesystem::path &searchPath) {
+  auto logger = getStaticLogger();
 
-	const char *card_name;
-	json_t *json_card;
-	json_object_foreach(json, card_name, json_card) {
-		logger->info("Found config for FPGA card {}", card_name);
+  json_t *json_ips = nullptr;
+  json_t *json_paths = nullptr;
+  const char *pci_slot = nullptr;
+  const char *pci_id = nullptr;
+  int do_reset = 0;
+  int affinity = 0;
+  int polling = 0;
 
-		json_t*     json_ips = nullptr;
-		json_t*     json_paths = nullptr;
-		const char* pci_slot = nullptr;
-		const char* pci_id   = nullptr;
-		int         do_reset = 0;
-		int         affinity = 0;
-		int 	    polling  = 0;
+  json_error_t err;
+  int ret = json_unpack_ex(
+      json_card, &err, 0, "{ s: o, s?: i, s?: b, s?: s, s?: s, s?: b, s?: o }",
+      "ips", &json_ips, "affinity", &affinity, "do_reset", &do_reset, "slot",
+      &pci_slot, "id", &pci_id, "polling", &polling, "paths", &json_paths);
 
-		json_error_t err;
-		int ret = json_unpack_ex(json_card, &err, 0, "{ s: o, s?: i, s?: b, s?: s, s?: s, s?: b, s?: o }",
-			"ips",		&json_ips,
-			"affinity", 	&affinity,
-			"do_reset", 	&do_reset,
-			"slot", 	&pci_slot,
-		    	"id", 		&pci_id,
-			"polling", 	&polling,
-			"paths", 	&json_paths);
+  if (ret != 0)
+    throw ConfigError(json_card, err, "", "Failed to parse card");
 
-		if (ret != 0)
-			throw ConfigError(json_card, err, "", "Failed to parse card");
+  auto card = std::unique_ptr<PCIeCard>(make());
 
-		auto card = std::unique_ptr<PCIeCard>(make());
+  // Populate generic properties
+  card->name = std::string(card_name);
+  card->vfioContainer = vc;
+  card->affinity = affinity;
+  card->doReset = do_reset != 0;
+  card->polling = (polling != 0);
 
-		// Populate generic properties
-		card->name = std::string(card_name);
-		card->vfioContainer = vc;
-		card->affinity = affinity;
-		card->doReset = do_reset != 0;
-		card->polling = (polling != 0);
+  kernel::pci::Device filter = defaultFilter;
 
-		kernel::pci::Device filter = defaultFilter;
+  if (pci_id)
+    filter.id = kernel::pci::Id(pci_id);
+  if (pci_slot)
+    filter.slot = kernel::pci::Slot(pci_slot);
 
-		if (pci_id)
-			filter.id = kernel::pci::Id(pci_id);
-		if (pci_slot)
-			filter.slot = kernel::pci::Slot(pci_slot);
+  // Search for FPGA card
+  card->pdev = kernel::pci::DeviceList::getInstance()->lookupDevice(filter);
+  if (!card->pdev) {
+    logger->warn("Failed to find PCI device");
+    return nullptr;
+  }
 
-		// Search for FPGA card
-		card->pdev = pci->lookupDevice(filter);
-		if (!card->pdev) {
-			logger->warn("Failed to find PCI device");
-			continue;
-		}
+  if (not card->init()) {
+    logger->warn("Cannot start FPGA card {}", card_name);
+    return nullptr;
+  }
 
-		if (not card->init()) {
-			logger->warn("Cannot start FPGA card {}", card_name);
-			continue;
-		}
+  // Load IPs from a separate json file
+  if (!json_is_string(json_ips)) {
+    logger->debug("FPGA IP cores config item is not a string.");
+    throw ConfigError(json_ips, "node-config-fpga-ips",
+                      "FPGA IP cores config item is not a string.");
+  }
+  if (!searchPath.empty()) {
+    std::filesystem::path json_ips_path =
+        searchPath / json_string_value(json_ips);
+    logger->debug("searching for FPGA IP cors config at {}",
+                  json_ips_path.string());
+    json_ips = json_load_file(json_ips_path.c_str(), 0, nullptr);
+  }
+  if (json_ips == nullptr) {
+    json_ips = json_load_file(json_string_value(json_ips), 0, nullptr);
+    logger->debug("searching for FPGA IP cors config at {}",
+                  json_string_value(json_ips));
+    if (json_ips == nullptr) {
+      throw ConfigError(json_ips, "node-config-fpga-ips",
+                        "Failed to find FPGA IP cores config");
+    }
+  }
 
-		// Load IPs from a separate json file
-		if (!json_is_string(json_ips)) {
-			logger->debug("FPGA IP cores config item is not a string.");
-			throw ConfigError(json_ips, "node-config-fpga-ips", "FPGA IP cores config item is not a string.");
-		}
-		if (!searchPath.empty()) {
-			std::filesystem::path json_ips_path = searchPath / json_string_value(json_ips);
-			logger->debug("searching for FPGA IP cors config at {}", json_ips_path.string());
-			json_ips = json_load_file(json_ips_path.c_str(), 0, nullptr);
-		}
-		if (json_ips == nullptr) {
-			json_ips = json_load_file(json_string_value(json_ips), 0, nullptr);
-			logger->debug("searching for FPGA IP cors config at {}", json_string_value(json_ips));
-			if (json_ips == nullptr) {
-				throw ConfigError(json_ips, "node-config-fpga-ips", "Failed to find FPGA IP cores config");
-			}
-		}
+  if (not json_is_object(json_ips))
+    throw ConfigError(json_ips, "node-config-fpga-ips",
+                      "FPGA IP core list must be an object!");
 
-		if (not json_is_object(json_ips))
-			throw ConfigError(json_ips, "node-config-fpga-ips", "FPGA IP core list must be an object!");
+  card->ips = ip::CoreFactory::make(card.get(), json_ips);
+  if (card->ips.empty())
+    throw ConfigError(json_ips, "node-config-fpga-ips",
+                      "Cannot initialize IPs of FPGA card {}", card_name);
 
-		card->ips = ip::CoreFactory::make(card.get(), json_ips);
-		if (card->ips.empty())
-			throw ConfigError(json_ips, "node-config-fpga-ips", "Cannot initialize IPs of FPGA card {}", card_name);
+  if (not card->check())
+    throw RuntimeError("Checking of FPGA card {} failed", card_name);
 
-		if (not card->check())
-			throw RuntimeError("Checking of FPGA card {} failed", card_name);
+  // Additional static paths for AXI-Steram switch
+  if (json_paths != nullptr) {
+    if (not json_is_array(json_paths))
+      throw ConfigError(json_paths, err, "",
+                        "Switch path configuration must be an array");
 
-		// Additional static paths for AXI-Steram switch
-		if (json_paths != nullptr) {
-			if (not json_is_array(json_paths))
-				throw ConfigError(json_paths, err, "", "Switch path configuration must be an array");
+    size_t i;
+    json_t *json_path;
+    json_array_foreach(json_paths, i, json_path) {
+      const char *from, *to;
+      int reverse = 0;
 
-			size_t i;
-			json_t *json_path;
-			json_array_foreach(json_paths, i, json_path) {
-				const char *from, *to;
-				int reverse = 0;
+      ret = json_unpack_ex(json_path, &err, 0, "{ s: s, s: s, s?: b }", "from",
+                           &from, "to", &to, "reverse", &reverse);
+      if (ret != 0)
+        throw ConfigError(json_path, err, "",
+                          "Cannot parse switch path config");
 
-				ret = json_unpack_ex(json_path, &err, 0, "{ s: s, s: s, s?: b }",
-					"from", &from,
-					"to", &to,
-					"reverse", &reverse
-				);
-				if (ret != 0)
-					throw ConfigError(json_path, err, "", "Cannot parse switch path config");
+      auto masterIpCore = card->lookupIp(from);
+      if (!masterIpCore)
+        throw ConfigError(json_path, "", "Unknown IP {}", from);
 
-				auto masterIpCore = card->lookupIp(from);
-				if (!masterIpCore)
-					throw ConfigError(json_path, "", "Unknown IP {}", from);
+      auto slaveIpCore = card->lookupIp(to);
+      if (!slaveIpCore)
+        throw ConfigError(json_path, "", "Unknown IP {}", to);
 
-				auto slaveIpCore = card->lookupIp(to);
-				if (!slaveIpCore)
-					throw ConfigError(json_path, "", "Unknown IP {}", to);
+      auto masterIpNode = std::dynamic_pointer_cast<ip::Node>(masterIpCore);
+      if (!masterIpNode)
+        throw ConfigError(json_path, "", "IP {} is not a streaming node", from);
 
-				auto masterIpNode = std::dynamic_pointer_cast<ip::Node>(masterIpCore);
-				if (!masterIpNode)
-					throw ConfigError(json_path, "", "IP {} is not a streaming node", from);
+      auto slaveIpNode = std::dynamic_pointer_cast<ip::Node>(slaveIpCore);
+      if (!slaveIpNode)
+        throw ConfigError(json_path, "", "IP {} is not a streaming node", to);
 
-				auto slaveIpNode = std::dynamic_pointer_cast<ip::Node>(slaveIpCore);
-				if (!slaveIpNode)
-					throw ConfigError(json_path, "", "IP {} is not a streaming node", to);
-
-				if (not masterIpNode->connect(*slaveIpNode, reverse != 0))
-					throw ConfigError(json_path, "", "Failed to connect node {} to {}", from, to);
-			}
-		}
-
-		cards.push_back(std::move(card));
-	}
-
-	return cards;
+      if (not masterIpNode->connect(*slaveIpNode, reverse != 0))
+        throw ConfigError(json_path, "", "Failed to connect node {} to {}",
+                          from, to);
+    }
+  }
+  // Deallocate JSON config
+  json_decref(json_ips);
+  return card;
 }
 
 PCIeCard::~PCIeCard()
diff --git a/fpga/lib/utils.cpp b/fpga/lib/utils.cpp
index 556c79853..f07a590c7 100644
--- a/fpga/lib/utils.cpp
+++ b/fpga/lib/utils.cpp
@@ -33,7 +33,6 @@
 
 using namespace villas;
 
-static std::shared_ptr<kernel::pci::DeviceList> pciDevices;
 static auto logger = villas::logging.get("streamer");
 
 fpga::ConnectString::ConnectString(std::string& connectString, int maxPortNum) :
@@ -64,24 +63,26 @@ void fpga::ConnectString::parseString(std::string& connectString)
 		return;
 	}
 
-	static const std::regex regex("([0-9]+)([<\\->]+)([0-9]+|stdin|stdout|pipe)");
-	std::smatch match;
+        static const std::regex regex(
+            "([0-9]+)([<\\->]+)([0-9]+|stdin|stdout|pipe|dma)");
+        std::smatch match;
 
-	if (!std::regex_match(connectString, match, regex) || match.size() != 4) {
-		logger->error("Invalid connect string: {}", connectString);
-		throw std::runtime_error("Invalid connect string");
-	}
+        if (!std::regex_match(connectString, match, regex) ||
+            match.size() != 4) {
+          logger->error("Invalid connect string: {}", connectString);
+          throw std::runtime_error("Invalid connect string");
+        }
 
-	if (match[2] == "<->") {
-		bidirectional = true;
-	} else if(match[2] == "<-") {
-		invert = true;
-	}
+        if (match[2] == "<->") {
+          bidirectional = true;
+        } else if (match[2] == "<-") {
+          invert = true;
+        }
 
-	std::string srcStr = (invert ? match[3] : match[1]);
-	std::string dstStr = (invert ? match[1] : match[3]);
+        std::string srcStr = (invert ? match[3] : match[1]);
+        std::string dstStr = (invert ? match[1] : match[3]);
 
-	srcAsInt = portStringToInt(srcStr);
+        srcAsInt = portStringToInt(srcStr);
 	dstAsInt = portStringToInt(dstStr);
 	if (srcAsInt == -1) {
 		srcIsStdin = true;
@@ -95,16 +96,16 @@ void fpga::ConnectString::parseString(std::string& connectString)
 
 int fpga::ConnectString::portStringToInt(std::string &str) const
 {
-	if (str == "stdin" || str == "stdout" || str == "pipe") {
-		return -1;
-	} else {
-		const int port = std::stoi(str);
+  if (str == "stdin" || str == "stdout" || str == "pipe" || str == "dma") {
+    return -1;
+  } else {
+    const int port = std::stoi(str);
 
-		if (port > maxPortNum || port < 0)
-			throw std::runtime_error("Invalid port number");
+    if (port > maxPortNum || port < 0)
+      throw std::runtime_error("Invalid port number");
 
-		return port;
-	}
+    return port;
+  }
 }
 
 
@@ -163,58 +164,87 @@ void fpga::setupColorHandling()
 	});
 }
 
-std::shared_ptr<fpga::PCIeCard>
-fpga::setupFpgaCard(const std::string &configFile, const std::string &fpgaName)
-{
-	pciDevices = std::make_shared<kernel::pci::DeviceList>();
+int fpga::createCards(json_t *config,
+                      std::list<std::shared_ptr<fpga::Card>> &cards,
+                      std::filesystem::path &searchPath) {
+  int numFpgas = 0;
+  auto vfioContainer = std::make_shared<kernel::vfio::Container>();
+  auto configDir = std::filesystem::path().parent_path();
 
-	auto vfioContainer = std::make_shared<kernel::vfio::Container>();
-	auto configDir = std::filesystem::path(configFile).parent_path();
+  json_t *fpgas = json_object_get(config, "fpgas");
+  if (fpgas == nullptr) {
+    logger->error("No section 'fpgas' found in config");
+    exit(1);
+  }
 
-	// Parse FPGA configuration
-	FILE* f = fopen(configFile.c_str(), "r");
-	if (!f)
-		throw RuntimeError("Cannot open config file: {}", configFile);
+  const char *card_name;
+  json_t *json_card;
+  json_object_foreach(fpgas, card_name, json_card) {
+    const char *interfaceName;
+    json_error_t err;
+    logger->info("Found config for FPGA card {}", card_name);
+    int ret = json_unpack_ex(json_card, &err, 0, "{s: s}", "interface",
+                             &interfaceName);
+    if (ret) {
+      throw ConfigError(json_card, err, "interface",
+                        "Failed to parse interface name for card {}",
+                        card_name);
+    }
+    std::string interfaceNameStr(interfaceName);
+    if (interfaceNameStr == "pcie") {
+      auto card = fpga::PCIeCardFactory::make(json_card, std::string(card_name),
+                                              vfioContainer, searchPath);
+      if (card) {
+        cards.push_back(std::move(card));
+        numFpgas++;
+      }
+    } else if (interfaceNameStr == "platform") {
+      throw RuntimeError("Platform interface not implemented yet");
+    } else {
+      throw RuntimeError("Unknown interface type {}", interfaceNameStr);
+    }
+  }
+  return numFpgas;
+}
 
-	json_t* json = json_loadf(f, 0, nullptr);
-	if (!json) {
-		logger->error("Cannot parse JSON config");
-		fclose(f);
-		throw RuntimeError("Cannot parse JSON config");
-	}
+std::shared_ptr<fpga::Card> fpga::setupFpgaCard(const std::string &configFile,
+                                                const std::string &fpgaName) {
+  auto vfioContainer = std::make_shared<kernel::vfio::Container>();
+  auto configDir = std::filesystem::path(configFile).parent_path();
 
-	fclose(f);
+  // Parse FPGA configuration
+  FILE *f = fopen(configFile.c_str(), "r");
+  if (!f)
+    throw RuntimeError("Cannot open config file: {}", configFile);
 
-	json_t* fpgas = json_object_get(json, "fpgas");
-	if (fpgas == nullptr) {
-		logger->error("No section 'fpgas' found in config");
-		exit(1);
-	}
+  json_t *json = json_loadf(f, 0, nullptr);
+  if (!json) {
+    logger->error("Cannot parse JSON config");
+    fclose(f);
+    throw RuntimeError("Cannot parse JSON config");
+  }
 
-	// Get the FPGA card plugin
-	auto fpgaCardFactory = plugin::registry->lookup<fpga::PCIeCardFactory>("pcie");
-	if (fpgaCardFactory == nullptr) {
-		logger->error("No FPGA plugin found");
-		exit(1);
-	}
+  fclose(f);
 
-	// Create all FPGA card instances using the corresponding plugin
-	auto cards = fpgaCardFactory->make(fpgas, pciDevices, vfioContainer, configDir);
+  // Create all FPGA card instances using the corresponding plugin
+  auto cards = std::list<std::shared_ptr<fpga::Card>>();
+  createCards(json, cards, configDir);
 
-	std::shared_ptr<fpga::PCIeCard> card;
-	for (auto &fpgaCard : cards) {
-		if (fpgaCard->name == fpgaName) {
-			return fpgaCard;
-		}
-	}
+  std::shared_ptr<fpga::Card> card = nullptr;
+  for (auto &fpgaCard : cards) {
+    if (fpgaCard->name == fpgaName) {
+      card = fpgaCard;
+      break;
+    }
+  }
+  // Deallocate JSON config
+  json_decref(json);
 
-	// Deallocate JSON config
-	json_decref(json);
+  if (!card)
+    throw RuntimeError("FPGA card {} not found in config or not working",
+                       fpgaName);
 
-	if (!card)
-		throw RuntimeError("FPGA card {} not found in config or not working", fpgaName);
-
-	return card;
+  return card;
 }
 
 std::unique_ptr<fpga::BufferedSampleFormatter> fpga::getBufferedSampleFormatter(