DPDK patches and discussions
 help / color / mirror / Atom feed
From: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
To: "yskoh@mellanox.com" <yskoh@mellanox.com>,
	"jerinj@marvell.com" <jerinj@marvell.com>
Cc: "bruce.richardson@intel.com" <bruce.richardson@intel.com>,
	Pavan Nikhilesh Bhagavatula <pbhagavatula@marvell.com>,
	Shahaf Shuler <shahafs@mellanox.com>,
	 "dev@dpdk.org" <dev@dpdk.org>,
	"thomas@monjalon.net" <thomas@monjalon.net>,
	"Gavin Hu (Arm Technology China)" <Gavin.Hu@arm.com>,
	nd <nd@arm.com>,
	Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>,
	nd <nd@arm.com>
Subject: Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
Date: Fri, 3 May 2019 03:54:09 +0000	[thread overview]
Message-ID: <VE1PR08MB51491D19A8F2671652BF477398350@VE1PR08MB5149.eurprd08.prod.outlook.com> (raw)
Message-ID: <20190503035409.g3TKGejJfniPQgyiCrGKkbo0loyZ8KVOjeJwFuCAIiE@z> (raw)
In-Reply-To: <926D3AC3-CA01-410A-8E23-4AB6581FA594@mellanox.com>

> >>> On Apr 15, 2019, at 1:13 PM, Honnappa Nagarahalli
> >>> <Honnappa.Nagarahalli@arm.com> wrote:
> >>>
> >>>>>>> Subject: [EXT] [PATCH 5/6] build: add option for armv8 crypto
> >>>>>>> extension
> >>>>>>>
> >>>>>>> CONFIG_RTE_MACHINE="armv8a"
> >>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=y
> >>>>>>
> >>>>>> This approach is not scalable. Even, it is not good for BlueField
> >>>>>> as you you need to maintain two images.
> >>>>>>
> >>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optional_.
> >>>>>> Access to crypto instructions is always at under runtime check.
> >>>>>> See the following in rte_armv8_pmd.c
> >>>>>>
> >>>>>>
> >>>>>>   /* Check CPU for support for AES instruction set */
> >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_AES)) {
> >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> >>>>>>           "AES instructions not supported by CPU");
> >>>>>>       return -EFAULT;
> >>>>>>   }
> >>>>>>
> >>>>>>   /* Check CPU for support for SHA instruction set */
> >>>>>>   if (!rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA1) ||
> >>>>>>       !rte_cpu_get_flag_enabled(RTE_CPUFLAG_SHA2)) {
> >>>>>>       ARMV8_CRYPTO_LOG_ERR(
> >>>>>>           "SHA1/SHA2 instructions not supported by CPU");
> >>>>>>       return -EFAULT;
> >>>>>>   }
> >>>>>>
> >>>>>> So In order to avoid one more config flags specific to armv8 in
> >>>>>> meson and makefile build infra And avoid the need for 6/6 patch.
> >>>>>> IMO, # Introduce optional CPU flag scheme in eal. Treat armv8
> >>>>>> crypto as optional flag # Skip the eal init check for optional flag.
> >>>>>>
> >>>>>> Do you see any issues with that approach?
> >>>>>
> >>>>> I also thought about that approach and that was my number 1 priority.
> >>>>> But, I had one question came to my mind. Maybe, arm people can
> >>>>> confirm it. Is it 100% guaranteed that compiler never makes use of
> >>>>> any of crypto instructions even if there's no specific
> >>>>> asm/intrinsic code?  The crypto extension has aes, pmull,
> >>>>> sha1 and sha2. In case of rte_memcpy() for x86, for example,
> >>>>> compiler may optimize code using avx512f instructions even though
> >>>>> it is written specifically with avx2 intrinsics (__mm256_*) unless
> >>>>> avx512f is
> >>> disabled.
> >>>>>
> >>>>> If a complier expert in arm (or anyone else) confirm it is
> >>>>> completely **optional**, then I'd love to take that approach for sure.
> >>>>>
> >>>>> Copied dpdk-on-arm ML.
> >>>>>
> >>>> I do not know the answer, will have to check with the compiler team.
> >>>> I will get
> >>> back on this.
> >>>
> >>> Any update yet?
> >> Currently, enabling 'crypto' flag will generate the crypto
> >> instructions only when crypto intrinsics are used. However, when
> >> 'sha3' (part of 8.2 crypto) flag is
> >
> > The default image is 8.1 spec and except octeontx2 every other SoC is
I am not following this. I think the default image is 8.0.

