On 24 November 2015 at 16:53, Cohen, Eugene <eug...@hp.com> wrote: > 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. >
Looking at the repo history, it turns out that this has been present since the original RVCT support was added to OpenSslLib in 2011. That does not make it right, though, and using this library to implement runtime services for authenticated variables is going to cause trouble regardless. So I am inclined to suggest it be removed, considering that EDK2 is a reference implementation, and we should not be setting bad examples. > 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? > I'd like to see RVCT support in upstream OpenSSL first. Otherwise, it will be a moving target, and we can never upgrade our OpenSSL version without the risk of breaking the build for RVCT in a way that requires new OpenSSL tickets to be created. Unfortunately, I don't have the bandwidth to get involved in that, since RVCT is not a priority for Linaro. (Note that RVCT is 32-bit only. The proprietary 64-bit ARM compiler is based on Clang. Also, the RVCT asm dialect is completely different, so none of the ALU and NEON accelerated ARM implementation for AES and SHA can be built with it without major surgery on the perlasm files) Then, we can see what issues remain in the EDK2 integration, the most important of which is the lack of softfloat support (but perhaps we can port the code from StdLibPkg so that it can be used in DXE?) Regards, Ard. > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Tuesday, November 24, 2015 8:49 AM > To: David Woodhouse <dw...@infradead.org> > Cc: Cohen, Eugene <eug...@hp.com>; Long, Qin <qin.l...@intel.com>; > edk2-devel@lists.01.org <edk2-de...@ml01.01.org>; Leif Lindholm > <leif.lindh...@arm.com> > Subject: Re: [edk2] CryptoPkg: OpenSSL build issue with RVCT > > On 24 November 2015 at 15:47, David Woodhouse <dw...@infradead.org> 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 edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
