public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
	"devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Kinney, Michael D" <michael.d.kinney@intel.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
	"Lu, Xiaoyu1" <xiaoyu1.lu@intel.com>,
	"Jiang, Guomin" <guomin.jiang@intel.com>,
	"Zurcher, Christopher" <christopher.zurcher@microsoft.com>,
	Rebecca Cran <quic_rcran@quicinc.com>,
	Ard Biesheuvel <ardb@kernel.org>, "Li, Yi1" <yi1.li@intel.com>
Subject: Re: [edk2-devel] [Patch v2 00/16] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
Date: Mon, 24 Oct 2022 05:43:43 +0000	[thread overview]
Message-ID: <CO1PR11MB4929BD051CD29C0D5B655CA6D22E9@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <MW4PR11MB587245BC2C56B018EEF1EFC68C2E9@MW4PR11MB5872.namprd11.prod.outlook.com>

Hi Jiewen,

I compared the builds of the CryptoPei, CryptoDxe, CryptoSmm using the
same OpensslLib instance with EC disabled.

I used an IA32 build of the MIN_PEI profile and an X64 build of the
MIN_DXE_MIN_SMM profile for the comparison.

Arch Profile          Driver        Old    Old-LZMA    New    New-LZMA
==== ===============  =========   =======  ========  =======  ========
IA32 MIN_PEI          CryptoPei   252,192   102,109  244,160    98,571
X64  MIN_DXE_MIN_SMM  CryptoDxe   812,288   313,340  811,008   312,913
X64  MIN_DXE_MIN_SMM  CryptoSmm   549,088   205,613  548,096   205,079


These results show that this patch series provide a very small reduction
in both the uncompressed and LZMA compressed sizes of these drivers.  This 
is the expected result because no functional changes were introduced.

The small decrease is likely due to the removal of the UnitTest overhead
that was included in all CryptoPkg.dsc build before this series.  This
series only links in the UnitTest related libs and hook of the ASSERT()
macros when building unit tests.

Best regards,

Mike

> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Sunday, October 23, 2022 8:54 PM
> To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>; Zurcher,
> Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>; Li,
> Yi1 <yi1.li@intel.com>
> Subject: RE: [edk2-devel] [Patch v2 00/16] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs
> 
> Sorry: I mean old configuration without ECC.
> 
> > -----Original Message-----
> > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao,
> > Jiewen
> > Sent: Monday, October 24, 2022 11:53 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, Xiaoyu1
> > <xiaoyu1.lu@intel.com>; Jiang, Guomin <guomin.jiang@intel.com>;
> > Zurcher, Christopher <christopher.zurcher@microsoft.com>; Rebecca Cran
> > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>; Li, Yi1
> > <yi1.li@intel.com>
> > Subject: Re: [edk2-devel] [Patch v2 00/16] CryptoPkg: Remove EC PCD and
> > merge perf opt OpensslLibs
> >
> > Thanks Mike
> > The update looks good to me. Reviewed-by: Jiewen Yao
> > <Jiewen.yao@intel.com>
> >
> > Do you have any data of size difference before and after? Please share with
> > us.
> >
> > I am more interested in the old configuration with ECC.
> >
> > Thank you
> > Yao, Jiewen
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D <michael.d.kinney@intel.com>
> > > Sent: Friday, October 21, 2022 2:35 AM
> > > To: devel@edk2.groups.io
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > > <jian.j.wang@intel.com>; Lu, Xiaoyu1 <xiaoyu1.lu@intel.com>; Jiang,
> > > Guomin <guomin.jiang@intel.com>; Zurcher, Christopher
> > > <christopher.zurcher@microsoft.com>; Rebecca Cran
> > > <quic_rcran@quicinc.com>; Ard Biesheuvel <ardb@kernel.org>; Li, Yi1
> > > <yi1.li@intel.com>
> > > Subject: [Patch v2 00/16] CryptoPkg: Remove EC PCD and merge perf opt
> > > OpensslLibs
> > >
> > > The recent addition of the Ecliptic Curve (EC) feature and the performance
> > > optimization feature increased the complexity for platforms to integrate
> > > and enable these features.  This series simplifies the platform
> > configuration
> > > as much as possible and improves the ability to manage the the size
> > impact
> > > of cryptographic services in each FW phase.  A Readme.md is also added
> > > that
> > > provides an overview of the CryptoPkg design and features along with
> > > platform
> > > integration recommendations.
> > >
> > > This series also addresses private library class declarations missing from
> > > CryptoPkg.dec and library instances not producing all the APIs defined
> > > by the library classes. A review of the CryptoPkg EDK II meta data files
> > > identified
> > > a number of additional cleanups. The CryptoPkg.dsc file was also updated
> > to
> > > improve CI coverage for future CryptoPkg changes and identified some
> > > unit test bug fixes.
> > >
> > > Change Summary
> > > ============
> > > * Document disabled/deprecated cryptographic services
> > > * Add missing UNI files in BaseCryptLib
> > > * Update BaseCryptLib internal functions to be STATIC and remove EFIAPI
> > > * Add GLOBAL_REMOVE_IF_UNREFERENCED to BaseCryptLib global
> > > variables
> > > * Fix BaseCryptLib unit tests
> > > * Cleanup BaseCryptLib and TlsLib INF files and
> > > * Move SysCall/inet_pton.c from BaseCryptLib to TlsLib that uses it.
> > > * Merge 4 performance optimized INFs into OpensslLib*Accel.inf
> > > * Remove use of PcdOpensslEcEnabled and use OpensslLibFull*.inf
> > instead
> > > * Add OpensslLib and IntrinsicLib to CryptoPkg.dec as private library
> > classes
> > > * Update all OpensslLib instances to always produce all APIs in OpensslLib
> > > class
> > > * Move PrintLib dependency from OpensslLib INF files to BaseCryptLib INF
> > > files
> > > * Update CryptoPkg.dsc files to provide full CI test coverage across all the
> > >    supported combinations of OpensslLib, BaseCryptLib, and TlsLib
> > instances.
> > > * Remove PACKAGE profile from CryptoPkg.dsc and add
> > > TARGET_UNIT_TESTS
> > >   profile.  Adding TARGET_UNIT_TESTS profile is required to prevent a few
> > > unit
> > >   test artifacts being included in non unit test builds of components.
> > > * Add CryptoPkg Readme.md with overview and platform integration
> > > details.
> > > * Update host-based unit tests to always use OpensslLibFull.inf and add
> > > unit
> > >   test coverage for OpensslLibFullAccel.inf.
> > > * Add Readme.md with CryptoPkg overview and platform integration
> > >   documentation
> > >
> > > New in V2
> > > =========
> > > * Fix service table in Readme.md
> > > * Fix VS2015x86 RELEASE builds by disabling warning 4718
> > > * Rebase to latest and add missing EC functions in EcSm2Null.c
> > > * Update perl scripts to auto-generate all OpensslLib INF files
> > > * Update OpensslLib INF files to match auto-generated format
> > >
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Jian J Wang <jian.j.wang@intel.com>
> > > Cc: Xiaoyu Lu <xiaoyu1.lu@intel.com>
> > > Cc: Guomin Jiang <guomin.jiang@intel.com>
> > > Cc: Christopher Zurcher <christopher.zurcher@microsoft.com>
> > > Cc: Rebecca Cran <quic_rcran@quicinc.com>
> > > Cc: Ard Biesheuvel <ardb@kernel.org>
> > > Cc: Yi Li <yi1.li@intel.com>
> > > Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
> > >
> > > Michael D Kinney (12):
> > >   CryptoPkg: Document and disable deprecated crypto services
> > >   CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format
> > >   CryptoPkg/Library/BaseCryptLib: Update internal functions/variables
> > >   CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes
> > >   CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib
> > >   CryptoPkg/Library/OpensslLib: Combine all performance optimized INFs
> > >   CryptoPkg/Library/OpensslLib: Produce consistent set of APIs
> > >   CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files
> > >   CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec
> > >   CryptoPkg: Update DSC to improve CI test coverage
> > >   CryptoPkg: Fixed host-based unit tests
> > >   CryptoPkg: Add Readme.md
> > >
> > > Yi Li (4):
> > >   Revert "CryptoPkg: Update process_files.pl to auto add PCD config
> > >     option"
> > >   CryptoPkg/Library/OpensslLib: Update process_files.pl INF generation
> > >   CryptoPkg/Library/OpensslLib: Add generated flag to Accel INF
> > >   CryptoPkg/Library/OpensslLib: update auto-generated files
> > >
> > >  CryptoPkg/CryptoPkg.ci.yaml                   |  11 +-
> > >  CryptoPkg/CryptoPkg.dec                       |  42 +-
> > >  CryptoPkg/CryptoPkg.dsc                       | 438 ++++++++---
> > >  .../Pcd/PcdCryptoServiceFamilyEnable.h        | 122 +--
> > >  .../Library/BaseCryptLib/BaseCryptLib.inf     |  10 +-
> > >  .../Library/BaseCryptLib/BaseCryptLib.uni     |   2 -
> > >  .../Library/BaseCryptLib/Hmac/CryptHmac.c     |   7 +
> > >  .../Library/BaseCryptLib/Kdf/CryptHkdf.c      |   5 +-
> > >  .../Library/BaseCryptLib/PeiCryptLib.inf      |   8 +-
> > >  .../Library/BaseCryptLib/PeiCryptLib.uni      |   2 -
> > >  CryptoPkg/Library/BaseCryptLib/Pem/CryptPem.c |   4 -
> > >  .../BaseCryptLib/Pk/CryptAuthenticode.c       |   2 +-
> > >  .../BaseCryptLib/Pk/CryptPkcs7VerifyCommon.c  |   3 +-
> > >  .../BaseCryptLib/Pk/CryptPkcs7VerifyEku.c     |   3 +
> > >  CryptoPkg/Library/BaseCryptLib/Pk/CryptTs.c   |  44 +-
> > >  CryptoPkg/Library/BaseCryptLib/Pk/CryptX509.c |   4 -
> > >  .../Library/BaseCryptLib/RuntimeCryptLib.inf  |   9 +-
> > >  .../Library/BaseCryptLib/RuntimeCryptLib.uni  |   2 -
> > >  .../Library/BaseCryptLib/SecCryptLib.inf      |  13 +-
> > >  .../{SmmCryptLib.uni => SecCryptLib.uni}      |  11 +-
> > >  .../Library/BaseCryptLib/SmmCryptLib.inf      |  12 -
> > >  .../Library/BaseCryptLib/SmmCryptLib.uni      |   2 -
> > >  .../BaseCryptLib/UnitTestHostBaseCryptLib.inf |  22 +-
> > >  .../Library/Include/openssl/opensslconf.h     | 328 +++++++-
> > >  .../Include/openssl/opensslconf_generated.h   | 333 ---------
> > >  CryptoPkg/Library/OpensslLib/EcSm2Null.c      | 383 ++++++++++
> > >  CryptoPkg/Library/OpensslLib/OpensslLib.inf   |  91 +--
> > >  CryptoPkg/Library/OpensslLib/OpensslLib.uni   |  10 +-
> > >  ...OpensslLibIa32.inf => OpensslLibAccel.inf} | 154 ++--
> > >  .../Library/OpensslLib/OpensslLibAccel.uni    |  14 +
> > >  .../OpensslLib/OpensslLibConstructor.c        |   6 +-
> > >  .../Library/OpensslLib/OpensslLibCrypto.inf   |  92 +--
> > >  .../Library/OpensslLib/OpensslLibCrypto.uni   |  11 +-
> > >  ...ensslLibIa32Gcc.inf => OpensslLibFull.inf} | 172 +++--
> > >  .../{OpensslLib.uni => OpensslLibFull.uni}    |  10 +-
> > >  ...lLibX64Gcc.inf => OpensslLibFullAccel.inf} | 212 ++++--
> > >  .../OpensslLib/OpensslLibFullAccel.uni        |  14 +
> > >  .../Library/OpensslLib/OpensslLibX64.inf      | 706 ------------------
> > >  CryptoPkg/Library/OpensslLib/SslNull.c        | 405 ++++++++++
> > >  CryptoPkg/Library/OpensslLib/process_files.pl | 168 +++--
> > >  .../SysCall/inet_pton.c                       |   0
> > >  CryptoPkg/Library/TlsLib/TlsConfig.c          |  12 +-
> > >  CryptoPkg/Library/TlsLib/TlsLib.inf           |  12 +-
> > >  CryptoPkg/Private/Library/IntrinsicLib.h      |  16 +
> > >  CryptoPkg/Private/Library/OpensslLib.h        |  14 +
> > >  CryptoPkg/Readme.md                           | 498 ++++++++++++
> > >  CryptoPkg/Test/CryptoPkgHostUnitTest.dsc      |  17 +-
> > >  .../UnitTest/Library/BaseCryptLib/HmacTests.c |  17 +-
> > >  .../UnitTest/Library/BaseCryptLib/TSTests.c   |   2 +-
> > >  .../TestBaseCryptLibHostAccel.inf             |  56 ++
> > >  50 files changed, 2711 insertions(+), 1820 deletions(-)
> > >  copy CryptoPkg/Library/BaseCryptLib/{SmmCryptLib.uni =>
> > > SecCryptLib.uni} (74%)
> > >  delete mode 100644
> > > CryptoPkg/Library/Include/openssl/opensslconf_generated.h
> > >  create mode 100644 CryptoPkg/Library/OpensslLib/EcSm2Null.c
> > >  rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32.inf =>
> > > OpensslLibAccel.inf} (79%)
> > >  create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibAccel.uni
> > >  rename CryptoPkg/Library/OpensslLib/{OpensslLibIa32Gcc.inf =>
> > > OpensslLibFull.inf} (79%)
> > >  copy CryptoPkg/Library/OpensslLib/{OpensslLib.uni => OpensslLibFull.uni}
> > > (56%)
> > >  rename CryptoPkg/Library/OpensslLib/{OpensslLibX64Gcc.inf =>
> > > OpensslLibFullAccel.inf} (78%)
> > >  create mode 100644
> > > CryptoPkg/Library/OpensslLib/OpensslLibFullAccel.uni
> > >  delete mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > >  create mode 100644 CryptoPkg/Library/OpensslLib/SslNull.c
> > >  rename CryptoPkg/Library/{BaseCryptLib => TlsLib}/SysCall/inet_pton.c
> > > (100%)
> > >  create mode 100644 CryptoPkg/Private/Library/IntrinsicLib.h
> > >  create mode 100644 CryptoPkg/Private/Library/OpensslLib.h
> > >  create mode 100644 CryptoPkg/Readme.md
> > >  create mode 100644
> > >
> > CryptoPkg/Test/UnitTest/Library/BaseCryptLib/TestBaseCryptLibHostAccel.i
> > > nf
> > >
> > > --
> > > 2.37.1.windows.1
> >
> >
> >
> > 
> >


  reply	other threads:[~2022-10-24  5:43 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-10-20 18:34 [Patch v2 00/16] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs Michael D Kinney
2022-10-20 18:34 ` [Patch v2 01/16] CryptoPkg: Document and disable deprecated crypto services Michael D Kinney
2022-10-20 18:34 ` [Patch v2 02/16] CryptoPkg/Library/BaseCryptLib: Add missing UNI file and fix format Michael D Kinney
2022-10-20 18:34 ` [Patch v2 03/16] CryptoPkg/Library/BaseCryptLib: Update internal functions/variables Michael D Kinney
2022-10-20 18:34 ` [Patch v2 04/16] CryptoPkg/Test/UnitTest/Library/BaseCryptLib: Unit test fixes Michael D Kinney
2022-10-20 18:34 ` [Patch v2 05/16] CryptoPkg/Library: Cleanup BaseCryptLib and TlsLib Michael D Kinney
2022-10-20 18:35 ` [Patch v2 06/16] CryptoPkg/Library/OpensslLib: Combine all performance optimized INFs Michael D Kinney
2022-10-20 18:35 ` [Patch v2 07/16] CryptoPkg/Library/OpensslLib: Produce consistent set of APIs Michael D Kinney
2022-10-20 18:35 ` [Patch v2 08/16] CryptoPkg/Library/OpensslLib: Remove PrintLib from INF files Michael D Kinney
2022-10-20 18:35 ` [Patch v2 09/16] CryptoPkg: Remove PcdOpensslEcEnabled from CryptoPkg.dec Michael D Kinney
2022-10-20 18:35 ` [Patch v2 10/16] CryptoPkg: Update DSC to improve CI test coverage Michael D Kinney
2022-10-20 18:35 ` [Patch v2 11/16] CryptoPkg: Fixed host-based unit tests Michael D Kinney
2022-10-20 18:35 ` [Patch v2 12/16] CryptoPkg: Add Readme.md Michael D Kinney
2022-10-20 18:35 ` [Patch v2 13/16] Revert "CryptoPkg: Update process_files.pl to auto add PCD config option" Michael D Kinney
2022-10-20 18:35 ` [Patch v2 14/16] CryptoPkg/Library/OpensslLib: Update process_files.pl INF generation Michael D Kinney
2022-10-20 18:35 ` [Patch v2 15/16] CryptoPkg/Library/OpensslLib: Add generated flag to Accel INF Michael D Kinney
2022-10-20 18:35 ` [Patch v2 16/16] CryptoPkg/Library/OpensslLib: update auto-generated files Michael D Kinney
2022-10-24  3:52 ` [Patch v2 00/16] CryptoPkg: Remove EC PCD and merge perf opt OpensslLibs Yao, Jiewen
     [not found] ` <1720E4F0EDFC384F.808@groups.io>
2022-10-24  3:54   ` [edk2-devel] " Yao, Jiewen
2022-10-24  5:43     ` Michael D Kinney [this message]
2022-10-24  6:24       ` Yao, Jiewen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CO1PR11MB4929BD051CD29C0D5B655CA6D22E9@CO1PR11MB4929.namprd11.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox