From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from ns.mahan.org (unknown [67.116.10.138]) by dpdk.org (Postfix) with ESMTP id 85B711F3 for ; Mon, 16 Sep 2013 23:42:12 +0200 (CEST) Received: from [192.168.71.39] (localhost [127.0.0.1]) (authenticated bits=0) by ns.mahan.org (8.14.5/8.14.5) with ESMTP id r8GLgkJm019445; Mon, 16 Sep 2013 14:42:46 -0700 (PDT) (envelope-from mahan@mahan.org) References: <1379363340-20870-1-git-send-email-thomas.monjalon@6wind.com> Mime-Version: 1.0 (1.0) In-Reply-To: <1379363340-20870-1-git-send-email-thomas.monjalon@6wind.com> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: quoted-printable Message-Id: X-Mailer: iPad Mail (10B329) From: Patrick Mahan Date: Mon, 16 Sep 2013 14:42:47 -0700 To: Thomas Monjalon Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH] pci: fix non-Intel devices probing X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 16 Sep 2013 21:42:13 -0000 I totally disagree with this statement. I am currently working on a non-int= el device that does need UIO support (I copied the igb UIO to create a new U= IO driver). This device not only needs bar0 but also bar1. I've modified t= he eal PCI support code to support this behavior and change this behavior wo= uld not be good, IMHO.=20 Patrick Coming to you from deep inside Fortress Mahan On Sep 16, 2013, at 1:29 PM, Thomas Monjalon wro= te: > From: David Marchand >=20 > There is no need to check for bars mapping, especially BAR0 is not require= d. > If bars mapping failed, then pci_uio_map_resource will fail and we won't r= each > 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. >=20 > Signed-off-by: David Marchand > Signed-off-by: Thomas Monjalon > --- > 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(-) >=20 > 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 =3D { > .name =3D "test_driver", > .devinit =3D my_driver_init, > .id_table =3D my_driver_id, > -#ifdef RTE_EAL_UNBIND_PORTS > .drv_flags =3D RTE_PCI_DRV_NEED_IGB_UIO, > -#endif > }; >=20 > struct rte_pci_driver my_driver2 =3D { > diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/comm= on/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 o= f device. */ > }; >=20 > -#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/linuxa= pp/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_dev= ice *dev) > { > struct rte_pci_id *id_table; > -#ifdef RTE_EAL_UNBIND_PORTS > - const char *module_name =3D NULL; > - int uio_status =3D -1; > - > - if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO) > - module_name =3D IGB_UIO_NAME; > -#endif >=20 > for (id_table =3D dr->id_table ; id_table->vendor_id !=3D 0; id_table++= ) { >=20 > @@ -953,10 +946,10 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *= dr, struct rte_pci_device *d > } >=20 > #ifdef RTE_EAL_UNBIND_PORTS > - /* Unbind PCI devices if needed */ > - if (module_name !=3D 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 >=20 > - /* We always should have BAR0 mapped */ > - if (!(dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND) && > - (rte_eal_process_type() =3D=3D RTE_PROC_PRIMARY) && > - dev->mem_resource[0].addr =3D=3D NULL) { > - RTE_LOG(ERR, EAL, > - "%s(): BAR0 is not mapped\n", > - __func__); > - return (-1); > - } > -=20 > /* reference driver structure */ > dev->driver =3D dr; >=20 > diff --git a/lib/librte_pmd_e1000/em_ethdev.c b/lib/librte_pmd_e1000/em_et= hdev.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 =3D { > { > .name =3D "rte_em_pmd", > .id_table =3D pci_id_em_map, > -#ifdef RTE_EAL_UNBIND_PORTS > .drv_flags =3D RTE_PCI_DRV_NEED_IGB_UIO, > -#endif > }, > .eth_dev_init =3D eth_em_dev_init, > .dev_private_size =3D 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 =3D { > { > .name =3D "rte_igb_pmd", > .id_table =3D pci_id_igb_map, > -#ifdef RTE_EAL_UNBIND_PORTS > .drv_flags =3D RTE_PCI_DRV_NEED_IGB_UIO, > -#endif > }, > .eth_dev_init =3D eth_igb_dev_init, > .dev_private_size =3D sizeof(struct e1000_adapter), > @@ -537,9 +535,7 @@ static struct eth_driver rte_igbvf_pmd =3D { > { > .name =3D "rte_igbvf_pmd", > .id_table =3D pci_id_igbvf_map, > -#ifdef RTE_EAL_UNBIND_PORTS > .drv_flags =3D RTE_PCI_DRV_NEED_IGB_UIO, > -#endif > }, > .eth_dev_init =3D eth_igbvf_dev_init, > .dev_private_size =3D sizeof(struct e1000_adapter), > diff --git a/lib/librte_pmd_ixgbe/ixgbe_ethdev.c b/lib/librte_pmd_ixgbe/ix= gbe_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 =3D { > { > .name =3D "rte_ixgbe_pmd", > .id_table =3D pci_id_ixgbe_map, > -#ifdef RTE_EAL_UNBIND_PORTS > .drv_flags =3D RTE_PCI_DRV_NEED_IGB_UIO, > -#endif > }, > .eth_dev_init =3D eth_ixgbe_dev_init, > .dev_private_size =3D sizeof(struct ixgbe_adapter), > @@ -822,9 +820,7 @@ static struct eth_driver rte_ixgbevf_pmd =3D { > { > .name =3D "rte_ixgbevf_pmd", > .id_table =3D pci_id_ixgbevf_map, > -#ifdef RTE_EAL_UNBIND_PORTS > .drv_flags =3D RTE_PCI_DRV_NEED_IGB_UIO, > -#endif > }, > .eth_dev_init =3D eth_ixgbevf_dev_init, > .dev_private_size =3D sizeof(struct ixgbe_adapter), > --=20 > 1.7.10.4