DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] pci: fix non-Intel devices probing
@ 2013-09-16 20:29 Thomas Monjalon
  2013-09-16 20:38 ` Thomas Monjalon
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Thomas Monjalon @ 2013-09-16 20:29 UTC (permalink / raw)
  To: dev

From: David Marchand <david.marchand@6wind.com>

There is no need to check for bars mapping, especially BAR0 is not required.
If bars mapping failed, then pci_uio_map_resource will fail and we won't reach
this check. So get rid of BAR0 check.
Besides, pci_uio_map_resource should only be called for Intel devices.
The flag RTE_PCI_DRV_NEED_IGB_UIO is set for all Intel devices, even when
RTE_EAL_UNBIND_PORTS is disabled.

Signed-off-by: David Marchand <david.marchand@6wind.com>
Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test/test_pci.c                     |    2 --
 lib/librte_eal/common/include/rte_pci.h |    2 --
 lib/librte_eal/linuxapp/eal/eal_pci.c   |   30 +++++++-----------------------
 lib/librte_pmd_e1000/em_ethdev.c        |    2 --
 lib/librte_pmd_e1000/igb_ethdev.c       |    4 ----
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 ----
 6 files changed, 7 insertions(+), 37 deletions(-)

diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index 30d3c9f..55f603d 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -95,9 +95,7 @@ struct rte_pci_driver my_driver = {
 	.name = "test_driver",
 	.devinit = my_driver_init,
 	.id_table = my_driver_id,
-#ifdef RTE_EAL_UNBIND_PORTS
 	.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 };
 
 struct rte_pci_driver my_driver2 = {
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index c3ff5b9..affaae0 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -184,10 +184,8 @@ struct rte_pci_driver {
 	uint32_t drv_flags;                     /**< Flags contolling handling of device. */
 };
 
-#ifdef RTE_EAL_UNBIND_PORTS
 /** Device needs igb_uio kernel module */
 #define RTE_PCI_DRV_NEED_IGB_UIO 0x0001
-#endif
 /** Device driver must be registered several times until failure */
 #define RTE_PCI_DRV_MULTIPLE 0x0002
 /** Device needs to be unbound even if no module is provided */
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index eeb6cd7..998d5ba 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -913,13 +913,6 @@ int
 rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	struct rte_pci_id *id_table;
-#ifdef RTE_EAL_UNBIND_PORTS
-	const char *module_name = NULL;
-	int uio_status = -1;
-
-	if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
-		module_name = IGB_UIO_NAME;
-#endif
 
 	for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) {
 
@@ -953,10 +946,10 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 		}
 
 #ifdef RTE_EAL_UNBIND_PORTS
-		/* Unbind PCI devices if needed */
-		if (module_name != NULL)
+		/* Unbind Intel devices and load uio ressources */
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
 		{
-			if (pci_switch_module(dr, dev, uio_status, module_name) < 0)
+			if (pci_switch_module(dr, dev, 1, IGB_UIO_NAME) < 0)
 				return -1;
 		}
 		else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
@@ -966,21 +959,12 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 				return -1;
 		}
 #else
-		/* just map the NIC resources */
-		if (pci_uio_map_resource(dev) < 0)
-			return -1;
+		/* just map the NIC resources for Intel devices */
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
+			if (pci_uio_map_resource(dev) < 0)
+				return -1;
 #endif
 
-		/* We always should have BAR0 mapped */
-		if (!(dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND) &&
-		    (rte_eal_process_type() == RTE_PROC_PRIMARY) &&
-		    dev->mem_resource[0].addr == NULL) {
-			RTE_LOG(ERR, EAL,
-				"%s(): BAR0 is not mapped\n",
-				__func__);
-			return (-1);
-		}
- 
 		/* reference driver structure */
 		dev->driver = dr;
 
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index fc63557..4fccc1d 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -280,9 +280,7 @@ static struct eth_driver rte_em_pmd = {
 	{
 		.name = "rte_em_pmd",
 		.id_table = pci_id_em_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_em_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 85835f7..c4cdae9 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -522,9 +522,7 @@ static struct eth_driver rte_igb_pmd = {
 	{
 		.name = "rte_igb_pmd",
 		.id_table = pci_id_igb_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_igb_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),
@@ -537,9 +535,7 @@ static struct eth_driver rte_igbvf_pmd = {
 	{
 		.name = "rte_igbvf_pmd",
 		.id_table = pci_id_igbvf_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_igbvf_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index 516fed4..9235f9d 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -807,9 +807,7 @@ static struct eth_driver rte_ixgbe_pmd = {
 	{
 		.name = "rte_ixgbe_pmd",
 		.id_table = pci_id_ixgbe_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_ixgbe_dev_init,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
@@ -822,9 +820,7 @@ static struct eth_driver rte_ixgbevf_pmd = {
 	{
 		.name = "rte_ixgbevf_pmd",
 		.id_table = pci_id_ixgbevf_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_ixgbevf_dev_init,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] pci: fix non-Intel devices probing
  2013-09-16 20:29 [dpdk-dev] [PATCH] pci: fix non-Intel devices probing Thomas Monjalon
@ 2013-09-16 20:38 ` Thomas Monjalon
  2013-09-16 21:42 ` Patrick Mahan
  2013-09-17 10:21 ` [dpdk-dev] [PATCH v2 0/2] " Thomas Monjalon
  2 siblings, 0 replies; 10+ messages in thread
