Re: [libvirt] [PATCH 1/4 v3] pci: Move some pci sriov helper code out of node device driver to util/pci

From: Roopa Prabhu&lt;roprabhu(a)cisco.com&gt; This patch moves some of the sriov related pci code from node_device driver to src/util/pci.[ch]. Some functions had to go thru name and argument list change to accommodate the move. Signed-off-by: Roopa Prabhu&lt;roprabhu(a)cisco.com&gt; Signed-off-by: Christian Benvenuti&lt;benve(a)cisco.com&gt; Signed-off-by: David Wang&lt;dwang2(a)cisco.com&gt; --- src/Makefile.am | 5 + src/conf/node_device_conf.c | 1 src/conf/node_device_conf.h | 7 - src/node_device/node_device_driver.h | 14 -- src/node_device/node_device_hal.c | 10 + src/node_device/node_device_linux_sysfs.c | 191 ------------------------ src/node_device/node_device_udev.c | 10 + src/util/pci.c | 230 +++++++++++++++++++++++++++++ src/util/pci.h | 14 ++ 9 files changed, 265 insertions(+), 217 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index cf7c003..8fe7120 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -954,7 +954,10 @@ libvirt_driver_nodedev_la_SOURCES = $(NODE_DEVICE_DRIVER_SOURCES) libvirt_driver_nodedev_la_CFLAGS = \ -I@top_srcdir@/src/conf $(AM_CFLAGS) libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_nodedev_la_LIBADD = +libvirt_driver_nodedev_la_LIBADD = \ + libvirt_util.la \ + ../gnulib/lib/libgnu.la + if HAVE_HAL libvirt_driver_nodedev_la_SOURCES += $(NODE_DEVICE_DRIVER_HAL_SOURCES) libvirt_driver_nodedev_la_CFLAGS += $(HAL_CFLAGS) diff --git a/src/conf/node_device_conf.c b/src/conf/node_device_conf.c index dde2921..548bbff 100644 --- a/src/conf/node_device_conf.c +++ b/src/conf/node_device_conf.c @@ -36,6 +36,7 @@ #include "util.h" #include "buf.h" #include "uuid.h" +#include "pci.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV diff --git a/src/conf/node_device_conf.h b/src/conf/node_device_conf.h index cef86d4..17be031 100644 --- a/src/conf/node_device_conf.h +++ b/src/conf/node_device_conf.h @@ -82,13 +82,6 @@ enum virNodeDevPCICapFlags { VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION = (1<< 1), }; -struct pci_config_address { - unsigned int domain; - unsigned int bus; - unsigned int slot; - unsigned int function; -}; - typedef struct _virNodeDevCapsDef virNodeDevCapsDef; typedef virNodeDevCapsDef *virNodeDevCapsDefPtr; struct _virNodeDevCapsDef { diff --git a/src/node_device/node_device_driver.h b/src/node_device/node_device_driver.h index 08779b1..673e95b 100644 --- a/src/node_device/node_device_driver.h +++ b/src/node_device/node_device_driver.h @@ -37,10 +37,6 @@ # define LINUX_SYSFS_VPORT_CREATE_POSTFIX "/vport_create" # define LINUX_SYSFS_VPORT_DELETE_POSTFIX "/vport_delete" -# define SRIOV_FOUND 0 -# define SRIOV_NOT_FOUND 1 -# define SRIOV_ERROR -1 - # define LINUX_NEW_DEVICE_WAIT_TIME 60 # ifdef HAVE_HAL @@ -63,14 +59,6 @@ int check_fc_host_linux(union _virNodeDevCapData *d); # define check_vport_capable(d) check_vport_capable_linux(d) int check_vport_capable_linux(union _virNodeDevCapData *d); -# define get_physical_function(s,d) get_physical_function_linux(s,d) -int get_physical_function_linux(const char *sysfs_path, - union _virNodeDevCapData *d); - -# define get_virtual_functions(s,d) get_virtual_functions_linux(s,d) -int get_virtual_functions_linux(const char *sysfs_path, - union _virNodeDevCapData *d); - # define read_wwn(host, file, wwn) read_wwn_linux(host, file, wwn) int read_wwn_linux(int host, const char *file, char **wwn); @@ -78,8 +66,6 @@ int read_wwn_linux(int host, const char *file, char **wwn); # define check_fc_host(d) (-1) # define check_vport_capable(d) (-1) -# define get_physical_function(sysfs_path, d) -# define get_virtual_functions(sysfs_path, d) # define read_wwn(host, file, wwn) # endif /* __linux__ */ diff --git a/src/node_device/node_device_hal.c b/src/node_device/node_device_hal.c index 421f5ad..481be97 100644 --- a/src/node_device/node_device_hal.c +++ b/src/node_device/node_device_hal.c @@ -35,6 +35,7 @@ #include "datatypes.h" #include "memory.h" #include "uuid.h" +#include "pci.h" #include "logging.h" #include "node_device_driver.h" @@ -146,8 +147,13 @@ static int gather_pci_cap(LibHalContext *ctx, const char *udi, (void)virStrToLong_ui(p+1,&p, 16,&d->pci_dev.function); } - get_physical_function(sysfs_path, d); - get_virtual_functions(sysfs_path, d); + if (!pciGetPhysicalFunction(sysfs_path,&d->pci_dev.physical_function)) + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + if (!pciGetVirtualFunctions(sysfs_path,&d->pci_dev.virtual_functions, +&d->pci_dev.num_virtual_functions) || + d->pci_dev.num_virtual_functions> 0) + d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; VIR_FREE(sysfs_path); } diff --git a/src/node_device/node_device_linux_sysfs.c b/src/node_device/node_device_linux_sysfs.c index f9ff20f..844231a 100644 --- a/src/node_device/node_device_linux_sysfs.c +++ b/src/node_device/node_device_linux_sysfs.c @@ -205,195 +205,4 @@ out: return retval; } - -static int logStrToLong_ui(char const *s, - char **end_ptr, - int base, - unsigned int *result) -{ - int ret = 0; - - ret = virStrToLong_ui(s, end_ptr, base, result); - if (ret != 0) { - VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s); - } else { - VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result); - } - - return ret; -} - - -static int parse_pci_config_address(char *address, struct pci_config_address *bdf) -{ - char *p = NULL; - int ret = -1; - - if ((address == NULL) || (logStrToLong_ui(address,&p, 16, -&bdf->domain) == -1)) { - goto out; - } - - if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, -&bdf->bus) == -1)) { - goto out; - } - - if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, -&bdf->slot) == -1)) { - goto out; - } - - if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, -&bdf->function) == -1)) { - goto out; - } - - ret = 0; - -out: - return ret; -} - - - - -static int get_sriov_function(const char *device_link, - struct pci_config_address **bdf) -{ - char *config_address = NULL; - char *device_path = NULL; - char errbuf[64]; - int ret = SRIOV_ERROR; - - VIR_DEBUG("Attempting to resolve device path from device link '%s'", - device_link); - - if (!virFileExists(device_link)) { - - VIR_DEBUG("SR IOV function link '%s' does not exist", device_link); - /* Not an SR IOV device, not an error, either. */ - ret = SRIOV_NOT_FOUND; - - goto out; - - } - - device_path = canonicalize_file_name (device_link); - if (device_path == NULL) { - memset(errbuf, '\0', sizeof(errbuf)); - VIR_ERROR(_("Failed to resolve device link '%s': '%s'"), device_link, - virStrerror(errno, errbuf, sizeof(errbuf))); - goto out; - } - - VIR_DEBUG("SR IOV device path is '%s'", device_path); - config_address = basename(device_path); - if (VIR_ALLOC(*bdf) != 0) { - VIR_ERROR(_("Failed to allocate memory for PCI device name")); - goto out; - } - - if (parse_pci_config_address(config_address, *bdf) != 0) { - VIR_ERROR(_("Failed to parse PCI config address '%s'"), config_address); - goto out; - } - - VIR_DEBUG("SR IOV function %.4x:%.2x:%.2x.%.1x", - (*bdf)->domain, - (*bdf)->bus, - (*bdf)->slot, - (*bdf)->function); - - ret = SRIOV_FOUND; - -out: - VIR_FREE(device_path); - return ret; -} - - -int get_physical_function_linux(const char *sysfs_path, - union _virNodeDevCapData *d ATTRIBUTE_UNUSED) -{ - int ret = -1; - char *device_link = NULL; - - VIR_DEBUG("Attempting to get SR IOV physical function for device " - "with sysfs path '%s'", sysfs_path); - - if (virBuildPath(&device_link, sysfs_path, "physfn") == -1) { - virReportOOMError(); - } else { - ret = get_sriov_function(device_link,&d->pci_dev.physical_function); - if (ret == SRIOV_FOUND) { - d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; - } - } - - VIR_FREE(device_link); - return ret; -} - - -int get_virtual_functions_linux(const char *sysfs_path, - union _virNodeDevCapData *d) -{ - int ret = -1; - DIR *dir = NULL; - struct dirent *entry = NULL; - char *device_link = NULL; - - VIR_DEBUG("Attempting to get SR IOV virtual functions for device" - "with sysfs path '%s'", sysfs_path); - - dir = opendir(sysfs_path); - if (dir == NULL) { - goto out; - } - - while ((entry = readdir(dir))) { - if (STRPREFIX(entry->d_name, "virtfn")) { - /* This local is just to avoid lines of code much> 80 col. */ - unsigned int *num_funcs =&d->pci_dev.num_virtual_functions; - - if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { - virReportOOMError(); - goto out; - } - - VIR_DEBUG("Number of virtual functions: %d", *num_funcs); - if (VIR_REALLOC_N(d->pci_dev.virtual_functions, - (*num_funcs) + 1) != 0) { - virReportOOMError(); - goto out; - } - - if (get_sriov_function(device_link, -&d->pci_dev.virtual_functions[*num_funcs]) - != SRIOV_FOUND) { - - /* We should not get back SRIOV_NOT_FOUND in this - * case, so if we do, it's an error. */ - VIR_ERROR(_("Failed to get SR IOV function from device link '%s'"), - device_link); - goto out; - } else { - (*num_funcs)++; - d->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; - } - - VIR_FREE(device_link); - } - } - - ret = 0; - -out: - if (dir) - closedir(dir); - VIR_FREE(device_link); - return ret; -} - #endif /* __linux__ */ diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c index 2c5d016..badf241 100644 --- a/src/node_device/node_device_udev.c +++ b/src/node_device/node_device_udev.c @@ -37,6 +37,7 @@ #include "uuid.h" #include "util.h" #include "buf.h" +#include "pci.h" #define VIR_FROM_THIS VIR_FROM_NODEDEV @@ -480,8 +481,13 @@ static int udevProcessPCI(struct udev_device *device, goto out; } - get_physical_function(syspath, data); - get_virtual_functions(syspath, data); + if (!pciGetPhysicalFunction(syspath,&data->pci_dev.physical_function)) + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_PHYSICAL_FUNCTION; + + if (!pciGetVirtualFunctions(syspath,&data->pci_dev.virtual_functions, +&data->pci_dev.num_virtual_functions) || + data->pci_dev.num_virtual_functions> 0) + data->pci_dev.flags |= VIR_NODE_DEV_CAP_FLAG_PCI_VIRTUAL_FUNCTION; ret = 0; diff --git a/src/util/pci.c b/src/util/pci.c index a79c164..c3f3bb4 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -32,6 +32,7 @@ #include<sys/types.h> #include<sys/stat.h> #include<unistd.h> +#include<stdlib.h> #include "logging.h" #include "memory.h" @@ -48,6 +49,10 @@ #define PCI_ID_LEN 10 /* "XXXX XXXX" */ #define PCI_ADDR_LEN 13 /* "XXXX:XX:XX.X" */ +#define SRIOV_FOUND 0 +#define SRIOV_NOT_FOUND 1 +#define SRIOV_ERROR -1 + struct _pciDevice { unsigned domain; unsigned bus; @@ -1679,3 +1684,228 @@ int pciDeviceIsAssignable(pciDevice *dev, return 1; } + +static int +logStrToLong_ui(char const *s, + char **end_ptr, + int base, + unsigned int *result) +{ + int ret = 0; + + ret = virStrToLong_ui(s, end_ptr, base, result); + if (ret != 0) { + VIR_ERROR(_("Failed to convert '%s' to unsigned int"), s); + } else { + VIR_DEBUG("Converted '%s' to unsigned int %u", s, *result); + } + + return ret; +} + +static int +pciParsePciConfigAddress(char *address, + struct pci_config_address *bdf) +{ + char *p = NULL; + int ret = -1; + + if ((address == NULL) || (logStrToLong_ui(address,&p, 16, +&bdf->domain) == -1)) { + goto out; + } + + if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, +&bdf->bus) == -1)) { + goto out; + } + + if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, +&bdf->slot) == -1)) { + goto out; + } + + if ((p == NULL) || (logStrToLong_ui(p+1,&p, 16, +&bdf->function) == -1)) { + goto out; + } + + ret = 0; + +out: + return ret; +} + +static int +pciGetPciConfigAddressFromSysfsDeviceLink(const char *device_link, + struct pci_config_address **bdf) +{ + char *config_address = NULL; + char *device_path = NULL; + char errbuf[64]; + int ret = -1; + + VIR_DEBUG("Attempting to resolve device path from device link '%s'", + device_link); + + if (!virFileExists(device_link)) { + VIR_DEBUG("sysfs_path '%s' does not exist", device_link); + return ret; + } + + device_path = canonicalize_file_name (device_link); + if (device_path == NULL) { + memset(errbuf, '\0', sizeof(errbuf)); + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to resolve device link '%s': '%s'"), + device_link, virStrerror(errno, errbuf, + sizeof(errbuf))); + return ret; + } + + config_address = basename(device_path); + if (VIR_ALLOC(*bdf) != 0) { + virReportOOMError(); + goto out; + } + + if (pciParsePciConfigAddress(config_address, *bdf) != 0) { + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to parse PCI config address '%s'"), + config_address); + VIR_FREE(*bdf); + goto out; + } + + VIR_DEBUG("pci_config_address %.4x:%.2x:%.2x.%.1x", + (*bdf)->domain, + (*bdf)->bus, + (*bdf)->slot, + (*bdf)->function); + + ret = 0; + +out: + VIR_FREE(device_path); + + return ret; +} + +#ifdef __linux__ +/* + * Returns Physical function given a virtual function + */ +int +pciGetPhysicalFunction(const char *vf_sysfs_path, + struct pci_config_address **physical_function) +{ + int ret = -1; + char *device_link = NULL; + + VIR_DEBUG("Attempting to get SR IOV physical function for device " + "with sysfs path '%s'", vf_sysfs_path); + + if (virBuildPath(&device_link, vf_sysfs_path, "physfn") == -1) { + virReportOOMError(); + return ret; + } else { + ret = pciGetPciConfigAddressFromSysfsDeviceLink(device_link, + physical_function); + } + + VIR_FREE(device_link); + + return ret; +} + +/* + * Returns virtual functions of a physical function + */ +int +pciGetVirtualFunctions(const char *sysfs_path, + struct pci_config_address ***virtual_functions, + unsigned int *num_virtual_functions) +{ + int ret = -1; + DIR *dir = NULL; + struct dirent *entry = NULL; + char *device_link = NULL; + char errbuf[64]; + + VIR_DEBUG("Attempting to get SR IOV virtual functions for device" + "with sysfs path '%s'", sysfs_path); + + dir = opendir(sysfs_path); + if (dir == NULL) { + memset(errbuf, '\0', sizeof(errbuf)); + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to open dir '%s': '%s'"), + sysfs_path, virStrerror(errno, errbuf, + sizeof(errbuf))); + return ret; + } + + *virtual_functions = NULL; + *num_virtual_functions = 0; + while ((entry = readdir(dir))) { + if (STRPREFIX(entry->d_name, "virtfn")) { + + if (virBuildPath(&device_link, sysfs_path, entry->d_name) == -1) { + virReportOOMError(); + goto out; + } + + VIR_DEBUG("Number of virtual functions: %d", + *num_virtual_functions); + if (VIR_REALLOC_N(*virtual_functions, + (*num_virtual_functions) + 1) != 0) { + virReportOOMError(); + VIR_FREE(device_link); + goto out; + } + + if (pciGetPciConfigAddressFromSysfsDeviceLink(device_link, +&((*virtual_functions)[*num_virtual_functions])) != + SRIOV_FOUND) { + /* We should not get back SRIOV_NOT_FOUND in this + * case, so if we do, it's an error. */ + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to get SR IOV function from device " + "link '%s'"), device_link); + VIR_FREE(device_link); + goto out; + } else { + (*num_virtual_functions)++; + } + VIR_FREE(device_link); + } + } + + ret = 0; + +out: + if (dir) + closedir(dir); + + return ret; +} +#else +int +pciGetPhysicalFunction(const char *vf_sysfs_path, + struct pci_config_address **physical_function) +{ + pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetPhysicalFunction is not " + "supported on non-linux platforms")); + return -1; +} + +int +pciGetVirtualFunctions(const char *sysfs_path, + struct pci_config_address ***virtual_functions, + unsigned int *num_virtual_functions) +{ + pciReportError(VIR_ERR_INTERNAL_ERROR, _("pciGetVirtualFunctions is not " + "supported on non-linux platforms")); + return -1; +} +#endif /* __linux__ */ diff --git a/src/util/pci.h b/src/util/pci.h index a351baf..fe9479e 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -27,6 +27,13 @@ typedef struct _pciDevice pciDevice; typedef struct _pciDeviceList pciDeviceList; +struct pci_config_address { + unsigned int domain; + unsigned int bus; + unsigned int slot; + unsigned int function; +}; + pciDevice *pciGetDevice (unsigned domain, unsigned bus, unsigned slot, @@ -74,4 +81,11 @@ int pciDeviceIsAssignable(pciDevice *dev, int strict_acs_check); int pciWaitForDeviceCleanup(pciDevice *dev, const char *matcher); +int pciGetPhysicalFunction(const char *sysfs_path, + struct pci_config_address **phys_fn); + +int pciGetVirtualFunctions(const char *sysfs_path, + struct pci_config_address ***virtual_functions, + unsigned int *num_virtual_functions); + #endif /* __VIR_PCI_H__ */