[edk2-devel] CryptoPkg OpensslLib INF files

[Michael D Kinney]

Hi Christopher,

Responses below.

Mike

> -----Original Message-----
> From: Christopher Zurcher <christopher.zurcher at outlook.com>
> Sent: Sunday, September 25, 2022 11:52 PM
> To: Kinney, Michael D <michael.d.kinney at intel.com>; devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>; Wang, Jian J
> <jian.j.wang at intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu at intel.com>; Jiang, Guomin <guomin.jiang at intel.com>
> Subject: RE: [edk2-devel] CryptoPkg OpensslLib INF files
> 
> Mike,
> I don't see any change to process_files.pl in your PR, have you made these changes by hand? We would either need changes to
> the perl script to support generating the unified INF or an expectation that the INFs would be re-combined manually whenever
> an update to OpenSSL is taken.

Can you help with these updates?  I have no experience with maintaining or testing changes to that script.

> 
> Regarding the .S files for GCC, as you found the assembly generated by OpenSSL is unfortunately not cross-compatible between
> GAS and NASM.

I also see warnings from VS20xx build about use of CRT section.  Does that have any impact to FW usages?

> 
> I'm also not clear on why the GCC build passes without 64-byte alignment but in testing I never observed any errors or
> failures with the GCC variants in QEMU or hardware-based testing.
> Related to that, it seems the [BuildOptions] section in the INF can't be used to pass DLINK_FLAGS; do you know if this is an
> intentional limitation or just unimplemented?

Building a library component never uses DLINK_FLAGS.  Only SLINK_FLAGS.  Modules that link against libraries to generate a 
loadable PE/COFF image use DLINK_FLAGS.  This is why DLINK_FLAGS provided in a library INF are not used.

