From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga18.intel.com (mga18.intel.com [134.134.136.126]) by dpdk.org (Postfix) with ESMTP id DFE114CA2 for ; Wed, 28 Feb 2018 15:44:18 +0100 (CET) X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Feb 2018 06:44:17 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.47,406,1515484800"; d="scan'208";a="34800376" Received: from bricha3-mobl3.ger.corp.intel.com ([10.237.221.77]) by orsmga001.jf.intel.com with SMTP; 28 Feb 2018 06:44:14 -0800 Received: by (sSMTP sendmail emulation); Wed, 28 Feb 2018 14:44:11 +0000 Date: Wed, 28 Feb 2018 14:44:10 +0000 From: Bruce Richardson To: Hemant Agrawal Cc: dev@dpdk.org, thomas@monjalon.net, shreyansh.jain@nxp.com, Akhil Goyal Message-ID: <20180228144409.GA10572@bricha3-MOBL3.ger.corp.intel.com> References: <1519752352-15442-1-git-send-email-hemant.agrawal@nxp.com> <1519752352-15442-8-git-send-email-hemant.agrawal@nxp.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1519752352-15442-8-git-send-email-hemant.agrawal@nxp.com> Organization: Intel Research and Development Ireland Ltd. User-Agent: Mutt/1.9.3 (2018-01-21) Subject: Re: [dpdk-dev] [PATCH 7/7] build: add meson support for dpaaX platforms 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, 28 Feb 2018 14:44:19 -0000 On Tue, Feb 27, 2018 at 10:55:52PM +0530, Hemant Agrawal wrote: > Signed-off-by: Akhil Goyal > Signed-off-by: Hemant Agrawal > --- Thanks for this. Some comments inline below. /Bruce > app/test-pmd/meson.build | 3 +++ > config/arm/arm64_dpaa2_linuxapp_gcc | 13 +++++++++++++ > config/arm/arm64_dpaa_linuxapp_gcc | 14 ++++++++++++++ > config/arm/meson.build | 13 +++++++++++++ > drivers/bus/dpaa/meson.build | 29 +++++++++++++++++++++++++++++ > drivers/bus/fslmc/meson.build | 28 ++++++++++++++++++++++++++++ > drivers/bus/meson.build | 4 ++-- > drivers/crypto/dpaa2_sec/meson.build | 16 ++++++++++++++++ > drivers/crypto/dpaa_sec/meson.build | 16 ++++++++++++++++ > drivers/crypto/meson.build | 3 +++ > drivers/event/dpaa/meson.build | 11 +++++++++++ > drivers/event/dpaa2/meson.build | 12 ++++++++++++ > drivers/event/meson.build | 2 +- > drivers/mempool/dpaa/meson.build | 9 +++++++++ > drivers/mempool/dpaa2/meson.build | 9 +++++++++ > drivers/mempool/meson.build | 2 +- > drivers/net/dpaa/meson.build | 15 +++++++++++++++ > drivers/net/dpaa2/meson.build | 16 ++++++++++++++++ > drivers/net/meson.build | 2 +- > 19 files changed, 212 insertions(+), 5 deletions(-) > create mode 100644 config/arm/arm64_dpaa2_linuxapp_gcc > create mode 100644 config/arm/arm64_dpaa_linuxapp_gcc > create mode 100644 drivers/bus/dpaa/meson.build > create mode 100644 drivers/bus/fslmc/meson.build > create mode 100644 drivers/crypto/dpaa2_sec/meson.build > create mode 100644 drivers/crypto/dpaa_sec/meson.build > create mode 100644 drivers/event/dpaa/meson.build > create mode 100644 drivers/event/dpaa2/meson.build > create mode 100644 drivers/mempool/dpaa/meson.build > create mode 100644 drivers/mempool/dpaa2/meson.build > create mode 100644 drivers/net/dpaa/meson.build > create mode 100644 drivers/net/dpaa2/meson.build > > diff --git a/app/test-pmd/meson.build b/app/test-pmd/meson.build > index 7ed74db..83f8bb1 100644 > --- a/app/test-pmd/meson.build > +++ b/app/test-pmd/meson.build > @@ -32,6 +32,9 @@ if dpdk_conf.has('RTE_LIBRTE_SOFTNIC_PMD') > sources += files('tm.c') > deps += 'pmd_softnic' > endif > +if dpdk_conf.has('RTE_LIBRTE_DPAA_PMD') > + deps += ['bus_dpaa', 'mempool_dpaa', 'pmd_dpaa'] > +endif > > dep_objs = [] > foreach d:deps > diff --git a/config/arm/arm64_dpaa2_linuxapp_gcc b/config/arm/arm64_dpaa2_linuxapp_gcc > new file mode 100644 > index 0000000..87337fb > --- /dev/null > +++ b/config/arm/arm64_dpaa2_linuxapp_gcc > @@ -0,0 +1,13 @@ > +[binaries] > +c = 'aarch64-linux-gnu-gcc' > +cpp = 'aarch64-linux-gnu-cpp' > +ar = 'aarch64-linux-gnu-gcc-ar' > + > +[host_machine] > +system = 'linux' > +cpu_family = 'aarch64' > +cpu = 'armv8-a' > +endian = 'little' > + > +[properties] > +implementor_id = 'dpaa2' > diff --git a/config/arm/arm64_dpaa_linuxapp_gcc b/config/arm/arm64_dpaa_linuxapp_gcc > new file mode 100644 > index 0000000..f769435 > --- /dev/null > +++ b/config/arm/arm64_dpaa_linuxapp_gcc > @@ -0,0 +1,14 @@ > +[binaries] > +c = 'aarch64-linux-gnu-gcc' > +cpp = 'aarch64-linux-gnu-cpp' > +ar = 'aarch64-linux-gnu-gcc-ar' > +strip = 'aarch64-linux-gnu-strip' > + > +[host_machine] > +system = 'linux' > +cpu_family = 'aarch64' > +cpu = 'armv8-a' > +endian = 'little' > + > +[properties] > +implementor_id = 'dpaa' Maybe separate the cross-build files out into a separate patch, since they are pretty independent of the rest of the changes. > diff --git a/config/arm/meson.build b/config/arm/meson.build > index 4e788a4..c1ab6ed 100644 > --- a/config/arm/meson.build > +++ b/config/arm/meson.build > @@ -54,6 +54,17 @@ flags_cavium = [ > ['RTE_MAX_LCORE', 96], > ['RTE_MAX_VFIO_GROUPS', 128], > ['RTE_RING_USE_C11_MEM_MODEL', false]] > +flags_dpaa = [ > + ['RTE_MACHINE', '"dpaa"'], > + ['RTE_CACHE_LINE_SIZE', 64], > + ['RTE_MAX_NUMA_NODES', 1], > + ['RTE_MAX_LCORE', 16]] > +flags_dpaa2 = [ > + ['RTE_MACHINE', '"dpaa2"'], > + ['RTE_CACHE_LINE_SIZE', 64], > + ['RTE_MAX_NUMA_NODES', 1], > + ['RTE_MAX_LCORE', 16], > + ['RTE_LIBRTE_DPAA2_USE_PHYS_IOVA', 'n']] > > ## Arm implementer ID (ARM DDI 0487C.a, Section G7.2.106, Page G7-5321) > impl_generic = ['Generic armv8', flags_generic, machine_args_generic] > @@ -69,6 +80,8 @@ impl_0x51 = ['Qualcomm', flags_generic, machine_args_generic] > impl_0x53 = ['Samsung', flags_generic, machine_args_generic] > impl_0x56 = ['Marvell', flags_generic, machine_args_generic] > impl_0x69 = ['Intel', flags_generic, machine_args_generic] > +impl_dpaa = ['NXP DPAA', flags_dpaa, machine_args_generic] > +impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic] > > > if cc.get_define('__clang__') != '' > diff --git a/drivers/bus/dpaa/meson.build b/drivers/bus/dpaa/meson.build > new file mode 100644 > index 0000000..c4257a5 > --- /dev/null > +++ b/drivers/bus/dpaa/meson.build > @@ -0,0 +1,29 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['pci', 'ethdev', 'eventdev'] > +sources = files('dpaa_bus.c', > + 'base/fman/fman.c', > + 'base/fman/fman_hw.c', > + 'base/fman/of.c', > + 'base/fman/netcfg_layer.c', > + 'base/qbman/process.c', > + 'base/qbman/bman.c', > + 'base/qbman/bman_driver.c', > + 'base/qbman/qman.c', > + 'base/qbman/qman_driver.c', > + 'base/qbman/dpaa_alloc.c', > + 'base/qbman/dpaa_sys.c') Minor nit - try to keep all files and list entries in alphabetical order. > + > +allow_experimental_apis = true > + > +if cc.has_argument('-Wno-cast-qual') > + cflags += '-Wno-cast-qual' > +endif > + > +includes += include_directories('include', 'base/qbman') > +cflags += ['-D_GNU_SOURCE'] > diff --git a/drivers/bus/fslmc/meson.build b/drivers/bus/fslmc/meson.build > new file mode 100644 > index 0000000..87475ee > --- /dev/null > +++ b/drivers/bus/fslmc/meson.build > @@ -0,0 +1,28 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['eal', 'ethdev', 'eventdev'] Another minor nit - eal isn't strictly necessary here as both ethdev and eventdev already depend on it, and dependencies are recursive. Explicitly calling out all dependencies is not wrong, but in previous prototyping I found that meson takes a lot longer to run when it has to sort through all the dependency chains. That's why in other libs and drivers I tried to keep the dependency lists to a minimum. As well as this, EAL is a standard dependency, so it's already in the deps array at this point. > +sources = files('qbman/qbman_portal.c', > + 'qbman/qbman_debug.c', > + 'mc/dpmng.c', > + 'mc/dpbp.c', > + 'mc/dpio.c', > + 'mc/mc_sys.c', > + 'mc/dpcon.c', > + 'mc/dpci.c', > + 'portal/dpaa2_hw_dpio.c', > + 'portal/dpaa2_hw_dpbp.c', > + 'portal/dpaa2_hw_dpci.c', > + 'fslmc_vfio.c', > + 'fslmc_bus.c') > + > +allow_experimental_apis = true > + > +includes += include_directories('../../../lib/librte_eal/linuxapp/eal') Is this not covered by the dependency on eal? Is it accessing things directly in the EAL internals? > +includes += include_directories('mc', 'qbman/include', 'portal') > +dpdk_conf.set('CONFIG_RTE_ARCH_ARM_TUNE', 'cortex-a72') This setting seems strange here? How is it used, and why set only inside this particular driver? > +cflags += ['-D_GNU_SOURCE'] > diff --git a/drivers/bus/meson.build b/drivers/bus/meson.build > index c6af500..2187f6b 100644 > --- a/drivers/bus/meson.build > +++ b/drivers/bus/meson.build > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2017 Intel Corporation > > -drivers = ['pci', 'vdev'] > -std_deps = ['eal'] > +drivers = ['pci', 'vdev', 'fslmc', 'dpaa'] Please keep alphabetical order. > +std_deps = ['eal', 'kvargs'] No big issue with this line change, but did you consider just making kvargs a dependency of the fslmc and dpaa buses directly, rather than making pci and vdev also depend on them? > config_flag_fmt = 'RTE_LIBRTE_@0@_BUS' > driver_name_fmt = 'rte_bus_@0@' > diff --git a/drivers/crypto/dpaa2_sec/meson.build b/drivers/crypto/dpaa2_sec/meson.build > new file mode 100644 > index 0000000..e80f15d > --- /dev/null > +++ b/drivers/crypto/dpaa2_sec/meson.build > @@ -0,0 +1,16 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['bus_fslmc', 'security', 'mempool_dpaa2'] > +sources = files('dpaa2_sec_dpseci.c', > + 'mc/dpseci.c') > + > +allow_experimental_apis = true > + > +includes += include_directories('mc', 'hw') > +dpdk_conf.set('RTE_DPAA2_SEC_PMD_MAX_NB_SESSIONS', 2048) I don't think this should be set here. For other libs and drivers, the constants are placed directly in config/rte_config.h (or put into a header file inside the driver itself). > +ext_deps += dep This line should be removed. You haven't assigned a dependency object to a variable called "dep" anywhere in this file. > diff --git a/drivers/crypto/dpaa_sec/meson.build b/drivers/crypto/dpaa_sec/meson.build > new file mode 100644 > index 0000000..10a7dd3 > --- /dev/null > +++ b/drivers/crypto/dpaa_sec/meson.build > @@ -0,0 +1,16 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['bus_dpaa', 'security'] > +sources = files('dpaa_sec.c') > + > +allow_experimental_apis = true > + > +includes += include_directories('../dpaa2_sec/') > +dpdk_conf.set('RTE_LIBRTE_DPAA_MAX_CRYPTODEV', 4) > +dpdk_conf.set('RTE_DPAA_SEC_PMD_MAX_NB_SESSIONS', 2048) As with other constants, I think this should go directly into rte_config.h, or in a header file in the driver directly. > +ext_deps += dep Drop this line. > diff --git a/drivers/crypto/meson.build b/drivers/crypto/meson.build > index 17041ad..a08bce5 100644 > --- a/drivers/crypto/meson.build > +++ b/drivers/crypto/meson.build > @@ -2,6 +2,9 @@ > # Copyright(c) 2017 Intel Corporation > > drivers = ['qat', 'null', 'openssl'] > + > + drivers += ['dpaa_sec', 'dpaa2_sec'] > + Don't indent. In other lists, we just allow list to flow onto multiple lines, rather than using +=. Using += is not wrong, it's just inconsistent, and again will break alphabetical ordering in this case. > std_deps = ['cryptodev'] # cryptodev pulls in all other needed deps > config_flag_fmt = 'RTE_LIBRTE_@0@_PMD' > driver_name_fmt = 'rte_pmd_@0@' > diff --git a/drivers/event/dpaa/meson.build b/drivers/event/dpaa/meson.build > new file mode 100644 > index 0000000..0a26d5d > --- /dev/null > +++ b/drivers/event/dpaa/meson.build > @@ -0,0 +1,11 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > +deps += ['mempool_dpaa', 'bus_dpaa', 'pmd_dpaa'] > +sources = files('dpaa_eventdev.c', > +) No need to move to a second line here. > + > +allow_experimental_apis = true > diff --git a/drivers/event/dpaa2/meson.build b/drivers/event/dpaa2/meson.build > new file mode 100644 > index 0000000..686b622 > --- /dev/null > +++ b/drivers/event/dpaa2/meson.build > @@ -0,0 +1,12 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > +deps += ['mempool_dpaa2', 'bus_fslmc', 'bus_vdev', 'pmd_dpaa2'] > +sources = files('dpaa2_hw_dpcon.c', > + 'dpaa2_eventdev.c' > +) > + > +allow_experimental_apis = true > diff --git a/drivers/event/meson.build b/drivers/event/meson.build > index d7bc485..f1c3678 100644 > --- a/drivers/event/meson.build > +++ b/drivers/event/meson.build > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2017 Intel Corporation > > -drivers = ['skeleton', 'sw', 'octeontx'] > +drivers = ['skeleton', 'sw', 'octeontx', 'dpaa', 'dpaa2'] alphabetically ordering, please. > std_deps = ['eventdev', 'kvargs'] > config_flag_fmt = 'RTE_LIBRTE_@0@_EVENTDEV_PMD' > driver_name_fmt = 'rte_pmd_@0@_event' > diff --git a/drivers/mempool/dpaa/meson.build b/drivers/mempool/dpaa/meson.build > new file mode 100644 > index 0000000..08423c2 > --- /dev/null > +++ b/drivers/mempool/dpaa/meson.build > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['bus_dpaa'] > +sources = files('dpaa_mempool.c') > diff --git a/drivers/mempool/dpaa2/meson.build b/drivers/mempool/dpaa2/meson.build > new file mode 100644 > index 0000000..402d087 > --- /dev/null > +++ b/drivers/mempool/dpaa2/meson.build > @@ -0,0 +1,9 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['mbuf', 'ethdev', 'kvargs', 'bus_fslmc'] > +sources = files('dpaa2_hw_mempool.c') > diff --git a/drivers/mempool/meson.build b/drivers/mempool/meson.build > index 5991856..47431cd 100644 > --- a/drivers/mempool/meson.build > +++ b/drivers/mempool/meson.build > @@ -1,7 +1,7 @@ > # SPDX-License-Identifier: BSD-3-Clause > # Copyright(c) 2017 Intel Corporation > > -drivers = ['ring', 'stack', 'octeontx'] > +drivers = ['ring', 'stack', 'octeontx', 'dpaa', 'dpaa2'] ordering. > std_deps = ['mempool'] > config_flag_fmt = 'RTE_LIBRTE_@0@_MEMPOOL' > driver_name_fmt = 'rte_mempool_@0@' > diff --git a/drivers/net/dpaa/meson.build b/drivers/net/dpaa/meson.build > new file mode 100644 > index 0000000..e5a2d17 > --- /dev/null > +++ b/drivers/net/dpaa/meson.build > @@ -0,0 +1,15 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > +deps += ['bus_dpaa'] > +deps += ['mempool_dpaa'] > + > +sources = files('dpaa_ethdev.c', > + 'dpaa_rxtx.c') > + > +allow_experimental_apis = true > + > +install_headers('rte_pmd_dpaa.h') > diff --git a/drivers/net/dpaa2/meson.build b/drivers/net/dpaa2/meson.build > new file mode 100644 > index 0000000..025c587 > --- /dev/null > +++ b/drivers/net/dpaa2/meson.build > @@ -0,0 +1,16 @@ > +# SPDX-License-Identifier: BSD-3-Clause > +# Copyright 2018 NXP > + > +if host_machine.system() != 'linux' > + build = false > +endif > + > +deps += ['bus_fslmc'] # same with vdev bus > +deps += ['mempool_dpaa2'] # same with vdev bus > +sources = files('dpaa2_ethdev.c', > + 'dpaa2_rxtx.c', > + 'base/dpaa2_hw_dpni.c', > + 'mc/dpni.c', > + 'mc/dpkg.c') > + > +includes += include_directories('base', 'mc') > diff --git a/drivers/net/meson.build b/drivers/net/meson.build > index 704cbe3..6d7a772 100644 > --- a/drivers/net/meson.build > +++ b/drivers/net/meson.build > @@ -4,7 +4,7 @@ > drivers = ['af_packet', 'bonding', > 'e1000', 'fm10k', 'i40e', 'ixgbe', > 'null', 'octeontx', 'pcap', 'ring', > - 'sfc', 'thunderx'] > + 'sfc', 'thunderx', 'dpaa', 'dpaa2'] ordering. > std_deps = ['ethdev', 'kvargs'] # 'ethdev' also pulls in mbuf, net, eal etc > std_deps += ['bus_pci'] # very many PMDs depend on PCI, so make std > std_deps += ['bus_vdev'] # same with vdev bus > -- > 2.7.4 >