Compare commits

...

2 Commits

Author SHA1 Message Date
David Benjamin 1b5bcb59f4 Make CRYPTO_is_NEON_capable aware of the buggy CPU.
If we're to allow the buggy CPU workaround to fire when __ARM_NEON__ is set,
CRYPTO_is_NEON_capable also needs to be aware of it. Also add an API to export
this value out of BoringSSL, so we can get some metrics on how prevalent this
chip is.

BUG=chromium:606629

Change-Id: I97d65a47a6130689098b32ce45a8c57c468aa405
Reviewed-on: https://boringssl-review.googlesource.com/7796
Reviewed-by: Adam Langley <agl@google.com>
2016-04-29 14:24:33 -04:00
David Benjamin b69307a1c4 Don't set a default armcap state in dynamic armcap modes.
The getauxval (and friends) code would be filling that in anyway. The default
only serves to enable NEON even if the OS is old enough to be missing getauxval
(and everything else).

Notably, this unbreaks the has_buggy_neon code when __ARM_NEON__ is set, as is
the case in Chrome for Android, as of M50.  Before, the default
OPENSSL_armcap_P value was getting in the way.

Arguably, this doesn't make a whole lot of sense. We're saying we'll let the
CPU run compiler-generated NEON code, but not our hand-crafted stuff. But, so
far, we only have evidence of the hand-written NEON tickling the bug and not
the compiler-generated stuff, so avoid the unintentional regression. (Naively,
I would expect the hand-crafted NEON is better at making full use of the
pipeline and is thus more likely to tickle the CPU bug.)

This is not the fix for M50, as in the associated Chromium bug, but it will fix
master and M51. M50 will instead want to revert
https://codereview.chromium.org/1730823002.

BUG=chromium:606629

Change-Id: I394f97fea2f09891dd8fa30e0ec6fc6b1adfab7a
Reviewed-on: https://boringssl-review.googlesource.com/7794
Reviewed-by: Adam Langley <agl@google.com>
2016-04-29 14:24:26 -04:00
3 changed files with 19 additions and 5 deletions
+6 -1
View File
@@ -288,6 +288,8 @@ static int has_broken_neon(const STRING_PIECE *cpuinfo) {
extern uint32_t OPENSSL_armcap_P;
static int g_has_broken_neon;
void OPENSSL_cpuid_setup(void) {
char *cpuinfo_data;
size_t cpuinfo_len;
@@ -317,7 +319,8 @@ void OPENSSL_cpuid_setup(void) {
}
/* Clear NEON support if known broken. */
if (has_broken_neon(&cpuinfo)) {
g_has_broken_neon = has_broken_neon(&cpuinfo);
if (g_has_broken_neon) {
hwcap &= ~HWCAP_NEON;
}
@@ -352,4 +355,6 @@ void OPENSSL_cpuid_setup(void) {
OPENSSL_free(cpuinfo_data);
}
int CRYPTO_has_broken_NEON(void) { return g_has_broken_neon; }
#endif /* OPENSSL_ARM && !OPENSSL_STATIC_ARMCAP */
-2
View File
@@ -79,8 +79,6 @@ uint32_t OPENSSL_armcap_P =
#endif
0;
#elif defined(__ARM_NEON__)
uint32_t OPENSSL_armcap_P = ARMV7_NEON;
#else
uint32_t OPENSSL_armcap_P = 0;
#endif
+13 -2
View File
@@ -110,13 +110,24 @@ OPENSSL_EXPORT char CRYPTO_is_NEON_capable_at_runtime(void);
/* CRYPTO_is_NEON_capable returns true if the current CPU has a NEON unit. If
* this is known statically then it returns one immediately. */
static inline int CRYPTO_is_NEON_capable(void) {
#if defined(__ARM_NEON__)
/* Only statically skip the runtime lookup on aarch64. On arm, one CPU is
* known to have a broken NEON unit which is known to fail with on some
* hand-written NEON assembly. For now, continue to apply the workaround even
* when the compiler is instructed to freely emit NEON code. See
* https://crbug.com/341598 and https://crbug.com/606629. */
#if defined(__ARM_NEON__) && !defined(OPENSSL_ARM)
return 1;
#else
return CRYPTO_is_NEON_capable_at_runtime();
#endif
}
#if defined(OPENSSL_ARM)
/* CRYPTO_has_broken_NEON returns one if the current CPU is known to have a
* broken NEON unit. See https://crbug.com/341598. */
OPENSSL_EXPORT int CRYPTO_has_broken_NEON(void);
#endif
/* CRYPTO_is_ARMv8_AES_capable returns true if the current CPU supports the
* ARMv8 AES instruction. */
int CRYPTO_is_ARMv8_AES_capable(void);
@@ -152,7 +163,7 @@ static inline int CRYPTO_is_ARMv8_PMULL_capable(void) {
}
#endif /* OPENSSL_STATIC_ARMCAP */
#endif /* OPENSSL_ARM */
#endif /* OPENSSL_ARM || OPENSSL_AARCH64 */
#if defined(__cplusplus)