diff mbox

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

Message ID 1507375221-16271-1-git-send-email-hemant.agrawal@nxp.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

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

Comments

Thomas Monjalon Oct. 7, 2017, 11:37 a.m. UTC | #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. UTC | #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. UTC | #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?
diff mbox

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