diff --git a/common/include/villas/graph/directed.hpp b/common/include/villas/graph/directed.hpp index 8667e3185..e24ae69bb 100644 --- a/common/include/villas/graph/directed.hpp +++ b/common/include/villas/graph/directed.hpp @@ -175,9 +175,9 @@ public: logger->debug("Remove edge {}", edgeId); // remove edge from global edge list it = edges.erase(it); - } else { - ++it; } + else + ++it; } logger->debug("Remove vertex {}", vertexId); @@ -200,10 +200,10 @@ public: Path &path, check_path_fn pathCheckFunc = checkPath) { - if (fromVertexId == toVertexId) { + if (fromVertexId == toVertexId) // arrived at the destination return true; - } else { + else { auto fromVertex = getVertex(fromVertexId); for (auto &edgeId : fromVertex->edges) { @@ -228,14 +228,12 @@ public: path.push_back(edgeId); // recursive, depth-first search - if (getPath(edgeOfFromVertex->to, toVertexId, path, pathCheckFunc) and - pathCheckFunc(path)) { + if (getPath(edgeOfFromVertex->to, toVertexId, path, pathCheckFunc) and pathCheckFunc(path)) // path found, we're done - return true; - } else { + return true; + else // tear down path that didn't lead to the destination path.pop_back(); - } } } diff --git a/common/include/villas/kernel/pci.hpp b/common/include/villas/kernel/pci.hpp index eac5554d0..3cea72473 100644 --- a/common/include/villas/kernel/pci.hpp +++ b/common/include/villas/kernel/pci.hpp @@ -83,22 +83,23 @@ struct Region { }; class Device { - public: Device(Id i, Slot s) : id(i), slot(s) { } + Device(Id i) : + id(i) + { } + + Device(Slot s) : + slot(s) + { } + bool operator==(const Device &other); - bool - operator==(const Id &other); - - bool - operator==(const Slot &other); - /** Get currently loaded driver for device */ std::string getDriver() const; @@ -118,7 +119,7 @@ public: Slot slot; }; -class DeviceList : public std::list { +class DeviceList : public std::list> { public: /** Initialize Linux PCI handle. * @@ -126,11 +127,14 @@ public: */ DeviceList(); - Device & - lookupDevice(const Slot &slot); + DeviceList::value_type + lookupDevice(const Slot &s); - Device & - lookupDevice(const Id &id); + DeviceList::value_type + lookupDevice(const Id &i); + + DeviceList::value_type + lookupDevice(const Device &f); }; } /* namespace pci */ diff --git a/common/lib/kernel/pci.cpp b/common/lib/kernel/pci.cpp index 78bdb3855..091b787d6 100644 --- a/common/lib/kernel/pci.cpp +++ b/common/lib/kernel/pci.cpp @@ -82,22 +82,34 @@ DeviceList::DeviceList() if (ret != 4) error("Failed to parse PCI slot number: %s", e->d_name); - emplace_back(id, slot); + emplace_back(std::make_shared(id, slot)); } closedir(dp); } -Device & +DeviceList::value_type DeviceList::lookupDevice(const Slot &s) { - return *std::find(begin(), end(), s); + return *std::find_if(begin(), end(), [s](const DeviceList::value_type &d) { + return d->slot == s; + }); } -Device & +DeviceList::value_type DeviceList::lookupDevice(const Id &i) { - return *std::find(begin(), end(), i); + return *std::find_if(begin(), end(), [i](const DeviceList::value_type &d) { + return d->id == i; + }); +} + +DeviceList::value_type +DeviceList::lookupDevice(const Device &d) +{ + return *std::find_if(begin(), end(), [d](const DeviceList::value_type &e) { + return *e == d; + }); } Id::Id(const std::string &str) : @@ -259,18 +271,6 @@ Device::operator==(const Device &f) return id == f.id && slot == f.slot; } -bool -Device::operator==(const Slot &s) -{ - return slot == s; -} - -bool -Device::operator==(const Id &i) -{ - return id == i; -} - std::list Device::getRegions() const { diff --git a/common/lib/kernel/vfio.cpp b/common/lib/kernel/vfio.cpp index d3a9a0446..a5e18ecde 100644 --- a/common/lib/kernel/vfio.cpp +++ b/common/lib/kernel/vfio.cpp @@ -78,7 +78,6 @@ namespace villas { Container::Container() : iova_next(0) { - Logger logger = logging.get("kernel:vfio"); static constexpr const char* requiredKernelModules[] = { @@ -86,49 +85,38 @@ Container::Container() }; for (const char* module : requiredKernelModules) { - if (kernel::module_load(module) != 0) { - logger->error("Kernel module '{}' required but could not be loaded. " + if (kernel::module_load(module) != 0) + throw RuntimeError("Kernel module '{}' required but could not be loaded. " "Please load manually!", module); - throw std::exception(); - } } /* Open VFIO API */ fd = open(VFIO_DEV, O_RDWR); - if (fd < 0) { - logger->error("Failed to open VFIO container"); - throw std::exception(); - } + if (fd < 0) + throw RuntimeError("Failed to open VFIO container"); /* Check VFIO API version */ version = ioctl(fd, VFIO_GET_API_VERSION); - if (version < 0 || version != VFIO_API_VERSION) { - logger->error("Failed to get VFIO version"); - throw std::exception(); - } + if (version < 0 || version != VFIO_API_VERSION) + throw RuntimeError("Failed to get VFIO version"); /* Check available VFIO extensions (IOMMU types) */ extensions = 0; for (unsigned int i = VFIO_TYPE1_IOMMU; i <= VFIO_NOIOMMU_IOMMU; i++) { int ret = ioctl(fd, VFIO_CHECK_EXTENSION, i); - if (ret < 0) { - logger->error("Failed to get VFIO extensions"); - throw std::exception(); - } - else if (ret > 0) { + if (ret < 0) + throw RuntimeError("Failed to get VFIO extensions"); + else if (ret > 0) extensions |= (1 << i); - } } hasIommu = false; if (not (extensions & (1 << VFIO_NOIOMMU_IOMMU))) { - if (not (extensions & (1 << VFIO_TYPE1_IOMMU))) { - logger->error("No supported IOMMU extension found"); - throw std::exception(); - } else { + if (not (extensions & (1 << VFIO_TYPE1_IOMMU))) + throw RuntimeError("No supported IOMMU extension found"); + else hasIommu = true; - } } logger->debug("Version: {:#x}", version); @@ -148,9 +136,8 @@ Container::~Container() /* Close container */ int ret = close(fd); - if (ret < 0) { + if (ret < 0) logger->error("Cannot close vfio container fd {}", fd); - } } @@ -158,6 +145,7 @@ std::shared_ptr Container::create() { std::shared_ptr container { new Container }; + return container; } @@ -227,19 +215,15 @@ Container::attachDevice(const char* name, int index) /* Open device fd */ device->fd = ioctl(group.fd, VFIO_GROUP_GET_DEVICE_FD, name); - if (device->fd < 0) { - logger->error("Failed to open VFIO device: {}", device->name); - throw std::exception(); - } + if (device->fd < 0) + throw RuntimeError("Failed to open VFIO device: {}", device->name); /* Get device info */ device->info.argsz = sizeof(device->info); int ret = ioctl(device->fd, VFIO_DEVICE_GET_INFO, &device->info); - if (ret < 0) { - logger->error("Failed to get VFIO device info for: {}", device->name); - throw std::exception(); - } + if (ret < 0) + throw RuntimeError("Failed to get VFIO device info for: {}", device->name); logger->debug("Device has {} regions", device->info.num_regions); logger->debug("Device has {} IRQs", device->info.num_irqs); @@ -258,10 +242,8 @@ Container::attachDevice(const char* name, int index) region.index = i; ret = ioctl(device->fd, VFIO_DEVICE_GET_REGION_INFO, ®ion); - if (ret < 0) { - logger->error("Failed to get region of VFIO device: {}", device->name); - throw std::exception(); - } + if (ret < 0) + throw RuntimeError("Failed to get region of VFIO device: {}", device->name); device->regions[i] = region; } @@ -276,10 +258,8 @@ Container::attachDevice(const char* name, int index) irq.index = i; ret = ioctl(device->fd, VFIO_DEVICE_GET_IRQ_INFO, &irq); - if (ret < 0) { - logger->error("Failed to get IRQs of VFIO device: {}", device->name); - throw std::exception(); - } + if (ret < 0) + throw RuntimeError("Failed to get IRQs of VFIO device: {}", device->name); device->irqs[i] = irq; } @@ -319,8 +299,7 @@ Container::attachDevice(const pci::Device &pdev) "(https://villas.fein-aachen.org/doc/fpga-setup.html) " "for help with troubleshooting."); - logger->error("Failed to get IOMMU group of device"); - throw std::exception(); + throw RuntimeError("Failed to get IOMMU group of device"); } /* VFIO device name consists of PCI BDF */ @@ -333,10 +312,8 @@ Container::attachDevice(const pci::Device &pdev) device.pci_device = &pdev; /* Check if this is really a vfio-pci device */ - if (not device.isVfioPciDevice()) { - logger->error("Device is not a vfio-pci device"); - throw std::exception(); - } + if (not device.isVfioPciDevice()) + throw RuntimeError("Device is not a vfio-pci device"); return device; } @@ -434,12 +411,10 @@ Container::getOrAttachGroup(int index) /* Group not yet part of this container, so acquire ownership */ auto group = Group::attach(*this, index); - if (not group) { - logger->error("Failed to attach to IOMMU group: {}", index); - throw std::exception(); - } else { + if (not group) + throw RuntimeError("Failed to attach to IOMMU group: {}", index); + else logger->debug("Attached new group {} to VFIO container", index); - } /* Push to our list */ groups.push_back(std::move(group)); @@ -764,18 +739,16 @@ Group::~Group() /* Release memory and close fds */ devices.clear(); - if (fd < 0) { + if (fd < 0) logger->debug("Destructing group that has not been attached"); - } else { + else { int ret = ioctl(fd, VFIO_GROUP_UNSET_CONTAINER); - if (ret != 0) { + if (ret != 0) logger->error("Cannot unset container for group fd {}", fd); - } ret = close(fd); - if (ret != 0) { + if (ret != 0) logger->error("Cannot close group fd {}", fd); - } } } diff --git a/common/lib/memory.cpp b/common/lib/memory.cpp index 6e92fd61d..081742e31 100644 --- a/common/lib/memory.cpp +++ b/common/lib/memory.cpp @@ -210,16 +210,15 @@ HostDmaRam::HostDmaRamAllocator::HostDmaRamAllocator(int num) : void* buf = mmap(nullptr, getSize(), PROT_READ|PROT_WRITE, MAP_SHARED, bufFd, 0); close(bufFd); - if (buf != MAP_FAILED) { + if (buf != MAP_FAILED) mm.createMapping(reinterpret_cast(buf), 0, getSize(), getName() + "-VA", mm.getProcessAddressSpace(), getAddrSpaceId()); - } else { + else logger->warn("Cannot map {}", bufPath); - } - } else { - logger->warn("Cannot open {}", bufPath); } + else + logger->warn("Cannot open {}", bufPath); logger->info("Mapped {} of size {} bytes", bufPath, getSize()); } diff --git a/common/lib/memory_manager.cpp b/common/lib/memory_manager.cpp index 060177938..aff256b72 100644 --- a/common/lib/memory_manager.cpp +++ b/common/lib/memory_manager.cpp @@ -237,18 +237,17 @@ MemoryTranslation::operator+=(const MemoryTranslation &other) /* The source stays the same and can only increase with merged translations */ //this->src = this->src; - if (otherSrcIsSmaller) { + if (otherSrcIsSmaller) /* Other mapping starts at lower addresses, so we actually arrive at * higher addresses */ this->dst += diff_lo; - } else { + else /* Other mapping starts at higher addresses than this, so we have to * increase the start * NOTE: for addresses equality, this just adds 0 */ this->src += diff_lo; - } logger->debug("result src: {:#x}", this->src); logger->debug("result dst: {:#x}", this->dst);