Remove Knights Landing and Knights Mill logic
Intel has removed support for Knights Landing and Knights Mill chips
(Xeon Phi) from GCC, LLVM, and SDE. The last of which means we can no
longer test the special-case to optimize for them. These special cases
date to OpenSSL's 64d92d74985ebb3d0be58a9718f9e080a14a8e7f, which
describes it as a "salvage operation" because they have Silvermont-like
performance issues and "ADCX/ADOX instructions are effectively
mishandled at decode time".
In principle, this is not very much code to continue "salvaging" them,
but we don't like having code we cannot test, so follow Intel's lead in
removing all support. With this change, Xeon Phi chips should keep
working (assuming their CPUID reports capabilities accurately), but will
likely perform much worse.
This should unbreak the SDE builders on CI.
Change-Id: I00c3c435222fc53c1a6c9fddf961146f837dee7d
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/69187
Commit-Queue: Bob Beck <bbe@google.com>
Auto-Submit: David Benjamin <davidben@google.com>
Reviewed-by: Bob Beck <bbe@google.com>
diff --git a/crypto/cpu_intel.c b/crypto/cpu_intel.c
index f657e4c..347eadc 100644
--- a/crypto/cpu_intel.c
+++ b/crypto/cpu_intel.c
@@ -208,15 +208,6 @@
// Reserved bit #30 is repurposed to signal an Intel CPU.
if (is_intel) {
edx |= (1u << 30);
-
- // Clear the XSAVE bit on Knights Landing to mimic Silvermont. This enables
- // some Silvermont-specific codepaths which perform better. See OpenSSL
- // commit 64d92d74985ebb3d0be58a9718f9e080a14a8e7f and
- // |CRYPTO_cpu_perf_is_like_silvermont|.
- if ((eax & 0x0fff0ff0) == 0x00050670 /* Knights Landing */ ||
- (eax & 0x0fff0ff0) == 0x00080650 /* Knights Mill (per SDE) */) {
- ecx &= ~(1u << 26);
- }
} else {
edx &= ~(1u << 30);
}
@@ -251,12 +242,6 @@
extended_features[0] &= ~(1u << 16);
}
- // Disable ADX instructions on Knights Landing. See OpenSSL commit
- // 64d92d74985ebb3d0be58a9718f9e080a14a8e7f.
- if ((ecx & (1u << 26)) == 0) {
- extended_features[0] &= ~(1u << 19);
- }
-
OPENSSL_ia32cap_P[0] = edx;
OPENSSL_ia32cap_P[1] = ecx;
OPENSSL_ia32cap_P[2] = extended_features[0];
diff --git a/crypto/internal.h b/crypto/internal.h
index a45f97b..15facb1 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -1521,7 +1521,6 @@
// otherwise select. See chacha-x86_64.pl.
//
// Bonnell, Silvermont's predecessor in the Atom lineup, will also be matched by
-// this. |OPENSSL_cpuid_setup| forces Knights Landing to also be matched by
// this. Goldmont (Silvermont's successor in the Atom lineup) added XSAVE so it
// isn't matched by this. Various sources indicate AMD first implemented MOVBE
// and XSAVE at the same time in Jaguar, so it seems like AMD chips will not be
@@ -1530,11 +1529,12 @@
// WARNING: This MUST NOT be used to guard the execution of the XSAVE
// instruction. This is the "hardware supports XSAVE" bit, not the OSXSAVE bit
// that indicates whether we can safely execute XSAVE. This bit may be set
- // even when XSAVE is disabled (by the operating system). See the comment in
- // cpu_intel.c and check how the users of this bit use it.
+ // even when XSAVE is disabled (by the operating system). See how the users of
+ // this bit use it.
//
- // We do not use |__XSAVE__| for static detection because the hack in
- // |OPENSSL_cpuid_setup| for Knights Landing CPUs needs to override it.
+ // Historically, the XSAVE bit was artificially cleared on Knights Landing
+ // and Knights Mill chips, but as Intel has removed all support from GCC,
+ // LLVM, and SDE, we assume they are no longer worth special-casing.
int hardware_supports_xsave = (OPENSSL_get_ia32cap(1) & (1u << 26)) != 0;
return !hardware_supports_xsave && CRYPTO_is_MOVBE_capable();
}
diff --git a/util/all_tests.go b/util/all_tests.go
index 09c98b9..4090943 100644
--- a/util/all_tests.go
+++ b/util/all_tests.go
@@ -94,8 +94,6 @@
"clx", // Cascade Lake
"cpx", // Cooper Lake
"icx", // Ice Lake server
- "knl", // Knights landing
- "knm", // Knights mill
"tgl", // Tiger Lake
}
