* [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
@ 2017-07-14 17:19 Ard Biesheuvel
2017-07-14 18:16 ` Leif Lindholm
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-07-14 17:19 UTC (permalink / raw)
To: edk2-devel, qin.long; +Cc: ting.ye, leif.lindholm, lersek, Ard Biesheuvel
Commit 0df6c8c157af ("BaseTools/tools_def AARCH64: avoid SIMD registers
in XIP code") updated the compiler flags used by AARCH64 when building
modules (including BASE libraries) that may execute before the MMU is
enabled.
This broke the build for OpensslLib/OpensslLibCrypto because the SIMD
register file is shared with the FPU, and since OpenSSL contains some
references to float/double types (which are mostly unused for UEFI btw),
disabling floating point prevents the compiler from building OpenSSL
at all.
When introducing the support for XIP CC flags, we were aware that this
would affect BASE libraries as well, but were not expecting this to
have any performance impact. However, in the case of software crypto,
it makes sense not to needlessly inhibit the compiler's ability to
generate fast code, and even if OpenssLib is a BASE library, it is
guaranteed not to run with the MMU off, so we can create a local
exception, and clear its XIP CC flags for AARCH64.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Note that this un-breaks the currently broken AARCH64 build for
platforms that have secure boot enabled
CryptoPkg/Library/OpensslLib/OpensslLib.inf | 11 +++++++++++
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 11 +++++++++++
2 files changed, 22 insertions(+)
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index cbabb34bdd7c..1d15da6660b2 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -580,3 +580,14 @@ [BuildOptions]
RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
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
+
+ #
+ # AARCH64 uses strict alignment and avoids SIMD registers for code that may execute
+ # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as BASE
+ # libraries, given that they may be included into such modules.
+ # This library, even though of the BASE type, is never used in such cases, and
+ # avoiding the SIMD register file (which is shared with the FPU) prevents the
+ # compiler from successfully building some of the OpenSSL source files that
+ # use floating point types, so clear the flags here.
+ #
+ GCC:*_*_AARCH64_CC_XIPFLAGS ==
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index 026b551bcafa..6fc8884da492 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -541,3 +541,14 @@ [BuildOptions]
RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
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
+
+ #
+ # AARCH64 uses strict alignment and avoids SIMD registers for code that may execute
+ # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as BASE
+ # libraries, given that they may be included into such modules.
+ # This library, even though of the BASE type, is never used in such cases, and
+ # avoiding the SIMD register file (which is shared with the FPU) prevents the
+ # compiler from successfully building some of the OpenSSL source files that
+ # use floating point types, so clear the flags here.
+ #
+ GCC:*_*_AARCH64_CC_XIPFLAGS ==
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
2017-07-14 17:19 [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags Ard Biesheuvel
@ 2017-07-14 18:16 ` Leif Lindholm
2017-07-14 18:22 ` Laszlo Ersek
2017-07-15 12:09 ` Long, Qin
2 siblings, 0 replies; 7+ messages in thread
From: Leif Lindholm @ 2017-07-14 18:16 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel, qin.long, ting.ye, lersek
On Fri, Jul 14, 2017 at 06:19:13PM +0100, Ard Biesheuvel wrote:
> Commit 0df6c8c157af ("BaseTools/tools_def AARCH64: avoid SIMD registers
> in XIP code") updated the compiler flags used by AARCH64 when building
> modules (including BASE libraries) that may execute before the MMU is
> enabled.
>
> This broke the build for OpensslLib/OpensslLibCrypto because the SIMD
> register file is shared with the FPU, and since OpenSSL contains some
> references to float/double types (which are mostly unused for UEFI btw),
> disabling floating point prevents the compiler from building OpenSSL
> at all.
>
> When introducing the support for XIP CC flags, we were aware that this
> would affect BASE libraries as well, but were not expecting this to
> have any performance impact. However, in the case of software crypto,
> it makes sense not to needlessly inhibit the compiler's ability to
> generate fast code, and even if OpenssLib is a BASE library, it is
> guaranteed not to run with the MMU off, so we can create a local
> exception, and clear its XIP CC flags for AARCH64.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
> Note that this un-breaks the currently broken AARCH64 build for
> platforms that have secure boot enabled
>
> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 11 +++++++++++
> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 11 +++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index cbabb34bdd7c..1d15da6660b2 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -580,3 +580,14 @@ [BuildOptions]
> RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
> 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
> +
> + #
> + # AARCH64 uses strict alignment and avoids SIMD registers for code that may execute
> + # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as BASE
> + # libraries, given that they may be included into such modules.
> + # This library, even though of the BASE type, is never used in such cases, and
> + # avoiding the SIMD register file (which is shared with the FPU) prevents the
> + # compiler from successfully building some of the OpenSSL source files that
> + # use floating point types, so clear the flags here.
> + #
> + GCC:*_*_AARCH64_CC_XIPFLAGS ==
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index 026b551bcafa..6fc8884da492 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -541,3 +541,14 @@ [BuildOptions]
> RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
> 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
> +
> + #
> + # AARCH64 uses strict alignment and avoids SIMD registers for code that may execute
> + # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as BASE
> + # libraries, given that they may be included into such modules.
> + # This library, even though of the BASE type, is never used in such cases, and
> + # avoiding the SIMD register file (which is shared with the FPU) prevents the
> + # compiler from successfully building some of the OpenSSL source files that
> + # use floating point types, so clear the flags here.
> + #
> + GCC:*_*_AARCH64_CC_XIPFLAGS ==
> --
> 2.9.3
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
2017-07-14 17:19 [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags Ard Biesheuvel
2017-07-14 18:16 ` Leif Lindholm
@ 2017-07-14 18:22 ` Laszlo Ersek
2017-07-14 18:41 ` Ard Biesheuvel
2017-07-15 12:09 ` Long, Qin
2 siblings, 1 reply; 7+ messages in thread
From: Laszlo Ersek @ 2017-07-14 18:22 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel, qin.long; +Cc: ting.ye, leif.lindholm
On 07/14/17 19:19, Ard Biesheuvel wrote:
> Commit 0df6c8c157af ("BaseTools/tools_def AARCH64: avoid SIMD registers
> in XIP code") updated the compiler flags used by AARCH64 when building
> modules (including BASE libraries) that may execute before the MMU is
> enabled.
>
> This broke the build for OpensslLib/OpensslLibCrypto because the SIMD
> register file is shared with the FPU, and since OpenSSL contains some
> references to float/double types (which are mostly unused for UEFI btw),
> disabling floating point prevents the compiler from building OpenSSL
> at all.
I'm with you until here.
>
> When introducing the support for XIP CC flags, we were aware that this
> would affect BASE libraries as well, but were not expecting this to
> have any performance impact. However, in the case of software crypto,
> it makes sense not to needlessly inhibit the compiler's ability to
> generate fast code, and even if OpenssLib is a BASE library, it is
> guaranteed not to run with the MMU off, so we can create a local
> exception, and clear its XIP CC flags for AARCH64.
I'm confused by the last paragraph. The fist two paragraphs, and commit
0df6c8c157af explain why SIMD register code gen was disabled (in brief,
to work around a GCC bug).
Now it turns out that some code in OpenSSL cannot be compiled at all,
without us allowing insn generation for SIMD registers (because OpenSSL
uses float/double). Such code is unsafe to execute for at least two
reasons ((a) with the MMU off it could trigger the same issue that
0df6c8c157af meant to protect against, and (b) because we don't use
floating point in UEFI). But, said code in OpenSSL is never executed, so
we can allow the potentially invalid code to be produced at least.
But why talk about performance then?
Do you mean that, in OpenSSL,
(a) is not an issue, because the MMU is always on by then, so commit
0df6c8c157af doesn't apply,
(b) although we don't use floating point, the integer arithmetic still
benefits from SIMD?
If so, I think (hope!) that I understand the argument, I'd just like a
bit more hand-holding in the structure of the argument. Can you reword
the third paragraph as "... another benefit of reenabling SIMD is
restoring performance for at least OpenSSL's integer arithmetic, without
risk of faults (because OpenSSL never runs with the MMU disabled)".
Sorry if I'm being dense. :)
Thanks
Laszlo
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> Note that this un-breaks the currently broken AARCH64 build for
> platforms that have secure boot enabled
>
> CryptoPkg/Library/OpensslLib/OpensslLib.inf | 11 +++++++++++
> CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 11 +++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> index cbabb34bdd7c..1d15da6660b2 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
> @@ -580,3 +580,14 @@ [BuildOptions]
> RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
> 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
> +
> + #
> + # AARCH64 uses strict alignment and avoids SIMD registers for code that may execute
> + # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as BASE
> + # libraries, given that they may be included into such modules.
> + # This library, even though of the BASE type, is never used in such cases, and
> + # avoiding the SIMD register file (which is shared with the FPU) prevents the
> + # compiler from successfully building some of the OpenSSL source files that
> + # use floating point types, so clear the flags here.
> + #
> + GCC:*_*_AARCH64_CC_XIPFLAGS ==
> diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> index 026b551bcafa..6fc8884da492 100644
> --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
> @@ -541,3 +541,14 @@ [BuildOptions]
> RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
> 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
> +
> + #
> + # AARCH64 uses strict alignment and avoids SIMD registers for code that may execute
> + # with the MMU off. This involves SEC, PEI_CORE and PEIM modules as well as BASE
> + # libraries, given that they may be included into such modules.
> + # This library, even though of the BASE type, is never used in such cases, and
> + # avoiding the SIMD register file (which is shared with the FPU) prevents the
> + # compiler from successfully building some of the OpenSSL source files that
> + # use floating point types, so clear the flags here.
> + #
> + GCC:*_*_AARCH64_CC_XIPFLAGS ==
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
2017-07-14 18:22 ` Laszlo Ersek
@ 2017-07-14 18:41 ` Ard Biesheuvel
2017-07-14 19:59 ` Laszlo Ersek
0 siblings, 1 reply; 7+ messages in thread
From: Ard Biesheuvel @ 2017-07-14 18:41 UTC (permalink / raw)
To: Laszlo Ersek; +Cc: edk2-devel@lists.01.org, qlong, Ye, Ting, Leif Lindholm
On 14 July 2017 at 19:22, Laszlo Ersek <lersek@redhat.com> wrote:
> On 07/14/17 19:19, Ard Biesheuvel wrote:
>> Commit 0df6c8c157af ("BaseTools/tools_def AARCH64: avoid SIMD registers
>> in XIP code") updated the compiler flags used by AARCH64 when building
>> modules (including BASE libraries) that may execute before the MMU is
>> enabled.
>>
>> This broke the build for OpensslLib/OpensslLibCrypto because the SIMD
>> register file is shared with the FPU, and since OpenSSL contains some
>> references to float/double types (which are mostly unused for UEFI btw),
>> disabling floating point prevents the compiler from building OpenSSL
>> at all.
>
> I'm with you until here.
>
>>
>> When introducing the support for XIP CC flags, we were aware that this
>> would affect BASE libraries as well, but were not expecting this to
>> have any performance impact. However, in the case of software crypto,
>> it makes sense not to needlessly inhibit the compiler's ability to
>> generate fast code, and even if OpenssLib is a BASE library, it is
>> guaranteed not to run with the MMU off, so we can create a local
>> exception, and clear its XIP CC flags for AARCH64.
>
> I'm confused by the last paragraph. The fist two paragraphs, and commit
> 0df6c8c157af explain why SIMD register code gen was disabled (in brief,
> to work around a GCC bug).
>
> Now it turns out that some code in OpenSSL cannot be compiled at all,
> without us allowing insn generation for SIMD registers (because OpenSSL
> uses float/double). Such code is unsafe to execute for at least two
> reasons ((a) with the MMU off it could trigger the same issue that
> 0df6c8c157af meant to protect against, and (b) because we don't use
> floating point in UEFI). But, said code in OpenSSL is never executed, so
> we can allow the potentially invalid code to be produced at least.
>
> But why talk about performance then?
>
> Do you mean that, in OpenSSL,
> (a) is not an issue, because the MMU is always on by then, so commit
> 0df6c8c157af doesn't apply,
> (b) although we don't use floating point, the integer arithmetic still
> benefits from SIMD?
>
> If so, I think (hope!) that I understand the argument, I'd just like a
> bit more hand-holding in the structure of the argument. Can you reword
> the third paragraph as "... another benefit of reenabling SIMD is
> restoring performance for at least OpenSSL's integer arithmetic, without
> risk of faults (because OpenSSL never runs with the MMU disabled)".
>
> Sorry if I'm being dense. :)
No, not at all, I just was a bit terse in the description.
Before the mentioned commit, XIP CC flags was set to -mstrict-align,
which is required for modules that may execute with the MMU off. This
has nothing to do with SIMD or FP.
When we added -mgeneral-regs-only to that, we broke the OpensslLib
build, which drew my attention to the fact that it, being a BASE
library, is also built with -mstrict-align, and /this/ is the
potential performance concern.
So clearing XIP CC flags serves two distinct purposes:
- removing -mgeneral-regs-only to unbreak the build
- removing -mstrict-align to let the compiler generate faster code.
--
Ard.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
2017-07-14 18:41 ` Ard Biesheuvel
@ 2017-07-14 19:59 ` Laszlo Ersek
0 siblings, 0 replies; 7+ messages in thread
From: Laszlo Ersek @ 2017-07-14 19:59 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: edk2-devel@lists.01.org, qlong, Ye, Ting, Leif Lindholm
On 07/14/17 20:41, Ard Biesheuvel wrote:
> On 14 July 2017 at 19:22, Laszlo Ersek <lersek@redhat.com> wrote:
>> On 07/14/17 19:19, Ard Biesheuvel wrote:
>>> Commit 0df6c8c157af ("BaseTools/tools_def AARCH64: avoid SIMD registers
>>> in XIP code") updated the compiler flags used by AARCH64 when building
>>> modules (including BASE libraries) that may execute before the MMU is
>>> enabled.
>>>
>>> This broke the build for OpensslLib/OpensslLibCrypto because the SIMD
>>> register file is shared with the FPU, and since OpenSSL contains some
>>> references to float/double types (which are mostly unused for UEFI btw),
>>> disabling floating point prevents the compiler from building OpenSSL
>>> at all.
>>
>> I'm with you until here.
>>
>>>
>>> When introducing the support for XIP CC flags, we were aware that this
>>> would affect BASE libraries as well, but were not expecting this to
>>> have any performance impact. However, in the case of software crypto,
>>> it makes sense not to needlessly inhibit the compiler's ability to
>>> generate fast code, and even if OpenssLib is a BASE library, it is
>>> guaranteed not to run with the MMU off, so we can create a local
>>> exception, and clear its XIP CC flags for AARCH64.
>>
>> I'm confused by the last paragraph. The fist two paragraphs, and commit
>> 0df6c8c157af explain why SIMD register code gen was disabled (in brief,
>> to work around a GCC bug).
>>
>> Now it turns out that some code in OpenSSL cannot be compiled at all,
>> without us allowing insn generation for SIMD registers (because OpenSSL
>> uses float/double). Such code is unsafe to execute for at least two
>> reasons ((a) with the MMU off it could trigger the same issue that
>> 0df6c8c157af meant to protect against, and (b) because we don't use
>> floating point in UEFI). But, said code in OpenSSL is never executed, so
>> we can allow the potentially invalid code to be produced at least.
>>
>> But why talk about performance then?
>>
>> Do you mean that, in OpenSSL,
>> (a) is not an issue, because the MMU is always on by then, so commit
>> 0df6c8c157af doesn't apply,
>> (b) although we don't use floating point, the integer arithmetic still
>> benefits from SIMD?
>>
>> If so, I think (hope!) that I understand the argument, I'd just like a
>> bit more hand-holding in the structure of the argument. Can you reword
>> the third paragraph as "... another benefit of reenabling SIMD is
>> restoring performance for at least OpenSSL's integer arithmetic, without
>> risk of faults (because OpenSSL never runs with the MMU disabled)".
>>
>> Sorry if I'm being dense. :)
>
> No, not at all, I just was a bit terse in the description.
>
> Before the mentioned commit, XIP CC flags was set to -mstrict-align,
> which is required for modules that may execute with the MMU off. This
> has nothing to do with SIMD or FP.
>
> When we added -mgeneral-regs-only to that, we broke the OpensslLib
> build, which drew my attention to the fact that it, being a BASE
> library, is also built with -mstrict-align, and /this/ is the
> potential performance concern.
>
> So clearing XIP CC flags serves two distinct purposes:
> - removing -mgeneral-regs-only to unbreak the build
> - removing -mstrict-align to let the compiler generate faster code.
>
Thanks! For completeness, it would be great if you could work this into
the commit message before pushing the patch.
Acked-by: Laszlo Ersek <lersek@redhat.com>
Cheers,
Laszlo
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
2017-07-14 17:19 [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags Ard Biesheuvel
2017-07-14 18:16 ` Leif Lindholm
2017-07-14 18:22 ` Laszlo Ersek
@ 2017-07-15 12:09 ` Long, Qin
2017-07-15 12:38 ` Ard Biesheuvel
2 siblings, 1 reply; 7+ messages in thread
From: Long, Qin @ 2017-07-15 12:09 UTC (permalink / raw)
To: Ard Biesheuvel, edk2-devel@lists.01.org
Cc: Ye, Ting, leif.lindholm@linaro.org, lersek@redhat.com
Reviewed-by: Long Qin <qin.long@intel.com>
Best Regards & Thanks,
LONG, Qin
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Saturday, July 15, 2017 1:19 AM
To: edk2-devel@lists.01.org; Long, Qin <qin.long@intel.com>
Cc: Ye, Ting <ting.ye@intel.com>; leif.lindholm@linaro.org; lersek@redhat.com; Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
Commit 0df6c8c157af ("BaseTools/tools_def AARCH64: avoid SIMD registers in XIP code") updated the compiler flags used by AARCH64 when building modules (including BASE libraries) that may execute before the MMU is enabled.
This broke the build for OpensslLib/OpensslLibCrypto because the SIMD register file is shared with the FPU, and since OpenSSL contains some references to float/double types (which are mostly unused for UEFI btw), disabling floating point prevents the compiler from building OpenSSL at all.
When introducing the support for XIP CC flags, we were aware that this would affect BASE libraries as well, but were not expecting this to have any performance impact. However, in the case of software crypto, it makes sense not to needlessly inhibit the compiler's ability to generate fast code, and even if OpenssLib is a BASE library, it is guaranteed not to run with the MMU off, so we can create a local exception, and clear its XIP CC flags for AARCH64.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
Note that this un-breaks the currently broken AARCH64 build for platforms that have secure boot enabled
CryptoPkg/Library/OpensslLib/OpensslLib.inf | 11 +++++++++++
CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf | 11 +++++++++++
2 files changed, 22 insertions(+)
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
index cbabb34bdd7c..1d15da6660b2 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf
@@ -580,3 +580,14 @@ [BuildOptions]
RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
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
+
+ #
+ # AARCH64 uses strict alignment and avoids SIMD registers for code
+ that may execute # with the MMU off. This involves SEC, PEI_CORE and
+ PEIM modules as well as BASE # libraries, given that they may be included into such modules.
+ # This library, even though of the BASE type, is never used in such
+ cases, and # avoiding the SIMD register file (which is shared with
+ the FPU) prevents the # compiler from successfully building some of
+ the OpenSSL source files that # use floating point types, so clear the flags here.
+ #
+ GCC:*_*_AARCH64_CC_XIPFLAGS ==
diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
index 026b551bcafa..6fc8884da492 100644
--- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
+++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf
@@ -541,3 +541,14 @@ [BuildOptions]
RVCT:*_*_ARM_CC_FLAGS = $(OPENSSL_FLAGS) --library_interface=aeabi_clib99 --diag_suppress=1296,1295,550,1293,111,68,177,223,144,513,188,128,546,1,3017 -JCryptoPkg/Include
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
+
+ #
+ # AARCH64 uses strict alignment and avoids SIMD registers for code
+ that may execute # with the MMU off. This involves SEC, PEI_CORE and
+ PEIM modules as well as BASE # libraries, given that they may be included into such modules.
+ # This library, even though of the BASE type, is never used in such
+ cases, and # avoiding the SIMD register file (which is shared with
+ the FPU) prevents the # compiler from successfully building some of
+ the OpenSSL source files that # use floating point types, so clear the flags here.
+ #
+ GCC:*_*_AARCH64_CC_XIPFLAGS ==
--
2.9.3
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags
2017-07-15 12:09 ` Long, Qin
@ 2017-07-15 12:38 ` Ard Biesheuvel
0 siblings, 0 replies; 7+ messages in thread
From: Ard Biesheuvel @ 2017-07-15 12:38 UTC (permalink / raw)
To: Long, Qin
Cc: edk2-devel@lists.01.org, Ye, Ting, lersek@redhat.com,
leif.lindholm@linaro.org
On 15 July 2017 at 13:09, Long, Qin <qin.long@intel.com> wrote:
> Reviewed-by: Long Qin <qin.long@intel.com>
>
Thanks all.
Pushed as e38eb2595b86 (with the commit log updated to clarify the
performance concern)
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-07-15 12:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-14 17:19 [PATCH] CryptoPkg/OpensslLib AARCH64: clear XIP CC flags Ard Biesheuvel
2017-07-14 18:16 ` Leif Lindholm
2017-07-14 18:22 ` Laszlo Ersek
2017-07-14 18:41 ` Ard Biesheuvel
2017-07-14 19:59 ` Laszlo Ersek
2017-07-15 12:09 ` Long, Qin
2017-07-15 12:38 ` Ard Biesheuvel
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox