Ard, thanks - as you can tell this is an issue in the original flags and not with the changes I've added. I agree that we should not redefine the fpu in this file but rather inherit from the build.
David, good comments - I'm working on an updated patch. I'm running into some issues related to printf and alloca that I need to address as well. I don't think we want each edk2 developer independently opening openssl tickets as we encounter issues. It would probably make more sense for Long to do this or, if we think this is an RVCT-unique issue, for Ard or Leif to do this. Do we already have these responsibilities identified? Eugene -----Original Message----- From: Ard Biesheuvel [mailto:[email protected]] Sent: Tuesday, November 24, 2015 8:49 AM To: David Woodhouse <[email protected]> Cc: Cohen, Eugene <[email protected]>; Long, Qin <[email protected]>; [email protected] <[email protected]>; Leif Lindholm <[email protected]> Subject: Re: [edk2] CryptoPkg: OpenSSL build issue with RVCT On 24 November 2015 at 15:47, David Woodhouse <[email protected]> wrote: > On Tue, 2015-11-24 at 14:19 +0000, Cohen, Eugene wrote: >> >> Here's a patch with this changes: >> >> --- >> edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf | 2 >> +- >> edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c | 1 >> + >> 2 files changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> b/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> index 2e74f6c..261861b 100644 >> --- a/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> +++ b/edk2/CryptoPkg/Library/OpensslLib/OpensslLib.inf >> @@ -889,6 +889,6 @@ >> # 513: a value of type cannot be assigned to an entity of type >> # 188: enumerated type mixed with another type (i.e. passing an integer >> as an enum without a cast) >> # 1296: Extended constant initialiser used >> - RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) >> --library_interface=aeabi_clib99 --fpu=vfpv3 >> --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188 >> + RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) >> --library_interface=aeabi_clib99 --fpu=vfpv3 >> --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546 This looks wrong. The UEFI spec disallows the use of hardware FP on 32-bit ARM, and yet we are passing --fpu=vfpv3. I actually tried building OpenSslLib for ARM using GCC, and got in trouble with the RNG routines that rely on FP, so my conclusion was that it requires some software FP routines (or a build time switch to omit those files and lose some functionality) Note that this is not a theoretical problem. Under Linux, you can only use FP in kernel mode if you explicitly wrap it with fp_begin/fp_end routines, and if you don't, anything that touches the FP register file will trap. This includes the UEFI runtime services invocations, since their entry/exit routines don't call those wrappers. And even if it wouldn't trap, you would still have to preserve the contents of the FP registers, since an ordinary context switch or syscall does not take care of that. -- Ard. > > > You neglected to add to the comments — they're there in the context of > your patch, explaining at least #513, #188 and #1296. You should be > adding the same for the newly-added #128 and #546. > > Additionally *all* of those comments should have a reference to the > specific OpenSSL RT ticket which covers the problem. And then we can > properly track them and remove them when they're obsolete, and don't > just accumulate them for ever. > >> XCODE:*_*_IA32_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 >> $(OPENSSL_FLAGS) -w >> XCODE:*_*_X64_CC_FLAGS = -mmmx -msse -U_WIN32 -U_WIN64 >> $(OPENSSL_FLAGS) -w >> diff --git >> a/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c >> b/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c >> index 35dde28..35e3b39 100644 >> --- a/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c >> +++ b/edk2/CryptoPkg/Library/OpensslLib/openssl-1.0.2d/crypto/x509/x509_vfy.c >> @@ -859,6 +859,7 @@ static int check_cert(X509_STORE_CTX *ctx) >> X509 *x; >> int ok, cnum; >> unsigned int last_reasons; >> + ok = 0; >> cnum = ctx->error_depth; >> x = sk_X509_value(ctx->chain, cnum); >> ctx->current_cert = x; > > Again, don't add this here without an upstream RT ticket being filed. > The aim is to get to *zero* delta between us and upstream OpenSSL, and > we won't get there if we do this kind of thing. > > -- > dwmw2 > _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
