From ecfb9a7897dcf0651c3115f7a883c1f3082668f9 Mon Sep 17 00:00:00 2001 From: IgnoreWarnings <119685519+IgnoreWarnings@users.noreply.github.com> Date: Fri, 8 Sep 2023 10:12:12 +0200 Subject: [PATCH] Kernel module loading system and bug fixes of vfio (#110) - feature: Header to specify which kernel modules (vfio support) to load - Bug fixes: wrong errror messages, bugged region cap - Quality of use improvements e.g. getter and str() --------- Signed-off-by: IgnoreWarnings Signed-off-by: IgnoreWarnings <119685519+IgnoreWarnings@users.noreply.github.com> Co-authored-by: Steffen Vogel --- common/include/villas/kernel/vfio.hpp | 34 ++++++++++++++++++++ common/include/villas/kernel/vfio_device.hpp | 2 ++ common/include/villas/memory_manager.hpp | 7 ++++ common/lib/kernel/vfio_container.cpp | 10 ++---- common/lib/kernel/vfio_device.cpp | 14 ++++---- 5 files changed, 54 insertions(+), 13 deletions(-) create mode 100644 common/include/villas/kernel/vfio.hpp diff --git a/common/include/villas/kernel/vfio.hpp b/common/include/villas/kernel/vfio.hpp new file mode 100644 index 000000000..0b59fcf43 --- /dev/null +++ b/common/include/villas/kernel/vfio.hpp @@ -0,0 +1,34 @@ +/** VFIO Kernel module loader + * + * Author: Pascal Bauer + * SPDX-FileCopyrightText: 2017 Institute for Automation of Complex Power Systems, RWTH Aachen University + * SPDX-License-Identifier: Apache-2.0 + *********************************************************************************/ + +//TODO: Move to cmake args +//#define FPGA_PLATFORM +#define FPGA_PCI + +#if defined(FPGA_PLATFORM) + #define KERNEL_MODULE_VFIO +#endif + +#if defined(FPGA_PCI) + #define KERNEL_MODULE_VFIO + #define KERNEL_MODULE_VFIO_PCI + #define KERNEL_MODULE_VFIO_IOMMU_TYPE1 +#endif + +static constexpr const char* const requiredKernelModules[] = { +#if defined(KERNEL_MODULE_VFIO) + "vfio", +#endif // KERNEL_MODULE_VFIO_PCI + +#if defined(KERNEL_MODULE_VFIO_PCI) + "vfio_pci", +#endif // KERNEL_MODULE_VFIO_PCI + +#if defined(KERNEL_MODULE_VFIO_IOMMU_TYPE1) + "vfio_iommu_type1" +#endif // KERNEL_MODULE_VFIO_IOMMU_TYPE1 +}; \ No newline at end of file diff --git a/common/include/villas/kernel/vfio_device.hpp b/common/include/villas/kernel/vfio_device.hpp index 03a2fcada..d396a9940 100644 --- a/common/include/villas/kernel/vfio_device.hpp +++ b/common/include/villas/kernel/vfio_device.hpp @@ -37,6 +37,8 @@ public: bool reset(); + std::string getName() { return name; }; + // Map a device memory region to the application address space (e.g. PCI BARs) void* regionMap(size_t index); diff --git a/common/include/villas/memory_manager.hpp b/common/include/villas/memory_manager.hpp index 9592b9334..01a11b60c 100644 --- a/common/include/villas/memory_manager.hpp +++ b/common/include/villas/memory_manager.hpp @@ -120,6 +120,13 @@ private: << ", size=0x" << mapping.size << ")"; } + + std::string toString() + { + std::stringstream s; + s << *this; + return s.str(); + } }; // Custom vertex in memory graph representing an address space diff --git a/common/lib/kernel/vfio_container.cpp b/common/lib/kernel/vfio_container.cpp index f0259b9ac..b813f99a6 100644 --- a/common/lib/kernel/vfio_container.cpp +++ b/common/lib/kernel/vfio_container.cpp @@ -32,6 +32,7 @@ #include #include +#include #include #include @@ -74,15 +75,10 @@ Container::Container() : groups(), log(logging.get("kernel:vfio::Container")) { - static constexpr - const char* requiredKernelModules[] = { - "vfio", "vfio_pci", "vfio_iommu_type1" - }; - for (const char* module : requiredKernelModules) { if (kernel::loadModule(module) != 0) { - throw RuntimeError("Kernel module '{}' required but could not be loaded. " - "Please load manually!", module); + throw RuntimeError("Kernel module '{}' required but could not be loaded. " + "Please load manually!", module); } } diff --git a/common/lib/kernel/vfio_device.cpp b/common/lib/kernel/vfio_device.cpp index b8f7ecd54..e476e227f 100644 --- a/common/lib/kernel/vfio_device.cpp +++ b/common/lib/kernel/vfio_device.cpp @@ -84,10 +84,12 @@ Device::Device(const std::string &name, int groupFileDescriptor, const kernel::p log->debug("device info: flags: 0x{:x}, num_regions: {}, num_irqs: {}", info.flags, info.num_regions, info.num_irqs); - // device_info.num_region reports always 9 and includes a VGA region, which is only supported on - // certain device IDs. So for non-VGA devices VFIO_PCI_CONFIG_REGION_INDEX will be the highest - // region index. This is the config space. - info.num_regions = VFIO_PCI_CONFIG_REGION_INDEX + 1; + if (pci_device != 0) { + // device_info.num_region reports always 9 and includes a VGA region, which is only supported on + // certain device IDs. So for non-VGA devices VFIO_PCI_CONFIG_REGION_INDEX will be the highest + // region index. This is the config space. + info.num_regions = VFIO_PCI_CONFIG_REGION_INDEX + 1; + } else { info.num_regions = 1; } // Reserve slots already so that we can use the []-operator for access irqs.resize(info.num_irqs); @@ -95,7 +97,7 @@ Device::Device(const std::string &name, int groupFileDescriptor, const kernel::p mappings.resize(info.num_regions); // Get device regions - for (size_t i = 0; i < info.num_regions && i < 8; i++) { + for (size_t i = 0; i < info.num_regions; i++) { struct vfio_region_info region; memset(®ion, 0, sizeof (region)); @@ -106,7 +108,7 @@ Device::Device(const std::string &name, int groupFileDescriptor, const kernel::p if (ret < 0) throw RuntimeError("Failed to get region {} of VFIO device: {}", i, name); - log->debug("region {} info: flags: 0x{:x}, cap_offset: 0x{:x}, size: 0x{:x}, offset: 0x{:x}", + log->debug("region {} info: flags: 0x{:x}, cap_offset: 0x{:x}, size: 0x{:x}, offset: 0x{:x}", region.index, region.flags, region.cap_offset, region.size, region.offset); regions[i] = region;