DPDK patches and discussions
 help / color / mirror / Atom feed
From: Yongseok Koh <yskoh@mellanox.com>
To: Honnappa Nagarahalli <Honnappa.Nagarahalli@arm.com>
Cc: "jerinj@marvell.com" <jerinj@marvell.com>,
	"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 <thomas@monjalon.net>,
	"Gavin Hu (Arm Technology China)" <Gavin.Hu@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 09:49:33 +0000	[thread overview]
Message-ID: <20190503094923.GB2510@mtidpdk.mti.labs.mlnx> (raw)
Message-ID: <20190503094933.ReUwskclynwI7GgvlFcIG4SJ96leTlAI2IjL2jCMGsM@z> (raw)
In-Reply-To: <VE1PR08MB51491D19A8F2671652BF477398350@VE1PR08MB5149.eurprd08.prod.outlook.com>

On Fri, May 03, 2019 at 03:54:09AM +0000, Honnappa Nagarahalli wrote:
> > >>> 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.

Like Jerin pointed out above, armv8 cryptodev already has runtime check of
cpuflags. If there's no support, it returns error. Unless we need a fallback
function with non-crypto instructions instead of returning error, I don't think
such hookup of func pointers are needed.

> > 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://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgcc.gnu.org%2Fgcc-8%2Fchanges.html&amp;data=02%7C01%7Cyskoh%40mellanox.com%7C5cd398e4cf1e45c1755a08d6cf7b0091%7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C636924524543262594&amp;sdata=4m4S2VQUVBMLYqpxmeLoAPqAcKGm9u1Wo5R7oE2CK94%3D&amp;reserved=0
> > 
> > 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.

I think what Jerin meant was about the case where compiler can generate crypto
instructions beyond intrinsics/asm like sha3 for 3-way exclusive OR
instructions. In this case, such function pointer can't help as we can't know
how compiler generates such instructions.

> 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.

Like I mentioned above, it may not be necessary. armv8 cryptodev links external
library, which is compiled separately (out of dpdk) with crypto support and we
don't have/need a fallback but returns error if no crypto support in runtime.

> > >> 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

  parent reply	other threads:[~2019-05-03  9:49 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
2019-05-03  3:54                   ` Honnappa Nagarahalli
2019-05-03  9:49                   ` Yongseok Koh [this message]
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=20190503094923.GB2510@mtidpdk.mti.labs.mlnx \
    --to=yskoh@mellanox.com \
    --cc=Gavin.Hu@arm.com \
    --cc=Honnappa.Nagarahalli@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 \
    /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).