From: Thomas Monjalon @ 2013-09-16 20:38 UTC (permalink / raw)
  To: dev

> There is no need to check for bars mapping, especially BAR0 is not required.
> If bars mapping failed, then pci_uio_map_resource will fail and we won't
> reach this check. So get rid of BAR0 check.
> Besides, pci_uio_map_resource should only be called for Intel devices.
> The flag RTE_PCI_DRV_NEED_IGB_UIO is set for all Intel devices, even when
> RTE_EAL_UNBIND_PORTS is disabled.

Addendum:
This patch is needed to run virtio-net-pmd and vmxnet3-usermap with DPDK 1.4.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] pci: fix non-Intel devices probing
  2013-09-16 20:29 [dpdk-dev] [PATCH] pci: fix non-Intel devices probing Thomas Monjalon
  2013-09-16 20:38 ` Thomas Monjalon
@ 2013-09-16 21:42 ` Patrick Mahan
  2013-09-16 21:59   ` Thomas Monjalon
  2013-09-17 10:21 ` [dpdk-dev] [PATCH v2 0/2] " Thomas Monjalon
  2 siblings, 1 reply; 10+ messages in thread
From: Patrick Mahan @ 2013-09-16 21:42 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

I totally disagree with this statement.  I am currently working on a non-intel device that does need UIO support (I copied the igb UIO to create a new UIO driver).  This device not only needs bar0 but also bar1.  I've modified the eal PCI support code to support this behavior and change this behavior would not be good, IMHO. 

Patrick

Coming to you from deep inside Fortress Mahan

