From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by dpdk.space (Postfix) with ESMTP id 5D366A0AC5
	for <public@inbox.dpdk.org>; Fri,  3 May 2019 11:49:39 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 37A972F42;
	Fri,  3 May 2019 11:49:38 +0200 (CEST)
Received: from EUR03-DB5-obe.outbound.protection.outlook.com
 (mail-eopbgr40040.outbound.protection.outlook.com [40.107.4.40])
 by dpdk.org (Postfix) with ESMTP id BFB7B2C6D
 for <dev@dpdk.org>; Fri,  3 May 2019 11:49:36 +0200 (CEST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com;
 s=selector2;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=YS1OIWSAy6gsRexK719y3MaEg7NEm3Y86SQwUoJm7Nw=;
 b=YJCdEB8WKuJDCzifZ1r3gTolTVKMpa3+32iAiFCtzPPFP0Fd2Yd0H6V6ep1dLSQYAS8rq7ZGNl79jVdysNk0RaEUT4g2L3QoYdOKHQs+pCmn0WMILfdIMMlXAUDHzeXA2x50KD0pVewCmB/fNCSj3tMX6UVOLMh8566gGdJa3/A=
Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com (52.134.72.27) by
 DB3PR0502MB4058.eurprd05.prod.outlook.com (52.134.68.149) with Microsoft SMTP
 Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id
 15.20.1856.11; Fri, 3 May 2019 09:49:34 +0000
Received: from DB3PR0502MB3980.eurprd05.prod.outlook.com
 ([fe80::e8d5:4aff:902d:6e98]) by DB3PR0502MB3980.eurprd05.prod.outlook.com
 ([fe80::e8d5:4aff:902d:6e98%5]) with mapi id 15.20.1856.008; Fri, 3 May 2019
 09:49:34 +0000
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>
Thread-Topic: [EXT] [PATCH 5/6] build: add option for armv8 crypto extension
Thread-Index: AQHU8cm5b8SJPn5CW0moCR+0z+ooT6Y9krUAgAAZAACAAuXVE4ATlfIAgAOUTQCAANibAIAAT8GAgABjQgA=
Date: Fri, 3 May 2019 09:49:33 +0000
Message-ID: <20190503094923.GB2510@mtidpdk.mti.labs.mlnx>
References: <20190412232451.30197-1-yskoh@mellanox.com>
 <20190412232451.30197-6-yskoh@mellanox.com>
 <BYAPR18MB2424A615C597E9F8549F770BC8290@BYAPR18MB2424.namprd18.prod.outlook.com>
 <8328F59C-14DF-412E-A8F7-6AA1F5061065@mellanox.com>
 <VE1PR08MB514978C5F96EC8FA0934C79F982B0@VE1PR08MB5149.eurprd08.prod.outlook.com>
 <3ACFB177-32B1-4AF9-BC60-DE1EBB4EC9C7@mellanox.com>
 <VE1PR08MB514979EA9CDF07C6810A7183983A0@VE1PR08MB5149.eurprd08.prod.outlook.com>
 <BYAPR18MB2424A606C4E9218775D71A5CC8340@BYAPR18MB2424.namprd18.prod.outlook.com>
 <926D3AC3-CA01-410A-8E23-4AB6581FA594@mellanox.com>
 <VE1PR08MB51491D19A8F2671652BF477398350@VE1PR08MB5149.eurprd08.prod.outlook.com>
In-Reply-To: <VE1PR08MB51491D19A8F2671652BF477398350@VE1PR08MB5149.eurprd08.prod.outlook.com>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-clientproxiedby: BYAPR11CA0040.namprd11.prod.outlook.com
 (2603:10b6:a03:80::17) To DB3PR0502MB3980.eurprd05.prod.outlook.com
 (2603:10a6:8:10::27)
authentication-results: spf=none (sender IP is )
 smtp.mailfrom=yskoh@mellanox.com; 
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: [209.116.155.178]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 844a57ff-4520-41ec-a189-08d6cfaca55f
x-ms-office365-filtering-ht: Tenant
x-microsoft-antispam: BCL:0; PCL:0;
 RULEID:(2390118)(7020095)(4652040)(8989299)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(5600141)(711020)(4605104)(4618075)(2017052603328)(7193020);
 SRVR:DB3PR0502MB4058; 
x-ms-traffictypediagnostic: DB3PR0502MB4058:
x-ms-exchange-purlcount: 2
x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr
x-microsoft-antispam-prvs: <DB3PR0502MB40582543BA8C8623536E541CC3350@DB3PR0502MB4058.eurprd05.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:9508;
x-forefront-prvs: 0026334A56
x-forefront-antispam-report: SFV:NSPM;
 SFS:(10009020)(396003)(366004)(346002)(136003)(39860400002)(376002)(189003)(199004)(6436002)(86362001)(186003)(5660300002)(8936002)(102836004)(229853002)(476003)(6486002)(11346002)(71200400001)(71190400001)(8676002)(386003)(66066001)(446003)(486006)(26005)(81156014)(81166006)(53546011)(6506007)(52116002)(68736007)(73956011)(66946007)(66556008)(66476007)(64756008)(66446008)(6916009)(1076003)(7736002)(305945005)(2906002)(76176011)(478600001)(99286004)(4326008)(54906003)(3846002)(6116002)(53936002)(256004)(6246003)(45080400002)(966005)(25786009)(14454004)(9686003)(6512007)(6306002)(33656002)(316002)(14444005)(6314003);
 DIR:OUT; SFP:1101; SCL:1; SRVR:DB3PR0502MB4058;
 H:DB3PR0502MB3980.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en;
 PTR:InfoNoRecords; MX:1; A:1; 
received-spf: None (protection.outlook.com: mellanox.com does not designate
 permitted sender hosts)
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam-message-info: MHVULdLZSjRLUiQpRKIBwrJyr0tGL7FbcTDZUqhMeYINyfQ1l6E3KariqGyOe3dGowDBwFm6KzQYtth2d/L26gRuAqYMBm08EHr+Lby/1BFl2HmUnQbGT6EZd9vjBEjcJV0pDqILQa4nw1BPZNkJ0euU7Iz7jg/lGUi1+SzwFdyrAZroLy+o257i2K/9L5MW2iuiyrhVby8HaCU1X3A4ZBrH8lQVbT53h2JAzVDUiMEH4B2Vpb8R8CzF+b96ZfNGkwQAKNrF6kmAQxTya9sXa0DiEjG5Mko/9e282KkWfo0GvhfDnh4N9u2g8N4o1ODJaGeqeTGiQsWKAgT5iysQBSA0CZ7FAolHWKR1CbryYcxNaCq3nezv90KXfPK2FS5OET3YyRMOPQEe+Rg7rUshJuxdyfP5sEN50EjY5uom4PY=
Content-Type: text/plain; charset="UTF-8"
Content-ID: <919784AE5DDDC2479F80296EE7024180@eurprd05.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: Mellanox.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 844a57ff-4520-41ec-a189-08d6cfaca55f
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 May 2019 09:49:33.8328 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB3PR0502MB4058
Subject: Re: [dpdk-dev] [EXT] [PATCH 5/6] build: add option for armv8 crypto
	extension
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
Message-ID: <20190503094933.ReUwskclynwI7GgvlFcIG4SJ96leTlAI2IjL2jCMGsM@z>

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=3D"armv8a"
> > >>>>>>> +CONFIG_RTE_ENABLE_ARMV8_CRYPTO=3Dy
> > >>>>>>
> > >>>>>> This approach is not scalable. Even, it is not good for BlueFiel=
d
> > >>>>>> as you you need to maintain two images.
> > >>>>>>
> > >>>>>> Unlike other CPU flags, arm64's crypto cpu flag is really _optio=
nal_.
> > >>>>>> 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 f=
lag.
> > >>>>>>
> > >>>>>> Do you see any issues with that approach?
> > >>>>>
> > >>>>> I also thought about that approach and that was my number 1 prior=
ity.
> > >>>>> But, I had one question came to my mind. Maybe, arm people can
> > >>>>> confirm it. Is it 100% guaranteed that compiler never makes use o=
f
> > >>>>> 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_*) unles=
s
> > >>>>> 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 tea=
m.
> > >>>> 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.
>=20
> > > 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 correspo=
nding support.
>=20
> >=20
> > Right, it sounds to me that we can disable the option without having th=
e new
> > config flag until such instructions get needed. According to gcc-8 rele=
ase 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 d=
isable
> > 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 fallbac=
k
function with non-crypto instructions instead of returning error, I don't t=
hink
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 =3D [
> >         ['RTE_USE_C11_MEM_MODEL', true]]
> >=20
> >  machine_args_generic =3D [
> > -       ['default', ['-march=3Darmv8-a+crc+crypto']],
> > +       ['default', ['-march=3Darmv8-a+crc']],
> >         ['native', ['-march=3Dnative']],
> >         ['0xd03', ['-mcpu=3Dcortex-a53']],
> >         ['0xd04', ['-mcpu=3Dcortex-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 =3D
> >  # CPU_ASFLAGS =3D
> >=20
> > -MACHINE_CFLAGS +=3D -march=3Darmv8-a+crc+crypto
> > +MACHINE_CFLAGS +=3D -march=3Darmv8-a+crc
> >=20
> >=20
> > [1] https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2=
Fgcc.gnu.org%2Fgcc-8%2Fchanges.html&amp;data=3D02%7C01%7Cyskoh%40mellanox.c=
om%7C5cd398e4cf1e45c1755a08d6cf7b0091%7Ca652971c7d2e4d9ba6a4d149256f461b%7C=
0%7C0%7C636924524543262594&amp;sdata=3D4m4S2VQUVBMLYqpxmeLoAPqAcKGm9u1Wo5R7=
oE2CK94%3D&amp;reserved=3D0
> >=20
> > Thanks,
> > Yongseok
> >=20
> > >> enabled, compiler can generate 3-way exclusive OR instructions beyon=
d
> > >> 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.
>=20
> > >
> > >> 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 fun=
ction
> 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 cry=
pto
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 kn=
ow
how compiler generates such instructions.

> Either ways, we need to have 2 sets of crypto PMD drivers. One that imple=
ments
> the actual functionality using crypto intrinsics/assembly. Only, this cod=
e
> needs to be compiled with '+crypto'. Second driver that implements just s=
tubs
> and returns error. This code will be compiled without '+crypto'. At run t=
ime,
> depending on the HWCAP, the correct driver/function pointers need to be h=
ooked
> up.

Like I mentioned above, it may not be necessary. armv8 cryptodev links exte=
rnal
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 runtim=
e.

> > >> 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.
>=20
> > >
> > >>
> > >> [1]
> > >> https://eur03.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2F=
wil
> > >> lnewton.name%2F2013%2F07%2F02%2Fusing-gnu-indirect-
> > functions%2F&amp;d
> > >>
> > ata=3D02%7C01%7Cyskoh%40mellanox.com%7Cda8fb7ed03e7406ded8908d6c
> > ee6d759
> > >> %7Ca652971c7d2e4d9ba6a4d149256f461b%7C0%7C0%7C63692388818
> > 9316743&amp;
> > >>
> > sdata=3Dx5XNd5WZ3EtiprPMiFzaskvigX8K0AoXA2w%2BKiN156c%3D&amp;res
> > erved=3D0