[PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0


Gerd Hoffmann
 

Very first take on updating openssl to 3.0.

Some hacks are in there still, only limited testing
(no CI runs), so cleary not complete yet. Review
comments and other hints are welcome nevertheless.

take care,
Gerd

Gerd Hoffmann (24):
CryptoPkg/openssl: update submodule to 3.0
CryptoPkg/openssl: process_files.pl: drop UefiAsm.conf
CryptoPkg/openssl: process_files.pl: expand *.a
CryptoPkg/openssl: process_files.pl: set api to 1.1.1
CryptoPkg/openssl: process_files.pl: change config header handling
CryptoPkg/openssl: process_files.pl: provider headers
CryptoPkg/openssl: process_files.pl: skip unused files
CryptoPkg/openssl: process_files.pl: clean up when done
CryptoPkg/openssl: process_files.pl: filter out crypto/buildinf.h
CryptoPkg/openssl: update generated files
CryptoPkg/BaseCryptLib: no openssl deprecation warnings please
CryptoPkg/BaseCryptLib; adapt CryptSm3.c to openssl 3.0 changes.
CryptoPkg/BaseCryptLib: add more bio print dummies
CryptoPkg/openssl: adapt rand_pool.c to openssl 3.0 changes
CryptoPkg/openssl: add dummy file store
CryptoPkg/openssl: move compiler_flags to buildinf.c
CryptoPkg/CrtLibSupport: add fcntl.h
CryptoPkg/CrtLibSupport: add strstr()
CryptoPkg/CrtLibSupport: add INT_MIN
CryptoPkg/CrtLibSupport: add UINT_MAX
CryptoPkg/CrtLibSupport: add MODULESDIR
CryptoPkg/openssl: process_files.pl: copy generated der/*.c source
files.
CryptoPkg/openssl: add generated files der source files
[hack] turn off -Werror

CryptoPkg/Library/OpensslLib/OpensslLib.inf | 1305 +++++----
.../Library/OpensslLib/OpensslLibCrypto.inf | 1220 +++++---
.../Library/OpensslLib/OpensslLibX64.inf | 1 +
.../Library/OpensslLib/OpensslLibX64Gcc.inf | 1 +
.../Library/BaseCryptLib/InternalCryptLib.h | 2 +
CryptoPkg/Library/Include/CrtLibSupport.h | 4 +
CryptoPkg/Library/Include/crypto/bn_conf.h | 29 +
CryptoPkg/Library/Include/crypto/dso_conf.h | 8 +-
CryptoPkg/Library/Include/fcntl.h | 9 +
CryptoPkg/Library/Include/openssl/asn1.h | 1128 +++++++
CryptoPkg/Library/Include/openssl/asn1t.h | 946 ++++++
CryptoPkg/Library/Include/openssl/bio.h | 884 ++++++
CryptoPkg/Library/Include/openssl/cmp.h | 592 ++++
CryptoPkg/Library/Include/openssl/cms.h | 493 ++++
CryptoPkg/Library/Include/openssl/conf.h | 211 ++
.../Library/Include/openssl/configuration.h | 286 ++
CryptoPkg/Library/Include/openssl/crmf.h | 227 ++
CryptoPkg/Library/Include/openssl/crypto.h | 556 ++++
CryptoPkg/Library/Include/openssl/ct.h | 573 ++++
CryptoPkg/Library/Include/openssl/err.h | 492 ++++
CryptoPkg/Library/Include/openssl/ess.h | 128 +
CryptoPkg/Library/Include/openssl/fipskey.h | 36 +
CryptoPkg/Library/Include/openssl/lhash.h | 288 ++
CryptoPkg/Library/Include/openssl/ocsp.h | 483 +++
.../Library/Include/openssl/opensslconf.h | 348 ---
CryptoPkg/Library/Include/openssl/opensslv.h | 114 +
CryptoPkg/Library/Include/openssl/pkcs12.h | 350 +++
CryptoPkg/Library/Include/openssl/pkcs7.h | 427 +++
CryptoPkg/Library/Include/openssl/safestack.h | 297 ++
CryptoPkg/Library/Include/openssl/srp.h | 285 ++
CryptoPkg/Library/Include/openssl/ssl.h | 2585 +++++++++++++++++
CryptoPkg/Library/Include/openssl/ui.h | 407 +++
CryptoPkg/Library/Include/openssl/x509.h | 1276 ++++++++
CryptoPkg/Library/Include/openssl/x509_vfy.h | 894 ++++++
CryptoPkg/Library/Include/openssl/x509v3.h | 1450 +++++++++
CryptoPkg/Library/Include/prov/bio.h | 32 +
CryptoPkg/Library/Include/prov/blake2.h | 120 +
CryptoPkg/Library/Include/prov/ciphercommon.h | 361 +++
.../Library/Include/prov/ciphercommon_aead.h | 47 +
.../Library/Include/prov/ciphercommon_ccm.h | 100 +
.../Library/Include/prov/ciphercommon_gcm.h | 129 +
CryptoPkg/Library/Include/prov/der_digests.h | 160 +
CryptoPkg/Library/Include/prov/der_dsa.h | 94 +
CryptoPkg/Library/Include/prov/der_ec.h | 286 ++
CryptoPkg/Library/Include/prov/der_ecx.h | 50 +
CryptoPkg/Library/Include/prov/der_rsa.h | 187 ++
CryptoPkg/Library/Include/prov/der_sm2.h | 37 +
CryptoPkg/Library/Include/prov/der_wrap.h | 46 +
CryptoPkg/Library/Include/prov/digestcommon.h | 123 +
.../Library/Include/prov/implementations.h | 516 ++++
CryptoPkg/Library/Include/prov/kdfexchange.h | 24 +
CryptoPkg/Library/Include/prov/macsignature.h | 30 +
CryptoPkg/Library/Include/prov/md5_sha1.h | 36 +
CryptoPkg/Library/Include/prov/names.h | 327 +++
CryptoPkg/Library/Include/prov/proverr.h | 27 +
CryptoPkg/Library/Include/prov/provider_ctx.h | 40 +
.../Library/Include/prov/provider_util.h | 138 +
.../Library/Include/prov/providercommon.h | 24 +
.../Library/Include/prov/securitycheck.h | 30 +
CryptoPkg/Library/Include/prov/seeding.h | 41 +
CryptoPkg/Library/OpensslLib/buildinf.h | 2 +-
.../Library/BaseCryptLib/Hash/CryptSm3.c | 14 +-
.../Library/BaseCryptLib/SysCall/CrtWrapper.c | 10 +
.../OpensslLib/{buildinf.h => buildinf.c} | 3 +-
.../Library/OpensslLib/der_digests_gen.c | 160 +
CryptoPkg/Library/OpensslLib/der_rsa_gen.c | 174 ++
CryptoPkg/Library/OpensslLib/der_wrap_gen.c | 46 +
CryptoPkg/Library/OpensslLib/ossl_store.c | 11 +
CryptoPkg/Library/OpensslLib/rand_pool.c | 20 +-
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/process_files.pl | 79 +-
71 files changed, 20510 insertions(+), 1351 deletions(-)
create mode 100644 CryptoPkg/Library/Include/crypto/bn_conf.h
create mode 100644 CryptoPkg/Library/Include/fcntl.h
create mode 100644 CryptoPkg/Library/Include/openssl/asn1.h
create mode 100644 CryptoPkg/Library/Include/openssl/asn1t.h
create mode 100644 CryptoPkg/Library/Include/openssl/bio.h
create mode 100644 CryptoPkg/Library/Include/openssl/cmp.h
create mode 100644 CryptoPkg/Library/Include/openssl/cms.h
create mode 100644 CryptoPkg/Library/Include/openssl/conf.h
create mode 100644 CryptoPkg/Library/Include/openssl/configuration.h
create mode 100644 CryptoPkg/Library/Include/openssl/crmf.h
create mode 100644 CryptoPkg/Library/Include/openssl/crypto.h
create mode 100644 CryptoPkg/Library/Include/openssl/ct.h
create mode 100644 CryptoPkg/Library/Include/openssl/err.h
create mode 100644 CryptoPkg/Library/Include/openssl/ess.h
create mode 100644 CryptoPkg/Library/Include/openssl/fipskey.h
create mode 100644 CryptoPkg/Library/Include/openssl/lhash.h
create mode 100644 CryptoPkg/Library/Include/openssl/ocsp.h
delete mode 100644 CryptoPkg/Library/Include/openssl/opensslconf.h
create mode 100644 CryptoPkg/Library/Include/openssl/opensslv.h
create mode 100644 CryptoPkg/Library/Include/openssl/pkcs12.h
create mode 100644 CryptoPkg/Library/Include/openssl/pkcs7.h
create mode 100644 CryptoPkg/Library/Include/openssl/safestack.h
create mode 100644 CryptoPkg/Library/Include/openssl/srp.h
create mode 100644 CryptoPkg/Library/Include/openssl/ssl.h
create mode 100644 CryptoPkg/Library/Include/openssl/ui.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509_vfy.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509v3.h
create mode 100644 CryptoPkg/Library/Include/prov/bio.h
create mode 100644 CryptoPkg/Library/Include/prov/blake2.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_aead.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_ccm.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_gcm.h
create mode 100644 CryptoPkg/Library/Include/prov/der_digests.h
create mode 100644 CryptoPkg/Library/Include/prov/der_dsa.h
create mode 100644 CryptoPkg/Library/Include/prov/der_ec.h
create mode 100644 CryptoPkg/Library/Include/prov/der_ecx.h
create mode 100644 CryptoPkg/Library/Include/prov/der_rsa.h
create mode 100644 CryptoPkg/Library/Include/prov/der_sm2.h
create mode 100644 CryptoPkg/Library/Include/prov/der_wrap.h
create mode 100644 CryptoPkg/Library/Include/prov/digestcommon.h
create mode 100644 CryptoPkg/Library/Include/prov/implementations.h
create mode 100644 CryptoPkg/Library/Include/prov/kdfexchange.h
create mode 100644 CryptoPkg/Library/Include/prov/macsignature.h
create mode 100644 CryptoPkg/Library/Include/prov/md5_sha1.h
create mode 100644 CryptoPkg/Library/Include/prov/names.h
create mode 100644 CryptoPkg/Library/Include/prov/proverr.h
create mode 100644 CryptoPkg/Library/Include/prov/provider_ctx.h
create mode 100644 CryptoPkg/Library/Include/prov/provider_util.h
create mode 100644 CryptoPkg/Library/Include/prov/providercommon.h
create mode 100644 CryptoPkg/Library/Include/prov/securitycheck.h
create mode 100644 CryptoPkg/Library/Include/prov/seeding.h
copy CryptoPkg/Library/OpensslLib/{buildinf.h => buildinf.c} (50%)
create mode 100644 CryptoPkg/Library/OpensslLib/der_digests_gen.c
create mode 100644 CryptoPkg/Library/OpensslLib/der_rsa_gen.c
create mode 100644 CryptoPkg/Library/OpensslLib/der_wrap_gen.c

--
2.33.1


Michael D Kinney
 

Hi Gerd,

Thank you for starting this work!

Can you point the community as a summary of the changes/improvements in v3.0 and your
take on why it is important to upgrade TianoCore.

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann
Sent: Friday, December 3, 2021 8:07 AM
To: devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
Pawel Polawski <ppolawsk@redhat.com>; Philippe Mathieu-Daudé <philmd@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Gerd
Hoffmann <kraxel@redhat.com>
Subject: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl submodule to v3.0

Very first take on updating openssl to 3.0.

Some hacks are in there still, only limited testing
(no CI runs), so cleary not complete yet. Review
comments and other hints are welcome nevertheless.

take care,
Gerd

Gerd Hoffmann (24):
CryptoPkg/openssl: update submodule to 3.0
CryptoPkg/openssl: process_files.pl: drop UefiAsm.conf
CryptoPkg/openssl: process_files.pl: expand *.a
CryptoPkg/openssl: process_files.pl: set api to 1.1.1
CryptoPkg/openssl: process_files.pl: change config header handling
CryptoPkg/openssl: process_files.pl: provider headers
CryptoPkg/openssl: process_files.pl: skip unused files
CryptoPkg/openssl: process_files.pl: clean up when done
CryptoPkg/openssl: process_files.pl: filter out crypto/buildinf.h
CryptoPkg/openssl: update generated files
CryptoPkg/BaseCryptLib: no openssl deprecation warnings please
CryptoPkg/BaseCryptLib; adapt CryptSm3.c to openssl 3.0 changes.
CryptoPkg/BaseCryptLib: add more bio print dummies
CryptoPkg/openssl: adapt rand_pool.c to openssl 3.0 changes
CryptoPkg/openssl: add dummy file store
CryptoPkg/openssl: move compiler_flags to buildinf.c
CryptoPkg/CrtLibSupport: add fcntl.h
CryptoPkg/CrtLibSupport: add strstr()
CryptoPkg/CrtLibSupport: add INT_MIN
CryptoPkg/CrtLibSupport: add UINT_MAX
CryptoPkg/CrtLibSupport: add MODULESDIR
CryptoPkg/openssl: process_files.pl: copy generated der/*.c source
files.
CryptoPkg/openssl: add generated files der source files
[hack] turn off -Werror

CryptoPkg/Library/OpensslLib/OpensslLib.inf | 1305 +++++----
.../Library/OpensslLib/OpensslLibCrypto.inf | 1220 +++++---
.../Library/OpensslLib/OpensslLibX64.inf | 1 +
.../Library/OpensslLib/OpensslLibX64Gcc.inf | 1 +
.../Library/BaseCryptLib/InternalCryptLib.h | 2 +
CryptoPkg/Library/Include/CrtLibSupport.h | 4 +
CryptoPkg/Library/Include/crypto/bn_conf.h | 29 +
CryptoPkg/Library/Include/crypto/dso_conf.h | 8 +-
CryptoPkg/Library/Include/fcntl.h | 9 +
CryptoPkg/Library/Include/openssl/asn1.h | 1128 +++++++
CryptoPkg/Library/Include/openssl/asn1t.h | 946 ++++++
CryptoPkg/Library/Include/openssl/bio.h | 884 ++++++
CryptoPkg/Library/Include/openssl/cmp.h | 592 ++++
CryptoPkg/Library/Include/openssl/cms.h | 493 ++++
CryptoPkg/Library/Include/openssl/conf.h | 211 ++
.../Library/Include/openssl/configuration.h | 286 ++
CryptoPkg/Library/Include/openssl/crmf.h | 227 ++
CryptoPkg/Library/Include/openssl/crypto.h | 556 ++++
CryptoPkg/Library/Include/openssl/ct.h | 573 ++++
CryptoPkg/Library/Include/openssl/err.h | 492 ++++
CryptoPkg/Library/Include/openssl/ess.h | 128 +
CryptoPkg/Library/Include/openssl/fipskey.h | 36 +
CryptoPkg/Library/Include/openssl/lhash.h | 288 ++
CryptoPkg/Library/Include/openssl/ocsp.h | 483 +++
.../Library/Include/openssl/opensslconf.h | 348 ---
CryptoPkg/Library/Include/openssl/opensslv.h | 114 +
CryptoPkg/Library/Include/openssl/pkcs12.h | 350 +++
CryptoPkg/Library/Include/openssl/pkcs7.h | 427 +++
CryptoPkg/Library/Include/openssl/safestack.h | 297 ++
CryptoPkg/Library/Include/openssl/srp.h | 285 ++
CryptoPkg/Library/Include/openssl/ssl.h | 2585 +++++++++++++++++
CryptoPkg/Library/Include/openssl/ui.h | 407 +++
CryptoPkg/Library/Include/openssl/x509.h | 1276 ++++++++
CryptoPkg/Library/Include/openssl/x509_vfy.h | 894 ++++++
CryptoPkg/Library/Include/openssl/x509v3.h | 1450 +++++++++
CryptoPkg/Library/Include/prov/bio.h | 32 +
CryptoPkg/Library/Include/prov/blake2.h | 120 +
CryptoPkg/Library/Include/prov/ciphercommon.h | 361 +++
.../Library/Include/prov/ciphercommon_aead.h | 47 +
.../Library/Include/prov/ciphercommon_ccm.h | 100 +
.../Library/Include/prov/ciphercommon_gcm.h | 129 +
CryptoPkg/Library/Include/prov/der_digests.h | 160 +
CryptoPkg/Library/Include/prov/der_dsa.h | 94 +
CryptoPkg/Library/Include/prov/der_ec.h | 286 ++
CryptoPkg/Library/Include/prov/der_ecx.h | 50 +
CryptoPkg/Library/Include/prov/der_rsa.h | 187 ++
CryptoPkg/Library/Include/prov/der_sm2.h | 37 +
CryptoPkg/Library/Include/prov/der_wrap.h | 46 +
CryptoPkg/Library/Include/prov/digestcommon.h | 123 +
.../Library/Include/prov/implementations.h | 516 ++++
CryptoPkg/Library/Include/prov/kdfexchange.h | 24 +
CryptoPkg/Library/Include/prov/macsignature.h | 30 +
CryptoPkg/Library/Include/prov/md5_sha1.h | 36 +
CryptoPkg/Library/Include/prov/names.h | 327 +++
CryptoPkg/Library/Include/prov/proverr.h | 27 +
CryptoPkg/Library/Include/prov/provider_ctx.h | 40 +
.../Library/Include/prov/provider_util.h | 138 +
.../Library/Include/prov/providercommon.h | 24 +
.../Library/Include/prov/securitycheck.h | 30 +
CryptoPkg/Library/Include/prov/seeding.h | 41 +
CryptoPkg/Library/OpensslLib/buildinf.h | 2 +-
.../Library/BaseCryptLib/Hash/CryptSm3.c | 14 +-
.../Library/BaseCryptLib/SysCall/CrtWrapper.c | 10 +
.../OpensslLib/{buildinf.h => buildinf.c} | 3 +-
.../Library/OpensslLib/der_digests_gen.c | 160 +
CryptoPkg/Library/OpensslLib/der_rsa_gen.c | 174 ++
CryptoPkg/Library/OpensslLib/der_wrap_gen.c | 46 +
CryptoPkg/Library/OpensslLib/ossl_store.c | 11 +
CryptoPkg/Library/OpensslLib/rand_pool.c | 20 +-
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/process_files.pl | 79 +-
71 files changed, 20510 insertions(+), 1351 deletions(-)
create mode 100644 CryptoPkg/Library/Include/crypto/bn_conf.h
create mode 100644 CryptoPkg/Library/Include/fcntl.h
create mode 100644 CryptoPkg/Library/Include/openssl/asn1.h
create mode 100644 CryptoPkg/Library/Include/openssl/asn1t.h
create mode 100644 CryptoPkg/Library/Include/openssl/bio.h
create mode 100644 CryptoPkg/Library/Include/openssl/cmp.h
create mode 100644 CryptoPkg/Library/Include/openssl/cms.h
create mode 100644 CryptoPkg/Library/Include/openssl/conf.h
create mode 100644 CryptoPkg/Library/Include/openssl/configuration.h
create mode 100644 CryptoPkg/Library/Include/openssl/crmf.h
create mode 100644 CryptoPkg/Library/Include/openssl/crypto.h
create mode 100644 CryptoPkg/Library/Include/openssl/ct.h
create mode 100644 CryptoPkg/Library/Include/openssl/err.h
create mode 100644 CryptoPkg/Library/Include/openssl/ess.h
create mode 100644 CryptoPkg/Library/Include/openssl/fipskey.h
create mode 100644 CryptoPkg/Library/Include/openssl/lhash.h
create mode 100644 CryptoPkg/Library/Include/openssl/ocsp.h
delete mode 100644 CryptoPkg/Library/Include/openssl/opensslconf.h
create mode 100644 CryptoPkg/Library/Include/openssl/opensslv.h
create mode 100644 CryptoPkg/Library/Include/openssl/pkcs12.h
create mode 100644 CryptoPkg/Library/Include/openssl/pkcs7.h
create mode 100644 CryptoPkg/Library/Include/openssl/safestack.h
create mode 100644 CryptoPkg/Library/Include/openssl/srp.h
create mode 100644 CryptoPkg/Library/Include/openssl/ssl.h
create mode 100644 CryptoPkg/Library/Include/openssl/ui.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509_vfy.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509v3.h
create mode 100644 CryptoPkg/Library/Include/prov/bio.h
create mode 100644 CryptoPkg/Library/Include/prov/blake2.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_aead.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_ccm.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_gcm.h
create mode 100644 CryptoPkg/Library/Include/prov/der_digests.h
create mode 100644 CryptoPkg/Library/Include/prov/der_dsa.h
create mode 100644 CryptoPkg/Library/Include/prov/der_ec.h
create mode 100644 CryptoPkg/Library/Include/prov/der_ecx.h
create mode 100644 CryptoPkg/Library/Include/prov/der_rsa.h
create mode 100644 CryptoPkg/Library/Include/prov/der_sm2.h
create mode 100644 CryptoPkg/Library/Include/prov/der_wrap.h
create mode 100644 CryptoPkg/Library/Include/prov/digestcommon.h
create mode 100644 CryptoPkg/Library/Include/prov/implementations.h
create mode 100644 CryptoPkg/Library/Include/prov/kdfexchange.h
create mode 100644 CryptoPkg/Library/Include/prov/macsignature.h
create mode 100644 CryptoPkg/Library/Include/prov/md5_sha1.h
create mode 100644 CryptoPkg/Library/Include/prov/names.h
create mode 100644 CryptoPkg/Library/Include/prov/proverr.h
create mode 100644 CryptoPkg/Library/Include/prov/provider_ctx.h
create mode 100644 CryptoPkg/Library/Include/prov/provider_util.h
create mode 100644 CryptoPkg/Library/Include/prov/providercommon.h
create mode 100644 CryptoPkg/Library/Include/prov/securitycheck.h
create mode 100644 CryptoPkg/Library/Include/prov/seeding.h
copy CryptoPkg/Library/OpensslLib/{buildinf.h => buildinf.c} (50%)
create mode 100644 CryptoPkg/Library/OpensslLib/der_digests_gen.c
create mode 100644 CryptoPkg/Library/OpensslLib/der_rsa_gen.c
create mode 100644 CryptoPkg/Library/OpensslLib/der_wrap_gen.c

--
2.33.1





Yao, Jiewen
 

Also, assuming you have done enough test, would you please provide:
1) size difference, Including PEI, SMM, DXE.
2) performance difference, Including PEI, SMM, DXE.
3) what unit test you have done (such as each crypto API)
4) what system test you have done (such as secure boot, trusted boot)

Thank you
Yao Jiewen

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com>
Sent: Saturday, December 4, 2021 12:33 AM
To: devel@edk2.groups.io; kraxel@redhat.com; Kinney, Michael D
<michael.d.kinney@intel.com>
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
Jiang, Guomin <guomin.jiang@intel.com>; Pawel Polawski
<ppolawsk@redhat.com>; Philippe Mathieu-Daudé <philmd@redhat.com>; Lu,
XiaoyuX <xiaoyux.lu@intel.com>
Subject: RE: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl
submodule to v3.0

Hi Gerd,

Thank you for starting this work!

Can you point the community as a summary of the changes/improvements in
v3.0 and your
take on why it is important to upgrade TianoCore.

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd
Hoffmann
Sent: Friday, December 3, 2021 8:07 AM
To: devel@edk2.groups.io
Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
<jian.j.wang@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
Pawel Polawski <ppolawsk@redhat.com>; Philippe Mathieu-Daudé
<philmd@redhat.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Gerd
Hoffmann <kraxel@redhat.com>
Subject: [edk2-devel] [PATCH 00/24] CryptoPkg/openssl: update openssl
submodule to v3.0

Very first take on updating openssl to 3.0.

Some hacks are in there still, only limited testing
(no CI runs), so cleary not complete yet. Review
comments and other hints are welcome nevertheless.

take care,
Gerd

Gerd Hoffmann (24):
CryptoPkg/openssl: update submodule to 3.0
CryptoPkg/openssl: process_files.pl: drop UefiAsm.conf
CryptoPkg/openssl: process_files.pl: expand *.a
CryptoPkg/openssl: process_files.pl: set api to 1.1.1
CryptoPkg/openssl: process_files.pl: change config header handling
CryptoPkg/openssl: process_files.pl: provider headers
CryptoPkg/openssl: process_files.pl: skip unused files
CryptoPkg/openssl: process_files.pl: clean up when done
CryptoPkg/openssl: process_files.pl: filter out crypto/buildinf.h
CryptoPkg/openssl: update generated files
CryptoPkg/BaseCryptLib: no openssl deprecation warnings please
CryptoPkg/BaseCryptLib; adapt CryptSm3.c to openssl 3.0 changes.
CryptoPkg/BaseCryptLib: add more bio print dummies
CryptoPkg/openssl: adapt rand_pool.c to openssl 3.0 changes
CryptoPkg/openssl: add dummy file store
CryptoPkg/openssl: move compiler_flags to buildinf.c
CryptoPkg/CrtLibSupport: add fcntl.h
CryptoPkg/CrtLibSupport: add strstr()
CryptoPkg/CrtLibSupport: add INT_MIN
CryptoPkg/CrtLibSupport: add UINT_MAX
CryptoPkg/CrtLibSupport: add MODULESDIR
CryptoPkg/openssl: process_files.pl: copy generated der/*.c source
files.
CryptoPkg/openssl: add generated files der source files
[hack] turn off -Werror

CryptoPkg/Library/OpensslLib/OpensslLib.inf | 1305 +++++----
.../Library/OpensslLib/OpensslLibCrypto.inf | 1220 +++++---
.../Library/OpensslLib/OpensslLibX64.inf | 1 +
.../Library/OpensslLib/OpensslLibX64Gcc.inf | 1 +
.../Library/BaseCryptLib/InternalCryptLib.h | 2 +
CryptoPkg/Library/Include/CrtLibSupport.h | 4 +
CryptoPkg/Library/Include/crypto/bn_conf.h | 29 +
CryptoPkg/Library/Include/crypto/dso_conf.h | 8 +-
CryptoPkg/Library/Include/fcntl.h | 9 +
CryptoPkg/Library/Include/openssl/asn1.h | 1128 +++++++
CryptoPkg/Library/Include/openssl/asn1t.h | 946 ++++++
CryptoPkg/Library/Include/openssl/bio.h | 884 ++++++
CryptoPkg/Library/Include/openssl/cmp.h | 592 ++++
CryptoPkg/Library/Include/openssl/cms.h | 493 ++++
CryptoPkg/Library/Include/openssl/conf.h | 211 ++
.../Library/Include/openssl/configuration.h | 286 ++
CryptoPkg/Library/Include/openssl/crmf.h | 227 ++
CryptoPkg/Library/Include/openssl/crypto.h | 556 ++++
CryptoPkg/Library/Include/openssl/ct.h | 573 ++++
CryptoPkg/Library/Include/openssl/err.h | 492 ++++
CryptoPkg/Library/Include/openssl/ess.h | 128 +
CryptoPkg/Library/Include/openssl/fipskey.h | 36 +
CryptoPkg/Library/Include/openssl/lhash.h | 288 ++
CryptoPkg/Library/Include/openssl/ocsp.h | 483 +++
.../Library/Include/openssl/opensslconf.h | 348 ---
CryptoPkg/Library/Include/openssl/opensslv.h | 114 +
CryptoPkg/Library/Include/openssl/pkcs12.h | 350 +++
CryptoPkg/Library/Include/openssl/pkcs7.h | 427 +++
CryptoPkg/Library/Include/openssl/safestack.h | 297 ++
CryptoPkg/Library/Include/openssl/srp.h | 285 ++
CryptoPkg/Library/Include/openssl/ssl.h | 2585 +++++++++++++++++
CryptoPkg/Library/Include/openssl/ui.h | 407 +++
CryptoPkg/Library/Include/openssl/x509.h | 1276 ++++++++
CryptoPkg/Library/Include/openssl/x509_vfy.h | 894 ++++++
CryptoPkg/Library/Include/openssl/x509v3.h | 1450 +++++++++
CryptoPkg/Library/Include/prov/bio.h | 32 +
CryptoPkg/Library/Include/prov/blake2.h | 120 +
CryptoPkg/Library/Include/prov/ciphercommon.h | 361 +++
.../Library/Include/prov/ciphercommon_aead.h | 47 +
.../Library/Include/prov/ciphercommon_ccm.h | 100 +
.../Library/Include/prov/ciphercommon_gcm.h | 129 +
CryptoPkg/Library/Include/prov/der_digests.h | 160 +
CryptoPkg/Library/Include/prov/der_dsa.h | 94 +
CryptoPkg/Library/Include/prov/der_ec.h | 286 ++
CryptoPkg/Library/Include/prov/der_ecx.h | 50 +
CryptoPkg/Library/Include/prov/der_rsa.h | 187 ++
CryptoPkg/Library/Include/prov/der_sm2.h | 37 +
CryptoPkg/Library/Include/prov/der_wrap.h | 46 +
CryptoPkg/Library/Include/prov/digestcommon.h | 123 +
.../Library/Include/prov/implementations.h | 516 ++++
CryptoPkg/Library/Include/prov/kdfexchange.h | 24 +
CryptoPkg/Library/Include/prov/macsignature.h | 30 +
CryptoPkg/Library/Include/prov/md5_sha1.h | 36 +
CryptoPkg/Library/Include/prov/names.h | 327 +++
CryptoPkg/Library/Include/prov/proverr.h | 27 +
CryptoPkg/Library/Include/prov/provider_ctx.h | 40 +
.../Library/Include/prov/provider_util.h | 138 +
.../Library/Include/prov/providercommon.h | 24 +
.../Library/Include/prov/securitycheck.h | 30 +
CryptoPkg/Library/Include/prov/seeding.h | 41 +
CryptoPkg/Library/OpensslLib/buildinf.h | 2 +-
.../Library/BaseCryptLib/Hash/CryptSm3.c | 14 +-
.../Library/BaseCryptLib/SysCall/CrtWrapper.c | 10 +
.../OpensslLib/{buildinf.h => buildinf.c} | 3 +-
.../Library/OpensslLib/der_digests_gen.c | 160 +
CryptoPkg/Library/OpensslLib/der_rsa_gen.c | 174 ++
CryptoPkg/Library/OpensslLib/der_wrap_gen.c | 46 +
CryptoPkg/Library/OpensslLib/ossl_store.c | 11 +
CryptoPkg/Library/OpensslLib/rand_pool.c | 20 +-
CryptoPkg/Library/OpensslLib/openssl | 2 +-
CryptoPkg/Library/OpensslLib/process_files.pl | 79 +-
71 files changed, 20510 insertions(+), 1351 deletions(-)
create mode 100644 CryptoPkg/Library/Include/crypto/bn_conf.h
create mode 100644 CryptoPkg/Library/Include/fcntl.h
create mode 100644 CryptoPkg/Library/Include/openssl/asn1.h
create mode 100644 CryptoPkg/Library/Include/openssl/asn1t.h
create mode 100644 CryptoPkg/Library/Include/openssl/bio.h
create mode 100644 CryptoPkg/Library/Include/openssl/cmp.h
create mode 100644 CryptoPkg/Library/Include/openssl/cms.h
create mode 100644 CryptoPkg/Library/Include/openssl/conf.h
create mode 100644 CryptoPkg/Library/Include/openssl/configuration.h
create mode 100644 CryptoPkg/Library/Include/openssl/crmf.h
create mode 100644 CryptoPkg/Library/Include/openssl/crypto.h
create mode 100644 CryptoPkg/Library/Include/openssl/ct.h
create mode 100644 CryptoPkg/Library/Include/openssl/err.h
create mode 100644 CryptoPkg/Library/Include/openssl/ess.h
create mode 100644 CryptoPkg/Library/Include/openssl/fipskey.h
create mode 100644 CryptoPkg/Library/Include/openssl/lhash.h
create mode 100644 CryptoPkg/Library/Include/openssl/ocsp.h
delete mode 100644 CryptoPkg/Library/Include/openssl/opensslconf.h
create mode 100644 CryptoPkg/Library/Include/openssl/opensslv.h
create mode 100644 CryptoPkg/Library/Include/openssl/pkcs12.h
create mode 100644 CryptoPkg/Library/Include/openssl/pkcs7.h
create mode 100644 CryptoPkg/Library/Include/openssl/safestack.h
create mode 100644 CryptoPkg/Library/Include/openssl/srp.h
create mode 100644 CryptoPkg/Library/Include/openssl/ssl.h
create mode 100644 CryptoPkg/Library/Include/openssl/ui.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509_vfy.h
create mode 100644 CryptoPkg/Library/Include/openssl/x509v3.h
create mode 100644 CryptoPkg/Library/Include/prov/bio.h
create mode 100644 CryptoPkg/Library/Include/prov/blake2.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_aead.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_ccm.h
create mode 100644 CryptoPkg/Library/Include/prov/ciphercommon_gcm.h
create mode 100644 CryptoPkg/Library/Include/prov/der_digests.h
create mode 100644 CryptoPkg/Library/Include/prov/der_dsa.h
create mode 100644 CryptoPkg/Library/Include/prov/der_ec.h
create mode 100644 CryptoPkg/Library/Include/prov/der_ecx.h
create mode 100644 CryptoPkg/Library/Include/prov/der_rsa.h
create mode 100644 CryptoPkg/Library/Include/prov/der_sm2.h
create mode 100644 CryptoPkg/Library/Include/prov/der_wrap.h
create mode 100644 CryptoPkg/Library/Include/prov/digestcommon.h
create mode 100644 CryptoPkg/Library/Include/prov/implementations.h
create mode 100644 CryptoPkg/Library/Include/prov/kdfexchange.h
create mode 100644 CryptoPkg/Library/Include/prov/macsignature.h
create mode 100644 CryptoPkg/Library/Include/prov/md5_sha1.h
create mode 100644 CryptoPkg/Library/Include/prov/names.h
create mode 100644 CryptoPkg/Library/Include/prov/proverr.h
create mode 100644 CryptoPkg/Library/Include/prov/provider_ctx.h
create mode 100644 CryptoPkg/Library/Include/prov/provider_util.h
create mode 100644 CryptoPkg/Library/Include/prov/providercommon.h
create mode 100644 CryptoPkg/Library/Include/prov/securitycheck.h
create mode 100644 CryptoPkg/Library/Include/prov/seeding.h
copy CryptoPkg/Library/OpensslLib/{buildinf.h => buildinf.c} (50%)
create mode 100644 CryptoPkg/Library/OpensslLib/der_digests_gen.c
create mode 100644 CryptoPkg/Library/OpensslLib/der_rsa_gen.c
create mode 100644 CryptoPkg/Library/OpensslLib/der_wrap_gen.c

--
2.33.1





Gerd Hoffmann
 

On Fri, Dec 03, 2021 at 04:32:48PM +0000, Michael D Kinney wrote:
Hi Gerd,

Thank you for starting this work!

Can you point the community as a summary of the changes/improvements in v3.0 and your
take on why it is important to upgrade TianoCore.
From the openssl website:

<quote>
The latest stable version is the 3.0 series. Also available is the
1.1.1 series which is our Long Term Support (LTS) version, supported
until 11th September 2023. All older versions (including 1.1.0, 1.0.2,
1.0.0 and 0.9.8) are now out of support and should not be used.
</quote>

So, long-term there is simply no way around upgrading. Version 1.1.1
will go out of support in less than two years, after that date we
wouldn't get security fixes any more.

I think it makes sense to start the porting effort early and not wait
until the last minute with the switch to 3.x

take care,
Gerd


Gerd Hoffmann
 

Hi,

I've continued working on this over the last weeks. Time for a status
update. All applies to the latest tree, sneak preview is here:
https://github.com/kraxel/edk2/commits/openssl3

Also, assuming you have done enough test, would you please provide:
1) size difference, Including PEI, SMM, DXE.
No changes in SEC and PEI. DXE:

openssl 1.1
- 399582 SecureBootConfigDxe
- 472182 SecurityStubDxe
- 532626 VariableSmm
- 656382 TlsDxe

openssl 3.0
+ 809886 SecureBootConfigDxe
+ 912310 SecurityStubDxe
+ 970898 VariableSmm
+ 1125758 TlsDxe

Most of that seems to come from some openssl core changes (the new
'provider' concept) and I don't see an easy way to cut that down.

That is with the same feature set we have right now (i.e. no elliptic
curves and thus no TLS 1.3 support).

2) performance difference, Including PEI, SMM, DXE.
Suggestions how to measure that?

3) what unit test you have done (such as each crypto API)
CryptoPkg/UnitTest passes.

4) what system test you have done (such as secure boot, trusted boot)
Secure boot works.
TlsDxe (boot from https server) works.
TPM not tested yet.


I still have a bunch of failures in CI, for some of them I'm not sure
how to handle them best:

(1) 32-bit builds on windows fail:

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?


(2) va_arg is not working with floats due to SEE being disabled:

INFO - /home/vsts/work/1/s/CryptoPkg/Library/OpensslLib/openssl/crypto/bio/bio_print.c:265:28: error: SSE register argument with SSE disabled
INFO - fvalue = va_arg(args, LDOUBLE);

I can't see a way to fix that given that va_arg typically refers to a
compiler builtin so I don't think there is a way to declare that a
EFIAPI function to change the calling convention. Not all builds fail
though, possibly because the compiler inlines with optimization turned
on.

Suggestions anyone?


(3) Some NOOPT builds are failing due to the size growing ...


take care,
Gerd


Yao, Jiewen
 

Thank you!
Good result. Comment below:

-----Original Message-----
From: kraxel@redhat.com <kraxel@redhat.com>
Sent: Monday, January 17, 2022 7:46 PM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
Cc: 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

Hi,

I've continued working on this over the last weeks. Time for a status
update. All applies to the latest tree, sneak preview is here:
https://github.com/kraxel/edk2/commits/openssl3

Also, assuming you have done enough test, would you please provide:
1) size difference, Including PEI, SMM, DXE.
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.

DXE:

openssl 1.1
- 399582 SecureBootConfigDxe
- 472182 SecurityStubDxe
- 532626 VariableSmm
- 656382 TlsDxe

openssl 3.0
+ 809886 SecureBootConfigDxe
+ 912310 SecurityStubDxe
+ 970898 VariableSmm
+ 1125758 TlsDxe

Most of that seems to come from some openssl core changes (the new
'provider' concept) and I don't see an easy way to cut that down.

That is with the same feature set we have right now (i.e. no elliptic
curves and thus no TLS 1.3 support).
[Jiewen] It almost doubles the size, which will becomes a big challenge for openssl3.0 adoption.



2) performance difference, Including PEI, SMM, DXE.
Suggestions how to measure that?
[Jiewen] Please just write an app to call the crypto API, multiple times.
https://github.com/tianocore/edk2/tree/master/CryptoPkg/Test/UnitTest/Library/BaseCryptLib
I think we can focus on SHA256/RSA2048 + AES, which is used in secure boot, and HTTPS boot.


3) what unit test you have done (such as each crypto API)
CryptoPkg/UnitTest passes.
[Jiewen] Good enough.


4) what system test you have done (such as secure boot, trusted boot)
Secure boot works.
TlsDxe (boot from https server) works.
TPM not tested yet.
[Jiewen] Good enough. TPM only includes HASH. I am not too worry about that.




I still have a bunch of failures in CI, for some of them I'm not sure
how to handle them best:

(1) 32-bit builds on windows fail:

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



(2) va_arg is not working with floats due to SEE being disabled:

INFO -
/home/vsts/work/1/s/CryptoPkg/Library/OpensslLib/openssl/crypto/bio/bio_pri
nt.c:265:28: error: SSE register argument with SSE disabled
INFO - fvalue = va_arg(args, LDOUBLE);

I can't see a way to fix that given that va_arg typically refers to a
compiler builtin so I don't think there is a way to declare that a
EFIAPI function to change the calling convention. Not all builds fail
though, possibly because the compiler inlines with optimization turned
on.

Suggestions anyone?
[Jiewen] This seems infrastructure issue.
Any suggestion, Mike ?




(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?




take care,
Gerd


Michael D Kinney
 

Gerd,

Thank you for the continued work on v3.0 support. Comments below.

Mike

-----Original Message-----
From: Yao, Jiewen <jiewen.yao@intel.com>
Sent: Tuesday, January 18, 2022 3:12 AM
To: kraxel@redhat.com; devel@edk2.groups.io
Cc: 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

Thank you!
Good result. Comment below:

-----Original Message-----
From: kraxel@redhat.com <kraxel@redhat.com>
Sent: Monday, January 17, 2022 7:46 PM
To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>
Cc: 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

Hi,

I've continued working on this over the last weeks. Time for a status
update. All applies to the latest tree, sneak preview is here:
https://github.com/kraxel/edk2/commits/openssl3

Also, assuming you have done enough test, would you please provide:
1) size difference, Including PEI, SMM, DXE.
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.

DXE:

openssl 1.1
- 399582 SecureBootConfigDxe
- 472182 SecurityStubDxe
- 532626 VariableSmm
- 656382 TlsDxe

openssl 3.0
+ 809886 SecureBootConfigDxe
+ 912310 SecurityStubDxe
+ 970898 VariableSmm
+ 1125758 TlsDxe

Most of that seems to come from some openssl core changes (the new
'provider' concept) and I don't see an easy way to cut that down.

That is with the same feature set we have right now (i.e. no elliptic
curves and thus no TLS 1.3 support).
[Jiewen] It almost doubles the size, which will becomes a big challenge for openssl3.0 adoption.



2) performance difference, Including PEI, SMM, DXE.
Suggestions how to measure that?
[Jiewen] Please just write an app to call the crypto API, multiple times.
https://github.com/tianocore/edk2/tree/master/CryptoPkg/Test/UnitTest/Library/BaseCryptLib
I think we can focus on SHA256/RSA2048 + AES, which is used in secure boot, and HTTPS boot.


3) what unit test you have done (such as each crypto API)
CryptoPkg/UnitTest passes.
[Jiewen] Good enough.


4) what system test you have done (such as secure boot, trusted boot)
Secure boot works.
TlsDxe (boot from https server) works.
TPM not tested yet.
[Jiewen] Good enough. TPM only includes HASH. I am not too worry about that.




I still have a bunch of failures in CI, for some of them I'm not sure
how to handle them best:

(1) 32-bit builds on windows fail:

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
We need to see if there are any OpenSSL config settings to completely remove use of
float/double types. UEFI envs do not support float/double. It is possible to
use them in a UEFI App or other UEFI FW components, but the use of those need
to do extra work to disable interrupts and save/restore state.


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



(2) va_arg is not working with floats due to SEE being disabled:

INFO -
/home/vsts/work/1/s/CryptoPkg/Library/OpensslLib/openssl/crypto/bio/bio_pri
nt.c:265:28: error: SSE register argument with SSE disabled
INFO - fvalue = va_arg(args, LDOUBLE);

I can't see a way to fix that given that va_arg typically refers to a
compiler builtin so I don't think there is a way to declare that a
EFIAPI function to change the calling convention. Not all builds fail
though, possibly because the compiler inlines with optimization turned
on.

Suggestions anyone?
[Jiewen] This seems infrastructure issue.
Any suggestion, Mike ?
As mentioned above, it would be better if OpenSSL had a config setting to
not use any float/double types.





(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?




take care,
Gerd


Gerd Hoffmann
 

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?

(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


Gerd Hoffmann
 

Hi,

I still have a bunch of failures in CI, for some of them I'm not sure
how to handle them best:

(1) 32-bit builds on windows fail:

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
We need to see if there are any OpenSSL config settings to completely remove use of
float/double types.
https://github.com/openssl/openssl/pull/17547

That'll solve __ftol2_sse, but the other ones are 64bit integer math ...

take care,
Gerd


Michael D Kinney
 

The 64-bit integer math intrinsics need to be addressed in the intrinsic lib for the CryptoPkg.

I would expect the X64 build to not require those and they should only be observed for the IA32 builds.

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Gerd Hoffmann
Sent: Friday, January 21, 2022 12:34 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Yao, Jiewen <jiewen.yao@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

Hi,

I still have a bunch of failures in CI, for some of them I'm not sure
how to handle them best:

(1) 32-bit builds on windows fail:

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
We need to see if there are any OpenSSL config settings to completely remove use of
float/double types.
https://github.com/openssl/openssl/pull/17547

That'll solve __ftol2_sse, but the other ones are 64bit integer math ...

take care,
Gerd





Michael D Kinney
 

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


Yao, Jiewen
 

-----Original Message-----
From: kraxel@redhat.com <kraxel@redhat.com>
Sent: Friday, January 21, 2022 4:31 PM
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/R
ecoveryModuleLoadPei linking
https://github.com/tianocore/edk2/tree/master/SecurityPkg/Library/FmpAuthe
nticationLibRsa2048Sha256.

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?
[Jiewen] You can just build the corresponding pkg, such as SecurityPkg, SignedCapsulePkg.




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?

(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


Kilian Kegel
 

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

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





 


Michael D Kinney
 

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 <kilian_kegel@...>
Sent: Monday, January 24, 2022 8:25 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; kraxel@...; Yao, Jiewen <jiewen.yao@...>; Sean Brogan <sean.brogan@...>; Bret Barkelew <Bret.Barkelew@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>; Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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<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

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



 


Pedro Falcato
 

Mike,

Just my two cents:

I think adding intrinsic libraries is a mixed bag, for the following reasons:

1) The intrinsic libraries are completely internal to the compilers. Breaking down that toolchain/code barrier is not a good idea if we want the project to compile using a good variety of compilers. The API is unstable (as it's internal to the compiler + version) and sometimes undocumented; while in reality the ABI doesn't really change (at least in the LLVM/GCC world, not sure about the other compilers), it's something to consider.

2) Linking the compiler's builtin libraries would fix our issues, except that it doesn't work in cases where object files are tagged with ABI (such as hard FP vs soft FP).

On the other hand, the latest libgcc_s.so symbol I can find was introduced in GCC 7.0 (2017) and is completely unrelated to any simple 64-bit integer math we may want to do. So, in our very simple integer-mathy case, it may end up being a solid option.
LLVM also has a testsuite for its intrinsics, which is handy :)

It's worth noting that (at least) LLVM/GCC expect standard freestanding <string.h> functions like memcpy, strlen, ... to be available; I think it would be a good idea to supply these and drop "-fno-builtin" so that the compiler can do its job and optimize code further.

Also, if we're going to add runtime library code, UBSan and Asan might be worth a look ;)

Best regards,
Pedro


On Mon, Jan 24, 2022 at 5:29 PM Michael D Kinney <michael.d.kinney@...> wrote:

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 <kilian_kegel@...>
Sent: Monday, January 24, 2022 8:25 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; kraxel@...; Yao, Jiewen <jiewen.yao@...>; Sean Brogan <sean.brogan@...>; Bret Barkelew <Bret.Barkelew@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>; Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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<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

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



 



--
Pedro Falcato


Kilian Kegel
 

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 …

 

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@...; 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 <kilian_kegel@...>
Sent: Monday, January 24, 2022 8:25 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; kraxel@...; Yao, Jiewen <jiewen.yao@...>; Sean Brogan <sean.brogan@...>; Bret Barkelew <Bret.Barkelew@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>; Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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<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

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



 

 


Gerd Hoffmann
 

Hi,

I think adding intrinsic libraries is a mixed bag, for the following
reasons:

1) The intrinsic libraries are completely internal to the compilers.
Breaking down that toolchain/code barrier is not a good idea if we want the
project to compile using a good variety of compilers. The API is unstable
(as it's internal to the compiler + version) and sometimes undocumented;
while in reality the ABI doesn't really change (at least in the LLVM/GCC
world, not sure about the other compilers), it's something to consider.
Yes. But apparently there is no way around them. We have them for arm.
We have them for openssl. IntelUndiPkg in edk2-staging has some too
(see IntelUndiPkg/LibC). And I wouldn't be surprised if there are more
cases ...

Having a policy to outlaw Intrinsics, but then hand out exceptions left
and right doesn't look like a good idea to me. I think we should revisit
that and accept that there simply is no way around Intrinsics in some
cases.

I think it makes sense to consolidate all the Intrinsics we have, i.e.
move them over to MdePkg, make everybody use that, so we have only a
single version to maintain.

I think it also makes sense to restrict Intrinsics to the cases where we
have no other option, to keep them as small as possible and also make it
as easy as possible to maintain them.

2) Linking the compiler's builtin libraries would fix our issues, except
that it doesn't work in cases where object files are tagged with ABI (such
as hard FP vs soft FP).
Also:

* On my system the gcc intrinsics are only available as shared library,
so the "just unpack the lib and use the object files" idea is not
going to work.

* I have my doubts that compiler's builtin libraries are optimized for
size, so I'd suspect we would see a noticeable size grow from that.

* I'd very much prefer to continue with the current approach to have
source code for the Intrinsics we need. In case we run into trouble
things tend to be much easier to fix when you have the source code at
hand. That's actually part of the open source success story.

take care,
Gerd


Kilian Kegel
 

Hi Gerd,

 

>* On my system the gcc intrinsics are only available as shared library,
>   so the "just unpack the lib and use the object files" idea is not
>   going to work.

 

This little C program makes an unsigned 64Bit division on PC compilers.

Running a 32Bit code generator, it usually invokes an intrinsic function.

  • MSFT: __aulldiv()
  • GCC: __udivdi3()

 

On my 32Bit Ubuntu standard installation I ran

  1. cc - Xlinker -Map=static.map hello.c -static
  2. cc  -Xlinker -Map=shared.map hello.c

 

The first .OBJ file mentioned in the .MAP file is in both cases:

/usr/lib/gcc/i686-linux-gnu/6/libgcc.a(_udivdi3.o)

 

 

Then for each a.out I did:

  • objdump -d a.out > static.dis
  • objdump -d a.out > shared.dis

 

In both cases the intrinsic function is fully linked into the .ELF executable.

 

>so the "just unpack the lib and use the object files" idea is not
>going to work.

It seems to me that GNU holds the intrinsic functions in a separate library

that can be used without any change, and is always correct by definition.

 

For Microsoft that is only true when a SDK is installed (INT64.LIB).

Without SDK the intrinsic functions were included in LIBCMT.LIB and

must be isolated manually.

 

Gerd, can you please doublecheck in your GCC build, if that works:

  1. add a 64Bit div to an x86 PEI module like:

 

  1. add libgcc.a as a search library, adjust the conf\tools_def.txt like:

DEBUG_GCCxx_IA32_DLINK_FLAGS   = …predefined parameter … /usr/lib/gcc/i686-linux-gnu/6/libgcc.a

to match your build system

  1. build the BIOS
  2. if the build gets ready, check the .MAP file whether it contains  __udivdi3() or not

 

>* I have my doubts that compiler's builtin libraries are optimized for
>   size, so I'd suspect we would see a noticeable size grow from that.

Please check the size of __udivdi3() and whether the tianocore reimplementation is smaller or not

 

If this works for all build platforms, independently of using the tianocore reimplementation or

using the original compiler intrinsics, this is correct location to place the address of the intrinsic library.

For all optimization modes, operation modes (64Bit/32Bit) the intrinsic library is available for the compiler.

 

GNU lists the intrinsics:

https://gcc.gnu.org/onlinedocs/gccint/Libgcc.html#Libgcc

 

The intrinsic library belongs to the compiler not to the build system.

If the build system changes from EDK2 to VS2022/MSBUILD, the compiler

expects the same intrinsic library.

 

Leave the intrinsic restrictions behind and just provide all required intrinsics the compiler needs

to fulfil the C-Standard!

 

Thanks a lot,

Kilian

https://github.com/tianocore/edk2-staging/tree/CdePkg#cdepkgblog

 

From: kraxel@...
Sent: Wednesday, January 26, 2022 12:02 PM
To: Pedro Falcato
Cc: edk2-devel-groups-io; Kinney, Michael D; Kilian Kegel; Yao, Jiewen; Sean Brogan; Bret Barkelew; 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,

> I think adding intrinsic libraries is a mixed bag, for the following
> reasons:
>
> 1) The intrinsic libraries are completely internal to the compilers.
> Breaking down that toolchain/code barrier is not a good idea if we want the
> project to compile using a good variety of compilers. The API is unstable
> (as it's internal to the compiler + version) and sometimes undocumented;
> while in reality the ABI doesn't really change (at least in the LLVM/GCC
> world, not sure about the other compilers), it's something to consider.

Yes.  But apparently there is no way around them.  We have them for arm.
We have them for openssl.  IntelUndiPkg in edk2-staging has some too
(see IntelUndiPkg/LibC).  And I wouldn't be surprised if there are more
cases ...

Having a policy to outlaw Intrinsics, but then hand out exceptions left
and right doesn't look like a good idea to me.  I think we should revisit
that and accept that there simply is no way around Intrinsics in some
cases.

I think it makes sense to consolidate all the Intrinsics we have, i.e.
move them over to MdePkg, make everybody use that, so we have only a
single version to maintain.

I think it also makes sense to restrict Intrinsics to the cases where we
have no other option, to keep them as small as possible and also make it
as easy as possible to maintain them.

> 2) Linking the compiler's builtin libraries would fix our issues, except
> that it doesn't work in cases where object files are tagged with ABI (such
> as hard FP vs soft FP).

Also:

 * On my system the gcc intrinsics are only available as shared library,
   so the "just unpack the lib and use the object files" idea is not
   going to work.

 * I have my doubts that compiler's builtin libraries are optimized for
   size, so I'd suspect we would see a noticeable size grow from that.

 * I'd very much prefer to continue with the current approach to have
   source code for the Intrinsics we need.  In case we run into trouble
   things tend to be much easier to fix when you have the source code at
   hand.  That's actually part of the open source success story.

take care,
  Gerd

 

Sent from Mail for Windows

 

From: Kilian Kegel
Sent: Tuesday, January 25, 2022 09:06 PM
To: Kinney, Michael D; devel@edk2.groups.io; kraxel@...; 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

 

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 …

 

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@...; 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 <kilian_kegel@...>
Sent: Monday, January 24, 2022 8:25 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; kraxel@...; Yao, Jiewen <jiewen.yao@...>; Sean Brogan <sean.brogan@...>; Bret Barkelew <Bret.Barkelew@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>; Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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<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

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


 

 

 


Andrew Fish
 

Very cool idea but …..

1) We don’t always use the systems native compiler and sometimes we use a cross compiler so making assumptions about system libs is not always valid.

On a Mac with Xcode clang I’ve got full SysV ABI libs (not supper helpful for EFI), but not EFI/MSFT x86_64 ABI. For bonus point i386 has been obsoleted in Xcode.

~/work/Compiler/Math>cat hello.c                                
#include <stdio.h>

int main(int argc, char **argv)
{
  unsigned long long ulldiv = ((unsigned long long) argc)/3;

  // prevent the optimizer to calculate result at build time
  printf("ulldiv %lld\n", ulldiv );
}
~/work/Compiler/Math>clang hello.c
Thus I can link Sys V ABI with the wrong calling convention for EFI.

But for the EFI ABIs not so much….
~/work/Compiler/Math>clang -target x86_64-pc-win32-macho hello.c
clang: warning: unable to find a Visual Studio installation; try running Clang from a developer command prompt [-Wmsvc-not-found]
hello.c:1:10: fatal error: 'stdio.h' file not found
#include <stdio.h>
         ^~~~~~~~~
1 error generated.
~/work/Compiler/Math>clang -arch i386 hello.c                   
ld: warning: The i386 architecture is deprecated for macOS (remove from the Xcode build setting: ARCHS)
ld: warning: ignoring file /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk/usr/lib/libSystem.tbd, missing required architecture i386 in file /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.0.sdk/usr/lib/libSystem.tbd (3 slices)
Undefined symbols for architecture i386:
  "_printf", referenced from:
      _main in hello-380c0a.o
ld: symbol(s) not found for architecture i386
clang: error: linker command failed with exit code 1 (use -v to see invocation)


2) Are these system libs architecturally defined to be free standing? Can they make assumptions about the runtime? If so seems like they could do random bad things to make them not work in EFI like a system call or make other assumptions like writing to a magic address to generate a trap. Just inspecting them tells us the implementation, not the architecture of the lib.

3) Are the paths to these libs architectural or are they arbitrary implementation that is normal abstracted by how the compiler is released? Could some packaging change break the magic paths to libs?

4) I asked the Xcode/clang team a long time ago what to do for free standing match and they pointed me at some open source implementation of these math libs that had been implemented in C. They did not want us using their libs that shipped with macOS. 


For the GCC/clang tools seems like we are better off just providing the code. 

We have magic for compiler specific inline assembly 

We have magic to abstract some compiler intrinsics today:

While we try to avoid it when at all possible the build system does support doing things differently for different compilers if we have to
[Sources.IA32]
IoLibGcc.c | GCC
IoLibMsc.c | MSFT
IoLib.c


Thanks,

Andrew Fish

PS The compiler still works like you think we just don’t have the libs you might need. 

~/work/Compiler/Math>cat i386.c             
typedef unsigned long long UINT64;

UINT64
main2(int argc, char **argv)
{
  UINT64 ulldiv = ((UINT64) argc)/3;

  // prevent the optimizer to calculate result at build time
  return  ulldiv;
}
~/work/Compiler/Math>clang -arch i386 i386.c -S
~/work/Compiler/Math>cat i386.S                
.section __TEXT,__text,regular,pure_instructions
.build_version macos, 12, 0 sdk_version 12, 0
.globl _main2                          ## -- Begin function main2
.p2align 4, 0x90
_main2:                                 ## @main2
.cfi_startproc
## %bb.0:
pushl %ebp
.cfi_def_cfa_offset 8
.cfi_offset %ebp, -8
movl %esp, %ebp
.cfi_def_cfa_register %ebp
subl $24, %esp
movl 12(%ebp), %eax
movl 8(%ebp), %eax
movl 8(%ebp), %ecx
movl %ecx, %edx
sarl $31, %edx
movl %esp, %eax
movl %edx, 4(%eax)
movl %ecx, (%eax)
movl $0, 12(%eax)
movl $3, 8(%eax)
calll ___udivdi3
movl %edx, -4(%ebp)
movl %eax, -8(%ebp)
movl -8(%ebp), %eax
movl -4(%ebp), %edx
addl $24, %esp
popl %ebp
retl
.cfi_endproc
                                        ## -- End function
.subsections_via_symbols

PPS Fun story about ABI differences as the macOS i386 ABI requires 16-byte aligned stack accesses and that is more strict than EFI. Luckily it does not break EFI, but it means you can NOT call macOS code from EFI code. To make the emulator work on macOS I had to write assembly gaskets to align the stacks to make calls between the worlds possible. Not my finest hour but it works….

This crazy is an example about how assumptions that are not EFI centric can leak into the generic libs produced for the compiler.

On Jan 27, 2022, at 2:26 PM, Kilian Kegel <KILIAN_KEGEL@...> wrote:

Hi Gerd,
 
>* On my system the gcc intrinsics are only available as shared library,
>   so the "just unpack the lib and use the object files" idea is not
>   going to work.
<D175366159184B98AC9B192EC485505B.png>
 
This little C program makes an unsigned 64Bit division on PC compilers.
Running a 32Bit code generator, it usually invokes an intrinsic function.
  • MSFT: __aulldiv()
  • GCC: __udivdi3()
 
On my 32Bit Ubuntu standard installation I ran
  1. cc - Xlinker -Map=static.map hello.c -static
  2. cc  -Xlinker -Map=shared.map hello.c
 
The first .OBJ file mentioned in the .MAP file is in both cases:
/usr/lib/gcc/i686-linux-gnu/6/libgcc.a(_udivdi3.o)
<DC03DFFFFFF14308B3A615804A1BF474.png>
 
<377AC53F424C47F794809BA1A5953904.png>
 
Then for each a.out I did:
  • objdump -d a.out > static.dis
  • objdump -d a.out > shared.dis
 
In both cases the intrinsic function is fully linked into the .ELF executable.
<B5A273DA2C3A4898BAEB0A354C667FE5.png>
 
>so the "just unpack the lib and use the object files" idea is not
>going to work.
It seems to me that GNU holds the intrinsic functions in a separate library
that can be used without any change, and is always correct by definition.
 
For Microsoft that is only true when a SDK is installed (INT64.LIB).
Without SDK the intrinsic functions were included in LIBCMT.LIB and
must be isolated manually.
 
Gerd, can you please doublecheck in your GCC build, if that works:
  1. add a 64Bit div to an x86 PEI module like:
<500D8F2283CD4FAE9B0E45647823894A.png>
 
  1. add libgcc.a as a search library, adjust the conf\tools_def.txt like:
DEBUG_GCCxx_IA32_DLINK_FLAGS   = …predefined parameter … /usr/lib/gcc/i686-linux-gnu/6/libgcc.a
to match your build system
  1. build the BIOS 
  2. if the build gets ready, check the .MAP file whether it contains  __udivdi3() or not
 
>* I have my doubts that compiler's builtin libraries are optimized for
>   size, so I'd suspect we would see a noticeable size grow from that.
Please check the size of __udivdi3() and whether the tianocore reimplementation is smaller or not
 
If this works for all build platforms, independently of using the tianocore reimplementation or
using the original compiler intrinsics, this is correct location to place the address of the intrinsic library.
For all optimization modes, operation modes (64Bit/32Bit) the intrinsic library is available for the compiler.
 
GNU lists the intrinsics:
 
The intrinsic library belongs to the compiler not to the build system.
If the build system changes from EDK2 to VS2022/MSBUILD, the compiler
expects the same intrinsic library.
 
Leave the intrinsic restrictions behind and just provide all required intrinsics the compiler needs
to fulfil the C-Standard!
 
Thanks a lot,
Kilian
 
From: kraxel@...
Sent: Wednesday, January 26, 2022 12:02 PM
To: Pedro Falcato
Cc: edk2-devel-groups-io; Kinney, Michael D; Kilian Kegel; Yao, Jiewen; Sean Brogan; Bret Barkelew; 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,

> I think adding intrinsic libraries is a mixed bag, for the following
> reasons:
> 
> 1) The intrinsic libraries are completely internal to the compilers.
> Breaking down that toolchain/code barrier is not a good idea if we want the
> project to compile using a good variety of compilers. The API is unstable
> (as it's internal to the compiler + version) and sometimes undocumented;
> while in reality the ABI doesn't really change (at least in the LLVM/GCC
> world, not sure about the other compilers), it's something to consider.

Yes.  But apparently there is no way around them.  We have them for arm.
We have them for openssl.  IntelUndiPkg in edk2-staging has some too
(see IntelUndiPkg/LibC).  And I wouldn't be surprised if there are more
cases ...

Having a policy to outlaw Intrinsics, but then hand out exceptions left
and right doesn't look like a good idea to me.  I think we should revisit
that and accept that there simply is no way around Intrinsics in some
cases.

I think it makes sense to consolidate all the Intrinsics we have, i.e.
move them over to MdePkg, make everybody use that, so we have only a
single version to maintain.

I think it also makes sense to restrict Intrinsics to the cases where we
have no other option, to keep them as small as possible and also make it
as easy as possible to maintain them.

> 2) Linking the compiler's builtin libraries would fix our issues, except
> that it doesn't work in cases where object files are tagged with ABI (such
> as hard FP vs soft FP).

Also:

 * On my system the gcc intrinsics are only available as shared library,
   so the "just unpack the lib and use the object files" idea is not
   going to work.

 * I have my doubts that compiler's builtin libraries are optimized for
   size, so I'd suspect we would see a noticeable size grow from that.

 * I'd very much prefer to continue with the current approach to have
   source code for the Intrinsics we need.  In case we run into trouble
   things tend to be much easier to fix when you have the source code at
   hand.  That's actually part of the open source success story.

take care,
  Gerd

 
Sent from Mail for Windows
 
From: Kilian Kegel
Sent: Tuesday, January 25, 2022 09:06 PM
To: Kinney, Michael D; devel@edk2.groups.io; kraxel@...; 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
 
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 …
<A1B2261BD5014BBF968AF0AA25EF9349.png>
 
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@...; 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 <kilian_kegel@...> 
Sent: Monday, January 24, 2022 8:25 AM
To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; kraxel@...; Yao, Jiewen <jiewen.yao@...>; Sean Brogan <sean.brogan@...>; Bret Barkelew <Bret.Barkelew@...>
Cc: devel@edk2.groups.io; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin <guomin.jiang@...>; Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
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<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
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.)”
 
This mindset violates edk2 coding style spec too: 
  • 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@...; 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@... <kraxel@...>
> Sent: Friday, January 21, 2022 12:31 AM
> To: Yao, Jiewen <jiewen.yao@...>
> Cc: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@...>; Wang, Jian J <jian.j.wang@...>; Jiang, Guomin
> <guomin.jiang@...>; Pawel Polawski <ppolawsk@...>; Lu, XiaoyuX <xiaoyux.lu@...>
> 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


 
 

 
<shared.dis><shared.map><static.dis><static.map>