Re: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0

["Kilian Kegel" <KILIAN_KEGEL@OUTLOOK.COM>]

[-- Attachment #1: Type: text/plain, Size: 8294 bytes --]

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<compilerversion>.lib”
  2.  Update the MSFT_DEF.txt tool chain definition path

DEBUG_*_IA32_DLINK_FLAGS     = %CONF_PATH%\ MSFTINTRINx86-32<compilerversion>.lib

RELEASE_*_IA32_DLINK_FLAGS   = %CONF_PATH%\ MSFTINTRINx86-32<compilerversion>.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<compilerversion>.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<https://emea01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fwww.open-std.org%2Fjtc1%2Fsc22%2Fwg14%2Fwww%2Fdocs%2Fn1256.pdf%23page%3D23&data=04%7C01%7C%7C1db233037ffb4811299008d9df47ba42%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637786321422685738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=NuCQGfgDshlvQOKAQerAQaALk11LZA7YKY4eqSwotDg%3D&reserved=0>
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<https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D1516%23c2&data=04%7C01%7C%7C1db233037ffb4811299008d9df47ba42%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637786321422685738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=TV9hIhMuhINFVj8PSOQzMnGiknw3HO5zKm7ub5%2BcDow%3D&reserved=0>

This mindset violates edk2 coding style spec too:
https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/2_guiding_principles<https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fedk2-docs.gitbook.io%2Fedk-ii-c-coding-standards-specification%2F2_guiding_principles&data=04%7C01%7C%7C1db233037ffb4811299008d9df47ba42%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637786321422685738%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=QVHxAAGiXywPxVI4wKzBtvbyfW16qvigghS1uwkIIcg%3D&reserved=0>

  *   Maintainability
  *   Extensibility
  *   Intellectual manageability
  *   Portability
  *   Reusability
  *   Standard techniques

Have fun,
Kilian

From: Michael D Kinney<mailto:michael.d.kinney@intel.com>
Sent: Friday, January 21, 2022 05:39 PM
To: kraxel@redhat.com<mailto:kraxel@redhat.com>; Yao, Jiewen<mailto:jiewen.yao@intel.com>; Sean Brogan<mailto:sean.brogan@microsoft.com>; Bret Barkelew<mailto:Bret.Barkelew@microsoft.com>; Kinney, Michael D<mailto:michael.d.kinney@intel.com>
Cc: devel@edk2.groups.io<mailto:devel@edk2.groups.io>; Wang, Jian J<mailto:jian.j.wang@intel.com>; Jiang, Guomin<mailto:guomin.jiang@intel.com>; Pawel Polawski<mailto:ppolawsk@redhat.com>; Lu, XiaoyuX<mailto:xiaoyux.lu@intel.com>
Subject: Re: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0

Comments below.

Mike

> -----Original Message-----
> From: kraxel@redhat.com <kraxel@redhat.com>
> Sent: Friday, January 21, 2022 12:31 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Jiang, Guomin
> <guomin.jiang@intel.com>; Pawel Polawski <ppolawsk@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>
> 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







[-- Attachment #2: Type: text/html, Size: 19396 bytes --]