From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (NAM11-BN8-obe.outbound.protection.outlook.com [40.107.236.122]) by mx.groups.io with SMTP id smtpd.web11.3962.1604366554877010714 for ; Mon, 02 Nov 2020 17:22:35 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@microsoft.com header.s=selector2 header.b=LjIBeoHN; spf=pass (domain: microsoft.com, ip: 40.107.236.122, mailfrom: bret.barkelew@microsoft.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=E8XRi57mtkfSA930Ak+XwGIhn9iJrXqU4DB9AyHRkAMIpcMru2XQXFGCnkNmkm9FCtAd/jzfhH+kYoI9MmwFiu6W4vnbc1iLVbiEeBUAzdy7CWv6fz4EQBJVo7wW4btj89u5LYjhW1fMiVtaALhj/6Phwbhq1wwO58+KpVXoRvk0yl1eYvRXrhgvj/LhEowoR2PK99p7x+r76JP8qSGmgvFi0H7+/eAbMclUaSOlr4xaJU1txnqpgY6Db9QjzCctXDLA5wCwz6isOgWBz3cMlsUqAup11Oy4qJysYbKu3bmcoBVDQ6xdUONC2LUzu8QTtVxrt/2Vft4Y85ggp9J0jg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ICZMOPUdDBZ7SVJjZ7OttSzBoZWktH15XKJbowFG5NQ=; b=hHIlqYD41V0GhWYKMZGEtnKdS+b405nvo49Hacprs2gKR4XC2OmxHRz0pCetrJSktmwzXX4Z+5HT1HONs4OHcBSHYyQUD/bWwVQ98LPLu/qFWpXJ8BLzMloUfEaje0DdOtE6+3cUPjPqvbBjuIcnYUgaePIMJOYF8C/M53M5mUDfTqi/HrpaApo3lROhTbA50NS+o2MlSSG+C2JCR+2gE7rHi4n6rkmqrEYEu6B+1fJOD+gSMB85+8TVa1BW1t+nZ9wmUQc6qNYW1vnWwSv7D6bwvDxKYhuExEtCNxHpiaVQwyrdZfhEizjfbJaJ5ilRPxAzNUohlYXA5xNE7J5PeA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=microsoft.com; dmarc=pass action=none header.from=microsoft.com; dkim=pass header.d=microsoft.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=ICZMOPUdDBZ7SVJjZ7OttSzBoZWktH15XKJbowFG5NQ=; b=LjIBeoHN90fAANSw1GYFl9S91XUfObKf+iZgGjfWrOjYSiY8TsKbVUqtqiE29b0AHiVm/HLHjsJh4vsELnCtxUeiVW2tBU1H+k1rRUK4jlNlKFqZxPKhg6qljVXIGP/4hylgyXoTDWjpjea45mIZdihz9INaBRiW6yXVH8UOVFE= Received: from MW4PR21MB1857.namprd21.prod.outlook.com (2603:10b6:303:74::12) by MW4PR21MB1923.namprd21.prod.outlook.com (2603:10b6:303:7c::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3564.6; Tue, 3 Nov 2020 01:22:33 +0000 Received: from MW4PR21MB1857.namprd21.prod.outlook.com ([fe80::7584:fc3a:1487:8fda]) by MW4PR21MB1857.namprd21.prod.outlook.com ([fe80::7584:fc3a:1487:8fda%6]) with mapi id 15.20.3564.010; Tue, 3 Nov 2020 01:22:32 +0000 From: "Bret Barkelew" To: "devel@edk2.groups.io" , "Yao, Jiewen" , "Zurcher, Christopher J" CC: "Wang, Jian J" , "Lu, XiaoyuX" , "Jiang, Guomin" , Sung-Uk Bin , Ard Biesheuvel Subject: Re: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation for CryptAes.c Thread-Topic: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation for CryptAes.c Thread-Index: AQHWryKkud5WRkeukE6GMwqYqZZ8Ram1cwOAgAArrQCAAAIfig== Date: Tue, 3 Nov 2020 01:22:32 +0000 Message-ID: References: <20201028184254.6923-1-christopher.j.zurcher@intel.com> , In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: msip_labels: MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Enabled=True;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SiteId=72f988bf-86f1-41af-91ab-2d7cd011db47;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_SetDate=2020-11-03T01:20:39.1057135Z;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_ContentBits=0;MSIP_Label_f42aa342-8706-4288-bd11-ebb85995028c_Method=Privileged authentication-results: edk2.groups.io; dkim=none (message not signed) header.d=none;edk2.groups.io; dmarc=none action=none header.from=microsoft.com; x-originating-ip: [71.212.128.184] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: c339d8fd-9173-4091-c857-08d87f96f084 x-ms-traffictypediagnostic: MW4PR21MB1923: x-ld-processed: 72f988bf-86f1-41af-91ab-2d7cd011db47,ExtAddr x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: n8Wy/IHVcdb4/JaZjV0Zt2/Ql44k+X0+q5onNzxU7QDxJQiYmCEEtEclaDXvk+w9pkbEpnHD/49z9uvWr2e4BCkFDPR22uyrLom900jtyTb/DwG38CkPcHncjOuXuYMcvy+8Wfj4+O32uBEvwbZBzsUK2fx3xCphJ070dDZLDwKQ/hvRX5TkjArdu/+T+KuCc1TmOFzQqzjI2tmmO1peBonW9sBFGT466nQPdN9Ysy7iScVq/oYzPycl07tFeD537A/aJOj3jXrf+JIO+9i/Wr886CK3Hdlmz3IEMIMxzAXU25HA0iwt3gNIvzLwIjQbdkwMF7Oxk7oeYa2ZymWZzAqQ6zCe7twWu/zZF76DSCkrxkwX0Uo9eiNS2mpkhxrDwp6t5ZPG3FsyoTu2AURaDQ== x-forefront-antispam-report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:MW4PR21MB1857.namprd21.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(136003)(396003)(346002)(39860400002)(376002)(26005)(53546011)(186003)(6506007)(7696005)(33656002)(8990500004)(86362001)(71200400001)(2906002)(83380400001)(5660300002)(966005)(55016002)(9686003)(316002)(66946007)(54906003)(166002)(4326008)(110136005)(66556008)(66446008)(8936002)(478600001)(82960400001)(10290500003)(82950400001)(64756008)(8676002)(66476007)(76116006)(52536014);DIR:OUT;SFP:1102; x-ms-exchange-antispam-messagedata: 6JLITbGxKGHYHI+JHB8FZ6K7IXS9AkTTmyzoZtMvLHnOmJlY5ke/IJL8WjV4Pz46QDM71jZr+3/ys0UxH279Ch3MSCWU181DB9nDxpZGY9WkhShXugxih3+TBi76aaYyulVOb7yttywBZnesQCwn7oymc+Pn5b43najpaxrXwMtDGCeJAU6cuVi7Q/o4egj93stOXRc6EeMZ0jZKruFZBuNjlPz+FGhinKgZn85OurvTYTEJH+FOoS7mxXUEuuWi976KpQ8G40YP9bc/CMzdJjhRnJy8rmWCetNKIM55nUvhOKkIDIjRkXlX/dvpVoKcV5kgdrt3nwKjj3i2ftYmlwAdAOGsWUlQz3K6SvODOZ1YACrIa5S5cv/eYhtIWlaL6oaOCAFKGh9zTUF/7C5oBW+KgAeexXZdFxcOD1pVYbES3q60V0+TYbBd8pp0rOfGZ85Dl4BzZhrjPrXYcW0PMEOF0CsfVHtSM2Ky+HPb3AASyOtX411EmOox0yF/aZHUdPkYeeFo/akGeWq62JM/0k+PWiua7L/URpAaR1kJYrAIyuGrPDOegw/5va0eQsTLXf6kB3VLY0hk2c2ngnlwHOqNHBuCbW8Pdrpz1gkJMpAID83buhsSAmNZ4QtqovGGITZ8AuLENua4YOlnQhdFtg== x-ms-exchange-transport-forked: True MIME-Version: 1.0 X-OriginatorOrg: microsoft.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MW4PR21MB1857.namprd21.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c339d8fd-9173-4091-c857-08d87f96f084 X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Nov 2020 01:22:32.8577 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 72f988bf-86f1-41af-91ab-2d7cd011db47 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: DLh/pK1ci+OKj+QoAEPe/YgOwSIQ2IW7XX21Epmeh8BmRq+XebvbZ2e0Sl6UOnSsoCIsa8+P8NUU+mGOe8Ku2g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR21MB1923 Content-Language: en-US Content-Type: multipart/alternative; boundary="_000_MW4PR21MB1857F56A32617DC7C36270BCEF110MW4PR21MB1857namp_" --_000_MW4PR21MB1857F56A32617DC7C36270BCEF110MW4PR21MB1857namp_ Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable I agree that this should not be a C file. Based on the description, would a= FixedAtBuild PCD work to wrap the code? I don=92t know that it warrants a full LibraryClass, but that=92s another = option. Have you looked at just implementing it in a flavor of the new EDK2 binary= crypto payloads? Those are shared and wouldn=92t incur the per-module cost= . - Bret From: Yao, Jiewen via groups.io Sent: Monday, November 2, 2020 5:13 PM To: Zurcher, Christopher J; devel@= edk2.groups.io Cc: Wang, Jian J; Lu, XiaoyuX; Jiang, Guomin; Sung-Uk Bin; Ard Biesheuvel Subject: [EXTERNAL] Re: [edk2-devel] [PATCH 0/1] CryptoPkg/BaseCryptLib: A= dd EVP implementation for CryptAes.c Thanks to provide the data. I have a little concern on what is the usage if we just add a standalone C= file. The EDKII build does not support cross package file including in INF, and = does not recommend cross module file including in INF. What is the expected= usage for the standalone CryptAes.c ? If you have a private project which want to use this, then you have to wri= te your own INF file. But if so, why not include this CryptAes.c along with= the new INF? Thank you Yao Jiewen > -----Original Message----- > From: Zurcher, Christopher J > Sent: Tuesday, November 3, 2020 6:37 AM > To: Yao, Jiewen ; devel@edk2.groups.io > Cc: Wang, Jian J ; Lu, XiaoyuX > ; Jiang, Guomin ; Sung-Uk > Bin ; Ard Biesheuvel > Subject: RE: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation > for CryptAes.c > > The size increase from adding the EVP interface to a module that does no= t > already include it (through the HMAC functions) is ~192KB. > Intel documentation on the IA version of the feature lists speed > improvement of ~32% to ~47% depending on the operation and key size. > Other architectures may see different speed improvements. I have only > tested this file with OvmfPkg through QEMU. > > I did not add this .c file to any INF file because it will add ~192KB to= any > module that includes AES functionality and it should be up to the end us= er if > they want to include this file for the size tradeoff vs. the speed gain = for their > particular architecture. Additionally as the only native OpensslLib > implementation available currently is for X64 architecture, any other > architecture would have a size increase with no speed improvement. > > Thanks, > Christopher Zurcher > > > -----Original Message----- > > From: Yao, Jiewen > > Sent: Friday, October 30, 2020 18:10 > > To: Zurcher, Christopher J ; > > devel@edk2.groups.io > > Cc: Wang, Jian J ; Lu, XiaoyuX > ; > > Jiang, Guomin ; Sung-Uk Bin bin@hp.com>; Ard > > Biesheuvel > > Subject: RE: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementatio= n > for > > CryptAes.c > > > > HI Zucher > > I am not sure why you only add a .c file, but do not add this c to any= INF > > file. This seems a dummy C file. > > I recommend you drop this and create a full patch to add C and update = INF > > file. > > > > Since you are talking performance and size, do you have any data? > > For example, how fast you have got? What is the size difference before= and > > after? > > This can help other people make decision to choose which version. > > > > > > Thank you > > Yao Jiewen > > > > > > > -----Original Message----- > > > From: Christopher J Zurcher > > > Sent: Thursday, October 29, 2020 2:43 AM > > > To: devel@edk2.groups.io > > > Cc: Yao, Jiewen ; Wang, Jian J > > > ; Lu, XiaoyuX ; Jiang, > Guomin > > > ; Sung-Uk Bin ; Ard > > > Biesheuvel > > > Subject: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation > for > > > CryptAes.c > > > > > > BZ: https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2= F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2507&data=3D04%7C01%7C= bret.barkelew%40microsoft.com%7C4a13842bdd6240d452ca08d87f95ba5f%7C72f988bf= 86f141af91ab2d7cd011db47%7C1%7C0%7C637399628344133791%7CUnknown%7CTWFpbGZsb= 3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C100= 0&sdata=3DoOONHll9TASuW8eBAMvvLUNZhb9jOSRN18liIP7BHrk%3D&reserved= =3D0 > > > > > > This patch provides a drop-in replacement for CryptAes.c which utili= zes > > > the EVP interface to OpenSSL. This enables access to the assembly- > optimized > > > algorithms. > > > > > > This patch has been unit-tested in both VS and CLANG build > environments > > > using both an X64 assembly-optimized implementation of OpensslLib an= d > > > the > > > standard implementation. > > > > > > Usage of this file does not require an assembly-optimized > implementation of > > > OpensslLib to function; it does however require one to provide a spe= ed > > > improvement. > > > > > > The C-only AES implementation included by CryptAes.c is extremely sm= all, > > > and since this file includes the EVP interface, it will significantl= y > > > increase the size of any module that includes it. As a result, I hav= e not > > > replaced the original CryptAes.c as a default in any of the CryptLib > > > implementations. > > > > > > Cc: Jiewen Yao > > > Cc: Jian J Wang > > > Cc: Xiaoyu Lu > > > Cc: Guomin Jiang > > > Cc: Sung-Uk Bin > > > Cc: Ard Biesheuvel > > > > > > Christopher J Zurcher (1): > > > CryptoPkg/BaseCryptLib: Add EVP implementation for CryptAes.c > > > > > > CryptoPkg/Library/BaseCryptLib/Cipher/CryptAesEvp.c | 262 > > > ++++++++++++++++++++ > > > 1 file changed, 262 insertions(+) > > > create mode 100644 > CryptoPkg/Library/BaseCryptLib/Cipher/CryptAesEvp.c > > > > > > -- > > > 2.28.0.windows.1 --_000_MW4PR21MB1857F56A32617DC7C36270BCEF110MW4PR21MB1857namp_ Content-Type: text/html; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable

I agree that this should not be a C file. Based on = the description, would a FixedAtBuild PCD work to wrap the code?

 

I don=92t know that it warrants a full LibraryClass= , but that=92s another option.

 

Have you looked at just implementing it in a flavor= of the new EDK2 binary crypto payloads? Those are shared and wouldn=92t in= cur the per-module cost.

 

- Bret

 

 

Thanks to provide th= e data.

I have a little concern on what is the usage if we just add a standalone C= file.

The EDKII build does not support cross package file including in INF, and = does not recommend cross module file including in INF. What is the expected= usage for the standalone CryptAes.c ?

If you have a private project which want to use this, then you have to wri= te your own INF file. But if so, why not include this CryptAes.c along with= the new INF?

Thank you
Yao Jiewen

> -----Original Message-----
> From: Zurcher, Christopher J <christopher.j.zurcher@intel.com><= br> > Sent: Tuesday, November 3, 2020 6:37 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.co= m>; Sung-Uk
> Bin <sunguk-bin@hp.com>; Ard Biesheuvel <ard.biesheuvel@arm.= com>
> Subject: RE: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementati= on
> for CryptAes.c
>
> The size increase from adding the EVP interface to a module that does= not
> already include it (through the HMAC functions) is ~192KB.
> Intel documentation on the IA version of the feature lists speed
> improvement of ~32% to ~47% depending on the operation and key size.<= br> > Other architectures may see different speed improvements. I have only=
> tested this file with OvmfPkg through QEMU.
>
> I did not add this .c file to any INF file because it will add ~192KB= to any
> module that includes AES functionality and it should be up to the end= user if
> they want to include this file for the size tradeoff vs. the speed ga= in for their
> particular architecture. Additionally as the only native OpensslLib > implementation available currently is for X64 architecture, any other=
> architecture would have a size increase with no speed improvement. >
> Thanks,
> Christopher Zurcher
>
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Friday, October 30, 2020 18:10
> > To: Zurcher, Christopher J <christopher.j.zurcher@intel.com&g= t;;
> > devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>;
> > Jiang, Guomin <guomin.jiang@intel.com>; Sung-Uk Bin <su= nguk-
> bin@hp.com>; Ard
> > Biesheuvel <ard.biesheuvel@arm.com>
> > Subject: RE: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP impleme= ntation
> for
> > CryptAes.c
> >
> > HI Zucher
> > I am not sure why you only add a .c file, but do not add this c = to any INF
> > file. This seems a dummy C file.
> > I recommend you drop this and create a full patch to add C and u= pdate INF
> > file.
> >
> > Since you are talking performance and size, do you have any data= ?
> > For example, how fast you have got? What is the size difference = before and
> > after?
> > This can help other people make decision to choose which version= .
> >
> >
> > Thank you
> > Yao Jiewen
> >
> >
> > > -----Original Message-----
> > > From: Christopher J Zurcher <christopher.j.zurcher@intel= .com>
> > > Sent: Thursday, October 29, 2020 2:43 AM
> > > To: devel@edk2.groups.io
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J<= br> > > > <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@i= ntel.com>; Jiang,
> Guomin
> > > <guomin.jiang@intel.com>; Sung-Uk Bin <sunguk-bin@= hp.com>; Ard
> > > Biesheuvel <ard.biesheuvel@arm.com>
> > > Subject: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implem= entation
> for
> > > CryptAes.c
> > >
> > > BZ: https://nam06.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fbugzil= la.tianocore.org%2Fshow_bug.cgi%3Fid%3D2507&amp;data=3D04%7C01%7Cbret.b= arkelew%40microsoft.com%7C4a13842bdd6240d452ca08d87f95ba5f%7C72f988bf86f141= af91ab2d7cd011db47%7C1%7C0%7C637399628344133791%7CUnknown%7CTWFpbGZsb3d8eyJ= WIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&= amp;sdata=3DoOONHll9TASuW8eBAMvvLUNZhb9jOSRN18liIP7BHrk%3D&amp;reserved= = =3D0
> > >
> > > This patch provides a drop-in replacement for CryptAes.c wh= ich utilizes
> > > the EVP interface to OpenSSL. This enables access to the as= sembly-
> optimized
> > > algorithms.
> > >
> > > This patch has been unit-tested in both VS and CLANG build<= br> > environments
> > > using both an X64 assembly-optimized implementation of Open= sslLib and
> > > the
> > > standard implementation.
> > >
> > > Usage of this file does not require an assembly-optimized > implementation of
> > > OpensslLib to function; it does however require one to prov= ide a speed
> > > improvement.
> > >
> > > The C-only AES implementation included by CryptAes.c is ext= remely small,
> > > and since this file includes the EVP interface, it will sig= nificantly
> > > increase the size of any module that includes it. As a resu= lt, I have not
> > > replaced the original CryptAes.c as a default in any of the= CryptLib
> > > implementations.
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > Cc: Sung-Uk Bin <sunguk-bin@hp.com>
> > > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> > >
> > > Christopher J Zurcher (1):
> > >   CryptoPkg/BaseCryptLib: Add EVP implementation = for CryptAes.c
> > >
> > >  CryptoPkg/Library/BaseCryptLib/Cipher/CryptAesEvp.c |= 262
> > > ++++++++++++++++++++
> > >  1 file changed, 262 insertions(+)
> > >  create mode 100644
> CryptoPkg/Library/BaseCryptLib/Cipher/CryptAesEvp.c
> > >
> > > --
> > > 2.28.0.windows.1





 

--_000_MW4PR21MB1857F56A32617DC7C36270BCEF110MW4PR21MB1857namp_--