[v5,1/8] config: add WFE config entry for aarch64
Checks
Commit Message
Add the RTE_USE_WFE configuration entry for aarch64, disabled by default.
It can be enabled selectively based on the performance benchmarking.
Signed-off-by: Gavin Hu <gavin.hu@arm.com>
Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
Reviewed-by: Steve Capper <steve.capper@arm.com>
Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
---
config/arm/meson.build | 1 +
config/common_base | 6 ++++++
2 files changed, 7 insertions(+)
Comments
On Thu, Sep 12, 2019 at 5:05 PM Gavin Hu <gavin.hu@arm.com> wrote:
>
> Add the RTE_USE_WFE configuration entry for aarch64, disabled by default.
s/RTE_USE_WFE/RTE_ARM_USE_WFE
> It can be enabled selectively based on the performance benchmarking.
>
> Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> Reviewed-by: Steve Capper <steve.capper@arm.com>
> Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
Does it make sense to add the Reviewed-by without CCing the people?
I understand, There may be an internal review before sending out to
mailing list, IMO, it better to give Reviewed-By in the mailing list.
Not sure about general practice and other people view.
> Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> ---
> config/arm/meson.build | 1 +
> config/common_base | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/config/arm/meson.build b/config/arm/meson.build
> index 979018e..18ecd53 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -116,6 +116,7 @@ impl_dpaa = ['NXP DPAA', flags_dpaa, machine_args_generic]
> impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic]
>
> dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> +dpdk_conf.set('RTE_ARM_USE_WFE', 0)
>
> if not dpdk_conf.get('RTE_ARCH_64')
> dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> diff --git a/config/common_base b/config/common_base
> index 8ef75c2..d4cf974 100644
> --- a/config/common_base
> +++ b/config/common_base
> @@ -570,6 +570,12 @@ CONFIG_RTE_CRYPTO_MAX_DEVS=64
> CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO=n
> CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG=n
>
> +# Use WFE instructions to implement the rte_wait_for_equal_xxx APIs,
> +# calling these APIs put the cores in low power state while waiting
> +# for the memory address to become equal to the expected value.
> +# This is supported only by aarch64.
> +CONFIG_RTE_ARM_USE_WFE=n
Since this comes as EAL config, IMO, it is better to move this to end
of EAL section.
i.e move after CONFIG_RTE_USE_LIBBSD
And I think, we should squash this patch to "eal: add the APIs to
wait until equall" as it single logical change.
> +
> #
> # Compile NXP CAAM JR crypto Driver
> #
> --
> 2.7.4
>
Hi Jerin,
Thanks for reviewing the series, my comments inline.
> -----Original Message-----
> From: Jerin Jacob <jerinjacobk@gmail.com>
> Sent: Thursday, September 12, 2019 11:49 PM
> To: Gavin Hu (Arm Technology China) <Gavin.Hu@arm.com>
> Cc: dev@dpdk.org; nd <nd@arm.com>; thomas@monjalon.net;
> stephen@networkplumber.org; hemant.agrawal@nxp.com;
> jerinj@marvell.com; Pavan Nikhilesh <pbhagavatula@marvell.com>;
> Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
> Subject: Re: [dpdk-dev] [PATCH v5 1/8] config: add WFE config entry for
> aarch64
>
> On Thu, Sep 12, 2019 at 5:05 PM Gavin Hu <gavin.hu@arm.com> wrote:
> >
> > Add the RTE_USE_WFE configuration entry for aarch64, disabled by default.
>
> s/RTE_USE_WFE/RTE_ARM_USE_WFE
Will fix in next version.
>
> > It can be enabled selectively based on the performance benchmarking.
> >
> > Signed-off-by: Gavin Hu <gavin.hu@arm.com>
> > Reviewed-by: Ruifeng Wang <ruifeng.wang@arm.com>
> > Reviewed-by: Steve Capper <steve.capper@arm.com>
> > Reviewed-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> > Reviewed-by: Phil Yang <phil.yang@arm.com>
>
> Does it make sense to add the Reviewed-by without CCing the people?
I will cc all the internal reviewers for the future patches.
>
> I understand, There may be an internal review before sending out to
> mailing list, IMO, it better to give Reviewed-By in the mailing list.
> Not sure about general practice and other people view.
If there is a guideline here, we will strictly follow it. Currently both ways are ok?
>
>
> > Acked-by: Pavan Nikhilesh <pbhagavatula@marvell.com>
> > ---
> > config/arm/meson.build | 1 +
> > config/common_base | 6 ++++++
> > 2 files changed, 7 insertions(+)
> >
> > diff --git a/config/arm/meson.build b/config/arm/meson.build
> > index 979018e..18ecd53 100644
> > --- a/config/arm/meson.build
> > +++ b/config/arm/meson.build
> > @@ -116,6 +116,7 @@ impl_dpaa = ['NXP DPAA', flags_dpaa,
> machine_args_generic]
> > impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic]
> >
> > dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
> > +dpdk_conf.set('RTE_ARM_USE_WFE', 0)
> >
> > if not dpdk_conf.get('RTE_ARCH_64')
> > dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
> > diff --git a/config/common_base b/config/common_base
> > index 8ef75c2..d4cf974 100644
> > --- a/config/common_base
> > +++ b/config/common_base
> > @@ -570,6 +570,12 @@ CONFIG_RTE_CRYPTO_MAX_DEVS=64
> > CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO=n
> > CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG=n
> >
> > +# Use WFE instructions to implement the rte_wait_for_equal_xxx APIs,
> > +# calling these APIs put the cores in low power state while waiting
> > +# for the memory address to become equal to the expected value.
> > +# This is supported only by aarch64.
> > +CONFIG_RTE_ARM_USE_WFE=n
>
> Since this comes as EAL config, IMO, it is better to move this to end
> of EAL section.
> i.e move after CONFIG_RTE_USE_LIBBSD
Will fix it in next version.
> And I think, we should squash this patch to "eal: add the APIs to
> wait until equall" as it single logical change.
Will fix it in next version.
>
> > +
> > #
> > # Compile NXP CAAM JR crypto Driver
> > #
> > --
> > 2.7.4
> >
@@ -116,6 +116,7 @@ impl_dpaa = ['NXP DPAA', flags_dpaa, machine_args_generic]
impl_dpaa2 = ['NXP DPAA2', flags_dpaa2, machine_args_generic]
dpdk_conf.set('RTE_FORCE_INTRINSICS', 1)
+dpdk_conf.set('RTE_ARM_USE_WFE', 0)
if not dpdk_conf.get('RTE_ARCH_64')
dpdk_conf.set('RTE_CACHE_LINE_SIZE', 64)
@@ -570,6 +570,12 @@ CONFIG_RTE_CRYPTO_MAX_DEVS=64
CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO=n
CONFIG_RTE_LIBRTE_PMD_ARMV8_CRYPTO_DEBUG=n
+# Use WFE instructions to implement the rte_wait_for_equal_xxx APIs,
+# calling these APIs put the cores in low power state while waiting
+# for the memory address to become equal to the expected value.
+# This is supported only by aarch64.
+CONFIG_RTE_ARM_USE_WFE=n
+
#
# Compile NXP CAAM JR crypto Driver
#