Hi Mike, thank you for your explanation. I understand all the technical aspects. But let me go into the details of my approach, that skips step 2) to 5) and adds step 1.5) >I think in practice, the intrinsic APIs we are seeing from use >of C code from submodules is a very limited set that do not >change across compiler releases, I totally agree. E.g INT64.LIB is the same since 2004 (WinXP DDK). But my perspective is different anyway: 1. an intrinsic library belongs to a particular compiler/linker couple 2. an intrinsic library does not belong to a UEFI tianocore or commercial BIOS feature set and should be managed outside from any EDK2 specific build description (.INF, .DSC) Let’s assume there is a C build environment fully installed, e.g. Microsoft VS2022, on an EDK2 build machine and all legal aspects were fulfilled. In that case the advanced developer is able to locate the library, that holds the intrinsic functions (intrinsic .OBJ modules) we needed to extract. (simply by checking the .MAP of a 32bit executable that pulls in the particular intrinsics) This is step 1) >One of the challenges is that compilers are allowed to add/remove/modify intrinsic APIs >across compiler releases. We would need to define a solution that will work if there are >these types of changes, which would potentially mean a different instance of the intrinsic >library for each tool chain tag. Here comes step 1.5): In case of Microsoft build it is LIBCMT.LIB that can be found when walking through the LIB environment string. It is easy to extend EDKSETUP.BAT to generate MSFTINTRINx86-32.LIB each time: 1. locate LIBCMT.LIB 2. extract the identified .OBJ modules from step 1: “LIB.EXE /extract:full_path_name.obj /out:name.obj LIBCMT.LIB” 3. bind extracted .OBJ to the MSFTINTRINx86-32.LIB: “LIB.EXE *.obj /out:%CONF_PATH%\MSFTINTRINx86-32.lib” Now MSFTINTRINx86-32.LIB is located in the conf directory. Adjust the DLINK_FLAGS in tools_def.txt to hold MSFTINTRINx86-32.LIB as a search library: DEBUG_VS2015_IA32_DLINK_FLAGS = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF /OPT:ICF=10 /MAP /ALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /DEBUG %CONF_PATH%\MSFTINTRINx86-32.LIB RELEASE_VS2015_IA32_DLINK_FLAGS = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /IGNORE:4254 /OPT:REF /OPT:ICF=10 /MAP /ALIGN:32 /SECTION:.xdata,D /SECTION:.pdata,D /MACHINE:X86 /LTCG /DLL /ENTRY:$(IMAGE_ENTRY_POINT) /SUBSYSTEM:EFI_BOOT_SERVICE_DRIVER /SAFESEH:NO /BASE:0 /DRIVER /MERGE:.rdata=.data %CONF_PATH%\MSFTINTRINx86-32.LIB >From now on the intrinsics are available for all 32Bit components. With that procedure it is guaranteed by design, that the intrinsics are always available and match a particular compiler/linker release. * it saves storage space since source code or binary modules don’t need to be kept * because they are not kept, they don’t need maintainance * no one needs to understand and document (in math details) the internals and the interface * no one needs to test for the math functions, as it is necessary for tianocore re-implementations * the compiler vendor itself is in charge in a court case The script below is a demonstration of the above arguments. It additionally adds memcpy() and memcmp() to MSFTINTRINx86-32.LIB, that the compiler sometimes needs, depending on optimization style, array size, instruction set, whatsoever … [cid:image001.png@01D8120F.B2B00880] I have checked some more .OBJ from LIBCMT.LIB and some of them (ftol3.obj to get __ltod3() long to double needed for difftime()) seem not to be space optimized for BIOS usage, because the ftol3.obj holds multiple functions (and so violates the ODR one definition rule). But also such cases could be handled automatically by a script (I wrote a C program < 200 lines) that disassembles (using Microsoft DUMPBIN.EXE) the .OBJ, splits by simple text processing into single-function-.ASM-files that could be assembled back to multiple .OBJ of smaller code size. For this approach compiler, library manager, disassembler (DUMPBIN, OBJDUMP) were needed, that are available on all build machines by definition. Best regards Kilian From: Kinney, Michael D Sent: Monday, January 24, 2022 06:28 PM To: Kilian Kegel; devel@edk2.groups.io; kraxel@redhat.com; Yao, Jiewen; Sean Brogan; Bret Barkelew; Kinney, Michael D Cc: devel@edk2.groups.io; Wang, Jian J; Jiang, Guomin; Pawel Polawski; Lu, XiaoyuX Subject: RE: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0 Hi Kilian, I am in favor of an intrinsic lib to improve the EDK II development environment. This has already been done for ARM compilers. The solution should mirror that approach. It would be best if we had source code (either in the edk2 repo or through a submodule) for the required intrinsic APIs. If source code is not possible and we have to use a binary, then that must be accessed through a submodule. The edk2 repo does not host binaries. We use repos such as edk2-non-osi for binaries. We also have to provide a solution that works with supported compilers (VS, GCC, CLANG, XCODE). One of the challenges is that compilers are allowed to add/remove/modify intrinsic APIs across compiler releases. We would need to define a solution that will work if there are these types of changes, which would potentially mean a different instance of the intrinsic library for each tool chain tag. I think in practice, the intrinsic APIs we are seeing from use of C code from submodules is a very limited set that do not change across compiler releases, so the maintenance of these intrinsic libs would be manageable. If we go down the source code path, we can break it up into the following tasks: 1. Identify the specific subset of intrinsic APIs from each compiler that is required for the edk2 use cases. 2. Obtain the function prototype and full documentation for each intrinsic API to support implementation and unit tests. 3. Implement the APIs for all compilers. 4. Implement unit tests for all APIs for all compilers using UnitTestFrameworkPkg unit tests. 5. Update MdeLibs.dsc.inc with the NULL instances for the intrinsic libs 6. Remove intrinsic APIs from EDK II modules that currently maintain their own implementations of intrinsic APIs. Best regards, Mike From: Kilian Kegel Sent: Monday, January 24, 2022 8:25 AM To: devel@edk2.groups.io; Kinney, Michael D ; kraxel@redhat.com; Yao, Jiewen ; Sean Brogan ; Bret Barkelew Cc: devel@edk2.groups.io; Wang, Jian J ; Jiang, Guomin ; Pawel Polawski ; Lu, XiaoyuX Subject: RE: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0 The 64-bit integer math intrinsics and other intrinsic problems could be solved easily for ever: 1. Putting all .OBJ files together from LIBCMT.H or INT64.LIB (for ll*.obj and ull*.obj only) ltod3.obj ftol2.obj lldiv.obj lldvrm.obj llmul.obj llrem.obj llshl.obj llshr.obj ulldiv.obj ulldvrm.obj ullrem.obj ullshr.obj memcmp.obj memcpycpy.obj and adjust for usability in EDK2 (remove / solve further internal dependencies or rewrite “*cpy” and “*cmp” functions) This is already done in IntrinsicLib.lib for some of the above functions, just complete this task! 1. Put all the .OBJ into a e.g. edk2\Conf \“MSFTINTRINx86-32.lib” 2. Update the MSFT_DEF.txt tool chain definition path DEBUG_*_IA32_DLINK_FLAGS = %CONF_PATH%\ MSFTINTRINx86-32.lib RELEASE_*_IA32_DLINK_FLAGS = %CONF_PATH%\ MSFTINTRINx86-32.lib 1. Resolve build conflicts with other existing intrinsic libraries from CryptoPkg, RedfishPkg… – remove these libraries >From now on all existing 32Bit components have access to their own compiler intrinsics without touching any .INF file and the problem is instantly gone. Please do the same for * GCCINTRINx86-32.lib Leave the intrinsic restrictions behind and just provide all required intrinsics the compiler needs to fulfil the C-Standard! UEFI shall conform the execution environment described in the C Specification http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1256.pdf#page=23 and shall not try to create a new restricted “UEFI execution environment” that currently prohibits some “expressions” (shift << >> , divide / % ) on some “data types” (64bit “long long”) but maybe in the future will prohibit some more “expressions” (logical AND &&, relational-expression < >) on still speculative “data types” (e.g. a 128bit “extended long”) or just because a new compiler (version) with some new optimization(ultra slow)/security(specdown/meltre) capabilities introduces some new intrinsic functions. Who knows… In contrast to: “I think we shouldn't add any intrinsics unless we are absolutely forced to. I do agree however that, for those intrinsics that we cannot at all avoid reimplementing, we should at least collect them in a common library. (In theory, I can also imagine reimplementing all possible intrinsics *if* the edk2 coding style spec / requirements are updated in parallel, permitting all new code to universally rely on the intrinsics, rather than the BaseLib / BaseMemoryLib functions.)” https://bugzilla.tianocore.org/show_bug.cgi?id=1516#c2 This mindset violates edk2 coding style spec too: https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/2_guiding_principles * Maintainability * Extensibility * Intellectual manageability * Portability * Reusability * Standard techniques Have fun, Kilian From: Michael D Kinney Sent: Friday, January 21, 2022 05:39 PM To: kraxel@redhat.com; Yao, Jiewen; Sean Brogan; Bret Barkelew; Kinney, Michael D Cc: devel@edk2.groups.io; Wang, Jian J; Jiang, Guomin; Pawel Polawski; Lu, XiaoyuX Subject: Re: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0 Comments below. Mike > -----Original Message----- > From: kraxel@redhat.com > > Sent: Friday, January 21, 2022 12:31 AM > To: Yao, Jiewen > > Cc: devel@edk2.groups.io; Kinney, Michael D >; Wang, Jian J >; Jiang, Guomin > >; Pawel Polawski >; Lu, XiaoyuX > > Subject: Re: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0 > > > > No changes in SEC and PEI. > > [Jiewen] Do you mean the Crypto consumer in PEI has no size difference? Such as > > https://github.com/tianocore/edk2/tree/master/SecurityPkg/Tcg/Tcg2Pei , > > https://github.com/tianocore/edk2/tree/master/SecurityPkg/FvReportPei , > > https://github.com/tianocore/edk2/tree/master/SignedCapsulePkg/Universal/RecoveryModuleLoadPei linking > https://github.com/tianocore/edk2/tree/master/SecurityPkg/Library/FmpAuthenticationLibRsa2048Sha256. > > PEI has this (OvmfIa32X64Pkg build): > > 7062 TpmMmioSevDecryptPei > 7830 StatusCodeHandlerPei > 7902 ReportStatusCodeRouterPei > 8470 FaultTolerantWritePei > 9734 SmmAccessPei > 11206 Tcg2ConfigPei > 11842 PeiVariable > 14730 Tcg2PlatformPei > 17274 TcgPei > 18438 S3Resume2Pei > 18682 DxeIpl > 18938 PcdPeim > 38014 CpuMpPei > 39554 PlatformPei > 45050 PeiCore > 49274 Tcg2Pei > > No size change for Tcg2Pei. > > The other modules are not there. Seems they are related to firmware > updates. We don't have that on ovmf as we can simply update the > firmware image files on the host machine ... > > Is there some target I could use to test-build those modules? > > > > INFO - OpensslLibCrypto.lib(rsa_lib.obj) : error LNK2001: unresolved external > > > symbol __allmul > > > INFO - OpensslLibCrypto.lib(rsa_lib.obj) : error LNK2001: unresolved external > > > symbol __aulldiv > > > INFO - OpensslLibCrypto.lib(bio_print.obj) : error LNK2001: unresolved external > > > symbol __aulldvrm > > > INFO - OpensslLibCrypto.lib(bio_print.obj) : error LNK2001: unresolved external > > > symbol __ftol2_sse > > > > > > Those symbols look like they reference helper functions to do 64bit math > > > on 32bit architecture. Any hints how to fix that? > > [Jiewen] Please add them to https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/IntrinsicLib > > Any hints where I could get them? Given this happens on windows builds > it's probably somewhere in the microsoft standard C library? Is that > available as open source somewhere? Sean and Bret may be able to help with these. There is also a BZ on this topic. https://bugzilla.tianocore.org/show_bug.cgi?id=1516 > > > > (3) Some NOOPT builds are failing due to the size growing ... > > [Jiewen] Size becomes big challenge... > > Have you tried to use https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver solution? > > Seems the idea is to have only one openssl copy in the dxe image by > calling a protocol instead of linking a lib. Makes sense. > > Is this documented somewhere? Is there some easy way to use that as > drop-in replacement? Or do we have to change all crypto users to call > the driver instead of linking the lib? > > take care, > Gerd