[dpdk-dev] [PATCH] eal: fslmc bus need vfio enabled for non PCI case as well

[dpdk-dev] [PATCH] eal: fslmc bus need vfio enabled for non PCI case as well

[Hemant Agrawal]

In case no_pci is configured, fslmc bus will still need the
the vfio to be enabled.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
 lib/librte_eal/linuxapp/eal/eal.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 7c78f2d..0484396 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -722,6 +722,13 @@ static int rte_eal_vfio_setup(void)
 		vfio_enabled |= pci_vfio_is_enabled();
 	}
 
+#ifdef RTE_LIBRTE_FSLMC_BUS
+	if (!vfio_enabled) {
+		if (!vfio_enable("vfio_fsl_mc"))
+			vfio_enabled = 1;
+	}
+#endif
+
 	if (vfio_enabled) {
 
 		/* if we are primary process, create a thread to communicate with
-- 
2.7.4

Re: [dpdk-dev] [PATCH] eal: fslmc bus need vfio enabled for non PCI case as well

[Thomas Monjalon]

13/07/2017 13:48, Hemant Agrawal:
> In case no_pci is configured, fslmc bus will still need the
> the vfio to be enabled.
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> +#ifdef RTE_LIBRTE_FSLMC_BUS
> +	if (!vfio_enabled) {
> +		if (!vfio_enable("vfio_fsl_mc"))
> +			vfio_enabled = 1;
> +	}
> +#endif

It seems to be a hack.
VFIO is not only PCI.
Why --no-pci is impacting VFIO?

Anatoly?

Re: [dpdk-dev] [PATCH] eal: fslmc bus need vfio enabled for non PCI case as well

[Hemant Agrawal]

On 10/6/2017 5:11 AM, Thomas Monjalon wrote:
> 13/07/2017 13:48, Hemant Agrawal:
>> In case no_pci is configured, fslmc bus will still need the
>> the vfio to be enabled.
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> +#ifdef RTE_LIBRTE_FSLMC_BUS
>> +	if (!vfio_enabled) {
>> +		if (!vfio_enable("vfio_fsl_mc"))
>> +			vfio_enabled = 1;
>> +	}
>> +#endif
>
> It seems to be a hack.
> VFIO is not only PCI.
> Why --no-pci is impacting VFIO?
>
> Anatoly?
>

At present there are only 2 users of VFIO. PCI and fsl-mc bus.


One options is that we always enable vfio irrespective of --no-pci flag. 
My believe is that vfio-pci will be present in most system supporting 
any other flavor of vfio (platform, fsl-mc etc).

The other option is what I proposed. i.e. if vfio is not already enabled 
the platform is FSLMC bus, enable it.

regards,
Hemant

Re: [dpdk-dev] [PATCH] eal: fslmc bus need vfio enabled for non PCI case as well

[Thomas Monjalon]

06/10/2017 18:22, Hemant Agrawal:
> On 10/6/2017 5:11 AM, Thomas Monjalon wrote:
> > 13/07/2017 13:48, Hemant Agrawal:
> >> In case no_pci is configured, fslmc bus will still need the
> >> the vfio to be enabled.
> >>
> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> ---
> >> --- a/lib/librte_eal/linuxapp/eal/eal.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> >> +#ifdef RTE_LIBRTE_FSLMC_BUS
> >> +	if (!vfio_enabled) {
> >> +		if (!vfio_enable("vfio_fsl_mc"))
> >> +			vfio_enabled = 1;
> >> +	}
> >> +#endif
> >
> > It seems to be a hack.
> > VFIO is not only PCI.
> > Why --no-pci is impacting VFIO?
> >
> > Anatoly?
> >
> 
> At present there are only 2 users of VFIO. PCI and fsl-mc bus.
> 
> 
> One options is that we always enable vfio irrespective of --no-pci flag. 
> My believe is that vfio-pci will be present in most system supporting 
> any other flavor of vfio (platform, fsl-mc etc).
> 
> The other option is what I proposed. i.e. if vfio is not already enabled 
> the platform is FSLMC bus, enable it.

Why not always enable it?

[dpdk-dev] [PATCH v2] eal: enable vfio independent of no PCI flag

[Hemant Agrawal]

In case no_pci is configured, other buses e.g. fslmc bus will
still need the the vfio to be enabled.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
v2: enabled VFIO, independent of no-pci flag as suggested by Thomas

 lib/librte_eal/linuxapp/eal/eal.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 28bc46b..76c980c 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -733,10 +733,8 @@ static int rte_eal_vfio_setup(void)
 {
 	int vfio_enabled = 0;
 
-	if (!internal_config.no_pci) {
-		pci_vfio_enable();
-		vfio_enabled |= pci_vfio_is_enabled();
-	}
+	pci_vfio_enable();
+	vfio_enabled |= pci_vfio_is_enabled();
 
 	if (vfio_enabled) {
 
-- 
2.7.4

Re: [dpdk-dev] [PATCH] eal: fslmc bus need vfio enabled for non PCI case as well

[Hemant Agrawal]

On 10/6/2017 10:38 PM, Thomas Monjalon wrote:
> 06/10/2017 18:22, Hemant Agrawal:
>> On 10/6/2017 5:11 AM, Thomas Monjalon wrote:
>>> 13/07/2017 13:48, Hemant Agrawal:
>>>> In case no_pci is configured, fslmc bus will still need the
>>>> the vfio to be enabled.
>>>>
>>>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>>>> ---
>>>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>>>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>>>> +#ifdef RTE_LIBRTE_FSLMC_BUS
>>>> +	if (!vfio_enabled) {
>>>> +		if (!vfio_enable("vfio_fsl_mc"))
>>>> +			vfio_enabled = 1;
>>>> +	}
>>>> +#endif
>>>
>>> It seems to be a hack.
>>> VFIO is not only PCI.
>>> Why --no-pci is impacting VFIO?
>>>
>>> Anatoly?
>>>
>>
>> At present there are only 2 users of VFIO. PCI and fsl-mc bus.
>>
>>
>> One options is that we always enable vfio irrespective of --no-pci flag.
>> My believe is that vfio-pci will be present in most system supporting
>> any other flavor of vfio (platform, fsl-mc etc).
>>
>> The other option is what I proposed. i.e. if vfio is not already enabled
>> the platform is FSLMC bus, enable it.
>
> Why not always enable it?
>
I agree. sent a v2 for this.

Regards,
Hemant

Re: [dpdk-dev] [PATCH v2] eal: enable vfio independent of no PCI flag

[Thomas Monjalon]

07/10/2017 13:20, Hemant Agrawal:
> In case no_pci is configured, other buses e.g. fslmc bus will
> still need the the vfio to be enabled.
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> ---
> v2: enabled VFIO, independent of no-pci flag as suggested by Thomas
[...]
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -733,10 +733,8 @@ static int rte_eal_vfio_setup(void)
>  {
>  	int vfio_enabled = 0;
>  
> -	if (!internal_config.no_pci) {
> -		pci_vfio_enable();
> -		vfio_enabled |= pci_vfio_is_enabled();
> -	}
> +	pci_vfio_enable();
> +	vfio_enabled |= pci_vfio_is_enabled();

You are enabling vfio_pci.
This part could stay conditionned by no_pci.

I was thinking you need vfio without vfio_pci. Am I right?
If yes, I suggest to enable only vfio root module.

Re: [dpdk-dev] [PATCH v2] eal: enable vfio independent of no PCI flag

[Hemant Agrawal]

Hi Thomas, Anatoly,

On 10/7/2017 5:07 PM, Thomas Monjalon wrote:
> 07/10/2017 13:20, Hemant Agrawal:
>> In case no_pci is configured, other buses e.g. fslmc bus will
>> still need the the vfio to be enabled.
>>
>> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
>> ---
>> v2: enabled VFIO, independent of no-pci flag as suggested by Thomas
> [...]
>> --- a/lib/librte_eal/linuxapp/eal/eal.c
>> +++ b/lib/librte_eal/linuxapp/eal/eal.c
>> @@ -733,10 +733,8 @@ static int rte_eal_vfio_setup(void)
>>  {
>>  	int vfio_enabled = 0;
>>
>> -	if (!internal_config.no_pci) {
>> -		pci_vfio_enable();
>> -		vfio_enabled |= pci_vfio_is_enabled();
>> -	}
>> +	pci_vfio_enable();
>> +	vfio_enabled |= pci_vfio_is_enabled();
>
> You are enabling vfio_pci.
> This part could stay conditionned by no_pci.
>
> I was thinking you need vfio without vfio_pci. Am I right?

yes
> If yes, I suggest to enable only vfio root module.
>

vfio_enable should be done only once. So, if I enable it for "vfio", 
pci_vfio_enable is not required.
In any case it is not storing any PCI specific data and there are no 
error checks here of "vfio_pci" enable failure.

So, if we use,
	vfio_enable("vfio");
	vfio_enabled |= vfio_is_enabled("vfio");

It seems no_pci check will not have any value.

let me know your thoughts?

Re: [dpdk-dev] [PATCH v2] eal: enable vfio independent of no PCI flag

[Thomas Monjalon]

10/10/2017 15:46, Hemant Agrawal:
> Hi Thomas, Anatoly,
> 
> On 10/7/2017 5:07 PM, Thomas Monjalon wrote:
> > 07/10/2017 13:20, Hemant Agrawal:
> >> In case no_pci is configured, other buses e.g. fslmc bus will
> >> still need the the vfio to be enabled.
> >>
> >> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> >> ---
> >> v2: enabled VFIO, independent of no-pci flag as suggested by Thomas
> > [...]
> >> --- a/lib/librte_eal/linuxapp/eal/eal.c
> >> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> >> @@ -733,10 +733,8 @@ static int rte_eal_vfio_setup(void)
> >>  {
> >>  	int vfio_enabled = 0;
> >>
> >> -	if (!internal_config.no_pci) {
> >> -		pci_vfio_enable();
> >> -		vfio_enabled |= pci_vfio_is_enabled();
> >> -	}
> >> +	pci_vfio_enable();
> >> +	vfio_enabled |= pci_vfio_is_enabled();
> >
> > You are enabling vfio_pci.
> > This part could stay conditionned by no_pci.
> >
> > I was thinking you need vfio without vfio_pci. Am I right?
> 
> yes
> > If yes, I suggest to enable only vfio root module.
> >
> 
> vfio_enable should be done only once. So, if I enable it for "vfio", 
> pci_vfio_enable is not required.
> In any case it is not storing any PCI specific data and there are no 
> error checks here of "vfio_pci" enable failure.
> 
> So, if we use,
> 	vfio_enable("vfio");
> 	vfio_enabled |= vfio_is_enabled("vfio");
> 
> It seems no_pci check will not have any value.
> 
> let me know your thoughts?

I don't know the code managing VFIO.
Anatoly, please can you help meeting the requirement of
VFIO always enabled?

[dpdk-dev] [PATCH v3] eal: enable vfio independent of PCI bus

[Hemant Agrawal]

VFIO may be used by buses other than PCI. This patch enables
the VFIO on the basis of vfio root presence.

Since vfio_enable should be called only once, pci_vfio_enable
is also removed.

A debug print is added in case vfio_pci module is not present.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
v3: changed the vfio enable for vfio root instead of pci

v2: enabled VFIO, independent of no-pci flag as suggested by Thomas
    removed fslmc specific vfio enable

 lib/librte_eal/linuxapp/eal/eal.c          | 8 ++++++--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 6 ------
 lib/librte_eal/linuxapp/eal/eal_vfio.c     | 2 +-
 lib/librte_eal/linuxapp/eal/eal_vfio.h     | 1 -
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index ec37c52..e0aceba 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -710,9 +710,13 @@ static int rte_eal_vfio_setup(void)
 {
 	int vfio_enabled = 0;
 
+	if (vfio_enable("vfio"))
+		return -1;
+	vfio_enabled = vfio_is_enabled("vfio");
+
 	if (!internal_config.no_pci) {
-		pci_vfio_enable();
-		vfio_enabled |= pci_vfio_is_enabled();
+		if (!pci_vfio_is_enabled())
+			RTE_LOG(DEBUG, EAL, "vfio pci modules not loaded\n");
 	}
 
 	if (vfio_enabled) {
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index aa9d96e..fc84705 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -661,12 +661,6 @@ pci_vfio_ioport_unmap(struct rte_pci_ioport *p)
 }
 
 int
-pci_vfio_enable(void)
-{
-	return vfio_enable("vfio_pci");
-}
-
-int
 pci_vfio_is_enabled(void)
 {
 	return vfio_is_enabled("vfio_pci");
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index b32cd09..ebb71f0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -489,7 +489,7 @@ vfio_enable(const char *modname)
 	/* inform the user that we are probing for VFIO */
 	RTE_LOG(INFO, EAL, "Probing VFIO support...\n");
 
-	/* check if vfio-pci module is loaded */
+	/* check if vfio module is loaded */
 	vfio_available = rte_eal_check_module(modname);
 
 	/* return error directly */
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index 26ea8e1..4bab363 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -207,7 +207,6 @@ int vfio_release_device(const char *sysfs_base, const char *dev_addr, int fd);
 int vfio_enable(const char *modname);
 int vfio_is_enabled(const char *modname);
 
-int pci_vfio_enable(void);
 int pci_vfio_is_enabled(void);
 
 int vfio_mp_sync_setup(void);
-- 
2.7.4

[dpdk-dev] FW: [PATCH v3] eal: enable vfio independent of PCI bus

[Hemant Agrawal]

Hi Anatoly,
	Will you please review the following patch?

Regards,
Hemant

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Hemant Agrawal
Sent: Wednesday, October 11, 2017 12:12 PM
To: thomas@monjalon.net; anatoly.burakov@intel.com
Cc: dev@dpdk.org
Subject: [dpdk-dev] [PATCH v3] eal: enable vfio independent of PCI bus

VFIO may be used by buses other than PCI. This patch enables the VFIO on the basis of vfio root presence.

Since vfio_enable should be called only once, pci_vfio_enable is also removed.

A debug print is added in case vfio_pci module is not present.

Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
---
v3: changed the vfio enable for vfio root instead of pci

v2: enabled VFIO, independent of no-pci flag as suggested by Thomas
    removed fslmc specific vfio enable

 lib/librte_eal/linuxapp/eal/eal.c          | 8 ++++++--
 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 6 ------
 lib/librte_eal/linuxapp/eal/eal_vfio.c     | 2 +-
 lib/librte_eal/linuxapp/eal/eal_vfio.h     | 1 -
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index ec37c52..e0aceba 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -710,9 +710,13 @@ static int rte_eal_vfio_setup(void)  {
 	int vfio_enabled = 0;
 
+	if (vfio_enable("vfio"))
+		return -1;
+	vfio_enabled = vfio_is_enabled("vfio");
+
 	if (!internal_config.no_pci) {
-		pci_vfio_enable();
-		vfio_enabled |= pci_vfio_is_enabled();
+		if (!pci_vfio_is_enabled())
+			RTE_LOG(DEBUG, EAL, "vfio pci modules not loaded\n");
 	}
 
 	if (vfio_enabled) {
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
index aa9d96e..fc84705 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci_vfio.c
@@ -661,12 +661,6 @@ pci_vfio_ioport_unmap(struct rte_pci_ioport *p)  }
 
 int
-pci_vfio_enable(void)
-{
-	return vfio_enable("vfio_pci");
-}
-
-int
 pci_vfio_is_enabled(void)
 {
 	return vfio_is_enabled("vfio_pci");
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.c b/lib/librte_eal/linuxapp/eal/eal_vfio.c
index b32cd09..ebb71f0 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.c
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.c
@@ -489,7 +489,7 @@ vfio_enable(const char *modname)
 	/* inform the user that we are probing for VFIO */
 	RTE_LOG(INFO, EAL, "Probing VFIO support...\n");
 
-	/* check if vfio-pci module is loaded */
+	/* check if vfio module is loaded */
 	vfio_available = rte_eal_check_module(modname);
 
 	/* return error directly */
diff --git a/lib/librte_eal/linuxapp/eal/eal_vfio.h b/lib/librte_eal/linuxapp/eal/eal_vfio.h
index 26ea8e1..4bab363 100644
--- a/lib/librte_eal/linuxapp/eal/eal_vfio.h
+++ b/lib/librte_eal/linuxapp/eal/eal_vfio.h
@@ -207,7 +207,6 @@ int vfio_release_device(const char *sysfs_base, const char *dev_addr, int fd);  int vfio_enable(const char *modname);  int vfio_is_enabled(const char *modname);
 
-int pci_vfio_enable(void);
 int pci_vfio_is_enabled(void);
 
 int vfio_mp_sync_setup(void);
--
2.7.4

Re: [dpdk-dev] FW: [PATCH v3] eal: enable vfio independent of PCI bus

[Burakov, Anatoly]

On 23-Oct-17 7:33 AM, Hemant Agrawal wrote:
> Hi Anatoly,
> 	Will you please review the following patch?
> 
> Regards,
> Hemant
> 

Hi Hemant,

My apologies, slipped through the cracks. Reviewing now.

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3] eal: enable vfio independent of PCI bus

[Burakov, Anatoly]

On 11-Oct-17 7:42 AM, Hemant Agrawal wrote:
> VFIO may be used by buses other than PCI. This patch enables
> the VFIO on the basis of vfio root presence.
> 
> Since vfio_enable should be called only once, pci_vfio_enable
> is also removed.
> 
> A debug print is added in case vfio_pci module is not present.
> 
> Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

-- 
Thanks,
Anatoly

Re: [dpdk-dev] [PATCH v3] eal: enable vfio independent of PCI bus

[Thomas Monjalon]

23/10/2017 13:47, Burakov, Anatoly:
> On 11-Oct-17 7:42 AM, Hemant Agrawal wrote:
> > VFIO may be used by buses other than PCI. This patch enables
> > the VFIO on the basis of vfio root presence.
> > 
> > Since vfio_enable should be called only once, pci_vfio_enable
> > is also removed.
> > 
> > A debug print is added in case vfio_pci module is not present.
> > 
> > Signed-off-by: Hemant Agrawal <hemant.agrawal@nxp.com>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

Applied, thanks