On 12/10/2014 5:21 PM, Burakov, Anatoly wrote: >> When vfio module is not loaded when kernel support vfio feature, the >> routine still try to open the container to get file description. >> >> This action is not safe, and of cause got error messages: >> >> EAL: Detected 40 lcore(s) >> EAL: unsupported IOMMU type! >> EAL: VFIO support could not be initialized >> EAL: Setting up memory... >> >> This may make user confuse, this patch make it reasonable and much more >> soomth to user. >> >> Signed-off-by: Michael Qiu <michael.qiu at intel.com> >> --- >> v5 --> v4 >> 1. Move rte_eal_check_module() body to eal.c >> 2. Clean up "unsupported IOMMU type" log >> >> v4 --> v3: >> 1. Remove RTE_LOG for params check >> 2. Remove "vfio" module check as "vfio_iommu_type1" >> loaded indecated "vfio" loaded >> >> v3 --> v2: >> 1. Add error log in rte_eal_check_module() >> 2. Some code clean up. >> >> v2 --> v1: >> 1. Move check_module() from rte_common.h to eal_private.h >> and rename to rte_eal_check_module(). >> To make it linuxapp only. >> 2. Some code clean up. >> >> lib/librte_eal/common/eal_private.h | 15 +++++++++++++++ >> lib/librte_eal/linuxapp/eal/eal.c | 27 +++++++++++++++++++++++++++ >> lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 26 >> +++++++++++++++++++++++--- >> 3 files changed, 65 insertions(+), 3 deletions(-) >> >> diff --git a/lib/librte_eal/common/eal_private.h >> b/lib/librte_eal/common/eal_private.h >> index 232fcec..4183b54 100644 >> --- a/lib/librte_eal/common/eal_private.h >> +++ b/lib/librte_eal/common/eal_private.h >> @@ -203,4 +203,19 @@ int rte_eal_alarm_init(void); >> */ >> int rte_eal_dev_init(void); >> >> +/** >> + * Function is to check if the kernel module(like, vfio, >> +vfio_iommu_type1, >> + * etc.) loaded. >> + * >> + * @param module_name >> + * The module's name which need to be checked >> + * >> + * @return >> + * -1 means some error happens(NULL pointer or open failure) >> + * 0 means the module not loaded >> + * 1 means the module loaded >> + */ >> +inline int >> +rte_eal_check_module(const char *module_name); > Just curious - why inline?
Just want to make it inline, no strong reason. If you do not agree I will remove. > >> + >> #endif /* _EAL_PRIVATE_H_ */ >> diff --git a/lib/librte_eal/linuxapp/eal/eal.c >> b/lib/librte_eal/linuxapp/eal/eal.c >> index 89f3b5e..40b462e 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal.c >> +++ b/lib/librte_eal/linuxapp/eal/eal.c >> @@ -859,3 +859,30 @@ int rte_eal_has_hugepages(void) { >> return ! internal_config.no_hugetlbfs; } >> + >> +inline int >> +rte_eal_check_module(const char *module_name) { >> + char mod_name[30]; /* Any module names can be longer than 30 >> bytes? */ >> + int ret = 0; >> + >> + if (NULL == module_name) >> + return -1; >> + >> + FILE * fd = fopen("/proc/modules", "r"); >> + if (NULL == fd) { >> + RTE_LOG(ERR, EAL, "Open /proc/modules failed!" >> + " error %i (%s)\n", errno, strerror(errno)); >> + return -1; >> + } >> + while(!feof(fd)) { >> + fscanf(fd, "%30s %*[^\n]", mod_name); > As far as I know, in fscanf terms, %30s will result in at most 30 character > string, i.e. 31 bytes (30 characters + null terminator). So it probably > should be %29s. Good catch, you are right. Thanks, Michael > >> + if(!strcmp(mod_name, module_name)) { >> + ret = 1; >> + break; >> + } >> + } >> + fclose(fd); >> + >> + return ret; >> +} >> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c >> b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c >> index c1246e8..16fe10f 100644 >> --- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c >> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c >> @@ -44,6 +44,7 @@ >> #include <rte_tailq.h> >> #include <rte_eal_memconfig.h> >> #include <rte_malloc.h> >> +#include <eal_private.h> >> >> #include "eal_filesystem.h" >> #include "eal_pci_init.h" >> @@ -339,10 +340,12 @@ pci_vfio_get_container_fd(void) >> ret = ioctl(vfio_container_fd, VFIO_CHECK_EXTENSION, >> VFIO_TYPE1_IOMMU); >> if (ret != 1) { >> if (ret < 0) >> - RTE_LOG(ERR, EAL, " could not get IOMMU >> type, " >> - "error %i (%s)\n", errno, >> strerror(errno)); >> + RTE_LOG(ERR, EAL, " could not get IOMMU >> type," >> + " error %i (%s)\n", errno, >> + strerror(errno)); >> else >> - RTE_LOG(ERR, EAL, " unsupported IOMMU >> type!\n"); >> + RTE_LOG(ERR, EAL, " unsupported IOMMU >> type" >> + " detected in VFIO\n"); >> close(vfio_container_fd); >> return -1; >> } >> @@ -783,11 +786,28 @@ pci_vfio_enable(void) { >> /* initialize group list */ >> int i; >> + int module_vfio_type1; >> >> for (i = 0; i < VFIO_MAX_GROUPS; i++) { >> vfio_cfg.vfio_groups[i].fd = -1; >> vfio_cfg.vfio_groups[i].group_no = -1; >> } >> + >> + module_vfio_type1 = rte_eal_check_module("vfio_iommu_type1"); >> + >> + /* return error directly */ >> + if (module_vfio_type1 == -1) { >> + RTE_LOG(INFO, EAL, "Could not get loaded module >> details!\n"); >> + return -1; >> + } >> + >> + /* return 0 if VFIO modules not loaded */ >> + if (module_vfio_type1 == 0) { >> + RTE_LOG(INFO, EAL, "VFIO modules not all loaded," >> + " skip VFIO support ...\n"); >> + return 0; >> + } >> + >> vfio_cfg.vfio_container_fd = pci_vfio_get_container_fd(); >> >> /* check if we have VFIO driver enabled */ >> -- >> 1.9.3 >