[dpdk-dev,v6] VFIO: Avoid to enable vfio while the module not loaded
Commit Message
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@intel.com>
---
v6 --> v5
1. Change rte_eal_check_module() to normal
function instead of inline
2. limit fscanf to get 29 charactors not include '\0'
lib/librte_eal/common/eal_private.h | 14 ++++++++++++++
lib/librte_eal/linuxapp/eal/eal.c | 27 +++++++++++++++++++++++++++
lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 26 +++++++++++++++++++++++---
3 files changed, 64 insertions(+), 3 deletions(-)
Comments
> 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@intel.com>
> ---
> v6 --> v5
> 1. Change rte_eal_check_module() to normal
> function instead of inline
> 2. limit fscanf to get 29 charactors not include '\0'
>
> lib/librte_eal/common/eal_private.h | 14 ++++++++++++++
> lib/librte_eal/linuxapp/eal/eal.c | 27 +++++++++++++++++++++++++++
> lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 26
> +++++++++++++++++++++++---
> 3 files changed, 64 insertions(+), 3 deletions(-)
>
> diff --git a/lib/librte_eal/common/eal_private.h
> b/lib/librte_eal/common/eal_private.h
> index 232fcec..2c751c6 100644
> --- a/lib/librte_eal/common/eal_private.h
> +++ b/lib/librte_eal/common/eal_private.h
> @@ -203,4 +203,18 @@ 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
> + */
> +int rte_eal_check_module(const char *module_name);
> +
> #endif /* _EAL_PRIVATE_H_ */
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index 89f3b5e..9c1a1cc 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; }
> +
> +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, "%29s %*[^\n]", mod_name);
> + 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
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Hi Thomas,
Will you plan to merge this patch? If yes, you plan to merge it into
1.8 or 2.0?
Thanks,
Michael
On 12/10/2014 7:48 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@intel.com>
>> ---
>> v6 --> v5
>> 1. Change rte_eal_check_module() to normal
>> function instead of inline
>> 2. limit fscanf to get 29 charactors not include '\0'
>>
>> lib/librte_eal/common/eal_private.h | 14 ++++++++++++++
>> lib/librte_eal/linuxapp/eal/eal.c | 27 +++++++++++++++++++++++++++
>> lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 26
>> +++++++++++++++++++++++---
>> 3 files changed, 64 insertions(+), 3 deletions(-)
>>
>> diff --git a/lib/librte_eal/common/eal_private.h
>> b/lib/librte_eal/common/eal_private.h
>> index 232fcec..2c751c6 100644
>> --- a/lib/librte_eal/common/eal_private.h
>> +++ b/lib/librte_eal/common/eal_private.h
>> @@ -203,4 +203,18 @@ 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
>> + */
>> +int rte_eal_check_module(const char *module_name);
>> +
>> #endif /* _EAL_PRIVATE_H_ */
>> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
>> index 89f3b5e..9c1a1cc 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; }
>> +
>> +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, "%29s %*[^\n]", mod_name);
>> + 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
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
>
2014-12-19 07:09, Qiu, Michael:
> Hi Thomas,
>
> Will you plan to merge this patch? If yes, you plan to merge it into
> 1.8 or 2.0?
It won't be in 1.8 for 2 reasons:
- there is a risk to break something in some environments, so need more tests
- you add a function protoype in eal common without bsd implementation
On 12/19/2014 4:24 PM, Thomas Monjalon wrote:
> 2014-12-19 07:09, Qiu, Michael:
>> Hi Thomas,
>>
>> Will you plan to merge this patch? If yes, you plan to merge it into
>> 1.8 or 2.0?
> It won't be in 1.8 for 2 reasons:
> - there is a risk to break something in some environments, so need more tests
> - you add a function protoype in eal common without bsd implementation
OK, fair enough, I check with you just as a reminder to be ensure that
you do not miss this thread.
> > 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@intel.com>
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Note that rte_eal_check_module has no bsd counterpart.
It could be needed later.
Applied
Thanks
Yep, apologies, it's my fault as it was my suggestion. I knew there was a linuxapp-only EAL header, for some reason I thought it's eal_private. Any suggestions on where to put this function? I don't think BSD needs this function.
Thanks,
Anatoly
> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, January 15, 2015 1:38 PM
> To: Qiu, Michael
> Cc: Burakov, Anatoly; dev@dpdk.org; Xie, Huawei
> Subject: Re: [PATCH v6] VFIO: Avoid to enable vfio while the module not
> loaded
>
> > > 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@intel.com>
> >
> > Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
> Note that rte_eal_check_module has no bsd counterpart.
> It could be needed later.
>
> Applied
>
> Thanks
> --
> Thomas
2015-01-15 13:42, Burakov, Anatoly:
> Yep, apologies, it's my fault as it was my suggestion.
> I knew there was a linuxapp-only EAL header, for some reason I thought it's eal_private.
> Any suggestions on where to put this function? I don't think BSD needs this function.
No, it's OK. I think it could be needed in bsd if a PMD
depends on a kernel driver or try to unload one.
PS: please don't top post
@@ -203,4 +203,18 @@ 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
+ */
+int rte_eal_check_module(const char *module_name);
+
#endif /* _EAL_PRIVATE_H_ */
@@ -859,3 +859,30 @@ int rte_eal_has_hugepages(void)
{
return ! internal_config.no_hugetlbfs;
}
+
+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, "%29s %*[^\n]", mod_name);
+ if(!strcmp(mod_name, module_name)) {
+ ret = 1;
+ break;
+ }
+ }
+ fclose(fd);
+
+ return ret;
+}
@@ -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 */