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.

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

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?

Thanks,
Christopher Zurcher

-----Original Message-----
From: Kinney, Michael D <michael.d.kin...@intel.com> 
Sent: Sunday, September 25, 2022 22:47
To: devel@edk2.groups.io; christopher.zurc...@outlook.com; Yao, Jiewen 
<jiewen....@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; Lu, Xiaoyu1 
<xiaoyu1...@intel.com>; Jiang, Guomin <guomin.ji...@intel.com>; Kinney, Michael 
D <michael.d.kin...@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.kin...@intel.com>
> Sent: Sunday, September 25, 2022 10:52 AM
> To: devel@edk2.groups.io; christopher.zurc...@outlook.com; Yao, Jiewen 
> <jiewen....@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; Lu, 
> Xiaoyu1 <xiaoyu1...@intel.com>; Jiang, Guomin 
> <guomin.ji...@intel.com>; Kinney, Michael D 
> <michael.d.kin...@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.kin...@intel.com>
> > Sent: Saturday, September 24, 2022 1:24 PM
> > To: devel@edk2.groups.io; christopher.zurc...@outlook.com; Yao, 
> > Jiewen <jiewen....@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; 
> > Lu, Xiaoyu1 <xiaoyu1...@intel.com>; Jiang, Guomin 
> > <guomin.ji...@intel.com>; Kinney, Michael D 
> > <michael.d.kin...@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > > Christopher Zurcher
> > > Sent: Friday, September 23, 2022 3:40 PM
> > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com>; 
> > > Kinney, Michael D <michael.d.kin...@intel.com>; Wang, Jian J 
> > > <jian.j.w...@intel.com>; Lu, Xiaoyu1 <xiaoyu1...@intel.com>; 
> > > Jiang, Guomin <guomin.ji...@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@edk2.groups.io <devel@edk2.groups.io> On Behalf Of 
> > > Yao, Jiewen
> > > Sent: Friday, September 23, 2022 15:33
> > > To: Kinney, Michael D <michael.d.kin...@intel.com>; 
> > > devel@edk2.groups.io; Wang, Jian J <jian.j.w...@intel.com>; Lu,
> Xiaoyu1
> > > <xiaoyu1...@intel.com>; Jiang, Guomin <guomin.ji...@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.kin...@intel.com>
> > > > Sent: Saturday, September 24, 2022 4:58 AM
> > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com>; 
> > > > Kinney, Michael D <michael.d.kin...@intel.com>; Wang, Jian J 
> > > > <jian.j.w...@intel.com>; Lu, Xiaoyu1 <xiaoyu1...@intel.com>; 
> > > > Jiang, Guomin <guomin.ji...@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 (#94314): https://edk2.groups.io/g/devel/message/94314
Mute This Topic: https://groups.io/mt/93879787/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-


Reply via email to