From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.8094.1604904992493694698 for ; Sun, 08 Nov 2020 22:56:35 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 09 Nov 2020 14:56:25 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Zurcher, Christopher J'" , "'Yao, Jiewen'" , , "'Laszlo Ersek'" Cc: "'Wang, Jian J'" , "'Lu, XiaoyuX'" , "'Kinney, Michael D'" , "'Ard Biesheuvel'" References: <20201103215834.7533-1-christopher.j.zurcher@intel.com> <1644D590FF4B7423.25549@groups.io> In-Reply-To: Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BBVENIIHY1IDAvMl0gQ3J5cHRvUGtnL09wZW5zc2xMaWI6IEFkZCBuYXRpdmUgaW5zdHJ1Y3Rpb24gc3VwcG9ydCBmb3IgWDY0?= Date: Mon, 9 Nov 2020 14:56:24 +0800 Message-ID: <006101d6b665$70a0b220$51e21660$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQERaLGbq5oCWXT0P0IWAbMRODnZFAIa4adAAjEIJzkBIylStwGAJpolAY5KELirBgrlkA== Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-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.=20 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 macho6= 4 object. That means this native support for X64 is for VS tool chain only.=20 Thanks Liming > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: Zurcher, Christopher J > =B7=A2=CB=CD=CA=B1=BC=E4: 2020=C4=EA11=D4=C27=C8=D5 3:36 > =CA=D5=BC=FE=C8=CB: Yao, Jiewen ; devel@edk2.group= s.io; Laszlo > Ersek ; gaoliming > =B3=AD=CB=CD: Wang, Jian J ; Lu, XiaoyuX > ; Kinney, Michael D ; > Ard Biesheuvel > =D6=F7=CC=E2: RE: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: Add = native > instruction support for X64 >=20 > 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 t= he > 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 >=20 > Mike/Laszlo/Liming, > Can you help me resolve the CI failures? >=20 > Thanks, > Christopher Zurcher >=20 > > -----Original Message----- > > From: Yao, Jiewen > > Sent: Friday, November 6, 2020 02:23 > > To: Zurcher, Christopher J ; > > devel@edk2.groups.io > > Cc: Wang, Jian J ; Lu, XiaoyuX > ; > > Kinney, Michael D ; Ard Biesheuvel > > > > 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 issu= e. Or > > how to change CI rule to add exception somewhere. > > > > However, I do have concern, when you say: "the module is not compatibl= e > 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 > > > Sent: Friday, November 6, 2020 5:50 PM > > > To: Yao, Jiewen ; devel@edk2.groups.io > > > Cc: Wang, Jian J ; Lu, XiaoyuX > > > ; Kinney, Michael D > ; > > > Ard Biesheuvel > > > 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 lowercas= e > > > 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 file= s do > > not > > > have "SPDX-License-Identifier: BSD-2-Clause-Patent" but it was alrea= dy > > > discussed on the list that these would be checked in with the OpenSS= L > > 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 n= ot > > > compatible with GCC builds. > > > > > > How should I proceed here? > > > > > > Thanks, > > > Christopher Zurcher > > > > > > > -----Original Message----- > > > > From: Yao, Jiewen > > > > Sent: Thursday, November 5, 2020 22:14 > > > > To: devel@edk2.groups.io; Yao, Jiewen ; > Zurcher, > > > > Christopher J > > > > Cc: Wang, Jian J ; Lu, XiaoyuX > > > ; > > > > Kinney, Michael D ; Ard Biesheuvel > > > > > > > > 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 merge= d. > > > > > > > > Please take a look and resolve it. > > > > > > > > Thank you > > > > Yao Jiewen > > > > > > > > > -----Original Message----- > > > > > From: devel@edk2.groups.io On Behalf Of > Yao, > > > > > Jiewen > > > > > Sent: Friday, November 6, 2020 1:56 PM > > > > > To: Zurcher, Christopher J ; > > > > > devel@edk2.groups.io > > > > > Cc: Wang, Jian J ; Lu, XiaoyuX > > > > > ; Kinney, Michael D > > > ; > > > > > Ard Biesheuvel > > > > > Subject: Re: [edk2-devel] [PATCH v5 0/2] CryptoPkg/OpensslLib: A= dd > > > native > > > > > instruction support for X64 > > > > > > > > > > Patch 1/2 reviewed-by: Jiewen Yao > > > > > > > > > > > -----Original Message----- > > > > > > From: Christopher J Zurcher > > > > > > Sent: Wednesday, November 4, 2020 5:59 AM > > > > > > To: devel@edk2.groups.io > > > > > > Cc: Yao, Jiewen ; Wang, Jian J > > > > > > ; Lu, XiaoyuX ; > Kinney, > > > > > > Michael D ; Ard Biesheuvel > > > > > > > > > > > > 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 buil= d > > 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 scrip= t 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 th= e > > native > > > > > > algorithms. > > > > > > > > > > > > Additional native architecture variants should be simple to ad= d by > > > > following > > > > > > the changes made for this architecture. > > > > > > > > > > > > The OpenSSL assembly files are traditionally generated at buil= d time > > > > using a > > > > > > perl script. To avoid that burden on EDK2 users, these end-result > > > > assembly > > > > > > files are generated during the configuration steps performed b= y 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 li= ne > > 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 > > > > > > Cc: Jian J Wang > > > > > > Cc: Xiaoyu Lu > > > > > > Cc: Mike Kinney > > > > > > Cc: Ard Biesheuvel > > > > > > > > > > > > Christopher J Zurcher (2): > > > > > > CryptoPkg/OpensslLib: Add native instruction support for X64 > > > > > > CryptoPkg/OpensslLib: Commit the auto-generated assembly fil= es > 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 > > > > > > > > > > > > > > > > > > > >=20 > > > > >