On Sep 16, 2013, at 1:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> From: David Marchand <david.marchand@6wind.com>
> 
> There is no need to check for bars mapping, especially BAR0 is not required.
> If bars mapping failed, then pci_uio_map_resource will fail and we won't reach
> this check. So get rid of BAR0 check.
> Besides, pci_uio_map_resource should only be called for Intel devices.
> The flag RTE_PCI_DRV_NEED_IGB_UIO is set for all Intel devices, even when
> RTE_EAL_UNBIND_PORTS is disabled.
> 
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
> ---
> app/test/test_pci.c                     |    2 --
> lib/librte_eal/common/include/rte_pci.h |    2 --
> lib/librte_eal/linuxapp/eal/eal_pci.c   |   30 +++++++-----------------------
> lib/librte_pmd_e1000/em_ethdev.c        |    2 --
> lib/librte_pmd_e1000/igb_ethdev.c       |    4 ----
> lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 ----
> 6 files changed, 7 insertions(+), 37 deletions(-)
> 
> diff --git a/app/test/test_pci.c b/app/test/test_pci.c
> index 30d3c9f..55f603d 100644
> --- a/app/test/test_pci.c
> +++ b/app/test/test_pci.c
> @@ -95,9 +95,7 @@ struct rte_pci_driver my_driver = {
>    .name = "test_driver",
>    .devinit = my_driver_init,
>    .id_table = my_driver_id,
> -#ifdef RTE_EAL_UNBIND_PORTS
>    .drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
> -#endif
> };
> 
> struct rte_pci_driver my_driver2 = {
> diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
> index c3ff5b9..affaae0 100644
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -184,10 +184,8 @@ struct rte_pci_driver {
>    uint32_t drv_flags;                     /**< Flags contolling handling of device. */
> };
> 
> -#ifdef RTE_EAL_UNBIND_PORTS
> /** Device needs igb_uio kernel module */
> #define RTE_PCI_DRV_NEED_IGB_UIO 0x0001
> -#endif
> /** Device driver must be registered several times until failure */
> #define RTE_PCI_DRV_MULTIPLE 0x0002
> /** Device needs to be unbound even if no module is provided */
> diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
> index eeb6cd7..998d5ba 100644
> --- a/lib/librte_eal/linuxapp/eal/eal_pci.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
> @@ -913,13 +913,6 @@ int
> rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
> {
>    struct rte_pci_id *id_table;
> -#ifdef RTE_EAL_UNBIND_PORTS
> -    const char *module_name = NULL;
> -    int uio_status = -1;
> -
> -    if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
> -        module_name = IGB_UIO_NAME;
> -#endif
> 
>    for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) {
> 
> @@ -953,10 +946,10 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>        }
> 
> #ifdef RTE_EAL_UNBIND_PORTS
> -        /* Unbind PCI devices if needed */
> -        if (module_name != NULL)
> +        /* Unbind Intel devices and load uio ressources */
> +        if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
>        {
> -            if (pci_switch_module(dr, dev, uio_status, module_name) < 0)
> +            if (pci_switch_module(dr, dev, 1, IGB_UIO_NAME) < 0)
>                return -1;
>        }
>        else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
> @@ -966,21 +959,12 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
>                return -1;
>        }
> #else
> -        /* just map the NIC resources */
> -        if (pci_uio_map_resource(dev) < 0)
> -            return -1;
> +        /* just map the NIC resources for Intel devices */
> +        if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
> +            if (pci_uio_map_resource(dev) < 0)
> +                return -1;
> #endif
> 
> -        /* We always should have BAR0 mapped */
> -        if (!(dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND) &&
> -            (rte_eal_process_type() == RTE_PROC_PRIMARY) &&
> -            dev->mem_resource[0].addr == NULL) {
> -            RTE_LOG(ERR, EAL,
> -                "%s(): BAR0 is not mapped\n",
> -                __func__);
> -            return (-1);
> -        }
> - 
>        /* reference driver structure */
>        dev->driver = dr;
> 
> diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
> index fc63557..4fccc1d 100644
> --- a/lib/librte_pmd_e1000/em_ethdev.c
> +++ b/lib/librte_pmd_e1000/em_ethdev.c
> @@ -280,9 +280,7 @@ static struct eth_driver rte_em_pmd = {
>    {
>        .name = "rte_em_pmd",
>        .id_table = pci_id_em_map,
> -#ifdef RTE_EAL_UNBIND_PORTS
>        .drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
> -#endif
>    },
>    .eth_dev_init = eth_em_dev_init,
>    .dev_private_size = sizeof(struct e1000_adapter),
> diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
> index 85835f7..c4cdae9 100644
> --- a/lib/librte_pmd_e1000/igb_ethdev.c
> +++ b/lib/librte_pmd_e1000/igb_ethdev.c
> @@ -522,9 +522,7 @@ static struct eth_driver rte_igb_pmd = {
>    {
>        .name = "rte_igb_pmd",
>        .id_table = pci_id_igb_map,
> -#ifdef RTE_EAL_UNBIND_PORTS
>        .drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
> -#endif
>    },
>    .eth_dev_init = eth_igb_dev_init,
>    .dev_private_size = sizeof(struct e1000_adapter),
> @@ -537,9 +535,7 @@ static struct eth_driver rte_igbvf_pmd = {
>    {
>        .name = "rte_igbvf_pmd",
>        .id_table = pci_id_igbvf_map,
> -#ifdef RTE_EAL_UNBIND_PORTS
>        .drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
> -#endif
>    },
>    .eth_dev_init = eth_igbvf_dev_init,
>    .dev_private_size = sizeof(struct e1000_adapter),
> diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> index 516fed4..9235f9d 100644
> --- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> +++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
> @@ -807,9 +807,7 @@ static struct eth_driver rte_ixgbe_pmd = {
>    {
>        .name = "rte_ixgbe_pmd",
>        .id_table = pci_id_ixgbe_map,
> -#ifdef RTE_EAL_UNBIND_PORTS
>        .drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
> -#endif
>    },
>    .eth_dev_init = eth_ixgbe_dev_init,
>    .dev_private_size = sizeof(struct ixgbe_adapter),
> @@ -822,9 +820,7 @@ static struct eth_driver rte_ixgbevf_pmd = {
>    {
>        .name = "rte_ixgbevf_pmd",
>        .id_table = pci_id_ixgbevf_map,
> -#ifdef RTE_EAL_UNBIND_PORTS
>        .drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
> -#endif
>    },
>    .eth_dev_init = eth_ixgbevf_dev_init,
>    .dev_private_size = sizeof(struct ixgbe_adapter),
> -- 
> 1.7.10.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] pci: fix non-Intel devices probing
  2013-09-16 21:42 ` Patrick Mahan
@ 2013-09-16 21:59   ` Thomas Monjalon
  2013-09-16 23:59     ` Stephen Hemminger
  0 siblings, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2013-09-16 21:59 UTC (permalink / raw)
  To: Patrick Mahan; +Cc: dev

16/09/2013 14:42, Patrick Mahan :
> On Sep 16, 2013, at 1:29 PM, Thomas Monjalon <thomas.monjalon@6wind.com> 
wrote:
> > There is no need to check for bars mapping, especially BAR0 is not
> > required. If bars mapping failed, then pci_uio_map_resource will fail and
> > we won't reach this check. So get rid of BAR0 check.
> > Besides, pci_uio_map_resource should only be called for Intel devices.
> > The flag RTE_PCI_DRV_NEED_IGB_UIO is set for all Intel devices, even when
> > RTE_EAL_UNBIND_PORTS is disabled.
> 
> I totally disagree with this statement.  I am currently working on a
> non-intel device that does need UIO support (I copied the igb UIO to create
> a new UIO driver).  This device not only needs bar0 but also bar1.  I've
> modified the eal PCI support code to support this behavior and change this
> behavior would not be good, IMHO.

I don't know which statement you are talking about.
You are writing a PMD based on UIO. Will you send it on dpdk.org ?
In this release, the only PMDs using UIO are Intel ones.
Also as explained, this check is never needed.
But I think you are talking about support of BAR mapping. This is not changed 
by this patch. Moreover, the BAR mapping is extended in coming release 1.4.1.

Note that it's simpler to discuss about a patch or a clearly pointed sentence. 
So, please be more precise or send your patch proposal.

PS: please don't top-post
-- 
Thomas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH] pci: fix non-Intel devices probing
  2013-09-16 21:59   ` Thomas Monjalon
@ 2013-09-16 23:59     ` Stephen Hemminger
  0 siblings, 0 replies; 10+ messages in thread
From: Stephen Hemminger @ 2013-09-16 23:59 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev

I agree with Thomas, the check is not needed.
After this all settles out, I have a patch to add support for I/O
resource tracking in base DPDK. It makes it easier to do the right
thing in drivers that need to use I/O instead of memory.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v2 0/2] fix non-Intel devices probing
  2013-09-16 20:29 [dpdk-dev] [PATCH] pci: fix non-Intel devices probing Thomas Monjalon
  2013-09-16 20:38 ` Thomas Monjalon
  2013-09-16 21:42 ` Patrick Mahan
@ 2013-09-17 10:21 ` Thomas Monjalon
  2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 1/2] pci: do not check BAR0 mapping Thomas Monjalon
  2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 2/2] pci: use igb_uio mapping only when needed Thomas Monjalon
  2 siblings, 2 replies; 10+ messages in thread
From: Thomas Monjalon @ 2013-09-17 10:21 UTC (permalink / raw)
  To: dev

This new version split the patch and improve some comments.

These patches will be included in the coming release 1.4.1r1.
They are needed to load vmxnet3-usermap and virtio-net-pmd with DPDK 1.4.

---

David Marchand (1):
  pci: do not check BAR0 mapping

Thomas Monjalon (1):
  pci: use igb_uio mapping only when needed

 app/test/test_pci.c                     |    2 --
 lib/librte_eal/common/include/rte_pci.h |    2 --
 lib/librte_eal/linuxapp/eal/eal_pci.c   |   29 +++++++----------------------
 lib/librte_pmd_e1000/em_ethdev.c        |    2 --
 lib/librte_pmd_e1000/igb_ethdev.c       |    4 ----
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 ----
 6 files changed, 7 insertions(+), 36 deletions(-)

-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v2 1/2] pci: do not check BAR0 mapping
  2013-09-17 10:21 ` [dpdk-dev] [PATCH v2 0/2] " Thomas Monjalon
@ 2013-09-17 10:21   ` Thomas Monjalon
  2013-09-17 11:26     ` Damien Millescamps
  2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 2/2] pci: use igb_uio mapping only when needed Thomas Monjalon
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2013-09-17 10:21 UTC (permalink / raw)
  To: dev

From: David Marchand <david.marchand@6wind.com>

Since DPDK 1.4, bars mapping is checked and prevent from initializing
drivers which do not use igb_uio mapping.
There is no need to check for bars mapping, especially BAR0 is not required.
If bars mapping failed, then pci_uio_map_resource will fail and we won't reach
this check. So get rid of BAR0 check.

Signed-off-by: David Marchand <david.marchand@6wind.com>
Acked-by: Stephen Hemminger <shemminger@vyatta.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c |    9 ---------
 1 file changed, 9 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 5c7f814..3913c65 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -963,15 +963,6 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			return -1;
 #endif
 
-		/* We always should have BAR0 mapped */
-		if (rte_eal_process_type() == RTE_PROC_PRIMARY && 
-			dev->mem_resource[0].addr == NULL) {
-			RTE_LOG(ERR, EAL,
-				"%s(): BAR0 is not mapped\n",
-				__func__);
-			return (-1);
-		}
- 
 		/* reference driver structure */
 		dev->driver = dr;
 
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [dpdk-dev] [PATCH v2 2/2] pci: use igb_uio mapping only when needed
  2013-09-17 10:21 ` [dpdk-dev] [PATCH v2 0/2] " Thomas Monjalon
  2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 1/2] pci: do not check BAR0 mapping Thomas Monjalon
