[dpdk-dev] [PATCH] pci: fix non-Intel devices probing

Patrick Mahan mahan at mahan.org
Mon Sep 16 23:42:47 CEST 2013


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 at 6wind.com> wrote:

> From: David Marchand <david.marchand at 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 at 6wind.com>
> Signed-off-by: Thomas Monjalon <thomas.monjalon at 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


More information about the dev mailing list