[edk2-devel] [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation for CryptAes.c

Tue Nov 3 21:54:42 UTC 2020

I'm OK with using a PCD here if there are no other objections.

--
Christopher Zurcher

From: Bret Barkelew <Bret.Barkelew at microsoft.com>
Sent: Monday, November 2, 2020 17:23
To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>; Zurcher, Christopher J <christopher.j.zurcher at intel.com>
Cc: Wang, Jian J <jian.j.wang at intel.com>; Lu, XiaoyuX <xiaoyux.lu at intel.com>; Jiang, Guomin <guomin.jiang at intel.com>; Sung-Uk Bin <sunguk-bin at hp.com>; Ard Biesheuvel <ard.biesheuvel at arm.com>
Subject: RE: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation for CryptAes.c

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't know that it warrants a full LibraryClass, but that's another option.

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

- Bret

From: Yao, Jiewen via groups.io<mailto:jiewen.yao=intel.com at groups.io>
Sent: Monday, November 2, 2020 5:13 PM
To: Zurcher, Christopher J<mailto:christopher.j.zurcher at intel.com>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
Cc: Wang, Jian J<mailto:jian.j.wang at intel.com>; Lu, XiaoyuX<mailto:xiaoyux.lu at intel.com>; Jiang, Guomin<mailto:guomin.jiang at intel.com>; Sung-Uk Bin<mailto:sunguk-bin at hp.com>; Ard Biesheuvel<mailto:ard.biesheuvel at arm.com>
Subject: [EXTERNAL] Re: [edk2-devel] [PATCH 0/1] CryptoPkg/BaseCryptLib: Add 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 write 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 at intel.com<mailto:christopher.j.zurcher at intel.com>>
> Sent: Tuesday, November 3, 2020 6:37 AM
> To: Yao, Jiewen <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>; devel at edk2.groups.io<mailto:devel at edk2.groups.io>
> Cc: Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Lu, XiaoyuX
> <xiaoyux.lu at intel.com<mailto:xiaoyux.lu at intel.com>>; Jiang, Guomin <guomin.jiang at intel.com<mailto:guomin.jiang at intel.com>>; Sung-Uk
> Bin <sunguk-bin at hp.com<mailto:sunguk-bin at hp.com>>; Ard Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at arm.com>>
> 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 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.
> 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 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 <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>
> > Sent: Friday, October 30, 2020 18:10
> > To: Zurcher, Christopher J <christopher.j.zurcher at intel.com<mailto:christopher.j.zurcher at intel.com>>;
> > devel at edk2.groups.io<mailto:devel at edk2.groups.io>
> > Cc: Wang, Jian J <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Lu, XiaoyuX
> <xiaoyux.lu at intel.com<mailto:xiaoyux.lu at intel.com>>;
> > Jiang, Guomin <guomin.jiang at intel.com<mailto:guomin.jiang at intel.com>>; Sung-Uk Bin <sunguk-
> bin at hp.com<mailto:bin at hp.com>>; Ard
> > Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at arm.com>>
> > Subject: RE: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation
> 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 <christopher.j.zurcher at intel.com<mailto:christopher.j.zurcher at intel.com>>
> > > Sent: Thursday, October 29, 2020 2:43 AM
> > > To: devel at edk2.groups.io<mailto:devel at edk2.groups.io>
> > > Cc: Yao, Jiewen <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>; Wang, Jian J
> > > <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>; Lu, XiaoyuX <xiaoyux.lu at intel.com<mailto:xiaoyux.lu at intel.com>>; Jiang,
> Guomin
> > > <guomin.jiang at intel.com<mailto:guomin.jiang at intel.com>>; Sung-Uk Bin <sunguk-bin at hp.com<mailto:sunguk-bin at hp.com>>; Ard
> > > Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at arm.com>>
> > > Subject: [PATCH 0/1] CryptoPkg/BaseCryptLib: Add EVP implementation
> for
> > > CryptAes.c
> > >
> > > BZ: https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D2507&data=04%7C01%7Cbret.barkelew%40microsoft.com%7C4a13842bdd6240d452ca08d87f95ba5f%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637399628344133791%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=oOONHll9TASuW8eBAMvvLUNZhb9jOSRN18liIP7BHrk%3D&reserved=0
> > >
> > > This patch provides a drop-in replacement for CryptAes.c which utilizes
> > > 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 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 provide a speed
> > > improvement.
> > >
> > > The C-only AES implementation included by CryptAes.c is extremely small,
> > > and since this file includes the EVP interface, it will significantly
> > > increase the size of any module that includes it. As a result, I have not
> > > replaced the original CryptAes.c as a default in any of the CryptLib
> > > implementations.
> > >
> > > Cc: Jiewen Yao <jiewen.yao at intel.com<mailto:jiewen.yao at intel.com>>
> > > Cc: Jian J Wang <jian.j.wang at intel.com<mailto:jian.j.wang at intel.com>>
> > > Cc: Xiaoyu Lu <xiaoyux.lu at intel.com<mailto:xiaoyux.lu at intel.com>>
> > > Cc: Guomin Jiang <guomin.jiang at intel.com<mailto:guomin.jiang at intel.com>>
> > > Cc: Sung-Uk Bin <sunguk-bin at hp.com<mailto:sunguk-bin at hp.com>>
> > > Cc: Ard Biesheuvel <ard.biesheuvel at arm.com<mailto:ard.biesheuvel at 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

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#66940): https://edk2.groups.io/g/devel/message/66940
Mute This Topic: https://groups.io/mt/77870226/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20201103/5b3bcdcf/attachment.htm>