diff mbox series

pci/linux: use RTE_IOVA_VA whenever possible

Message ID 20180907145340.79670-1-dariusz.stojaczyk@intel.com (mailing list archive)
State Rejected, archived
Delegated to: Thomas Monjalon
Headers
Series pci/linux: use RTE_IOVA_VA whenever possible |

Checks

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

Commit Message

Stojaczyk, Dariusz Sept. 7, 2018, 2:53 p.m. UTC 
  This allows DPDK to use RTE_IOVA_VA with VFIO/UIO-bound PCI
devices present on the system, but not attached to any
rte_pci_driver at the time of init.

So far we used RTE_IOVA_VA whenever there was at least one
device attached to a driver with an RTE_PCI_DRV_IOVA_AS_VA flag,
meaning that other drivers which didn't explicitly report such
flag could have been forced to work in RTE_IOVA_VA as well.

This patch makes the RTE_PCI_DRV_IOVA_AS_VA explicitly a hint.
If it's set, but RTE_IOVA_VA cannot be used, then EAL will print
a proper warning.

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
---
 drivers/bus/pci/linux/pci.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

Comments

Thomas Monjalon Oct. 11, 2018, 9:48 a.m. UTC | #1 
Someone to review this patch please?


07/09/2018 16:53, Darek Stojaczyk:
> This allows DPDK to use RTE_IOVA_VA with VFIO/UIO-bound PCI
> devices present on the system, but not attached to any
> rte_pci_driver at the time of init.
> 
> So far we used RTE_IOVA_VA whenever there was at least one
> device attached to a driver with an RTE_PCI_DRV_IOVA_AS_VA flag,
> meaning that other drivers which didn't explicitly report such
> flag could have been forced to work in RTE_IOVA_VA as well.
> 
> This patch makes the RTE_PCI_DRV_IOVA_AS_VA explicitly a hint.
> If it's set, but RTE_IOVA_VA cannot be used, then EAL will print
> a proper warning.
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  drivers/bus/pci/linux/pci.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 04648ac93..961e24024 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -534,7 +534,7 @@ pci_one_device_bound_uio(void)
>   * Any one of the device has iova as va
>   */
>  static inline int
> -pci_one_device_has_iova_va(void)
> +pci_one_device_want_iova_va(void)
>  {
>  	struct rte_pci_device *dev = NULL;
>  	struct rte_pci_driver *drv = NULL;
> @@ -635,7 +635,7 @@ rte_pci_get_iommu_class(void)
>  {
>  	bool is_bound;
>  	bool is_vfio_noiommu_enabled = true;
> -	bool has_iova_va;
> +	bool want_iova_va;
>  	bool is_bound_uio;
>  	bool iommu_no_va;
>  
> @@ -643,7 +643,7 @@ rte_pci_get_iommu_class(void)
>  	if (!is_bound)
>  		return RTE_IOVA_DC;
>  
> -	has_iova_va = pci_one_device_has_iova_va();
> +	want_iova_va = pci_one_device_want_iova_va();
>  	is_bound_uio = pci_one_device_bound_uio();
>  	iommu_no_va = !pci_devices_iommu_support_va();
>  #ifdef VFIO_PRESENT
> @@ -651,11 +651,10 @@ rte_pci_get_iommu_class(void)
>  					true : false;
>  #endif
>  
> -	if (has_iova_va && !is_bound_uio && !is_vfio_noiommu_enabled &&
> -			!iommu_no_va)
> +	if (!is_bound_uio && !is_vfio_noiommu_enabled && !iommu_no_va)
>  		return RTE_IOVA_VA;
>  
> -	if (has_iova_va) {
> +	if (want_iova_va) {
>  		RTE_LOG(WARNING, EAL, "Some devices want iova as va but pa will be used because.. ");
>  		if (is_vfio_noiommu_enabled)
>  			RTE_LOG(WARNING, EAL, "vfio-noiommu mode configured\n");
>
Alejandro Lucero Oct. 11, 2018, 10 a.m. UTC | #2 
On Fri, Sep 7, 2018 at 3:55 PM Darek Stojaczyk <dariusz.stojaczyk@intel.com>
wrote:

> This allows DPDK to use RTE_IOVA_VA with VFIO/UIO-bound PCI
> devices present on the system, but not attached to any
> rte_pci_driver at the time of init.
>
> So far we used RTE_IOVA_VA whenever there was at least one
> device attached to a driver with an RTE_PCI_DRV_IOVA_AS_VA flag,
> meaning that other drivers which didn't explicitly report such
> flag could have been forced to work in RTE_IOVA_VA as well.
>

This is the opposite. Just one device not being able to use IOVA VA makes
all to use IOVA PA.


>
> This patch makes the RTE_PCI_DRV_IOVA_AS_VA explicitly a hint.
> If it's set, but RTE_IOVA_VA cannot be used, then EAL will print
> a proper warning.
>
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  drivers/bus/pci/linux/pci.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> index 04648ac93..961e24024 100644
> --- a/drivers/bus/pci/linux/pci.c
> +++ b/drivers/bus/pci/linux/pci.c
> @@ -534,7 +534,7 @@ pci_one_device_bound_uio(void)
>   * Any one of the device has iova as va
>   */
>  static inline int
> -pci_one_device_has_iova_va(void)
> +pci_one_device_want_iova_va(void)
>  {
>         struct rte_pci_device *dev = NULL;
>         struct rte_pci_driver *drv = NULL;
> @@ -635,7 +635,7 @@ rte_pci_get_iommu_class(void)
>  {
>         bool is_bound;
>         bool is_vfio_noiommu_enabled = true;
> -       bool has_iova_va;
> +       bool want_iova_va;
>         bool is_bound_uio;
>         bool iommu_no_va;
>
> @@ -643,7 +643,7 @@ rte_pci_get_iommu_class(void)
>         if (!is_bound)
>                 return RTE_IOVA_DC;
>
> -       has_iova_va = pci_one_device_has_iova_va();
> +       want_iova_va = pci_one_device_want_iova_va();
>         is_bound_uio = pci_one_device_bound_uio();
>         iommu_no_va = !pci_devices_iommu_support_va();
>  #ifdef VFIO_PRESENT
> @@ -651,11 +651,10 @@ rte_pci_get_iommu_class(void)
>                                         true : false;
>  #endif
>
> -       if (has_iova_va && !is_bound_uio && !is_vfio_noiommu_enabled &&
> -                       !iommu_no_va)
> +       if (!is_bound_uio && !is_vfio_noiommu_enabled && !iommu_no_va)
>                 return RTE_IOVA_VA;
>

This is wrong. A device not able to work with IOVA VA will fail.


>
> -       if (has_iova_va) {
> +       if (want_iova_va) {
>                 RTE_LOG(WARNING, EAL, "Some devices want iova as va but pa
> will be used because.. ");
>                 if (is_vfio_noiommu_enabled)
>                         RTE_LOG(WARNING, EAL, "vfio-noiommu mode
> configured\n");
> --
> 2.17.1
>
>
Anatoly Burakov Oct. 11, 2018, 10:26 a.m. UTC | #3 
On 11-Oct-18 11:00 AM, Alejandro Lucero wrote:
> On Fri, Sep 7, 2018 at 3:55 PM Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> wrote:
> 
>> This allows DPDK to use RTE_IOVA_VA with VFIO/UIO-bound PCI
>> devices present on the system, but not attached to any
>> rte_pci_driver at the time of init.
>>
>> So far we used RTE_IOVA_VA whenever there was at least one
>> device attached to a driver with an RTE_PCI_DRV_IOVA_AS_VA flag,
>> meaning that other drivers which didn't explicitly report such
>> flag could have been forced to work in RTE_IOVA_VA as well.
>>
> 
> This is the opposite. Just one device not being able to use IOVA VA makes
> all to use IOVA PA.
> 
> 
>>
>> This patch makes the RTE_PCI_DRV_IOVA_AS_VA explicitly a hint.
>> If it's set, but RTE_IOVA_VA cannot be used, then EAL will print
>> a proper warning.
>>
>> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
>> ---
>>   drivers/bus/pci/linux/pci.c | 11 +++++------
>>   1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
>> index 04648ac93..961e24024 100644
>> --- a/drivers/bus/pci/linux/pci.c
>> +++ b/drivers/bus/pci/linux/pci.c
>> @@ -534,7 +534,7 @@ pci_one_device_bound_uio(void)
>>    * Any one of the device has iova as va
>>    */
>>   static inline int
>> -pci_one_device_has_iova_va(void)
>> +pci_one_device_want_iova_va(void)
>>   {
>>          struct rte_pci_device *dev = NULL;
>>          struct rte_pci_driver *drv = NULL;
>> @@ -635,7 +635,7 @@ rte_pci_get_iommu_class(void)
>>   {
>>          bool is_bound;
>>          bool is_vfio_noiommu_enabled = true;
>> -       bool has_iova_va;
>> +       bool want_iova_va;
>>          bool is_bound_uio;
>>          bool iommu_no_va;
>>
>> @@ -643,7 +643,7 @@ rte_pci_get_iommu_class(void)
>>          if (!is_bound)
>>                  return RTE_IOVA_DC;
>>
>> -       has_iova_va = pci_one_device_has_iova_va();
>> +       want_iova_va = pci_one_device_want_iova_va();
>>          is_bound_uio = pci_one_device_bound_uio();
>>          iommu_no_va = !pci_devices_iommu_support_va();
>>   #ifdef VFIO_PRESENT
>> @@ -651,11 +651,10 @@ rte_pci_get_iommu_class(void)
>>                                          true : false;
>>   #endif
>>
>> -       if (has_iova_va && !is_bound_uio && !is_vfio_noiommu_enabled &&
>> -                       !iommu_no_va)
>> +       if (!is_bound_uio && !is_vfio_noiommu_enabled && !iommu_no_va)
>>                  return RTE_IOVA_VA;
>>
> 
> This is wrong. A device not able to work with IOVA VA will fail.
> 
> 
>>
>> -       if (has_iova_va) {
>> +       if (want_iova_va) {
>>                  RTE_LOG(WARNING, EAL, "Some devices want iova as va but pa
>> will be used because.. ");
>>                  if (is_vfio_noiommu_enabled)
>>                          RTE_LOG(WARNING, EAL, "vfio-noiommu mode
>> configured\n");
>> --
>> 2.17.1
>>
>>
> 

For these cases, i think the explicit IOVA mode on command line should 
work better. If the device has not reported IOVA as VA capability, it is 
to be assumed unsupported.
Jerin Jacob Oct. 11, 2018, 10:47 a.m. UTC | #4 
-----Original Message-----
> Date: Thu, 11 Oct 2018 11:26:05 +0100
> From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
> To: Alejandro Lucero <alejandro.lucero@netronome.com>,
>  dariusz.stojaczyk@intel.com
> CC: dev <dev@dpdk.org>, Santosh Shukla <santosh.shukla@caviumnetworks.com>,
>  Hemant Agrawal <hemant.agrawal@nxp.com>, Jerin Jacob
>  <jerin.jacob@caviumnetworks.com>, Maxime Coquelin
>  <maxime.coquelin@redhat.com>, chas3@att.com
> Subject: Re: [dpdk-dev] [PATCH] pci/linux: use RTE_IOVA_VA whenever possible
> User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
>  Thunderbird/52.9.1
> 
> 
> On 11-Oct-18 11:00 AM, Alejandro Lucero wrote:
> > On Fri, Sep 7, 2018 at 3:55 PM Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> > wrote:
> > 
> > > This allows DPDK to use RTE_IOVA_VA with VFIO/UIO-bound PCI
> > > devices present on the system, but not attached to any
> > > rte_pci_driver at the time of init.
> > > 
> > > So far we used RTE_IOVA_VA whenever there was at least one
> > > device attached to a driver with an RTE_PCI_DRV_IOVA_AS_VA flag,
> > > meaning that other drivers which didn't explicitly report such
> > > flag could have been forced to work in RTE_IOVA_VA as well.
> > > 
> > 
> > This is the opposite. Just one device not being able to use IOVA VA makes
> > all to use IOVA PA.
> > 
> > 
> > > 
> > > This patch makes the RTE_PCI_DRV_IOVA_AS_VA explicitly a hint.
> > > If it's set, but RTE_IOVA_VA cannot be used, then EAL will print
> > > a proper warning.
> > > 
> > > Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> > > ---
> > >   drivers/bus/pci/linux/pci.c | 11 +++++------
> > >   1 file changed, 5 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > > index 04648ac93..961e24024 100644
> > > --- a/drivers/bus/pci/linux/pci.c
> > > +++ b/drivers/bus/pci/linux/pci.c
> > > @@ -534,7 +534,7 @@ pci_one_device_bound_uio(void)
> > >    * Any one of the device has iova as va
> > >    */
> > >   static inline int
> > > -pci_one_device_has_iova_va(void)
> > > +pci_one_device_want_iova_va(void)
> > >   {
> > >          struct rte_pci_device *dev = NULL;
> > >          struct rte_pci_driver *drv = NULL;
> > > @@ -635,7 +635,7 @@ rte_pci_get_iommu_class(void)
> > >   {
> > >          bool is_bound;
> > >          bool is_vfio_noiommu_enabled = true;
> > > -       bool has_iova_va;
> > > +       bool want_iova_va;
> > >          bool is_bound_uio;
> > >          bool iommu_no_va;
> > > 
> > > @@ -643,7 +643,7 @@ rte_pci_get_iommu_class(void)
> > >          if (!is_bound)
> > >                  return RTE_IOVA_DC;
> > > 
> > > -       has_iova_va = pci_one_device_has_iova_va();
> > > +       want_iova_va = pci_one_device_want_iova_va();
> > >          is_bound_uio = pci_one_device_bound_uio();
> > >          iommu_no_va = !pci_devices_iommu_support_va();
> > >   #ifdef VFIO_PRESENT
> > > @@ -651,11 +651,10 @@ rte_pci_get_iommu_class(void)
> > >                                          true : false;
> > >   #endif
> > > 
> > > -       if (has_iova_va && !is_bound_uio && !is_vfio_noiommu_enabled &&
> > > -                       !iommu_no_va)
> > > +       if (!is_bound_uio && !is_vfio_noiommu_enabled && !iommu_no_va)
> > >                  return RTE_IOVA_VA;
> > > 
> > 
> > This is wrong. A device not able to work with IOVA VA will fail.
> > 
> > 
> > > 
> > > -       if (has_iova_va) {
> > > +       if (want_iova_va) {
> > >                  RTE_LOG(WARNING, EAL, "Some devices want iova as va but pa
> > > will be used because.. ");
> > >                  if (is_vfio_noiommu_enabled)
> > >                          RTE_LOG(WARNING, EAL, "vfio-noiommu mode
> > > configured\n");
> > > --
> > > 2.17.1
> > > 
> > > 
> > 
> 
> For these cases, i think the explicit IOVA mode on command line should
> work better. If the device has not reported IOVA as VA capability, it is
> to be assumed unsupported.

Yes.

> 
> --
> Thanks,
> Anatoly
Thomas Monjalon Oct. 28, 2018, 6:56 p.m. UTC | #5 
11/10/2018 12:47, Jerin Jacob:
> -----Original Message-----
> > Date: Thu, 11 Oct 2018 11:26:05 +0100
> > From: "Burakov, Anatoly" <anatoly.burakov@intel.com>
> > To: Alejandro Lucero <alejandro.lucero@netronome.com>,
> >  dariusz.stojaczyk@intel.com
> > CC: dev <dev@dpdk.org>, Santosh Shukla <santosh.shukla@caviumnetworks.com>,
> >  Hemant Agrawal <hemant.agrawal@nxp.com>, Jerin Jacob
> >  <jerin.jacob@caviumnetworks.com>, Maxime Coquelin
> >  <maxime.coquelin@redhat.com>, chas3@att.com
> > Subject: Re: [dpdk-dev] [PATCH] pci/linux: use RTE_IOVA_VA whenever possible
> > User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101
> >  Thunderbird/52.9.1
> > 
> > 
> > On 11-Oct-18 11:00 AM, Alejandro Lucero wrote:
> > > On Fri, Sep 7, 2018 at 3:55 PM Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> > > wrote:
> > > 
> > > > This allows DPDK to use RTE_IOVA_VA with VFIO/UIO-bound PCI
> > > > devices present on the system, but not attached to any
> > > > rte_pci_driver at the time of init.
> > > > 
> > > > So far we used RTE_IOVA_VA whenever there was at least one
> > > > device attached to a driver with an RTE_PCI_DRV_IOVA_AS_VA flag,
> > > > meaning that other drivers which didn't explicitly report such
> > > > flag could have been forced to work in RTE_IOVA_VA as well.
> > > > 
> > > 
> > > This is the opposite. Just one device not being able to use IOVA VA makes
> > > all to use IOVA PA.
> > > 
> > > 
> > > > 
> > > > This patch makes the RTE_PCI_DRV_IOVA_AS_VA explicitly a hint.
> > > > If it's set, but RTE_IOVA_VA cannot be used, then EAL will print
> > > > a proper warning.
> > > > 
> > > > Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> > > > ---
> > > >   drivers/bus/pci/linux/pci.c | 11 +++++------
> > > >   1 file changed, 5 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
> > > > index 04648ac93..961e24024 100644
> > > > --- a/drivers/bus/pci/linux/pci.c
> > > > +++ b/drivers/bus/pci/linux/pci.c
> > > > @@ -534,7 +534,7 @@ pci_one_device_bound_uio(void)
> > > >    * Any one of the device has iova as va
> > > >    */
> > > >   static inline int
> > > > -pci_one_device_has_iova_va(void)
> > > > +pci_one_device_want_iova_va(void)
> > > >   {
> > > >          struct rte_pci_device *dev = NULL;
> > > >          struct rte_pci_driver *drv = NULL;
> > > > @@ -635,7 +635,7 @@ rte_pci_get_iommu_class(void)
> > > >   {
> > > >          bool is_bound;
> > > >          bool is_vfio_noiommu_enabled = true;
> > > > -       bool has_iova_va;
> > > > +       bool want_iova_va;
> > > >          bool is_bound_uio;
> > > >          bool iommu_no_va;
> > > > 
> > > > @@ -643,7 +643,7 @@ rte_pci_get_iommu_class(void)
> > > >          if (!is_bound)
> > > >                  return RTE_IOVA_DC;
> > > > 
> > > > -       has_iova_va = pci_one_device_has_iova_va();
> > > > +       want_iova_va = pci_one_device_want_iova_va();
> > > >          is_bound_uio = pci_one_device_bound_uio();
> > > >          iommu_no_va = !pci_devices_iommu_support_va();
> > > >   #ifdef VFIO_PRESENT
> > > > @@ -651,11 +651,10 @@ rte_pci_get_iommu_class(void)
> > > >                                          true : false;
> > > >   #endif
> > > > 
> > > > -       if (has_iova_va && !is_bound_uio && !is_vfio_noiommu_enabled &&
> > > > -                       !iommu_no_va)
> > > > +       if (!is_bound_uio && !is_vfio_noiommu_enabled && !iommu_no_va)
> > > >                  return RTE_IOVA_VA;
> > > > 
> > > 
> > > This is wrong. A device not able to work with IOVA VA will fail.
> > > 
> > > 
> > > > 
> > > > -       if (has_iova_va) {
> > > > +       if (want_iova_va) {
> > > >                  RTE_LOG(WARNING, EAL, "Some devices want iova as va but pa
> > > > will be used because.. ");
> > > >                  if (is_vfio_noiommu_enabled)
> > > >                          RTE_LOG(WARNING, EAL, "vfio-noiommu mode
> > > > configured\n");
> > > > --
> > > > 2.17.1
> > > > 
> > > > 
> > > 
> > 
> > For these cases, i think the explicit IOVA mode on command line should
> > work better. If the device has not reported IOVA as VA capability, it is
> > to be assumed unsupported.
> 
> Yes.

Patch marked as rejected
diff mbox series

Patch

diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 04648ac93..961e24024 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -534,7 +534,7 @@  pci_one_device_bound_uio(void)
  * Any one of the device has iova as va
  */
 static inline int
-pci_one_device_has_iova_va(void)
+pci_one_device_want_iova_va(void)
 {
 	struct rte_pci_device *dev = NULL;
 	struct rte_pci_driver *drv = NULL;
@@ -635,7 +635,7 @@  rte_pci_get_iommu_class(void)
 {
 	bool is_bound;
 	bool is_vfio_noiommu_enabled = true;
-	bool has_iova_va;
+	bool want_iova_va;
 	bool is_bound_uio;
 	bool iommu_no_va;
 
@@ -643,7 +643,7 @@  rte_pci_get_iommu_class(void)
 	if (!is_bound)
 		return RTE_IOVA_DC;
 
-	has_iova_va = pci_one_device_has_iova_va();
+	want_iova_va = pci_one_device_want_iova_va();
 	is_bound_uio = pci_one_device_bound_uio();
 	iommu_no_va = !pci_devices_iommu_support_va();
 #ifdef VFIO_PRESENT
@@ -651,11 +651,10 @@  rte_pci_get_iommu_class(void)
 					true : false;
 #endif
 
-	if (has_iova_va && !is_bound_uio && !is_vfio_noiommu_enabled &&
-			!iommu_no_va)
+	if (!is_bound_uio && !is_vfio_noiommu_enabled && !iommu_no_va)
 		return RTE_IOVA_VA;
 
-	if (has_iova_va) {
+	if (want_iova_va) {
 		RTE_LOG(WARNING, EAL, "Some devices want iova as va but pa will be used because.. ");
 		if (is_vfio_noiommu_enabled)
 			RTE_LOG(WARNING, EAL, "vfio-noiommu mode configured\n");