From: "Zurcher, Christopher J" <christopher.j.zurcher@intel.com>
To: "Yao, Jiewen" <jiewen.yao@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
"Lu, XiaoyuX" <xiaoyux.lu@intel.com>,
Ard Biesheuvel <ard.biesheuvel@arm.com>
Subject: Re: [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction support for X64
Date: Tue, 20 Oct 2020 21:09:59 +0000 [thread overview]
Message-ID: <MWHPR1101MB21250A29D3076316CB4182D1B31F0@MWHPR1101MB2125.namprd11.prod.outlook.com> (raw)
In-Reply-To: <CY4PR11MB12881CBF65EC161F1056D8AD8C010@CY4PR11MB1288.namprd11.prod.outlook.com>
Comments inline.
> -----Original Message-----
> From: Yao, Jiewen <jiewen.yao@intel.com>
> Sent: Saturday, October 17, 2020 18:26
> To: Zurcher, Christopher J <christopher.j.zurcher@intel.com>;
> devel@edk2.groups.io
> Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>;
> Ard Biesheuvel <ard.biesheuvel@arm.com>
> Subject: RE: [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction
> support for X64
>
> Thanks Zurcher.
>
> Comment:
> 1) I do not see the copy right header and license header for uefi-asm.conf.
> Please add it.
>
> 2) Is "ApiHooks.c" only for Visual Studio? Do we need it for GCC and LLVM?
> If it is only for visual studio, I suggest we rename to MsftApiHooks.c, and
> add "| MSFT" in INF.
ApiHooks.c is for any compiler building the assembly files, not only Visual Studio.
>
> 3) Since nasm is only for X64¸ please use [Source.X64] for them.
Since the .c file set can change depending on which assembly target is selected (X64 vs. IA32 etc.) and since this .inf file is only for X64 targets, I will move all sources under the [Sources.X64] header. Is that OK? Leaving the current [Sources] adding a second autogen section under [Sources.X64] for only the .nasm files would add another layer of complexity to process_files.pl.
>
> Please refer to
> https://github.com/tianocore/edk2/blob/master/MdePkg/Library/BaseLib/BaseLib.
> inf, on 2) and 3).
>
> I want to get ACK from Mike Kinney, to see if it is right way to manually add
> nasm and ApiHooks.c. Or if we need some extra documentation for that.
> Please do add him as reviewer in next version patch.
I have added Mike Kinney for the next patch set but I have already had multiple discussions with him on this topic and the current implementation was agreed upon.
Thanks,
Christopher Zurcher
>
> I treat the ApiHooks.c is a work-around and it should be removed in the
> future, once openssl resolve it. Maybe we need a Bugzilla to track this.
>
>
> Thank you
> Yao Jiewen
>
>
> > -----Original Message-----
> > From: Christopher J Zurcher <christopher.j.zurcher@intel.com>
> > Sent: Thursday, October 15, 2020 4:49 AM
> > To: devel@edk2.groups.io
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>; Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ard
> > Biesheuvel <ard.biesheuvel@arm.com>
> > Subject: [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction
> > support for X64
> >
> > V3 Changes:
> > Added definitions for ptrdiff_t and wchar_t to CrtLibSupport.h for
> > LLVM/Clang build support.
> > Added -UWIN32 to GCC Flags for LLVM/Clang build support.
> > Added missing AES GCM assembly file.
> >
> > V2 Changes:
> > Limit scope of assembly config to SHA and AES functions.
> > Removed IA32 native support (reduced config was causing build failure and
> > can be added in a later patch).
> > Removed XMM instructions from assembly generation.
> > Added automatic copyright header porting for generated assembly files.
> >
> > This patch adds support for building the native instruction algorithms for
> > the X64 architecture in OpensslLib. The process_files.pl script was
> modified
> > to parse the .asm file targets from the OpenSSL build config data struct,
> and
> > generate the necessary assembly files for the EDK2 build environment.
> >
> > For the X64 variant, OpenSSL includes calls to a Windows error handling
> API,
> > and that function has been stubbed out in ApiHooks.c.
> >
> > For all variants, a constructor is added to call the required CPUID
> function
> > within OpenSSL to facilitate processor capability checks in the native
> > algorithms.
> >
> > Additional native architecture variants should be simple to add by
> following
> > the changes made for this architecture.
> >
> > The OpenSSL assembly files are traditionally generated at build time using
> a
> > perl script. To avoid that burden on EDK2 users, these end-result assembly
> > files are generated during the configuration steps performed by the package
> > maintainer (through process_files.pl). The perl generator scripts inside
> > OpenSSL do not parse file comments as they are only meant to create
> > intermediate build files, so process_files.pl contains additional hooks to
> > preserve the copyright headers as well as clean up tabs and line endings to
> > comply with EDK2 coding standards. The resulting file headers align with
> > the generated .h files which are already included in the EDK2 repository.
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Xiaoyu Lu <xiaoyux.lu@intel.com>
> > Cc: Ard Biesheuvel <ard.biesheuvel@arm.com>
> >
> > Christopher J Zurcher (2):
> > CryptoPkg/OpensslLib: Add native instruction support for X64
> > CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64
> >
> > CryptoPkg/Library/OpensslLib/OpensslLib.inf |
> 2 +-
> > CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf |
> 2 +-
> > CryptoPkg/Library/OpensslLib/OpensslLibX64.inf |
> 657 +++
> > CryptoPkg/Library/Include/CrtLibSupport.h |
> 2 +
> > CryptoPkg/Library/Include/openssl/opensslconf.h |
> 3 -
> > CryptoPkg/Library/OpensslLib/ApiHooks.c |
> 18 +
> > CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c |
> 34 +
> > CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-mb-x86_64.nasm |
> > 732 +++
> > CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha1-x86_64.nasm |
> > 1916 ++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-sha256-x86_64.nasm |
> > 78 +
> > CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-x86_64.nasm |
> > 5103 ++++++++++++++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/aes/vpaes-x86_64.nasm |
> > 1173 +++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/modes/aesni-gcm-x86_64.nasm |
> > 34 +
> > CryptoPkg/Library/OpensslLib/X64/crypto/modes/ghash-x86_64.nasm |
> > 1569 ++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-mb-x86_64.nasm |
> > 3137 ++++++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-x86_64.nasm |
> > 2884 +++++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-mb-x86_64.nasm |
> > 3461 +++++++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-x86_64.nasm |
> > 3313 +++++++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha512-x86_64.nasm |
> > 1938 ++++++++
> > CryptoPkg/Library/OpensslLib/X64/crypto/x86_64cpuid.nasm |
> 491
> > ++
> > CryptoPkg/Library/OpensslLib/process_files.pl |
> 223 +-
> > CryptoPkg/Library/OpensslLib/uefi-asm.conf |
> 15 +
> > 22 files changed, 26735 insertions(+), 50 deletions(-)
> > create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > create mode 100644 CryptoPkg/Library/OpensslLib/ApiHooks.c
> > create mode 100644 CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
> > mb-x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
> > sha1-x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
> > sha256-x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/aesni-
> > x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/aes/vpaes-
> > x86_64.nasm
> > create mode 100644
> > CryptoPkg/Library/OpensslLib/X64/crypto/modes/aesni-gcm-x86_64.nasm
> > create mode 100644
> > CryptoPkg/Library/OpensslLib/X64/crypto/modes/ghash-x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-
> > mb-x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha1-
> > x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-
> > mb-x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha256-
> > x86_64.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/X64/crypto/sha/sha512-
> > x86_64.nasm
> > create mode 100644
> > CryptoPkg/Library/OpensslLib/X64/crypto/x86_64cpuid.nasm
> > create mode 100644 CryptoPkg/Library/OpensslLib/uefi-asm.conf
> >
> > --
> > 2.28.0.windows.1
next prev parent reply other threads:[~2020-10-20 21:10 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-14 20:48 [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction support for X64 Zurcher, Christopher J
2020-10-14 20:48 ` [PATCH v3 1/2] " Zurcher, Christopher J
2020-10-14 20:48 ` [PATCH v3 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files " Zurcher, Christopher J
2020-10-18 1:25 ` [PATCH v3 0/2] CryptoPkg/OpensslLib: Add native instruction support " Yao, Jiewen
2020-10-20 21:09 ` Zurcher, Christopher J [this message]
2020-10-23 22:56 ` 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=MWHPR1101MB21250A29D3076316CB4182D1B31F0@MWHPR1101MB2125.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