> > 8.1 and For octeotx2 crypto is supported. If so, Should we worry this case?
I assume we all are talking about the distro/binary portable build. IMO, we should not just look at the existing SoCs.
The CPU specific builds have the freedom to compile as per their corresponding support.

> 
> Right, it sounds to me that we can disable the option without having the new
> config flag until such instructions get needed. According to gcc-8 release note
> [1], currently '+crypto' implies '+aes' and '+sha2' while '+sha3' and '+sm4' are
> newly introduced. Given that armv8 crypto PMD uses external binary of
> Marvell. I don't see any reason to enable '+crypto'. How about simply disable
> it from armv8 build configs?
I think it should be fine. But, this alone is not enough. The run time detection of the crypto feature and hooking up the correct pointers needs to be added.

> 
> diff --git a/config/arm/meson.build b/config/arm/meson.build index
> 7fa6ed3105..abc8cf346c 100644
> --- a/config/arm/meson.build
> +++ b/config/arm/meson.build
> @@ -74,7 +74,7 @@ flags_octeontx2_extra = [
>         ['RTE_USE_C11_MEM_MODEL', true]]
> 
>  machine_args_generic = [
> -       ['default', ['-march=armv8-a+crc+crypto']],
> +       ['default', ['-march=armv8-a+crc']],
>         ['native', ['-march=native']],
>         ['0xd03', ['-mcpu=cortex-a53']],
>         ['0xd04', ['-mcpu=cortex-a35']], diff --git
> a/mk/machine/armv8a/rte.vars.mk b/mk/machine/armv8a/rte.vars.mk index
> 8252efbb7b..5e3ffc3adf 100644
> --- a/mk/machine/armv8a/rte.vars.mk
> +++ b/mk/machine/armv8a/rte.vars.mk
> @@ -28,4 +28,4 @@
>  # CPU_LDFLAGS =
>  # CPU_ASFLAGS =
> 
> -MACHINE_CFLAGS += -march=armv8-a+crc+crypto
> +MACHINE_CFLAGS += -march=armv8-a+crc
> 
> 
> [1] https://gcc.gnu.org/gcc-8/changes.html
> 
> Thanks,
> Yongseok
> 
> >> enabled, compiler can generate 3-way exclusive OR instructions beyond
> >> the intrinsics.
> >
> > The very same problem will be applicable for Linux kernel too for
> distribution binary case.
> > If the above statement is true about 8.2 crypto and crypto generation
> > without Intrinsics then we need to see how linux kernel handling that
> > and align our solution based on that.
Yes, the compiler team cited Linux kernel example, I have not verified it myself.

> >
> >> Compiler team cannot provide a guarantee that other crypto
> >> instructions will not be used beyond the intrinsics.
> >>
> >> The current suggestion is to use GNU indirect function [1] or
> >> similar. I am not
> >
> > Not sure how it helps? If we know the compiler is generating a
> > specific function With crypto instruction then we can generate
> > _alternative_ function for the same With hwcap?.How do we know which
> function compiler using compiler instructions?
This feature is similar to using function pointers and choosing which function pointer to use at run time. If this feature is used, the function pointer to use is decided during dynamic linking stage.
Either ways, we need to have 2 sets of crypto PMD drivers. One that implements the actual functionality using crypto intrinsics/assembly. Only, this code needs to be compiled with '+crypto'. Second driver that implements just stubs and returns error. This code will be compiled without '+crypto'. At run time, depending on the HWCAP, the correct driver/function pointers need to be hooked up.

> >
> >
> >> sure on GNU indirect function portability.
> >
> > We are using HWCAP scheme, So we may not need the very exact GNU
> > indirect scheme to fix the issue.
Agree, using indirect functions is not a must.

> >
> >>
> >> [1]
> >> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwil
> >> lnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-
> functions%2F&amp;d
> >>
> ata=02%7C01%7Cyskoh%40mellanox.com%7Cda8fb7ed03e7406ded8908d6c
> ee6d759
> >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63692388818
> 9316743&amp;
> >>
> sdata=x5XNd5WZ3EtiprPMiFzaskvigX8K0AoXA2w%2BKiN156c%3D&amp;res
> erved=0
> >>
> >>>
> >>> Thanks
> >>> Yongseok


  parent reply	other threads:[~2019-05-03  3:54 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-04-12 23:24 [dpdk-dev] [PATCH 0/6] build: fix build for arm64 Yongseok Koh
2019-04-12 23:24 ` Yongseok Koh
2019-04-12 23:24 ` [dpdk-dev] [PATCH 1/6] meson: disable octeontx for buggy compilers on arm64 Yongseok Koh
2019-04-12 23:24   ` Yongseok Koh
2019-04-13  5:52   ` [dpdk-dev] [EXT] " Pavan Nikhilesh Bhagavatula
2019-04-13  5:52     ` Pavan Nikhilesh Bhagavatula
2019-04-15 18:16     ` Yongseok Koh
2019-04-15 18:16       ` Yongseok Koh
2019-04-12 23:24 ` [dpdk-dev] [PATCH 2/6] meson: change default cache line size for cortex-a72 Yongseok Koh
2019-04-12 23:24   ` Yongseok Koh
2019-04-13  6:43   ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-04-13  6:43     ` Jerin Jacob Kollanukkaran
2019-04-15  4:35     ` Honnappa Nagarahalli
2019-04-15  4:35       ` Honnappa Nagarahalli
2019-04-15 13:40       ` Honnappa Nagarahalli
2019-04-15 13:40         ` Honnappa Nagarahalli
2019-04-15 20:40         ` Yongseok Koh
2019-04-15 20:40           ` Yongseok Koh
2019-04-15 20:44           ` Honnappa Nagarahalli
2019-04-15 20:44             ` Honnappa Nagarahalli
2019-04-12 23:24 ` [dpdk-dev] [PATCH 3/6] net/mlx: fix library search in meson build Yongseok Koh
2019-04-12 23:24   ` Yongseok Koh
2019-04-15  9:19   ` Bruce Richardson
2019-04-15  9:19     ` Bruce Richardson
2019-04-15 19:48     ` Yongseok Koh
2019-04-15 19:48       ` Yongseok Koh
2019-04-15 10:12   ` Luca Boccassi
2019-04-15 10:12     ` Luca Boccassi
2019-04-15 19:48     ` Yongseok Koh
2019-04-15 19:48       ` Yongseok Koh
2019-04-18  9:25       ` Luca Boccassi
2019-04-18  9:25         ` Luca Boccassi
2019-04-18 10:14         ` Bruce Richardson
2019-04-18 10:14           ` Bruce Richardson
2019-04-18 11:25           ` Yongseok Koh
2019-04-18 11:25             ` Yongseok Koh
2019-04-12 23:24 ` [dpdk-dev] [PATCH 4/6] meson: add Mellanox BlueField cross-compile config Yongseok Koh
2019-04-12 23:24   ` Yongseok Koh
2019-04-13  7:04   ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-04-13  7:04     ` Jerin Jacob Kollanukkaran
2019-04-12 23:24 ` [dpdk-dev] [PATCH 5/6] build: add option for armv8 crypto extension Yongseok Koh
2019-04-12 23:24   ` Yongseok Koh
2019-04-13  7:22   ` [dpdk-dev] [EXT] " Jerin Jacob Kollanukkaran
2019-04-13  7:22     ` Jerin Jacob Kollanukkaran
2019-04-15  4:52     ` Honnappa Nagarahalli
2019-04-15  4:52       ` Honnappa Nagarahalli
2019-04-15 18:43     ` Yongseok Koh
2019-04-15 18:43       ` Yongseok Koh
2019-04-15 20:13       ` Honnappa Nagarahalli
2019-04-15 20:13         ` Honnappa Nagarahalli
2019-04-17 16:28         ` Yongseok Koh
2019-04-17 16:28           ` Yongseok Koh
2019-04-30  3:33           ` Honnappa Nagarahalli
2019-04-30  3:33             ` Honnappa Nagarahalli
2019-05-02  1:54             ` Yongseok Koh
2019-05-02  1:54               ` Yongseok Koh
2019-05-02 10:13             ` Jerin Jacob Kollanukkaran
2019-05-02 10:13               ` Jerin Jacob Kollanukkaran
2019-05-02 23:08               ` Yongseok Koh
2019-05-02 23:08                 ` Yongseok Koh
2019-05-02 23:33                 ` Yongseok Koh
2019-05-02 23:33                   ` Yongseok Koh
2019-05-03 10:28                   ` Jerin Jacob Kollanukkaran
2019-05-03 10:28                     ` Jerin Jacob Kollanukkaran
2019-05-03  3:54                 ` Honnappa Nagarahalli [this message]
2019-05-03  3:54                   ` Honnappa Nagarahalli
2019-05-03  9:49                   ` Yongseok Koh
2019-05-03  9:49                     ` Yongseok Koh
2019-05-03 14:21                     ` Honnappa Nagarahalli
2019-05-03 14:21                       ` Honnappa Nagarahalli
2019-04-12 23:24 ` [dpdk-dev] [PATCH 6/6] mk: disable armv8 crypto extension for Mellanox BlueField Yongseok Koh
2019-04-12 23:24   ` Yongseok Koh
2019-04-13  7:12 ` [dpdk-dev] [EXT] [PATCH 0/6] build: fix build for arm64 Jerin Jacob Kollanukkaran
2019-04-13  7:12   ` Jerin Jacob Kollanukkaran
2019-04-15 20:56   ` Yongseok Koh
2019-04-15 20:56     ` Yongseok Koh
2019-04-16  5:57     ` Jerin Jacob Kollanukkaran
2019-04-16  5:57       ` Jerin Jacob Kollanukkaran
2019-04-17 20:06 ` [dpdk-dev] " Thomas Monjalon
2019-04-17 20:06   ` Thomas Monjalon
2019-04-17 20:24   ` Honnappa Nagarahalli
2019-04-17 20:24     ` Honnappa Nagarahalli
2019-04-17 22:14   ` Yongseok Koh
2019-04-17 22:14     ` Yongseok Koh
2019-04-18  1:47 ` [dpdk-dev] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64 Yongseok Koh
2019-04-18  1:47   ` Yongseok Koh
2019-04-18  1:47   ` [dpdk-dev] [PATCH v2 2/4] meson: change default cache line size for armv8 Yongseok Koh
2019-04-18  1:47     ` Yongseok Koh
2019-04-18  5:00     ` Honnappa Nagarahalli
2019-04-18  5:00       ` Honnappa Nagarahalli
2019-04-18  8:23       ` [dpdk-dev] [EXT] " Hemant Agrawal
2019-04-18  8:23         ` Hemant Agrawal
2019-04-18 11:32         ` Yongseok Koh
2019-04-18 11:32           ` Yongseok Koh
2019-04-18  1:47   ` [dpdk-dev] [PATCH v2 3/4] net/mlx: fix library search in meson build Yongseok Koh
2019-04-18  1:47     ` Yongseok Koh
2019-04-18  1:47   ` [dpdk-dev] [PATCH v2 4/4] meson: add Mellanox BlueField cross-compile config Yongseok Koh
2019-04-18  1:47     ` Yongseok Koh
2019-04-18  7:21   ` [dpdk-dev] [EXT] [PATCH v2 1/4] meson: disable octeontx for buggy compilers on arm64 Jerin Jacob Kollanukkaran
2019-04-18  7:21     ` Jerin Jacob Kollanukkaran
2019-04-18 10:41     ` Yongseok Koh
2019-04-18 10:41       ` Yongseok Koh
2019-04-18 11:04       ` Thomas Monjalon
2019-04-18 11:04         ` Thomas Monjalon
2019-04-18 11:10         ` Yongseok Koh
2019-04-18 11:10           ` Yongseok Koh
2019-04-18 11:49 ` [dpdk-dev] [PATCH v3 1/4] drivers/event: " Yongseok Koh
2019-04-18 11:49   ` Yongseok Koh
2019-04-18 11:49   ` [dpdk-dev] [PATCH v3 2/4] meson: change default config for armv8 Yongseok Koh
2019-04-18 11:49     ` Yongseok Koh
2019-04-18 14:25     ` Honnappa Nagarahalli
2019-04-18 14:25       ` Honnappa Nagarahalli
2019-04-18 11:49   ` [dpdk-dev] [PATCH v3 3/4] net/mlx: fix library search in meson build Yongseok Koh
2019-04-18 11:49     ` Yongseok Koh
2019-04-18 12:02     ` Luca Boccassi
2019-04-18 12:02       ` Luca Boccassi
2019-04-18 11:49   ` [dpdk-dev] [PATCH v3 4/4] meson: add Mellanox BlueField cross-compile config Yongseok Koh
2019-04-18 11:49     ` Yongseok Koh
2019-04-18 16:23     ` Thomas Monjalon
2019-04-18 16:23       ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=VE1PR08MB51491D19A8F2671652BF477398350@VE1PR08MB5149.eurprd08.prod.outlook.com \
    --to=honnappa.nagarahalli@arm.com \
    --cc=Gavin.Hu@arm.com \
    --cc=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=jerinj@marvell.com \
    --cc=nd@arm.com \
    --cc=pbhagavatula@marvell.com \
    --cc=shahafs@mellanox.com \
    --cc=thomas@monjalon.net \
    --cc=yskoh@mellanox.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).