Merge pull request #75 from VILLASframework/fix/vfio-crashes

Fix/vfio crashes

fpga/.vscode/launch.json

@@ -8,9 +8,9 @@
             "name": "(gdb) Launch",
             "type": "cppdbg",
             "request": "launch",
-            "program": "${workspaceFolder}/build/src/villas-fpga-pipe",
+            "program": "${workspaceFolder}/build/src/villas-fpga-ctrl",
             "args": [
-                "-c", "${workspaceFolder}/etc/fpgas.json"
+                "-c", "${workspaceFolder}/etc/fpgas.json", "--connect", "\"2<->stdout\"", "--no-dma"
             ],
             "stopAtEntry": false,
             "cwd": "${workspaceFolder}",

fpga/common

@@ -1 +1 @@
-Subproject commit 51e32a4b9164b176da5ba3843b645ad6b49b3ec2
+Subproject commit 31b9f1dcf034b1747b66ac9d107d244108ede71f

fpga/include/villas/fpga/card.hpp

@@ -88,8 +88,8 @@ public:
 	std::shared_ptr<ip::Core>
 	lookupIp(const ip::IpIdentifier &id) const;
 
-	bool
-	mapMemoryBlock(const MemoryBlock &block);
+	bool mapMemoryBlock(const MemoryBlock &block);
+	bool unmapMemoryBlock(const MemoryBlock &block);
 
 private:
 	// Cache a set of already mapped memory blocks

fpga/include/villas/fpga/fpgaHelper.hpp

@@ -32,6 +32,10 @@ namespace fpga {
 std::shared_ptr<fpga::PCIeCard>
 setupFpgaCard(const std::string &configFile, const std::string &fpgaName);
 
+void configCrossBarUsingConnectString(std::string connectString,
+	std::shared_ptr<villas::fpga::ip::Dma> dma,
+	std::vector<std::shared_ptr<fpga::ip::AuroraXilinx>>& aurora_channels);
+
 void setupColorHandling();
 
 } /* namespace fpga */

fpga/include/villas/fpga/node.hpp

@@ -94,11 +94,6 @@ public:
 
 	friend class NodeFactory;
 
-	struct StreamPort {
-		int portNumber;
-		std::string nodeName;
-	};
-
 	const StreamVertex& getMasterPort(const std::string &name) const
 	{
 		return *portsMaster.at(name);

fpga/lib/card.cpp

@@ -167,6 +167,9 @@ std::list<std::shared_ptr<PCIeCard>> PCIeCardFactory::make(json_t *json, std::sh
 
 PCIeCard::~PCIeCard()
 {
+	// Ensure IP destructors are called before memory is unmapped
+	ips.clear();
+
 	auto &mm = MemoryManager::get();
 
 	// Unmap all memory blocks
@@ -215,6 +218,28 @@ std::shared_ptr<ip::Core> PCIeCard::lookupIp(const ip::IpIdentifier &id) const
 	return nullptr;
 }
 
+bool PCIeCard::unmapMemoryBlock(const MemoryBlock &block)
+{
+	if (memoryBlocksMapped.find(block.getAddrSpaceId()) == memoryBlocksMapped.end()) {
+		throw std::runtime_error("Block " + std::to_string(block.getAddrSpaceId()) + " is not mapped but was requested to be unmapped.");
+	}
+
+	auto &mm = MemoryManager::get();
+
+	auto translation = mm.getTranslation(addrSpaceIdDeviceToHost, block.getAddrSpaceId());
+
+	const uintptr_t iova = translation.getLocalAddr(0);
+	const size_t size = translation.getSize();
+
+	logger->debug("Unmap block {} at IOVA {:#x} of size {:#x}",
+			block.getAddrSpaceId(), iova, size);
+	vfioContainer->memoryUnmap(iova, size);
+
+	memoryBlocksMapped.erase(block.getAddrSpaceId());
+
+	return true;
+}
+
 bool PCIeCard::mapMemoryBlock(const MemoryBlock &block)
 {
 	if (not vfioContainer->isIommuEnabled()) {

fpga/lib/fpgaHelper.cpp

@@ -26,6 +26,7 @@
 #include <string>
 #include <algorithm>
 #include <jansson.h>
+#include <regex>
 
 #include <CLI11.hpp>
 #include <rang.hpp>
@@ -48,6 +49,64 @@ using namespace villas;
 static std::shared_ptr<kernel::pci::DeviceList> pciDevices;
 static auto logger = villas::logging.get("streamer");
 
+const std::shared_ptr<villas::fpga::ip::Node> portStringToStreamVertex(std::string &str,
+	std::shared_ptr<villas::fpga::ip::Dma> dma,
+	std::vector<std::shared_ptr<fpga::ip::AuroraXilinx>>& aurora_channels)
+{
+	if (str == "stdin" || str == "stdout") {
+		 return dma;
+	} else {
+		int port = std::stoi(str);
+
+		if (port > 7 || port < 0)
+			throw std::runtime_error("Invalid port number");
+
+		return aurora_channels[port];
+	}
+}
+// parses a string lik "1->2" or "1<->stdout" and configures the crossbar
+void fpga::configCrossBarUsingConnectString(std::string connectString,
+	std::shared_ptr<villas::fpga::ip::Dma> dma,
+	std::vector<std::shared_ptr<fpga::ip::AuroraXilinx>>& aurora_channels)
+{
+	bool bidirectional = false;
+	bool invert = false;
+
+	if (connectString.empty())
+		return;
+
+	if (connectString == "loopback") {
+		logger->info("Connecting loopback");
+		// is this working?
+		dma->connectLoopback();
+	}
+
+	static std::regex re("([0-9]+)([<\\->]+)([0-9]+|stdin|stdout)");
+	std::smatch match;
+
+	if (!std::regex_match(connectString, match, re)) {
+		logger->error("Invalid connect string: {}", connectString);
+		throw std::runtime_error("Invalid connect string");
+	}
+	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]);
+	logger->info("Connect string {}: Connecting {} to {}, {}directional",
+		connectString, srcStr, dstStr,
+		(bidirectional ? "bi" : "uni"));
+	auto src = portStringToStreamVertex(srcStr, dma, aurora_channels);
+	auto dest = portStringToStreamVertex(dstStr, dma, aurora_channels);
+	src->connect(src->getDefaultMasterPort(), dest->getDefaultSlavePort());
+	if (bidirectional) {
+		dest->connect(dest->getDefaultMasterPort(), src->getDefaultSlavePort());
+	}
+}
+
 void fpga::setupColorHandling()
 {
 	// Handle Control-C nicely

fpga/lib/ips/dma.cpp

@@ -191,22 +191,26 @@ bool Dma::reset()
 	int timeout = 500;
 
 	while (timeout > 0) {
-		if (XAxiDma_ResetIsDone(&xDma))
+		if (XAxiDma_ResetIsDone(&xDma)) {
+			logger->info("DMA has been reset.");
 			return true;
+		}
 
 		timeout--;
 	}
-	if (timeout == 0) {
-		logger->error("DMA reset timed out");
-	} else {
-		logger->info("DMA has been reset.");
-	}
+	logger->error("DMA reset timed out");
 
 	return false;
 }
 
 Dma::~Dma()
 {
+	// Unmap memory in our ownership, MemoryBlock gets deleted and removed from
+	// graph by this destructor as well.
+	if (hasScatterGather()) {
+		card->unmapMemoryBlock(*sgRingTx);
+		card->unmapMemoryBlock(*sgRingRx);
+	}
 	Dma::reset();
 }
 

fpga/src/CMakeLists.txt

@@ -20,18 +20,13 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 ##############################################################################
 
-add_executable(villas-fpga-pipe villas-fpga-pipe.cpp)
-add_executable(villas-fpga-cat villas-fpga-cat.cpp)
-add_executable(villas-fpga-xbar-select villas-fpga-xbar-select.cpp)
+add_executable(villas-fpga-ctrl villas-fpga-ctrl.cpp)
 
-target_link_libraries(villas-fpga-pipe PUBLIC
-	villas-fpga
-)
-target_link_libraries(villas-fpga-cat PUBLIC
-	villas-fpga
-)
-target_link_libraries(villas-fpga-xbar-select PUBLIC
+target_link_libraries(villas-fpga-ctrl PUBLIC
 	villas-fpga
 )
 
 add_executable(pcimem pcimem.c)
+
+configure_file(villas-fpga-cat.sh villas-fpga-cat.sh COPYONLY)
+configure_file(villas-fpga-xbar-select.sh villas-fpga-xbar-select.sh COPYONLY)

fpga/src/villas-fpga-cat.sh

@@ -0,0 +1,18 @@
+#!/bin/bash
+# Copyright 2023 Niklas Eiling <niklas.eiling@rwth-aachen.de>
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+CWD=$(dirname -- "$0")
+
+${CWD}/villas-fpga-ctrl -c ${CWD}/../../etc/fpgas.json --connect "2<->stdout"

fpga/src/villas-fpga-ctrl.cpp

@@ -1,7 +1,9 @@
 /** Streaming data from STDIN/OUT to FPGA.
  *
  * @author Daniel Krebs <github@daniel-krebs.net>
+ * @author Niklas Eiling <niklas.eiling@rwth-aachen.de>
  * @copyright 2017-2022, Steffen Vogel
+ * @copyright 2022-2023, Niklas Eiling
  * @license GNU General Public License (version 3)
  *
  * VILLASfpga
@@ -48,6 +50,8 @@ using namespace villas;
 static std::shared_ptr<kernel::pci::DeviceList> pciDevices;
 static auto logger = villas::logging.get("cat");
 
+
+
 int main(int argc, char* argv[])
 {
 	// Command Line Parser
@@ -60,6 +64,11 @@ int main(int argc, char* argv[])
 
 		std::string fpgaName = "vc707";
 		app.add_option("--fpga", fpgaName, "Which FPGA to use");
+		std::string connectStr = "";
+		app.add_option("-x,--connect", connectStr, "Connect a FPGA port with another or stdin/stdout");
+		bool noDma = false;
+		app.add_flag("--no-dma", noDma, "Do not setup DMA, only setup FPGA and Crossbar links");
+
 		app.parse(argc, argv);
 
 		// Logging setup
@@ -105,50 +114,48 @@ int main(int argc, char* argv[])
 			aurora->dump();
 
 		// Configure Crossbar switch
-#if 1
-		aurora_channels[2]->connect(aurora_channels[2]->getDefaultMasterPort(), dma->getDefaultSlavePort());
-		dma->connect(dma->getDefaultMasterPort(), aurora_channels[2]->getDefaultSlavePort());
-#else
-		dma->connectLoopback();
-#endif
-		for (auto b : block) {
-			dma->makeAccesibleFromVA(b);
-		}
+		fpga::configCrossBarUsingConnectString(connectStr, dma, aurora_channels);
 
-		auto &mm = MemoryManager::get();
-		mm.getGraph().dump("graph.dot");
-
-		// Setup read transfer
-		dma->read(block[0], block[0].getSize());
-		size_t cur = 0, next = 1;
-		while (true) {
-			dma->read(block[next], block[next].getSize());
-			auto bytesRead = dma->readComplete();
-			// Setup read transfer
-
-			//auto valuesRead = bytesRead / sizeof(int32_t);
-			//logger->info("Read {} bytes", bytesRead);
-
-			//for (size_t i = 0; i < valuesRead; i++)
-			//	std::cerr << std::hex << mem[i] << ";";
-			//std::cerr << std::endl;
-
-			for (size_t i = 0; i*4 < bytesRead; i++) {
-				int32_t ival = mem[cur][i];
-				float fval = *((float*)(&ival)); // cppcheck-suppress invalidPointerCast
-				//std::cerr << std::hex << ival << ",";
-				std::cerr << fval << std::endl;
-				/*int64_t ival = (int64_t)(mem[1] & 0xFFFF) << 48 |
-					 (int64_t)(mem[1] & 0xFFFF0000) << 16 |
-					 (int64_t)(mem[0] & 0xFFFF) << 16 |
-					 (int64_t)(mem[0] & 0xFFFF0000) >> 16;
-				double dval = *((double*)(&ival));
-				std::cerr << std::hex << ival << "," << dval << std::endl;
-				bytesRead -= 8;*/
-				//logger->info("Read value: {}", dval);
+		if (!noDma) {
+			for (auto b : block) {
+				dma->makeAccesibleFromVA(b);
+			}
+
+			auto &mm = MemoryManager::get();
+			mm.getGraph().dump("graph.dot");
+
+			// Setup read transfer
+			dma->read(block[0], block[0].getSize());
+			size_t cur = 0, next = 1;
+			while (true) {
+				dma->read(block[next], block[next].getSize());
+				auto bytesRead = dma->readComplete();
+				// Setup read transfer
+
+				//auto valuesRead = bytesRead / sizeof(int32_t);
+				//logger->info("Read {} bytes", bytesRead);
+
+				//for (size_t i = 0; i < valuesRead; i++)
+				//	std::cerr << std::hex << mem[i] << ";";
+				//std::cerr << std::endl;
+
+				for (size_t i = 0; i*4 < bytesRead; i++) {
+					int32_t ival = mem[cur][i];
+					float fval = *((float*)(&ival)); // cppcheck-suppress invalidPointerCast
+					//std::cerr << std::hex << ival << ",";
+					std::cerr << fval << std::endl;
+					/*int64_t ival = (int64_t)(mem[1] & 0xFFFF) << 48 |
+						(int64_t)(mem[1] & 0xFFFF0000) << 16 |
+						(int64_t)(mem[0] & 0xFFFF) << 16 |
+						(int64_t)(mem[0] & 0xFFFF0000) >> 16;
+					double dval = *((double*)(&ival));
+					std::cerr << std::hex << ival << "," << dval << std::endl;
+					bytesRead -= 8;*/
+					//logger->info("Read value: {}", dval);
+				}
+				cur = next;
+				next = (next + 1) % (sizeof(mem)/sizeof(mem[0]));
 			}
-			cur = next;
-			next = (next + 1) % (sizeof(mem)/sizeof(mem[0]));
 		}
 	} catch (const RuntimeError &e) {
 		logger->error("Error: {}", e.what());

fpga/src/villas-fpga-xbar-select.cpp

@@ -1,118 +0,0 @@
-/** Streaming data from STDIN/OUT to FPGA.
- *
- * @author Daniel Krebs <github@daniel-krebs.net>
- * @copyright 2017-2022, Steffen Vogel
- * @license GNU General Public License (version 3)
- *
- * VILLASfpga
- *
- * This program is free software: you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program.  If not, see <http://www.gnu.org/licenses/>.
- *********************************************************************************/
-
-#include <csignal>
-#include <iostream>
-#include <vector>
-#include <string>
-#include <algorithm>
-#include <jansson.h>
-
-#include <CLI11.hpp>
-#include <rang.hpp>
-
-#include <villas/exceptions.hpp>
-#include <villas/log.hpp>
-#include <villas/utils.hpp>
-#include <villas/utils.hpp>
-
-#include <villas/fpga/core.hpp>
-#include <villas/fpga/card.hpp>
-#include <villas/fpga/vlnv.hpp>
-#include <villas/fpga/ips/dma.hpp>
-#include <villas/fpga/ips/rtds.hpp>
-#include <villas/fpga/ips/aurora_xilinx.hpp>
-#include <villas/fpga/fpgaHelper.hpp>
-
-using namespace villas;
-
-static std::shared_ptr<kernel::pci::DeviceList> pciDevices;
-static auto logger = villas::logging.get("cat");
-
-int main(int argc, char* argv[])
-{
-	// Command Line Parser
-	CLI::App app{"VILLASfpga xbar select"};
-
-	try {
-		std::string configFile;
-		app.add_option("-c,--config", configFile, "Configuration file")
-				->check(CLI::ExistingFile);
-
-		std::string fpgaName = "vc707";
-		app.add_option("--fpga", fpgaName, "Which FPGA to use");
-		app.parse(argc, argv);
-
-		// Logging setup
-		spdlog::set_level(spdlog::level::debug);
-		fpga::setupColorHandling();
-
-		if (configFile.empty()) {
-			logger->error("No configuration file provided/ Please use -c/--config argument");
-			return 1;
-		}
-
-		auto card = fpga::setupFpgaCard(configFile, fpgaName);
-
-		std::vector<std::shared_ptr<fpga::ip::AuroraXilinx>> aurora_channels;
-		for (int i = 0; i < 4; i++) {
-			auto name = fmt::format("aurora_8b10b_ch{}", i);
-			auto id = fpga::ip::IpIdentifier("xilinx.com:ip:aurora_8b10b:", name);
-			auto aurora = std::dynamic_pointer_cast<fpga::ip::AuroraXilinx>(card->lookupIp(id));
-			if (aurora == nullptr) {
-				logger->error("No Aurora interface found on FPGA");
-				return 1;
-			}
-
-			aurora_channels.push_back(aurora);
-		}
-
-		auto dma = std::dynamic_pointer_cast<fpga::ip::Dma>
-					(card->lookupIp(fpga::Vlnv("xilinx.com:ip:axi_dma:")));
-		if (dma == nullptr) {
-			logger->error("No DMA found on FPGA ");
-			return 1;
-		}
-
-		for (auto aurora : aurora_channels)
-			aurora->dump();
-
-		// Configure Crossbar switch
-		// connect DINO to RTDS
-		//aurora_channels[0]->connect(aurora_channels[0]->getDefaultMasterPort(), aurora_channels[2]->getDefaultSlavePort());
-		//aurora_channels[2]->connect(aurora_channels[2]->getDefaultMasterPort(), aurora_channels[0]->getDefaultSlavePort());
-		// connect DINO to OPAL-RT
-		//aurora_channels[1]->connect(aurora_channels[1]->getDefaultMasterPort(), aurora_channels[2]->getDefaultSlavePort());
-		//aurora_channels[2]->connect(aurora_channels[2]->getDefaultMasterPort(), aurora_channels[1]->getDefaultSlavePort());
-		// connect OPAL-RT to RTDS
-		aurora_channels[0]->connect(aurora_channels[0]->getDefaultMasterPort(), aurora_channels[1]->getDefaultSlavePort());
-		aurora_channels[1]->connect(aurora_channels[1]->getDefaultMasterPort(), aurora_channels[0]->getDefaultSlavePort());
-
-	} catch (const RuntimeError &e) {
-		logger->error("Error: {}", e.what());
-		return -1;
-	} catch (const CLI::ParseError &e) {
-		return app.exit(e);
-	}
-
-	return 0;
-}

fpga/src/villas-fpga-xbar-select.sh

@@ -0,0 +1,19 @@
+#!/bin/bash
+# Copyright 2023 Niklas Eiling <niklas.eiling@rwth-aachen.de>
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+CWD=$(dirname -- "$0")
+
+# currently connects RTDS and OPAL-RT
+${CWD}/villas-fpga-ctrl -c ${CWD}/../../etc/fpgas.json --connect "0<->1" --no-dma