From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id CB198378B for ; Wed, 11 Oct 2017 16:19:35 +0200 (CEST) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 11 Oct 2017 07:19:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,361,1503385200"; d="scan'208";a="1229618904" Received: from dwdohert-mobl.ger.corp.intel.com (HELO [10.252.29.115]) ([10.252.29.115]) by fmsmga002.fm.intel.com with ESMTP; 11 Oct 2017 07:19:32 -0700 To: Gaetan Rivet , dev@dpdk.org References: From: "Doherty, Declan" Message-ID: <98afeb56-69b4-dcff-df5b-897d279c5072@intel.com> Date: Wed, 11 Oct 2017 15:19:31 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v2 00/14] Move PCI away from the EAL X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Oct 2017 14:19:36 -0000 On 18/09/2017 10:31 AM, Gaetan Rivet wrote: > Hi all, > > Here is a new version of the PCI bus move out of the EAL. > > The EAL PCI implementation is divided in two parts: > > - librte_pci: library offering helpers to handle PCI objects > - librte_bus_pci: bus driver for PCI devices > > This allows other libraries / tools to use PCI elements (location, mappings, > parsing operations, etc) without forcing a dependency on a bus driver. > > The latter should not have to export helpers that others might need. It > is focused on defining the rte_pci_device, rte_pci_driver objects and > their handling. > > The cryptodev library has hard dependencies on rte_pci_devices (used by > generic probe function). Other similar libs (ether and eventdev) avoided > the issue by inlining such functions and expecting users to include the > relevant headers once the PCI bus has already been built. > > @Declan: > I proposed a solution that would avoid inlining those functions, > which does not feel right. Let me know what you think of it or if you > think of a better solution. I think it would be best to have cryptodev > completely independent from PCI / vdev as far as the lib in concerned > (the vdev bus will move as well). > Hey Gaetan, apologies for the delay in getting back to you on this, I had been looking at this but got sidelined onto other issues before usersapce and I'm only getting back to it now. I think that while your solution works it just highlights the dependency which probably shouldn't be there between the cryptodev library and PCI devices. I've had a look, and the functions in the cryptodev which you moved don't really provide that much useful functionality. I done some testing and completely removed them and just update the QAT PMD which is the only crypto PMD which was using them and it seems much cleaner to me. I'll push a patch for this change later today and it will allow you to drop the patch "cryptodev: move PCI specific helpers to drivers/crypto" from this set. Regards Declan > v2: > > + Made rte_eal_using_phys_addrs common to both linux and bsd interfaces. > + Added documentation of EAL API changes in release note. > + Fixed a few rebase-related mistakes. > + Fixed parallel build race condition reported by Luca Boccassi. > + Grouped together commits breaking compilation: > > -> pci: introduce PCI lib and bus > -> lib: include rte_bus_pci > -> drivers: include rte_bus_pci > -> test: include rte_bus_pci > -> app/testpmd: include rte_bus_pci > -> cryptodev: move PCI specific helpers to drivers/crypto > > Until all of them have been applied, compilation is broken. > I am currently wondering whether merging some of them might > be sensible. > > + Not included in this series: > > Several filesystem-related functions are currently > private to the EAL and directly linked. This is not good, > but the solution seems to be to have a new lib offering an FS abstraction. > This seems an overreach for this patchset and should probably come in a > second step. > > Gaetan Rivet (14): > eal: expose rte_eal_using_phys_addrs > ethdev: remove useless PCI dependency > bus: properly include rte_debug > eal: remove references to PCI > pci: introduce PCI lib and bus > lib: include rte_bus_pci > drivers: include rte_bus_pci > test: include rte_bus_pci > app/testpmd: include rte_bus_pci > cryptodev: move PCI specific helpers to drivers/crypto > pci: avoid inlining functions > pci: avoid over-complicated macro > pci: deprecate misnamed functions > doc: add notes on EAL PCI API update > > app/test-pmd/testpmd.h | 1 + > config/common_base | 10 + > doc/guides/rel_notes/deprecation.rst | 10 + > doc/guides/rel_notes/release_17_11.rst | 28 + > drivers/Makefile | 2 +- > drivers/bus/Makefile | 2 + > drivers/bus/pci/Makefile | 59 ++ > drivers/bus/pci/bsd/Makefile | 32 + > drivers/bus/pci/bsd/rte_pci.c | 671 +++++++++++++++++++++ > drivers/bus/pci/include/rte_bus_pci.h | 387 ++++++++++++ > drivers/bus/pci/linux/Makefile | 37 ++ > drivers/bus/pci/linux/rte_pci.c | 723 +++++++++++++++++++++++ > drivers/bus/pci/linux/rte_pci_init.h | 97 +++ > drivers/bus/pci/linux/rte_pci_uio.c | 568 ++++++++++++++++++ > drivers/bus/pci/linux/rte_pci_vfio.c | 675 +++++++++++++++++++++ > drivers/bus/pci/linux/rte_vfio_mp_sync.c | 425 +++++++++++++ > drivers/bus/pci/private.h | 174 ++++++ > drivers/bus/pci/rte_bus_pci_version.map | 21 + > drivers/bus/pci/rte_pci_common.c | 543 +++++++++++++++++ > drivers/bus/pci/rte_pci_common_uio.c | 235 ++++++++ > drivers/crypto/Makefile | 4 +- > drivers/crypto/pci/Makefile | 52 ++ > drivers/crypto/pci/rte_cryptodev_pci.c | 128 ++++ > drivers/crypto/pci/rte_cryptodev_pci.h | 94 +++ > drivers/crypto/pci/rte_cryptodev_pci_version.map | 7 + > drivers/crypto/qat/qat_qp.c | 1 + > drivers/event/octeontx/ssovf_probe.c | 1 + > drivers/net/ark/ark_ethdev.c | 1 + > drivers/net/avp/avp_ethdev.c | 2 + > drivers/net/bnxt/bnxt.h | 1 + > drivers/net/bonding/rte_eth_bond_args.c | 1 + > drivers/net/cxgbe/base/adapter.h | 1 + > drivers/net/cxgbe/cxgbe_ethdev.c | 1 + > drivers/net/e1000/em_ethdev.c | 1 + > drivers/net/e1000/igb_ethdev.c | 1 + > drivers/net/e1000/igb_pf.c | 1 + > drivers/net/ena/ena_ethdev.h | 1 + > drivers/net/enic/base/vnic_dev.h | 4 +- > drivers/net/enic/enic_ethdev.c | 1 + > drivers/net/enic/enic_main.c | 1 + > drivers/net/i40e/i40e_ethdev.c | 1 + > drivers/net/i40e/i40e_ethdev_vf.c | 1 + > drivers/net/ixgbe/ixgbe_ethdev.c | 1 + > drivers/net/ixgbe/ixgbe_ethdev.h | 1 + > drivers/net/mlx5/mlx5.c | 1 + > drivers/net/mlx5/mlx5_ethdev.c | 1 + > drivers/net/sfc/sfc.h | 1 + > drivers/net/sfc/sfc_ethdev.c | 1 + > drivers/net/thunderx/nicvf_ethdev.c | 1 + > drivers/net/virtio/virtio_ethdev.c | 1 + > drivers/net/virtio/virtio_pci.h | 1 + > drivers/net/vmxnet3/vmxnet3_ethdev.c | 1 + > lib/Makefile | 2 + > lib/librte_cryptodev/Makefile | 1 - > lib/librte_cryptodev/rte_cryptodev_pci.h | 92 --- > lib/librte_cryptodev/rte_cryptodev_pmd.c | 94 --- > lib/librte_cryptodev/rte_cryptodev_version.map | 2 - > lib/librte_eal/bsdapp/eal/Makefile | 3 - > lib/librte_eal/bsdapp/eal/eal.c | 1 - > lib/librte_eal/bsdapp/eal/eal_memory.c | 6 + > lib/librte_eal/bsdapp/eal/eal_pci.c | 670 --------------------- > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 16 +- > lib/librte_eal/common/Makefile | 2 +- > lib/librte_eal/common/eal_common_bus.c | 1 + > lib/librte_eal/common/eal_common_pci.c | 580 ------------------ > lib/librte_eal/common/eal_common_pci_uio.c | 233 -------- > lib/librte_eal/common/eal_private.h | 143 ----- > lib/librte_eal/common/include/rte_memory.h | 11 + > lib/librte_eal/common/include/rte_pci.h | 598 ------------------- > lib/librte_eal/linuxapp/eal/Makefile | 10 - > lib/librte_eal/linuxapp/eal/eal.c | 1 - > lib/librte_eal/linuxapp/eal/eal_interrupts.c | 1 - > lib/librte_eal/linuxapp/eal/eal_memory.c | 2 +- > lib/librte_eal/linuxapp/eal/eal_pci.c | 722 ---------------------- > lib/librte_eal/linuxapp/eal/eal_pci_init.h | 97 --- > lib/librte_eal/linuxapp/eal/eal_pci_uio.c | 567 ------------------ > lib/librte_eal/linuxapp/eal/eal_pci_vfio.c | 674 --------------------- > lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c | 424 ------------- > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 16 +- > lib/librte_ether/rte_ethdev.c | 1 - > lib/librte_ether/rte_ethdev.h | 2 - > lib/librte_ether/rte_ethdev_pci.h | 1 + > lib/librte_eventdev/rte_eventdev_pmd_pci.h | 1 + > lib/librte_pci/Makefile | 48 ++ > lib/librte_pci/include/rte_pci.h | 285 +++++++++ > lib/librte_pci/rte_pci.c | 210 +++++++ > lib/librte_pci/rte_pci_version.map | 16 + > mk/rte.app.mk | 3 + > test/test/test_kni.c | 1 + > test/test/virtual_pmd.c | 1 + > 90 files changed, 5603 insertions(+), 4951 deletions(-) > create mode 100644 drivers/bus/pci/Makefile > create mode 100644 drivers/bus/pci/bsd/Makefile > create mode 100644 drivers/bus/pci/bsd/rte_pci.c > create mode 100644 drivers/bus/pci/include/rte_bus_pci.h > create mode 100644 drivers/bus/pci/linux/Makefile > create mode 100644 drivers/bus/pci/linux/rte_pci.c > create mode 100644 drivers/bus/pci/linux/rte_pci_init.h > create mode 100644 drivers/bus/pci/linux/rte_pci_uio.c > create mode 100644 drivers/bus/pci/linux/rte_pci_vfio.c > create mode 100644 drivers/bus/pci/linux/rte_vfio_mp_sync.c > create mode 100644 drivers/bus/pci/private.h > create mode 100644 drivers/bus/pci/rte_bus_pci_version.map > create mode 100644 drivers/bus/pci/rte_pci_common.c > create mode 100644 drivers/bus/pci/rte_pci_common_uio.c > create mode 100644 drivers/crypto/pci/Makefile > create mode 100644 drivers/crypto/pci/rte_cryptodev_pci.c > create mode 100644 drivers/crypto/pci/rte_cryptodev_pci.h > create mode 100644 drivers/crypto/pci/rte_cryptodev_pci_version.map > delete mode 100644 lib/librte_cryptodev/rte_cryptodev_pci.h > delete mode 100644 lib/librte_eal/bsdapp/eal/eal_pci.c > delete mode 100644 lib/librte_eal/common/eal_common_pci.c > delete mode 100644 lib/librte_eal/common/eal_common_pci_uio.c > delete mode 100644 lib/librte_eal/common/include/rte_pci.h > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci.c > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_init.h > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_uio.c > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_pci_vfio.c > delete mode 100644 lib/librte_eal/linuxapp/eal/eal_vfio_mp_sync.c > create mode 100644 lib/librte_pci/Makefile > create mode 100644 lib/librte_pci/include/rte_pci.h > create mode 100644 lib/librte_pci/rte_pci.c > create mode 100644 lib/librte_pci/rte_pci_version.map >