> 
> Thanks,
> Christopher Zurcher
> 
> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney at intel.com>
> Sent: Sunday, September 25, 2022 22:47
> To: devel at edk2.groups.io; christopher.zurcher at outlook.com; Yao, Jiewen <jiewen.yao at intel.com>; Wang, Jian J
> <jian.j.wang at intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu at intel.com>; Jiang, Guomin <guomin.jiang at intel.com>; Kinney, Michael D
> <michael.d.kinney at intel.com>
> Subject: RE: [edk2-devel] CryptoPkg OpensslLib INF files
> 
> Hi Christopher,
> 
> I have the following PR that has some proposed ideas to combine all the optimized opensll libs into one new INF.  It also
> addresses some missing CI test coverage and host based unit test coverage for this optimized openssl lib
> 
> https://github.com/tianocore/edk2/pull/3402
> 
> Please review and test to make sure I have not broken any use cases.
> 
> I know Jiewen asked if it was possible to merge this INF into the OpensslLib.inf.  That may be possible, but will require a
> little more investigation.
> 
> Thanks,
> 
> Mike
> 
> 
> > -----Original Message-----
> > From: Kinney, Michael D <michael.d.kinney at intel.com>
> > Sent: Sunday, September 25, 2022 10:52 AM
> > To: devel at edk2.groups.io; christopher.zurcher at outlook.com; Yao, Jiewen
> > <jiewen.yao at intel.com>; Wang, Jian J <jian.j.wang at intel.com>; Lu,
> > Xiaoyu1 <xiaoyu1.lu at intel.com>; Jiang, Guomin
> > <guomin.jiang at intel.com>; Kinney, Michael D
> > <michael.d.kinney at intel.com>
> > Subject: RE: [edk2-devel] CryptoPkg OpensslLib INF files
> >
> > Hi Christopher,
> >
> > I tried this path and the build does break for GCC5 due to NASM source files using some VS20xx specific section names.
> >
> > We will keep the .S files for GCC5 compatibility.
> >
> > I also noticed that your patches did not add the build of these optimized INFs to the CryptoPkg DSC file.
> > I am working on a branch that includes that update along with
> > combining the 4 new INFs into a single OpensslLibOpt.inf.
> >
> > I have also noticed that these optimized libs have larger PE/COFF
> > section alignment requirements than the default alignment for VS20xx toolchains.  IA32 requires 64-byte alignment.  X64
> required 256-byte alignment.
> > We do not want to apply these larger alignment requirements to all
> > modules.  This can increase FLASH overhead, especially for uncompressed PEIMs.
> >
> > When building modules that consume the optimized OpensslLib, then
> > modules require the use of <BuildOptions> in the scope of that specific module in the DSC file to increase the alignment
> size.
> >
> >     <BuildOptions>
> >       MSFT:*_*_IA32_DLINK_FLAGS = /ALIGN:64
> >       MSFT:*_*_X64_DLINK_FLAGS  = /ALIGN:256
> >
> > What does not make sense is that GCC5 builds use 32-byte alignment by
> > default and do not generate a build error from linking this Openssl
> > content that required 64-byte or 256-byte alignment.  Have the GCC5 builds of these optimized OpensslLibs been tested?  Are
> exceptions being generated for unaligned access?
> >
> > Thanks,
> >
> > Mike
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney at intel.com>
> > > Sent: Saturday, September 24, 2022 1:24 PM
> > > To: devel at edk2.groups.io; christopher.zurcher at outlook.com; Yao,
> > > Jiewen <jiewen.yao at intel.com>; Wang, Jian J <jian.j.wang at intel.com>;
> > > Lu, Xiaoyu1 <xiaoyu1.lu at intel.com>; Jiang, Guomin
> > > <guomin.jiang at intel.com>; Kinney, Michael D
> > > <michael.d.kinney at intel.com>
> > > Subject: RE: [edk2-devel] CryptoPkg OpensslLib INF files
> > >
> > > Hi Christopher,
> > >
> > > I see that IA32 uses .nasm files and IA32Gcc uses .S files.
> > >
> > > EDK II support use of NASM files from both VS and GCC builds.
> > >
> > > Is there any reason why the .nasm files generated by OpenSSL can not
> > > be used for both VS and GCC builds and remove the .S files?
> > >
> > > Thanks,
> > >
> > > Mike
> > >
> > >
> > > > -----Original Message-----
> > > > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > > > Christopher Zurcher
> > > > Sent: Friday, September 23, 2022 3:40 PM
> > > > To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>;
> > > > Kinney, Michael D <michael.d.kinney at intel.com>; Wang, Jian J
> > > > <jian.j.wang at intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu at intel.com>;
> > > > Jiang, Guomin <guomin.jiang at intel.com>
> > > > Subject: Re: [edk2-devel] CryptoPkg OpensslLib INF files
> > > >
> > > > I looked at doing this previously and found that depending on the
> > > > selection of accelerated algorithms (in UefiAsm.conf)
> > you
> > > > can end up with different sets of non-assembly source files, so
> > > > that a unified INF would have to contain a copy of the
> > > entire
> > > > Sources section for each architecture target. The build options
> > > > can also be affected such that you'd have different sets
> > of
> > > > those as well (the OPENSSL_FLAGS_CONFIG define).
> > > >
> > > > If we can commit to limiting the accelerated algorithms to the
> > > > current selection, it should be possible to unify the
> > files.
> > > >
> > > > Thanks,
> > > > Christopher Zurcher
> > > >
> > > > -----Original Message-----
> > > > From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> > > > Yao, Jiewen
> > > > Sent: Friday, September 23, 2022 15:33
> > > > To: Kinney, Michael D <michael.d.kinney at intel.com>;
> > > > devel at edk2.groups.io; Wang, Jian J <jian.j.wang at intel.com>; Lu,
> > Xiaoyu1
> > > > <xiaoyu1.lu at intel.com>; Jiang, Guomin <guomin.jiang at intel.com>
> > > > Subject: Re: [edk2-devel] CryptoPkg OpensslLib INF files
> > > >
> > > > Hi Mike
> > > > Yes, I agree with you.
> > > >
> > > > If we have a way to reduce the number of INF, we should. Feel free to submit patch.
> > > >
> > > > BTW: Do you think we have chance to combine OpensslLibOpt.inf with OpensslLib.inf, with PCD Feature Flag: "Opt"?
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -----Original Message-----
> > > > > From: Kinney, Michael D <michael.d.kinney at intel.com>
> > > > > Sent: Saturday, September 24, 2022 4:58 AM
> > > > > To: devel at edk2.groups.io; Yao, Jiewen <jiewen.yao at intel.com>;
> > > > > Kinney, Michael D <michael.d.kinney at intel.com>; Wang, Jian J
> > > > > <jian.j.wang at intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu at intel.com>;
> > > > > Jiang, Guomin <guomin.jiang at intel.com>
> > > > > Subject: CryptoPkg OpensslLib INF files
> > > > >
> > > > > Hi Jiewen,
> > > > >
> > > > > I see we now have 6 INF files for the OpensslLib
> > > > >
> > > > > * OpensslLib.inf
> > > > > * OpensslLibCrypto.inf
> > > > > * OpensslLibIa32.inf
> > > > > * OpensslLibIa32Gcc.inf
> > > > > * OpensslLibX64.inf
> > > > > * OpensslLibX64Gcc.inf
> > > > >
> > > > > If I look at the difference between OpensslLib and
> > > > > OpensslLibCrypto, the OpensslLibCrypto includes the "ssl" source files.
> > > > >
> > > > > This looks like a similar problem as the "ec" sources.  But the "ec"
> > > > > sources were addressed with a PCD FeatureFlag expression so we
> > > > > did not have to add another INF.
> > > > >
> > > > > Could the same technique be applied to the "ssl" sources so we
> > > > > can get back to just OpensslLib.inf with an SSL PCD and an EC
> > > > > PCD to conditionally build the extra source files?
> > > > >
> > > > > For the other 4 INF files, these contain the assembly optimized
> > > > > algorithms for IA32/X64.  I think these 4 INFs can be combined into a single INF.
> > > > > Perhaps OpensslLibOpt.inf?
> > > > >
> > > > > Mike
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > > > 
> > > >



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