@ 2013-09-17 10:21   ` Thomas Monjalon
  2013-09-17 11:27     ` Damien Millescamps
  1 sibling, 1 reply; 10+ messages in thread
From: Thomas Monjalon @ 2013-09-17 10:21 UTC (permalink / raw)
  To: dev

pci_uio_map_resource() should only be called for Intel devices
(using igb_uio kernel module).
The flag RTE_PCI_DRV_NEED_IGB_UIO is set for all those devices, even when
RTE_EAL_UNBIND_PORTS is disabled.

Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
---
 app/test/test_pci.c                     |    2 --
 lib/librte_eal/common/include/rte_pci.h |    2 --
 lib/librte_eal/linuxapp/eal/eal_pci.c   |   20 +++++++-------------
 lib/librte_pmd_e1000/em_ethdev.c        |    2 --
 lib/librte_pmd_e1000/igb_ethdev.c       |    4 ----
 lib/librte_pmd_ixgbe/ixgbe_ethdev.c     |    4 ----
 6 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index 30d3c9f..55f603d 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -95,9 +95,7 @@ struct rte_pci_driver my_driver = {
 	.name = "test_driver",
 	.devinit = my_driver_init,
 	.id_table = my_driver_id,
-#ifdef RTE_EAL_UNBIND_PORTS
 	.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 };
 
 struct rte_pci_driver my_driver2 = {
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 1314d20..3cfce1b 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -184,10 +184,8 @@ struct rte_pci_driver {
 	uint32_t drv_flags;                     /**< Flags contolling handling of device. */
 };
 
-#ifdef RTE_EAL_UNBIND_PORTS
 /** Device needs igb_uio kernel module */
 #define RTE_PCI_DRV_NEED_IGB_UIO 0x0001
-#endif
 /** Device driver must be registered several times until failure */
 #define RTE_PCI_DRV_MULTIPLE 0x0002
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 3913c65..ec6a7e2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -913,13 +913,6 @@ int
 rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
 {
 	struct rte_pci_id *id_table;
-#ifdef RTE_EAL_UNBIND_PORTS
-	const char *module_name = NULL;
-	int uio_status = -1;
-
-	if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
-		module_name = IGB_UIO_NAME;
-#endif
 
 	for (id_table = dr->id_table ; id_table->vendor_id != 0; id_table++) {
 
@@ -953,14 +946,15 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 		}
 
 #ifdef RTE_EAL_UNBIND_PORTS
-		/* Unbind PCI devices if needed */
-		if (module_name != NULL)
-			if (pci_switch_module(dr, dev, uio_status, module_name) < 0)
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
+			/* unbind driver and load uio resources for Intel NICs */
+			if (pci_switch_module(dr, dev, 1, IGB_UIO_NAME) < 0)
 				return -1;
 #else
-		/* just map the NIC resources */
-		if (pci_uio_map_resource(dev) < 0)
-			return -1;
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
+			/* just map resources for Intel NICs */
+			if (pci_uio_map_resource(dev) < 0)
+				return -1;
 #endif
 
 		/* reference driver structure */
diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_ethdev.c
index 9c6ff20..103c9cf 100644
--- a/lib/librte_pmd_e1000/em_ethdev.c
+++ b/lib/librte_pmd_e1000/em_ethdev.c
@@ -280,9 +280,7 @@ static struct eth_driver rte_em_pmd = {
 	{
 		.name = "rte_em_pmd",
 		.id_table = pci_id_em_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_em_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),
diff --git a/lib/librte_pmd_e1000/igb_ethdev.c b/lib/librte_pmd_e1000/igb_ethdev.c
index 14bdc2b..89709f6 100644
--- a/lib/librte_pmd_e1000/igb_ethdev.c
+++ b/lib/librte_pmd_e1000/igb_ethdev.c
@@ -522,9 +522,7 @@ static struct eth_driver rte_igb_pmd = {
 	{
 		.name = "rte_igb_pmd",
 		.id_table = pci_id_igb_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_igb_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),
@@ -537,9 +535,7 @@ static struct eth_driver rte_igbvf_pmd = {
 	{
 		.name = "rte_igbvf_pmd",
 		.id_table = pci_id_igbvf_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_igbvf_dev_init,
 	.dev_private_size = sizeof(struct e1000_adapter),
diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
index e26d5b9..d90cd92 100644
--- a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
+++ b/lib/librte_pmd_ixgbe/ixgbe_ethdev.c
@@ -807,9 +807,7 @@ static struct eth_driver rte_ixgbe_pmd = {
 	{
 		.name = "rte_ixgbe_pmd",
 		.id_table = pci_id_ixgbe_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_ixgbe_dev_init,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
@@ -822,9 +820,7 @@ static struct eth_driver rte_ixgbevf_pmd = {
 	{
 		.name = "rte_ixgbevf_pmd",
 		.id_table = pci_id_ixgbevf_map,
-#ifdef RTE_EAL_UNBIND_PORTS
 		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
-#endif
 	},
 	.eth_dev_init = eth_ixgbevf_dev_init,
 	.dev_private_size = sizeof(struct ixgbe_adapter),
-- 
1.7.10.4

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v2 1/2] pci: do not check BAR0 mapping
  2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 1/2] pci: do not check BAR0 mapping Thomas Monjalon
@ 2013-09-17 11:26     ` Damien Millescamps
  0 siblings, 0 replies; 10+ messages in thread
From: Damien Millescamps @ 2013-09-17 11:26 UTC (permalink / raw)
  To: dev

On 09/17/2013 12:21 PM, Thomas Monjalon wrote:
> From: David Marchand <david.marchand@6wind.com>
>
> Since DPDK 1.4, bars mapping is checked and prevent from initializing
> drivers which do not use igb_uio mapping.
> There is no need to check for bars mapping, especially BAR0 is not required.
> If bars mapping failed, then pci_uio_map_resource will fail and we won't reach
> this check. So get rid of BAR0 check.
>
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> Acked-by: Stephen Hemminger <shemminger@vyatta.com>
Acked-by: Damien Millescamps <damien.millescamps@6wind.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [dpdk-dev] [PATCH v2 2/2] pci: use igb_uio mapping only when needed
  2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 2/2] pci: use igb_uio mapping only when needed Thomas Monjalon
@ 2013-09-17 11:27     ` Damien Millescamps
  0 siblings, 0 replies; 10+ messages in thread
From: Damien Millescamps @ 2013-09-17 11:27 UTC (permalink / raw)
  To: dev

On 09/17/2013 12:21 PM, Thomas Monjalon wrote:
> pci_uio_map_resource() should only be called for Intel devices
> (using igb_uio kernel module).
> The flag RTE_PCI_DRV_NEED_IGB_UIO is set for all those devices, even when
> RTE_EAL_UNBIND_PORTS is disabled.
>
> Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com>
Acked-by: Damien Millescamps <damien.millescamps@6wind.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2013-09-17 11:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-09-16 20:29 [dpdk-dev] [PATCH] pci: fix non-Intel devices probing Thomas Monjalon
2013-09-16 20:38 ` Thomas Monjalon
2013-09-16 21:42 ` Patrick Mahan
2013-09-16 21:59   ` Thomas Monjalon
2013-09-16 23:59     ` Stephen Hemminger
2013-09-17 10:21 ` [dpdk-dev] [PATCH v2 0/2] " Thomas Monjalon
2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 1/2] pci: do not check BAR0 mapping Thomas Monjalon
2013-09-17 11:26     ` Damien Millescamps
2013-09-17 10:21   ` [dpdk-dev] [PATCH v2 2/2] pci: use igb_uio mapping only when needed Thomas Monjalon
2013-09-17 11:27     ` Damien Millescamps

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).