回复: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add native instruction support for X64

["gaoliming" <gaoliming@byosoft.com.cn>]

Zurcher:
  https://www.nasm.us/xdoc/2.13.02rc3/html/nasmdoc7.html mentions wrt
..imagebase usage in 7.6 win64: Microsoft Win64 Object Files. 
  So, it is only for win64 object. VS and CLANGPDB tool chain uses nasm
win64 object. But, GCC5 uses nasm elf64 object, and XCODE uses nasm macho64
object.

  That means this native support for X64 is for VS tool chain only. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Zurcher, Christopher J <christopher.j.zurcher@intel.com>
> 发送时间: 2020年11月7日 3:36
> 收件人: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io; Laszlo
> Ersek <lersek@redhat.com>; gaoliming <gaoliming@byosoft.com.cn>
> 抄送: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> <xiaoyux.lu@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>;
> Ard Biesheuvel <ard.biesheuvel@arm.com>
> 主题: RE: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add native
> instruction support for X64
> 
> Yes, in response to your request, it was discussed on the mailing list
that the
> OpenSSL assembly code is not compatible with GCC in this format. It is
fully
> functional with VS toolchain and CLANGPDB toolchain. The GCC toolchain
> appears to be unable to handle some aspects of NASM code, particularly the
> COMMON section and the "wrt ..imagebase" style of position-independent
> code.
> See 7.6.1 here: https://www.nasm.us/xdoc/2.13.02rc3/html/nasmdoc7.html
> 
> Mike/Laszlo/Liming,
> Can you help me resolve the CI failures?
> 
> Thanks,
> Christopher Zurcher
> 
> > -----Original Message-----
> > From: Yao, Jiewen <jiewen.yao@intel.com>
> > Sent: Friday, November 6, 2020 02:23
> > 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>;
> > Kinney, Michael D <michael.d.kinney@intel.com>; Ard Biesheuvel
> > <ard.biesheuvel@arm.com>
> > Subject: RE: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add
native
> > instruction support for X64
> >
> > Hi Zurcher
> > I am not CI person, so I recommend you have a discussion with Mike,
Laszlo,
> > or Liming to see what is the best way to resolve the coding style issue.
Or
> > how to change CI rule to add exception somewhere.
> >
> > However, I do have concern, when you say: "the module is not compatible
> with
> > GCC builds."
> >
> > In previous review, I already gave the comment to pass build with GCC
and
> > CLANG besides MSVC. Do you mean this patch cannot pass GCC build?
> >
> > Thank you
> > Yao Jiewen
> >
> > > -----Original Message-----
> > > From: Zurcher, Christopher J <christopher.j.zurcher@intel.com>
> > > Sent: Friday, November 6, 2020 5:50 PM
> > > To: Yao, Jiewen <jiewen.yao@intel.com>; devel@edk2.groups.io
> > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > > <xiaoyux.lu@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > Subject: RE: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add
> native
> > > instruction support for X64
> > >
> > > I think some of these errors are not relevant based on the nature of
the
> > > commit:
> > >
> > > One of the errors reported is that the added typedefs (ptrdiff_t,
wchar_t)
> > do
> > > not follow coding guidelines, but the typedefs are required for
OpenSSL
> > > compatibility and they match the already-existing style:
> > >
> > >  typedef UINTN          size_t;
> > >  typedef UINTN          u_int;
> > > +typedef INTN           ptrdiff_t;
> > >  typedef INTN           ssize_t;
> > >  typedef INT32          time_t;
> > >  typedef UINT8          __uint8_t;
> > >  typedef UINT8          sa_family_t;
> > >  typedef UINT8          u_char;
> > >  typedef UINT32         uid_t;
> > >  typedef UINT32         gid_t;
> > > +typedef CHAR16         wchar_t;
> > >
> > > Another error is that the auto-generated assembly files do not start
with
> > > capital letters, but these filenames come from OpenSSL with lowercase
> > > filenames, and we already have opensslconf.h in the Include folder
which
> > > has a lowercase filename.
> > >
> > > Another error type reported is that the auto-generated assembly files
do
> > not
> > > have "SPDX-License-Identifier: BSD-2-Clause-Patent" but it was already
> > > discussed on the list that these would be checked in with the OpenSSL
> > header
> > > similar to opensslconf.h:
> > >
> https://github.com/tianocore/edk2/blob/master/CryptoPkg/Library/Include/
> > > openssl/opensslconf.h
> > >
> > > Additionally, there is an error that OpensslLibX64.inf is not in
> > CryptoPkg.dsc,
> > > but I am not sure if it is appropriate to include as the module is not
> > > compatible with GCC builds.
> > >
> > > How should I proceed here?
> > >
> > > Thanks,
> > > Christopher Zurcher
> > >
> > > > -----Original Message-----
> > > > From: Yao, Jiewen <jiewen.yao@intel.com>
> > > > Sent: Thursday, November 5, 2020 22:14
> > > > To: devel@edk2.groups.io; Yao, Jiewen <jiewen.yao@intel.com>;
> Zurcher,
> > > > Christopher J <christopher.j.zurcher@intel.com>
> > > > Cc: Wang, Jian J <jian.j.wang@intel.com>; Lu, XiaoyuX
> > > <xiaoyux.lu@intel.com>;
> > > > Kinney, Michael D <michael.d.kinney@intel.com>; Ard Biesheuvel
> > > > <ard.biesheuvel@arm.com>
> > > > Subject: RE: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add
> native
> > > > instruction support for X64
> > > >
> > > > Hi Zurcher
> > > > I created https://github.com/tianocore/edk2/pull/1092
> > > >
> > > > However, there are failures in CI test. So this patch is NOT merged.
> > > >
> > > > Please take a look and resolve it.
> > > >
> > > > Thank you
> > > > Yao Jiewen
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> Yao,
> > > > > Jiewen
> > > > > Sent: Friday, November 6, 2020 1:56 PM
> > > > > 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>; Kinney, Michael D
> > > <michael.d.kinney@intel.com>;
> > > > > Ard Biesheuvel <ard.biesheuvel@arm.com>
> > > > > Subject: Re: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add
> > > native
> > > > > instruction support for X64
> > > > >
> > > > > Patch 1/2 reviewed-by: Jiewen Yao <Jiewen.yao@intel.com>
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: Christopher J Zurcher <christopher.j.zurcher@intel.com>
> > > > > > Sent: Wednesday, November 4, 2020 5:59 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>;
> Kinney,
> > > > > > Michael D <michael.d.kinney@intel.com>; Ard Biesheuvel
> > > > > > <ard.biesheuvel@arm.com>
> > > > > > Subject: [PATCH v5 0/2] CryptoPkg/OpensslLib: Add native
> instruction
> > > > > > support for X64
> > > > > >
> > > > > > V5 Changes:
> > > > > >   Move ApiHooks.c into X64 folder
> > > > > >   Update process_files.pl to clean architecture-specific
subfolders
> > > > without
> > > > > >     removing them
> > > > > >   Rebased INF file to merge latest changes regarding RngLib vs.
> > TimerLib
> > > > > >
> > > > > > V4 Changes:
> > > > > >   Add copyright header to uefi-asm.conf
> > > > > >   Move [Sources.X64] block to cover entire X64-specific config
> > > > > >
> > > > > > 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: Mike Kinney <michael.d.kinney@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
> > |
> > > > 653 +++
> > > > > >  CryptoPkg/Library/Include/CrtLibSupport.h
> > |
> > > > 2 +
> > > > > >  CryptoPkg/Library/Include/openssl/opensslconf.h
> > |
> > > > 3 -
> > > > > >  CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c
> > |
> > > > 34 +
> > > > > >  CryptoPkg/Library/OpensslLib/X64/ApiHooks.c
> > |
> > > > 18 +
> > > > > >
> 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
> > |
> > > > 232 +-
> > > > > >  CryptoPkg/Library/OpensslLib/uefi-asm.conf
> > |
> > > > 21 +
> > > > > >  22 files changed, 26746 insertions(+), 50 deletions(-)
> > > > > >  create mode 100644
> CryptoPkg/Library/OpensslLib/OpensslLibX64.inf
> > > > > >  create mode 100644
> > > > > CryptoPkg/Library/OpensslLib/OpensslLibConstructor.c
> > > > > >  create mode 100644
> CryptoPkg/Library/OpensslLib/X64/ApiHooks.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
> > > > >
> > > > >
> > > > >
> > > > > 
> > > > >