From b6ff452e53c639de9b6aea520c85074a829a89ab Mon Sep 17 00:00:00 2001
From: Daniel Krebs <github@daniel-krebs.net>
Date: Tue, 15 May 2018 11:23:48 +0200
Subject: [PATCH] vfio: minor refactoring

---
 fpga/include/villas/kernel/vfio.hpp |  8 ++++++--
 fpga/lib/kernel/vfio.cpp            | 26 +++++++++++++++-----------
 2 files changed, 21 insertions(+), 13 deletions(-)

diff --git a/fpga/include/villas/kernel/vfio.hpp b/fpga/include/villas/kernel/vfio.hpp
index 0e3fc453f..1a09e2e16 100644
--- a/fpga/include/villas/kernel/vfio.hpp
+++ b/fpga/include/villas/kernel/vfio.hpp
@@ -18,7 +18,8 @@
 #include <linux/vfio.h>
 #include <sys/mman.h>
 
-#define VFIO_DEV(x)	"/dev/vfio/" x
+#define VFIO_PATH	"/dev/vfio/"
+#define VFIO_DEV	VFIO_PATH "vfio"
 
 /* Forward declarations */
 struct pci_device;
@@ -90,7 +91,7 @@ public:
 	~VfioGroup();
 
 	static std::unique_ptr<VfioGroup>
-	attach(int containerFd, int groupIndex);
+	attach(const VfioContainer& container, int groupIndex);
 
 private:
 	/// VFIO group file descriptor
@@ -138,6 +139,9 @@ public:
 	bool isIommuEnabled() const
 	{ return this->hasIommu; }
 
+	const int& getFd() const
+	{ return fd; }
+
 private:
 	VfioGroup& getOrAttachGroup(int index);
 
diff --git a/fpga/lib/kernel/vfio.cpp b/fpga/lib/kernel/vfio.cpp
index 7d29c9ee8..67cc6d19e 100644
--- a/fpga/lib/kernel/vfio.cpp
+++ b/fpga/lib/kernel/vfio.cpp
@@ -87,7 +87,7 @@ VfioContainer::VfioContainer()
 	}
 
 	/* Open VFIO API */
-	fd = open(VFIO_DEV("vfio"), O_RDWR);
+	fd = open(VFIO_DEV, O_RDWR);
 	if (fd < 0) {
 		logger->error("Failed to open VFIO container");
 		throw std::exception();
@@ -412,7 +412,7 @@ VfioContainer::getOrAttachGroup(int index)
 	}
 
 	// group not yet part of this container, so acquire ownership
-	auto group = VfioGroup::attach(fd, index);
+	auto group = VfioGroup::attach(*this, index);
 	if(not group) {
 		logger->error("Failed to attach to IOMMU group: {}", index);
 		throw std::exception();
@@ -538,6 +538,7 @@ VfioDevice::pciHotReset()
 
 	const size_t reset_infolen = sizeof(struct vfio_pci_hot_reset_info) +
 	                             sizeof(struct vfio_pci_dependent_device) * 64;
+
 	auto reset_info = reinterpret_cast<struct vfio_pci_hot_reset_info*>
 	                    (calloc(1, reset_infolen));
 
@@ -562,6 +563,8 @@ VfioDevice::pciHotReset()
 		}
 	}
 
+	free(reset_info);
+
 	const size_t resetlen = sizeof(struct vfio_pci_hot_reset) +
 	                        sizeof(int32_t) * 1;
 	auto reset = reinterpret_cast<struct vfio_pci_hot_reset*>
@@ -571,10 +574,10 @@ VfioDevice::pciHotReset()
 	reset->count = 1;
 	reset->group_fds[0] = this->group.fd;
 
-	const bool success = ioctl(this->fd, VFIO_DEVICE_PCI_HOT_RESET, reset) == 0;
+	int ret = ioctl(this->fd, VFIO_DEVICE_PCI_HOT_RESET, reset);
+	const bool success = (ret == 0);
 
 	free(reset);
-	free(reset_info);
 
 	if(not success and not group.container->isIommuEnabled()) {
 		logger->info("PCI hot reset failed, but this is expected without IOMMU");
@@ -740,13 +743,16 @@ VfioGroup::~VfioGroup()
 
 
 std::unique_ptr<VfioGroup>
-VfioGroup::attach(int containerFd, int groupIndex)
+VfioGroup::attach(const VfioContainer& container, int groupIndex)
 {
 	std::unique_ptr<VfioGroup> group { new VfioGroup(groupIndex) };
 
 	/* Open group fd */
 	std::stringstream groupPath;
-	groupPath << VFIO_DEV("") << groupIndex;
+	groupPath << VFIO_PATH
+	          << (container.isIommuEnabled() ? "" : "noiommu-")
+	          << groupIndex;
+
 	group->fd = open(groupPath.str().c_str(), O_RDWR);
 	if (group->fd < 0) {
 		logger->error("Failed to open VFIO group {}", group->index);
@@ -756,13 +762,11 @@ VfioGroup::attach(int containerFd, int groupIndex)
 	logger->debug("VFIO group {} (fd {}) has path {}",
 	              groupIndex, group->fd, groupPath.str());
 
-	int ret;
-
 	/* Claim group ownership */
-	ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &containerFd);
+	int ret = ioctl(group->fd, VFIO_GROUP_SET_CONTAINER, &container.getFd());
 	if (ret < 0) {
 		logger->error("Failed to attach VFIO group {} to container fd {} (error {})",
-		              group->index, containerFd, ret);
+		              group->index, container.getFd(), ret);
 		return nullptr;
 	}
 
@@ -771,7 +775,7 @@ VfioGroup::attach(int containerFd, int groupIndex)
 	                     VFIO_TYPE1_IOMMU :
 	                     VFIO_NOIOMMU_IOMMU;
 
-	ret = ioctl(containerFd, VFIO_SET_IOMMU, iommu_type);
+	ret = ioctl(container.getFd(), VFIO_SET_IOMMU, iommu_type);
 	if (ret < 0) {
 		logger->error("Failed to set IOMMU type of container: {}", ret);
 		return nullptr;