Re: [edk2-devel] [PATCH 1/1] CryptoPkg/OpensslLib: Add native instruction support for IA32 and X64

["Zurcher, Christopher J" <christopher.j.zurcher@intel.com>]

The specific performance improvement depends on the operation; the OS provisioning I mentioned in the [Patch 0/1] thread removed the hashing as a bottleneck and improved the overall operation speed over 4x (saving 2.5 minutes of flashing time), but a direct SHA256 benchmark on the particular silicon I have available showed over 12x improvement. I have not benchmarked the improvements to boot time. I do not know the use case targeted by BZ 2507 so I don't know what benefit will be seen there.

I will look at unifying the INF files in the next patch-set and will also add the OpensslLibCrypto.inf case.

I have not exercised the AVX code specifically, as it is coming directly from OpenSSL and includes checks against the CPUID capability flags before executing. I'm not entirely familiar with AVX requirements; is there a known environment restriction against AVX instructions in EDK2?

Regarding RNG, it looks like we already have architecture-specific variants of RdRand...?

There was some off-list feedback regarding the number of files required to be checked in here. OpenSSL does not include assembler-specific implementations of these files and instead relies on "perlasm" scripts which are parsed by a translation script at build time (in the normal OpenSSL build flow) to generate the resulting .nasm files. The implementation I have shared here generates these files as part of the OpensslLib maintainer process, similar to the existing header files which are also generated. Since process_files.pl already requires the package maintainer to have a Perl environment installed, this does not place any additional burden on them.
An alternative implementation has been proposed which would see only a listing/script of the required generator operations to be checked in, and any platform build which intended to utilize the native algorithms would require a local Perl environment as well as any underlying environment dependencies (such as a version check against the NASM executable) for every developer, and additional pre-build steps to run the generator scripts.

Are there any strong opinions here around adding Perl as a build environment dependency vs. checking in maintainer-generated assembly "intermediate" build files?

Thanks,
Christopher Zurcher

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> Sent: Wednesday, March 25, 2020 18:23
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Zurcher,
> Christopher J <christopher.j.zurcher@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>;
> Eugene Cohen <eugene@hp.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Subject: Re: [edk2-devel] [PATCH 1/1] CryptoPkg/OpensslLib: Add native
> instruction support for IA32 and X64
> 
> Some more comment:
> 
> 3) Do you consider to enable RNG instruction as well?
> 
> 4) I saw you added some code for AVX instruction, such as YMM register.
> Have you validated that code, to make sure it can work correctly in current
> environment?
> 
> 
> 
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, Jiewen
> > Sent: Thursday, March 26, 2020 9:15 AM
> > To: devel@edk2.groups.io; Zurcher, Christopher J
> > <christopher.j.zurcher@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>;
> > Eugene Cohen <eugene@hp.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > Subject: Re: [edk2-devel] [PATCH 1/1] CryptoPkg/OpensslLib: Add native
> > instruction support for IA32 and X64
> >
> > HI Christopher
> > Thanks for the contribution. I think it is good enhancement.
> >
> > Do you have any data show what performance improvement we can get?
> > Did the system boot faster with the this? Which feature ?
> > UEFI Secure Boot? TCG Measured Boot? HTTPS boot?
> >
> >
> > Comment for the code:
> > 1) I am not sure if we need separate OpensslLibIa32 and OpensslLibX64.
> > Can we just define single INF, such as OpensslLibHw.inf ?
> >
> > 2) Do we also need add a new version for OpensslLibCrypto.inf ?
> >
> >
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Zurcher,
> > > Christopher J
> > > Sent: Tuesday, March 17, 2020 6:27 PM
> > > To: devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > <xiaoyux.lu@intel.com>;
> > > Eugene Cohen <eugene@hp.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > > Subject: [edk2-devel] [PATCH 1/1] CryptoPkg/OpensslLib: Add native
> > instruction
> > > support for IA32 and X64
> > >
> >
> >
> >
> 
> 
>