[v2] eal: enable vfio independent of no PCI flag

Message ID 1507375221-16271-1-git-send-email-hemant.agrawal@nxp.com
State New
Headers show
Series
  • [v2] eal: enable vfio independent of no PCI flag
Related show

Commit Message

Hemant Agrawal Oct. 7, 2017, 11:20 a.m.
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(-)

-- 
2.7.4

Comments

Thomas Monjalon Oct. 7, 2017, 11:37 a.m. | #1
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.
Hemant Agrawal Oct. 10, 2017, 1:46 p.m. | #2
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?
Thomas Monjalon Oct. 10, 2017, 4:27 p.m. | #3
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?

Patch

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) {